Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Use effectiveGasPrice in ante handler for dynamic fee tx #817

Merged
merged 10 commits into from
Dec 15, 2021

Conversation

yihuang
Copy link
Contributor

@yihuang yihuang commented Dec 9, 2021

Closes: #814

Description

Solution:

  • use effectiveGasPrice when check minimal-gas-prices, and deduct fee in ante handler
  • implement an EthMempoolFeeDecorator

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

Closes: evmos#814

Solution:
- use effectiveGasPrice when check minimal-gas-prices, and deduct fee in ante handler
- implement an EthMempoolFeeDecorator
@codecov
Copy link

codecov bot commented Dec 9, 2021

Codecov Report

Merging #817 (ed92b57) into main (5d237a5) will decrease coverage by 0.35%.
The diff coverage is 43.28%.

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

@@            Coverage Diff             @@
##             main     #817      +/-   ##
==========================================
- Coverage   57.34%   56.99%   -0.36%     
==========================================
  Files          72       72              
  Lines        5981     6041      +60     
==========================================
+ Hits         3430     3443      +13     
- Misses       2354     2398      +44     
- Partials      197      200       +3     
Impacted Files Coverage Δ
x/evm/types/access_list_tx.go 69.07% <0.00%> (-1.87%) ⬇️
x/evm/types/legacy_tx.go 92.24% <0.00%> (-2.96%) ⬇️
x/evm/types/msg.go 65.65% <0.00%> (-4.24%) ⬇️
x/evm/types/tx_data.go 85.41% <ø> (ø)
x/evm/types/dynamic_fee_tx.go 75.97% <14.28%> (-2.64%) ⬇️
app/ante/eth.go 84.37% <63.63%> (-1.21%) ⬇️
app/ante/ante.go 38.27% <100.00%> (-11.12%) ⬇️
x/evm/keeper/utils.go 72.05% <100.00%> (-0.80%) ⬇️
x/evm/types/tx_args.go 0.00% <0.00%> (ø)
... and 2 more

x/evm/types/dynamic_fee_tx.go Outdated Show resolved Hide resolved
x/evm/types/dynamic_fee_tx.go Outdated Show resolved Hide resolved
app/ante/eth.go Show resolved Hide resolved
Copy link
Contributor

@thomas-nguy thomas-nguy left a comment

Choose a reason for hiding this comment

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

lgtm

Co-authored-by: Thomas Nguy <81727899+thomas-nguy@users.noreply.github.com>
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

ACK, minor comments

app/ante/eth.go Outdated Show resolved Hide resolved
app/ante/eth.go Outdated Show resolved Hide resolved
x/evm/keeper/utils.go Show resolved Hide resolved
x/evm/types/legacy_tx.go Show resolved Hide resolved
x/evm/types/access_list_tx.go Show resolved Hide resolved
@fedekunze fedekunze enabled auto-merge (squash) December 15, 2021 02:14
@fedekunze fedekunze merged commit ccc6f5b into evmos:main Dec 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem: eip-1559 tx fee deduction is problematic
3 participants