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

Do not charge gas for feecollector address query #1795

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

codchen
Copy link
Collaborator

@codchen codchen commented Jul 30, 2024

Describe your changes and provide context

Charging gas for a potentially cached operation could cause non-determinism

Testing performed to validate your change

applied on archive node to get it out of apphash error

@codchen codchen requested a review from yzang2019 July 30, 2024 02:59
@codchen codchen changed the title Do not charge cache for feecollector address query Do not charge gas for feecollector address query Jul 30, 2024
@codchen codchen force-pushed the fix-feecollector-cache branch from 9966e05 to 7bbb5c8 Compare July 30, 2024 03:04
Copy link

codecov bot commented Jul 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.73%. Comparing base (9355f52) to head (7bbb5c8).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1795      +/-   ##
==========================================
- Coverage   60.83%   60.73%   -0.10%     
==========================================
  Files         257      257              
  Lines       22490    22491       +1     
==========================================
- Hits        13682    13661      -21     
- Misses       7842     7866      +24     
+ Partials      966      964       -2     
Files Coverage Δ
x/evm/keeper/coinbase.go 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

@codchen codchen requested a review from philipsu522 July 30, 2024 14:33
@philipsu522 philipsu522 merged commit d57ab7d into main Jul 30, 2024
48 checks passed
@philipsu522 philipsu522 deleted the fix-feecollector-cache branch July 30, 2024 16:00
philipsu522 pushed a commit that referenced this pull request Jul 30, 2024
Do not charge cache for feecollector address query
yzang2019 added a commit that referenced this pull request Aug 15, 2024
* main:
  Add more DEX dapp tests (#1809)
  Add basic LST integration tests  (#1814)
  Allow CW->ERC pointers to be called through wasmd precompile (#1785)
  Bump nonce even if tx fails (#1778)
  Fix docker setup for local cluster (#1806)
  Tune Configs (#1813)
  V5.7.5 release (#1805)
  Evidence Max Bytes Update (#1812)
  Add dApp Tests (#1802)
  [EVM] Tune configs (#1800)
  Revert dex removal (#1801)
  Do not charge gas for feecollector address query (#1795)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants