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

Merge Development into feat/scheduled-sc-execution plus some gas fix #3705

Merged

Conversation

AdoAdoAdo
Copy link
Contributor

No description provided.

raduchis and others added 30 commits November 11, 2021 14:56
# Conflicts:
#	cmd/node/config/enableEpochs.toml
#	node/nodeRunner.go
# Conflicts:
#	cmd/node/config/enableEpochs.toml
#	config/epochConfig.go
#	node/nodeRunner.go
Jailed node should be unstaked if enough validators
# Conflicts:
#	storage/pruning/pruningStorer.go
#	storage/pruning/pruningStorer_test.go
#	testscommon/trie/snapshotPruningStorerStub.go
raduchis
raduchis previously approved these changes Jan 19, 2022
…e than max gas accepted in txs dest me (in this case at least one mini block should be accepted, otherwise the shard will be stuck with processing at the current meta block)
@@ -1362,15 +1403,15 @@ func (txs *transactions) ProcessMiniBlock(
return processedTxHashes, index, err
}

gasProvidedByTxInSelfShard, errComputeGas := txs.computeGasProvided(
gasProvidedByTxInSelfShard, err = txs.computeGasProvided(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍
missed that

raduchis
raduchis previously approved these changes Jan 21, 2022
raduchis
raduchis previously approved these changes Jan 21, 2022
iulianpascalau
iulianpascalau previously approved these changes Jan 21, 2022

for index := range miniBlockScrs {
if !haveTime() {
err = process.ErrTimeIsOut
return processedTxHashes, index, err
}

gasProvidedByTxInSelfShard, errComputeGas := scr.computeGasProvided(
gasProvidedByTxInSelfShard, err = scr.computeGasProvided(
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 main reason that using defer function that handles an internal err variable is a bad practice.
This should have been split in at least 3 functions:

scr.processSomethingWithoutErrHandling()
err := scr.processOtherStuffWithErrHandling()
scr.handleErr(err)

In this way the intent will be better shown and the code would have been more robust

We should stop writing code like this and refactor wherever we can this.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

raduchis
raduchis previously approved these changes Jan 21, 2022
@AdoAdoAdo AdoAdoAdo merged commit c9edfcd into feat/scheduled-sc-execution Jan 21, 2022
@AdoAdoAdo AdoAdoAdo deleted the merge-snapshot-improvement-feat-scheduled branch January 21, 2022 12:57
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

9 participants