Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

Add recent op codes #37

Open
eddyashton opened this issue Aug 9, 2019 · 4 comments
Open

Add recent op codes #37

eddyashton opened this issue Aug 9, 2019 · 4 comments
Labels
backlog WIBNIF, not on immediate roadmap compatibility Full compatibility with current spec/other implementations enhancement New feature or request

Comments

@eddyashton
Copy link
Member

Since the eEVM was first implemented, additional op codes have been added to the EVM spec (notably RETURNDATASIZE, RETURNDATACOPY, and REVERT). Currently contracts must be compiled explicitly targeting the Homestead spec, or they will produce unaccepted op codes. Adding support for these would make it possible to run more recent real-world example contracts, and to validate the eEVM against current external test sets.

@eddyashton eddyashton added the enhancement New feature or request label Aug 9, 2019
@eddyashton eddyashton added backlog WIBNIF, not on immediate roadmap compatibility Full compatibility with current spec/other implementations labels Aug 16, 2019
@jafri
Copy link
Contributor

jafri commented Feb 7, 2020

Would be great to see this!

The 5 opcodes missing in reference to the yellow paper are:
RETURNDATASIZE
RETURNDATACOPY
REVERT
STATICCALL
INVALID

@jafri
Copy link
Contributor

jafri commented Feb 12, 2020

@eddyashton did you have any ideas around REVERT? As a patricia merkle tree is not used here for committing and reverting, perhaps a changelog per "context" that stores the initial state of each account and KV "touched" would work better. In the case of a revert, this changelog would hold the minimum information required to reverse all state changes.

@eddyashton
Copy link
Member Author

@jafri I think the right way to handle REVERT (from eevm::Processor's perspective) is:

  • return ExitReason::reverted, either by throwing a special exception type or mimicking the halt-handler used by STOP
  • trust that the caller+store implementation do not consider these executed changes to be permanent

The latter point deserves some futher explanation:
A reversible changelog requires each action to be reversible, and this is a hard problem in the general case. Instead its simpler to treat the results of execution as proposed changes, and only apply these changes to a permanent representation if the execution succeeds. The implementation of eevm::Storage used in EVM-for-CCF gets this behaviour for free, since its backed by an ephemeral CCF transaction object that we only commit (to be seen by other transactions) if it succeeds; if it fails then our changes are discarded.

As a sketch of a similar setup in the style of SimpleStorage:

class RevertibleStorage : public Storage
{
  using StateMap = std::map<uint256_t, uint256_t>;
  StateMap  local_changes;
  StateMap& permanent_state;

public:
  RevertibleStorage(StateMap& ps) : permanent_state(ps) {}

  void store(const uint256_t& key, const uint256_t& value) override
  {
    local_changes[key] = value;
  }

  uint256_t load(const uint256_t& key) override
  {
    const auto local_it = local_changes.find(key);
    if (local_it != local_changes.end())
    {
      return e->second();
    }
    else
    {
      return permanent_state[key];
    }
  }

  ...

  void apply()
  {
    for (const auto& [k, v]: local_changes)
    {
      permanent_state[k] = v;
    }
  }
};

...

const eevm::ExecResult e = p.run(...);
if (e.er == eevm::ExitReason::returned || e.er == eevm::ExitReason::halted)
{
  revertible_state.apply();
}
// If this isn't true, then our permanent_state is unaffected and the changes are discarded when
// revertible_state goes out of scope.

A less intrusive way to get the same result is just to copy the global state before execution - then one holds the original state and the other can be executed on, if execution fails the latter is discarded.

This would be more difficult if we tracked gas, in which case all gas changes would be permanent/instantly applied, while the corresponding state changes may be discarded. Luckily we don't track gas at all in eEVM, so we don't have to worry about it.

Hope that helps, happy to answer any other questions you have.

@jafri
Copy link
Contributor

jafri commented Feb 17, 2020

@eddyashton that would work, will need local changes for Accounts and local changes for Storage

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backlog WIBNIF, not on immediate roadmap compatibility Full compatibility with current spec/other implementations enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants