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

Queue future attestations instead of throwing error #3632

Merged
merged 1 commit into from
Jan 18, 2022
Merged

Queue future attestations instead of throwing error #3632

merged 1 commit into from
Jan 18, 2022

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Jan 17, 2022

Motivation

There are a lot of error like this after we pass gossip attestations to forkchoice

Jan-17 09:31:23.920 [NETWORK]         error: Error adding aggregated attestation to forkchoice slot=2158356 code=FORKCHOICE_ERROR_INVALID_ATTESTATION, err={"code":"FUTURE_SLOT","attestationSlot":2158356,"latestPermissibleSlot":2158355}
Error: FORKCHOICE_ERROR_INVALID_ATTESTATION

Description

  • It's designed to queue attestations for future reprocess instead of throwing error as in the spec:
  # Attestations can only affect the fork choice of subsequent slots.
    # Delay consideration in the fork choice until their slot is in the past.

and our impl

lodestar/packages/fork-choice/src/forkChoice/forkChoice.ts

Line 434 in c581c18

this.queuedAttestations.add({

  • Other clients, like lighthouse does not throw that error too.
  • Tested successfully in contabo-20

Closes #3631

@codecov
Copy link

codecov bot commented Jan 17, 2022

Codecov Report

Merging #3632 (bd20bfd) into master (c581c18) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3632   +/-   ##
=======================================
  Coverage   37.20%   37.20%           
=======================================
  Files         312      312           
  Lines        8451     8451           
  Branches     1316     1316           
=======================================
  Hits         3144     3144           
  Misses       5160     5160           
  Partials      147      147

@codeclimate
Copy link

codeclimate bot commented Jan 17, 2022

Code Climate has analyzed commit bd20bfd and detected 0 issues on this pull request.

View more on Code Climate.

@github-actions
Copy link
Contributor

Performance Report

✔️ no performance regression detected

Full benchmark results
Benchmark suite Current: ce1f45b Previous: 5295a78 Ratio
BeaconState.hashTreeRoot - No change 650.00 ns/op 653.00 ns/op 1.00
BeaconState.hashTreeRoot - 1 full validator 145.07 us/op 171.95 us/op 0.84
BeaconState.hashTreeRoot - 32 full validator 2.1376 ms/op 2.6720 ms/op 0.80
BeaconState.hashTreeRoot - 512 full validator 28.826 ms/op 34.015 ms/op 0.85
BeaconState.hashTreeRoot - 1 validator.effectiveBalance 149.90 us/op 181.28 us/op 0.83
BeaconState.hashTreeRoot - 32 validator.effectiveBalance 2.3999 ms/op 3.3096 ms/op 0.73
BeaconState.hashTreeRoot - 512 validator.effectiveBalance 33.585 ms/op 36.723 ms/op 0.91
BeaconState.hashTreeRoot - 1 balances 111.98 us/op 118.31 us/op 0.95
BeaconState.hashTreeRoot - 32 balances 851.44 us/op 1.0628 ms/op 0.80
BeaconState.hashTreeRoot - 512 balances 8.3975 ms/op 9.9492 ms/op 0.84
BeaconState.hashTreeRoot - 250000 balances 163.80 ms/op 195.50 ms/op 0.84
processSlot - 1 slots 53.405 us/op 75.162 us/op 0.71
processSlot - 32 slots 3.3771 ms/op 4.1269 ms/op 0.82
getCommitteeAssignments - req 1 vs - 250000 vc 6.0013 ms/op 5.6599 ms/op 1.06
getCommitteeAssignments - req 100 vs - 250000 vc 8.4508 ms/op 8.0216 ms/op 1.05
getCommitteeAssignments - req 1000 vs - 250000 vc 9.0472 ms/op 8.4592 ms/op 1.07
computeProposers - vc 250000 23.432 ms/op 28.097 ms/op 0.83
computeEpochShuffling - vc 250000 210.76 ms/op 212.32 ms/op 0.99
getNextSyncCommittee - vc 250000 402.55 ms/op 463.28 ms/op 0.87
altair processAttestation - 250000 vs - 7PWei normalcase 54.004 ms/op 51.434 ms/op 1.05
altair processAttestation - 250000 vs - 7PWei worstcase 56.381 ms/op 53.361 ms/op 1.06
altair processAttestation - setStatus - 1/6 committees join 12.720 ms/op 16.569 ms/op 0.77
altair processAttestation - setStatus - 1/3 committees join 31.248 ms/op 31.467 ms/op 0.99
altair processAttestation - setStatus - 1/2 committees join 38.611 ms/op 49.415 ms/op 0.78
altair processAttestation - setStatus - 2/3 committees join 47.654 ms/op 66.927 ms/op 0.71
altair processAttestation - setStatus - 4/5 committees join 58.965 ms/op 80.133 ms/op 0.74
altair processAttestation - setStatus - 100% committees join 78.384 ms/op 98.719 ms/op 0.79
altair processAttestation - updateEpochParticipants - 1/6 committees join 16.269 ms/op 17.123 ms/op 0.95
altair processAttestation - updateEpochParticipants - 1/3 committees join 25.661 ms/op 36.273 ms/op 0.71
altair processAttestation - updateEpochParticipants - 1/2 committees join 28.924 ms/op 34.180 ms/op 0.85
altair processAttestation - updateEpochParticipants - 2/3 committees join 39.422 ms/op 43.731 ms/op 0.90
altair processAttestation - updateEpochParticipants - 4/5 committees join 35.258 ms/op 38.309 ms/op 0.92
altair processAttestation - updateEpochParticipants - 100% committees join 36.950 ms/op 40.461 ms/op 0.91
altair processAttestation - updateAllStatus 26.532 ms/op 26.386 ms/op 1.01
altair processBlock - 250000 vs - 7PWei normalcase 56.649 ms/op 55.641 ms/op 1.02
altair processBlock - 250000 vs - 7PWei worstcase 135.39 ms/op 160.38 ms/op 0.84
altair processEpoch - mainnet_e81889 1.2662 s/op 1.2519 s/op 1.01
mainnet_e81889 - altair beforeProcessEpoch 318.41 ms/op 333.89 ms/op 0.95
mainnet_e81889 - altair processJustificationAndFinalization 94.710 us/op 124.71 us/op 0.76
mainnet_e81889 - altair processInactivityUpdates 20.686 ms/op 20.411 ms/op 1.01
mainnet_e81889 - altair processRewardsAndPenalties 256.33 ms/op 208.31 ms/op 1.23
mainnet_e81889 - altair processRegistryUpdates 12.451 us/op 20.423 us/op 0.61
mainnet_e81889 - altair processSlashings 4.3370 us/op 6.6220 us/op 0.65
mainnet_e81889 - altair processEth1DataReset 4.8600 us/op 6.7050 us/op 0.72
mainnet_e81889 - altair processEffectiveBalanceUpdates 14.206 ms/op 14.642 ms/op 0.97
mainnet_e81889 - altair processSlashingsReset 16.571 us/op 37.780 us/op 0.44
mainnet_e81889 - altair processRandaoMixesReset 35.976 us/op 49.325 us/op 0.73
mainnet_e81889 - altair processHistoricalRootsUpdate 6.3270 us/op 9.5760 us/op 0.66
mainnet_e81889 - altair processParticipationFlagUpdates 126.97 ms/op 116.30 ms/op 1.09
mainnet_e81889 - altair processSyncCommitteeUpdates 4.0910 us/op 6.0750 us/op 0.67
mainnet_e81889 - altair afterProcessEpoch 266.32 ms/op 256.21 ms/op 1.04
altair processInactivityUpdates - 250000 normalcase 81.475 ms/op 87.795 ms/op 0.93
altair processInactivityUpdates - 250000 worstcase 78.041 ms/op 88.192 ms/op 0.88
altair processParticipationFlagUpdates - 250000 anycase 112.12 ms/op 105.53 ms/op 1.06
altair processRewardsAndPenalties - 250000 normalcase 185.19 ms/op 167.02 ms/op 1.11
altair processRewardsAndPenalties - 250000 worstcase 177.72 ms/op 162.63 ms/op 1.09
altair processSyncCommitteeUpdates - 250000 414.14 ms/op 470.44 ms/op 0.88
Tree 40 250000 create 933.14 ms/op 1.1866 s/op 0.79
Tree 40 250000 get(125000) 376.87 ns/op 337.29 ns/op 1.12
Tree 40 250000 set(125000) 1.9948 us/op 3.0254 us/op 0.66
Tree 40 250000 toArray() 49.494 ms/op 47.121 ms/op 1.05
Tree 40 250000 iterate all - toArray() + loop 42.020 ms/op 57.966 ms/op 0.72
Tree 40 250000 iterate all - get(i) 140.26 ms/op 134.28 ms/op 1.04
MutableVector 250000 create 29.941 ms/op 30.919 ms/op 0.97
MutableVector 250000 get(125000) 17.865 ns/op 15.455 ns/op 1.16
MutableVector 250000 set(125000) 599.89 ns/op 832.94 ns/op 0.72
MutableVector 250000 toArray() 10.371 ms/op 10.252 ms/op 1.01
MutableVector 250000 iterate all - toArray() + loop 10.683 ms/op 10.143 ms/op 1.05
MutableVector 250000 iterate all - get(i) 3.6806 ms/op 3.7313 ms/op 0.99
Array 250000 create 6.3146 ms/op 6.3564 ms/op 0.99
Array 250000 clone - spread 2.6535 ms/op 3.1652 ms/op 0.84
Array 250000 get(125000) 1.2140 ns/op 1.4160 ns/op 0.86
Array 250000 set(125000) 1.2420 ns/op 1.4410 ns/op 0.86
Array 250000 iterate all - loop 200.84 us/op 149.66 us/op 1.34
aggregationBits - 2048 els - readonlyValues 280.44 us/op 312.30 us/op 0.90
aggregationBits - 2048 els - zipIndexesInBitList 47.752 us/op 55.698 us/op 0.86
regular array get 100000 times 78.108 us/op 58.203 us/op 1.34
wrappedArray get 100000 times 79.985 us/op 59.994 us/op 1.33
arrayWithProxy get 100000 times 35.140 ms/op 35.729 ms/op 0.98
ssz.Root.equals 1.4160 us/op 1.4460 us/op 0.98
ssz.Root.equals with valueOf() 1.6770 us/op 1.8010 us/op 0.93
byteArrayEquals with valueOf() 1.5680 us/op 1.7290 us/op 0.91
phase0 processBlock - 250000 vs - 7PWei normalcase 12.840 ms/op 14.984 ms/op 0.86
phase0 processBlock - 250000 vs - 7PWei worstcase 93.826 ms/op 111.38 ms/op 0.84
phase0 afterProcessEpoch - 250000 vs - 7PWei 255.22 ms/op 245.40 ms/op 1.04
phase0 beforeProcessEpoch - 250000 vs - 7PWei 684.71 ms/op 870.78 ms/op 0.79
phase0 processEpoch - mainnet_e58758 927.51 ms/op 1.0656 s/op 0.87
mainnet_e58758 - phase0 beforeProcessEpoch 546.36 ms/op 620.50 ms/op 0.88
mainnet_e58758 - phase0 processJustificationAndFinalization 104.61 us/op 120.19 us/op 0.87
mainnet_e58758 - phase0 processRewardsAndPenalties 117.30 ms/op 144.88 ms/op 0.81
mainnet_e58758 - phase0 processRegistryUpdates 109.51 us/op 126.76 us/op 0.86
mainnet_e58758 - phase0 processSlashings 3.6650 us/op 7.5180 us/op 0.49
mainnet_e58758 - phase0 processEth1DataReset 4.5280 us/op 6.3590 us/op 0.71
mainnet_e58758 - phase0 processEffectiveBalanceUpdates 11.472 ms/op 11.739 ms/op 0.98
mainnet_e58758 - phase0 processSlashingsReset 17.246 us/op 33.075 us/op 0.52
mainnet_e58758 - phase0 processRandaoMixesReset 25.298 us/op 41.298 us/op 0.61
mainnet_e58758 - phase0 processHistoricalRootsUpdate 5.3310 us/op 8.6400 us/op 0.62
mainnet_e58758 - phase0 processParticipationRecordUpdates 20.671 us/op 30.936 us/op 0.67
mainnet_e58758 - phase0 afterProcessEpoch 209.57 ms/op 207.50 ms/op 1.01
phase0 processEffectiveBalanceUpdates - 250000 normalcase 12.576 ms/op 13.158 ms/op 0.96
phase0 processEffectiveBalanceUpdates - 250000 worstcase 0.5 1.3686 s/op 1.8808 s/op 0.73
phase0 processRegistryUpdates - 250000 normalcase 67.605 us/op 95.254 us/op 0.71
phase0 processRegistryUpdates - 250000 badcase_full_deposits 3.4729 ms/op 4.4438 ms/op 0.78
phase0 processRegistryUpdates - 250000 worstcase 0.5 2.0376 s/op 2.7291 s/op 0.75
phase0 getAttestationDeltas - 250000 normalcase 49.977 ms/op 39.151 ms/op 1.28
phase0 getAttestationDeltas - 250000 worstcase 39.628 ms/op 39.469 ms/op 1.00
phase0 processSlashings - 250000 worstcase 38.117 ms/op 48.459 ms/op 0.79
shuffle list - 16384 els 14.269 ms/op 17.501 ms/op 0.82
shuffle list - 250000 els 203.12 ms/op 215.05 ms/op 0.94
getEffectiveBalances - 250000 vs - 7PWei 12.148 ms/op 14.146 ms/op 0.86
pass gossip attestations to forkchoice per slot 17.410 ms/op 19.981 ms/op 0.87
computeDeltas 3.7787 ms/op 4.2616 ms/op 0.89
getPubkeys - index2pubkey - req 1000 vs - 250000 vc 2.2782 ms/op 2.6566 ms/op 0.86
getPubkeys - validatorsArr - req 1000 vs - 250000 vc 792.46 us/op 836.12 us/op 0.95
BLS verify - blst-native 2.1956 ms/op 2.8045 ms/op 0.78
BLS verifyMultipleSignatures 3 - blst-native 4.3687 ms/op 5.5784 ms/op 0.78
BLS verifyMultipleSignatures 8 - blst-native 9.4293 ms/op 12.119 ms/op 0.78
BLS verifyMultipleSignatures 32 - blst-native 33.637 ms/op 44.010 ms/op 0.76
BLS aggregatePubkeys 32 - blst-native 44.607 us/op 59.074 us/op 0.76
BLS aggregatePubkeys 128 - blst-native 171.00 us/op 231.91 us/op 0.74
getAttestationsForBlock 97.217 ms/op 97.898 ms/op 0.99
CheckpointStateCache - add get delete 21.815 us/op 26.412 us/op 0.83
validate gossip signedAggregateAndProof - struct 5.6411 ms/op 6.7436 ms/op 0.84
validate gossip signedAggregateAndProof - treeBacked 5.3050 ms/op 6.4961 ms/op 0.82
validate gossip attestation - struct 2.4643 ms/op 3.0435 ms/op 0.81
validate gossip attestation - treeBacked 2.4356 ms/op 3.0929 ms/op 0.79
bytes32 toHexString 1.8370 us/op 2.4630 us/op 0.75
bytes32 Buffer.toString(hex) 782.00 ns/op 851.00 ns/op 0.92
bytes32 Buffer.toString(hex) from Uint8Array 1.0400 us/op 1.1400 us/op 0.91
bytes32 Buffer.toString(hex) + 0x 813.00 ns/op 890.00 ns/op 0.91
Object access 1 prop 0.38400 ns/op 0.46200 ns/op 0.83
Map access 1 prop 0.32300 ns/op 0.35200 ns/op 0.92
Object get x1000 20.721 ns/op 17.734 ns/op 1.17
Map get x1000 1.1070 ns/op 1.0230 ns/op 1.08
Object set x1000 119.05 ns/op 123.66 ns/op 0.96
Map set x1000 73.269 ns/op 85.930 ns/op 0.85
Return object 10000 times 0.42490 ns/op 0.42630 ns/op 1.00
Throw Error 10000 times 6.8063 us/op 7.2879 us/op 0.93
enrSubnets - fastDeserialize 64 bits 1.4220 us/op 1.7640 us/op 0.81
enrSubnets - ssz BitVector 64 bits 19.575 us/op 19.662 us/op 1.00
enrSubnets - fastDeserialize 4 bits 513.00 ns/op 599.00 ns/op 0.86
enrSubnets - ssz BitVector 4 bits 3.3870 us/op 3.5430 us/op 0.96
RateTracker 1000000 limit, 1 obj count per request 203.98 ns/op 219.57 ns/op 0.93
RateTracker 1000000 limit, 2 obj count per request 153.72 ns/op 166.68 ns/op 0.92
RateTracker 1000000 limit, 4 obj count per request 125.34 ns/op 138.99 ns/op 0.90
RateTracker 1000000 limit, 8 obj count per request 113.83 ns/op 126.12 ns/op 0.90
RateTracker with prune 4.3120 us/op 6.1090 us/op 0.71

by benchmarkbot/action

dapplion
dapplion approved these changes Jan 17, 2022
View reviewed changes
Copy link
Contributor

@dapplion dapplion left a comment

Choose a reason for hiding this comment

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

So we had implemented the queue but not removed the error that prevents its use?

@twoeths
Copy link
Contributor Author

twoeths commented Jan 18, 2022

So we had implemented the queue but not removed the error that prevents its use?

yes

@twoeths twoeths merged commit e2c79e4 into master Jan 18, 2022
@twoeths twoeths deleted the tuyen/forkchoice-future-attestations branch January 18, 2022 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dapplion dapplion dapplion approved these changes

@g11tech g11tech g11tech approved these changes

@mpetrunic mpetrunic Awaiting requested review from mpetrunic

@wemeetagain wemeetagain Awaiting requested review from wemeetagain

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error passing gossip attestations to forkchoice
3 participants
@twoeths @dapplion @g11tech