Skip to content
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

Component for scheduled SCRs #3038

Merged

Conversation

SebastianMarian
Copy link
Contributor

@SebastianMarian SebastianMarian commented Apr 23, 2021

  • Implemented functionality of saving/loading/setting/getting for scheduled scrs generated after execution of scheduled txs. These scrs should be included in the next block after the one in which the corresponding scheduled txs were included and afterwards were executed.

@SebastianMarian SebastianMarian added the type:feature New feature or request label Apr 23, 2021
@SebastianMarian SebastianMarian self-assigned this Apr 23, 2021
@SebastianMarian SebastianMarian changed the title [WIP] Component for scheduled sc rs [WIP] Component for scheduled SCRs Apr 23, 2021
@SebastianMarian SebastianMarian changed the title [WIP] Component for scheduled SCRs Component for scheduled SCRs Apr 28, 2021
@SebastianMarian SebastianMarian marked this pull request as ready for review April 28, 2021 10:10
@sasurobert sasurobert self-requested a review April 28, 2021 10:39
factory/blockProcessorCreator.go Outdated Show resolved Hide resolved
genesis/process/disabled/scheduledTxsExecutionHandler.go Outdated Show resolved Hide resolved
process/block/baseProcess.go Outdated Show resolved Hide resolved
process/block/preprocess/scheduledTxsExecution.go Outdated Show resolved Hide resolved
process/common.go Outdated Show resolved Hide resolved
process/coordinator/process_test.go Show resolved Hide resolved
process/sync/baseSync.go Outdated Show resolved Hide resolved
process/sync/baseSync.go Outdated Show resolved Hide resolved
)

// ScheduledTxsExecutionStub -
type ScheduledTxsExecutionStub struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add instead one mock and/or one stub for the scheduledTxsExecution in testscommon ?
then we don't need to redefine in every package

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

FeeHandler: &mock.FeeAccumulatorStub{},
BlockSizeComputation: &mock.BlockSizeComputationStub{},
BalanceComputation: &mock.BalanceComputationStub{},
EconomicsFee: &mock.FeeHandlerStub{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reformat file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the way it is automatically formatted. I'm not sure if something is wrong with my settings or not...

AdoAdoAdo
AdoAdoAdo previously approved these changes May 6, 2021
sasurobert
sasurobert previously approved these changes May 6, 2021
@@ -695,6 +699,18 @@ func (boot *baseBootstrap) rollBack(revertUsingForkNonce bool) error {
)
}

mapScheduledSCRs, err := process.GetScheduledSCRsFromStorage(prevHeaderHash, boot.store, boot.marshalizer)
if err != nil {
log.Debug("get scheduled scrs from storage",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't be this critical ?
maybe make a new function for lines 702 - 713 as the base function is already too big

@SebastianMarian SebastianMarian dismissed stale reviews from sasurobert and AdoAdoAdo via c7c9d32 May 6, 2021 10:14
@SebastianMarian SebastianMarian merged commit 0c12921 into feat/scheduled-sc-execution May 6, 2021
@SebastianMarian SebastianMarian deleted the component-for-scheduled-SCRs branch May 6, 2021 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants