-
Notifications
You must be signed in to change notification settings - Fork 231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update to latest light client libp2p protocol #3623
Conversation
d46b934
to
c0dadb9
Compare
Tested this against the test cases in the consensus-specs repo, and also manually checked logs of local testnet to ensure the gossip is properly sent. |
tests/consensus_spec/altair/test_fixture_sync_protocol_update_ranking.nim
Show resolved
Hide resolved
c882132
to
a7f322e
Compare
Incorporates the latest changes to the light client sync protocol based on Devconnect AMS feedback. Note that this breaks compatibility with the previous prototype, due to changes to data structures and endpoints. See ethereum/consensus-specs#2802 Due to the amount of changes, it is best to review the heavily edited files from scratch (likewise, for the spec).
9b7651e
to
d82b9c4
Compare
Ran CI tests 5 times, flakiness seems to be gone now. There was some randomness involved, in the way how we simulated sync committee signatures, where sometimes it would generate too many and would actually reach supermajority, breaking the test case assumption. This is now fixed by putting a cap on randomization to not exceed the supplied target participation percentage. |
beacon_chain/consensus_object_pools/block_pools_types_light_client.nim
Outdated
Show resolved
Hide resolved
Linux i386 Nim 1.2:
|
https://ci.status.im/blue/organizations/jenkins/nimbus%2Fnimbus-eth2%2Fmultibranch/detail/PR-3623/19/pipeline/87 shows the same thing, even after 9bcc2b3:
|
## Latest slot for which updates were broadcasted on libp2p gossip. | ||
## Tracks `update.signature_slot`. | ||
|
||
broadcastGossipFut*: Future[void] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this belongs in BeaconNode
(which is where validator_duties keeps things for its own needs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved it into a separate pool type, as it is also used in gossip_validation
(minus the Future
). I think it's a nice compromise between having it in the block_pools_types_light_client
where it clearly does not belong, and fragmenting these few vars across too many files.
Manually validated against light client poc. |
Incorporates the latest changes to the light client sync protocol based
on Devconnect AMS feedback. Note that this breaks compatibility with the
previous prototype, due to changes to data structures and endpoints.
See ethereum/consensus-specs#2802
Due to the amount of changes, it is best to review the heavily edited
files from scratch (likewise, for the spec).