feature(dot/sync): implement *tipSyncer.hasCurrentWorker, add minPeers, slotDuration to chainSync, check if syncing in *core.Service.HandleTransactionMessage
#1881
Conversation
noot
requested review from
arijitAD,
EclesioMeloJunior,
edwardmack,
jimjbrettj,
kishansagathiya,
omar391 and
timwu20
as code owners
Codecov Report
@@ Coverage Diff @@
## development #1881 +/- ##
===============================================
+ Coverage 59.42% 59.76% +0.33%
===============================================
Files 192 187 -5
Lines 19521 19185 -336
===============================================
- Hits 11601 11465 -136
+ Misses 6009 5814 -195
+ Partials 1911 1906 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
dot/network/service.go
Outdated
| @@ -738,3 +738,8 @@ func (*Service) StartingBlock() int64 { | |||
| // TODO: refactor this to get the data from the sync service | |||
| return 0 | |||
| } | |||
|
|
|||
| // IsSynced returns whether we are synced (not longer in bootstrap mode) or not | |||
There was a problem hiding this comment.
// IsSynced returns whether we are synced (no longer in bootstrap mode) or not
dot/sync/chain_sync_test.go
Outdated
| const defaultMinPeers = 1 | ||
|
|
||
| var ( | ||
| testTimeout = time.Second * 5 | ||
| defaultSlotDuration = time.Second * 6 | ||
| ) |
There was a problem hiding this comment.
const (
defaultMinPeers = 1
testTimeout = time.Second * 5
defaultSlotDuration = time.Second * 6
)
dot/sync/tip_syncer.go
Outdated
| continue | ||
| } | ||
|
|
||
| // worker start is less than than existing worker's start |
There was a problem hiding this comment.
// worker start is less than existing worker's start
dot/sync/tip_syncer.go
Outdated
| continue | ||
| } | ||
|
|
||
| // worker start is greater than than existing worker's start |
There was a problem hiding this comment.
// worker start is less than existing worker's start
| logger.Debug("ignoring TransactionMessage, not yet synced") | ||
| return false, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
question: if we're not synced yet, this msg could not be added to a retry queue? is ok to ignore TransactionMessage?
There was a problem hiding this comment.
we potentially won't be able to validate it for 3 days or more, in which case the transaction is almost certain to have been included (or discarded) already. also the memory usage from storing all of them might be a concern. I think it's ok to just ignore them for now, maybe in the future we can consider some queue
|
🎉 This PR is included in version 0.6.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
…ers, slotDuration to `chainSync`, check if syncing in `*core.Service.HandleTransactionMessage` (ChainSafe#1881)
Changes
*tipSyncer.hasCurrentWorkerminPeers,slotDurationtochainSyncand use these where needed*core.Service.HandleTransactionMessageto not validate transaction messages while in bootstrap sync mode, as the runtime will not be the same as it is near the head and will cause lots of failure messages.Tests
Issues
chainSync#1860Primary Reviewer