-
Notifications
You must be signed in to change notification settings - Fork 129
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: use last finalized block on startup #3737
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## development #3737 +/- ##
===============================================
- Coverage 50.51% 50.23% -0.29%
===============================================
Files 230 230
Lines 29006 29003 -3
===============================================
- Hits 14653 14570 -83
- Misses 12856 12935 +79
- Partials 1497 1498 +1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using this flag property (isStartup
) I would suggest to: inside the bootstrapSync
method inserting the call to GetHighestFinalisedHeader
before starting the for loop, which means that the process will start using the latest finalized header.
Also, I've suggested to transform currentSyncInformation
method into isBootstrapSync
where it doesn't need to return the block but only a bool indicating if we keep on bootstrap or change
func (cs *chainSync) bootstrapSync() {
currentBlock, err := cs.blockState.GetHighestFinalisedHeader()
if err != nil {
panic("cannot find highest finalised header")
}
for {
...
isBootstrap := cs.isBootstrapSync(currentBlock)
if isBootstrap {
cs.workerPool.useConnectedPeers()
err = cs.requestMaxBlocksFrom(currentBlock, networkInitialSync)
if err != nil {
logger.Errorf("requesting max blocks from best block header: %s", err)
}
currentBlock, err = cs.blockState.BestBlockHeader()
if err != nil {
return nil, false, fmt.Errorf("getting best block header: %w", err)
}
} else {
...
}
}
}
func (cs *chainSync) isBootstrapSync(syncHeader *types.Header) bool {
syncTarget := cs.peerViewSet.getTarget()
return syncHeader.Number+network.MaxBlocksInResponse < syncTarget
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
53a8579
to
bb167b6
Compare
bb167b6
to
2d1561e
Compare
# [0.9.0](v0.8.0...v0.9.0) (2024-3-1) ### Bug Fixes * add a limit of number of bytes while scale decoding a slice ([#3733](#3733)) ([5edbf89](5edbf89)) * **docs:** Fixing link to polkadot runtime fundamentals to the right one ([#3763](#3763)) ([a785d32](a785d32)) * don't panic if we fail to convert hex to bytes ([#3734](#3734)) ([12234de](12234de)) * **dot/sync:** execute p2p handshake when there is no target ([#3695](#3695)) ([a9db0ec](a9db0ec)) * fix index out of range undeterministic error in rpc test ([#3718](#3718)) ([d099384](d099384)) * fix non deterministic panic during TestStableNetworkRPC integration test ([#3756](#3756)) ([ee3d243](ee3d243)) * **lib/trie:** use `MustBeHashed` for V1 trie nodes with larger storage values ([#3739](#3739)) ([f5e48a9](f5e48a9)) * **mocks:** Set fixed version for uber mockgen in CI ([#3656](#3656)) ([ea9877e](ea9877e)) * **runtime/storage:** support nested storage transactions ([#3670](#3670)) ([3e99f6d](3e99f6d)) * segfault on node restart ([#3736](#3736)) ([d1ca7aa](d1ca7aa)) * **state-version:** should be uint8 instead of uint32 ([#3779](#3779)) ([c8fdb14](c8fdb14)) * update paseo chain spec ([#3770](#3770)) ([6a54f28](6a54f28)) * use last finalized block on startup ([#3737](#3737)) ([c262642](c262642)) ### Features * **config:** dynamically set version based on environment ([#3693](#3693)) ([5c534c9](5c534c9)) * **staging:** Expose RPC on Westend Staging Node ([#3687](#3687)) ([c374eaa](c374eaa)) * **tests/scripts:** create script to retrieve trie state via rpc ([#3714](#3714)) ([5ccea40](5ccea40))
🎉 This PR is included in version 0.9.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [0.9.0](v0.8.0...v0.9.0) (2024-3-1) ### Bug Fixes * add a limit of number of bytes while scale decoding a slice ([#3733](#3733)) ([5edbf89](5edbf89)) * **docs:** Fixing link to polkadot runtime fundamentals to the right one ([#3763](#3763)) ([a785d32](a785d32)) * don't panic if we fail to convert hex to bytes ([#3734](#3734)) ([12234de](12234de)) * **dot/sync:** execute p2p handshake when there is no target ([#3695](#3695)) ([a9db0ec](a9db0ec)) * fix index out of range undeterministic error in rpc test ([#3718](#3718)) ([d099384](d099384)) * fix non deterministic panic during TestStableNetworkRPC integration test ([#3756](#3756)) ([ee3d243](ee3d243)) * **lib/trie:** use `MustBeHashed` for V1 trie nodes with larger storage values ([#3739](#3739)) ([f5e48a9](f5e48a9)) * **mocks:** Set fixed version for uber mockgen in CI ([#3656](#3656)) ([ea9877e](ea9877e)) * **runtime/storage:** support nested storage transactions ([#3670](#3670)) ([3e99f6d](3e99f6d)) * segfault on node restart ([#3736](#3736)) ([d1ca7aa](d1ca7aa)) * **state-version:** should be uint8 instead of uint32 ([#3779](#3779)) ([c8fdb14](c8fdb14)) * update paseo chain spec ([#3770](#3770)) ([6a54f28](6a54f28)) * use last finalized block on startup ([#3737](#3737)) ([c262642](c262642)) ### Features * **config:** dynamically set version based on environment ([#3693](#3693)) ([5c534c9](5c534c9)) * **staging:** Expose RPC on Westend Staging Node ([#3687](#3687)) ([c374eaa](c374eaa)) * **tests/scripts:** create script to retrieve trie state via rpc ([#3714](#3714)) ([5ccea40](5ccea40))
Changes
Tests
go test -tags integration github.com/ChainSafe/gossamer
Issues
Primary Reviewer
@EclesioMeloJunior