Skip to content

Fix the args order in evaluating isBetterUpdate for new partial update - #5090

Merged
g11tech merged 1 commit into
unstablefrom
g11tech/better-update-fix
Feb 2, 2023
Merged

Fix the args order in evaluating isBetterUpdate for new partial update#5090
g11tech merged 1 commit into
unstablefrom
g11tech/better-update-fix

Conversation

@g11tech

@g11tech g11tech commented Feb 1, 2023

Copy link
Copy Markdown
Contributor

As reported by @claravanstaden in discord, the light client updates were not seemingly correct, on debugging figured out that the first assigned update of the period might not be getting replaced by better updates which may arrive later.

On log debugging + code inspection it was discovered that the arg order to isBetterUpdate function was incorrect leading to opposite evalution.

this PR fixes the same

@g11tech
g11tech requested a review from a team as a code owner February 1, 2023 17:56
@github-actions

github-actions Bot commented Feb 1, 2023

Copy link
Copy Markdown
Contributor

Performance Report

鉁旓笍 no performance regression detected

Full benchmark results
Benchmark suite Current: aaf3421 Previous: bff1438 Ratio
getPubkeys - index2pubkey - req 1000 vs - 250000 vc 759.50 us/op 1.0618 ms/op 0.72
getPubkeys - validatorsArr - req 1000 vs - 250000 vc 45.106 us/op 58.949 us/op 0.77
BLS verify - blst-native 1.1458 ms/op 1.3359 ms/op 0.86
BLS verifyMultipleSignatures 3 - blst-native 2.3347 ms/op 2.7105 ms/op 0.86
BLS verifyMultipleSignatures 8 - blst-native 5.0005 ms/op 5.8152 ms/op 0.86
BLS verifyMultipleSignatures 32 - blst-native 18.083 ms/op 20.637 ms/op 0.88
BLS aggregatePubkeys 32 - blst-native 24.343 us/op 27.071 us/op 0.90
BLS aggregatePubkeys 128 - blst-native 94.649 us/op 106.60 us/op 0.89
getAttestationsForBlock 52.671 ms/op 79.336 ms/op 0.66
isKnown best case - 1 super set check 266.00 ns/op 301.00 ns/op 0.88
isKnown normal case - 2 super set checks 254.00 ns/op 296.00 ns/op 0.86
isKnown worse case - 16 super set checks 263.00 ns/op 293.00 ns/op 0.90
CheckpointStateCache - add get delete 5.0630 us/op 6.8650 us/op 0.74
validate gossip signedAggregateAndProof - struct 2.7184 ms/op 2.9809 ms/op 0.91
validate gossip attestation - struct 1.2948 ms/op 1.4241 ms/op 0.91
pickEth1Vote - no votes 1.2750 ms/op 1.4659 ms/op 0.87
pickEth1Vote - max votes 9.8863 ms/op 13.564 ms/op 0.73
pickEth1Vote - Eth1Data hashTreeRoot value x2048 8.6171 ms/op 10.046 ms/op 0.86
pickEth1Vote - Eth1Data hashTreeRoot tree x2048 14.679 ms/op 18.251 ms/op 0.80
pickEth1Vote - Eth1Data fastSerialize value x2048 661.68 us/op 806.22 us/op 0.82
pickEth1Vote - Eth1Data fastSerialize tree x2048 7.2790 ms/op 9.8562 ms/op 0.74
bytes32 toHexString 469.00 ns/op 800.00 ns/op 0.59
bytes32 Buffer.toString(hex) 331.00 ns/op 440.00 ns/op 0.75
bytes32 Buffer.toString(hex) from Uint8Array 531.00 ns/op 667.00 ns/op 0.80
bytes32 Buffer.toString(hex) + 0x 330.00 ns/op 439.00 ns/op 0.75
Object access 1 prop 0.16000 ns/op 0.22500 ns/op 0.71
Map access 1 prop 0.15800 ns/op 0.18200 ns/op 0.87
Object get x1000 6.2760 ns/op 7.0220 ns/op 0.89
Map get x1000 0.58600 ns/op 0.64500 ns/op 0.91
Object set x1000 49.637 ns/op 77.438 ns/op 0.64
Map set x1000 41.582 ns/op 59.698 ns/op 0.70
Return object 10000 times 0.22770 ns/op 0.27720 ns/op 0.82
Throw Error 10000 times 4.0247 us/op 4.6653 us/op 0.86
fastMsgIdFn sha256 / 200 bytes 3.3160 us/op 3.8630 us/op 0.86
fastMsgIdFn h32 xxhash / 200 bytes 264.00 ns/op 362.00 ns/op 0.73
fastMsgIdFn h64 xxhash / 200 bytes 366.00 ns/op 504.00 ns/op 0.73
fastMsgIdFn sha256 / 1000 bytes 11.223 us/op 12.760 us/op 0.88
fastMsgIdFn h32 xxhash / 1000 bytes 385.00 ns/op 507.00 ns/op 0.76
fastMsgIdFn h64 xxhash / 1000 bytes 436.00 ns/op 595.00 ns/op 0.73
fastMsgIdFn sha256 / 10000 bytes 100.60 us/op 111.60 us/op 0.90
fastMsgIdFn h32 xxhash / 10000 bytes 1.8480 us/op 2.1290 us/op 0.87
fastMsgIdFn h64 xxhash / 10000 bytes 1.2850 us/op 1.5490 us/op 0.83
enrSubnets - fastDeserialize 64 bits 1.2230 us/op 1.9280 us/op 0.63
enrSubnets - ssz BitVector 64 bits 465.00 ns/op 653.00 ns/op 0.71
enrSubnets - fastDeserialize 4 bits 160.00 ns/op 223.00 ns/op 0.72
enrSubnets - ssz BitVector 4 bits 466.00 ns/op 670.00 ns/op 0.70
prioritizePeers score -10:0 att 32-0.1 sync 2-0 95.415 us/op 118.40 us/op 0.81
prioritizePeers score 0:0 att 32-0.25 sync 2-0.25 122.83 us/op 144.32 us/op 0.85
prioritizePeers score 0:0 att 32-0.5 sync 2-0.5 159.34 us/op 215.89 us/op 0.74
prioritizePeers score 0:0 att 64-0.75 sync 4-0.75 294.69 us/op 397.00 us/op 0.74
prioritizePeers score 0:0 att 64-1 sync 4-1 352.46 us/op 494.23 us/op 0.71
array of 16000 items push then shift 1.5822 us/op 1.7872 us/op 0.89
LinkedList of 16000 items push then shift 8.5200 ns/op 10.061 ns/op 0.85
array of 16000 items push then pop 77.636 ns/op 131.84 ns/op 0.59
LinkedList of 16000 items push then pop 8.2850 ns/op 10.092 ns/op 0.82
array of 24000 items push then shift 2.2850 us/op 2.6056 us/op 0.88
LinkedList of 24000 items push then shift 8.5820 ns/op 10.587 ns/op 0.81
array of 24000 items push then pop 71.592 ns/op 91.934 ns/op 0.78
LinkedList of 24000 items push then pop 8.1020 ns/op 11.061 ns/op 0.73
intersect bitArray bitLen 8 12.751 ns/op 14.371 ns/op 0.89
intersect array and set length 8 73.907 ns/op 123.84 ns/op 0.60
intersect bitArray bitLen 128 42.126 ns/op 49.219 ns/op 0.86
intersect array and set length 128 1.0061 us/op 1.4175 us/op 0.71
Buffer.concat 32 items 2.6160 us/op 3.1760 us/op 0.82
Uint8Array.set 32 items 2.3760 us/op 3.1450 us/op 0.76
pass gossip attestations to forkchoice per slot 2.2970 ms/op 5.1304 ms/op 0.45
computeDeltas 3.2051 ms/op 3.5371 ms/op 0.91
computeProposerBoostScoreFromBalances 1.7346 ms/op 1.8859 ms/op 0.92
altair processAttestation - 250000 vs - 7PWei normalcase 2.0483 ms/op 2.5784 ms/op 0.79
altair processAttestation - 250000 vs - 7PWei worstcase 3.2193 ms/op 4.7340 ms/op 0.68
altair processAttestation - setStatus - 1/6 committees join 134.99 us/op 151.78 us/op 0.89
altair processAttestation - setStatus - 1/3 committees join 266.78 us/op 291.79 us/op 0.91
altair processAttestation - setStatus - 1/2 committees join 357.78 us/op 391.30 us/op 0.91
altair processAttestation - setStatus - 2/3 committees join 456.74 us/op 489.00 us/op 0.93
altair processAttestation - setStatus - 4/5 committees join 638.54 us/op 719.32 us/op 0.89
altair processAttestation - setStatus - 100% committees join 744.17 us/op 787.62 us/op 0.94
altair processBlock - 250000 vs - 7PWei normalcase 14.774 ms/op 20.132 ms/op 0.73
altair processBlock - 250000 vs - 7PWei normalcase hashState 25.682 ms/op 31.144 ms/op 0.82
altair processBlock - 250000 vs - 7PWei worstcase 45.400 ms/op 59.825 ms/op 0.76
altair processBlock - 250000 vs - 7PWei worstcase hashState 71.565 ms/op 75.977 ms/op 0.94
phase0 processBlock - 250000 vs - 7PWei normalcase 2.0095 ms/op 2.1419 ms/op 0.94
phase0 processBlock - 250000 vs - 7PWei worstcase 27.538 ms/op 30.618 ms/op 0.90
altair processEth1Data - 250000 vs - 7PWei normalcase 469.40 us/op 511.35 us/op 0.92
vc - 250000 eb 1 eth1 1 we 0 wn 0 - smpl 15 6.7540 us/op 8.0970 us/op 0.83
vc - 250000 eb 0.95 eth1 0.1 we 0.05 wn 0 - smpl 219 19.528 us/op 28.949 us/op 0.67
vc - 250000 eb 0.95 eth1 0.3 we 0.05 wn 0 - smpl 42 9.3210 us/op 12.008 us/op 0.78
vc - 250000 eb 0.95 eth1 0.7 we 0.05 wn 0 - smpl 18 8.2540 us/op 11.237 us/op 0.73
vc - 250000 eb 0.1 eth1 0.1 we 0 wn 0 - smpl 1020 94.792 us/op 116.29 us/op 0.82
vc - 250000 eb 0.03 eth1 0.03 we 0 wn 0 - smpl 11777 638.90 us/op 673.22 us/op 0.95
vc - 250000 eb 0.01 eth1 0.01 we 0 wn 0 - smpl 16384 925.49 us/op 1.1349 ms/op 0.82
vc - 250000 eb 0 eth1 0 we 0 wn 0 - smpl 16384 880.48 us/op 1.0203 ms/op 0.86
vc - 250000 eb 0 eth1 0 we 0 wn 0 nocache - smpl 16384 2.3172 ms/op 2.6025 ms/op 0.89
vc - 250000 eb 0 eth1 1 we 0 wn 0 - smpl 16384 1.4719 ms/op 1.9175 ms/op 0.77
vc - 250000 eb 0 eth1 1 we 0 wn 0 nocache - smpl 16384 3.7743 ms/op 4.2736 ms/op 0.88
Tree 40 250000 create 323.15 ms/op 365.20 ms/op 0.88
Tree 40 250000 get(125000) 186.83 ns/op 196.16 ns/op 0.95
Tree 40 250000 set(125000) 836.25 ns/op 1.0017 us/op 0.83
Tree 40 250000 toArray() 18.640 ms/op 20.538 ms/op 0.91
Tree 40 250000 iterate all - toArray() + loop 18.275 ms/op 22.853 ms/op 0.80
Tree 40 250000 iterate all - get(i) 69.331 ms/op 75.562 ms/op 0.92
MutableVector 250000 create 10.586 ms/op 14.150 ms/op 0.75
MutableVector 250000 get(125000) 6.3840 ns/op 6.5890 ns/op 0.97
MutableVector 250000 set(125000) 263.31 ns/op 266.18 ns/op 0.99
MutableVector 250000 toArray() 3.1334 ms/op 3.5806 ms/op 0.88
MutableVector 250000 iterate all - toArray() + loop 3.0775 ms/op 3.7855 ms/op 0.81
MutableVector 250000 iterate all - get(i) 1.5016 ms/op 1.6078 ms/op 0.93
Array 250000 create 2.7026 ms/op 3.1862 ms/op 0.85
Array 250000 clone - spread 1.2551 ms/op 1.2203 ms/op 1.03
Array 250000 get(125000) 0.59900 ns/op 0.63800 ns/op 0.94
Array 250000 set(125000) 0.67700 ns/op 0.66500 ns/op 1.02
Array 250000 iterate all - loop 83.803 us/op 115.14 us/op 0.73
effectiveBalanceIncrements clone Uint8Array 300000 30.775 us/op 38.323 us/op 0.80
effectiveBalanceIncrements clone MutableVector 300000 380.00 ns/op 371.00 ns/op 1.02
effectiveBalanceIncrements rw all Uint8Array 300000 167.12 us/op 178.68 us/op 0.94
effectiveBalanceIncrements rw all MutableVector 300000 80.626 ms/op 105.68 ms/op 0.76
phase0 afterProcessEpoch - 250000 vs - 7PWei 112.94 ms/op 124.74 ms/op 0.91
phase0 beforeProcessEpoch - 250000 vs - 7PWei 41.921 ms/op 53.999 ms/op 0.78
altair processEpoch - mainnet_e81889 304.09 ms/op 375.60 ms/op 0.81
mainnet_e81889 - altair beforeProcessEpoch 62.165 ms/op 74.973 ms/op 0.83
mainnet_e81889 - altair processJustificationAndFinalization 19.723 us/op 24.346 us/op 0.81
mainnet_e81889 - altair processInactivityUpdates 5.3740 ms/op 6.7539 ms/op 0.80
mainnet_e81889 - altair processRewardsAndPenalties 61.432 ms/op 78.301 ms/op 0.78
mainnet_e81889 - altair processRegistryUpdates 2.3220 us/op 3.3910 us/op 0.68
mainnet_e81889 - altair processSlashings 607.00 ns/op 1.1360 us/op 0.53
mainnet_e81889 - altair processEth1DataReset 1.6040 us/op 932.00 ns/op 1.72
mainnet_e81889 - altair processEffectiveBalanceUpdates 1.2470 ms/op 1.6374 ms/op 0.76
mainnet_e81889 - altair processSlashingsReset 5.5560 us/op 9.7590 us/op 0.57
mainnet_e81889 - altair processRandaoMixesReset 5.8360 us/op 11.077 us/op 0.53
mainnet_e81889 - altair processHistoricalRootsUpdate 728.00 ns/op 963.00 ns/op 0.76
mainnet_e81889 - altair processParticipationFlagUpdates 2.7210 us/op 6.0830 us/op 0.45
mainnet_e81889 - altair processSyncCommitteeUpdates 884.00 ns/op 816.00 ns/op 1.08
mainnet_e81889 - altair afterProcessEpoch 126.55 ms/op 141.38 ms/op 0.90
phase0 processEpoch - mainnet_e58758 341.71 ms/op 412.10 ms/op 0.83
mainnet_e58758 - phase0 beforeProcessEpoch 122.95 ms/op 118.92 ms/op 1.03
mainnet_e58758 - phase0 processJustificationAndFinalization 17.370 us/op 20.923 us/op 0.83
mainnet_e58758 - phase0 processRewardsAndPenalties 64.123 ms/op 62.405 ms/op 1.03
mainnet_e58758 - phase0 processRegistryUpdates 7.6330 us/op 7.2900 us/op 1.05
mainnet_e58758 - phase0 processSlashings 494.00 ns/op 583.00 ns/op 0.85
mainnet_e58758 - phase0 processEth1DataReset 434.00 ns/op 968.00 ns/op 0.45
mainnet_e58758 - phase0 processEffectiveBalanceUpdates 1.3142 ms/op 1.2878 ms/op 1.02
mainnet_e58758 - phase0 processSlashingsReset 3.2100 us/op 4.3870 us/op 0.73
mainnet_e58758 - phase0 processRandaoMixesReset 4.4300 us/op 7.0460 us/op 0.63
mainnet_e58758 - phase0 processHistoricalRootsUpdate 505.00 ns/op 635.00 ns/op 0.80
mainnet_e58758 - phase0 processParticipationRecordUpdates 3.9300 us/op 4.9890 us/op 0.79
mainnet_e58758 - phase0 afterProcessEpoch 95.540 ms/op 102.70 ms/op 0.93
phase0 processEffectiveBalanceUpdates - 250000 normalcase 1.2785 ms/op 1.2778 ms/op 1.00
phase0 processEffectiveBalanceUpdates - 250000 worstcase 0.5 1.4786 ms/op 2.0404 ms/op 0.72
altair processInactivityUpdates - 250000 normalcase 19.030 ms/op 30.075 ms/op 0.63
altair processInactivityUpdates - 250000 worstcase 25.074 ms/op 30.344 ms/op 0.83
phase0 processRegistryUpdates - 250000 normalcase 6.7340 us/op 8.5930 us/op 0.78
phase0 processRegistryUpdates - 250000 badcase_full_deposits 267.97 us/op 325.65 us/op 0.82
phase0 processRegistryUpdates - 250000 worstcase 0.5 124.00 ms/op 140.11 ms/op 0.89
altair processRewardsAndPenalties - 250000 normalcase 67.319 ms/op 70.662 ms/op 0.95
altair processRewardsAndPenalties - 250000 worstcase 67.828 ms/op 75.408 ms/op 0.90
phase0 getAttestationDeltas - 250000 normalcase 6.4364 ms/op 7.0994 ms/op 0.91
phase0 getAttestationDeltas - 250000 worstcase 6.7168 ms/op 7.0371 ms/op 0.95
phase0 processSlashings - 250000 worstcase 3.2154 ms/op 3.6757 ms/op 0.87
altair processSyncCommitteeUpdates - 250000 172.49 ms/op 188.46 ms/op 0.92
BeaconState.hashTreeRoot - No change 253.00 ns/op 318.00 ns/op 0.80
BeaconState.hashTreeRoot - 1 full validator 53.189 us/op 61.202 us/op 0.87
BeaconState.hashTreeRoot - 32 full validator 542.82 us/op 612.12 us/op 0.89
BeaconState.hashTreeRoot - 512 full validator 5.0354 ms/op 5.8393 ms/op 0.86
BeaconState.hashTreeRoot - 1 validator.effectiveBalance 64.521 us/op 66.511 us/op 0.97
BeaconState.hashTreeRoot - 32 validator.effectiveBalance 877.83 us/op 995.19 us/op 0.88
BeaconState.hashTreeRoot - 512 validator.effectiveBalance 11.446 ms/op 12.452 ms/op 0.92
BeaconState.hashTreeRoot - 1 balances 48.301 us/op 57.585 us/op 0.84
BeaconState.hashTreeRoot - 32 balances 465.43 us/op 452.04 us/op 1.03
BeaconState.hashTreeRoot - 512 balances 4.3695 ms/op 5.0882 ms/op 0.86
BeaconState.hashTreeRoot - 250000 balances 73.704 ms/op 78.720 ms/op 0.94
aggregationBits - 2048 els - zipIndexesInBitList 15.664 us/op 18.026 us/op 0.87
regular array get 100000 times 32.296 us/op 34.312 us/op 0.94
wrappedArray get 100000 times 32.394 us/op 34.204 us/op 0.95
arrayWithProxy get 100000 times 15.622 ms/op 15.660 ms/op 1.00
ssz.Root.equals 531.00 ns/op 635.00 ns/op 0.84
byteArrayEquals 524.00 ns/op 571.00 ns/op 0.92
shuffle list - 16384 els 6.7026 ms/op 7.2523 ms/op 0.92
shuffle list - 250000 els 98.364 ms/op 103.95 ms/op 0.95
processSlot - 1 slots 8.3890 us/op 10.187 us/op 0.82
processSlot - 32 slots 1.3253 ms/op 1.5458 ms/op 0.86
getEffectiveBalanceIncrementsZeroInactive - 250000 vs - 7PWei 199.63 us/op 210.25 us/op 0.95
getCommitteeAssignments - req 1 vs - 250000 vc 2.9182 ms/op 2.9627 ms/op 0.98
getCommitteeAssignments - req 100 vs - 250000 vc 4.1453 ms/op 4.2873 ms/op 0.97
getCommitteeAssignments - req 1000 vs - 250000 vc 4.4969 ms/op 4.6576 ms/op 0.97
RootCache.getBlockRootAtSlot - 250000 vs - 7PWei 4.6500 ns/op 5.2100 ns/op 0.89
state getBlockRootAtSlot - 250000 vs - 7PWei 723.39 ns/op 644.49 ns/op 1.12
computeProposers - vc 250000 10.618 ms/op 10.979 ms/op 0.97
computeEpochShuffling - vc 250000 100.59 ms/op 110.78 ms/op 0.91
getNextSyncCommittee - vc 250000 175.44 ms/op 180.39 ms/op 0.97

by benchmarkbot/action

@g11tech
g11tech enabled auto-merge (squash) February 1, 2023 18:07
@g11tech
g11tech merged commit 3200014 into unstable Feb 2, 2023
@g11tech
g11tech deleted the g11tech/better-update-fix branch February 2, 2023 03:35
@claravanstaden claravanstaden mentioned this pull request Feb 4, 2023
Merged
wemeetagain pushed a commit that referenced this pull request Feb 6, 2023
@g11tech @wemeetagain
Fix the args order in evaluating isBetterUpdate for new partial update (
781fe3d 
#5090)
@wemeetagain wemeetagain mentioned this pull request Feb 6, 2023
Merged
@wemeetagain

wemeetagain commented Mar 21, 2023

Copy link
Copy Markdown
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

@twoeths twoeths twoeths 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

@g11tech @wemeetagain @twoeths