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

Review eip-4844 sync code #5053

Merged
merged 11 commits into from
Feb 19, 2023
Merged

Review eip-4844 sync code #5053

merged 11 commits into from
Feb 19, 2023

Conversation

dapplion
Copy link
Contributor

Motivation

Spec clarified a couple points that influence our implementation:

Description

@dapplion dapplion requested a review from a team as a code owner January 25, 2023 10:59
@github-actions
Copy link
Contributor

github-actions bot commented Jan 25, 2023 •
edited
Loading

Performance Report

✔️ no performance regression detected

Full benchmark results
Benchmark suite Current: 146a3e3 Previous: 3857acc Ratio
getPubkeys - index2pubkey - req 1000 vs - 250000 vc 787.92 us/op 763.86 us/op 1.03
getPubkeys - validatorsArr - req 1000 vs - 250000 vc 44.846 us/op 43.145 us/op 1.04
BLS verify - blst-native 1.1955 ms/op 1.1680 ms/op 1.02
BLS verifyMultipleSignatures 3 - blst-native 2.4284 ms/op 2.3747 ms/op 1.02
BLS verifyMultipleSignatures 8 - blst-native 5.2147 ms/op 5.1051 ms/op 1.02
BLS verifyMultipleSignatures 32 - blst-native 18.901 ms/op 18.427 ms/op 1.03
BLS aggregatePubkeys 32 - blst-native 25.287 us/op 24.592 us/op 1.03
BLS aggregatePubkeys 128 - blst-native 100.20 us/op 96.088 us/op 1.04
getAttestationsForBlock 52.689 ms/op 52.551 ms/op 1.00
isKnown best case - 1 super set check 271.00 ns/op 260.00 ns/op 1.04
isKnown normal case - 2 super set checks 261.00 ns/op 263.00 ns/op 0.99
isKnown worse case - 16 super set checks 246.00 ns/op 259.00 ns/op 0.95
CheckpointStateCache - add get delete 5.1580 us/op 4.9660 us/op 1.04
validate gossip signedAggregateAndProof - struct 2.7467 ms/op 2.7048 ms/op 1.02
validate gossip attestation - struct 1.2893 ms/op 1.2912 ms/op 1.00
pickEth1Vote - no votes 1.1992 ms/op 1.2316 ms/op 0.97
pickEth1Vote - max votes 10.549 ms/op 9.8154 ms/op 1.07
pickEth1Vote - Eth1Data hashTreeRoot value x2048 8.1502 ms/op 8.9291 ms/op 0.91
pickEth1Vote - Eth1Data hashTreeRoot tree x2048 13.975 ms/op 14.588 ms/op 0.96
pickEth1Vote - Eth1Data fastSerialize value x2048 622.19 us/op 607.01 us/op 1.03
pickEth1Vote - Eth1Data fastSerialize tree x2048 6.1858 ms/op 7.4602 ms/op 0.83
bytes32 toHexString 477.00 ns/op 470.00 ns/op 1.01
bytes32 Buffer.toString(hex) 333.00 ns/op 326.00 ns/op 1.02
bytes32 Buffer.toString(hex) from Uint8Array 538.00 ns/op 534.00 ns/op 1.01
bytes32 Buffer.toString(hex) + 0x 337.00 ns/op 331.00 ns/op 1.02
Object access 1 prop 0.16200 ns/op 0.16100 ns/op 1.01
Map access 1 prop 0.15900 ns/op 0.16200 ns/op 0.98
Object get x1000 6.3690 ns/op 6.5770 ns/op 0.97
Map get x1000 0.58400 ns/op 0.60200 ns/op 0.97
Object set x1000 50.940 ns/op 49.918 ns/op 1.02
Map set x1000 42.920 ns/op 42.407 ns/op 1.01
Return object 10000 times 0.23300 ns/op 0.23090 ns/op 1.01
Throw Error 10000 times 4.1057 us/op 3.9954 us/op 1.03
fastMsgIdFn sha256 / 200 bytes 3.3890 us/op 3.3470 us/op 1.01
fastMsgIdFn h32 xxhash / 200 bytes 272.00 ns/op 273.00 ns/op 1.00
fastMsgIdFn h64 xxhash / 200 bytes 371.00 ns/op 376.00 ns/op 0.99
fastMsgIdFn sha256 / 1000 bytes 11.437 us/op 11.308 us/op 1.01
fastMsgIdFn h32 xxhash / 1000 bytes 400.00 ns/op 401.00 ns/op 1.00
fastMsgIdFn h64 xxhash / 1000 bytes 445.00 ns/op 449.00 ns/op 0.99
fastMsgIdFn sha256 / 10000 bytes 100.74 us/op 101.83 us/op 0.99
fastMsgIdFn h32 xxhash / 10000 bytes 1.8660 us/op 1.8750 us/op 1.00
fastMsgIdFn h64 xxhash / 10000 bytes 1.3270 us/op 1.3330 us/op 1.00
enrSubnets - fastDeserialize 64 bits 1.2410 us/op 1.2420 us/op 1.00
enrSubnets - ssz BitVector 64 bits 458.00 ns/op 467.00 ns/op 0.98
enrSubnets - fastDeserialize 4 bits 168.00 ns/op 166.00 ns/op 1.01
enrSubnets - ssz BitVector 4 bits 461.00 ns/op 476.00 ns/op 0.97
prioritizePeers score -10:0 att 32-0.1 sync 2-0 92.231 us/op 91.623 us/op 1.01
prioritizePeers score 0:0 att 32-0.25 sync 2-0.25 119.98 us/op 117.93 us/op 1.02
prioritizePeers score 0:0 att 32-0.5 sync 2-0.5 169.44 us/op 164.31 us/op 1.03
prioritizePeers score 0:0 att 64-0.75 sync 4-0.75 301.83 us/op 296.76 us/op 1.02
prioritizePeers score 0:0 att 64-1 sync 4-1 356.06 us/op 351.82 us/op 1.01
array of 16000 items push then shift 1.6249 us/op 1.6064 us/op 1.01
LinkedList of 16000 items push then shift 8.8530 ns/op 8.6080 ns/op 1.03
array of 16000 items push then pop 80.302 ns/op 77.381 ns/op 1.04
LinkedList of 16000 items push then pop 8.6130 ns/op 8.3800 ns/op 1.03
array of 24000 items push then shift 2.2598 us/op 2.3107 us/op 0.98
LinkedList of 24000 items push then shift 8.5580 ns/op 8.6480 ns/op 0.99
array of 24000 items push then pop 72.343 ns/op 70.546 ns/op 1.03
LinkedList of 24000 items push then pop 8.4220 ns/op 8.3030 ns/op 1.01
intersect bitArray bitLen 8 12.995 ns/op 12.982 ns/op 1.00
intersect array and set length 8 75.028 ns/op 76.244 ns/op 0.98
intersect bitArray bitLen 128 42.883 ns/op 43.155 ns/op 0.99
intersect array and set length 128 1.0298 us/op 1.0253 us/op 1.00
Buffer.concat 32 items 2.9100 us/op 2.8960 us/op 1.00
Uint8Array.set 32 items 2.9250 us/op 2.3390 us/op 1.25
pass gossip attestations to forkchoice per slot 3.3010 ms/op 2.2660 ms/op 1.46
computeDeltas 3.3436 ms/op 3.3616 ms/op 0.99
computeProposerBoostScoreFromBalances 1.7257 ms/op 1.7546 ms/op 0.98
altair processAttestation - 250000 vs - 7PWei normalcase 2.0915 ms/op 2.1331 ms/op 0.98
altair processAttestation - 250000 vs - 7PWei worstcase 3.2842 ms/op 3.2182 ms/op 1.02
altair processAttestation - setStatus - 1/6 committees join 139.35 us/op 135.31 us/op 1.03
altair processAttestation - setStatus - 1/3 committees join 273.24 us/op 269.94 us/op 1.01
altair processAttestation - setStatus - 1/2 committees join 359.74 us/op 349.37 us/op 1.03
altair processAttestation - setStatus - 2/3 committees join 453.89 us/op 452.25 us/op 1.00
altair processAttestation - setStatus - 4/5 committees join 642.30 us/op 631.69 us/op 1.02
altair processAttestation - setStatus - 100% committees join 755.01 us/op 732.72 us/op 1.03
altair processBlock - 250000 vs - 7PWei normalcase 19.445 ms/op 19.276 ms/op 1.01
altair processBlock - 250000 vs - 7PWei normalcase hashState 26.126 ms/op 26.384 ms/op 0.99
altair processBlock - 250000 vs - 7PWei worstcase 51.889 ms/op 53.812 ms/op 0.96
altair processBlock - 250000 vs - 7PWei worstcase hashState 68.314 ms/op 65.733 ms/op 1.04
phase0 processBlock - 250000 vs - 7PWei normalcase 1.9239 ms/op 1.9920 ms/op 0.97
phase0 processBlock - 250000 vs - 7PWei worstcase 27.896 ms/op 26.958 ms/op 1.03
altair processEth1Data - 250000 vs - 7PWei normalcase 472.92 us/op 465.87 us/op 1.02
vc - 250000 eb 1 eth1 1 we 0 wn 0 - smpl 15 6.7260 us/op 6.8010 us/op 0.99
vc - 250000 eb 0.95 eth1 0.1 we 0.05 wn 0 - smpl 219 19.404 us/op 19.301 us/op 1.01
vc - 250000 eb 0.95 eth1 0.3 we 0.05 wn 0 - smpl 42 8.4560 us/op 8.3090 us/op 1.02
vc - 250000 eb 0.95 eth1 0.7 we 0.05 wn 0 - smpl 18 6.3330 us/op 6.4090 us/op 0.99
vc - 250000 eb 0.1 eth1 0.1 we 0 wn 0 - smpl 1020 73.997 us/op 73.792 us/op 1.00
vc - 250000 eb 0.03 eth1 0.03 we 0 wn 0 - smpl 11777 621.32 us/op 603.63 us/op 1.03
vc - 250000 eb 0.01 eth1 0.01 we 0 wn 0 - smpl 16384 864.92 us/op 883.93 us/op 0.98
vc - 250000 eb 0 eth1 0 we 0 wn 0 - smpl 16384 840.78 us/op 836.43 us/op 1.01
vc - 250000 eb 0 eth1 0 we 0 wn 0 nocache - smpl 16384 2.1552 ms/op 2.2392 ms/op 0.96
vc - 250000 eb 0 eth1 1 we 0 wn 0 - smpl 16384 1.4810 ms/op 1.6554 ms/op 0.89
vc - 250000 eb 0 eth1 1 we 0 wn 0 nocache - smpl 16384 3.5357 ms/op 3.9194 ms/op 0.90
Tree 40 250000 create 289.45 ms/op 312.64 ms/op 0.93
Tree 40 250000 get(125000) 174.35 ns/op 178.04 ns/op 0.98
Tree 40 250000 set(125000) 891.10 ns/op 953.11 ns/op 0.93
Tree 40 250000 toArray() 16.552 ms/op 17.139 ms/op 0.97
Tree 40 250000 iterate all - toArray() + loop 16.748 ms/op 18.009 ms/op 0.93
Tree 40 250000 iterate all - get(i) 65.487 ms/op 64.853 ms/op 1.01
MutableVector 250000 create 10.636 ms/op 9.6274 ms/op 1.10
MutableVector 250000 get(125000) 6.1350 ns/op 6.3320 ns/op 0.97
MutableVector 250000 set(125000) 242.71 ns/op 246.06 ns/op 0.99
MutableVector 250000 toArray() 2.6983 ms/op 2.7661 ms/op 0.98
MutableVector 250000 iterate all - toArray() + loop 2.8137 ms/op 2.8806 ms/op 0.98
MutableVector 250000 iterate all - get(i) 1.4908 ms/op 1.4795 ms/op 1.01
Array 250000 create 2.5012 ms/op 2.5276 ms/op 0.99
Array 250000 clone - spread 1.2222 ms/op 1.1882 ms/op 1.03
Array 250000 get(125000) 0.60300 ns/op 0.57900 ns/op 1.04
Array 250000 set(125000) 0.67400 ns/op 0.65300 ns/op 1.03
Array 250000 iterate all - loop 81.322 us/op 108.46 us/op 0.75
effectiveBalanceIncrements clone Uint8Array 300000 27.628 us/op 26.677 us/op 1.04
effectiveBalanceIncrements clone MutableVector 300000 399.00 ns/op 375.00 ns/op 1.06
effectiveBalanceIncrements rw all Uint8Array 300000 165.87 us/op 165.88 us/op 1.00
effectiveBalanceIncrements rw all MutableVector 300000 82.152 ms/op 79.027 ms/op 1.04
phase0 afterProcessEpoch - 250000 vs - 7PWei 110.60 ms/op 108.98 ms/op 1.01
phase0 beforeProcessEpoch - 250000 vs - 7PWei 33.579 ms/op 31.998 ms/op 1.05
altair processEpoch - mainnet_e81889 302.07 ms/op 331.96 ms/op 0.91
mainnet_e81889 - altair beforeProcessEpoch 46.120 ms/op 71.190 ms/op 0.65
mainnet_e81889 - altair processJustificationAndFinalization 18.658 us/op 15.148 us/op 1.23
mainnet_e81889 - altair processInactivityUpdates 6.7375 ms/op 5.4655 ms/op 1.23
mainnet_e81889 - altair processRewardsAndPenalties 67.636 ms/op 63.172 ms/op 1.07
mainnet_e81889 - altair processRegistryUpdates 2.8160 us/op 3.1300 us/op 0.90
mainnet_e81889 - altair processSlashings 527.00 ns/op 445.00 ns/op 1.18
mainnet_e81889 - altair processEth1DataReset 523.00 ns/op 481.00 ns/op 1.09
mainnet_e81889 - altair processEffectiveBalanceUpdates 1.1756 ms/op 1.2047 ms/op 0.98
mainnet_e81889 - altair processSlashingsReset 4.9380 us/op 4.6160 us/op 1.07
mainnet_e81889 - altair processRandaoMixesReset 4.5910 us/op 4.3580 us/op 1.05
mainnet_e81889 - altair processHistoricalRootsUpdate 615.00 ns/op 569.00 ns/op 1.08
mainnet_e81889 - altair processParticipationFlagUpdates 2.3890 us/op 2.3850 us/op 1.00
mainnet_e81889 - altair processSyncCommitteeUpdates 546.00 ns/op 478.00 ns/op 1.14
mainnet_e81889 - altair afterProcessEpoch 115.98 ms/op 115.90 ms/op 1.00
phase0 processEpoch - mainnet_e58758 316.29 ms/op 318.09 ms/op 0.99
mainnet_e58758 - phase0 beforeProcessEpoch 120.30 ms/op 125.49 ms/op 0.96
mainnet_e58758 - phase0 processJustificationAndFinalization 15.001 us/op 15.623 us/op 0.96
mainnet_e58758 - phase0 processRewardsAndPenalties 53.088 ms/op 52.881 ms/op 1.00
mainnet_e58758 - phase0 processRegistryUpdates 7.8770 us/op 7.8410 us/op 1.00
mainnet_e58758 - phase0 processSlashings 503.00 ns/op 501.00 ns/op 1.00
mainnet_e58758 - phase0 processEth1DataReset 548.00 ns/op 530.00 ns/op 1.03
mainnet_e58758 - phase0 processEffectiveBalanceUpdates 939.62 us/op 940.53 us/op 1.00
mainnet_e58758 - phase0 processSlashingsReset 3.6030 us/op 2.7010 us/op 1.33
mainnet_e58758 - phase0 processRandaoMixesReset 4.7960 us/op 4.1240 us/op 1.16
mainnet_e58758 - phase0 processHistoricalRootsUpdate 710.00 ns/op 540.00 ns/op 1.31
mainnet_e58758 - phase0 processParticipationRecordUpdates 5.1920 us/op 3.8680 us/op 1.34
mainnet_e58758 - phase0 afterProcessEpoch 94.175 ms/op 94.872 ms/op 0.99
phase0 processEffectiveBalanceUpdates - 250000 normalcase 1.2323 ms/op 1.2327 ms/op 1.00
phase0 processEffectiveBalanceUpdates - 250000 worstcase 0.5 1.5139 ms/op 1.4413 ms/op 1.05
altair processInactivityUpdates - 250000 normalcase 24.743 ms/op 20.121 ms/op 1.23
altair processInactivityUpdates - 250000 worstcase 26.023 ms/op 22.878 ms/op 1.14
phase0 processRegistryUpdates - 250000 normalcase 6.0690 us/op 6.3960 us/op 0.95
phase0 processRegistryUpdates - 250000 badcase_full_deposits 237.13 us/op 240.72 us/op 0.99
phase0 processRegistryUpdates - 250000 worstcase 0.5 129.07 ms/op 124.42 ms/op 1.04
altair processRewardsAndPenalties - 250000 normalcase 69.583 ms/op 44.065 ms/op 1.58
altair processRewardsAndPenalties - 250000 worstcase 69.361 ms/op 42.864 ms/op 1.62
phase0 getAttestationDeltas - 250000 normalcase 6.5725 ms/op 6.4900 ms/op 1.01
phase0 getAttestationDeltas - 250000 worstcase 6.3943 ms/op 6.6982 ms/op 0.95
phase0 processSlashings - 250000 worstcase 3.2881 ms/op 3.4019 ms/op 0.97
altair processSyncCommitteeUpdates - 250000 173.01 ms/op 174.86 ms/op 0.99
BeaconState.hashTreeRoot - No change 250.00 ns/op 258.00 ns/op 0.97
BeaconState.hashTreeRoot - 1 full validator 51.744 us/op 50.158 us/op 1.03
BeaconState.hashTreeRoot - 32 full validator 479.21 us/op 532.62 us/op 0.90
BeaconState.hashTreeRoot - 512 full validator 5.4412 ms/op 5.0830 ms/op 1.07
BeaconState.hashTreeRoot - 1 validator.effectiveBalance 61.832 us/op 60.691 us/op 1.02
BeaconState.hashTreeRoot - 32 validator.effectiveBalance 913.94 us/op 874.30 us/op 1.05
BeaconState.hashTreeRoot - 512 validator.effectiveBalance 11.327 ms/op 11.109 ms/op 1.02
BeaconState.hashTreeRoot - 1 balances 47.611 us/op 49.117 us/op 0.97
BeaconState.hashTreeRoot - 32 balances 429.70 us/op 442.87 us/op 0.97
BeaconState.hashTreeRoot - 512 balances 4.3133 ms/op 4.3663 ms/op 0.99
BeaconState.hashTreeRoot - 250000 balances 76.350 ms/op 74.841 ms/op 1.02
aggregationBits - 2048 els - zipIndexesInBitList 14.768 us/op 15.224 us/op 0.97
regular array get 100000 times 32.026 us/op 32.787 us/op 0.98
wrappedArray get 100000 times 32.011 us/op 32.773 us/op 0.98
arrayWithProxy get 100000 times 15.298 ms/op 15.060 ms/op 1.02
ssz.Root.equals 537.00 ns/op 546.00 ns/op 0.98
byteArrayEquals 531.00 ns/op 538.00 ns/op 0.99
shuffle list - 16384 els 6.7428 ms/op 6.8453 ms/op 0.99
shuffle list - 250000 els 99.189 ms/op 99.829 ms/op 0.99
processSlot - 1 slots 8.2310 us/op 8.2980 us/op 0.99
processSlot - 32 slots 1.3013 ms/op 1.3219 ms/op 0.98
getEffectiveBalanceIncrementsZeroInactive - 250000 vs - 7PWei 185.06 us/op 184.12 us/op 1.01
getCommitteeAssignments - req 1 vs - 250000 vc 2.9071 ms/op 2.8425 ms/op 1.02
getCommitteeAssignments - req 100 vs - 250000 vc 4.0733 ms/op 4.0162 ms/op 1.01
getCommitteeAssignments - req 1000 vs - 250000 vc 4.3754 ms/op 4.4767 ms/op 0.98
RootCache.getBlockRootAtSlot - 250000 vs - 7PWei 4.5500 ns/op 4.3100 ns/op 1.06
state getBlockRootAtSlot - 250000 vs - 7PWei 887.24 ns/op 937.30 ns/op 0.95
computeProposers - vc 250000 10.399 ms/op 10.617 ms/op 0.98
computeEpochShuffling - vc 250000 101.19 ms/op 101.81 ms/op 0.99
getNextSyncCommittee - vc 250000 175.64 ms/op 175.51 ms/op 1.00

by benchmarkbot/action

@dapplion
Copy link
Contributor Author

@g11tech can you take over this PR and get it merge ready?

@g11tech g11tech force-pushed the dapplion/eip4844-network-review branch from 20111de to 8911f75 Compare February 19, 2023 14:14
g11tech
g11tech approved these changes Feb 19, 2023
View reviewed changes
Copy link
Contributor

@g11tech g11tech left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm leaving unmerged for second pair of eyes

wemeetagain
wemeetagain approved these changes Feb 19, 2023
View reviewed changes
Copy link
Member

@wemeetagain wemeetagain left a comment

Choose a reason for hiding this comment

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

Looks good. Appreciate you calling out the changes in the description

@wemeetagain wemeetagain merged commit f75d109 into unstable Feb 19, 2023
@wemeetagain wemeetagain deleted the dapplion/eip4844-network-review branch February 19, 2023 23:34
dapplion
dapplion commented Feb 20, 2023
View reviewed changes
packages/beacon-node/package.json
@@ -131,6 +131,7 @@
"@multiformats/multiaddr": "^11.0.0",
"@types/datastore-level": "^3.0.0",
"buffer-xor": "^2.0.2",
"c-kzg": "^1.0.9",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@g11tech Do we want to install c-kzg always now? Current strategy was to keep as optional dependency right?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, we can install it always 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure? deneb is not necessary now and it can crash Lodestar installations

@wemeetagain
Copy link
Member

🎉 This PR is included in v1.6.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@g11tech g11tech g11tech approved these changes

@wemeetagain wemeetagain wemeetagain approved these changes

Assignees

@g11tech g11tech

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

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