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

Problem: native fee handler don't respect DefaultPriorityReduction #416

Closed
wants to merge 4 commits into from

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Mar 11, 2024

  • vendor fee deduct decorator, so it'll calls the custom checkTxFeeWithValidatorMinGasPrices method.
  • a little refactor to remove DeductTxCostsFromUserBalance and call the DeductFees directly

next: will vendor the DeductFees method and modify the impl for parallel exec

Closes: #XXX

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)

x/evm/handler_test.go Outdated Show resolved Hide resolved
x/evm/handler_test.go Outdated Show resolved Hide resolved
@yihuang yihuang changed the title Problem: deduct fee method is not shared Problem: DeductTxCostsFromUserBalance method is not necessary Mar 11, 2024
@yihuang yihuang requested a review from mmsqe March 11, 2024 05:20
@yihuang yihuang changed the title Problem: DeductTxCostsFromUserBalance method is not necessary Problem: native fee handler don't respect DefaultPriorityReduction Mar 11, 2024
@mmsqe
Copy link
Collaborator

mmsqe commented Mar 11, 2024

seems fetch sender account not work as expected now

Solution:
- vendor fee deduct decorator
- a little refactor to remove `DeductTxCostsFromUserBalance` and call the DeductFees directly

next: will modify the DeductFees to provide a different impl for parallel exec

Apply suggestions from code review

Signed-off-by: yihuang <[email protected]>

fixes

fix err
@yihuang
Copy link
Collaborator Author

yihuang commented Mar 12, 2024

seems fetch sender account not work as expected now

WDYM? test passed now.

@yihuang yihuang closed this Mar 13, 2024
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.

2 participants