feat(l1): properly calculate enr sequence field
#2679
Conversation
Lines of code reportTotal lines added: Detailed view |
cmd/ethrex/l2.rs
Outdated
| peer_table.clone(), | ||
| local_p2p_node, | ||
| local_node_record.clone(), |
There was a problem hiding this comment.
why do we have to pass a node record through rpc?
There was a problem hiding this comment.
The admin_nodeInfo msg requires the node record
See crates/networking/rpc/rpc.rs:446
There was a problem hiding this comment.
I was thinking about this, and for the moment a copy of the node record is enough
| @@ -552,8 +554,9 @@ impl Discv4Server { | |||
| tcp_port: node.tcp_port, | |||
| }; | |||
|
|
|||
| let ping = | |||
| Message::Ping(PingMessage::new(from, to, expiration).with_enr_seq(self.ctx.enr_seq)); | |||
| let enr_seq = self.ctx.local_node_record.lock().await.seq; | |||
There was a problem hiding this comment.
is enr ever updated? seems like a constant to me here
There was a problem hiding this comment.
the enr is not being change anywhere in the code, so the sequence is also not updated. The important part is to have a reference to the node_record in every part of the code to get the seq number.
There was a problem hiding this comment.
The enr seq is updated in the test discv4::server::tests::discovery_enr_message where a field is changed and after the seq is updated, both servers have the info
There was a problem hiding this comment.
The whole point of the ticket is to implement the logic to how and where to update the enr...
…ass/ethrex into feat/properly-calculate-enr-seq
cmd/ethrex/utils.rs
Outdated
| serde_json::from_reader(file) | ||
| pub fn read_node_config_file(file_path: PathBuf) -> Result<NodeConfigFile, String> { | ||
| match std::fs::File::open(file_path) { | ||
| Ok(file) => serde_json::from_reader(file).map_err(|e| format!("Invlid config file {}", e)), |
Motivation
The seq field in the node record was hardcoded with the unix time.
Description
The enr_seq field is updated by one when the node_record is changed. The ping/pong messages are sent with the enr_seq in it, so the peer knows when an update is made in the node_record. Since we don't modify the node_record yet, the enr_seq is not being updated. There is a new PR incoming (#2654) which is using this funtionality to inform the peers about changes in the node_record.
A reference was added to the p2pcontext in order to be able to access the current NodeRecord seq in several parts of the code.
Some functions firms were changed to accept this improvement.
A new config struct has been built to persist the enr seq field and also store the known peers in the same file.
The test discv4::server::tests::discovery_enr_message checks this feature
enr
Closes #1756