Skip to content

Fix gas price calculations in tracer#2016

Merged
palango merged 5 commits intomasterfrom
paul/fix-tracer-gas
Feb 23, 2023
Merged

Fix gas price calculations in tracer#2016
palango merged 5 commits intomasterfrom
paul/fix-tracer-gas

Conversation

@palango
Copy link
Copy Markdown
Collaborator

@palango palango commented Feb 20, 2023

Description

Fix the tracing module to correctly use the base fee when calculating
gas prices.

Tested

Adds a regression test.

Related issues

Resolves #2002

@palango palango requested a review from a team as a code owner February 20, 2023 12:35
@palango palango requested review from hbandura and piersy and removed request for a team February 20, 2023 12:35
Comment thread eth/tracers/api.go Outdated
@palango palango force-pushed the paul/fix-tracer-gas branch from 1f791de to 021618e Compare February 20, 2023 12:36
@palango palango changed the title Fix gas price calculations Fix gas price calculations in tracer Feb 20, 2023
@akeyless-target-app
Copy link
Copy Markdown

akeyless-target-app Bot commented Feb 20, 2023

Coverage from tests in ./e2e_test/... for ./consensus/istanbul/... at commit 8141184

coverage: 43.9% of statements across all listed packages
coverage:  47.7% of statements in consensus/istanbul
coverage:  23.7% of statements in consensus/istanbul/announce
coverage:  54.3% of statements in consensus/istanbul/backend
coverage:   0.0% of statements in consensus/istanbul/backend/backendtest
coverage:  24.3% of statements in consensus/istanbul/backend/internal/replica
coverage:  55.5% of statements in consensus/istanbul/core
coverage:  45.0% of statements in consensus/istanbul/db
coverage:   0.0% of statements in consensus/istanbul/proxy
coverage:  64.4% of statements in consensus/istanbul/uptime
coverage:  51.8% of statements in consensus/istanbul/validator
coverage:  79.2% of statements in consensus/istanbul/validator/random
CommentID: 5425b7de98

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 20, 2023

Codecov Report

Base: 54.82% // Head: 54.85% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (8141184) compared to base (e831c00).
Patch coverage: 58.90% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2016      +/-   ##
==========================================
+ Coverage   54.82%   54.85%   +0.02%     
==========================================
  Files         688      695       +7     
  Lines       92542    93274     +732     
==========================================
+ Hits        50737    51165     +428     
- Misses      37993    38257     +264     
- Partials     3812     3852      +40     
Impacted Files Coverage Δ
cmd/geth/config.go 0.00% <0.00%> (ø)
cmd/geth/main.go 20.00% <ø> (ø)
cmd/utils/flags.go 2.77% <0.00%> (-0.02%) ⬇️
consensus/istanbul/config.go 0.00% <0.00%> (ø)
consensus/istanbul/core/preprepare.go 71.23% <0.00%> (+7.03%) ⬆️
contracts/gasprice_minimum/gasprice_minimum.go 45.61% <0.00%> (-14.86%) ⬇️
contracts/testutil/utils.go 30.00% <ø> (ø)
core/chain_makers.go 66.39% <0.00%> (-1.11%) ⬇️
core/sys_context.go 78.57% <ø> (ø)
core/vm/operations_acl.go 4.44% <ø> (ø)
... and 65 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@palango palango force-pushed the paul/fix-tracer-gas branch from 021618e to 62aa19d Compare February 20, 2023 13:00
@palango palango requested review from gastonponti and removed request for piersy February 20, 2023 15:24
Comment thread eth/tracers/api.go Outdated
Copy link
Copy Markdown
Contributor

@carterqw2 carterqw2 left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Copy Markdown
Contributor

@gastonponti gastonponti left a comment

Choose a reason for hiding this comment

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

The majority are nits, and use the actual number that the mock will call for the gasPrice

Comment thread eth/tracers/api.go Outdated
Comment thread eth/tracers/api_test.go Outdated
Comment thread eth/tracers/api_test.go Outdated
Comment thread eth/tracers/api_test.go Outdated
Comment thread eth/tracers/api_test.go Outdated
Comment thread eth/tracers/api_test.go Outdated
Comment thread eth/tracers/api_test.go Outdated
@palango palango force-pushed the paul/fix-tracer-gas branch from 8846a39 to a73449d Compare February 23, 2023 11:30
@palango palango force-pushed the paul/fix-tracer-gas branch from 3955cbb to 8141184 Compare February 23, 2023 16:31
@palango palango merged commit 0caacdf into master Feb 23, 2023
@palango palango deleted the paul/fix-tracer-gas branch February 23, 2023 16:46
palango added a commit that referenced this pull request Mar 10, 2023
* tracer: Add regression test for #2002

* tracer: Fix gas price calculations (#2002)

Fix the tracing module to correctly use the base fee when calculating
gas prices.

* tracer: Only pass fee currency into `getBaseFee`

* tracer: PR review

* tests: Move contract code to testutils
@palango palango mentioned this pull request Mar 10, 2023
gastonponti pushed a commit that referenced this pull request Mar 13, 2023
* tracer: Add regression test for #2002

* tracer: Fix gas price calculations (#2002)

Fix the tracing module to correctly use the base fee when calculating
gas prices.

* tracer: Only pass fee currency into `getBaseFee`

* tracer: PR review

* tests: Move contract code to testutils
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.

"insufficient funds for gas" error when doing debug_traceBlockByNumber on 0x108F49C

3 participants