-
Notifications
You must be signed in to change notification settings - Fork 592
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
Add separate min gas price option for txes with high gas wanted #777
Add separate min gas price option for txes with high gas wanted #777
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woah awesome job on this!
This is extremely well done imo, thanks for refactoring everything into Mempool options, and cleaning up the config parsing
I think your right re: Not moving high gas thresholds as something done in consensus as well, and just keeping it in app.go. (Which this PR does) |
Codecov Report
@@ Coverage Diff @@
## main #777 +/- ##
==========================================
+ Coverage 19.96% 19.97% +0.01%
==========================================
Files 189 189
Lines 24542 24547 +5
==========================================
+ Hits 4899 4904 +5
Misses 18788 18788
Partials 855 855
Continue to review full report at Codecov.
|
32cac99
to
24f9976
Compare
Closes: #724
Description
Open questions to reviewer:
I think we should not move
HighGasTxThreshold
parameter into consensus, but leave it just as a node option. If so, it's better to add it toapp.toml
Other
Do not waste time for choosing words, if something is wrong just said it straightforward, i am OK with that.
For contributor use:
docs/
) or specification (x/<module>/spec/
)Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorer