Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: sync committee contribution #1284

Merged
merged 26 commits into from
Sep 10, 2024
Merged

feat: sync committee contribution #1284

merged 26 commits into from
Sep 10, 2024

Conversation

rodrigo-o
Copy link
Collaborator

@rodrigo-o rodrigo-o commented Aug 26, 2024 •
edited
Loading

Motivation

SyncCommitteeMessages are aggregated by sync committee aggregators. This is the next step in the Sync Committee feature addition.

Description

This PR adds the calculation of aggregate duties, it's lookahead, and sync contribution production and publish. [Link to the Validator Spec]

Closes #835
Closes #837
Closes #841

rodrigo-o added 18 commits August 26, 2024 18:30
@rodrigo-o
Merge branch 'sync-committee-message-production' into sync_committee_…
3c9803d 
…contribution - squash

commit 2a24d5b
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Mon Aug 26 18:27:10 2024 -0300

    Removed function needed in next PRs but not used

commit f8b84b7
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Mon Aug 26 18:15:39 2024 -0300

    Format

commit d60cea4
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Mon Aug 26 18:11:28 2024 -0300

    Fixed an issue at Sync committee period change after second change

commit bb56817
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Mon Aug 26 13:43:25 2024 -0300

    Some additional clean-up

commit 57ef340
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Aug 23 20:58:56 2024 -0300

    duties clean up + logging

commit 97e6b3a
Merge: 3215928 9835380
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Aug 23 20:44:27 2024 -0300

    Merge branch 'main' into sync-committee-message-production

commit 3215928
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 22 20:04:40 2024 -0300

    format

commit fc07190
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 22 20:03:28 2024 -0300

    Checked duties working on period recalculation

commit 9f68516
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Wed Aug 21 18:55:55 2024 -0300

    Initial sync_committee duty recalculation

commit 4d72f18
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Wed Aug 21 12:55:42 2024 -0300

    Messages broadcasted to the network

commit 868bac4
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Aug 20 18:48:19 2024 -0300

    Fixed small issue during duties update

commit d9106c7
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Aug 20 18:06:26 2024 -0300

    Initial test of sync aggregates without publishing

commit 75501d5
Merge: eab7a05 666ff39
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Mon Aug 19 17:30:35 2024 -0300

    Merge branch 'validator-state-management-refactor' into sync-committee-message-production

commit 666ff39
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Aug 16 17:15:23 2024 -0300

    Added a comment from the previous implementation

commit 5ff062b
Merge: 3defc24 43a40d4
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Aug 16 16:45:09 2024 -0300

    Merge branch 'main' into validator-state-management-refactor

commit 3defc24
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Aug 16 14:18:52 2024 -0300

    Cleaned up logging

commit 2d4690d
Merge: 7675b4b 84ee168
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Aug 16 13:39:21 2024 -0300

    Merge branch 'main' into validator-state-management-refactor

commit 7675b4b
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Aug 16 11:09:41 2024 -0300

    Calculate next epoch duties ahead of time

commit 14ce12f
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Aug 16 00:31:17 2024 -0300

    Refactored Duties functions out of the ValidatorSet

commit d9e8d21
Merge: 18f0564 f0f8111
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 15 18:03:57 2024 -0300

    Merge branch 'main' into validator-state-management-refactor

commit 18f0564
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 15 17:35:07 2024 -0300

    Simplified comute_attesters_for_epoch and removed slot from duties

commit 0987fc2
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 15 15:05:46 2024 -0300

    Quick fix regarding attesters calculation

commit e17a224
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 15 15:01:42 2024 -0300

    Further clean duties

commit 35d2c51
Merge: 8c1d818 2f77b11
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 15 11:47:42 2024 -0300

    Merge branch 'main' into validator-state-management-refactor

commit 8c1d818
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 15 11:46:01 2024 -0300

    Initial Duties cleanup

commit c030f1b
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 15 11:10:15 2024 -0300

    ValidatorSet and Validator cleanup

commit 5331a63
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Wed Aug 14 17:11:30 2024 -0300

    Epoch 0 completely working from ValidatorSet

commit c99729f
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Wed Aug 14 11:57:45 2024 -0300

    handle_tick completely moved to ValidatorSet

commit 3abb064
Merge: 37a593d 52f5e0f
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Wed Aug 14 10:29:38 2024 -0300

    Merge branch 'main' into validator-state-management-refactor

commit 37a593d
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Aug 13 17:40:30 2024 -0300

    Fix some warnings from compile and credo

commit 654b5ea
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Aug 13 17:12:15 2024 -0300

    Formatted and fixed all lint and dialyzer issues

commit cde101c
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Aug 13 16:36:11 2024 -0300

    Fixed how keystore functions handle the validator set

commit 0c85ce1
Merge: 616f892 f38288c
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Aug 13 15:05:12 2024 -0300

    Merge branch 'main' into validator-state-management-refactor

commit 616f892
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Aug 13 12:28:01 2024 -0300

    Removed slot and root from Validator and started cleaning up the ValidatorSet

commit cfc5efe
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Mon Aug 12 23:56:23 2024 -0300

    Hybrid version working with new_head handled at the ValidatorSet

commit a604213
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Aug 9 19:38:39 2024 -0300

    Notify Head now working from the ValidatorSet instead of Validators

commit 69150f2
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 8 20:57:01 2024 -0300

    Initial attesters_for_epoch implementation working

commit cb0b580
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 8 14:09:25 2024 -0300

    Calculate proposers for the whole epoch

commit 6046c52
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Wed Aug 7 20:06:54 2024 -0300

    ValidatorSet rename + started moving duties to all validators

commit 9a3b4c3
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Wed Aug 7 15:08:58 2024 -0300

    Created the new ValidatorPool and moved everything to work with it instead of Setup

commit e7faf69
Merge: 2055e78 4c814a3
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Mon Aug 5 20:05:20 2024 -0300

    Merge branch 'main' into validator-state-management-refactor

commit 2055e78
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Aug 2 15:48:53 2024 -0300

    Small cleanup of the Validator.Setup

commit b9423e1
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 1 12:12:07 2024 -0300

    Small fix regarding an info message that should be debug and a unused var

commit b1e9a3e
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Thu Aug 1 11:56:01 2024 -0300

    Ticker removal

commit 170db89
Merge: 5721ccf ae42589
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Wed Jul 31 17:31:26 2024 -0300

    Merge branch 'main' into validator-manager-genserver-removal

commit 5721ccf
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Wed Jul 31 17:27:03 2024 -0300

    Remove unneded diffs

commit 9eed365
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Wed Jul 31 17:20:39 2024 -0300

    Added an async subscribe to topic to avoid issues in test

commit 55c5d90
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 30 21:19:05 2024 -0300

    Simplify a diff

commit 46efa3a
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 30 21:11:17 2024 -0300

    Fixed some dialyzer issues removing unused functions

commit ff42301
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 30 21:05:25 2024 -0300

    Simplified the Ticker and added dialyzer to the 'make lint' task

commit 2de70bf
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 30 17:45:34 2024 -0300

    	Small fixe after renaming the ticker

commit 3802005
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 30 17:14:06 2024 -0300

    renamed validator manager and clock to Validator.Setup and Ticker

commit e3d7327
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 30 17:05:33 2024 -0300

    ValidatorsManager Genserver removal

commit cb99b04
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 30 15:07:07 2024 -0300

    Format and genesis_time addition to libp2p starts on tests

commit b5c75b4
Merge: 9985081 40faca6
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 30 13:17:23 2024 -0300

    Merge branch 'main' into validator-manager-genserver-removal

commit 9985081
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 30 12:03:13 2024 -0300

    Remove unneded diffs

commit b8623ee
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Mon Jul 29 20:10:28 2024 -0300

    Make the clock a ticker

commit 3c5961f
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Fri Jul 26 14:11:21 2024 -0300

    Finally fixing the issue

commit 11c9b72
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 23 18:53:19 2024 -0300

    Just deactivated attestation publish to check the node running without issues

commit c85151e
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 23 14:22:25 2024 -0300

    Moved notify_tick to libp2p to be sure the issues is just with new blocks and not with libp2p calling the ValidatorManager

commit b6c2bed
Author: Rodrigo Oliveri <rodrigooliveri10@gmail.com>
Date:   Tue Jul 23 12:46:23 2024 -0300

    This are the minimum changes to reproduce the invalid signature error
@rodrigo-o
Added sync_committee_aggregator? function
324474d
@rodrigo-o
Added get_sync_committee_selection proof
d9c6d83
@rodrigo-o
Merge branch 'main' into sync_committee_contribution
ead256a
@rodrigo-o
Initial aggregation duties calculation effort
7fce4c0
@rodrigo-o
Make the sync committee aggregator check work at calculating duties, …
e8bfecf 
…and do it for every epoch
@rodrigo-o
Fixed warning temporarily
e6918c3
@rodrigo-o
Addition of collect and start looking into aggregate publish
b39c5d4
@rodrigo-o
Added sync_subnet_info and renamed the old subnet_info module
7573e02
@rodrigo-o
Fixed typo on handle_gossip
6c86446
@rodrigo-o
Merge branch 'main' into sync_committee_contribution
8fe7938
@rodrigo-o
Added all the path to reach to aggregation, aggregate still WIP
1c00037
@rodrigo-o
Contribution generated and published, but not validated to be picked up
1a59abf
@rodrigo-o
Fixed an issue regarding agregation bit size outside of minimal and f…
22bfd13 
…iltering aggregators only in the subnet they aggreagte
@rodrigo-o
format
74f102c
@rodrigo-o
Fixed boundary check + sync committe message duplication in contribution
603a428
@rodrigo-o
Fixed sync contribution signature calculation for validators repeated…
03663ef 
… in the same subcommittee
@rodrigo-o
Fixed Duties update that reduced the ammount of broadcasters every slot
64a0d46
@rodrigo-o rodrigo-o mentioned this pull request Sep 6, 2024
Open
@rodrigo-o
Copy link
Collaborator Author

rodrigo-o commented Sep 6, 2024 •
edited
Loading

Right now we are in a stable position both in the minimal and the main spec, here is an example of a 200 epoch run in minimal:

image
Here is an example of another node picking up contributions published by ourselves in the first, second and third subcommittee

image
Here is the whole run still on going

rodrigo-o
rodrigo-o commented Sep 6, 2024
View reviewed changes
Makefile Show resolved Hide resolved
@rodrigo-o rodrigo-o marked this pull request as ready for review September 8, 2024 21:48
@rodrigo-o rodrigo-o requested a review from a team as a code owner September 8, 2024 21:48
@rodrigo-o rodrigo-o mentioned this pull request Sep 9, 2024
Closed
rodrigo-o
rodrigo-o commented Sep 9, 2024
View reviewed changes
lib/lambda_ethereum_consensus/validator/duties.ex
Comment on lines +29 to +41
@type sync_committee_aggregator_duty :: %{
aggregated?: boolean(),
selection_proof: Bls.signature(),
contribution_domain: Types.domain(),
subcommittee_index: Types.uint64()
}

@type sync_committee_duty :: %{
# Given that we send messages in EVERY slot, instead
# of tracking them all, we just keep track of the last
# slot we broadcasted to avoid double publishs.
last_slot_broadcasted: Types.slot(),
broadcasted?: boolean(),
message_domain: Types.domain(),
validator_index: Types.validator_index(),
subnet_ids: [Types.uint64()],
validator_index: Types.validator_index()
aggregation: [sync_committee_aggregator_duty()]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duties haven't been refactored, if we agreed with this way to represent them we might want to move attestation duties to follow this same pattern, separating the main attestation from its aggregation duty. This shouldn't impact the code a lot but should be easier to read and clearly separate the two kind of duties.

Arkenan
Arkenan approved these changes Sep 10, 2024
View reviewed changes
Copy link
Collaborator

@Arkenan Arkenan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Rodri, thanks for the PR. Here are some comments. Most are style changes, although there are some performance concerns that we already talked about offline. Feel free to open new issues for those, + TODOs in code referring to them.

test/unit/sync_subnet_info.exs Outdated Show resolved Hide resolved
lib/lambda_ethereum_consensus/validator/validator_set.ex Outdated Show resolved Hide resolved
lib/lambda_ethereum_consensus/validator/validator_set.ex Show resolved Hide resolved
lib/lambda_ethereum_consensus/validator/validator.ex Show resolved Hide resolved
lib/types/subnet_info.ex Show resolved Hide resolved
lib/lambda_ethereum_consensus/validator/validator.ex Outdated Show resolved Hide resolved
lib/lambda_ethereum_consensus/validator/utils.ex Show resolved Hide resolved
lib/lambda_ethereum_consensus/validator/duties.ex Outdated Show resolved Hide resolved
lib/lambda_ethereum_consensus/p2p/gossip/sync_committee.ex Outdated Show resolved Hide resolved
lib/lambda_ethereum_consensus/validator/duties.ex Outdated Show resolved Hide resolved
@rodrigo-o rodrigo-o mentioned this pull request Sep 10, 2024
Open
@rodrigo-o rodrigo-o merged commit 2cf6ef5 into main Sep 10, 2024
13 checks passed
@rodrigo-o rodrigo-o deleted the sync_committee_contribution branch September 10, 2024 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Arkenan Arkenan Arkenan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Lambda Ethereum Consensus Node
Status: Done
Milestone
No milestone
2 participants
@rodrigo-o @Arkenan