Commit Graph

2919 Commits

Author SHA1 Message Date
Jordan Hrycaj 9545767c53 update ec_recover
why:
  Previous version was based on lru_cache which is ugly. This module is
  based on the stew/keyed_queue library module.

other:
  There are still some other modules rely on lru_cache which should be
  removed.
2022-01-22 08:26:57 +02:00
Kim De Mey d2f56463ce
Remove the use of ByteList in PortalStream and for FoundContent (#937) 2022-01-20 21:21:20 +01:00
Kim De Mey 3909675f29
Use socket destroy instead of close to avoid extra FIN send (#935)
Also add missing push raises Defect
2022-01-20 15:04:23 +01:00
KonradStaniec cccd1e4640
Add tooling necessary to improve utp-testing (#931)
* Add tooling necessary to improve utp-testing
2022-01-19 16:06:23 +01:00
Kim De Mey 69b0cc07b6
Add data radius cache per portal protocol (#930) 2022-01-19 15:56:14 +01:00
Jordan Hrycaj 261c0b51a7
Redesign of BaseVMState descriptor (#923)
* Redesign of BaseVMState descriptor

why:
  BaseVMState provides an environment for executing transactions. The
  current descriptor also provides data that cannot generally be known
  within the execution environment, e.g. the total gasUsed which is
  available not before after all transactions have finished.

  Also, the BaseVMState constructor has been replaced by a constructor
  that does not need pre-initialised input of the account database.

also:
  Previous constructor and some fields are provided with a deprecated
  annotation (producing a lot of noise.)

* Replace legacy directives in production sources

* Replace legacy directives in unit test sources

* fix CI (missing premix update)

* Remove legacy directives

* chase CI problem

* rebased

* Re-introduce 'AccountsCache' constructor optimisation for 'BaseVmState' re-initialisation

why:
  Constructing a new 'AccountsCache' descriptor can be avoided sometimes
  when the current state root is properly positioned already. Such a
  feature existed already as the update function 'initStateDB()' for the
  'BaseChanDB' where the accounts cache was linked into this desctiptor.

  The function 'initStateDB()' was removed and re-implemented into the
  'BaseVmState' constructor without optimisation. The old version was of
  restricted use as a wrong accounts cache state would unconditionally
  throw an exception rather than conceptually ask for a remedy.

  The optimised 'BaseVmState' re-initialisation has been implemented for
  the 'persistBlocks()' function.

also:
  moved some test helpers to 'test/replay' folder

* Remove unused & undocumented fields from Chain descriptor

why:
  Reduces attack surface in general & improves reading the code.
2022-01-18 16:19:32 +00:00
Kim De Mey 14dd763900
Add populate history db option to fluffy (#929) 2022-01-18 15:08:02 +01:00
Kim De Mey 81ebfd2b2a
Add Portal protocol config and add config options to fluffy cli (#928) 2022-01-18 09:01:22 +01:00
Kim De Mey f19a64a2fe
Add uTP over discv5 in portal wire protocol (#922)
* Add uTP over discv5 in portal wire protocol
2022-01-14 16:07:14 +01:00
Jacek Sieka f47c710ada
stew: bump (#924)
`Result` changes and `endians2` performance fix
2022-01-13 21:11:54 +01:00
Kim De Mey 73b4a73423
Bump nim-eth and nim-ssz-serialization vendor modules (#926)
- nim-eth: uTP updates, ENR improvemens and extra discv5 metrics
- nim-ssz-serialization: require BitList pairs
- update github actions cache keys to invalidate cache
2022-01-12 21:41:23 +01:00
Jordan Hrycaj cea2a83b0a
Update/clarify tx validation (#917)
details:
  1. The check for cumulativeGasUsed + tx.gasLimit <= header.gasLimit
     makes neither sense nor is it part of the Eip1559 specs. Nevertheless
     a check tx.gasLimit <= header.gasLimit is added to satisfy some
     unit test (see comments in validateTransaction() body.)
  2. As a replacement check for the one removed in 1, a check for
     cumulativeGasUsed + gasBurned <= header.gasLimit has been added
     (see comments in processTransactionImpl() body.)
  3. Prototypes for processTransaction() variants have been cleaned up and
     commented.

why:
  Detail 1. in particular produces an error for tightly packed blocks when
  the last tx in the list has a generous gasLimit.
2022-01-10 09:04:06 +00:00
Kim De Mey 79647e5580
Allow access to contentDB from portal wire protocol (#920)
- Allow access to contentDB from portal wire protocol
- Use this to do the db.get in `handleFindContent` directly
- Use this to check the `contentKeys` list in `handleOffer`
2022-01-06 09:06:05 +01:00
Kim De Mey a8640fe57c
History content keys (#919)
* Change History content key to us SSZ Union and adjust tests

* Change slot to byteBE instead of LE

This is currently not specified in the Portal network
specifications, but we are using already BE for the actual content
key, so change it also here to remain consistent.
2022-01-05 09:49:49 +01:00
Kim De Mey f4ca598c03
Some version of the findContent JSON-RPC proc (#918)
Also includes fixing instantiation of the history network json-rpc
API handles.
2021-12-20 11:57:55 +01:00
Kim De Mey 796787e0e5
Add state and history content key tests (#916) 2021-12-20 09:53:00 +01:00
Kim De Mey 092d946c72
Add state ContentKey test and adjust history ContentKey test (#914) 2021-12-15 08:48:56 +01:00
Jamie Lokier 6a7803a9e4
EVMC: Improve `--evm`, remove it in non-EVMC builds, change imports
- Remove the `--evm` option on non-EVMC builds.
  `when` around an option doesn't work with confutils; it fails to compile.
  Workaround that by setting the `ignore` pragma on EVMC-specific options.
  (Thanks @jangko for that new pragma).  I prefer this to a solution which
  moves the whole option's pragma elsewhere, especially if we add more options.

- Improve the help text, so that it shows the standard library extension on
  each target platform (or none if on another platform).

- Undo b3f21bf4 "add missing evmc_enabled conditional compilation in
  evmc_dynamic_loader".  Move the conditional to `nimbus.nim`, and take more
  care there to only use the loader function in EVMC builds.

  It's ok to just not include this EVMC-only module (like some other EVMC
  modules), rather than making the module itself a bit broken: Without this
  change, it references a function that's not imported or linked to, and it
  only links because there is no call sequence reaching that function.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-12-14 15:51:09 +00:00
Kim De Mey f3d0e97997
Add Portal protocol testing docs and more (#912)
- Add portal network ping and findNodes JSON-RPC endpoints
- clean-up of some cli arguments
- Add protocol_interop.md document and adjust/fix other
documentation
2021-12-13 14:12:51 +01:00
Jordan Hrycaj 1f774c01a2
Jordan/accounts cache scenario (#904)
* crash test scenario

details:
  Example code for inspecting nested block chain and accounts cache
  database transaction framework. There seems to be a pathological
  case where the system crashes after a rollback (as appeared in the
  tx-pool packer code.)

* simplified crash scenario

* Workable solution (as suggested by Andri)

details:
  Avoiding db.rollback() (db.commit() is OK) while vmState.stateDB is
  alive.

* Rename text_txcrash => test_accounts_cache

why:
  Unit tests covers part of accounts_cache handling

* comment update
2021-12-13 11:58:05 +00:00
Kim De Mey 70625bc02c
Use one bootstrap nodes argument for discv5 and Portal (#911)
Currently bootstrap nodes for discv5 and for the Portal nodes
were provided through separate cli arguments. This is however
confusing and cumbersome as typically when (currently) testing
a node will have both discv5 and the Portal networks enabled.
We merge them into one argument.

If a node happens not to support a Portal network, it will be
removed after message request failure.

Commit also contains additional clean-up and nim-eth bump.
2021-12-13 09:06:29 +01:00
jangko d5082df5d8
fix config using new features from nim-confutils
- `network` and `discovery` got additional longDesc,
  the help text now become more descriptive.
- `networkId` and `networkParams` now is ignored by confutils
  and become ordinary fields of NimbusConf.
2021-12-12 16:45:58 +07:00
jangko d5d0a44b0e
bump nim-confutils with addition features needed by nimbus-eth1
new features:
- multiple lines long description
- ignore property in addition to hidden
2021-12-12 16:38:10 +07:00
Jamie Lokier 4b89ca3215
EVM: `writeContract` fixes, never return contract code as `RETURNDATA`
This fixes #867 "EIP-170 related consensus error at Goerli block 5080941", and
equivalent on other networks.

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

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

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

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

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

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

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

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

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

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

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

Overall four behaviour changes:

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

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

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

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-12-12 16:34:13 +07:00
jangko b3f21bf40c
add missing evmc_enabled conditional compilation in evmc_dynamic_loader 2021-12-12 16:32:13 +07:00
Jamie Lokier cf49b8d4fc
EVMC: Option `--evm`, load third-party EVM as a shared library
This patch adds:

- Load and use a third-party EVM in a shared library, instead of Nimbus EVM.
- New option `--evm` to specify which library to load.
- The library and this loader conforms to the [EVMC]
  (https://evmc.ethereum.org/) 9.x specification.

Any third-party EVM which is compatible with EVMC version 9.x and supports EVM1
contract code will be accepted.  The operating system's shared library format
applies.  These are `.so*` files on Linux, `.dll` files on Windows and `.dylib`
files on Mac.

The alternative EVM can be selected in two ways:

- Nimbus command line option `--evm:<path>`.
- Environment variable `NIMBUS_EVM=<path>`.

The reason for an environment variable is this allows all the test programs to
run with a third-party EVM as well.  Some don't parse command line options.

There are some limitations to be aware of:

- The third-party EVM must use EVMC version 9.x, no other major version.
  EVMC 9.x supports EIP-1559 / London fork and older transactions.

- Nested `*CALL` and `CREATE*` operations don't use the third-party EVM yet.
  These call the built-in Nimbus EVM.  This mixing of different EVMs between
  levels is explicitly allowed in specs, so there is no problem doing it.

- The third-party EVM doesn't need to support precompiles, because those are
  nested calls, which use the built-in Nimbus EVM.

- Third-party EVMs execute contracts correctly, but fail the final `rootHash`
  match.  The reason is that some account state changes, which are correct, are
  currently inside the Nimbus EVM and need to be moved to EVMC host logic.
  *This is a known work in progress*.  The EVM execution itself is fine.

Test results using "evmone" third-party EVM:

- [evmone](https://github.com/ethereum/evmone) has been tested.  Only on
  Linux but it "should" work on Windows and Mac equally well.

- [Version 0.8.1](https://github.com/ethereum/evmone/releases/tag/v0.8.1) was
  used because it is compatible with EVMC 9.x, which is required for the
  EIP-1559 / London fork, which Nimbus supports.  Version 0.8.0 could be used
  but it looks like an important bug was fixed in 0.8.1.

- evmone runs fine and the trace output looks good.  The calls and arguments
  are the same as the built-in Nimbus EVM for tests that have been checked
  manually, except evmone skips some calls that can be safely skipped.

- The final `rootHash` is incorrect, due to the *work in progress* mentioned
  above which is not part of the evmone execution.  Due to this, it's possible
  to try evmone and verify expected behaviours, which also validates our own
  EVMC implementation, but it can't be used as a full substitute yet.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-12-10 16:32:18 +00:00
Jamie Lokier 072934272b
Transaction: Map `evmc_result` back to `Computation` result
This missing part of EVMC processing allows third-party EVMs to work.

It fixes EVMC result processing (at the top-level of calls, not nested calls)
to use the EVMC result object, instead of reading so much internal state of the
Nimbus `Computation` object.

It has been tested by calling [`evmone`](https://github.com/ethereum/evmone)
and getting useful results with tracing enabled (`showTxCalls = true`).  It's
even able to run parts of the fixtures test suite.

There are other issues with account balances, etc that need to be worked on to
get the correct _final_ results, but the EVM execution is correct with this.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-12-10 16:27:53 +00:00
Jamie Lokier 6ef9bfd21b
EVMC: Byte-endian conversions for 256-bit numeric values
Perform byte-endian conversion for 256-bit numeric values, but not 256-bit
hashes.  These conversions are necessary for EVMC binary compatibility.

In new EVMC, all host-side conversions are explicit, calling `flip256`.

These conversions are performed in the EVMC "glue" code, which deals with the
binary interface, so the host services aren't aware of conversions.

We intend to skip these conversions when Nimbus host calls Nimbus EVM, even
when it's a shared library, using a negotiated EVMC extension.  But for now
we're focused on correctness and cross-validation with third party EVMs.

The overhead of endian conversion is not too high because most EVMC host calls
access the database anyway.  `getTxContext` does not, so the conversions from
that are cached here.  Also, well-optimised EVMs don't call it often.

It is arguable whether endian conversion should occur for storage slots (`key`).

In favour of no conversion: Slot keys are 32-byte blobs, and this is clear in
the EVMC definition where slot keys are `evmc_bytes32` (not `evmc_uint256be`),
meaning treating as a number is _not_ expected by EVMC.  Although they are
often small numbers, sometimes they are a hash from the contract code plus a
number.  Slot keys are hashed on the host side with Keccak256 before any
database calls, so the host side does not look at them numerically.

In favour of conversion: They are often small numbers and it is helpful to log
them as such, rather than a long string of zero digits with 1-2 non-zero.  The
representation in JSON has leading zeros removed, like a number rather than a
32-byte blob.  There is also an interesting space optimisation when the keys
are used unhashed in storage.

Nimbus currently treats slot keys on the host side as numbers, and the tests
pass when endian conversion is done.  So to remain consistent with other parts
of Nimbus we convert slot keys.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-12-10 16:23:27 +00:00
Jamie Lokier df29b98079
Hive: Add Arrow Glacier fork to Hive integration + related fixes
Adds changes to Nimbus Hive support for the new
[Arrow Glacier fork](https://eips.ethereum.org/EIPS/eip-4345).

While here:

- Fix typo in `nimbus.sh`:
  `HIVE_FORK_MUIRGLACIER` => `HIVE_FORK_MUIR_GLACIER` (just a comment).

- Add `muirGlacierBlock` to the JSON generated in `extract_consensus_data.nim`.
  This makes it symmetric with the JSON parsed in `mapper.jq`.

- Removed "At5" network names which are not used by any of the test suite.
  These are `ByzantiumToConstantinopleAt5`, `ConstantinopleFixToIstanbulAt5`
  and `IstanbulToBerlinAt5`.

  The motivation for removing these instead of systematically including all
  possibilities was that I realised `LondonToArrowGlacierAt5` does not appear
  anywhere in the current test suite, even though `ArrowGlacier` does.  As each
  section in the code is rather large already, I thought it cleaner to not add
  this one, and keep only the ones the test suite actually uses.

  This also now better matches the code in `test_blockchain_json.nim`.

- Sorted `HomesteadToDaoAt5` before `HomesteadToEIP150At5` because the DAO
  fork happened earlier than EIP-150 in real life.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-12-10 13:40:54 +00:00
Jamie Lokier 74253c88e3
Arrow Glacier fork
Add the new [Arrow Glacier fork](https://eips.ethereum.org/EIPS/eip-4345).

Only the difficulty calculation is changed, but as a new fork it still affects
a number of places in the code.

To the best of my knowledge the change is only scheduled on Mainnet.

In addition:

- The fork date comments in `chain_config.nim` have been checked against the
  real networks, set consistently in UTC instead of random timezones, and made
  neater.  Maybe we'll keep these when transferring config to a file someday.

- It's added to forkid hash tests (EIP-2124/EIP-2364), of course.

Signed-off-by: Jamie Lokier <jamie@shareable.org>
2021-12-10 13:40:51 +00:00
Jordan Hrycaj 55f7a4425f
Jordan/pow cache management (#888)
* PoW wrapper for verification & mining

why:
  It eases data management of per-Epoch lookup tables. Also some unit
  tests show limits of usefulness on non-specialised machines for
  mining besides developing tests.

details:
  For PoW verification, this patch provides a pretty wrapper hiding the
  details of the ethash/Hashimoto lookup cache management.

  For mining on my development system without special hardware, the
  underlying ethash functions are prohibitively slow. It takes
   * ~20 minutes to prepare the full ethash/Hashimoto lookup dataset
   * a second to run ~25k nonce tests (in the mining loop)

  The mining part might be of some use for generating test data for
  the tx-pool, though.

* Using PowRef as replacement for EpochHashCache + hashimotoLight()

* Fix typo (CI failed)

why:
  was below log level when testing locally

* fix canonical naming
2021-12-10 08:49:57 +00:00
Kim De Mey 10a3946194
Bit of cleanup (#903)
* Rename FindNode to FindNodes as per spec

* Use consistently lower case starting camelCase for consts

Style guide nep1 allows for both CamelCase and camelCase for
consts, but we seem to use more often camelCase. Using this now
consistently.

* Change some procs to func
2021-12-08 11:54:22 +01:00
Kim De Mey 2ea35ef82a
Add resolve call for Portal networks (#902)
* Add resolve call for Portal networks

And:
- Refactor some code by adding a findNodeVerified call
- add the portal network lookup json-rpc call that uses resolve
- add usage of this lookup in the portal testnet tests
- Additional comments

* Let recordsFromBytes fail on invalid ENRs

This behaviour is more similar as how it is done in discovery v5
base layer.
2021-12-08 09:26:31 +01:00
Kim De Mey 63b0945b52
Use json rpc client to run tests on portal testnet (#899)
Tests have also been added & adjusted. Original ones were actually
failing but unnoticed due to mistake in port passed in curl command.
2021-12-03 09:51:25 +01:00
Kim De Mey ed04aad203
Fix fluffy discovery rpc tests after nim-json-rpc bump (#897) 2021-11-30 16:04:24 +01:00
jangko cdfcdfc85b
hive: nodocker rpc simulator skeleton 2021-11-30 16:03:41 +07:00
jangko f051c2530e
fixes related to nim-json-rpc bump 2021-11-30 14:13:20 +07:00
jangko dcd86bb221
vendor: bump nim-json-rpc
bugfix and features:
- Switch to the Chronos HTTP client (adds support for HTTPS)
- Allow dynamic RPC method names in the 'rpc' macro
- Restore the support for using the news package
2021-11-30 11:47:11 +07:00
Kim De Mey 0072089909
Implement part of the discv5 json-rpc calls (#894)
* Implement part of the discv5 json-rpc calls

* Use json marshalling for NodeId and Record everywhere
2021-11-29 10:39:37 +01:00
Kim De Mey 5f4ea7822c
Have one generic installPortalApiHandlers to be used for different Portal networks (#893) 2021-11-24 14:53:01 +01:00
Kim De Mey 77cc9b6215
Add support for passing bootstrap file and write local enr file (#892)
Also use this now in the local testnet script instead of hardcoded
values.
2021-11-24 12:12:25 +01:00
Kim De Mey 903350bdde
Add local testnet script and required json-rpc calls (#891)
- Add basic discv5 and portal json-rpc calls and activate them in
fluffy
- Renames in the rpc folder
- Add local testnet script and run this script in CI
- bump nim-eth
2021-11-24 08:45:55 +01:00
KonradStaniec f9ed83c221
bump nim eth (#890) 2021-11-22 12:35:08 +01:00
Kim De Mey 02afda1b45
Run everything fluffy with chronosStrictException (#889) 2021-11-18 17:52:44 +01:00
Kim De Mey 78f585399f
Bump submodules (#887)
* Bump submodules

* Fix missing import for state and history networks
2021-11-18 12:06:53 +01:00
Kim De Mey bdbd8e4c87
Add SSZ Unions through case objects (#882)
* Add SSZ Unions through case objects

* Add connection id content response test and improve other test vectors

* Implement content keys and ids for state network as per spec

Content keys case object is used so that it can be serialized and
deserialized as an SSZ Union.

* Let message Union in Portal wire protocol start at 0 as per new spec
2021-11-17 17:11:17 +01:00
Kim De Mey 414fdafab9
Improve the populating of the routing table (#886)
- Search for the node on an incoming portal message and try to add
it to the routing table
- Don't rely on discv5 nodes for bootstrapping portal networks for
now.
- Attempt to repopulate the routing table when it is at 0 or drops
to 0.
2021-11-16 17:50:08 +01:00
jangko 7066b48303
add -d:unittest2DisableParamFiltering to .nimble
fix #840
2021-11-14 16:04:19 +07:00
kdeme 6de2be146a
Bump unittest2 and testutils 2021-11-14 16:02:22 +07:00
jangko 37fe6fa32f
fixes hive/consensus_sim command line param
recent changes of confutils based config requires
some command line adjustment.
2021-11-09 17:55:13 +07:00