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.

Sign up for GitHub

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

Jump to bottom

Fix edge case situation for remaining txs #3682

Merged
merged 5 commits into from
Jan 14, 2022
Merged

Fix edge case situation for remaining txs #3682

merged 5 commits into from
Jan 14, 2022

Conversation

SebastianMarian
Copy link
Contributor

@SebastianMarian SebastianMarian commented Jan 10, 2022 •
edited

  • Fixed edge case situation for remaining txs added for scheduled execution

@SebastianMarian
* Fixed edge case situation for remainingTxs
4e60f29 
* Changed epoch activation flags
@SebastianMarian SebastianMarian self-assigned this Jan 10, 2022
@SebastianMarian SebastianMarian changed the base branch from master to development January 10, 2022 23:26
@codecov
Copy link

codecov bot commented Jan 10, 2022 •
edited

Codecov Report

Merging #3682 (4e60f29) into feat/scheduled-sc-execution (0b9ef1f) will increase coverage by 1.47%.
The diff coverage is 70.28%.

❗ Current head 4e60f29 differs from pull request most recent head fcf9589. Consider uploading reports for the commit fcf9589 to get more accurate results
Impacted file tree graph

@@                       Coverage Diff                       @@
##           feat/scheduled-sc-execution    #3682      +/-   ##
===============================================================
+ Coverage                        72.05%   73.53%   +1.47%     
===============================================================
  Files                              644      597      -47     
  Lines                            63897    78345   +14448     
===============================================================
+ Hits                             46044    57612   +11568     
- Misses                           13533    16158    +2625     
- Partials                          4320     4575     +255
Impacted Files Coverage Δ
api/gin/webServer.go 0.00% <0.00%> (ø)
api/gin/writers.go 0.00% <0.00%> (ø)
api/logs/logSender.go 83.72% <0.00%> (+1.90%) ⬆️
cmd/assessment/benchmarks/arwenBenchmark.go 0.00% <0.00%> (-77.78%) ⬇️
cmd/assessment/benchmarks/delegationBenchmark.go 0.00% <0.00%> (-91.67%) ⬇️
cmd/assessment/benchmarks/erc20Benchmark.go 0.00% <0.00%> (-80.96%) ⬇️
cmd/assessment/benchmarks/results.go 95.45% <ø> (+0.71%) ⬆️
cmd/assessment/hostParameters/hostInfo.go 80.95% <ø> (+2.57%) ⬆️
.../assessment/hostParameters/hostParametersGetter.go 72.41% <ø> (+4.41%) ⬆️
cmd/termui/presenter/blockInfoGetters.go 90.90% <ø> (ø)
... and 774 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 a3be0b0...fcf9589. Read the comment docs.

@SebastianMarian SebastianMarian changed the base branch from development to feat/scheduled-sc-execution January 11, 2022 13:21
@SebastianMarian SebastianMarian changed the title [WIP] Feat scheduled sc execution with new config Fix edge case situation for remaining txs Jan 11, 2022
@SebastianMarian SebastianMarian marked this pull request as ready for review January 11, 2022 19:45
@raduchis raduchis self-requested a review January 12, 2022 13:29
raduchis
raduchis reviewed Jan 12, 2022
View reviewed changes
process/block/preprocess/miniBlockBuilder.go Outdated
@@ -147,51 +147,52 @@ func (mbb *miniBlocksBuilder) updateAccountShardsInfo(tx *transaction.Transactio

// function returns through the first parameter if the given transaction can be added to the miniBlock
// second return values returns an error in case no more transactions can be added to the miniBlocks
func (mbb *miniBlocksBuilder) checkAddTransaction(wtx *txcache.WrappedTransaction) (canAddTx bool, canAddMore bool, tx *transaction.Transaction) {
func (mbb *miniBlocksBuilder) checkAddTransaction(wtx *txcache.WrappedTransaction) (canAddTx bool, canAddMore bool, shouldAddToRemaining bool, tx *transaction.Transaction) {
Copy link
Contributor

Choose a reason for hiding this comment

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

description could be updated. Also a struct could be returned instead of 4 params.

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

process/block/preprocess/miniBlockBuilder.go Outdated
}

receiverShardID := wtx.ReceiverShardID
miniBlock, ok := mbb.miniBlocks[receiverShardID]
if !ok {
log.Debug("mini block is not created", "shard", receiverShardID)
return false, true, tx
return false, true, false, tx
Copy link
Contributor

Choose a reason for hiding this comment

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

could this not be processed in the scheduledTxs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this check is also done in scheduled, so it will not pass also there. Anyway this error could not appear as in the init method all the miniblocks combination are created: 0->0, 0->1, 0->2 and 0->meta

@AdoAdoAdo AdoAdoAdo merged commit 01cc2c0 into feat/scheduled-sc-execution Jan 14, 2022
@AdoAdoAdo AdoAdoAdo deleted the feat-scheduled-sc-execution-with-new-config branch January 14, 2022 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raduchis raduchis raduchis approved these changes

@AdoAdoAdo AdoAdoAdo AdoAdoAdo approved these changes

Assignees

@SebastianMarian SebastianMarian

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@SebastianMarian @raduchis @AdoAdoAdo