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.

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

Fix gossip block error handler #2842

Merged
merged 1 commit into from
Jul 15, 2021
Merged

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Jul 15, 2021

Motivation

  • Node fails to track latest head due to incorrect error handler of Gossip Block handler when a future block or unknown parent root gossip block comes
  • onBeaconBlockByRoot handler is not correct for finalized block so we may get lower score as found by @mpetrunic and hence we may not receive gossip block frequently

Description

  • Use the new BlockGossipError instead of BlockError
  • Fix onBeaconBlockByRoot for finalized block
  • Add more logs

Other than that, move forkchocie updateHead after we emit/process block in order to process updated attestations in the block

Closes #2840

Steps to test or reproduce

Sync prater

Jul-15 04:43:42.049 []                 info: Synced - finalized: 25561 0xc276…85a0 - head: 818018 0x94c9…d6f7 - clockSlot: 818018 - peers: 26
Jul-15 04:43:54.022 []                 info: Synced - finalized: 25561 0xc276…85a0 - head: 818019 0x0261…af05 - clockSlot: 818019 - peers: 26
Jul-15 04:44:06.002 []                 info: Synced - finalized: 25561 0xc276…85a0 - head: 818020 0xf595…464f - clockSlot: 818020 - peers: 27

@github-actions github-actions bot added scope-networking All issues related to networking, gossip, and libp2p. Validator labels Jul 15, 2021
@@ -71,9 +75,10 @@ export function getGossipHandlers(modules: ValidatorFnsModules): GossipHandlers
}
} catch (e) {
if (
e instanceof BlockError &&
e instanceof BlockGossipError &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the main fix

@codeclimate
Copy link

codeclimate bot commented Jul 15, 2021

Code Climate has analyzed commit 6eab9ec and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@github-actions
Copy link
Contributor

⚠️ Performance Alert ⚠️

Possible performance regression was detected for some benchmarks.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold.

Benchmark suite Current: c48e0fc Previous: c38f1fe Ratio
aggregationBits - 2048 els - readonlyValues 692.96 us/op 223.22 us/op 3.10
aggregationBits - 2048 els - zipIndexesInBitList 252.12 us/op 47.478 us/op 5.31

🚀🚀 Significant benchmark improvement detected

Benchmark suite Current: c48e0fc Previous: c38f1fe Ratio
epoch altair - 250000 vs - 7PWei - processEth1DataReset 1.3340 us/op 2.9770 us/op 0.45
epoch altair - 250000 vs - 7PWei - processEffectiveBalanceUpdates 38.561 ms/op 82.576 ms/op 0.47
epoch altair - 250000 vs - 7PWei - processSyncCommitteeUpdates 1.0220 us/op 2.5240 us/op 0.40
Full benchmark results
Benchmark suite Current: c48e0fc Previous: c38f1fe Ratio
getCommitteeAssignments - req 1000 vs - 250000 vc 8.7821 ms/op 8.2486 ms/op 1.06
epoch altair - 250000 vs - 7PWei - processJustificationAndFinalization 270.19 us/op 185.58 us/op 1.46
epoch altair - 250000 vs - 7PWei - processInactivityUpdates 2.4296 s/op 1.8663 s/op 1.30
epoch altair - 250000 vs - 7PWei - processRewardsAndPenalties 724.27 ms/op 761.06 ms/op 0.95
epoch altair - 250000 vs - 7PWei - processRegistryUpdates 20.317 us/op 17.382 us/op 1.17
epoch altair - 250000 vs - 7PWei - processSlashings 86.562 us/op 91.536 us/op 0.95
epoch altair - 250000 vs - 7PWei - processEth1DataReset 1.3340 us/op 2.9770 us/op 0.45
epoch altair - 250000 vs - 7PWei - processEffectiveBalanceUpdates 38.561 ms/op 82.576 ms/op 0.47
epoch altair - 250000 vs - 7PWei - processSlashingsReset 24.460 us/op 17.557 us/op 1.39
epoch altair - 250000 vs - 7PWei - processRandaoMixesReset 40.677 us/op 38.885 us/op 1.05
epoch altair - 250000 vs - 7PWei - processHistoricalRootsUpdate 3.2680 us/op 1.9830 us/op 1.65
epoch altair - 250000 vs - 7PWei - processParticipationFlagUpdates 319.19 ms/op 238.94 ms/op 1.34
epoch altair - 250000 vs - 7PWei - processSyncCommitteeUpdates 1.0220 us/op 2.5240 us/op 0.40
epoch altair - 250000 vs - 7PWei - prepareEpochProcessState 509.39 ms/op 403.11 ms/op 1.26
Process block - 250000 vs - 7PWei - with 0 validator exit 436.26 us/op 627.45 us/op 0.70
Process block - 250000 vs - 7PWei - with 1 validator exit 23.819 ms/op 22.503 ms/op 1.06
Process block - 250000 vs - 7PWei - with 16 validator exits 30.035 ms/op 23.122 ms/op 1.30
epoch phase0 - 250000 vs - 7PWei - processJustificationAndFinalization 82.905 us/op 85.104 us/op 0.97
epoch phase0 - 250000 vs - 7PWei - processRewardsAndPenalties 449.52 ms/op 484.01 ms/op 0.93
epoch phase0 - 250000 vs - 7PWei - processRegistryUpdates 24.838 us/op 19.072 us/op 1.30
epoch phase0 - 250000 vs - 7PWei - processSlashings 69.519 us/op 52.132 us/op 1.33
epoch phase0 - 250000 vs - 7PWei - processFinalUpdates 40.133 ms/op 39.635 ms/op 1.01
epoch phase0 - 250000 vs - 7PWei - prepareEpochProcessState 764.50 ms/op 636.73 ms/op 1.20
getAttestationDeltas - 250000 vs - 7PWei 86.421 ms/op 74.847 ms/op 1.15
processSlots - 250000 vs - 7PWei - 32 empty slots 6.2684 s/op 5.5372 s/op 1.13
shuffle list - 16384 els 2.7726 ms/op 1.6767 ms/op 1.65
shuffle list - 250000 els 40.195 ms/op 24.053 ms/op 1.67
aggregationBits - 2048 els - readonlyValues 692.96 us/op 223.22 us/op 3.10
aggregationBits - 2048 els - zipIndexesInBitList 252.12 us/op 47.478 us/op 5.31
ssz.Root.equals 2.2160 us/op 1.1830 us/op 1.87
getPubkeys - persistent - req 1000 vs - 250000 vc 19.137 us/op 17.935 us/op 1.07
BLS verify - blst-native 2.1030 ms/op 1.8587 ms/op 1.13
BLS verifyMultipleSignatures 3 - blst-native 4.0768 ms/op 3.8096 ms/op 1.07
BLS verifyMultipleSignatures 8 - blst-native 9.2485 ms/op 8.2058 ms/op 1.13
BLS verifyMultipleSignatures 32 - blst-native 33.353 ms/op 32.336 ms/op 1.03
BLS aggregatePubkeys 32 - blst-native 47.772 us/op 40.334 us/op 1.18
BLS aggregatePubkeys 128 - blst-native 182.29 us/op 156.05 us/op 1.17
validate gossip signedAggregateAndProof - struct 7.3269 ms/op 4.5256 ms/op 1.62
validate gossip signedAggregateAndProof - treeBacked 5.2433 ms/op 4.4658 ms/op 1.17
validate gossip attestation - struct 2.2716 ms/op 2.1001 ms/op 1.08
validate gossip attestation - treeBacked 2.4801 ms/op 2.1237 ms/op 1.17

by benchmarkbot/action

@twoeths
Copy link
Contributor Author

twoeths commented Jul 15, 2021

@twoeths twoeths marked this pull request as ready for review July 15, 2021 04:41
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.

Nice pick!

@dapplion dapplion merged commit 4344839 into master Jul 15, 2021
@dapplion dapplion deleted the tuyen/fix-gossip-error-handler branch July 15, 2021 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope-networking All issues related to networking, gossip, and libp2p.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node fails to track correct head
2 participants