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

fix(ante): handle zero fee case on evm transactions #1753

Merged
merged 8 commits into from
Sep 19, 2023
Merged

Conversation

GAtom22
Copy link
Contributor

@GAtom22 GAtom22 commented Sep 6, 2023

Description

This change addresses the case reported by @luchenqun of having zero fees for evm transactions
(more details on this PR)

Nit: Before merging these changes, would be great to add some e2e tests to cover this case once the nix testing setup is merged


Closes ENG-2046

@GAtom22 GAtom22 changed the title fix: handle zero fee case on eth ante fix(ante): handle zero fee case on evm transactions Sep 6, 2023
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #1753 (8830e3d) into main (8c968be) will increase coverage by 0.00%.
The diff coverage is 75.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1753   +/-   ##
=======================================
  Coverage   70.32%   70.33%           
=======================================
  Files         297      297           
  Lines       22287    22293    +6     
=======================================
+ Hits        15673    15679    +6     
  Misses       5847     5847           
  Partials      767      767           
Files Changed Coverage
app/ante/evm/eth.go 75.00%

@GAtom22 GAtom22 marked this pull request as ready for review September 6, 2023 22:28
@GAtom22 GAtom22 requested a review from a team as a code owner September 6, 2023 22:28
@GAtom22 GAtom22 requested review from MalteHerrmann, 0xstepit, Vvaradinov and facs95 and removed request for a team September 6, 2023 22:28
@linear
Copy link

linear bot commented Sep 7, 2023

ENG-2046 Fix bug for Zero fee EVM tx

Context

When the genesis parameter feemarket sets no_base_fee=true and min_gas_price=0, the user send a evm transaction with a gasPrice of 0 fails.

Expected

Tx should be successful

There's a PR addressing this issue already.

Tasks:

  • Add e2e test on nix setup for this configuration

More details of the issue on this PR

Copy link
Contributor

@facs95 facs95 left a comment

Choose a reason for hiding this comment

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

LGMT!

Copy link
Contributor

@MalteHerrmann MalteHerrmann left a comment

Choose a reason for hiding this comment

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

LGTM! Great work 🚀

Copy link
Contributor

@Vvaradinov Vvaradinov left a comment

Choose a reason for hiding this comment

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

LGTM! Great work!

@github-actions github-actions bot added the tests label Sep 19, 2023
@GAtom22 GAtom22 merged commit e041fdf into main Sep 19, 2023
38 of 39 checks passed
@GAtom22 GAtom22 deleted the GAtom22/evm-fees branch September 19, 2023 14:59
@GAtom22
Copy link
Contributor Author

GAtom22 commented Oct 16, 2023

https://github.com/Mergifyio backport release/v15.0.x

@mergify
Copy link
Contributor

mergify bot commented Oct 16, 2023

backport release/v15.0.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Oct 16, 2023
* fix: handle zero fee case on eth ante

* refactor style

* add changelog entry

* add zero fees unit test case

* refactor helper functions

* add zero fees integration tests

* fix lint issues

(cherry picked from commit e041fdf)

# Conflicts:
#	CHANGELOG.md
GAtom22 added a commit that referenced this pull request Oct 16, 2023
…1889)

* fix(ante): handle zero fee case on evm transactions (#1753)

* fix: handle zero fee case on eth ante

* refactor style

* add changelog entry

* add zero fees unit test case

* refactor helper functions

* add zero fees integration tests

* fix lint issues

(cherry picked from commit e041fdf)

# Conflicts:
#	CHANGELOG.md

* fix conflicts

---------

Co-authored-by: Tom <54514587+GAtom22@users.noreply.github.com>
Co-authored-by: tom <tomasguerraalda@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants