-
Notifications
You must be signed in to change notification settings - Fork 32
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
Submitter: setGasPrice() refactor #2462
Merged
aureliusbtc
merged 23 commits into
submitter/dynamic-tx-fees
from
submitter/dynamic-tx-fees-refactor
Apr 6, 2024
Merged
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
d2f3593
Feat: initial refactor of setGasPrice()
dwasse f60ee8f
Feat: add more tracing
dwasse ecf6ede
Cleanup: comments
dwasse 7012588
Feat: BaseGasPrice -> MinGasPrice
dwasse a0426d3
Cleanup: remove applyMinGasPrice()
dwasse 850ab76
Feat: initial test cases
dwasse 9e6c08c
Feat: add WithOracleOverride cases
dwasse c09a8c1
Feat: add PrevTx cases
dwasse e2086a5
Feat: add assertGasValues() helper
dwasse e5bbdd2
Cleanup: pass useDynamic to applyGasFloor()
dwasse 1e56c27
Feat: remove old submitter test
dwasse 19a4445
Cleanup: lint
dwasse 6c52b8b
[goreleaser]
dwasse d245a73
Cleanup: populateFromPrevTx() -> bumpFromPrevTx()
dwasse b4394d1
Feat: cap GasTipCap by GasFeeCap
dwasse 5250e95
[goreleaser]
dwasse fddb1a8
Feat: default min gas price to 1 gwei
dwasse 3cb6dae
[goreleaser]
dwasse e1e0695
Feat: add minTipCap var
dwasse 25b9345
Fix: minTipCap = 10 wei
dwasse ed148a0
Cleanup: comment
dwasse de10984
[goreleaser]
dwasse a4057c5
Fix: test
dwasse File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -31,8 +31,8 @@ | |||||
DoNotBatch bool `yaml:"skip_batching"` | ||||||
// MaxGasPrice is the maximum gas price to use for transactions | ||||||
MaxGasPrice *big.Int `yaml:"max_gas_price"` | ||||||
// BaseGasPrice is the gas price that will be used if 0 is returned from the gas price oracle | ||||||
BaseGasPrice *big.Int `yaml:"base_gas_price"` | ||||||
// MinGasPrice is the gas price that will be used if 0 is returned from the gas price oracle | ||||||
MinGasPrice *big.Int `yaml:"min_gas_price"` | ||||||
// BumpIntervalSeconds is the number of seconds to wait before bumping a transaction | ||||||
BumpIntervalSeconds int `yaml:"bump_interval_seconds"` | ||||||
// GasBumpPercentages is the percentage to bump the gas price by | ||||||
|
@@ -67,8 +67,8 @@ | |||||
// DefaultMaxPrice is the default max price of a tx. | ||||||
var DefaultMaxPrice = big.NewInt(500 * params.GWei) | ||||||
|
||||||
// DefaultBaseGasPrice is the default max price of a tx. | ||||||
var DefaultBaseGasPrice = big.NewInt(1 * params.GWei) | ||||||
// DefaultMinGasPrice is the default min price of a tx. | ||||||
var DefaultMinGasPrice = big.NewInt(10 * params.GWei) | ||||||
|
||||||
// note: there's probably a way to clean these getters up with generics, the real problem comes with the fact that | ||||||
// that this would require the caller to override the entire struct, which is not ideal.. | ||||||
|
@@ -110,17 +110,17 @@ | |||||
return | ||||||
} | ||||||
|
||||||
// GetBaseGasPrice returns the maximum gas price to use for transactions. | ||||||
func (c *Config) GetBaseGasPrice(chainID int) (basePrice *big.Int) { | ||||||
basePrice = c.BaseGasPrice | ||||||
// GetMinGasPrice returns the maximum gas price to use for transactions. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
func (c *Config) GetMinGasPrice(chainID int) (minPrice *big.Int) { | ||||||
minPrice = c.MinGasPrice | ||||||
|
||||||
chainConfig, ok := c.Chains[chainID] | ||||||
if ok && chainConfig.BaseGasPrice != nil { | ||||||
basePrice = chainConfig.BaseGasPrice | ||||||
if ok && chainConfig.MinGasPrice != nil { | ||||||
minPrice = chainConfig.MinGasPrice | ||||||
} | ||||||
|
||||||
if basePrice == nil || basePrice == big.NewInt(0) { | ||||||
basePrice = DefaultBaseGasPrice | ||||||
if minPrice == nil || minPrice == big.NewInt(0) { | ||||||
minPrice = DefaultMinGasPrice | ||||||
} | ||||||
return | ||||||
} | ||||||
|
@@ -217,9 +217,9 @@ | |||||
c.MaxGasPrice = maxPrice | ||||||
} | ||||||
|
||||||
// SetBaseGasPrice is a helper function that sets the base gas price. | ||||||
func (c *Config) SetBaseGasPrice(basePrice *big.Int) { | ||||||
c.BaseGasPrice = basePrice | ||||||
// SetMinGasPrice is a helper function that sets the base gas price. | ||||||
func (c *Config) SetMinGasPrice(basePrice *big.Int) { | ||||||
c.MinGasPrice = basePrice | ||||||
} | ||||||
|
||||||
// SetGlobalEIP1559Support is a helper function that sets the global EIP1559 support. | ||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Min gas price of 10 Gwei is an overshoot.
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.
This makes sense- I'm confused of the intention here though in the base PR?
https://github.com/synapsecns/sanguine/pull/2432/files#diff-9cb5c46a90d8a5d8f06ab1c53594fbc934b93eaf5131c538424f0c4666f94f49R429
To me this seems like we will never set gas price below 10 gwei if no
prevTx
is suppliedThere 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.
The unit there is in wei
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.
Let's introduce a min value for GasPrice/GasFeeCap (DefaultMinGasPrice) and a separate one for GasTipCap. The latter one is set to 10 Wei (aka just 10) in the base PR.