Commit Graph

1148 Commits

Author SHA1 Message Date
Jamie Lokier b28396d10d
Sync: Add packet tracing to `eth/65` in a consistent format
The format is reasonably useful and not too large, when looking at the
behaviour of sync processes.  It doesn't try to show all the details of
packets, just something at a useful level of detail to see what's going on.
The consistent presentation has proven helpful too, e.g. when grepping.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:56 +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
Jamie Lokier c435409292
Sync: Move `blockchain_sync` code and use it with `eth/65`
Move `blockchain_sync.nim` from `nim-eth` to `nimbus-eth1`.

This lets `blockchain_sync` use the `eth/65` protocol to synchronise with more
modern peers than before.

Practically, the effect is the sync process runs more quickly and reliably than
before.  It finds usable peers, and they are up to date.

Note, this is mostly old code, and it mostly performs "classic sync", the
original Ethereum method.  Here's a summary of this code:

- It decides on a blockchain canonical head by sampling a few peers.
- Starting from block 0 (genesis), it downloads each block header and
  block, mostly in order.
- After it downloads each block, it executes the EVM transactions in that block
  and updates state trie from that, before going to the next block.
- This way the database state is updated by EVM executions in block order,
  and new state is persisted to the trie database after each block.

Even though it mentions Geth "fast sync" (comments near end of file), and has
some elements, it isn't really.  The most obvious missing part is this code
_doesn't download a state trie_, it calculates all state from block 0.
Geth "fast sync" has several parts:

1. Find an agreed common chain among several peers to treat as probably secure,
   and a sufficiently long suffix to provide "statistical economic consensus"
   when it is validated.
2. Perform a subset of PoW calculations, skipping forward over a segment to
   verify some of the PoWs according to a pattern in the relevant paper.
3. Download the state trie from the block at the start of that last segment.
4. Execute only the blocks/transactions in that last segment, using the
   downloaded state trie, to fill out the later states and properly validate the
   blocks in the last segment.

Some other issues with `blockchain_sync` code:

- If it ever reaches the head of the chain, it doesn't follow new blocks with
  increasing block numbers, at least not rapidly.
- If the chain undergoes a reorg, this code won't fetch a block number it has
  already fetched, so it can't accept the reorg.  It will end up conflicted
  with peers. This hasn't mattered because the development focus has been on
  the bulk of the catching up process, not the real-time head and reorgs.
- So it probably doesn't work correctly when it gets close to the head due to
  many small reorgs, though it might for subtle reasons.
- Some of the network message handling isn't sufficiently robust, and it
  discards some replies that have valid data according to specification.
- On rare occasions the initial query mapping block hash to number can
  fail (because the peer's state changes).
- It makes some assumptions about the state of peers based on their responses
  which may not be valid (I'm not convinced they are).  The method for working
  out "trusted" peers that agree a common chain prefix is clever.  It compares
  peers by asking each peer if it has the header matching another peer's
  canonical head block by hash.  But it's not clear that merely knowing about a
  block constitutes agreement about the canonical chain.  (If it did, query by
  block number would give the same answer more authoritatively.)

Nonetheless, being able to run this sync process on `eth/65` is useful.

<# interactive rebase in progress; onto 66532e8a

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:53 +01:00
Jamie Lokier 936a18b4f4
Eth65: Rename and stop exporting `protocolVersion`
This constant shouldn't be used outside `protocol_eth65`.

When we support multiple `eth/NN` versions side by side, or even just have
multiple code files, there's a risk some code would import just one of the
files (e.g. `protocol_eth65`), use `protocolVersion`, and incorrectly act as
though that version is the one active on the node.

In fact that happened, and now it can't happen.  Other code needs to query the
`EtheruemNode` to find what versions are really active.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:52 +01:00
Jamie Lokier 5ddfd7dd05
RPC: Calculate `eth_protocolVersion` from live protocol
In RPC `eth_protocolVersion`, look at the live `EthereumNode` to find which
version of `eth/NN` protocol is active, instead of trusting a compile-time
constant.  It's better to check dynamically.  GraphQL already does this.

As a result, the RPC code doesn't depend on `eth_protocol` any more.

To make sure there are no more accidental users of the old constant,
`protocolVersion` is no longer exported from `protocol_eth65`.

(The simplest way to support `eth/65` was to make `eth_protocolVersion` use
`protocol_eth65.protocolVersion`, to get 65.  But that's silly.  More
seriously, when we add another version (`eth/66`) running alongside `eth/65`,
that expression would still compile ok yet return the wrong value, while still
passing the RPC test suite.)

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:51 +01:00
Jamie Lokier bb282a5348
RPC: Add comment about `eth_protocolVersion` expectations
While looking at the RPC `eth_protocolVersion` to see exactly what it should
report when there are multiple `eth/NN` versions supported by a node, I found
some differences in the documentation:

- The old Ethereum wiki documents it as returning a decimal string.

- But Infura documents it as returning 0x-prefixed hex string.

- Geth 1.10.0 has removed this call entirely "as it makes no sense".

https://eth.wiki/json-rpc/API#eth_protocolversion
https://infura.io/docs/ethereum/json-rpc/eth-protocolVersion
https://blog.ethereum.org/2021/03/03/geth-v1-10-0/#compatibility
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:49 +01:00
Jamie Lokier 00647b373c
Sync: Switch other modules over to the `eth/65` module
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:41 +01:00
Jamie Lokier 889a796b72
Sync: Fix `NewBlockAnnounce` RLP decoding errors
Turns out `{.rlpInline.}` doesn't do anything.
It's documented but not implemented.

Due to this, whenever a peer sent us a `NewBlock` message, we had an RLP
decoding error processing it, and disconnected the peer thinking it was the
peer's error.

These messages are sent often by good peers, so whenever we connected to a
really good peer, we'd end up disconnecting from it due to this.

Because a block body is a list of transactions, the parse errors looked
suspiciously like EIP-2718/2976/2930/1559 typed transaction RLP errors.
But it was a failure to parse `BlockBody` inline.

Conveniently, the `EthBlock` type defined for another reason is encoded exactly
the way `NewBlockAnnounce` needs to be, so we can reuse that type.

This didn't stand out before updating to `eth/65`, because with old protocols
we tend to only connect to old peers, which may be out of date themselves and
have no new blocks to send.  Also, we didn't really investigate occasional
disconnects before, we assumed they're just part of P2P life.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:38 +01:00
Jamie Lokier 3161d395a6
Sync: Support for `eth/65` protocol
This patch adds the `eth/65` protocol, documented at
https://github.com/ethereum/devp2p/blob/master/caps/eth.md.

This is an intentionally simple patch, designed to not break, change or depend
on other functionality much, so that the "_old_ sync" methods can be run
usefully again and observed.  This patch isn't "new sync" (a different set of
sync algorithms), but it is one of the foundations.

For a while now Nimbus Eth1 only supported protocol `eth/63`.  But that's
obsolete, and very few nodes still support it.  This meant Nimbus Eth1 would
make slow progress trying to sync, as most up to date clients rejected it.

The current specification is `eth/66`, and the functionality we really need is
in `eth/64`.

So why `eth/65`?

- `eth/64` is essential because of the `forkId` feature.  But `eth/64` is on
  its way out as well.  Many clients, especially the most up to date Geth
  running the current hard-forks (Berlin/London) don't talk `eth/64` any more.

- `eth/66` is the current specification, but some clients don't talk `eth/66`
  yet.  We'd like to have the best peer connectivity during tests, and
  currently everything that talks `eth/66` also talks `eth/65`.

- Nimbus Eth1 RLPx only talks one version at a time.  (Without changes to the
  RLPx module.  When those go in we'll add `eth/64..eth/66` for greater peer
  reach and testing the `eth/66` behaviour.  For simplicity and decoupling,
  this patch contains just one version, the most useful.)

What are `eth/64` and `eth/65`?

- `eth/64` (EIP-2364) added `forkId` which allows nodes to distinguish between
  Ethereum (ETH) and Ethereum Classic (ETC) blockchains, which share the same
  genesis block.  `forkId` also protects the system when a new hard fork is
  going to be rolled out, by blocking interaction with out of date nodes.  The
  feature is required nowadays.

  We send the right details to allow connection (this has been tested a lot),
  but don't apply the full validation rules of EIP-2124/EIP-2364 in this patch.
  It's to keep this patch simple (in its effects) and because those rules have
  consequences best tested separately.  In practice the other node will reject
  us when we would reject it, so this is ok for testing, as long as it doesn't
  get seriously deployed.

- `eth/65` added more efficient transaction pool methods.

- Then a later version of `eth/65` (without a new number) added typed
  transactions, described in [EIP-2976](https://eips.ethereum.org/EIPS/eip-2976).

Why it's moved to `nimbus-eth1`:

- Mainly because `eth/64` onwards depend on the current state of block
  synchronisation, as well as the blockchain's sequence of hard-fork block
  numbers, both of which are part of `nimbus-eth1` run-time state.  These
  aren't available to pure `nim-eth` code.  Although it would be possible to
  add an API to let `nimbus-eth1` set these numbers, there isn't any point
  because the protocol would still only be useful to `nimbus-eth1`.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-07-27 14:12:35 +01:00
Jordan Hrycaj a0d0e35a70
Renamed source file clique_utils => clique_helpers (#762)
* Renamed source file clique_utils => clique_helpers

why:
  New name is more in line with other modules where local libraries
  are named similarly.

* re-implemented PoA verification module as clique_verify.nim

details:
  The verification code was ported from the go sources and provisionally
  stored in the clique_misc.nim source file.

todo:
  Bring it to life.

* re-design Snapshot descriptor as: ref object

why:
  Avoids some copying descriptor objects

details:
  The snapshot management in clique_snapshot.nim has been cleaned up.

todo:
  There is a lot of unnecessary copying & sub-list manipulation of
  seq[BlockHeader] lists which needs to be simplified by managing
  index intervals.

* optimised sequence handling for Clique/PoA

why:
  To much ado about nothing

details:
  * Working with shallow sequences inside PoA processing avoids
    unnecessary copying.
  * Using degenerate lists in the cliqueVerify() batch where only the
    parent (and no other ancestor) is needed.

todo:
  Expose only functions that are needed, shallow sequences should be
  handles with care.

* fix var-parameter function argument

* Activate PoA engine -- currently proof of concept

details:
  PoA engine is activated with newChain(extraValidation = true) applied
  to a PoA network.

status and todo:
  The extraValidation flag on the Chain object can be set at a later
  state which allows to pre-load parts of the block chain without
  verification. Setting it later will only go back the block chain to
  the latest epoch checkpoint. This is inherent to the Clique protocol,
  needs testing though.

  PoA engine works in fine weather mode on Goerli replay. With the
  canonical eip-225 tests, there are quite a few fringe conditions
  that fail. These can easily fudged over to make things work but need
  some more work to understand and correct properly.

* Make the last offending verification header available

why:
  Makes some fringe case tests work.

details:
  Within a failed transaction comprising several blocks, this
  feature help to identify the offending block if there was a
  PoA verification error.

* Make PoA header verifier store the final snapshot

why:
  The last snapshot needed by the verifier is the one of the parent but
  the list of authorised signer is derived from the current snapshot. So
  updating to the latest snapshot provides the latest signers list.

details:
  Also, PoA processing has been implemented as transaction in
  persistBlocks() with Clique state rollback.

  Clique tests succeed now.

* Avoiding double yields in iterator => replaced by template

why:
  Tanks to Andri who observed it (see #762)

* Calibrate logging interval and fix logging event detection

why:
  Logging interval as copied from Go implementation was too large and
  needed re-calibration. Elapsed time calculation was bonkers, negative
  the wrong way round.
2021-07-21 14:31:52 +01:00
Kim De Mey e37bafd47e
Add a first, simple, content network test (#760)
* Add a first, simple, content network test

* Use sqlite as nimbus_db_backend for fluffy tests

* Fix backend selection for sqlite
2021-07-15 15:12:33 +02:00
Jordan Hrycaj cfe955c962
Feature/implement poa processing (#748)
* re-shuffled Clique functions

why:
  Due to the port from the go-sources, the interface logic is not optimal
  for nimbus. The main visible function is currently snapshot() and most
  of the _procurement_ of this function result has been moved to a
  sub-directory.

* run eip-225 Clique test against p2p/chain.persistBlocks()

why:
  Previously, loading the test block chains was fugdged with the purpose
  only to fill the database. As it is now clear how nimbus works on
  Goerli, the same can be achieved with a more realistic scenario.

details:
  Eventually these tests will be pre-cursor to the reply tests for the
  Goerli chain supporting TDD approach with more simple cases.

* fix exception annotations for executor module

why:
  needed for exception tracking

details:
  main annoyance are vmState methods (in state.nim) which potentially
  throw a base level Exception (a proc would only throws CatchableError)

* split p2p/chain into sub-modules and fix exception annotations

why:
  make space for implementing PoA stuff

* provide over-loadable Clique PRNG

why:
  There is a PRNG provided for generating reproducible number sequences.
  The functions which employ the PRNG to generate time slots were ported
  ported from the go-implementation. They are currently unused.

* implement trusted signer assembly in p2p/chain.persistBlocks()

details:
  * PoA processing moved there at the end of a transaction. Currently,
   there is no action (eg. transaction rollback) if this fails.
  * The unit tests with staged blocks work ok. In particular, there should
    be tests with to-be-rejected blocks.
  * TODO: 1.Optimise throughput/cache handling; 2.Verify headers

* fix statement cast in pool.nim

* added table features to LRU cache

why:
  Clique uses the LRU cache using a mixture of volatile online items
  from the LRU cache and database checkpoints for hard synchronisation.
  For performance, Clique needs more table like features.

details:
  First, last, and query key added, as well as efficient random delete
  added. Also key-item pair iterator added for debugging.

* re-factored LRU snapshot caching

why:
  Caching was sub-optimal (aka. bonkers) in that it skipped over memory
  caches in many cases and so mostly rebuild the snapshot from the
  last on-disk checkpoint.

details;
  The LRU snapshot toValue() handler has been moved into the module
  clique_snapshot. This is for the fact that toValue() is not supposed
  to see the whole LRU cache database. So there must be a higher layer
  working with the the whole LRU cache and the on-disk checkpoint
  database.

also:
  some clean up

todo:
  The code still assumes that the block headers are valid in itself. This
  is particular important when an epoch header (aka re-sync header) is
  processed as it must contain the PoA result of all previous headers.

  So blocks need to be verified when they come in before used for PoA
  processing.

* fix some snapshot cache fringe cases

why:
  Must not index empty sequences in clique_snapshot module
2021-07-14 16:13:27 +01:00
Jordan Hrycaj fbff3aea68
Feature/goerli replay clique poa (#743)
* extract unused clique/mining support into separate file

why:
  mining is currently unsupported by nimbus

* Replay first 51840 transactions from Goerli block chain

why:
  Currently Goerli is loaded but the block headers are not verified.
  Replaying allows real data PoA development.

details:
  Simple stupid gzipped dump/undump layer for debugging based on
  the zlib module (no nim-faststream support.)

  This is a replay running against p2p/chain.persistBlocks() where
  the data were captured from.

* prepare stubs for PoA engine

* split executor source into sup-modules

why:
  make room for updates, clique integration should go into
  executor/update_poastate.nim

* Simplify p2p/executor.processBlock() function prototype

why:
  vmState argument always wraps basicChainDB

* split processBlock() into sub-functions

why:
  isolate the part where it will support clique/poa

* provided additional processTransaction() function prototype without _fork_ argument

why:
  with the exception of some tests, the _fork_ argument is always derived
  from the other prototype argument _vmState_

details:
  similar situation with makeReceipt()

* provide new processBlock() version explicitly supporting PoA

details:
  The new processBlock() version supporting PoA is the general one also
  supporting non-PoA networks, it needs an additional _Clique_ descriptor
  function argument for PoA state (if any.)
  The old processBlock() function without the _Clique_ descriptor argument
  retorns an error on PoA networgs (e.g. Goerli.)

* re-implemented Clique descriptor as _ref object_

why:
  gives more flexibility when moving around the descriptor object

details:
  also cleaned up a bit the clique sources

* comments for clarifying handling of Clique/PoA state descriptor
2021-07-06 14:14:45 +01:00
jangko 8482cb3ed3
EIP-3541: Fixes typo, 0xFE -> 0xEF 2021-06-30 20:44:34 +07:00
jangko 697b38b844
EIP-3529: Replace SSTORE_CLEARS_SCHEDULE in evmc host_service 2021-06-30 20:35:10 +07:00
jangko db8988fe64
EIP-1559: Fee market change for ETH 1.0 chain
Transaction and BlockHeader already updated in nim-eth repo
to support EIP-1559

EIP-1559 header validation and gasLimit validation
already implemented in previous commit

This commit deals with block validation:
- Effective gasPrice per EIP-1559
- new miner reward based on priorityFee
2021-06-30 20:30:39 +07:00
jangko 7600046a11
EIP-1559: unify PoA and PoW gasLimit and baseFee validation
Turn out both EthHash and Clique are using the same gasLimit
validation.

They also share the same EIP-1559 baseFee validation.
2021-06-30 20:21:45 +07:00
KonradStaniec 32e57a6aa1
[FIX] Add missing gas used validation (#740) 2021-06-30 11:42:55 +02:00
jangko 472e4457e3
EIP-3529: Reduce the max gas refunded after a transaction
Previously max gas refunded was defined as gas_used div 2.
Here we name the constant 2 as MAX_REFUND_QUOTIENT and
change its value to 5.

The new equation will be: gas_used div MAX_REFUND_QUOTIENT
2021-06-29 07:37:17 +07: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 8982e6c649
EIP-3529: Remove the SELFDESTRUCT refund.
- remove it from both nim-evm and nim-evm2
2021-06-29 07:37:17 +07:00
jangko e08c9ef2d9
EIP-3541: Reject new contracts starting with the 0xEF byte 2021-06-29 07:36:56 +07:00
jangko b4221381d6
EIP-3554: Difficulty Bomb Delay to December 2021 2021-06-29 07:36:41 +07:00
jangko b51fad5fa7
EIP-3198: add baseFee op code in nim-evm2 2021-06-29 07:35:16 +07:00
jangko 05e9b891f0
EIP-3198: add baseFee op code in nim-evm 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
jangko 4a188788bd
preparation for EIP-1559 implementation
- unify signTx in test_helper and signTransaction in rpc_utils
  and put it into transaction.nim
- clean up mess by previous EIP-2930
2021-06-29 07:33:48 +07:00
Jordan Hrycaj a49a812879
Jordan/fix some failing nohive tests (#727)
* continue importing rlp blocks

why:
  a chain of blocks to be imported might have legit blocks
  after rejected blocks

details:
  import loop only stops if the import list is exhausted or if there
  was a decoding error. this adds another four to the count of successful
  no-hive tests.

* verify DAO marked extra data field in block header

why:
  was ignored, scores another two no-hive tests

* verify minimum required difficulty in header validator

why:
  two more nohive tests to succeed

details:
  * subsumed extended header tests under validateKinship() and renamed it
    more appropriately validateHeaderAndKinship()
  * enhanced readability of p2p/chain.nim
  * cleaned up test_blockchain_json.nim

* verify positive gasUsed unless no transactions

why:
  solves another to nohive tests

details:
  straightened test_blockchain_json chech so there is no unconditional
  rejection anymore (based on the input test  scenario)
2021-06-24 16:29:21 +01:00
Jordan Hrycaj 2d6bf34175
Re-adjust canonical head to parent of block to be inserted (#726)
* Re-adjust canonical head to parent of block to be inserted

why:
  of the failing tests that remain to be solved, 30 of those will succeed
  if the canonical database chain head is cleverly adjusted -- yes, it
  looks like a hack, indeed.

details:
  at the moment, this hack works for the non-hive tests only and is
  triggered by a boolean argument passed on to the chain.persistBlocks()
  method.

* Use parent instead of canonical head for block to be inserted

why:
  side chains need to be inserted typically somewhere before the
  canonical head.

details:
  the previous _hack_ was unnecessary and removed, it was inspired by
  some verification in persistBlocks() which explicitly referenced the
  canonical head (which now might or might not refer to the newly inserted
  header.)

* remove unnecessary code + comment
2021-06-22 17:52:31 +01:00
Jordan Hrycaj cad1b5a678
verify age of uncle's parent (#719)
why:
  parent must be older => check needed for bcFrontierToHomestead test
  cases UncleFromFrontierInHomestead and UnclePopulation
2021-06-18 08:37:59 +01:00
jangko d0782cdb0d
fixes some of graphql resolver
following recent fixes in upstream hive,
we also update our graphql resolvers
2021-06-17 18:18:28 +07:00
Jordan Hrycaj eb7c0be3d4
after rebase fix (#715)
why:
  file name has changed
2021-06-17 09:17:49 +01:00
Jordan Hrycaj 82e6cd991d maintenance update
why:
  some handy features were intended to support the unit test from
  the clique/clique_test.go source (the other one is from
  clique/snapshot_test.go.)
  as this test cannot realistically be implemented without the full
  api (includes mining support), it is left as that
2021-06-17 08:03:57 +01:00
Jordan Hrycaj 90b012ad3f clarify epoch sync handling (effectively a comment update only)
why:
  autorisation list verification is performed in the main module along
  with other header verifications
2021-06-17 08:03:57 +01:00
Jordan Hrycaj dd7ca174f0 all snapshot unit tests succeed
details:
  for extra verbosity compile as: nim c -r -d:debug [..] test_clique.nim
2021-06-17 08:03:57 +01:00
Jordan Hrycaj 61e460c125 Most snapshot unit tests work
details:
  three test cases still fail which are skipped
  test suite is linked to all_tests list
2021-06-17 08:03:57 +01:00
Jordan Hrycaj 87edd80557 Update snapshot smoke test
details:
  can initialise & load all tests

todo:
  double check tests that are supposed to return error
  follow up succesful voting results
2021-06-17 08:03:57 +01:00
Jordan Hrycaj 1de2cc1a77 Basic tests for Clique PoA/Consensus engine
details:
  test scenario from eip-225 reference implementation,
  set up unittes2 test framework
  smoke test for first sample ok (not functional yet)
2021-06-17 08:03:57 +01:00
Jordan Hrycaj 491149c6d5 Eip225 clique/PoA consensus protocol
details:
  formal port from go-lang sources, compiles but will not do anything
  useful yet
2021-06-17 08:03:57 +01:00
Jamie Lokier 6d4205b0b0
Transaction: Just enough support to work with nested calls
Proper nested call functionality is being skipped in this iteration of new EVMC
host code to keep it simpler, to allow testing and architecture to be built
around the less complicated non-nested cases first.

Instead, nested calls use the old `Computation` path, and bypass any
third-party EVM that may be loaded.  The results are the same, and mixing
different EVMs in this way is actually permitted in the EVMC specification.

This approach also means third-party EVMs we test don't need to support
precompiles and we don't need to specially handle those cases.
(E.g. "evmone" doesn't support precompiles, just EVM1 opcodes).

(These before/after scope actions are approximately copy-pasted from
`nimbus/vm/evmc_host.nim`, making their detailed behaviour "obviously correct".
Of course they are subject to tests as well.  The small stack property of
a3c8a5c3 "EVMC: Small stacks when using EVMC, closes #575 (segfaults)" is
carefully retained.)

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:41 +01:00
Jamie Lokier 97b4aa5619
Transaction: Calculate EIP-1283/2200/2929 gas refund in `setStorage`
Make the host service `setStorage` calculate the gas refund itself, instead of
depending on EVM internals.

In EVMC, host `setStorage` is responsible for adding the gas refund using the
rules of EIP-1283 (Constantinople), EIP-2200 (Istanbul) or EIP-2929 (Berlin).

It is not obvious that the host must do it from EVMC documentation, but it's
how it has to be.  Note, this is very different from the gas _cost_, which the
host does not calculate.

Gas refund doesn't affect computation.  It is applied after the whole
transaction is complete, so it can be tracked on the host or EVM side.  But
`setStorage` doesn't return enough information for the EVM to calculate the
refund, so the host must do it when `setStorage` is used.

For now, this continues using Nimbus `Computation` just to hold the gas refund,
to fit around existing structures and get new EVMC working.  But the host can't
keep using `Computation`, so gas refund will be moved out of it in due course.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:40 +01:00
Jamie Lokier eb52fd3906
Transaction: Make `emitLog` use `Computation` to pass tests
When processing log operations on the EVMC host side, it causes incorrect
`rootHash` results in some tests.  This patch fixes the results.

The cause of these results is known: `Computation` is still doing parts of
contract scope entry/exit which need to be moved to the host.  For now, as a
temporary workaround, update logs in `Computation` as it did before.

This makes test pass when using Nimbus EVM.  (It breaks third-party EVMs when
`LOG*` ops are used, although most other tests pass.)

We can't keep this as it prevents complete host/EVM separation, but it's useful
in the current code, and it's fine to develop other functionality on top.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:40 +01:00
Jamie Lokier ce0c13c4ca
Transaction: Make `selfDestruct` use `Computation` to pass tests
When processing self destructs on the EVMC host side, it causes incorrect
`rootHash` results in some tests.  This patch fixes the results.

The cause of these results is known: `Computation` is still doing parts of
contract scope entry/exit which need to be moved to the host.  For now, as a
temporary workaround, update self destructs in `Computation` as it did before.

This makes test pass when using Nimbus EVM.  (It breaks third-party EVMs when
`SELFDESTRUCT` ops are used, although most other tests pass.)

We can't keep this as it prevents complete host/EVM separation, but it's useful
in the current code, and it's fine to develop other functionality on top.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:40 +01:00
Jamie Lokier b734240291
Transaction: Fix bounds error getting data address in empty seq
In the unusual case where log data is zero-length, `data[0].addr` is invalid
and Nim thoughtfully raises `IndexOutOfBounds`, a `Defect` so it's not even
in `CatchableError`.

This is done in the EVMC host services to handle `LOG*` ops, and it made one
of the EVM tests silently fail with no error message.  The fix is obvious.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:40 +01:00
Jamie Lokier a5dc0bd283
EVMC: Using `{.show.}` trace all calls from the host into the EVM
Show calls from the host into the EVM.  Shows the call, `evmc_message` fields,
and `evmc_result` fields when the call returns.

(When `show_tx_calls` is manually set to true.)

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:40 +01:00
Jamie Lokier ffd34a69fe
EVMC: Using `{.show.}` trace all calls from EVM to host services
When `show_tx_calls` is manually set to true, show all the calls from the EVM
to the host, including name, arguments and results.

For example this shows each call to `setStorage`, the key, value and storage
result.  This output allows the externally-visible activity of an EVM to be
seen, and it's been useful for guessing what went wrong when a test fails.

In theory, if two EVMs show the same activity in this log, they should have the
same effect on account states, gas, etc. and the same final `roothash`
(which is the only value some tests check).

ps. Ideally we'd use `{.push show.}`...`{.pop.}`, just like with `inline`.
But we can't: https://github.com/nim-lang/Nim/issues/12867

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:40 +01:00
Jamie Lokier 43b66a3a05
EVMC: `{.show.}` pragma to show EVMC host call arguments and results
New pragma `{.show.}` on a proc definition turns on tracing for that proc.
Every call to it shows the name, arguments and results, if `show_tx_calls` is
manually set to true.  This is to trace calls from EVM to host.

This started as a template which took a block expression, but the closure it
used led to illegal capture errors.  It was easier to write a macro.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:40 +01:00
Jamie Lokier 0e2bc8408d
Transaction: Run all computations via EVMC `execute`
1. Send all EVM executions through the EVMC `execute` function.

   It leads to the same place in the end as calling `Computation` before, but
   `execute` is the API function used by all EVMC implementations, and it is
   very explicit what data is passed back and forth.

2. As a consequence this starts using the new `host_services` code from EVM, so
   this is a significant change to the paths used for account state processing.

3. Because we will have to remove the `newComputation` call on the host side,
   anticipating that the contract code is now saved in `host` instead of being
   copied around.  As it's saved in `host`, there is no need to pass it
   separately to `evmcExecComputation`.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:39 +01:00
Jamie Lokier df71c8bec9
EVMC: Disable byte-endian conversion of 256-bit values on EVM side
We'll re-enable endian conversions based on a negotiated run-time option later,
but for now let's remove one complication to testing the new EVMC paths, and
also gain a little performance.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:39 +01:00
Jamie Lokier e8a79e7246
EVMC: Reference EVMC entry point so it's linked into the program
Even though `evmc_create_nimbus_evm` is called, it fails at link time because
the definition of that function isn't included unless it is pulled in
explicitly.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:39 +01:00
Jamie Lokier 10807bce39
EVMC: Binary compatibility on the VM side for calling `execute`
This provides the functions a loadable VM must provide for a host to use it.
The main access to a loaded EVM is `evmc_create_nimbus_evm`, and this meant to
be the only exported function the caller starts with.

That provides access to other functions, also defined in this patch, to
configure the EVM and then the key interesting function is `execute`.

`execute` runs a full computation, here using Nimbus EVM `Computation`.

(Note, even though everything is EVMC binary-compatible, there is a small
dependency on `TransactionHost` in `execute` here, which prevents this being
used by a host that is not Nimbus at the moment.  It is necessary for some
tests, and will eventually go away.)

Although this provides the VM-side functionality needed by the host, it does
not contain the glue functions for `Computation` to call the host, which are
already part of the Nimbus EVM in `nimbus/vm/evmc_api.nim`.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:39 +01:00
Jamie Lokier 0b19f42158
EVMC: Binary compatibility glue on the host side
1. This provides the necessary type adjustments for host services to be
   (optionally) callable via the EVMC binary-compatible interface.  This layer
   is stashed away in a glue module so the host services continue to use
   appropriate Nim types, and are still callable directly.

   Inlining is used to ensure there should be no real overhead, including stack
   frame size for the `call` function.  Note, `import` must be used for
   `{.inline.}` to work effectively.

2. This also provides a key call in the other direction, the version of host to
   EVM `execute` that is called on the host side.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:39 +01:00
Jamie Lokier d49fa0bb86
Transaction: Add "host services", accessors to host state from EVM
This provides "host services", functions provided by the application to an EVM.
They are a key part of EVMC compatibility, but we will switch to using these
with "native" EVM as well.

These are functions like `getStorage`, `setStorage` and `emitLog` for accessing
the account state, because the EVM is not allowed direct access to the database.

This code is adapted from `nimbus/vm/evmc_host.nim` and other places, but there
is more emphasis on being host-side only, no dependency on the EVM or
`Computation` type.  It uses `TransactionHost` and types in `host_types`.

These host services have two goals: To be compatible with EVMC, and to be a
good way for the Nimbus EVM to access the data it needs.  In our new Nimbus
internal architecture, the EVM will only access the databases and other
application state via these host service functions.

The reason for containing the EVM like this, even "native" EVM, is that having
one good interface to the data makes it a lot easier to change how the database
works, which is on the roadmap.

These functions almost have EVMC signatures, but they are not binary compatible
with EVMC.  (Binary compatibility is provided by another module).  It would be
fine for Nimbus EVM to call these functions directly when linked directly.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:29:39 +01:00
Jamie Lokier a4f92a6543
Transaction: Use same log level for all block rejection causes
Block validation failure isn't an error, it's correct rejection of a bad block
from the network.  All conditions that lead to block rejection return a simple
boolean.

When a block is rejected, most reasons log at `debug` level.  Only `stateRoot`
mismatch shouts a loud, highlighted, multi-line error message with big red
`error` alert.

Historically this was to assist EVM development, because it was more likely to
be a Nimbus EVM bug than a real bad block.  But now the EVM is in good shape,
has a large and thorough testsuite, and `stateRoot` mismatch is more likely to
be a real bad block that should be rejected with less fuss.

If there's a genuine EVM bug, we'll still get an alert: Consensus failure will
quickly become obvious, and the block where it happens is easily fetched.

So a big, loud error is no longer useful, and it became a problem during tests.
Recently a few hundred tests were added that trigger it, and now successful
test output is filled with attention-grabbing errors which aren't really errors
or particularly useful.

Since it's not really an error, the original motivation is now backwards, and
other reasons warn at `debug` level, make this like the others.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 18:07:50 +01: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 7c90d8de70
EVM: Remove `vm_forks` everywhere, use common forks list instead
The common forks list was already used, redirected via `vm_forks` for
historical compatibility.  Remove the old `vm_forks` now 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 aee0fe39d2
EVM: Remove `vm_types2` everywhere, use common forks list instead
File `vm_types2` is obsolete.  Remove this file and divert all imports to the
common forks list outside the EVM, or in some cases they don't need it anyway.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 15:36:31 +01:00
Jamie Lokier 84269ddccf
Forks: Use capitalized names again for presentation (logging etc.)
Fork names were originally capitalized, and were made lower case by @narimiran
in commit 36a7519 to satisfy `parseEnum` in some tests.  Restore the
capitalization and make the tests work with it.

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 b3a788c7ce
Transaction: Move contract address generation outside the EVM
The current EVM generates its own new contract addresses, and this is why there
are separate `msg.contractAddress` and `msg.codeAddress` fields in the
computation start message.

In EVMC, account updates are only allowed on the host side, including contract
generation, and the start message has one destination field, `msg.destination`.
The EVM cannot select addresses, only use them.  It's a sensible design.

The difference makes the current EVM incompatible with EVMC and its message
format, so this patch corrects the difference.  It moves contract address
generation to the host side.  This simplifies the EVM and its API a little.

(As an API change, this is incompatible with vm2, so it's guarded under
`evmc_enabled` to allow vm2 to continue to build and run at this time.  This is
also why there are fewer deletions than would otherwise be expected.)

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 15:36:30 +01:00
Jamie Lokier 775231eef1
EVM: Apply EIP-6 in the code (affects both vm and vm2)
The rationale in EIP-6[1] for changing names to `selfDestruct` applies to code
as much as it does to specs.  Also, Ethereum uses the new names consistently,
so it's useful for our code to match the terms used in later EIP specs and
testsuite entries.

This change is straightforward, and is a prerequisite for patches to come that
do things with the `selfDestruct` fields.

[1] https://eips.ethereum.org/EIPS/eip-6
Hudson Jameson, "EIP-6: Renaming SUICIDE opcode," Ethereum Improvement
Proposals, no. 6, November 2015.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-08 15:36:30 +01:00
Jamie Lokier ef7773daa6
Whisper: Remove Whisper-specific hexstring/JSON/key storage support
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 18:12:48 +01:00
Jamie Lokier 613f06e61c
Whisper: Remove all the main Whisper code (config, startup, RPC etc)
This is the main patch which removes Whisper code from `nimbus-eth1` code.
It removes all configuration, help, startup, JSON-RPC calls and most types.

Note, there is still Whisper functionality in `nim-eth`.  Also, the "wrapper"
under `wrappers/` isn't dealt with by this change, but it's not built by
default (and might not currently work).

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 18:12:48 +01:00
Jamie Lokier f5c69f372a
Whisper: Remove all code which starts Whisper running
This is the patch which removes Whisper functionality from `nimbus-eth1`,
even though the code has yet to be removed after.

After this change, enabling Whisper has no effect.  Its configuration is
ignored and it won't be started.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 18:12:48 +01:00
Jamie Lokier 774f697c73
Whisper: Disable Whisper (Shh) protocol by default
This commit turns Whisper off by default, without changing anything else.

So this can be cherry-picked if you just want to disable Whisper without
removing it.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 18:12:47 +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
Jamie Lokier 8f9c593dac
EVM: Remove `vm_types2` unused `op_codes`/`opcode_values` re-exports
Everything builds with this section removed, all build options.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 16:54:38 +01:00
Jamie Lokier fdebf6c1f7
EVM: Remove unused file `vm_message`
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 16:54:38 +01:00
Jamie Lokier 5e718bcbe2
EVM: Remove most unused imports of `vm_*` files
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 16:54:38 +01:00
Jamie Lokier beb750b8df
Transaction: Remove no longer used `setupComputation`
The last caller of `setupComputation` is gone, now that it's been replaced by
the single entry point for all EVM calls, `runComputation`.

With this removal, EVM's `Computation` type should no longer be used anywhere
outside the call module (except in some tests and the EVM itself).

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 16:54:34 +01:00
Jamie Lokier 8b4f5a1103 Transaction: Change transactions to go through unified EVM runner
Simplify transaction validations to use `runComputation`; drop other code.

Getting everything right up to this point to pass all the tests was trickier
than it looks.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 18:40:41 +03:00
Jamie Lokier bf6569bdeb Assembler: Change assembler tests to go through unified EVM runner
Simplify how assembler tests are run to use `runComputation`; drop other code.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 18:40:41 +03:00
Jamie Lokier 9211a15c0a Fixtures: Change JSON fixture tests to go through unified EVM runner
Simplify how JSON fixtures tests are run to use `runComputation`.
Drop other code.

These use the `noTransfer` option, which is similar enough to calling
`c.executeOpcodes()` instead of `c.execComputation()`.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 18:40:41 +03:00
Jamie Lokier 5fb3c51e5e Transaction: Skip balance & nonce updates option in `runComputation`
Add another flag to disable a processing step when a call doesn't come from
a real transaction:

- `noTransfer`: Don't update balances, nonces, code.

This is to support VM fixtures tests which require account balances and nonces
to be unchanged when running the account's code.

These tests call `c.executeOpcodes()`, an internal function of the EVM, instead
of the usual `c.execComputation()`.  It goes direct to the bytecode dispatcher,
skipping parts of `Computation` that are normally called.

But we can't keep calling `c.executeOpcodes()` and have a single entry point to
the VM, let alone an EVMC entry point.

`noTransfer` provides similar enough behaviour to calling `c.executeOpcodes()`
that these tests can use the new single entry point like everything else.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-06-01 18:40:41 +03:00
Jamie Lokier deffa20b07
RPC and GraphQL: Change `estimateGas` to go through unified EVM runner
Simplify `estimateGas` to use `runComputation`; drop other code.

The RPC/GraphQL `estimateGas` operation is quite different from the `call`
operation.  It is much more like ordinary transaction execution than `call`,
though there are still enough differences that tx validation cannot be used.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-29 08:23:20 +01:00
Jamie Lokier b16aa2f1f7
RPC and GraphQL: Change `call` op to go through unified EVM runner
Simplify `call` to use `runComputation`; drop other code.

The RPC/GraphQL `call` operation differs in many ways from regular transaction
calls.  The following flags are set, to disable various steps in processing.
All four are true (disabling the corresponding step) for `call`:

- `noIntrinsic`:  Don't charge intrinsic gas.
- `noAccessList`: Don't initialise EIP2929 access list.
- `noGasCharge`:  Don't charge sender account for gas.
- `noRefund`:     Don't apply gas refund/burn rule.

Interestingly, the RPC/GraphQL `estimateGas` operation doesn't behave so
differently from regular transactions.  It might be that not all these steps
should be disabled for `call` either.  But until we investigate what
RPC/GraphQL clients are expecting, keep the same behaviour as before.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-29 08:23:20 +01:00
Jamie Lokier 306c8e92c2
Transaction: `runComputation` options for non-standard EVM behaviour
The following four flags are added, to change various steps in EVM processing
when a call doesn't come from a real transaction:

- `noIntrinsic`:  Don't charge intrinsic gas.
- `noAccessList`: Don't initialise EIP2929 access list.
- `noGasCharge`:  Don't charge sender account for gas.
- `noRefund`:     Don't apply gas refund/burn rule.

This is to support RPC and GraphQL `call` operations, which behave differently
in some ways from regular transaction calls, and to support some test suites.

In EVMC terms, all these alterations can be performed on the host side.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-29 08:23:19 +01:00
Jamie Lokier 12bf0fd346
Transaction: EIP-2930 (Berlin): Extra intrinsic gas for access list
Calculate extra intrinsic gas for an EIP-2930 transaction with access list.

While we're here, do the rest of the intrinsic gas calculation.  Make it clear,
explicit and in one place.  (Previous code delegated parts of the calculation
to `transaction.nim` but had to do the rest locally due to mismatched types.)

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-29 06:54:19 +01:00
Jamie Lokier f6cc6e3ed1
Transaction: EIP-2930 (Berlin): Per-transaction extra access list
Add `accessList` to the `runComputation` API for EIP-2930 transactions.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-29 06:53:11 +01:00
Jamie Lokier 76031ccbe4
Transaction: EIP-2929 (Berlin): Initial access list
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-29 06:53:11 +01:00
Jamie Lokier c6d50a0ef7
Transaction: Unified runner `runComputation` for all EVM call types
New entry point `runComputation`, for all EVM calls.
(Later the intent is `runComputationAsync`.)

As noted in commit 297d789, there are six entry points calling EVM computation,
with different parameters and expecting different behaviours.  Parameters were
dealt with in `setupComputation`.  Behaviours are unified in `runComputation`,
with options passed via `CallParams`.

This code performs the steps used when validating a transaction.  Options for
non-standard behaviour for RPC, GraphQL and tests to be added as required.

This replaces `setupComputation`, `execComputation` and `executeOpcodes`
(other than its own calls).  As a result `Computation` and other EVM types are
no longer referenced in the main program, and many imports can be dropped.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-29 06:53:11 +01:00
Jamie Lokier 94f95efd9e
Transaction: Move `setupComputation` to its own file
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-29 06:53:11 +01:00
Jamie Lokier 4e51a5bb35
Fixtures: Change fixture tests to use shared `setupComputation`
Change fixtures tests to use shared `setupComputation` instead of
their own slightly different variant.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-28 16:48:09 +01:00
Jamie Lokier a5385e5344 RPC/GraphQL: Change RPC/GraphQL to use shared `setupComputation`
Change RPC/GraphQL calls to the EVM to use shared `setupComputation`
instead of their own special variant.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-28 16:10:19 +01:00
Jamie Lokier 8b33cbe568
Assembler: Change assembler tests to use shared `setupComputation`
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-28 15:50:02 +01:00
Jamie Lokier c655d59b5f
Assembler: Rearrange logic where assembler tests call EVM
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-28 15:49:58 +01:00
Jamie Lokier 0d3117344a
Transaction: Change tx validation to use shared `setupComputation`
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-28 15:22:06 +01:00
Jamie Lokier 297d789d21
Transaction: Merge entry point for `*...SetupComputation` functions
There are currently six entry points to running an EVM computation, all with
slightly different parameters, and expecting slightly different EVM behaviours.

First step in merging them is a common `setupComputation` that replaces all
the different `*...SetupComputation` functions.

This uses the `TransactionHost` type because it's a step towards using that
type for all EVM calls using only EVMC.  For now an EVMC message is created
then translated to EVM-internal `Message`.  It is done this way to build up
the new interface in stages where all tests pass at each stage.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-28 12:12:10 +01:00
Jamie Lokier e60ad129a2
Transaction: New object `TransactionHost` for "EVMC host services"
`TransactionHost` represents the interface to the EVM from the application once
we've fully transitioned to EVMC.  It represents a managed EVM call, and the
"EVMC host" side of the host<->EVM boundary.

This holds transaction state which sits outside the EVM, but a pointer to this
is passed around by the EVM as _opaque_ type `evmc_host_context`.

To the EVM, this offers "host services", which manage account state that the
EVM cannot do, such as balance transfers, call costs, storage, logs, gas
refunds, nested calls and new contract addresses.  The EVM has no direct access
to the account state database or network; it's all via "host services".

To the application (host side), this object represents a managed EVM call, with
inputs, a method to run, outputs, hidden transaction state, and later async
scheduling.  It is to replace `Computation` on the application side, while
`Computation` will remain but just be for the EVM side.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-28 12:12:02 +01:00
Jamie Lokier 537cac1bf5
EVM: Move where `continuation` is cleared to fix a potential stall
This fixes a bug spotted by @mjfh that was introduced by commit 2a7ccceb:

    try:
      if not c.continuation.isNil:
        (c.continuation)()
        c.continuation = nil
      c.selectVM(fork)
    except CatchableError as e:
      ...

The call to `(c.continuation)()` was moved by 2a7ccceb inside the `try` so
that, like all the Op functions do already, if the continuation raises, the
interpreter's general catch turns the exception into a an error status result.

But if the continuation raises an exception, `continuation` is not cleared in
the next line, and at the next resumption the continuation is called again.
It may loop doing this.

This doesn't currently happen because the continuations don't really raise, but
it's still a correctness issue.

This fix also allows a continuation to spawn a second continuation, if it
encounters a second suspension point.  This also doesn't happen currently,
but the pattern will become useful with async EVM.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-27 12:16:37 +01:00
Jamie Lokier 90a961243e
Clear up meaning of `ZERO_ADDRESS`, delete `CREATE_CONTRACT_ADDRESS`
There is no valid `CREATE_CONTRACT_ADDRESS`.  Some places on the internet say
account zero means contract creation, but that's not correct.

Transactions to `ZERO_ADDRESS` are legitimate transfers to that account, not
contract creations.  They are used to "burn" Eth.  People also send Eth to
address zero by accident, unrecoverably, due to poor user interface issues.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-27 12:14:51 +01:00
Jamie Lokier 745129c4ac
Transaction: Don't calculate `contractAddress` redundantly
Each place in `call_evm` that sets up an EVM call calculates the new contract
address for contract creations.  But it's redundant, because `newComputation`
ignores the provided value and does the calculation again.

Remove the unused address calculation.

This is also a step to merging different entry points and EVMC.  This change
ends up with the same value in both `msg.contractAddress` and `msg.codeAddress`
for every entry point, and this is good because it matches the EVMC message
structure, where they are replaced by only one value called `msg.destination`

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-27 12:01:47 +01:00
Jamie Lokier fa74dc909e
Fixtures: Verify EVM continuation is clear after `c.executeOpcodes`
`c.executeOpcodes` is called by some JSON fixture tests.  These tests bypass
some of the setup and return, and because of this call, continuations aren't
processed either.  Opcodes that use continuations will behave incorrectly.

The opcodes used in these particular tests don't use continuations currently,
so just add some assertions to verify this remains the case.

This is only used by local tests, and the call to `c.executeOpcodes` will be
replaced by the common entry point (that handles things like this correctly in
all cases) so we don't need to spend more time on this.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-27 11:54:44 +01:00
jangko 5fc57e4093
add validateKinship in persistBlocks of nimbus/p2p/chain.nim
put jordan's work #668 into effect, and this bring down
failing consensus test cases from 59 to 44
2021-05-27 16:28:26 +07:00
Jordan Hrycaj e5947f4db6 Deep copy semantics for LRU cache
why:
  follows standard nim semantics

details:
  changed Table to TableRef in previous patch which was the
  wrong choice (see andri's comment.)
2021-05-26 11:12:52 +01:00
Jordan Hrycaj 7b72109afa Use sorted RLP serialisation for LRU cache
why:
  previously, table data were stored with the table iterator. while
  loading a table with permuted entries will always reconstruct equivalent
  tables (in the sense of `==`), serialisation data are not comparable.

  this patch produces always the same serialised data for equivalent
  tables.
2021-05-26 07:58:12 +01:00
Jordan Hrycaj b83b47e541 LRU cache tests makeover
why:
  source-local unit tests would hardly be triggered by github CI as rightly
  criticised with the last patch.

details:
  source-local unit tests have been moved to tests folder.

  this version also contains rlp serialisation code so rlp encode/decode
  will apply tranparently. this is not needed in p2p/validate but will be
   useful with the clique protocol.
2021-05-26 07:58:12 +01:00
jangko 396f3e9909
add missing poaEngine configuration in config.nim
later we will use real engine configuration it if become available to us
2021-05-24 14:35:47 +07:00
Jordan Hrycaj 179cc75c32 Update LRU complexity comment
why:
  hash tables might worst-case degrade into linear mode, so it is not
  strictly O(1)
2021-05-24 07:57:21 +01:00
Jordan Hrycaj 1965ea027b updated LRU cache to run with complexity O(1)
why:
  to be used in Clique consensus protocol which suggests 4k cache entries.
  the previous implementation used OrderTable[] which has complexity O(n)
  for deleting entries.
2021-05-24 07:57:21 +01:00
Jordan Hrycaj a5e0bb6139 use general lru_cache for EpochHashCache
why:
  generic implementation will be also be used elsewhere
2021-05-24 07:57:21 +01:00
Jordan Hrycaj 3663b1603f pulled out cache logic into separate file
why:
  handy to re-use, eg. for upcoming clique implementation
2021-05-24 07:57:21 +01:00
Jordan Hrycaj d6a5cecb98 re-wrote validation with exceptionless functions
why:
  exceptions were from test code should be avoided in production code
2021-05-24 07:57:21 +01:00
Jordan Hrycaj 40c7bdfc06 update lookup cache management
details:
  enable fifo behaviour, using cache as argument
2021-05-24 07:57:21 +01:00
Jordan Hrycaj ce8e5511e3 backport from test_blockchain_json, see issue #666 2021-05-24 07:57:21 +01:00
jangko b82061bf46
don't mix customBootNodes and bootNodes usage 2021-05-20 14:04:17 +07:00
jangko a0d10f5728
drop PublicNetwork enum usage and replace it with NetworkId
we cannot limit the `--networkid` switch to values available in
`PublicNetwork` enum. it should able to accept very wide range of
custom NetworkId.
2021-05-20 14:04:16 +07:00
jangko ec1af91370
implement "net_nodeInfo" rpc
fixes #569
2021-05-19 16:35:13 +07:00
jangko 0ecf9fe1af
add more query fields and resolvers to graphql api
after EIP2718/EIP2930, we have additional fields:

type AccessTuple {
  address: Address!
  storageKeys : [Bytes32!]
}

type Transaction {
  r: BigInt!
  s: BigInt!
  v: BigInt!
  # Envelope transaction support
  type: Int
  accessList: [AccessTuple!]
}

close #606
2021-05-18 07:32:03 +07:00
jangko d2b47139e1
fixes `importRlpBlocks` in conf_utils.nim
now we are importing block one by one to satisfy
some of hive test cases.

we also catch exception instead of letting it terminate the
process.
2021-05-17 18:43:44 +07:00
jangko 76543da456
disable EIP-2537: Precompile for BLS12-381 curve operations
reason: not included in berlin hard fork

but we keep the code around, for future inclusion
2021-05-17 01:29:03 +07: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 6fc3df637c
reenable EIP-2565: modExp gas cost
now it's officially included in berlin hard fork
2021-05-17 01:28:31 +07:00
jangko 01a27ff328
EIP-2930: optional access list
the new AccessListTx contains address and storage keys
that will go into global access list. and this come with
prices.... in ether
2021-05-16 19:54:48 +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
jangko 87c6ec7309
eip2718: add tx validate for AccessListTx 2021-05-15 18:09:36 +07:00
jangko 77f080c8c2
eip2718: nimbus is compileable 2021-05-15 18:09:36 +07:00
jangko 79044f1e92
eip2718: test_blockchain_json pass test 2021-05-15 18:09:35 +07:00
jangko f2491e6307
fixes crappy custom genesis and chain config parser
instead of using stdlib/json, now we switch to json_serialization
the result is much tidier code and more robust when parsing
optional fields.

fixes #635
2021-05-13 16:04:08 +07:00
jangko a57ac65c8c
fixes --customnetwork parser
now it can ignore an optional fork e.g. MuirGlacier
2021-05-12 19:05:32 +07:00
jangko 97f4226171
update berlin fork number in config.nim
also update test_forkid because of berlin changes
2021-05-12 17:24:27 +07:00
jangko 5ee918f4ef
fixes test_graphql crash due to recent changes related to `chainId`
now test_graphql takes another route to initialize the empty db
that is safer instead of bypassing commonly used route.
2021-05-12 09:45:09 +07:00
jangko f6a0e4bcbd
fixes wrong usage of `chainId` in places where it should be networkId
fixes #643
2021-05-12 09:45:09 +07:00
jangko 2d3d450075
fixes `validateFixedLenHex` in graphql/ethapi.nim
now it can detect too long hex besides padding too short hex
2021-05-12 08:12:26 +07:00
jangko d0546becfb
add query complexity calculator to graphql/ethapi
this will allow us to pass two more hive tests
2021-05-10 22:22:04 +07:00
jangko 76189c6357
fixes graphql.scalar.Bytes32
relaxing the rigid length validation of Bytes32 scalar.
allow it to parse hex less than 64 bytes, and add leading
zeroes if needed.
2021-05-05 19:48:56 +07:00
jangko b8c55229e7
implement graphql.Query.account
although this is not part of EIP 1767
but the hive test cases derived from besu
test cases contains this.
we add this now to pass more test hive.graphql cases
2021-05-05 19:18:35 +07:00
jangko 9c04202412
using inheritance on Node in graphql resolvers
this remove usage of unsafe cast
2021-05-05 11:20:12 +07:00
jangko dabff33689
turn all CatchableError into error code, not only EVMError 2021-05-05 11:20:12 +07:00
jangko 0dd6d1e3d4
add some comments to graphql/sendRawTransaction
the comment is about missing things need to be addressed
by sendRawTransaction
2021-05-05 11:20:11 +07:00
jangko 728f3a24be
fixes `rpcMakeCall`: using parent.stateRoot
instead of using header.stateRoot, now it is using parent.stateRoot
2021-05-05 11:20:11 +07:00
jangko c200cebb1d
fixes various bugs in graphql/ethapi.nim 2021-05-05 11:00:12 +07:00
Jamie Lokier 1574136a25
Precompiles: Change precompile tests to use fixtureCallEvm
Move the EVM setup and call in precompile tests to `fixtureCallEvm` in
`call_evm`.  Extra return values needed for testing are returned specially, and
the convention for reporting gas used is changed to match `asmCallEvm`.

Although the precompile tests used `execPrecompiles` before, `executeOpcodes`
does perfectly well as a substitute, allowing `fixtureCallEvm` to be shared.

_Significantly, this patch also makes `Computation` more or less an internal
type of the EVM now._

Nothing outside the EVM (except `call_evm`) needs access any more to
`Computation`, `execComputation`, `executeOpcodes` or `execPrecompiles`.
Many imports can be trimmed, some files removed, and EVMC is much closer.

(As a bonus, the functions in `call_evm` reveal what capabilities parts of the
program have needed over time, makes certain bugs and inconsistencies clearer,
and suggests how to refactor into a more useful shared entry point.)

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-05 02:31:46 +01:00
Jamie Lokier 751068a4d4
EVM call: Simplify and make consistent how to select the fork
Allow the fork to be specified consistently through an `option[Fork]` instead
of varying inconsistencies depending on which call.  When fork is not
specified, the `BaseVMState` code picks the correct fork by default for the
block number and chain.

This change actually deletes code, because a number of functions (RPC etc) had
redundant code to pick the fork, which always resolved to same as default.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-05 02:26:21 +01:00
Jamie Lokier 236a65d598
Fixtures: Make fixture "vm json tests" use new function fixtureCallEvm
Move the EVM setup and call in fixtures "vm json tests" to new function
`fixtureCallEvm` in `call_evm`.  Extra return values needed for testing are
returned specially.

This entry point is different from all other `..CallEvm` type functions,
because it uses `executeOpcodes` instead of `execComputation`, so it doesn't
update the account balance or nonce on entry and exit from the EVM.

The new code is a bit redundant and simplistic intentionally, as the purpose is
to move functionality to `call_evm` with high confidence nothing really
changed.  The calls will be jointly refactored afterwards to merge differences.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-04 15:21:15 +01:00
Jamie Lokier 9e99bb6cd9
Fixtures: Prepare fixtureSetupComputation to support fixtureCallEvm
In the `text_vm_json` ("fixtures") test code, there is another variant of
`rpcSetupComputation` and `txSetupComputation` with slightly different
paremeters.  The similarity is obvious.

It is a special setup for testing, though, as it requires slightly different
parameters.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-04 15:21:14 +01:00
jangko 39ce2390ae
fixes `getRecipient`: using `sender` param instead of calculating sender itself
usually, there is always a sender around `getRecipient` call.
no need to recalculate sender. and more important, in some of
JSON-RPC/GraphQL call, the sender is come from `rpcCallData`,
not from `tx.getSender`. or in ohter situation when the tx is
an unsigned tx, without `r,s,v` fields to calculate sender.
2021-05-04 15:31:47 +07:00
Jamie Lokier d2586c3a73
Assembler: Make macro_assembler tests use new function asmCallEvm
Move the EVM setup and call in `macro_assembler` (`runVM`) entirely to new
function `asmCallEvm` in `call_evm`.  Extra return values needed for
testing are returned specially from `asmCallEvm`.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-04 01:36:00 +01:00
Jamie Lokier 834449d943
Assembler: Second asmSetupComputation, calls the first
The second `asmSetupComputation looks up state by block number and preceding
block number, modifies the first transaction with code for testing, and uses
some parts of that transaction to setup an an EVM test.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-04 01:03:55 +01:00
Jamie Lokier 5728491d60
Assembler: First asmSetupComputation to support asmCallEvm
In the `macro_assembler` test code, `initComputation` is another variant of
`rpcSetupComputation` and `txSetupComputation` with slightly different
paremeters.  The similarity is obvious.

It is a special setup for testing, though, as it requires a contract-creation
transaction for parameters, but sets up a `CALL` execution not `CREATE`.

Gather this into `call_evm`: `initComputation` -> `asmSetupComputation`.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-04 01:03:55 +01:00
Jamie Lokier cc7307186d
RPC: Don't export rpcSetupComputation
The point of the `call_vm` exercise is to allow `Computation` to become an
internal type of the EVM, not used as API by the rest of the program.  So
`rpcSetupComputation` should be private.  It was left exported by mistake.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-04 01:00:51 +01:00
Jamie Lokier c7e1cb61ee
Transaction: Make transaction validation use new function txCallEvm
Split out and move the EVM setup and call in `processTransaction` to
`call_evm`.  This is the last part of the main program which calls the EVM
to be moved.  (There's still test code.)

While we're here, move the EIP2929 access list setup too, as the similarity
to `rpcInitialAccessListEIP2929` is obvious.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-04 00:56:03 +01:00
Jamie Lokier 4187eb1959
Transaction: Prepare txRefundGas to support txCallEvm
There's only one call left to `refundGas(Transaction, ...)`, and the
similarity to the tail of `rpcEstimateGas` is obvious.

Gather this into `call_evm`: `refundGas` -> `txRefundGas`.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-03 19:51:20 +01:00
Jamie Lokier 52fd8b8129
Transaction: Prepare txSetupComputation to support txCallEvm
After recent changes, there's only one call left to `setupComputation`, and
it's just a variant like `rpcSetupComputation` but for transaction processing.
The similarity to `rpcSetupComputation` is obvious.

Gather this into `call_evm`: `setupComputation` -> `txSetupComputation`.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-03 19:51:20 +01:00
Jamie Lokier 7eb4471004
Bugfix: Avoid numeric overflow when validating transaction value
It's possible for `tx.value` in the transaction to have a deliberately
constructed large 256-bit value, such that adding `gasLimit * gasPrice` to it
overflows to a small value.

Prior to this patch, the code would allow such a transaction to pass
validation, even though such a large transfer cannot be valid.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-03 19:34:22 +01:00
Jamie Lokier 9a5e0d2833
RPC: Change rpcEstimateGas to use the EVM without a fake transaction
Change `rpcEstimateGas` to setup and execute a computation directly, in a
similar way to `rpcDoCall` and `rpcMakeCall`, instead of constructing a fake
transaction and then validating it.

This patch does not (or should not) change any behaviour.

Although this looks a bit messy as it duplicates parts of `validateTransaction`
and `processTransaction`, proc names have been used to hopefully keep the
meanings clear, and it's just a stepping stone as those transaction functions
will be changed next.  Also the behaviour of RPC `estimateGas` may not be
correct (although this patch is careful not to change it), so it's good to make
it explicit so we can see how it differs from other RPCs.

Doing this change exposed some interesting behaviour differences between the
`call` RPC and `estimateGas` RPC, which may be bugs, or may be intentional.
These differences are now obvious and explicit.

The unclear areas are not well documented by any of the clients, even Infura
which says a bit more than the others.  So to find out if they are intended,
we'll have to run tests against other Ethereum services.

Guessing, on the face of it, it looks likely that RPC `call` should:

- Setup EIP2929 access lists
- Account for intrinsic gas (maybe not because zero-gas transactions are ok)

And it looks likely that RPC `estimateGas` should:

- Not return zero when an account has insufficient balance
- Maybe use a different gas cost estimate when one isn't supplied in the RPC

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-03 16:41:37 +01:00
Jamie Lokier 76c4c72abb
RPC: Prepare rpcSetupComputation to support estimateGas
The RPC `estimateGas` behaves differently from RPC `call` in a number of ways.

These differences may be bugs due to `rpcEstimateGas` calling the EVM in a very
different way than `rpcDoCall`, or they may be intentional.  To be sure, we'll
need to test behaviour with Geth, Infura etc to find out (their documentation
isn't enough.)  For now, though, we'll keep the same behaviour as we always had.

`rpcEstimateGas` cannot use `rpcSetupComputation` as it is, because
`estimateGas` accounts for "intrinsic gas", and `call` does not.

This patch changes `rpcSetupComputation` to accomodate both behaviours.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-03 16:41:33 +01:00
Jamie Lokier f3f872d707
GraphQL: Typo in error message for GraphQL "call" request
Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-03 15:08:50 +01:00
Jamie Lokier 6dd14e4f4f
GraphQL: Move EVM-calling function makeCall to rpcMakeCall
`makeCall` used by GraphQL is another way to setup and call the EVM.
Move it to `transaction/call_evm`.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-03 10:59:13 +01:00
Jamie Lokier 8bda81496a
RPC: Move EVM-calling function estimateGas to rpcEstimateGas
`estimateGas` used by JSON-RPC is another way to setup and call the EVM,
also used by GraphQL.  Move it to `transaction/call_evm`.

This function has too much direct knowledge of details that shouldn't be used
outside transaction handling code, details we need to change when changing the
db and transaction memory layer.

Moving this one exposed quite a bit of abstraction leakage, as it calls
directly to the hexary trie db around `processTransaction`.

It looks like the _intended_ functionality of `estimateGas` is similar to
`rpcDoCall` with the only real difference being to not store the final state.
It looks like the extra stuff in `estimateGas` compared with `doCall` is a
messy workaround for computation not exposing the right API ("don't save final
state") for RPC to use.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-05-03 10:59:10 +01:00