Eoe scheduled roothash sync #3456
Conversation
…start notarized shard header
…n epoch and some refactor
…h-sync # Conflicts: # go.mod # go.sum
…r integration tests steps
epochStart/bootstrap/process.go
Outdated
| if !ok { | ||
| return epochStart.ErrWrongTypeAssertion | ||
| } | ||
|
|
||
| log.Debug("start in epoch bootstrap: started syncUserAccountsState") | ||
| err = e.syncUserAccountsState(ownShardHdr.GetRootHash()) | ||
| ownShardHdr, rootHashToSync, withScheduled, additionalHeaders, err := e.getDataToSync( |
There was a problem hiding this comment.
Maybe an private DTO here -> type dataToSync struct {} with all these members: ownShardHdr, rootHashToSync, withScheduled, additionalHeaders
epochStart/bootstrap/process.go
Outdated
| return nil, nil, nil, err | ||
| } | ||
|
|
||
| ownShardHdr, headers, err := e.dataSyncerWithScheduled.updateSyncDataIfNeeded( |
There was a problem hiding this comment.
Could be on single line
| } | ||
|
|
||
| mbs = append(mbs, processedMiniBlockHeaders[i]) | ||
| crossMbsProcessed[mbHeader.GetSenderShardID()] = mbs |
There was a problem hiding this comment.
This could be moved after L195, not needed to be called each time
There was a problem hiding this comment.
as discussed, append can change the mbs pointer
| header data.HeaderHandler, | ||
| withScheduled bool, | ||
| ) ([]bootstrapStorage.MiniBlocksInMeta, []bootstrapStorage.PendingMiniBlocksInfo, error) { | ||
| log.Debug("TESTTEST: getProcessedAndPendingMiniBlocksWithScheduled", "withScheduled", withScheduled) |
There was a problem hiding this comment.
Remove it or refactor the message
| processedMiniBlocks = removeMbsFromProcessed(processedMiniBlocks, mapMbHeaderHandlers) | ||
| pendingMiniBlocks = addMbsToPending(pendingMiniBlocks, mapMbHeaderHandlers) | ||
|
|
||
| return processedMiniBlocks, pendingMiniBlocks, err |
There was a problem hiding this comment.
nil instead err to be more clear that in this point there is no error
process/sync/baseSync.go
Outdated
| @@ -1057,6 +1068,13 @@ func (boot *baseBootstrap) requestHeaders(fromNonce uint64, toNonce uint64) { | |||
| } | |||
| } | |||
|
|
|||
| // SetNodeSyncState sets the synced state of the node | |||
| func (boot *baseBootstrap) SetNodeSyncState(synced bool) { | |||
There was a problem hiding this comment.
Is this method used?
| rootHash = scheduledRootHash | ||
| } else { | ||
| log.Debug("storageBootstrapper.applyHeaderInfo scheduledTxsExecutionHandler.GetScheduledRootHashForHeader", "error", err) |
There was a problem hiding this comment.
I think this is not a real error, if one header does not have sheduled txs inside. We can remove L262 and L265 and add in L267 a log with both, rootHash an scheduledRootHash like this one:
log.Debug("storageBootstrapper.applyHeaderInfo", "rootHash", rootHash, "scheduledRootHash", scheduledRootHash)
storage/factory/openStorage.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| cache, err := lrucache.NewCache(10) |
There was a problem hiding this comment.
Maybe we can use a constant with a properly name instead 10
update/sync/syncTransactions.go
Outdated
| @@ -58,7 +58,7 @@ func NewPendingTransactionsSyncer(args ArgsNewPendingTransactionsSyncer) (*pendi | |||
| return nil, process.ErrNilRequestHandler | |||
| } | |||
|
|
|||
| p := &pendingTransactions{ | |||
| p := &transactionsSync{ | |||
There was a problem hiding this comment.
t or ts instead p here and for all pointer receivers in this file
update/sync/syncTransactions.go
Outdated
| @@ -165,7 +165,7 @@ func (p *pendingTransactions) requestTransactionsFor(miniBlock *block.MiniBlock) | |||
| } | |||
|
|
|||
| // receivedMiniBlock is a callback function when a new transactions was received | |||
There was a problem hiding this comment.
remove or refactor this comment
epochStart/bootstrap/process.go
Outdated
| if !ok { | ||
| return epochStart.ErrWrongTypeAssertion | ||
| } | ||
|
|
||
| log.Debug("start in epoch bootstrap: started syncUserAccountsState") | ||
| err = e.syncUserAccountsState(ownShardHdr.GetRootHash()) | ||
| ownShardHdr, rootHashToSync, withScheduled, additionalHeaders, err := e.getDataToSync( |
| header data.HeaderHandler, | ||
| withScheduled bool, | ||
| ) ([]bootstrapStorage.MiniBlocksInMeta, []bootstrapStorage.PendingMiniBlocksInfo, error) { | ||
| log.Debug("TESTTEST: getProcessedAndPendingMiniBlocksWithScheduled", "withScheduled", withScheduled) |
| for i := range list { | ||
| for j, mbHash := range list[i].MiniBlocksHashes { | ||
| if bytes.Equal(mbHash, mbHandler.GetHash()) { | ||
| list[i].MiniBlocksHashes = append(list[i].MiniBlocksHashes[j:], list[i].MiniBlocksHashes[:j+1]...) |
| @@ -70,7 +70,7 @@ import ( | |||
| ) | |||
|
|
|||
| // StepDelay is used so that transactions can disseminate properly | |||
| var StepDelay = time.Second / 10 | |||
| var StepDelay = time.Millisecond * 180 | |||
There was a problem hiding this comment.
for @raduchis check teamcity
process/block/shardblock.go
Outdated
| @@ -1123,6 +1129,11 @@ func (sp *shardProcessor) snapShotEpochStartFromMeta(header data.ShardHeaderHand | |||
| } | |||
|
|
|||
| rootHash := epochStartShData.RootHash | |||
| schRootHash := epochStartShData.GetScheduledRootHash() | |||
| if schRootHash!= nil{ | |||
epochStart/bootstrap/process.go
Outdated
| } | ||
|
|
||
| ownShardHdr, rootHashToSync, headers, err := e.updateDataForScheduled(shardNotarizedHeader) | ||
| res.ownShardHdr, res.rootHashToSync, res.additionalHeaders, err = e.updateDataForScheduled(shardNotarizedHeader) |
There was a problem hiding this comment.
Suggestion: res, err := e.updateDataForScheduled(shardNotarizedHeader), move L899-L895 inside updateDataForScheduled, and return nil, err in L903 and 908
This PR enables the use of the scheduled roothash on epoch start instead of the intermediate roothash.
There are several changes done in order to make this possible such as synchronizing extra blocks/miniblocks, changing what roothash is used for the snapshot so that new nodes or shuffled nodes can synchronize from the epoch start block and have the same security guarantees as before introducing the scheduled smart contract calls.
The unit tests will come in a follow up PR targeted to feat/scheduled-sc-execution