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

Fix domain derivation on LC syncAggregate validation #5252

Merged
merged 1 commit into from
Mar 9, 2023
Merged

Fix domain derivation on LC syncAggregate validation #5252

merged 1 commit into from
Mar 9, 2023

Conversation

dapplion
Copy link
Contributor

@dapplion dapplion commented Mar 9, 2023

Motivation

Description

Fix off by one error causing invalid signature errors on fork boundary

@dapplion dapplion requested a review from a team as a code owner March 9, 2023 15:03
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023

Performance Report

✔️ no performance regression detected

Full benchmark results
Benchmark suite Current: 2ebc877 Previous: 4d89a0e Ratio
getPubkeys - index2pubkey - req 1000 vs - 250000 vc 527.34 us/op 858.65 us/op 0.61
getPubkeys - validatorsArr - req 1000 vs - 250000 vc 47.964 us/op 46.688 us/op 1.03
BLS verify - blst-native 1.2358 ms/op 1.1724 ms/op 1.05
BLS verifyMultipleSignatures 3 - blst-native 2.5268 ms/op 2.3848 ms/op 1.06
BLS verifyMultipleSignatures 8 - blst-native 5.4454 ms/op 5.2322 ms/op 1.04
BLS verifyMultipleSignatures 32 - blst-native 19.885 ms/op 18.927 ms/op 1.05
BLS aggregatePubkeys 32 - blst-native 26.433 us/op 25.447 us/op 1.04
BLS aggregatePubkeys 128 - blst-native 103.65 us/op 99.369 us/op 1.04
getAttestationsForBlock 59.009 ms/op 51.953 ms/op 1.14
isKnown best case - 1 super set check 258.00 ns/op 245.00 ns/op 1.05
isKnown normal case - 2 super set checks 257.00 ns/op 242.00 ns/op 1.06
isKnown worse case - 16 super set checks 257.00 ns/op 240.00 ns/op 1.07
CheckpointStateCache - add get delete 5.4630 us/op 4.7720 us/op 1.14
validate gossip signedAggregateAndProof - struct 2.9624 ms/op 2.6672 ms/op 1.11
validate gossip attestation - struct 1.4318 ms/op 1.2940 ms/op 1.11
pickEth1Vote - no votes 1.3538 ms/op 1.3116 ms/op 1.03
pickEth1Vote - max votes 10.324 ms/op 10.072 ms/op 1.03
pickEth1Vote - Eth1Data hashTreeRoot value x2048 9.8512 ms/op 8.3245 ms/op 1.18
pickEth1Vote - Eth1Data hashTreeRoot tree x2048 16.415 ms/op 14.284 ms/op 1.15
pickEth1Vote - Eth1Data fastSerialize value x2048 754.72 us/op 723.33 us/op 1.04
pickEth1Vote - Eth1Data fastSerialize tree x2048 7.2454 ms/op 7.9155 ms/op 0.92
bytes32 toHexString 550.00 ns/op 476.00 ns/op 1.16
bytes32 Buffer.toString(hex) 434.00 ns/op 338.00 ns/op 1.28
bytes32 Buffer.toString(hex) from Uint8Array 637.00 ns/op 534.00 ns/op 1.19
bytes32 Buffer.toString(hex) + 0x 449.00 ns/op 340.00 ns/op 1.32
Object access 1 prop 0.21600 ns/op 0.15800 ns/op 1.37
Map access 1 prop 0.16700 ns/op 0.16000 ns/op 1.04
Object get x1000 8.6900 ns/op 6.3350 ns/op 1.37
Map get x1000 0.69900 ns/op 0.61500 ns/op 1.14
Object set x1000 76.358 ns/op 51.000 ns/op 1.50
Map set x1000 61.423 ns/op 43.492 ns/op 1.41
Return object 10000 times 0.27960 ns/op 0.23190 ns/op 1.21
Throw Error 10000 times 4.7363 us/op 4.1396 us/op 1.14
fastMsgIdFn sha256 / 200 bytes 3.9010 us/op 3.3690 us/op 1.16
fastMsgIdFn h32 xxhash / 200 bytes 347.00 ns/op 286.00 ns/op 1.21
fastMsgIdFn h64 xxhash / 200 bytes 538.00 ns/op 396.00 ns/op 1.36
fastMsgIdFn sha256 / 1000 bytes 12.601 us/op 11.285 us/op 1.12
fastMsgIdFn h32 xxhash / 1000 bytes 480.00 ns/op 416.00 ns/op 1.15
fastMsgIdFn h64 xxhash / 1000 bytes 615.00 ns/op 476.00 ns/op 1.29
fastMsgIdFn sha256 / 10000 bytes 112.53 us/op 101.48 us/op 1.11
fastMsgIdFn h32 xxhash / 10000 bytes 2.1080 us/op 1.8960 us/op 1.11
fastMsgIdFn h64 xxhash / 10000 bytes 1.5540 us/op 1.3490 us/op 1.15
enrSubnets - fastDeserialize 64 bits 1.9950 us/op 1.2570 us/op 1.59
enrSubnets - ssz BitVector 64 bits 748.00 ns/op 505.00 ns/op 1.48
enrSubnets - fastDeserialize 4 bits 240.00 ns/op 178.00 ns/op 1.35
enrSubnets - ssz BitVector 4 bits 671.00 ns/op 504.00 ns/op 1.33
prioritizePeers score -10:0 att 32-0.1 sync 2-0 132.98 us/op 105.45 us/op 1.26
prioritizePeers score 0:0 att 32-0.25 sync 2-0.25 192.58 us/op 128.99 us/op 1.49
prioritizePeers score 0:0 att 32-0.5 sync 2-0.5 297.17 us/op 162.86 us/op 1.82
prioritizePeers score 0:0 att 64-0.75 sync 4-0.75 425.17 us/op 295.59 us/op 1.44
prioritizePeers score 0:0 att 64-1 sync 4-1 490.55 us/op 353.28 us/op 1.39
array of 16000 items push then shift 1.8796 us/op 1.6070 us/op 1.17
LinkedList of 16000 items push then shift 10.500 ns/op 8.6870 ns/op 1.21
array of 16000 items push then pop 131.96 ns/op 74.284 ns/op 1.78
LinkedList of 16000 items push then pop 12.720 ns/op 8.5110 ns/op 1.49
array of 24000 items push then shift 3.4694 us/op 2.3490 us/op 1.48
LinkedList of 24000 items push then shift 14.333 ns/op 8.5960 ns/op 1.67
array of 24000 items push then pop 126.85 ns/op 77.430 ns/op 1.64
LinkedList of 24000 items push then pop 16.929 ns/op 8.2190 ns/op 2.06
intersect bitArray bitLen 8 27.720 ns/op 12.967 ns/op 2.14
intersect array and set length 8 146.94 ns/op 74.740 ns/op 1.97
intersect bitArray bitLen 128 69.495 ns/op 42.473 ns/op 1.64
intersect array and set length 128 1.9412 us/op 1.0166 us/op 1.91
Buffer.concat 32 items 3.3080 us/op 2.6380 us/op 1.25
Uint8Array.set 32 items 3.4200 us/op 2.9310 us/op 1.17
pass gossip attestations to forkchoice per slot 3.4835 ms/op 3.7206 ms/op 0.94
computeDeltas 3.8672 ms/op 3.3664 ms/op 1.15
computeProposerBoostScoreFromBalances 2.1567 ms/op 1.7665 ms/op 1.22
altair processAttestation - 250000 vs - 7PWei normalcase 4.2490 ms/op 2.1025 ms/op 2.02
altair processAttestation - 250000 vs - 7PWei worstcase 5.5775 ms/op 3.1880 ms/op 1.75
altair processAttestation - setStatus - 1/6 committees join 178.47 us/op 135.14 us/op 1.32
altair processAttestation - setStatus - 1/3 committees join 323.39 us/op 275.04 us/op 1.18
altair processAttestation - setStatus - 1/2 committees join 589.92 us/op 363.90 us/op 1.62
altair processAttestation - setStatus - 2/3 committees join 496.85 us/op 457.12 us/op 1.09
altair processAttestation - setStatus - 4/5 committees join 711.40 us/op 638.87 us/op 1.11
altair processAttestation - setStatus - 100% committees join 897.98 us/op 752.22 us/op 1.19
altair processBlock - 250000 vs - 7PWei normalcase 27.424 ms/op 19.173 ms/op 1.43
altair processBlock - 250000 vs - 7PWei normalcase hashState 42.398 ms/op 28.545 ms/op 1.49
altair processBlock - 250000 vs - 7PWei worstcase 81.028 ms/op 49.778 ms/op 1.63
altair processBlock - 250000 vs - 7PWei worstcase hashState 107.90 ms/op 66.658 ms/op 1.62
phase0 processBlock - 250000 vs - 7PWei normalcase 4.3437 ms/op 2.0118 ms/op 2.16
phase0 processBlock - 250000 vs - 7PWei worstcase 50.760 ms/op 27.300 ms/op 1.86
altair processEth1Data - 250000 vs - 7PWei normalcase 777.41 us/op 518.56 us/op 1.50
vc - 250000 eb 1 eth1 1 we 0 wn 0 - smpl 15 15.021 us/op 6.5920 us/op 2.28
vc - 250000 eb 0.95 eth1 0.1 we 0.05 wn 0 - smpl 219 39.716 us/op 19.023 us/op 2.09
vc - 250000 eb 0.95 eth1 0.3 we 0.05 wn 0 - smpl 42 18.275 us/op 8.2160 us/op 2.22
vc - 250000 eb 0.95 eth1 0.7 we 0.05 wn 0 - smpl 18 12.255 us/op 6.4750 us/op 1.89
vc - 250000 eb 0.1 eth1 0.1 we 0 wn 0 - smpl 1020 128.36 us/op 74.742 us/op 1.72
vc - 250000 eb 0.03 eth1 0.03 we 0 wn 0 - smpl 11777 775.72 us/op 611.23 us/op 1.27
vc - 250000 eb 0.01 eth1 0.01 we 0 wn 0 - smpl 16384 1.0072 ms/op 903.99 us/op 1.11
vc - 250000 eb 0 eth1 0 we 0 wn 0 - smpl 16384 1.1988 ms/op 842.54 us/op 1.42
vc - 250000 eb 0 eth1 0 we 0 wn 0 nocache - smpl 16384 3.3916 ms/op 2.2814 ms/op 1.49
vc - 250000 eb 0 eth1 1 we 0 wn 0 - smpl 16384 1.7216 ms/op 1.5965 ms/op 1.08
vc - 250000 eb 0 eth1 1 we 0 wn 0 nocache - smpl 16384 4.5673 ms/op 3.8836 ms/op 1.18
Tree 40 250000 create 393.84 ms/op 305.02 ms/op 1.29
Tree 40 250000 get(125000) 207.96 ns/op 178.30 ns/op 1.17
Tree 40 250000 set(125000) 1.2668 us/op 896.01 ns/op 1.41
Tree 40 250000 toArray() 23.906 ms/op 16.762 ms/op 1.43
Tree 40 250000 iterate all - toArray() + loop 23.741 ms/op 17.954 ms/op 1.32
Tree 40 250000 iterate all - get(i) 80.354 ms/op 64.431 ms/op 1.25
MutableVector 250000 create 13.305 ms/op 9.5588 ms/op 1.39
MutableVector 250000 get(125000) 6.7990 ns/op 6.2470 ns/op 1.09
MutableVector 250000 set(125000) 403.87 ns/op 232.95 ns/op 1.73
MutableVector 250000 toArray() 4.1333 ms/op 2.5511 ms/op 1.62
MutableVector 250000 iterate all - toArray() + loop 4.0000 ms/op 2.6547 ms/op 1.51
MutableVector 250000 iterate all - get(i) 1.5640 ms/op 1.4544 ms/op 1.08
Array 250000 create 3.1663 ms/op 2.3915 ms/op 1.32
Array 250000 clone - spread 1.2752 ms/op 1.1040 ms/op 1.16
Array 250000 get(125000) 0.66400 ns/op 0.52900 ns/op 1.26
Array 250000 set(125000) 0.73400 ns/op 0.61100 ns/op 1.20
Array 250000 iterate all - loop 85.530 us/op 81.421 us/op 1.05
effectiveBalanceIncrements clone Uint8Array 300000 35.485 us/op 23.926 us/op 1.48
effectiveBalanceIncrements clone MutableVector 300000 416.00 ns/op 345.00 ns/op 1.21
effectiveBalanceIncrements rw all Uint8Array 300000 175.75 us/op 160.46 us/op 1.10
effectiveBalanceIncrements rw all MutableVector 300000 92.655 ms/op 76.542 ms/op 1.21
phase0 afterProcessEpoch - 250000 vs - 7PWei 122.68 ms/op 110.77 ms/op 1.11
phase0 beforeProcessEpoch - 250000 vs - 7PWei 37.110 ms/op 33.794 ms/op 1.10
altair processEpoch - mainnet_e81889 335.08 ms/op 319.97 ms/op 1.05
mainnet_e81889 - altair beforeProcessEpoch 65.227 ms/op 49.092 ms/op 1.33
mainnet_e81889 - altair processJustificationAndFinalization 17.297 us/op 16.287 us/op 1.06
mainnet_e81889 - altair processInactivityUpdates 6.0881 ms/op 5.6331 ms/op 1.08
mainnet_e81889 - altair processRewardsAndPenalties 68.326 ms/op 69.713 ms/op 0.98
mainnet_e81889 - altair processRegistryUpdates 2.6540 us/op 2.6550 us/op 1.00
mainnet_e81889 - altair processSlashings 456.00 ns/op 413.00 ns/op 1.10
mainnet_e81889 - altair processEth1DataReset 543.00 ns/op 688.00 ns/op 0.79
mainnet_e81889 - altair processEffectiveBalanceUpdates 1.2670 ms/op 1.2397 ms/op 1.02
mainnet_e81889 - altair processSlashingsReset 4.7370 us/op 4.8920 us/op 0.97
mainnet_e81889 - altair processRandaoMixesReset 7.6140 us/op 5.1720 us/op 1.47
mainnet_e81889 - altair processHistoricalRootsUpdate 1.0140 us/op 600.00 ns/op 1.69
mainnet_e81889 - altair processParticipationFlagUpdates 3.1660 us/op 2.4240 us/op 1.31
mainnet_e81889 - altair processSyncCommitteeUpdates 840.00 ns/op 771.00 ns/op 1.09
mainnet_e81889 - altair afterProcessEpoch 133.92 ms/op 123.34 ms/op 1.09
phase0 processEpoch - mainnet_e58758 360.59 ms/op 363.37 ms/op 0.99
mainnet_e58758 - phase0 beforeProcessEpoch 139.76 ms/op 132.74 ms/op 1.05
mainnet_e58758 - phase0 processJustificationAndFinalization 18.486 us/op 16.111 us/op 1.15
mainnet_e58758 - phase0 processRewardsAndPenalties 63.047 ms/op 68.698 ms/op 0.92
mainnet_e58758 - phase0 processRegistryUpdates 8.6140 us/op 7.5690 us/op 1.14
mainnet_e58758 - phase0 processSlashings 573.00 ns/op 491.00 ns/op 1.17
mainnet_e58758 - phase0 processEth1DataReset 500.00 ns/op 506.00 ns/op 0.99
mainnet_e58758 - phase0 processEffectiveBalanceUpdates 1.0611 ms/op 1.2353 ms/op 0.86
mainnet_e58758 - phase0 processSlashingsReset 3.5130 us/op 3.6700 us/op 0.96
mainnet_e58758 - phase0 processRandaoMixesReset 5.1530 us/op 4.1580 us/op 1.24
mainnet_e58758 - phase0 processHistoricalRootsUpdate 703.00 ns/op 578.00 ns/op 1.22
mainnet_e58758 - phase0 processParticipationRecordUpdates 4.9790 us/op 4.0700 us/op 1.22
mainnet_e58758 - phase0 afterProcessEpoch 101.72 ms/op 94.536 ms/op 1.08
phase0 processEffectiveBalanceUpdates - 250000 normalcase 1.2821 ms/op 1.2035 ms/op 1.07
phase0 processEffectiveBalanceUpdates - 250000 worstcase 0.5 1.5372 ms/op 1.3799 ms/op 1.11
altair processInactivityUpdates - 250000 normalcase 26.580 ms/op 24.328 ms/op 1.09
altair processInactivityUpdates - 250000 worstcase 27.369 ms/op 26.336 ms/op 1.04
phase0 processRegistryUpdates - 250000 normalcase 6.7880 us/op 6.6370 us/op 1.02
phase0 processRegistryUpdates - 250000 badcase_full_deposits 282.83 us/op 241.91 us/op 1.17
phase0 processRegistryUpdates - 250000 worstcase 0.5 128.12 ms/op 129.83 ms/op 0.99
altair processRewardsAndPenalties - 250000 normalcase 63.866 ms/op 65.517 ms/op 0.97
altair processRewardsAndPenalties - 250000 worstcase 70.363 ms/op 69.828 ms/op 1.01
phase0 getAttestationDeltas - 250000 normalcase 6.9395 ms/op 6.4260 ms/op 1.08
phase0 getAttestationDeltas - 250000 worstcase 6.8811 ms/op 6.4140 ms/op 1.07
phase0 processSlashings - 250000 worstcase 3.6436 ms/op 3.3899 ms/op 1.07
altair processSyncCommitteeUpdates - 250000 175.41 ms/op 175.15 ms/op 1.00
BeaconState.hashTreeRoot - No change 274.00 ns/op 355.00 ns/op 0.77
BeaconState.hashTreeRoot - 1 full validator 52.049 us/op 50.711 us/op 1.03
BeaconState.hashTreeRoot - 32 full validator 564.21 us/op 495.53 us/op 1.14
BeaconState.hashTreeRoot - 512 full validator 5.3909 ms/op 5.1217 ms/op 1.05
BeaconState.hashTreeRoot - 1 validator.effectiveBalance 66.184 us/op 60.779 us/op 1.09
BeaconState.hashTreeRoot - 32 validator.effectiveBalance 976.08 us/op 900.25 us/op 1.08
BeaconState.hashTreeRoot - 512 validator.effectiveBalance 12.106 ms/op 11.108 ms/op 1.09
BeaconState.hashTreeRoot - 1 balances 53.866 us/op 50.022 us/op 1.08
BeaconState.hashTreeRoot - 32 balances 487.90 us/op 444.36 us/op 1.10
BeaconState.hashTreeRoot - 512 balances 4.9761 ms/op 4.3766 ms/op 1.14
BeaconState.hashTreeRoot - 250000 balances 77.423 ms/op 79.342 ms/op 0.98
aggregationBits - 2048 els - zipIndexesInBitList 17.003 us/op 16.360 us/op 1.04
regular array get 100000 times 35.002 us/op 32.583 us/op 1.07
wrappedArray get 100000 times 45.080 us/op 32.491 us/op 1.39
arrayWithProxy get 100000 times 16.487 ms/op 15.284 ms/op 1.08
ssz.Root.equals 636.00 ns/op 527.00 ns/op 1.21
byteArrayEquals 579.00 ns/op 527.00 ns/op 1.10
shuffle list - 16384 els 7.1700 ms/op 6.5920 ms/op 1.09
shuffle list - 250000 els 104.99 ms/op 96.635 ms/op 1.09
processSlot - 1 slots 9.2680 us/op 8.9370 us/op 1.04
processSlot - 32 slots 1.4164 ms/op 1.3022 ms/op 1.09
getEffectiveBalanceIncrementsZeroInactive - 250000 vs - 7PWei 220.21 us/op 187.76 us/op 1.17
getCommitteeAssignments - req 1 vs - 250000 vc 2.9818 ms/op 2.8148 ms/op 1.06
getCommitteeAssignments - req 100 vs - 250000 vc 4.2768 ms/op 4.0293 ms/op 1.06
getCommitteeAssignments - req 1000 vs - 250000 vc 5.0081 ms/op 4.3349 ms/op 1.16
RootCache.getBlockRootAtSlot - 250000 vs - 7PWei 5.5400 ns/op 4.7000 ns/op 1.18
state getBlockRootAtSlot - 250000 vs - 7PWei 776.36 ns/op 941.27 ns/op 0.82
computeProposers - vc 250000 11.427 ms/op 10.316 ms/op 1.11
computeEpochShuffling - vc 250000 107.03 ms/op 98.357 ms/op 1.09
getNextSyncCommittee - vc 250000 191.88 ms/op 171.10 ms/op 1.12

by benchmarkbot/action

g11tech
g11tech reviewed Mar 9, 2023
View reviewed changes
packages/light-client/src/spec/validateLightClientUpdate.ts
@@ -113,7 +113,7 @@ export function validateLightClientUpdate(

const signingRoot = ssz.phase0.SigningData.hashTreeRoot({
objectRoot: ssz.phase0.BeaconBlockHeader.hashTreeRoot(update.attestedHeader.beacon),
domain: store.config.getDomain(update.signatureSlot, DOMAIN_SYNC_COMMITTEE),
domain: store.config.getDomain(update.signatureSlot - 1, DOMAIN_SYNC_COMMITTEE),
Copy link
Contributor

Choose a reason for hiding this comment

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

or should we use attestedHeader.beacon.slot?

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh just checked the linked PR , lgtm 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

sadly, attested_header.beacon.slot may be incorrect.

Example:

BELLATRIX_FORK_EPOCH = 0
CAPELLA_FORK_EPOCH = 255
DENEB_FORK_EPOCH = 256

attested_header.slot = 8159 (bellatrix)
signature_slot = 8192 (Deneb)
fork_version_slot = 8191 (Capella)

so, sync committee would be determined by sync committee at slot 8192, the fork version is determined by slot 8191, and the object is from slot 8159

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks @etan-status for the insightful example 🙏 ❤️

@g11tech g11tech merged commit 9c7c08b into unstable Mar 9, 2023
@g11tech g11tech deleted the dapplion/lc-sync-aggregate branch March 9, 2023 17:20
@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

@etan-status etan-status etan-status left review comments

@g11tech g11tech g11tech 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.
4 participants
@dapplion @wemeetagain @g11tech @etan-status