-
Notifications
You must be signed in to change notification settings - Fork 562
Minimum GasUsed proportional to GasLimit #1087
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1087 +/- ##
==========================================
+ Coverage 59.55% 59.57% +0.01%
==========================================
Files 84 85 +1
Lines 6933 6971 +38
==========================================
+ Hits 4129 4153 +24
- Misses 2591 2601 +10
- Partials 213 217 +4
|
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.
@crypto-facs LGMT, left minor comments.
Do you think it makes sense to add unit tests here?
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.
Thanks @crypto-facs! Changes LGTM. Some things pending:
- tests to check that the multiplier logic works as intended
- update specification
- add migration logic to set the new parameter during upgrade
- migration 1 -> 2 (consensus version of the evm module)
- define migrator
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.
LGTM, minor change requested
Closes: #1085
Description
More info about this issue on the issue, but in summary this PR aims to add a new functionality on which to control the minimum amount of gas to be charged proportional to the
GasLimit
of the transaction. The equation is the following:max(gasUsed, gasLimit / MinGasDenominator)
MinGasDenominator
being an EVM module param for the minimum about to be charged to be able to be updated as the network changes.NOTE: This is a breaking change
For contributor use:
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerFor admin use:
WIP
,R4R
,docs
, etc)