Commit Graph

57 Commits

Author SHA1 Message Date
jangko 667cb6d750
EIP 4399 changes: random -> prevRandao 2022-02-27 14:37:35 +07:00
jangko 28cdfcaf6b
fix EIP-4399 'random' opcode
- fix previous implementation of EIP-4399
- now `random` opcode can be used with evmc_enabled
2022-02-08 20:23:40 +07:00
jangko d7f1d698ce EIP-4399 implementation of nim-vm2
new addition:
  - `RANDOM` opcode
  - `random` field of BlockHeader(previously `mixDigest`)
  - `PostMerge` temporary name of this new EVM version
2022-02-01 18:11:14 +02:00
Jamie Lokier 4b89ca3215
EVM: `writeContract` fixes, never return contract code as `RETURNDATA`
This fixes #867 "EIP-170 related consensus error at Goerli block 5080941", and
equivalent on other networks.

This combines a change on the EVM-caller side with an EVM-side change from
@jangko 6548ff98 "fixes CREATE/CREATE2's `returndata` bug", making the caller
EVM ignore any data except from `REVERT`.

Either change works by itself.  The reason for both is to ensure we definitely
comply with ambiguous EVMC expectations from either side of that boundary, and
it makes the internal API clearer.

As well as fixing a specific consensus issue, there are some other EVM logic
changes too: Refactored `writeContract`, how `RETURNDATA` is handled inside the
EVM, and changed behaviour with quirks before EIP-2 (Homestead).

The fix allows sync to pass block 5080941 on Goerli, and probably equivalent on
other networks.  Here's a trace at batch 5080897..5081088:

```
TRC 2021-10-01 21:18:12.883+01:00 Persisting blocks                  file=persist_blocks.nim:43 fromBlock=5080897 toBlock=5081088
...
DBG 2021-10-01 21:18:13.270+01:00 Contract code size exceeds EIP170  topics="vm computation" file=computation.nim:236 limit=24577 actual=31411
DBG 2021-10-01 21:18:13.271+01:00 gasUsed neq cumulativeGasUsed      file=process_block.nim:68 block=5080941/0A3537BC5BDFC637349E1C77D9648F2F65E2BF973ABF7956618F854B769DF626 gasUsed=3129669 cumulativeGasUsed=3132615
TRC 2021-10-01 21:18:13.271+01:00 peer disconnected                  file=blockchain_sync.nim:407 peer=<IP:PORT>
```

Although it says "Contract code size" and "gasUsed", this bug is more general
than either contract size or gas.  It's due to incorrect behaviour of EVM
instructions `RETURNDATA` and `RETURNDATASIZE`.

Sometimes when `writeContract` decides to reject writing the contract for any
of several reasons (for example just insufficient gas), the unwritten contract
code was being used as the "return data", and given to the caller.  If the
caller used `RETURNDATA` or `RETURNDATASIZE` ops, those incorrectly reported
the contract code that didn't get written.

EIP-211 (https://eips.ethereum.org/EIPS/eip-211) describes `RETURNDATA`:
> "`CREATE` and `CREATE2` are considered to return the empty buffer in the
> success case and the failure data in the failure case".

The language is ambiguous.  In fact "failure case" means when the contract uses
`REVERT` to finish.  It doesn't mean other failures like out of gas, EIP-170
limit, EIP-3541, etc.

To be thorough, and to ensure we always do the right thing with real EVMC when
that's finalised, this patch fixes the `RETURNDATA` issue in two places, either
of which make Goerli block 5080941 pass.

`writeContract` has been refactored to be caller, and so has where it's called.
It sets an error in the usual way if contract writing is rejected -- that's
anticipating EVMC, where we'll use different error codes later.

Overall four behaviour changes:

1. On the callee side, it doesn't set `c.outputData` except for `REVERT`.
2. On the caller side, it doesn't read `child.outputData` except for `REVERT`.
3. There was a bug in processing before Homestead fork (EIP-2).  We did not
   match the spec or other implementations; now we do.  When there's
   insufficient gas, before Homestead it's treated as success but with an empty
   contract.

   d117c8f3fd/ethereum/processblock.py (L304)
   https://github.com/ethereum/go-ethereum/blob/401354976bb4/core/vm/instructions.go#L586

4. The Byzantium check has been removed, as it's unnecessary.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-12-12 16:34:13 +07:00
jangko 908dc21478
evm: fixes EIP2929 opcodes
op balanceEIP2929, extCodeHashEIP2929, extCodeSizeEIP2929, and
extCodeCopyEIP2929 are fixed due to their wrong gasConsume
position
2021-09-22 11:58:06 +07:00
bmoo b09ad5cacb
code cleanup removed unused imports 2021-08-18 10:35:36 +07:00
Jamie Lokier 20e1831e6f
vm2: Use `ContractSalt` type for `CREATE2` salt
As this branch of vm2 doesn't support EVMC, this EVMC-motivated change is only
required here for internal compatibility.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-08-05 11:07:10 +01:00
Jamie Lokier ab9067133c
Tracing: Remove some trace messages that occur a lot during sync
Disable some trace messages which appeared a lot in the output and probably
aren't so useful any more, when block processing is functioning well at high
speed.

Turning on the trace level globally is useful to get a feel for what's
happening, but only if each category is kept to a reasonable amount.

As well as overwhelming the output so that it's hard to see general activity,
some of these messages happen so much they severely slow down processing.  Ones
called every time an EVM opcode uses some gas are particularly extreme.

These messages have all been chosen as things which are probably not useful any
more (the relevant functionality has been debugged and is tested plenty).

These have been commented out rather than removed.  It may be that turning
trace topics on/off, or other selection, is a better longer term solution, but
that will require better command line options and good defaults for sure.
(I think higher levels `tracev` and `tracevv` levels (extra verbose) would be
more useful for this sort of deep tracing on request.)

For now, enabling `--log-level:TRACE` on the command line is quite useful as
long as we keep each category reasonable, and this patch tries to keep that
balance.

- Don't show "has transactions" on virtually every block imported.
- Don't show "Sender" and "txHash" lines on every transaction processed.
- Don't show "GAS CONSUMPTION" on every opcode executed", this is way too much.
- Don't show "GAS RETURNED" and "GAS REFUND" on each contract call.
- Don't show "op: Stop" on every Stop opcode, which means every transaction.
- Don't show "Insufficient funds" whenever a contract can't call another.
- Don't show "ECRecover", "SHA256 precompile", "RIPEMD160", "Identity"
  or even "Call precompile" every time a precompile is called.  These are
  very well tested now.
- Don't show "executeOpcodes error" whenever a contract returns an error.
  (This is changed to `trace` too, it's a normal event that is well tested.)

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:55 +01:00
jangko 05d905b136
EIP-3529: Replace SSTORE_CLEARS_SCHEDULE
SSTORE_CLEARS_SCHEDULE or FeeSchedule[RefundsClear] in evm
have initial value of 15_000 when introduced by EIP-2200.

EIP-2200 also set new value for SSTORE_RESET_GAS
from 5000 to to 5000 - COLD_SLOAD_COST

Now with EIP-3529, SSTORE_CLEARS_SCHEDULE beecome
SSTORE_RESET_GAS + ACCESS_LIST_STORAGE_KEY_COST

or 5000 - COLD_SLOAD_COST + ACCESS_LIST_STORAGE_KEY_COST
of 5000 - 2100 + 1900 = 4800
2021-06-29 07:37:17 +07:00
jangko b51fad5fa7
EIP-3198: add baseFee op code in nim-evm2 2021-06-29 07:35:16 +07:00
jangko 5159ad7aac
preparation for London hard fork
This preparation is needed for subsequent
EIPs included in London.

- Add London to Fork enum
- Block number to fork
- Parsing London fork in chain config
- Prepare gas costs table for London
- Prepare EVM opcode dispatcher for London
- Block rewards for London
- Prepare hive script for London
2021-06-29 07:34:45 +07:00
Jamie Lokier 534be53873
vm2: Remove vm2 `forks_list` everywhere, use common forks list
Remove file vm2 file `forks_list, and divert all imports to the common forks
list outside the EVM.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 15:36:31 +01:00
Jamie Lokier 05bc174bef
Forks: Use a common fork list outside the EVMs
Many places outside the EVM use `Fork` and the fork list, and in general we
want progressively fewer dependencies on EVM internal types and files.

This may prove to be a temporary location, especially when we implement
issue #640.  But it's a fine temporary location if so.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 15:36:31 +01:00
Jamie Lokier e2689792b0
vm2: Remove `toSymbolName` unnecessary symbol-text-symbol conversion
There's no need for macro `toSymbolName` to convert fork enum values to their
presentation texts (logging etc) then re-parse them back to a fork enum value.
`asFork` is already used in the same function and works without these steps,
so use it consistently.

Same applies to `op.toSymbolName` and `asOp`.

This makes the code simpler, and removes a text pattern-matching requirement.
The patch has been checked to confirm it doesn't change the compiled code.

Motivation: The forks list will be removed from VM because it is used outside
the VM as well.  Doing so highlighted vm2's `toSymbolName`.  It's not needed,
and it's best if the VM doesn't constrain text strings used outside the VM

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 16:54:38 +01:00
jangko 3ccc4642f2
disable EIP-2315: Simple Subroutines for the EVM
reason: not included in berlin hard fork
2021-05-17 01:29:03 +07:00
jangko a2f77e8627
eip2718: rename vm2 ChainId op code to ChainIdOp
this is to avoid clash with ChainId type
imported from eth/common
2021-05-15 18:09:36 +07:00
Jordan Hrycaj 5d0d44c38f re-named compu_helper.nim => computation.nim
why:
  exports all except one of the original computation.nim functional
  objects
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 49afac46b7 move dispatcher case switch from interpterer_dispatcher.nim into separate file
why:
  insulate for improving maintenance
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 0a4c34f13b removed circular import dependencies
overview:
  can be verified by running "make check_vm2 X=0" in the nimbus directory
  (be patient when running it.) the X=0 flag is necessary if there is a
  native NIM compiler which may bail out at some vendor imports.

details:
  when compiling state_transaction.nim, the nim flag vm2_enabled must
  be set in order to avoid implicit import of native VM definitions.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj caabc9c292 removed kludge and simplified sources oph_call.nim and oph_create.nim
why:
  kludge not needed anymore for oph_handlers.nim sub-sources and sources
  that rely on oph_handlers.nim (but not state_transactions.nim which
  relies on computation.nim.)
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 73900270db extracted macros from oph_helpers.nim => oph_gen_handlers.nim
why:
  imports mostly need to import only one of either
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 45558282f7 merged oph_*_kludge.nim sources => single oph_kludge.nim 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 69a1ee5fc8 re-named some v2state_transactions.nim to its original name without the v2
also:
  re-integrated stack_defs.nim back into stack.nim

why:
  the v2 prefix of the file name was used as a visual aid when
  comparing vm2 against vm sources
2021-04-28 15:24:14 +03:00
Jordan Hrycaj e6eee3f3a6 prepared v2state_transaction.nim so it can be compiled locally
details:
  use the -d:kludge:1 flag for syntax check
2021-04-28 15:24:14 +03:00
Jordan Hrycaj ff6921eb1a re-named some v2*.nim sources to its original name *.nim (without the v2)
why:
  the v2 prefix of the file name was used as a visual aid when
  comparing vm2 against vm sources

details:
  all renamed v2*.nim sources compile locally with the -d:kludge:1 flag
  set or without (some work with either)

  only sources not renamed yet: v2state_transactions.nim
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 1b3117edbd re-implemented handler-call statement by doubly nested case statement
why:
  the new implementation lost more then 25% execution time on the test
  suite when compared to the original VM. so the handler call and the
  surrounding statements have been wrapped in a big case statement similar
  to the original VM implementation. on Linux/x64, the execution time of
  the new VM2 seems to be on par with the old VM.

details:
  on Linux/x64, computed goto works and is activated with the -d:release
  flag. here the execution time of the new VM2 was tested short of 0.02%
  better than the old VM. without the computed goto, it is short of
  0.4% slower than the old VM.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 3ed234e0a1 clean up cyclic-import-breaker function stubs where possible for op handlers
why:
  using function stubs made it possible to check the syntax of an op
  handler source file by compiling this very file. this was previously
  impossible due cyclic import/include mechanism.

details:
  only oph_call.nim, oph_create.nim and subsequently op_handlers.nim
  still need the -d:kludge:1 flag for syntax check compiling. this flag
  also works with interpreter_dispatch.nim which imports op_handlers.nim.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj a86bcefc7a re-named v2gas_costs.nim to its original name v2gas_costs.nim
why:
  the v2 prefix of the file name was used as a visual aid when
  comparing vm2 against vm sources
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 72b36e154b eliminated v2opcode_values, v2forks in favour of op_codes, forks_list 2021-04-28 15:24:14 +03:00
Jordan Hrycaj b2ce6d9e70 re-arrange functions from v2computation.nim and interpreter_dispatch.nim
why:
  step towards breaking circular dependency

details:
  some functions from v2computation.nim have been extracted into
  compu_helper.nim which does not explicitly back-import
  v2computation.nim. all non recursive op handlers now import this source
  file rather than v2computation.nim.

  recursive call/create op handler still need to import v2computation.nim.

  the executeOpcodes() function from interpreter_dispatch.nim has been
  moved to v2computation.nim which allows for <import> rather than
  <include> the interpreter_dispatch.nim source.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 9b70ab5f8f update handler prototype using call-by-reference argument
why:
  this allows for passing back information which can eventually be
  used for reducing use of exceptions

caveat:
  call/create currently needs to un-capture the call-by-reference
  (wrapper) argument using the Computation reference inside
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 06b34a4a56 remove obsolete files
why:
  opcodes_impl.nim was fully replaced by functionality from
  op_handlers.nim
2021-04-28 15:24:14 +03:00
Jordan Hrycaj b388e966cc simplify interpreter_dispatch.nim code
details:
  replace generated macro loop/switch by explicit call using the
  fork/op handler matrix (encapsulated via opHandlersRun() function)
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 51587208b4 verifying new op handler tables layout against original tables
why:
  the previous approach was replacing the function-lets in
  opcode_impl.nim by the particulate table handlers. the test
  functions will verify the the handler functions are sort of
  correct but not the assignments in the fork tables.

  the handler names of old and new for tables are checked here.

caveat:
  verifying tables currently takes a while at compile time.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 4e2af7937b clean up code for call handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 4ac32d360b re-integrated call op handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 1bdbfda37f re-integrated/added Create and Create2 handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 1bb6ef43a1 re-integrated handlers with op codes 0xf2/return ..0xff/selfdestruct 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 7436e516fd re-integrated/added EIP2929 handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 716bd64419 re-integrated 0x60..0xaf (push, dup, swap, log) op handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj f4bc9c4561 updated circular dependency klugde
why:
  removing <when> clauses and replacing gas calculation by stubs
  makes up for better reading of the code
2021-04-28 15:24:14 +03:00
Jordan Hrycaj d373ab6460 re-integrated 0x5# op handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj 0f1c7cee43 re-integrated 0x4# op handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj a9719f2dae added comments for better readability
and:
  reduced some import clause arguments
2021-04-28 15:24:14 +03:00
Jordan Hrycaj fb94aa8a35 re-integrated 0x3# op handlers 2021-04-28 15:24:14 +03:00
Jordan Hrycaj fda676062f integrated current op handlers into opcodes_impl.nim (tbc.)
why:
  integration tests will verify op code handlers which wher rephrased
  from the very opcodes_impl module into the handler tables.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 5e7d4ac9c5 experimental op handlers table (tbc.)
details:
  the op handler table is accessible via op_handlers.nim module

  op handler function implementations are found in the op_handlers/
  sub-directory

kludge:
  for development and pre-testing, any new module can be individually
  compiled setting the kludge flag using -d:kludge:1. this causes some
  proc/func replacements in turn allowing for omitting imports that would
  otherwise cause a circular dependency. otherwise individual compilation
  would fail.

  in order to prove the overall correctness of the code, the
  op_handlers.nim is imported by opcodes_impl.nim when compiling all,
  nimbus or test.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj f37591ca35 need to remove vm2_defined sentinel
why:
  subsequent development will compile sources as main without setting
  the vm2_enabled flag. also, the doc generator would fail an vm2 without
  setting the flag for the vm2 files.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 5c01b3548d extract forks definition (all but rename v2forks.nim)
why:
  new name forks_list.nim file name matches additional documentation
  file names.

details:
  v2forks.nim remains a hollowed out shell serving as interface file.
2021-04-28 15:24:14 +03:00
Jordan Hrycaj 579fed5010 manually rewrite op-codes
why:
  activate NIM comments needed re-write. as there is no advantage in using
  the macro replacing a few missing op-codes by "Nop##" name symbols, the
  macro wrapper has been removed.

details:
  when explicitly accessed by numeric value ##, missing Op enum entries
  result in a symbol name something like "Op ##".

  rather than implicitly using a macro to fix the op-codes list, missing
  entries are detected at compile time when a fatal exception is thrown.

  the static compile time check verifies that
    all op-codes 0 .. 255 are defined
    op code name/mnemonic has at least 2 chars and starts with a capital
    op code name/mnemonic is not NIM auto-generated (i.e. has a space)

  also, original '#' comments are exposed as doc comments '##'
2021-04-28 15:24:14 +03:00