mirror of https://github.com/status-im/nim-eth.git
04ff8e460f
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> |
||
|---|---|---|
| .. | ||
| eth_types.nim | ||
| eth_types_json_serialization.nim | ||
| state_accessors.nim | ||
| transaction.nim | ||
| utils.nim | ||