This changes fixes a bug in `CREATE2` ops when used with EVMC.
Because it changes the salt type, it affects non-EVMC code as well.
The salt was passed through EVMC with the wrong byte order, although this went
unnoticed as the Nimbus host flipped the byte order before using it.
This was found when running Nimbus with third-party EVM,
["evmone"](https://github.com/ethereum/evmone).
There are different ways to remedy this.
If treated as a number, Nimbus EVM would byte-flip the value when calling EVMC,
then Nimbus host would flip the received value. Finally, it would be flipped a
third time when generating the address in `generateSafeAddress`. The first two
flips can be eliminated by negotiation (like other numbers), but there would
always be one flip.
As a bit pattern, Nimbus EVM would flip the same way it does when dealing with
hashes on the stack (e.g. with `getBlockHash`). Nimbus host wouldn't flip at
all - and when using third-party EVMs there would be no flips in Nimbus.
Because this value is not for arithmetic, any bit pattern is valid, and there
shouldn't be any flips when using a third-party EVM, the bit-pattern
interpretation is favoured. The only flip is done in Nimbus EVM (and might be
eliminated in an optimised version).
As suggested, we'll define a new "opaque 256 bits" type to hold this value.
(Similar to `Hash256`, but the salt isn't necessarily a hash.)
Signed-off-by: Jamie Lokier <jamie@shareable.org>
Nimbus types generally use the bit count not the byte count, e.g. `UInt256`,
`Hash256`, so make `ZERO_HASH256` (which has type `Hash256`) fit this pattern.
Signed-off-by: Jamie Lokier <jamie@shareable.org>
* Provide PoA voting header generator
why:
Handy for hive/smoke test
details:
Header generator is a re-implementation of the generator previously
used for the canonical reference tests.
* try fixing ci out-of-mem condition
why:
for some reason, the ci began behaving like a real win7/i386 machine
where gcc is limited to 64k optimiser space
* fix comments, typos ..
new command line options:
--ws Enable the Websocket JSON-RPC server
--wsbind:<value> Set address:port pair(s) (comma-separated) Websocket JSON-RPC server will bind to (default: localhost:8546)
--wsapi:<value> Enable specific set of Websocket RPC API from list (comma-separated) (available: eth, debug)
fixes#770
new command line options:
--clique-period:<value> Enables clique support. value is block time in seconds
--engine-signer:<value> Enables mining. value is EthAddress in hex
--import-key:<path> Import unencrypted 32 bytes hex private key file
fixes#771
* Provide API
details:
API is bundled via clique.nim.
* Set extraValidation as default for PoA chains
why:
This triggers consensus verification and an update of the list
of authorised signers. These signers are integral part of the
PoA block chain.
todo:
Option argument to control validation for the nimbus binary.
* Fix snapshot state block number
why:
Using sub-sequence here, so the len() function was wrong.
* Optional start where block verification begins
why:
Can speed up time building loading initial parts of block chain. For
PoA, this allows to prove & test that authorised signers can be
(correctly) calculated starting at any point on the block chain.
todo:
On Goerli around blocks #193537..#197568, processing time increases
disproportionally -- needs to be understand
* For Clique test, get old grouping back (7 transactions per log entry)
why:
Forgot to change back after troubleshooting
* Fix field/function/module-name misunderstanding
why:
Make compilation work
* Use eth_types.blockHash() rather than utils.hash() in Clique modules
why:
Prefer lib module
* Dissolve snapshot_misc.nim
details:
.. into clique_verify.nim (the other source file clique_unused.nim
is inactive)
* Hide unused AsyncLock in Clique descriptor
details:
Unused here but was part of the Go reference implementation
* Remove fakeDiff flag from Clique descriptor
details:
This flag was a kludge in the Go reference implementation used for the
canonical tests. The tests have been adapted so there is no need for
the fakeDiff flag and its implementation.
* Not observing minimum distance from epoch sync point
why:
For compiling PoA state, the go implementation will walk back to the
epoch header with at least 90000 blocks apart from the current header
in the absence of other synchronisation points.
Here just the nearest epoch header is used. The assumption is that all
the checkpoints before have been vetted already regardless of the
current branch.
details:
The behaviour of using the nearest vs the minimum distance epoch is
controlled by a flag and can be changed at run time.
* Analysing processing time (patch adds some debugging/visualisation support)
why:
At the first half million blocks of the Goerli replay, blocks on the
interval #194854..#196224 take exceptionally long to process, but not
due to PoA processing.
details:
It turns out that much time is spent in p2p/excecutor.processBlock()
where the elapsed transaction execution time is significantly greater
for many of these blocks.
Between the 1371 blocks #194854..#196224 there are 223 blocks with more
than 1/2 seconds execution time whereas there are only 4 such blocks
before and 13 such after this range up to #504192.
* fix debugging symbol in clique_desc (causes CI failing)
* Fixing canonical reference tests
why:
Two errors were introduced earlier but ovelooked:
1. "Remove fakeDiff flag .." patch was incomplete
2. "Not observing minimum distance .." introduced problem w/tests 23/24
details:
Fixing 2. needed to revert the behaviour by setting the
applySnapsMinBacklog flag for the Clique descriptor. Also a new
test was added to lock the new behaviour.
* Remove cruft
why:
Clique/PoA processing was intended to take place somewhere in
executor/process_block.processBlock() but was decided later to run
from chain/persist_block.persistBlock() instead.
* Update API comment
* ditto
Using the same packet tracing format to match `protocol_eth65`.
There aren't many calls, and this makes them clear.
Signed-off-by: Jamie Lokier <jamie@shareable.org>
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>
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>
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>
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>
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>
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>
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>
* 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.
* 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
* 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
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
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
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
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
* 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)
* 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
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
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Change fixtures tests to use shared `setupComputation` instead of
their own slightly different variant.
Signed-off-by: Jamie Lokier <jamie@shareable.org>
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>
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>
`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>
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>
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>
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>
`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>
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.
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.
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.
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
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
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
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>
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>
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>
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>
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.
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>
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>
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>
Jamie Lokier
Jordan Hrycaj
jangko
Kim De Mey
KonradStaniec