1. Generalises the special cases for serialising RLP `seq[Transaction]`.
Previously it only used the special case inside `BlockBody` and `EthBlock`.
Now it uses it for all `seq[Transaction]` regardless of what objects they
are parts of, or no object at all. `openArray[Transaction]` is also
included, as this was found to be necessary to match in some places.
2. Bug fix parsing `Transaction`: Always read the first byte to get the
transaction type instead of parsing an RLP `int`. This way invalid or
adversarial input gives a correct error (i.e. invalid type code).
When it was read with `rlp.read(int)`, those inputs gave many crazy
messages (e.g. "too large to fit in memory"). In the specification it's a
byte. (Technically the input is not RLP and we shouldn't be using the RLP
parser anyway to parse standalone transaction objects).
3. Bug fix parsing `Transaction`: If a typed transaction is detected in
`seq[Transaction]`, the previous code removed the RLP (blob) wrapper, then
passed the contents to `read(Transaction)`. That meant a blob-wrapped
legacy transaction would be accepted. This is incorrect. The new code
passes the contents to the typed transaction decoder, which correctly
rejects a wrapped legacy transaction as having invalid type.
Change 1 has a large, practical effect on `eth/65` syncing with peers.
Serialisation of `eth` message types `Transactions` and `PooledTransactions`
have been broken since the introduction of typed transactions (EIP-2718), as
used in Berlin/London forks. (The special case for `seq[Transaction]` inside
`BlockBody` only fixed message type `BlockBodies`.)
Due to this, whenever a peer sent us a `Transactions` 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 within a few tens of
seconds due to this.
This didn't get noticed 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 typed transactions. Also, we didn't really investigate occasional
disconnects before, we assumed they're just part of P2P life.
The root cause is the RLP serialisation of individual `Transaction` is meant to
be subtly different from arrays/sequences of `Transaction` objects in network
messages. RFC-2976 covers this but it's quite subtle:
- Individual transactions are encoded and stored as either `RLP([fields..])`
for legacy transactions, or `Type || RLP([fields..])`. Both of these
encodings are byte sequences. The part after `Type` doesn't have to be
RLP in theory, but all types so far use RLP. EIP-2718 covers this.
- In arrays (sequences), transactions are encoded as either `RLP([fields..])`
for legacy transactions, or `RLP(Type || RLP([fields..]))` for all typed
transactions to date. Spot the extra `RLP(..)` blob encoding, to make it
valid RLP inside a larger RLP. EIP-2976 covers this, "Typed Transactions
over Gossip", although it's not very clear about the blob encoding.
In practice the extra `RLP(..)` applies to all arrays/sequences of transactions
that are to be RLP-encoded as a list. In principle, it should be all
aggregates (object fields etc.), but it's enough for us to enable it for all
arrays/sequences, as this is what's used in the protocol and EIP-2976.
Signed-off-by: Jamie Lokier <jamie@shareable.org>
Storing large blobs in a "WITHOUT ROWID" table turns out to be extremely
slow when the tree must be rebalanced.
* Split out keystore capability into separate interface, making each
keystore a separate instance
* Disable "WITHOUT ROWID" optimization by default
* Implement prefix lookup that allows iterating over all values with a
certain prefix in their key
* Fix raw Exceptions in hexary caused by forward declarations
* Fix raw Exceptions in trie/db caused by forward declarations
* And now we can remove those db Proc CatchableError raises
`les_protocol.nim` failed to build, due to very silly Nim bugs
nim-lang/Nim#8792 and nim-lang/Nim#17102.
import
../../[rlp, keys], ../../common/eth_types,
../[rlpx, kademlia, blockchain_utils], ../private/p2p_types,
The silly part is `../` has to be quoted if it's before a group of files, but
not before a single file. Most places in PR #344 / 7624153 use the workaround
`".."/` but it was missed in `les_protocol.nim`:
nimbus-eth1/vendor/nim-eth/eth/p2p/rlpx_protocols/les_protocol.nim(14, 3)
Error: cannot open file: ../../[rlp,keys]
Signed-off-by: Jamie Lokier <jamie@shareable.org>
* Add build_dcli target and add it to CI
* Fix local imports for dcli
* And use local imports for all other files too
* Use local imports in tests and rlpx protocols
Fixesstatus-im/nim-eth#341, status-im/nimbus-eth1#489.
When using discv4 (Kademlia) to find peers, there is a crash after a few
minutes. It occurs for most of us on Eth1 mainnet, and everyone on Ropsten.
The cause is `findNodes` being called twice in succession to the same peer,
within about 5 seconds of each other. ("About" 5 seconds, because Chronos does
not guarantee to run the timeout branch at a particular time, due to queuing
and clock reading delays.)
Then `findNodes` sends a duplicate message to the peer and calls
`waitNeighbours` to listen for the reply. There's already a `waitNeighbours`
callback in a shared table, so that function hits an assert failure.
Ignoring the assert would be wrong as it would break timeout logic, and sending
`FindNodes` twice in rapid succession also makes us a bad peer.
As a simple workaround, just skip `findNodes` in this state and return a fake
empty `Neighbours` reply. This is a bit of a hack as `findNodes` should not be
called like this; there's a logic error at a higher level. But it works.
Tested for about 4 days constant operation on Ropsten. The crash which occured
every few minutes no longer occurs, and discv4 keeps working.
Signed-off-by: Jamie Lokier <jamie@shareable.org>
KonradStaniec
Kim De Mey
Jamie Lokier
Ștefan Talpalaru
jangko
Jacek Sieka