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

Fix edge case of set scheduled SCRs at bootstrap #3742

Merged

Conversation

SebastianMarian
Copy link
Contributor

  • Fixed some misleading logs
  • Fixed an edge case situation which wrongly set scheduled SCRs for the current block when a node starts in epoch

* Fixed an edge case situation which wrongly set scheduled SCRs for the current block when a node starts in epoch
@SebastianMarian SebastianMarian added the type:feature New feature or request label Feb 1, 2022
@SebastianMarian SebastianMarian self-assigned this Feb 1, 2022
@codecov
Copy link

codecov bot commented Feb 1, 2022

Codecov Report

Merging #3742 (a8f7b14) into development (43d7554) will increase coverage by 0.02%.
The diff coverage is 88.88%.

❗ Current head a8f7b14 differs from pull request most recent head c00f579. Consider uploading reports for the commit c00f579 to get more accurate results

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #3742      +/-   ##
===============================================
+ Coverage        74.21%   74.23%   +0.02%     
===============================================
  Files              599      599              
  Lines            79186    79245      +59     
===============================================
+ Hits             58766    58826      +60     
+ Misses           15855    15854       -1     
  Partials          4565     4565              
Impacted Files Coverage Δ
process/block/metablock.go 55.32% <0.00%> (ø)
process/block/preprocess/gasComputation.go 94.32% <0.00%> (-1.68%) ⬇️
process/block/preprocess/transactions.go 57.77% <84.61%> (+1.13%) ⬆️
process/coordinator/process.go 74.25% <87.50%> (+0.11%) ⬆️
epochStart/bootstrap/startInEpochScheduled.go 75.12% <100.00%> (+1.57%) ⬆️
process/block/baseProcess.go 68.13% <100.00%> (+0.13%) ⬆️
process/block/postprocess/feeHandler.go 85.96% <100.00%> (ø)
process/block/preprocess/scheduledTxsExecution.go 96.12% <100.00%> (+0.21%) ⬆️
process/block/preprocess/smartContractResults.go 74.09% <100.00%> (+0.29%) ⬆️
process/block/shardblock.go 64.26% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 700446a...c00f579. Read the comment docs.

@sasurobert sasurobert self-requested a review February 2, 2022 07:56
AdoAdoAdo
AdoAdoAdo previously approved these changes Feb 2, 2022
@@ -488,6 +488,15 @@ func (ste *scheduledTxsExecution) IsScheduledTx(txHash []byte) bool {
return ok
}

func getNumScheduledSCRs(mapScheduledSCRs map[block.Type][]data.TransactionHandler) int {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

for _, txHash := range mb.TxHashes {
scheduledTxs[string(txHash)] = struct{}{}
log.Debug("startInEpochWithScheduledDataSyncer.getScheduledTransactionHashesWithDestMe", "hash", txHash)
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 a little too much log.debug - please delete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is only for scheduled txs, if they exist, from one single block at bootstrap -> start in epoch. So, it is printed only once, but it helps for debugging if needed!

Copy link
Contributor

Choose a reason for hiding this comment

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

ah. I missed that it is only start in epoch. alright.

sasurobert
sasurobert previously approved these changes Feb 2, 2022
AdoAdoAdo
AdoAdoAdo previously approved these changes Feb 2, 2022
AdoAdoAdo and others added 3 commits February 2, 2022 15:38
…th-scheduled

process, integrationTests, testscommon: fix total gas provided with scheduled SC calls
@AdoAdoAdo AdoAdoAdo dismissed stale reviews from sasurobert and themself via 21f46c8 February 2, 2022 14:12
AdoAdoAdo
AdoAdoAdo previously approved these changes Feb 2, 2022
iulianpascalau
iulianpascalau previously approved these changes Feb 2, 2022
@@ -61,7 +61,7 @@ func (f *feeHandler) GetAccumulatedFees() *big.Int {
// GetDeveloperFees returns the total accumulated developer fees
func (f *feeHandler) GetDeveloperFees() *big.Int {
f.mut.RLock()
developerFees := f.developerFees
developerFees := big.NewInt(0).Set(f.developerFees)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

sasurobert
sasurobert previously approved these changes Feb 2, 2022
@@ -61,7 +61,7 @@ func (f *feeHandler) GetAccumulatedFees() *big.Int {
// GetDeveloperFees returns the total accumulated developer fees
func (f *feeHandler) GetDeveloperFees() *big.Int {
f.mut.RLock()
developerFees := f.developerFees
developerFees := big.NewInt(0).Set(f.developerFees)
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

AdoAdoAdo
AdoAdoAdo previously approved these changes Feb 3, 2022
iulianpascalau
iulianpascalau previously approved these changes Feb 4, 2022
gabi-vuls
gabi-vuls previously approved these changes Feb 4, 2022
Copy link
Collaborator

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

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

System test passed .

@AdoAdoAdo AdoAdoAdo dismissed stale reviews from gabi-vuls, iulianpascalau, and themself via c00f579 February 4, 2022 10:03
@AdoAdoAdo AdoAdoAdo merged commit 647c24a into development Feb 4, 2022
@AdoAdoAdo AdoAdoAdo deleted the fix-edge-case-of-set-scheduled-scrs-at-bootstrap branch February 4, 2022 10:33
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

5 participants