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.

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

validatormock: integrate DutyPrepareSyncContribution #1384

Merged
merged 6 commits into from
Nov 4, 2022

Conversation

xenowits
Copy link
Contributor

@xenowits xenowits commented Nov 3, 2022

Integrate DutyPrepareSyncContribution into validatormock.

category: feature
ticket: #1262

@xenowits xenowits self-assigned this Nov 3, 2022
func (s *SyncCommMember) Aggregate(ctx context.Context, slot eth2p0.Slot) error {
wait(ctx, s.getSelectionsOK(slot))
wait(ctx, s.dutiesOK, s.getSelectionsOK(slot))
// TODO(xenowits): Add aggregate function.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will be addressed in this ticket

@xenowits xenowits force-pushed the xenowits/integrate-prep-synccontrib branch from 847c633 to 179f6a0 Compare November 3, 2022 11:02
@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Base: 54.15% // Head: 53.66% // Decreases project coverage by -0.48% ⚠️

Coverage data is based on head (ee1fe32) compared to base (b2cc747).
Patch coverage: 0.00% of modified lines in pull request are covered.

❗ Current head ee1fe32 differs from pull request most recent head 30c2c2f. Consider uploading reports for the commit 30c2c2f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1384      +/-   ##
==========================================
- Coverage   54.15%   53.66%   -0.49%     
==========================================
  Files         144      144              
  Lines       17804    17860      +56     
==========================================
- Hits         9641     9584      -57     
- Misses       6836     6950     +114     
+ Partials     1327     1326       -1     
Impacted Files Coverage Δ
app/vmock.go 72.72% <ø> (-1.07%) ⬇️
testutil/validatormock/attest.go 58.49% <ø> (ø)
testutil/validatormock/synccomm.go 0.00% <0.00%> (ø)
testutil/validatormock/validatormock.go 33.33% <ø> (ø)
core/qbft/qbft.go 72.26% <0.00%> (-9.25%) ⬇️
app/peerinfo/peerinfo.go 54.48% <0.00%> (-3.45%) ⬇️
core/priority/prioritiser.go 61.65% <0.00%> (-3.11%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

testutil/validatormock/synccomm.go Outdated Show resolved Hide resolved
testutil/validatormock/synccomm.go Outdated Show resolved Hide resolved
) (syncSelections, error) {
return nil, nil
// prepareSubcommittees returns the assignment of validators to sync subcommittees. It assumes that all validators are included in all sync subnets (subcommittees).
func prepareSubcommittees(ctx context.Context, eth2Cl eth2wrap.Client, duties syncDuties) (subCommittees, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use the SyncCommitteeDuty.ValidatorSyncCommitteeIndices field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah

Copy link
Contributor

Choose a reason for hiding this comment

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

remove this method, you can get the subcommittees from the duty directly in prepareSyncSelections

// ])
// . We can infer that the validator is assigned to two subnets (subcommittees), zero and three since: subnets = [40 / 128, 450 / 128 ] = [0, 3].

for _, commIdx := range duty.ValidatorSyncCommitteeIndices {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

testutil/validatormock/synccomm.go Outdated Show resolved Hide resolved
) (syncSelections, error) {
return nil, nil
// getSubcommittees returns the subcommittee indexes for the provided sync committee duty.
func getSubcommittees(ctx context.Context, eth2Cl eth2client.SpecProvider, duty eth2v1.SyncCommitteeDuty) ([]eth2p0.CommitteeIndex, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, move this below prepareSyncSelections

@xenowits xenowits added the merge when ready Indicates bulldozer bot may merge when all checks pass label Nov 4, 2022
@obol-bulldozer obol-bulldozer bot merged commit 9f10449 into main Nov 4, 2022
@obol-bulldozer obol-bulldozer bot deleted the xenowits/integrate-prep-synccontrib branch November 4, 2022 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge when ready Indicates bulldozer bot may merge when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants