Repository navigation
feat(discovery): republish the ENR's eth2 entry at fork boundaries - #735
Draft
MegaRedHand wants to merge 3 commits into
Draft
MegaRedHand wants to merge 3 commits into
MegaRedHand wants to merge 3 commits into
Conversation
The served ENR kept its startup digest across every fork-digest boundary, because ethrex gave a consumer no way to edit the record it serves. Peers discovering a long-running node after a boundary read the stale digest and rejected it. ethrex's feat/discv5-lock-free-local-enr makes the SharedLocalNode slot the only copy of the served record: an ArcSwap that discovery reads on every packet and that every writer updates by compare-and-swap. It also adds edit_record, which re-signs with one seq bump and re-applies the edit if IP voting swapped a record in first. The pin moves there, and arc-swap enters the lockfile at 1.9.2, since 1.9.3 is still inside the publish-age cooldown. LocalEnrWriter holds the slot and the node key, and transition::apply offers it the schedule's ENRForkID on every call. It edits only when the served entry differs, since every edit bumps seq and a bumped seq sends each peer back for the whole record. It is a separate, crate-private type so the key does not ride along with LocalEnr into the RPC server. LocalEnr itself now reads the slot without a lock.
The gap the previous commit closes was documented in five places. Two of them used it to explain why the ENR carries no syncnets entry; those now say the record can be edited and nothing writes the set to it yet.
ethrex's update_local_ip sets ip or ip6 and clears the other; it never touches the udp or tcp ports, which the new table claimed it did.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft, stacked on #732. Pins ethrex to
80308d0d, the head of ethrex'sfeat/discv5-lock-free-local-enr. That branch is lambdaclass/ethrex#7488, one commit stacked onfeat/discv5-predicate-queries(#732's pin). Not mergeable until the ethrex side is in a release and the pin goes back to a tag.Motivation
#732 made
/eth/v1/node/identityread the ENR discv5 serves, but the other direction was impossible: ethrex served a private copy of the record and gave a consumer no way to edit it. So the servedeth2entry kept the startup digest across every fork-digest boundary, and peers discovering a long-running node after one read the stale digest and rejected it.What this does
1. ethrex pin bump
ethrex's new commit makes
SharedLocalNodethe only copy of the served record: a lock-freeArcSwapslot that discovery reads on every PING/PONG/NODES/handshake, and that every writer updates by compare-and-swap.SharedLocalNode::load()SharedLocalNode::edit_record(&signer, FnMut(&mut NodeRecordPairs))NodeRecord::edit(oneseqbump, re-signed), swapped in by CAS; on a conflict (e.g. IP voting got in first) the edit is re-applied on the newer recordDiscoveryServer::spawn(local_node, signer, ...)arc-swapenters the lockfile at 1.9.2: 1.9.3 is inside the 14-day cooldown until Nov 1.make cooldown-checkpasses.2. Republishing
eth2applycall, not only on a switch. The compare makes it a no-op between boundaries, and the served entry always matches the scheduleseqand a bumpedseqsends every peer back for an identical recordLocalEnrWriter(crate-private) holds the slot and the node key.LocalEnr(read-only) is what goes to the RPC server, so the key does noteth2from bytes computed once, so re-running it on a newer record gives the same resultupdate_served_enr(the warn-only stub) is gone.Testing
make lint,cargo check --workspace --all-targets --locked,make cooldown-checkcargo test --workspace --profile release-fast --lib --bins: all pass. leanSpec spec suites not run.a_new_fork_id_is_what_discovery_serves_next: the edited record carries the neweth2,seq+ 1, a valid signature, unchangedattnets/quic/ip/udp, and a peer on the new digest admits itrepublishing_the_served_fork_id_leaves_the_record_alone: same entry, byte-identical recorda_crossing_moves_the_served_eth2_entry_once: throughtransition::apply, one edit at the boundary, none on a repeat calla_consumer_edit_is_what_peers_fetchcovers that with two real discv5 nodes.Follow-ups
syncnetscan go in the ENR now (same writer); nothing writes it yet.beacon_wire.mdandspec_deviations.mdnow say so.