Prior to this patch, top-level EVM executions and nested EVM executions did
their `getStorage` and other requests using a completely different set of host
functions. It was just unfinished, to get top-level "new" EVMC working.
This finishes the job - it stops using the old methods. Effect:
- Functionality added at the EVMC host level will be used by all EVM calls.
(The target here is Beam Sync).
- The old set of functions are no longer used, so they can be removed.
- When EVMC host call tracing is enabled (`showTxCalls = true`), it traces
the calls from nested EVM executions as well as top-level.
Signed-off-by: Jamie Lokier <jamie@shareable.org>
We've been filling a "vtable"-like at run time, but it's not necessary.
The new object is a global `let x = evmc_host_interface(...)`, we assume it's
initialised before the first use, and we take its address with `.unsafeAddr`.
(If we use `ref evmc_host_interface`, Nim decides (correctly) that the
functions which use it aren't GC-safe because it's a global.)
Signed-off-by: Jamie Lokier <jamie@shareable.org>
This combines two things, a C stack usage change with EVM nested calls
via EVMC, and changes to host call tracing.
Feature-wise, the tracing is improved:
- Storage keys and values are make more sense.
- The message/result/context objects are shown with all relevant fields.
- `call` trace is split into entry/exit, so these can be shown around the
called contract's operations, instead of only showing the `call` parameters
after the nested call is finished.
- Nested calls are indented, which helps to highlight the flow.
- C stack usage considerably reduced in nested calls when more functionality
is enabled (either tracing here, or other things to come).
This will seem like a minor patch, but C stack usage was the real motivation,
after plenty of time in the debugger.
Nobody cares about stack when `showTxCalls` (you can just use a big stack when
debugging). But these subtle changes around the `call` path were found to be
necessary for passing all tests when the EVMC nested call code is completed,
and that's a prerequisite for many things: async EVM, dynamic EVM, Beam Sync,
and to fix https://github.com/status-im/nimbus-eth1/issues/345.
Signed-off-by: Jamie Lokier <jamie@shareable.org>
The update for London (EIP-1559) in 1cdb30df ("bump nim-emvc with evmc revision
8.0.0 to 9.0.0") really bumped EVMC ABI version from 7.5 up to 9.
In other words, it skipped Berlin, going direct from Istanbul to London.
That was accompanied by EVMC changes in 05e9b891 ("EIP-3198: add baseFee op
code in nim-evm"), which added the API changes needed for London.
But the missing Berlin functions weren't added in the move to London.
As a result, our EVMC host became incompatible with Berlin, London, and really
all revisions of the ABI, and if a third party EVM was loaded, it crashed.
This commit adds the missing Berlin host support, and makes our ABI
binary-compatible with real EVMC again.
Signed-off-by: Jamie Lokier <jamie@shareable.org>
* Update readme with adjusted intro and dev updates link
* Remove pcre from prerequisites as it no longer is
* Change to hackmd doc as status notes seem not accessible
although they are technically different, but in reality,
many networks are using the same id for ChainId dan NetworkId.
in this commit, we set networkid from config file's chainId.
- allow clique period and epoch to be configured via config file
- this also activate poaEngine mode
- remove clique period configuration from cli to reduce confusion
- fix#786
websock:
- "Host" header override (#87)
- Use caps for log-levels in tests
- Perform utf-8 validation at message boundaries (#90)
json-rpc:
- fix both ws rpc server and client due to websock breaking changes
As this branch of vm2 doesn't support EVMC, this EVMC-motivated change is only
required here for internal compatibility.
Signed-off-by: Jamie Lokier <jamie@shareable.org>
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>
Add these to the list of specially disabled tests during `make test`,
as they are exceptionally slow.
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>
Use a hard-coded number instead of the same expression as the client, so that
bugs introduced via that expression are detected. Using the same expression as
the client can hide issues when the value is wrong in both places.
When the expected value genuinely changes, it'll be obvious.
Just change this number.
Signed-off-by: Jamie Lokier <jamie@shareable.org>
jangko
KonradStaniec
bmoo
Jamie Lokier
Kim De Mey
Jordan Hrycaj
Ștefan Talpalaru