feat(l1): implement eth/69 #2860
Conversation
…com:lambdaclass/ethrex into refactor/capability-struct-instead-of-tuple
…com:lambdaclass/ethrex into refactor/capability-struct-instead-of-tuple
|
A way to test the new version is to change the go-ethereum release to |
| let decoder = Decoder::new(&decompressed_data)?; | ||
| let (id, decoder): (u64, _) = decoder.decode_field("request-id")?; | ||
| let (receipts, _): (Vec<Vec<Receipt>>, _) = decoder.decode_field("receipts")?; | ||
| if has_bloom(msg_data)? { |
There was a problem hiding this comment.
but wait, when we call decode, we don't know which protocol we're in?
There was a problem hiding this comment.
I haven't read the spec for the change on eth/69, but if we support both versions, and the remote peer supports both versions, can we know which version the remote peer will be using without looking into the message data?
I mean, is the newest matching version assumed or the peers are free to send eth/68 or eth/69 if both are supported?
In any case, I think this implementation is free from expecting proper behavior from remote peers.
There was a problem hiding this comment.
Exactly that. we don't know which version of the protocol we're using when decoding the message if we support both version. Whenever we deprecate eth/68, this code becomes obsolete, and the bloom field shouldn't be send (it can be derived from the logs).
Even if determine to use a version of the protocol with a given peer, there isn't an easy way to tell de decoder which version we want
There was a problem hiding this comment.
But even if you support both versions, does this mean that you can use both versions at the same time? I was assuming that if both support 69, then you just use 69, whereas if one only support 68, then you use 68. Isn't that the way it works?
There was a problem hiding this comment.
Let me know if my reasoning is incorrect:
I am talking to peer1. In the handshake, I said I support both 68 and 69, he said he supports only 68. So we will talk only in eth68. When I receive a message from peer1, I know that we agreed to talk in eth68, so I know beforehand what "decode" I should use. I don't need a sophisticated logic to infere which version the message is.
There was a problem hiding this comment.
If multiple versions are shared of the same (equal name) capability, the numerically highest wins, others are ignored.
https://github.com/ethereum/devp2p/blob/master/rlpx.md
There was a problem hiding this comment.
That means that our decoding code should receive a context that includes the capability version.
There was a problem hiding this comment.
yes, or maybe after handshake, somehow you just initialize Receipts69 (that implements Receipt trait)
| pub(crate) block_hash: BlockHash, | ||
| pub(crate) genesis: BlockHash, | ||
| pub(crate) fork_id: ForkId, | ||
| pub enum StatusMessage { |
There was a problem hiding this comment.
why do we have an enum instead of having StatusMessage68 and StatusMessage69 implementing RLPxMessage individually?
There was a problem hiding this comment.
when we decode the message in crates/networking/p2p/rlpx/message.rs:110 we don't know which version the message is, yet. Using the enum as a proxy let us change the decode implementation depending on the version and from the Message struct perspective is the same.
|
looking great, just a small comment |
### **Motivation** A new version of the eth protocol has been released (eth/69) and some clients are supporting both eth/68 and eth/69 at the same time. We need a way of being able to encode/decode the msg from both versions of the protocol. Also, we need a way to remove easily the old version when it becomes deprecated. The new version includes changes in the status and receipts messages and a new message RangeBlockUpdate. ### **Description** A new file structure was created to accommodate the new implementation: crates/networking/p2p/rlpx/eth/ crates/networking/p2p/rlpx/eth/eth68/ -> (status.rs and receipt.rs) crates/networking/p2p/rlpx/eth/eth69/ -> (status.rs and receipt.rs) In the new eth/69 protocol the messages status and receipts have been changed, so a new intermediate structure was created to address this. The encode for each version is decided by the type of the inner struct, but the decode is not that straight forward as we don't know what version of msg we received. For the status msg was easier, as we received the eth version inside the msg we can decide which version of the protocol we are using. For the receipt msg, the only difference is the bloom field. A new function `has_bloom` has been implemented to detect when a msg have the bloom field to process it as a eth/68. The newer eth/69 receipts msg won't have the bloom field (it can be calculated from the logs). Also, as the new structure is a proxy to the different version of the structure some new getters were needed. The new msg BlockRangeUpdate was introduced with eth/69. This message includes the earliest block available, the latest block available and the its hash. This messages is been send once every epoch (aprox 6 mins). In some places, the `receipts` need to be encoded using the _bloom_ field to validate the receipt_root of a block header. The function encode_inner_with_bloom was used for these cases. ### **Future improvements** - We're not using the received `BlockRangeUpdate` message, we can use this information to discard peers quickly. - The function `has_bloom` probably could be improved or at least not use private decode functions. - As mentioned before, this implementation is made with the assumption that eth/68 is going to be deprecated in the future. - We are currently not running the latest version of hive that includes the Closes lambdaclass#2805 & lambdaclass#2785 References: [EIP-7642](https://eips.ethereum.org/EIPS/eip-7642#rationale) [geth implementation](ethereum/go-ethereum#29158) [eth/69](https://github.com/ethereum/devp2p/blob/master/caps/eth.md)
Motivation
A new version of the eth protocol has been released (eth/69) and some clients are supporting both eth/68 and eth/69 at the same time. We need a way of being able to encode/decode the msg from both versions of the protocol. Also, we need a way to remove easily the old version when it becomes deprecated.
The new version includes changes in the status and receipts messages and a new message RangeBlockUpdate.
Description
A new file structure was created to accommodate the new implementation:
crates/networking/p2p/rlpx/eth/
crates/networking/p2p/rlpx/eth/eth68/ -> (status.rs and receipt.rs)
crates/networking/p2p/rlpx/eth/eth69/ -> (status.rs and receipt.rs)
In the new eth/69 protocol the messages status and receipts have been changed, so a new intermediate structure was created to address this. The encode for each version is decided by the type of the inner struct, but the decode is not that straight forward as we don't know what version of msg we received.
For the status msg was easier, as we received the eth version inside the msg we can decide which version of the protocol we are using.
For the receipt msg, the only difference is the bloom field. A new function
has_bloomhas been implemented to detect when a msg have the bloom field to process it as a eth/68. The newer eth/69 receipts msg won't have the bloom field (it can be calculated from the logs).Also, as the new structure is a proxy to the different version of the structure some new getters were needed.
The new msg BlockRangeUpdate was introduced with eth/69. This message includes the earliest block available, the latest block available and the its hash. This messages is been send once every epoch (aprox 6 mins).
In some places, the
receiptsneed to be encoded using the bloom field to validate the receipt_root of a block header. The function encode_inner_with_bloom was used for these cases.Future improvements
BlockRangeUpdatemessage, we can use this information to discard peers quickly.has_bloomprobably could be improved or at least not use private decode functions.Closes #2805 & #2785
References:
EIP-7642
geth implementation
eth/69