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

Always add gossip attestations to forkchoice #5259

Merged
merged 2 commits into from
Mar 24, 2023
Merged

Always add gossip attestations to forkchoice #5259

merged 2 commits into from
Mar 24, 2023

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Mar 11, 2023

Motivation

  • Gossip handler: Right now we do early return if beacon node does not have any validators as aggregator on specified subnets, i.e we didn't add gossip attestations to forkchoice in that case

Description

  • We should always add attestations to forkchoice
  • The attnetsService.shouldProcess should only decide if we add attestation to our pool to prepare for aggregator's duty later

@twoeths twoeths requested a review from a team as a code owner March 11, 2023 04:24
@github-actions
Copy link
Contributor

Performance Report

✔️ no performance regression detected

Full benchmark results
Benchmark suite Current: c489be4 Previous: 27feb58 Ratio
getPubkeys - index2pubkey - req 1000 vs - 250000 vc 825.10 us/op 667.16 us/op 1.24
getPubkeys - validatorsArr - req 1000 vs - 250000 vc 44.317 us/op 46.863 us/op 0.95
BLS verify - blst-native 1.2030 ms/op 1.2342 ms/op 0.97
BLS verifyMultipleSignatures 3 - blst-native 2.4251 ms/op 2.4719 ms/op 0.98
BLS verifyMultipleSignatures 8 - blst-native 5.0781 ms/op 5.3569 ms/op 0.95
BLS verifyMultipleSignatures 32 - blst-native 18.323 ms/op 19.124 ms/op 0.96
BLS aggregatePubkeys 32 - blst-native 24.496 us/op 25.484 us/op 0.96
BLS aggregatePubkeys 128 - blst-native 96.472 us/op 99.074 us/op 0.97
getAttestationsForBlock 53.802 ms/op 50.686 ms/op 1.06
isKnown best case - 1 super set check 256.00 ns/op 257.00 ns/op 1.00
isKnown normal case - 2 super set checks 248.00 ns/op 250.00 ns/op 0.99
isKnown worse case - 16 super set checks 252.00 ns/op 250.00 ns/op 1.01
CheckpointStateCache - add get delete 4.8800 us/op 5.0420 us/op 0.97
validate gossip signedAggregateAndProof - struct 2.7218 ms/op 2.7913 ms/op 0.98
validate gossip attestation - struct 1.3057 ms/op 1.3469 ms/op 0.97
pickEth1Vote - no votes 1.2485 ms/op 1.3477 ms/op 0.93
pickEth1Vote - max votes 10.976 ms/op 11.183 ms/op 0.98
pickEth1Vote - Eth1Data hashTreeRoot value x2048 8.9556 ms/op 9.9599 ms/op 0.90
pickEth1Vote - Eth1Data hashTreeRoot tree x2048 13.657 ms/op 16.777 ms/op 0.81
pickEth1Vote - Eth1Data fastSerialize value x2048 665.23 us/op 708.51 us/op 0.94
pickEth1Vote - Eth1Data fastSerialize tree x2048 6.7741 ms/op 5.1412 ms/op 1.32
bytes32 toHexString 500.00 ns/op 555.00 ns/op 0.90
bytes32 Buffer.toString(hex) 371.00 ns/op 424.00 ns/op 0.88
bytes32 Buffer.toString(hex) from Uint8Array 579.00 ns/op 595.00 ns/op 0.97
bytes32 Buffer.toString(hex) + 0x 383.00 ns/op 452.00 ns/op 0.85
Object access 1 prop 0.17100 ns/op 0.18000 ns/op 0.95
Map access 1 prop 0.15600 ns/op 0.17500 ns/op 0.89
Object get x1000 6.7800 ns/op 6.7100 ns/op 1.01
Map get x1000 0.60200 ns/op 0.64500 ns/op 0.93
Object set x1000 51.186 ns/op 55.293 ns/op 0.93
Map set x1000 42.659 ns/op 45.167 ns/op 0.94
Return object 10000 times 0.23310 ns/op 0.24790 ns/op 0.94
Throw Error 10000 times 4.0932 us/op 4.2911 us/op 0.95
fastMsgIdFn sha256 / 200 bytes 3.4020 us/op 3.5400 us/op 0.96
fastMsgIdFn h32 xxhash / 200 bytes 279.00 ns/op 293.00 ns/op 0.95
fastMsgIdFn h64 xxhash / 200 bytes 386.00 ns/op 408.00 ns/op 0.95
fastMsgIdFn sha256 / 1000 bytes 11.608 us/op 11.711 us/op 0.99
fastMsgIdFn h32 xxhash / 1000 bytes 423.00 ns/op 426.00 ns/op 0.99
fastMsgIdFn h64 xxhash / 1000 bytes 474.00 ns/op 468.00 ns/op 1.01
fastMsgIdFn sha256 / 10000 bytes 98.041 us/op 103.22 us/op 0.95
fastMsgIdFn h32 xxhash / 10000 bytes 1.8670 us/op 1.9540 us/op 0.96
fastMsgIdFn h64 xxhash / 10000 bytes 1.3060 us/op 1.4020 us/op 0.93
enrSubnets - fastDeserialize 64 bits 1.2690 us/op 1.3500 us/op 0.94
enrSubnets - ssz BitVector 64 bits 476.00 ns/op 513.00 ns/op 0.93
enrSubnets - fastDeserialize 4 bits 178.00 ns/op 191.00 ns/op 0.93
enrSubnets - ssz BitVector 4 bits 482.00 ns/op 570.00 ns/op 0.85
prioritizePeers score -10:0 att 32-0.1 sync 2-0 103.29 us/op 120.82 us/op 0.85
prioritizePeers score 0:0 att 32-0.25 sync 2-0.25 130.30 us/op 143.30 us/op 0.91
prioritizePeers score 0:0 att 32-0.5 sync 2-0.5 165.40 us/op 190.51 us/op 0.87
prioritizePeers score 0:0 att 64-0.75 sync 4-0.75 296.32 us/op 338.58 us/op 0.88
prioritizePeers score 0:0 att 64-1 sync 4-1 370.99 us/op 410.35 us/op 0.90
array of 16000 items push then shift 1.6134 us/op 1.7340 us/op 0.93
LinkedList of 16000 items push then shift 8.5030 ns/op 9.2490 ns/op 0.92
array of 16000 items push then pop 92.510 ns/op 116.67 ns/op 0.79
LinkedList of 16000 items push then pop 8.2290 ns/op 9.3770 ns/op 0.88
array of 24000 items push then shift 2.3312 us/op 2.4299 us/op 0.96
LinkedList of 24000 items push then shift 8.4920 ns/op 9.2290 ns/op 0.92
array of 24000 items push then pop 75.093 ns/op 86.400 ns/op 0.87
LinkedList of 24000 items push then pop 8.1800 ns/op 8.8820 ns/op 0.92
intersect bitArray bitLen 8 12.834 ns/op 13.611 ns/op 0.94
intersect array and set length 8 79.248 ns/op 82.574 ns/op 0.96
intersect bitArray bitLen 128 43.554 ns/op 44.549 ns/op 0.98
intersect array and set length 128 1.0331 us/op 1.1011 us/op 0.94
Buffer.concat 32 items 2.8710 us/op 2.7110 us/op 1.06
Uint8Array.set 32 items 2.5580 us/op 2.9890 us/op 0.86
pass gossip attestations to forkchoice per slot 3.1182 ms/op 2.3099 ms/op 1.35
computeDeltas 3.2365 ms/op 3.1386 ms/op 1.03
computeProposerBoostScoreFromBalances 1.7663 ms/op 1.8597 ms/op 0.95
altair processAttestation - 250000 vs - 7PWei normalcase 2.3123 ms/op 3.2352 ms/op 0.71
altair processAttestation - 250000 vs - 7PWei worstcase 3.2122 ms/op 5.0015 ms/op 0.64
altair processAttestation - setStatus - 1/6 committees join 137.97 us/op 155.06 us/op 0.89
altair processAttestation - setStatus - 1/3 committees join 273.91 us/op 288.80 us/op 0.95
altair processAttestation - setStatus - 1/2 committees join 370.80 us/op 387.00 us/op 0.96
altair processAttestation - setStatus - 2/3 committees join 465.94 us/op 498.23 us/op 0.94
altair processAttestation - setStatus - 4/5 committees join 649.00 us/op 692.40 us/op 0.94
altair processAttestation - setStatus - 100% committees join 777.84 us/op 814.37 us/op 0.96
altair processBlock - 250000 vs - 7PWei normalcase 16.329 ms/op 18.099 ms/op 0.90
altair processBlock - 250000 vs - 7PWei normalcase hashState 24.887 ms/op 31.701 ms/op 0.79
altair processBlock - 250000 vs - 7PWei worstcase 50.973 ms/op 55.225 ms/op 0.92
altair processBlock - 250000 vs - 7PWei worstcase hashState 70.287 ms/op 73.593 ms/op 0.96
phase0 processBlock - 250000 vs - 7PWei normalcase 2.1055 ms/op 2.6761 ms/op 0.79
phase0 processBlock - 250000 vs - 7PWei worstcase 30.096 ms/op 34.308 ms/op 0.88
altair processEth1Data - 250000 vs - 7PWei normalcase 515.76 us/op 567.59 us/op 0.91
vc - 250000 eb 1 eth1 1 we 0 wn 0 - smpl 15 7.9850 us/op 9.1610 us/op 0.87
vc - 250000 eb 0.95 eth1 0.1 we 0.05 wn 0 - smpl 219 24.534 us/op 30.056 us/op 0.82
vc - 250000 eb 0.95 eth1 0.3 we 0.05 wn 0 - smpl 42 11.108 us/op 14.135 us/op 0.79
vc - 250000 eb 0.95 eth1 0.7 we 0.05 wn 0 - smpl 18 8.9960 us/op 12.000 us/op 0.75
vc - 250000 eb 0.1 eth1 0.1 we 0 wn 0 - smpl 1020 111.75 us/op 119.15 us/op 0.94
vc - 250000 eb 0.03 eth1 0.03 we 0 wn 0 - smpl 11777 651.68 us/op 706.87 us/op 0.92
vc - 250000 eb 0.01 eth1 0.01 we 0 wn 0 - smpl 16384 964.90 us/op 992.35 us/op 0.97
vc - 250000 eb 0 eth1 0 we 0 wn 0 - smpl 16384 908.47 us/op 905.89 us/op 1.00
vc - 250000 eb 0 eth1 0 we 0 wn 0 nocache - smpl 16384 2.5041 ms/op 2.4112 ms/op 1.04
vc - 250000 eb 0 eth1 1 we 0 wn 0 - smpl 16384 1.4962 ms/op 1.4886 ms/op 1.01
vc - 250000 eb 0 eth1 1 we 0 wn 0 nocache - smpl 16384 4.4287 ms/op 5.1360 ms/op 0.86
Tree 40 250000 create 400.85 ms/op 409.28 ms/op 0.98
Tree 40 250000 get(125000) 197.59 ns/op 206.29 ns/op 0.96
Tree 40 250000 set(125000) 1.0499 us/op 1.2186 us/op 0.86
Tree 40 250000 toArray() 23.757 ms/op 24.781 ms/op 0.96
Tree 40 250000 iterate all - toArray() + loop 25.162 ms/op 23.022 ms/op 1.09
Tree 40 250000 iterate all - get(i) 77.294 ms/op 77.149 ms/op 1.00
MutableVector 250000 create 11.663 ms/op 13.600 ms/op 0.86
MutableVector 250000 get(125000) 6.3180 ns/op 6.7450 ns/op 0.94
MutableVector 250000 set(125000) 360.75 ns/op 466.77 ns/op 0.77
MutableVector 250000 toArray() 4.3764 ms/op 4.9990 ms/op 0.88
MutableVector 250000 iterate all - toArray() + loop 3.7765 ms/op 4.3264 ms/op 0.87
MutableVector 250000 iterate all - get(i) 1.5106 ms/op 1.5763 ms/op 0.96
Array 250000 create 2.6601 ms/op 3.9952 ms/op 0.67
Array 250000 clone - spread 1.0960 ms/op 1.4000 ms/op 0.78
Array 250000 get(125000) 0.54700 ns/op 0.78700 ns/op 0.70
Array 250000 set(125000) 0.60400 ns/op 0.76400 ns/op 0.79
Array 250000 iterate all - loop 86.103 us/op 91.475 us/op 0.94
effectiveBalanceIncrements clone Uint8Array 300000 30.116 us/op 49.053 us/op 0.61
effectiveBalanceIncrements clone MutableVector 300000 335.00 ns/op 432.00 ns/op 0.78
effectiveBalanceIncrements rw all Uint8Array 300000 168.91 us/op 174.68 us/op 0.97
effectiveBalanceIncrements rw all MutableVector 300000 86.561 ms/op 96.804 ms/op 0.89
phase0 afterProcessEpoch - 250000 vs - 7PWei 118.10 ms/op 122.84 ms/op 0.96
phase0 beforeProcessEpoch - 250000 vs - 7PWei 45.712 ms/op 47.974 ms/op 0.95
altair processEpoch - mainnet_e81889 304.82 ms/op 394.56 ms/op 0.77
mainnet_e81889 - altair beforeProcessEpoch 51.612 ms/op 78.738 ms/op 0.66
mainnet_e81889 - altair processJustificationAndFinalization 20.142 us/op 23.849 us/op 0.84
mainnet_e81889 - altair processInactivityUpdates 6.1301 ms/op 8.5357 ms/op 0.72
mainnet_e81889 - altair processRewardsAndPenalties 66.851 ms/op 86.802 ms/op 0.77
mainnet_e81889 - altair processRegistryUpdates 2.6850 us/op 3.4940 us/op 0.77
mainnet_e81889 - altair processSlashings 561.00 ns/op 1.1190 us/op 0.50
mainnet_e81889 - altair processEth1DataReset 533.00 ns/op 988.00 ns/op 0.54
mainnet_e81889 - altair processEffectiveBalanceUpdates 1.2567 ms/op 2.0277 ms/op 0.62
mainnet_e81889 - altair processSlashingsReset 4.5400 us/op 7.2720 us/op 0.62
mainnet_e81889 - altair processRandaoMixesReset 4.4920 us/op 7.5800 us/op 0.59
mainnet_e81889 - altair processHistoricalRootsUpdate 1.1900 us/op 1.1290 us/op 1.05
mainnet_e81889 - altair processParticipationFlagUpdates 2.9370 us/op 6.0800 us/op 0.48
mainnet_e81889 - altair processSyncCommitteeUpdates 454.00 ns/op 1.0120 us/op 0.45
mainnet_e81889 - altair afterProcessEpoch 126.79 ms/op 138.64 ms/op 0.91
phase0 processEpoch - mainnet_e58758 357.13 ms/op 447.95 ms/op 0.80
mainnet_e58758 - phase0 beforeProcessEpoch 145.83 ms/op 200.36 ms/op 0.73
mainnet_e58758 - phase0 processJustificationAndFinalization 17.941 us/op 28.329 us/op 0.63
mainnet_e58758 - phase0 processRewardsAndPenalties 67.256 ms/op 77.967 ms/op 0.86
mainnet_e58758 - phase0 processRegistryUpdates 8.3580 us/op 18.453 us/op 0.45
mainnet_e58758 - phase0 processSlashings 489.00 ns/op 1.1640 us/op 0.42
mainnet_e58758 - phase0 processEth1DataReset 472.00 ns/op 1.0810 us/op 0.44
mainnet_e58758 - phase0 processEffectiveBalanceUpdates 1.2811 ms/op 1.8260 ms/op 0.70
mainnet_e58758 - phase0 processSlashingsReset 5.4240 us/op 7.9510 us/op 0.68
mainnet_e58758 - phase0 processRandaoMixesReset 5.0280 us/op 10.833 us/op 0.46
mainnet_e58758 - phase0 processHistoricalRootsUpdate 1.2700 us/op 1.2530 us/op 1.01
mainnet_e58758 - phase0 processParticipationRecordUpdates 3.9970 us/op 8.1320 us/op 0.49
mainnet_e58758 - phase0 afterProcessEpoch 99.711 ms/op 115.24 ms/op 0.87
phase0 processEffectiveBalanceUpdates - 250000 normalcase 1.2458 ms/op 2.1488 ms/op 0.58
phase0 processEffectiveBalanceUpdates - 250000 worstcase 0.5 1.5534 ms/op 2.6929 ms/op 0.58
altair processInactivityUpdates - 250000 normalcase 25.538 ms/op 28.616 ms/op 0.89
altair processInactivityUpdates - 250000 worstcase 19.213 ms/op 34.103 ms/op 0.56
phase0 processRegistryUpdates - 250000 normalcase 7.4910 us/op 14.054 us/op 0.53
phase0 processRegistryUpdates - 250000 badcase_full_deposits 265.26 us/op 323.89 us/op 0.82
phase0 processRegistryUpdates - 250000 worstcase 0.5 130.44 ms/op 148.43 ms/op 0.88
altair processRewardsAndPenalties - 250000 normalcase 67.613 ms/op 74.460 ms/op 0.91
altair processRewardsAndPenalties - 250000 worstcase 64.067 ms/op 77.564 ms/op 0.83
phase0 getAttestationDeltas - 250000 normalcase 6.8945 ms/op 7.2291 ms/op 0.95
phase0 getAttestationDeltas - 250000 worstcase 6.7260 ms/op 8.0830 ms/op 0.83
phase0 processSlashings - 250000 worstcase 3.4580 ms/op 4.0580 ms/op 0.85
altair processSyncCommitteeUpdates - 250000 174.76 ms/op 208.61 ms/op 0.84
BeaconState.hashTreeRoot - No change 281.00 ns/op 271.00 ns/op 1.04
BeaconState.hashTreeRoot - 1 full validator 51.623 us/op 55.037 us/op 0.94
BeaconState.hashTreeRoot - 32 full validator 521.81 us/op 531.61 us/op 0.98
BeaconState.hashTreeRoot - 512 full validator 5.5171 ms/op 6.9407 ms/op 0.79
BeaconState.hashTreeRoot - 1 validator.effectiveBalance 64.748 us/op 73.808 us/op 0.88
BeaconState.hashTreeRoot - 32 validator.effectiveBalance 881.26 us/op 1.0175 ms/op 0.87
BeaconState.hashTreeRoot - 512 validator.effectiveBalance 11.731 ms/op 12.876 ms/op 0.91
BeaconState.hashTreeRoot - 1 balances 49.914 us/op 52.376 us/op 0.95
BeaconState.hashTreeRoot - 32 balances 452.84 us/op 503.76 us/op 0.90
BeaconState.hashTreeRoot - 512 balances 4.2007 ms/op 4.8631 ms/op 0.86
BeaconState.hashTreeRoot - 250000 balances 75.658 ms/op 73.326 ms/op 1.03
aggregationBits - 2048 els - zipIndexesInBitList 16.357 us/op 27.603 us/op 0.59
regular array get 100000 times 34.106 us/op 35.883 us/op 0.95
wrappedArray get 100000 times 32.606 us/op 34.925 us/op 0.93
arrayWithProxy get 100000 times 15.229 ms/op 15.939 ms/op 0.96
ssz.Root.equals 559.00 ns/op 608.00 ns/op 0.92
byteArrayEquals 563.00 ns/op 595.00 ns/op 0.95
shuffle list - 16384 els 6.8076 ms/op 7.5334 ms/op 0.90
shuffle list - 250000 els 99.512 ms/op 110.30 ms/op 0.90
processSlot - 1 slots 9.6450 us/op 9.8230 us/op 0.98
processSlot - 32 slots 1.3945 ms/op 1.5569 ms/op 0.90
getEffectiveBalanceIncrementsZeroInactive - 250000 vs - 7PWei 214.81 us/op 211.46 us/op 1.02
getCommitteeAssignments - req 1 vs - 250000 vc 2.9537 ms/op 2.9753 ms/op 0.99
getCommitteeAssignments - req 100 vs - 250000 vc 4.2144 ms/op 4.2834 ms/op 0.98
getCommitteeAssignments - req 1000 vs - 250000 vc 4.5210 ms/op 4.5836 ms/op 0.99
RootCache.getBlockRootAtSlot - 250000 vs - 7PWei 5.1900 ns/op 5.3400 ns/op 0.97
state getBlockRootAtSlot - 250000 vs - 7PWei 671.22 ns/op 821.72 ns/op 0.82
computeProposers - vc 250000 11.133 ms/op 11.525 ms/op 0.97
computeEpochShuffling - vc 250000 103.90 ms/op 108.40 ms/op 0.96
getNextSyncCommittee - vc 250000 185.40 ms/op 186.54 ms/op 0.99

by benchmarkbot/action

dapplion
dapplion suggested changes Mar 11, 2023
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.

Are you sure this is required? What problem or correctness issue is it trying to fix?

@twoeths
Copy link
Contributor Author

twoeths commented Mar 12, 2023

@dapplion right now we only pass attestations to fork_choice if we have an aggregator on that subnet/slot, I don't see any requirements for that.

I checked from the spec: Run ``on_attestation`` upon receiving a new ``attestation`` from either within a block or directly on the wire. => we always have to do that as I understand

https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#on_attestation

also this was instructed from Cayman as I remember, could you double check @wemeetagain ?

@dapplion
Copy link
Contributor

Right, if we have already done the work of verifying the attestation, might as well include it in the fork-choice

@twoeths twoeths enabled auto-merge (squash) March 23, 2023 23:11
@twoeths twoeths merged commit fceb26a into unstable Mar 24, 2023
@twoeths twoeths deleted the tuyen/always_add_attestations_to_forkchoice branch March 24, 2023 05:08
@wemeetagain
Copy link
Member

🎉 This PR is included in v1.7.0 🎉

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

@wemeetagain wemeetagain wemeetagain approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@twoeths @dapplion @wemeetagain