Etcm 1030#1069
Conversation
|
|
||
| //not used atm, a part of the future ExecutionSync | ||
| class FetcherService(validator: BlockValidator, blockchainReader: BlockchainReader, syncConfig: SyncConfig) { | ||
| val batchSize = syncConfig.blockHeadersPerRequest |
There was a problem hiding this comment.
I think that this is an unfortunate pattern. the class does not depend on the whole sysconfig class - it depends on one field from that class. how can we better express such dependencies?
if it's one field perhaps a tagged number would be good enough?
There was a problem hiding this comment.
I'm not sure I know what's a tagged number🤔
| val endNumber: Option[BigInt] = | ||
| fetchUntilBlock.fold(Some(_), blockchainReader.getBlockHeaderByHash(_).map(_.number)) | ||
| val startNumber: Option[BigInt] = | ||
| startFromBlock.fold(Some(_), blockchainReader.getBlockHeaderByHash(_).map(_.number)) |
There was a problem hiding this comment.
correct me if I'm wrong but wouldn't it be beneficial to request a block by hash from a peer?
There was a problem hiding this comment.
actually thinking more on that - we wouldn't know the hash of the block we want to fetch from the peer if we don't have that block ourselves. And the point of fetching from multiple peers is exactly to get alternative chains, so we do need to fetch by number here
dzajkowski
left a comment
dzajkowski
left a comment
There was a problem hiding this comment.
I think that this one merits some discussion. maybe we can address it on the synch meeting?
AnastasiiaL
force-pushed
the
ETCM-1030
branch
3 times, most recently
from
f8dcd4e to
a51b6c8
Compare
6 participants
Description
ExecutionSync is gonna be orchestrating the whole regular sync process and is gonna replace current RegularSync class.
Note: tests will be added in a separate PR