[SHIP-2208] Enable EIP-1559 Transactions on Scroll #13687
Conversation
There was a problem hiding this comment.
LGTM!
Thanks for making this change.
Nit: Can we please add to the PR description that we're also changing the gas estimation mode from SuggestedPrice to BlockHistoryEstimator? To make it more readable for reviewers and since it's easy to miss in the changes.
| PriceMin = '0' | ||
| EIP1559DynamicFees = true | ||
| PriceMin = '1 wei' | ||
| BumpMin = '100 wei' |
There was a problem hiding this comment.
based on the current gas Prices the BumpMin is too low, I'd least bump up a couple of gwei
There was a problem hiding this comment.
happy to modify these values but we'll need to re-run the soak test and get QA signoff. during the tests we didn't encounter any issues with gas but i'm happy to adjust these values
There was a problem hiding this comment.
Thats is because while running the tests there may not have been gas spikes. when there is a sudden gas spike & if the transaction needs to bump, bumping slowly with 100wei will take quite a long time to get the OCR rounds included. if we are worried about the soak tests, I'd say run a quicker one, since we are only bumping up, validating everything works should be enough
|
KodeyThomas
dismissed stale reviews from mohamed-mehany and simsonraj
via
fa1b5d5
|
|
||
| [GasEstimator.BlockHistory] | ||
| # Force an error if someone enables the estimator by accident; we never want to run the block history estimator on Scroll | ||
| BlockHistorySize = 0 | ||
| BlockHistorySize = 200 |
There was a problem hiding this comment.
200 seems rather large. Optimism has 24 which seems of a similar magnitude.
There was a problem hiding this comment.
agree, happy to reduce this
Description
This PR enables EIP-1559 Transactions for Scroll. Although mostly just configuration changes, a soak test revealed that when enabling EIP-1559 transactions a significant amount of warnings occurred when using gas estimation.
This occurs due to the
L1MessageTxtransaction type being included in the gas estimation which we are unable to decode. This transaction type has been explicitly ignored during gas estimation using a similar mechanism to Celo which was introduced in #10660.This PR also migrates away from the
SuggestedPricegas estimator toBlockHistoryEstimatorMotivation
SHIP-2208