Root encoding is on the hot path for block verification both in the
consensus (when syncing) and execution clients and oddly consititutes a
significant part of resource usage even though it is not that much work.
While the trie code is capable of producing a transaction root and
similar feats, it turns out that it is quite inefficient - even for
small work loads.
This PR brings in a helper for the specific use case of building tries
of lists of values whose key is the RLP-encoded index of the item.
As it happens, such keys follow a particular structure where items end
up "almost" sorted, with the exception for the item at index 0 which
gets encoded as `[0x80]`, ie the empty list, thus moving it to a new
location.
Armed with this knowledge and the understanding that inserting ordered
items into a trie easily can be done with a simple recursion, this PR
brings a ~100x improvement in CPU usage (360ms vs 33s) and a ~50x
reduction in memory usage (70mb vs >3gb!) for the simple test of
encoding 1000000 keys.
In part, the memory usage reduction is due to a trick where the hash of
the item is computed as the item is being added instead of storing it in
the value.
There are further reductions possible such as maintaining a hasher per
level instead of storing hash values as well as using a direct-to-hash
rlp encoder.
Transaction signing is something that happens in a lot of places - this
PR introduces primitives for transaction signing in `transaction_utils`
such that we can use the same logic across web3/eth1/etc for this simple
operation.
`transaction_utils` also contains a few more "spec-derived" helpers for
working with transactions, such as the computation of a contract address
etc that cannot easily be introduced in `transactions` itself without
bringing in dependencies like secp and rlp, so they end up in a separate
module.
Finally, since these modules collect "versions" of these transaction
types across different eips, some tests are moved to follow the same
structure.
As it happens, the two share the exact same interface (even the test
suite removed in this PR passes) - `minilru` has an edge on efficiency
however, avoiding the doubly linked list node allocations etc
Since these types were written, we've gained an executable spec:
https://github.com/ethereum/execution-specs
This PR aligns some of the types we use with this spec to simplify
comparisons and cross-referencing.
Using a `distinct` type is a tradeoff between nim ergonomics, type
safety and the ability to work around nim quirks and stdlib weaknesses.
In particular, it allows us to overload common functions such as `hash`
with correct and performant versions as well as maintain control over
string conversions etc at the cost of a little bit of ceremony when
instantiating them.
Apart from distinct byte types, `Hash32`, is introduced in lieu of the
existing `Hash256`, again aligning this commonly used type with the spec
which picks bytes rather than bits in the name.
* Using unsigned types for message type and requst IDs
why:
Negative values are neither defined for RLP nor in the protocol specs
which refer to the RLPs (see yellow paper app B clause (199).
* Fix `int` argument (must be `uint`) in fuzzing tests
why:
Not part of all tests so it slipped through.
* Restricting exception catcher
why:
`CatchableError` is not needed here
* Check data length before converting to `openArray[]`
why:
Getting the first entry of an `openArray[]` crashes with `IndexDefect`.
This is particularly annoying when decoding messages in rlpx.
* Added unit test using rlpx message that causes this problem to detect
- Rework to have exception raise only at rlp decoding and use
result types from then onwards
- Adjust the current API to have result versions and deprecated
the ones which had var Record + bool
- Add PublickKey to the Record object, as this allows us to skip
fromRaw calls whenever access is needed to the public key
- Add a TypedRecord.fromRecord which cannot fail and deprecate
the old one
- Some other minor clean-up & re-ordering
- Rework adding and updating of fields by having an insert call
that gets used everywhere. Avoiding also duplicate keys. One
side-effect of this is that ENR sequence number will always get
updated on an update call, even if nothing changes.
- Deprecate initRecord as it is only used in tests and is flawed
- Assert when predefined keys go into the extra custom pairs.
Any of the predefined keys are only to be passed now via specific
parameters to make sure that the correct types are stored in ENR.
- Clearify the Opt.none behaviour for Record.update
- When setting ipv6, allow for tcp/udp port fields to be used
default
- General clean-up
- Rework/clean-up completely the ENR tests.
A first step in cleaning up RLP, which has lots of interesting issues -
the next step would be to clean up the exception handling as well
(Resultify?)
* remove `RlpNode` (unused)
* single-pass parsing for most functionality via RlpItem
* stricter conformance to spec
* remove float support
* warn about signed integers
* raise on invalid RLP earlier
* avoid several pointless allocations, in particular in `listLen`,
`listElem` etc
* include spec docs
EIP-4844 blob sidecars are a concept that only exists in the mempool.
After inclusion of a transaction into an execution block, only the
versioned hash within the transaction remains. To improve type safety,
replace the `Transaction.networkPayload` member with a wrapper type
`PooledTransaction` that is used in contexts where blob sidecars exist.
This is more according to the structure of the code itself.
And also, currently some of these tests live under tests/rlp but
some under tests/common. Could use some further re-organisation
within the different tests, but this move is a first step.
1. Fix Assertion error when receipt is not a List nor has a single
byte value. Receiving such garbage data would cause a crash.
2. Fix decoding of Receipt list by adding the missing Blob
encapsulation
Also added tests for these scenarios.
This also makes the uTP SendCallBack not returning a Future any
more as it is not used in sendData anyhow. And in case of uTP
over discv5, discv5 send call is already not async.
This gives quite a noticable throughput benchmark improvement over
with uTP over UDP, and a slightly noticable with uTP over discv5
* Fix chronos related deprecated warnings in discv5
* Fix chronos related deprecated warnings in uTP code
* Improve discv5 closeWait order of events and add noCancel
* Refactor p2pProtocol internals
* Attempt to fix rlp crash with Nim v2 (#658)
* Attempt to fix rlp crash with Nim v2
* Fix test_ecies for nim v2
* Reduce compiler warnings
* Resolve ambiquity in testutils
* Disable nim devel continue-on-error
- Add a multiple sockets use test for uTP over discv5
- Use assign2 for the biggest consumer of genericAssignAux in uTP
- Avoid calling exists on the growable buffer when there is no
place in the socket window.
`blobGasUsed` was not mentioned in `test_rlp_codec` test names, despite
being used. Further, update the idea of `genOptionalFieldsValidation`
to also check for `blobGasUsed`, and fix the check to catch unsupported
combinations of optional fields.
* Replace registerCustomScalarFunction with createCustomFunction
The nice thing about `registerCustomScalarFunction` and
`registerCustomScalarFunction` was that it kept the sqlite calls
of type sqlite3_value_xxx inside kvstore_sqlite.
The big downside however is that the "custom" call is awfully
specific due to the specific function signature it demands.
It is likely that for each different use case, a new version
needs to be added.
In this PR the code gets altered to `createCustomFunction` which
allows to just create a custom function with directly xFunc
signature of sqlite. The downside of this is that you still need
to import sqlite_abi in your local code to have access to each of
the sqlite3_value_xxx and sqlite3_result_xxx calls. These could of
course also be wrapped.
* Fix custom sql func test
Csaba Kiraly
andri lim
tersec
Jacek Sieka
Kim De Mey
Jordan Hrycaj
Etan Kissling
web3-developer