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

Refactor and add unit tests for scheduled sc execution #2956

Conversation

SebastianMarian
Copy link
Contributor

@SebastianMarian SebastianMarian commented Apr 2, 2021

  • Added unit tests for scheduled txs execution component
  • Refactor transactionsV2

@SebastianMarian SebastianMarian self-assigned this Apr 2, 2021
@SebastianMarian SebastianMarian changed the title [WIP] Refactor and add unit tests for scheduled sc execution Refactor and add unit tests for scheduled sc execution Apr 6, 2021
@SebastianMarian SebastianMarian marked this pull request as ready for review April 6, 2021 20:22
@sasurobert sasurobert self-requested a review April 7, 2021 06:29
maxCrossShardScCallsOrSpecialTxsPerShard := 0

processingInfo := processedTxsInfo{}
cpmbi := txs.initCreateAndProcessMiniBlocks()
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe some better name. it is complicated - cpmbi

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

}

senderAddressToSkip := []byte("")
csmbi := txs.initCreateScheduledMiniBlocks()
Copy link
Contributor

Choose a reason for hiding this comment

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

there is little diference between csmbi and cpmbi from line 204.

txs.blockSizeComputation.AddNumMiniBlocks(1)
}
csmbi.mapCrossShardScCallTxs[receiverShardID]++
if csmbi.mapCrossShardScCallTxs[receiverShardID] > csmbi.maxCrossShardScCallTxsPerShard {
Copy link
Contributor

Choose a reason for hiding this comment

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

csmbi.mapCrossShardScCallTxs[receiverShardID] take out into a variable

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

cpmbi.firstCrossShardScCallOrSpecialTxFound = true
txs.blockSizeComputation.AddNumMiniBlocks(1)
}
cpmbi.mapCrossShardScCallsOrSpecialTxs[receiverShardID]++
Copy link
Contributor

Choose a reason for hiding this comment

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

cpmbi.mapCrossShardScCallsOrSpecialTxs[receiverShardID] take out into a separate variable

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

}

isMiniBlockEmpty := len(miniBlock.TxHashes) == 0
firstMiniBlockSplitForReceiverShardFound := (txMbInfo.isReceiverSmartContractAddress && cpmbi.mapScsForShard[receiverShardID] == 0 && cpmbi.mapTxsForShard[receiverShardID] > 0) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

this is an incredibly complicated condition. can we make it easier ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

refactored


senderAddressToSkip = []byte("")

gasRefunded := txs.gasHandler.GasRefunded(txHash)
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 gasRefunded approach should be modified exactly like in PR: #3016

@AdoAdoAdo AdoAdoAdo self-requested a review April 19, 2021 08:21
SebastianMarian and others added 3 commits April 19, 2021 13:34
@AdoAdoAdo AdoAdoAdo merged commit 94775f7 into feat/scheduled-sc-execution Apr 19, 2021
@AdoAdoAdo AdoAdoAdo deleted the refactor-and-unit-tests-for-scheduled-sc-execution branch April 19, 2021 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants