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

test DynamicFeeTx #649

Merged
merged 15 commits into from
Oct 19, 2021
Merged

test DynamicFeeTx #649

merged 15 commits into from
Oct 19, 2021

Conversation

JayT106
Copy link
Contributor

@JayT106 JayT106 commented Oct 7, 2021

  • eth_call gRPC
  • benchmark tests
  • state transition tests
  • AnteHandler

Closes: #622

Description


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)

@fedekunze fedekunze marked this pull request as draft October 7, 2021 23:13
@JayT106 JayT106 force-pushed the test-dynamicFeeTx branch 5 times, most recently from 4eb31ad to 3ef628c Compare October 12, 2021 16:37
@codecov
Copy link

codecov bot commented Oct 12, 2021

Codecov Report

Merging #649 (b7e5d73) into main (2476cc5) will increase coverage by 0.06%.
The diff coverage is 38.46%.

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

@@            Coverage Diff             @@
##             main     #649      +/-   ##
==========================================
+ Coverage   56.91%   56.98%   +0.06%     
==========================================
  Files          63       63              
  Lines        5557     5568      +11     
==========================================
+ Hits         3163     3173      +10     
- Misses       2231     2234       +3     
+ Partials      163      161       -2     
Impacted Files Coverage Δ
app/test_helpers.go 0.00% <0.00%> (ø)
x/evm/types/msg.go 70.94% <0.00%> (ø)
x/evm/keeper/utils.go 75.00% <100.00%> (+6.50%) ⬆️
app/export.go 13.28% <0.00%> (-0.19%) ⬇️
app/ante/eth.go 87.98% <0.00%> (+0.90%) ⬆️

@JayT106 JayT106 force-pushed the test-dynamicFeeTx branch 2 times, most recently from a0a9e41 to 2a623f8 Compare October 12, 2021 23:08
@JayT106 JayT106 marked this pull request as ready for review October 13, 2021 22:32
@fedekunze
Copy link
Contributor

@JayT106 is this still WIP? if not, can you update the title? 🙏

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

x/evm/keeper/state_transition_benchmark_test.go Outdated Show resolved Hide resolved
x/evm/keeper/utils.go Outdated Show resolved Hide resolved
expectPass: false,
dynamicTxFee: true,
},
// TODO: is this case valid?
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you mean by this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because in this case, the effectiveTip will be 0, So there is no fee that has been deducting in authante, which causes the error returns. Is it an expecting behavior?

x/evm/types/msg.go Show resolved Hide resolved
@JayT106 JayT106 changed the title test DynamicFeeTx - WIP test DynamicFeeTx Oct 15, 2021
@fedekunze fedekunze enabled auto-merge (squash) October 18, 2021 12:27
@JayT106
Copy link
Contributor Author

JayT106 commented Oct 19, 2021

@fedekunze There is a non-deterministic test case TestEstimateGas/Case_contract_deployment in main, I will try to see the root cause.

@fedekunze fedekunze merged commit 1076307 into evmos:main Oct 19, 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.

Update tests to use DynamicFeeTx
2 participants