sync txs and miniblocks at bootstrap #7582
Conversation
process/sync/baseSync.go
Outdated
| // sleepTime defines the time in milliseconds between each iteration made in syncBlocks method | ||
| const sleepTime = 50 * time.Millisecond | ||
| const minimumProcessWaitTime = time.Millisecond * 100 | ||
| const DefaultTimeToWaitForRequestedData = 5 * time.Minute |
There was a problem hiding this comment.
not needed as exported
|
|
||
| // sync all txs into pools | ||
|
|
||
| err = boot.txSyncer.SyncTransactionsFor(miniBlocks, header.GetEpoch(), ctx) |
There was a problem hiding this comment.
here ctx is reused but already canceled on L1062
process/sync/baseSync.go
Outdated
| return err | ||
| } | ||
|
|
||
| _, err = boot.txSyncer.GetTransactions() |
There was a problem hiding this comment.
is this call needed?
There was a problem hiding this comment.
Pull request overview
This PR adds synchronization of transactions and miniblocks during the bootstrap process to ensure all necessary data is available before block processing begins.
- Creates transaction and miniblock syncers during bootstrap initialization
- Adds synchronization logic to fetch miniblocks and transactions for headers during the bootstrap preparation phase
- Updates test expectations to account for additional RegisterHandler calls from the new miniblock syncer
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| process/sync/baseSync.go | Adds new syncers (miniBlocksSyncer and txSyncer), implements syncMiniBlocksAndTxsForHeader method to sync miniblocks and transactions, modifies prepareForSyncIfNeeded to optionally sync transactions |
| process/sync/shardblock.go | Calls createTxSyncer during ShardBootstrap initialization |
| process/sync/metablock.go | Calls createTxSyncer during MetaBootstrap initialization |
| process/sync/shardblock_test.go | Adds UnsignedTransactionUnit storer to test setup, updates expected RegisterHandler call count from 2 to 3, improves error checking |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // sync all txs into pools | ||
|
|
||
| err = boot.txSyncer.SyncTransactionsFor(miniBlocks, header.GetEpoch(), ctx) |
There was a problem hiding this comment.
The context is being used after it has been cancelled. The context is created and cancelled before calling SyncTransactionsFor on line 1074. This will cause the transaction sync operation to fail immediately since it receives an already-cancelled context. A new context should be created for the SyncTransactionsFor call, or the cancel should be deferred until after both sync operations complete.
process/sync/baseSync.go
Outdated
| } | ||
|
|
||
| if withTxs { | ||
| err = boot.syncMiniBlocksAndTxsForHeader(currentHeader) |
There was a problem hiding this comment.
Incorrect header passed to syncMiniBlocksAndTxsForHeader. The function is being called with currentHeader instead of hdr, which is the header being processed in the loop. This will cause the wrong header's miniblocks and transactions to be synced.
| err = boot.syncMiniBlocksAndTxsForHeader(currentHeader) | |
| err = boot.syncMiniBlocksAndTxsForHeader(hdr) |
AdoAdoAdo
left a comment
AdoAdoAdo
left a comment
There was a problem hiding this comment.
no other comments besides the existing ones.
sstanculeanu
dismissed stale reviews from AdoAdoAdo and themself
The base branch was changed.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/supernova-async-exec #7582 +/- ##
=============================================================
- Coverage 77.73% 77.66% -0.07%
=============================================================
Files 875 874 -1
Lines 120400 120635 +235
=============================================================
+ Hits 93591 93694 +103
- Misses 20662 20768 +106
- Partials 6147 6173 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sstanculeanu
merged commit 3652641
into
feat/supernova-async-exec
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?