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

gas consumption vs gas provided #3615

Merged
merged 7 commits into from
Dec 8, 2021
Merged

Conversation

AdoAdoAdo
Copy link
Contributor

@AdoAdoAdo AdoAdoAdo commented Dec 2, 2021

use the actual gas consumed for txs preselection
don't limit intrashard txs due to stuck status

@AdoAdoAdo AdoAdoAdo changed the title use the actual gas consumed for txs preselection; don't limit intrash… gas consumption vs gas provided Dec 2, 2021
@codecov
Copy link

codecov bot commented Dec 2, 2021

Codecov Report

Merging #3615 (82ae8cc) into development (cc789ff) will not change coverage.
The diff coverage is 72.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #3615   +/-   ##
============================================
  Coverage        73.69%   73.69%           
============================================
  Files              585      585           
  Lines            75317    75317           
============================================
  Hits             55503    55503           
  Misses           15405    15405           
  Partials          4409     4409           
Impacted Files Coverage Δ
factory/processComponents.go 62.38% <0.00%> (ø)
process/block/metablock.go 55.09% <0.00%> (ø)
process/block/preprocess/transactions.go 57.72% <0.00%> (ø)
process/coordinator/process.go 74.59% <75.00%> (ø)
process/block/preprocess/basePreProcess.go 78.81% <100.00%> (ø)
process/block/preprocess/gasComputation.go 97.43% <100.00%> (ø)
process/block/shardblock.go 67.07% <100.00%> (ø)

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 2882926...82ae8cc. Read the comment docs.

iulianpascalau
iulianpascalau previously approved these changes Dec 2, 2021
Copy link
Contributor

@iulianpascalau iulianpascalau left a comment

Choose a reason for hiding this comment

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

checked with the first implementation, looks good 👍

…rd is stuck situation too early, as now could exist many mini blocks (max. 3) from me to the same receiver in one block, because of the recently implemented splitting mechanism from 1.5 bil. to 600 mil. per mini block
iulianpascalau
iulianpascalau previously approved these changes Dec 2, 2021
@@ -124,8 +124,8 @@ func (gc *gasComputation) GasPenalized(hash []byte) uint64 {
return gasPenalized
}

// TotalGasConsumed gets the total gas consumed
func (gc *gasComputation) TotalGasConsumed() uint64 {
// TotalGasProvided gets the total gas consumed
Copy link
Contributor

Choose a reason for hiding this comment

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

...total gas provided

@@ -596,7 +596,7 @@ func (sp *shardProcessor) indexBlockIfNeeded(
return
}

gasConsumedInHeader := sp.baseProcessor.gasConsumedProvider.TotalGasConsumed()
gasConsumedInHeader := sp.baseProcessor.gasConsumedProvider.TotalGasProvided()
Copy link
Contributor

Choose a reason for hiding this comment

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

gasProvidedInHeader, and maybe we should change also in HeaderGasConsumption DTO (L610) from GasConsumed to GasProvided

gabi-vuls
gabi-vuls previously approved these changes Dec 3, 2021
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.
Errors are safe and will be solved soon.
image

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.
Errors are safe and will be solved with PR #3623
image

@@ -621,7 +621,7 @@ func (mp *metaProcessor) indexBlock(
Header: metaBlock,
SignersIndexes: signersIndexes,
HeaderGasConsumption: indexer.HeaderGasConsumption{
GasConsumed: gasConsumedInHeader,
GasProvided: gasConsumedInHeader,
Copy link
Contributor

Choose a reason for hiding this comment

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

gasProvidedInHeader

func (gc *gasComputation) TotalGasProvided() uint64 {
totalGasConsumed := uint64(0)
totalGasProvided := uint64(0)

gc.mutGasConsumed.RLock()
for _, gasConsumed := range gc.gasConsumed {
Copy link
Contributor

Choose a reason for hiding this comment

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

gasProvided and gc.gasProvided ?

@LucianMincu LucianMincu merged commit b93ad25 into development Dec 8, 2021
@LucianMincu LucianMincu deleted the use-correct-gas-consumption branch December 8, 2021 13:07
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

5 participants