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

Forkchoice fix bug and cleanup #5126

Merged
merged 13 commits into from
Mar 19, 2023
Merged

Forkchoice fix bug and cleanup #5126

merged 13 commits into from
Mar 19, 2023

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Feb 10, 2023 •
edited
Loading

Motivation

  • Clean up unnecessary functions/variables and flag
  • Refactor forkchoice nodeIsViableForHead()

Description

Implements ethereum/consensus-specs#3290

  • Remove bestJustifiedCheckpoint functions and variables
  • Remove SAFE_SLOTS_TO_UPDATE_JUSTIFIED
  • Remove computeUnrealizedFull flag
  • Refactor forkchoice nodeIsViableForHead()

@twoeths twoeths changed the title Tuyen/forkchoice fixbug and cleanup Forkchoice fixbug and cleanup Feb 10, 2023
@twoeths twoeths changed the title Forkchoice fixbug and cleanup Forkchoice fix bug and cleanup Feb 10, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2023 •
edited
Loading

Performance Report

✔️ no performance regression detected

Full benchmark results
Benchmark suite Current: fd59c07 Previous: 9f7b830 Ratio
getPubkeys - index2pubkey - req 1000 vs - 250000 vc 755.75 us/op 861.38 us/op 0.88
getPubkeys - validatorsArr - req 1000 vs - 250000 vc 46.909 us/op 44.746 us/op 1.05
BLS verify - blst-native 1.2086 ms/op 1.1912 ms/op 1.01
BLS verifyMultipleSignatures 3 - blst-native 2.4550 ms/op 2.4247 ms/op 1.01
BLS verifyMultipleSignatures 8 - blst-native 5.2782 ms/op 5.2066 ms/op 1.01
BLS verifyMultipleSignatures 32 - blst-native 19.048 ms/op 18.814 ms/op 1.01
BLS aggregatePubkeys 32 - blst-native 25.579 us/op 25.446 us/op 1.01
BLS aggregatePubkeys 128 - blst-native 99.735 us/op 98.350 us/op 1.01
getAttestationsForBlock 53.628 ms/op 54.389 ms/op 0.99
isKnown best case - 1 super set check 255.00 ns/op 263.00 ns/op 0.97
isKnown normal case - 2 super set checks 246.00 ns/op 258.00 ns/op 0.95
isKnown worse case - 16 super set checks 246.00 ns/op 259.00 ns/op 0.95
CheckpointStateCache - add get delete 4.8900 us/op 4.8630 us/op 1.01
validate gossip signedAggregateAndProof - struct 2.7085 ms/op 2.7466 ms/op 0.99
validate gossip attestation - struct 1.2845 ms/op 1.3128 ms/op 0.98
pickEth1Vote - no votes 1.1966 ms/op 1.2335 ms/op 0.97
pickEth1Vote - max votes 9.4318 ms/op 10.995 ms/op 0.86
pickEth1Vote - Eth1Data hashTreeRoot value x2048 8.5637 ms/op 8.9736 ms/op 0.95
pickEth1Vote - Eth1Data hashTreeRoot tree x2048 14.698 ms/op 14.287 ms/op 1.03
pickEth1Vote - Eth1Data fastSerialize value x2048 631.52 us/op 620.72 us/op 1.02
pickEth1Vote - Eth1Data fastSerialize tree x2048 7.7163 ms/op 8.0655 ms/op 0.96
bytes32 toHexString 480.00 ns/op 462.00 ns/op 1.04
bytes32 Buffer.toString(hex) 331.00 ns/op 335.00 ns/op 0.99
bytes32 Buffer.toString(hex) from Uint8Array 521.00 ns/op 531.00 ns/op 0.98
bytes32 Buffer.toString(hex) + 0x 331.00 ns/op 336.00 ns/op 0.99
Object access 1 prop 0.15300 ns/op 0.16000 ns/op 0.96
Map access 1 prop 0.15700 ns/op 0.16200 ns/op 0.97
Object get x1000 6.4420 ns/op 6.9900 ns/op 0.92
Map get x1000 0.59800 ns/op 0.59000 ns/op 1.01
Object set x1000 49.258 ns/op 49.299 ns/op 1.00
Map set x1000 41.279 ns/op 41.435 ns/op 1.00
Return object 10000 times 0.22590 ns/op 0.22890 ns/op 0.99
Throw Error 10000 times 3.9697 us/op 4.1251 us/op 0.96
fastMsgIdFn sha256 / 200 bytes 3.2330 us/op 3.2830 us/op 0.98
fastMsgIdFn h32 xxhash / 200 bytes 264.00 ns/op 273.00 ns/op 0.97
fastMsgIdFn h64 xxhash / 200 bytes 375.00 ns/op 383.00 ns/op 0.98
fastMsgIdFn sha256 / 1000 bytes 10.903 us/op 11.155 us/op 0.98
fastMsgIdFn h32 xxhash / 1000 bytes 390.00 ns/op 398.00 ns/op 0.98
fastMsgIdFn h64 xxhash / 1000 bytes 470.00 ns/op 452.00 ns/op 1.04
fastMsgIdFn sha256 / 10000 bytes 99.953 us/op 100.15 us/op 1.00
fastMsgIdFn h32 xxhash / 10000 bytes 1.8220 us/op 1.8490 us/op 0.99
fastMsgIdFn h64 xxhash / 10000 bytes 1.3370 us/op 1.3210 us/op 1.01
enrSubnets - fastDeserialize 64 bits 1.2430 us/op 1.2220 us/op 1.02
enrSubnets - ssz BitVector 64 bits 472.00 ns/op 471.00 ns/op 1.00
enrSubnets - fastDeserialize 4 bits 168.00 ns/op 167.00 ns/op 1.01
enrSubnets - ssz BitVector 4 bits 476.00 ns/op 481.00 ns/op 0.99
prioritizePeers score -10:0 att 32-0.1 sync 2-0 101.53 us/op 103.74 us/op 0.98
prioritizePeers score 0:0 att 32-0.25 sync 2-0.25 126.48 us/op 129.10 us/op 0.98
prioritizePeers score 0:0 att 32-0.5 sync 2-0.5 159.00 us/op 164.72 us/op 0.97
prioritizePeers score 0:0 att 64-0.75 sync 4-0.75 304.36 us/op 300.45 us/op 1.01
prioritizePeers score 0:0 att 64-1 sync 4-1 377.57 us/op 359.30 us/op 1.05
array of 16000 items push then shift 1.6504 us/op 1.6231 us/op 1.02
LinkedList of 16000 items push then shift 8.8740 ns/op 9.1910 ns/op 0.97
array of 16000 items push then pop 82.139 ns/op 79.586 ns/op 1.03
LinkedList of 16000 items push then pop 8.4830 ns/op 8.7370 ns/op 0.97
array of 24000 items push then shift 2.3953 us/op 2.3419 us/op 1.02
LinkedList of 24000 items push then shift 8.6640 ns/op 10.232 ns/op 0.85
array of 24000 items push then pop 75.117 ns/op 75.817 ns/op 0.99
LinkedList of 24000 items push then pop 8.4610 ns/op 10.523 ns/op 0.80
intersect bitArray bitLen 8 13.239 ns/op 13.186 ns/op 1.00
intersect array and set length 8 77.526 ns/op 77.046 ns/op 1.01
intersect bitArray bitLen 128 43.890 ns/op 43.891 ns/op 1.00
intersect array and set length 128 1.0533 us/op 1.0445 us/op 1.01
Buffer.concat 32 items 2.6480 us/op 2.6310 us/op 1.01
Uint8Array.set 32 items 2.7550 us/op 2.4670 us/op 1.12
pass gossip attestations to forkchoice per slot 3.0043 ms/op 4.0135 ms/op 0.75
computeDeltas 2.8655 ms/op 3.3805 ms/op 0.85
computeProposerBoostScoreFromBalances 1.7462 ms/op 1.7649 ms/op 0.99
altair processAttestation - 250000 vs - 7PWei normalcase 2.2127 ms/op 2.1264 ms/op 1.04
altair processAttestation - 250000 vs - 7PWei worstcase 3.7001 ms/op 3.5853 ms/op 1.03
altair processAttestation - setStatus - 1/6 committees join 137.22 us/op 137.77 us/op 1.00
altair processAttestation - setStatus - 1/3 committees join 281.36 us/op 264.04 us/op 1.07
altair processAttestation - setStatus - 1/2 committees join 372.76 us/op 365.08 us/op 1.02
altair processAttestation - setStatus - 2/3 committees join 442.62 us/op 446.97 us/op 0.99
altair processAttestation - setStatus - 4/5 committees join 644.51 us/op 649.28 us/op 0.99
altair processAttestation - setStatus - 100% committees join 755.35 us/op 750.65 us/op 1.01
altair processBlock - 250000 vs - 7PWei normalcase 17.192 ms/op 17.240 ms/op 1.00
altair processBlock - 250000 vs - 7PWei normalcase hashState 25.861 ms/op 29.648 ms/op 0.87
altair processBlock - 250000 vs - 7PWei worstcase 48.357 ms/op 53.680 ms/op 0.90
altair processBlock - 250000 vs - 7PWei worstcase hashState 67.551 ms/op 67.358 ms/op 1.00
phase0 processBlock - 250000 vs - 7PWei normalcase 2.3632 ms/op 2.1095 ms/op 1.12
phase0 processBlock - 250000 vs - 7PWei worstcase 29.101 ms/op 27.778 ms/op 1.05
altair processEth1Data - 250000 vs - 7PWei normalcase 495.82 us/op 454.52 us/op 1.09
vc - 250000 eb 1 eth1 1 we 0 wn 0 - smpl 15 7.8200 us/op 6.8130 us/op 1.15
vc - 250000 eb 0.95 eth1 0.1 we 0.05 wn 0 - smpl 219 24.613 us/op 19.424 us/op 1.27
vc - 250000 eb 0.95 eth1 0.3 we 0.05 wn 0 - smpl 42 9.8470 us/op 8.4850 us/op 1.16
vc - 250000 eb 0.95 eth1 0.7 we 0.05 wn 0 - smpl 18 6.9430 us/op 6.5340 us/op 1.06
vc - 250000 eb 0.1 eth1 0.1 we 0 wn 0 - smpl 1020 104.34 us/op 74.115 us/op 1.41
vc - 250000 eb 0.03 eth1 0.03 we 0 wn 0 - smpl 11777 655.98 us/op 624.86 us/op 1.05
vc - 250000 eb 0.01 eth1 0.01 we 0 wn 0 - smpl 16384 914.64 us/op 903.30 us/op 1.01
vc - 250000 eb 0 eth1 0 we 0 wn 0 - smpl 16384 896.01 us/op 872.07 us/op 1.03
vc - 250000 eb 0 eth1 0 we 0 wn 0 nocache - smpl 16384 2.5756 ms/op 2.1554 ms/op 1.19
vc - 250000 eb 0 eth1 1 we 0 wn 0 - smpl 16384 1.5003 ms/op 1.5988 ms/op 0.94
vc - 250000 eb 0 eth1 1 we 0 wn 0 nocache - smpl 16384 3.9852 ms/op 3.8614 ms/op 1.03
Tree 40 250000 create 323.77 ms/op 283.87 ms/op 1.14
Tree 40 250000 get(125000) 193.74 ns/op 175.74 ns/op 1.10
Tree 40 250000 set(125000) 983.07 ns/op 850.71 ns/op 1.16
Tree 40 250000 toArray() 22.247 ms/op 16.536 ms/op 1.35
Tree 40 250000 iterate all - toArray() + loop 22.710 ms/op 16.781 ms/op 1.35
Tree 40 250000 iterate all - get(i) 75.774 ms/op 65.674 ms/op 1.15
MutableVector 250000 create 10.957 ms/op 9.1957 ms/op 1.19
MutableVector 250000 get(125000) 6.5380 ns/op 6.2270 ns/op 1.05
MutableVector 250000 set(125000) 288.06 ns/op 253.67 ns/op 1.14
MutableVector 250000 toArray() 3.8405 ms/op 2.7164 ms/op 1.41
MutableVector 250000 iterate all - toArray() + loop 3.7343 ms/op 2.8070 ms/op 1.33
MutableVector 250000 iterate all - get(i) 1.5342 ms/op 1.4718 ms/op 1.04
Array 250000 create 3.3257 ms/op 2.7196 ms/op 1.22
Array 250000 clone - spread 1.0956 ms/op 1.0649 ms/op 1.03
Array 250000 get(125000) 0.52900 ns/op 0.50900 ns/op 1.04
Array 250000 set(125000) 0.62000 ns/op 0.59800 ns/op 1.04
Array 250000 iterate all - loop 84.530 us/op 87.784 us/op 0.96
effectiveBalanceIncrements clone Uint8Array 300000 28.436 us/op 22.410 us/op 1.27
effectiveBalanceIncrements clone MutableVector 300000 320.00 ns/op 317.00 ns/op 1.01
effectiveBalanceIncrements rw all Uint8Array 300000 170.69 us/op 164.79 us/op 1.04
effectiveBalanceIncrements rw all MutableVector 300000 78.961 ms/op 75.594 ms/op 1.04
phase0 afterProcessEpoch - 250000 vs - 7PWei 116.34 ms/op 110.84 ms/op 1.05
phase0 beforeProcessEpoch - 250000 vs - 7PWei 37.948 ms/op 41.860 ms/op 0.91
altair processEpoch - mainnet_e81889 337.34 ms/op 294.81 ms/op 1.14
mainnet_e81889 - altair beforeProcessEpoch 67.512 ms/op 46.442 ms/op 1.45
mainnet_e81889 - altair processJustificationAndFinalization 18.206 us/op 15.597 us/op 1.17
mainnet_e81889 - altair processInactivityUpdates 5.8800 ms/op 4.9764 ms/op 1.18
mainnet_e81889 - altair processRewardsAndPenalties 67.029 ms/op 66.428 ms/op 1.01
mainnet_e81889 - altair processRegistryUpdates 2.2490 us/op 2.8710 us/op 0.78
mainnet_e81889 - altair processSlashings 484.00 ns/op 528.00 ns/op 0.92
mainnet_e81889 - altair processEth1DataReset 534.00 ns/op 515.00 ns/op 1.04
mainnet_e81889 - altair processEffectiveBalanceUpdates 1.3585 ms/op 1.2204 ms/op 1.11
mainnet_e81889 - altair processSlashingsReset 4.3540 us/op 3.9320 us/op 1.11
mainnet_e81889 - altair processRandaoMixesReset 4.5080 us/op 5.8940 us/op 0.76
mainnet_e81889 - altair processHistoricalRootsUpdate 633.00 ns/op 722.00 ns/op 0.88
mainnet_e81889 - altair processParticipationFlagUpdates 2.0790 us/op 2.5600 us/op 0.81
mainnet_e81889 - altair processSyncCommitteeUpdates 465.00 ns/op 601.00 ns/op 0.77
mainnet_e81889 - altair afterProcessEpoch 121.90 ms/op 122.88 ms/op 0.99
phase0 processEpoch - mainnet_e58758 327.74 ms/op 357.97 ms/op 0.92
mainnet_e58758 - phase0 beforeProcessEpoch 122.42 ms/op 133.51 ms/op 0.92
mainnet_e58758 - phase0 processJustificationAndFinalization 18.648 us/op 14.771 us/op 1.26
mainnet_e58758 - phase0 processRewardsAndPenalties 55.666 ms/op 63.364 ms/op 0.88
mainnet_e58758 - phase0 processRegistryUpdates 10.955 us/op 7.7560 us/op 1.41
mainnet_e58758 - phase0 processSlashings 638.00 ns/op 485.00 ns/op 1.32
mainnet_e58758 - phase0 processEth1DataReset 653.00 ns/op 483.00 ns/op 1.35
mainnet_e58758 - phase0 processEffectiveBalanceUpdates 1.0077 ms/op 941.84 us/op 1.07
mainnet_e58758 - phase0 processSlashingsReset 3.2170 us/op 4.3410 us/op 0.74
mainnet_e58758 - phase0 processRandaoMixesReset 7.5620 us/op 4.6570 us/op 1.62
mainnet_e58758 - phase0 processHistoricalRootsUpdate 855.00 ns/op 543.00 ns/op 1.57
mainnet_e58758 - phase0 processParticipationRecordUpdates 4.6970 us/op 3.8620 us/op 1.22
mainnet_e58758 - phase0 afterProcessEpoch 106.39 ms/op 93.705 ms/op 1.14
phase0 processEffectiveBalanceUpdates - 250000 normalcase 1.2819 ms/op 1.2049 ms/op 1.06
phase0 processEffectiveBalanceUpdates - 250000 worstcase 0.5 1.4813 ms/op 1.4505 ms/op 1.02
altair processInactivityUpdates - 250000 normalcase 23.225 ms/op 18.583 ms/op 1.25
altair processInactivityUpdates - 250000 worstcase 22.717 ms/op 24.905 ms/op 0.91
phase0 processRegistryUpdates - 250000 normalcase 7.1320 us/op 6.8140 us/op 1.05
phase0 processRegistryUpdates - 250000 badcase_full_deposits 267.34 us/op 231.33 us/op 1.16
phase0 processRegistryUpdates - 250000 worstcase 0.5 115.71 ms/op 111.03 ms/op 1.04
altair processRewardsAndPenalties - 250000 normalcase 57.937 ms/op 66.597 ms/op 0.87
altair processRewardsAndPenalties - 250000 worstcase 53.408 ms/op 71.246 ms/op 0.75
phase0 getAttestationDeltas - 250000 normalcase 6.7246 ms/op 6.4617 ms/op 1.04
phase0 getAttestationDeltas - 250000 worstcase 6.7206 ms/op 6.4165 ms/op 1.05
phase0 processSlashings - 250000 worstcase 3.4680 ms/op 3.3578 ms/op 1.03
altair processSyncCommitteeUpdates - 250000 179.89 ms/op 170.08 ms/op 1.06
BeaconState.hashTreeRoot - No change 271.00 ns/op 262.00 ns/op 1.03
BeaconState.hashTreeRoot - 1 full validator 51.094 us/op 50.460 us/op 1.01
BeaconState.hashTreeRoot - 32 full validator 506.90 us/op 489.19 us/op 1.04
BeaconState.hashTreeRoot - 512 full validator 5.0205 ms/op 4.9894 ms/op 1.01
BeaconState.hashTreeRoot - 1 validator.effectiveBalance 62.570 us/op 63.620 us/op 0.98
BeaconState.hashTreeRoot - 32 validator.effectiveBalance 921.61 us/op 859.64 us/op 1.07
BeaconState.hashTreeRoot - 512 validator.effectiveBalance 11.468 ms/op 10.760 ms/op 1.07
BeaconState.hashTreeRoot - 1 balances 45.275 us/op 49.100 us/op 0.92
BeaconState.hashTreeRoot - 32 balances 415.44 us/op 427.64 us/op 0.97
BeaconState.hashTreeRoot - 512 balances 4.2359 ms/op 4.3150 ms/op 0.98
BeaconState.hashTreeRoot - 250000 balances 71.189 ms/op 77.900 ms/op 0.91
aggregationBits - 2048 els - zipIndexesInBitList 15.907 us/op 14.630 us/op 1.09
regular array get 100000 times 39.388 us/op 31.552 us/op 1.25
wrappedArray get 100000 times 31.976 us/op 31.369 us/op 1.02
arrayWithProxy get 100000 times 15.508 ms/op 15.277 ms/op 1.02
ssz.Root.equals 532.00 ns/op 530.00 ns/op 1.00
byteArrayEquals 521.00 ns/op 521.00 ns/op 1.00
shuffle list - 16384 els 6.7928 ms/op 6.7503 ms/op 1.01
shuffle list - 250000 els 99.995 ms/op 99.184 ms/op 1.01
processSlot - 1 slots 8.8050 us/op 9.0510 us/op 0.97
processSlot - 32 slots 1.3110 ms/op 1.2885 ms/op 1.02
getEffectiveBalanceIncrementsZeroInactive - 250000 vs - 7PWei 34.019 ms/op 187.01 us/op 181.91
getCommitteeAssignments - req 1 vs - 250000 vc 2.9238 ms/op 2.8930 ms/op 1.01
getCommitteeAssignments - req 100 vs - 250000 vc 4.1271 ms/op 3.9903 ms/op 1.03
getCommitteeAssignments - req 1000 vs - 250000 vc 4.4548 ms/op 4.3115 ms/op 1.03
RootCache.getBlockRootAtSlot - 250000 vs - 7PWei 5.0400 ns/op 4.5300 ns/op 1.11
state getBlockRootAtSlot - 250000 vs - 7PWei 954.43 ns/op 784.41 ns/op 1.22
computeProposers - vc 250000 10.802 ms/op 10.391 ms/op 1.04
computeEpochShuffling - vc 250000 97.372 ms/op 100.39 ms/op 0.97
getNextSyncCommittee - vc 250000 172.40 ms/op 176.03 ms/op 0.98

by benchmarkbot/action

dapplion
dapplion reviewed Feb 13, 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.

Overall looks good

packages/fork-choice/src/protoArray/protoArray.ts Show resolved Hide resolved
@ChainSafe ChainSafe deleted a comment from wemeetagain Mar 3, 2023
This was referenced Mar 17, 2023
Closed
Merged
@twoeths twoeths force-pushed the tuyen/forkchoice_fixbug_and_cleanup branch from 6afd85f to 2881b02 Compare March 17, 2023 23:27
@twoeths twoeths force-pushed the tuyen/forkchoice_fixbug_and_cleanup branch from afb2122 to 5050722 Compare March 18, 2023 23:33
@twoeths twoeths marked this pull request as ready for review March 18, 2023 23:53
@twoeths twoeths requested a review from a team as a code owner March 18, 2023 23:53
@dapplion dapplion merged commit 8bc1f69 into unstable Mar 19, 2023
@dapplion dapplion deleted the tuyen/forkchoice_fixbug_and_cleanup branch March 19, 2023 08:35
@philknows philknows modified the milestones: v1.6.0, v1.7.0 Mar 19, 2023
@twoeths twoeths mentioned this pull request Mar 24, 2023
Merged
@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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.7.0
Development

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