Network altair gossip and discovery#2302
Conversation
fd7fadf to
6b06b22
Compare
16cf433 to
ecddeae
Compare
6b06b22 to
b6728c1
Compare
2ae7861 to
63fd5da
Compare
f54c9ec to
4573f9f
Compare
63fd5da to
713c27b
Compare
|
@AgeManning would appreciate an initial pass over this guy as well. This does not contain the changes for a sync committee subnet service which makes subscriptions for sync committee subnets yet. |
cfb9fc8 to
db847c3
Compare
1d78f67 to
5b6fe3d
Compare
6cef740 to
d251539
Compare
ae7994b to
2abce36
Compare
| // predicate for finding nodes with a matching fork and valid tcp port | ||
| let eth2_fork_predicate = move |enr: &Enr| { | ||
| enr.eth2() == Ok(enr_fork_id.clone()) && (enr.tcp().is_some() || enr.tcp6().is_some()) | ||
| // `next_fork_epoch` and `next_fork_version` can be different so that |
There was a problem hiding this comment.
@AgeManning this might cause compatibility issues with lighthouse nodes that are not altair enabled.
A non altair enabled node will reject an altair node during discovery because it requires the entire enr fork id to be the same. However, nodes with different next_fork_epoch and next_fork_version in their enr fork id should still be accepted according to the spec.
There was a problem hiding this comment.
Can we just match on either current fork digest or next fork version?
I think its fine to discover peers who say they are on the next fork digest. After we dial we'll kick them if they are not compatible.
There was a problem hiding this comment.
Didn't get you.
I'm saying if node A (old networking) runs a discovery query and finds node B (altair networking), then it will reject node B in the discovery step because node A is running predicate enr_fork_digest_a == enr_fork_digest_b which is false. So old nodes will not be able to discover altair nodes (other way works fine). Not sure if I'm missing something here.
There was a problem hiding this comment.
Tested this in a local testnet and it is an issue. The non-altair node rejects the altair nodes. Made a PR to fix the issue here #2433
|
@AgeManning this is passing all tests and ready for review. Also, there's a potential incompatibility with the enr predicate that I have highlighted above. |
bd27ae2 to
88e0227
Compare
## Issue Addressed Closes sigp#2354 ## Proposed Changes Add a `minify` method to `slashing_protection::Interchange` that keeps only the maximum-epoch attestation and maximum-slot block for each validator. Specifically, `minify` constructs "synthetic" attestations (with no `signing_root`) containing the maximum source epoch _and_ the maximum target epoch from the input. This is equivalent to the `minify_synth` algorithm that I've formally verified in this repository: https://github.com/michaelsproul/slashing-proofs ## Additional Info Includes the JSON loading optimisation from sigp#2347
6cb3b8a to
6bbb343
Compare
## Proposed Changes Fixing a typo in the advanced networking docs which mentions ``--target-peer`` instead of the correct ``--target-peers`` flag
…the sync period boundary
Based on "Expand and refactor fork functions"
0afdace to
4bd66ac
Compare
Proposed Changes
Implements gossip and discovery changes for altair hard fork. Builds on
network-altair-rpcbranch.