zksync-era
zksync-era copied to clipboard
feat(Gas Oracle): Move gas prices to be `U256` to avoid overflows
What ❔
Replace variables types from u64
to U256
.
Why ❔
Prevent possible overflows.
Checklist
- [x] PR title corresponds to the body of PR (we generate changelog entries from PRs).
- [x] Code has been formatted via
zk fmt
andzk lint
.
Not sure if this PR is ready to be reviewed, but I think that if we're going to switch to
U256
becauseu64
can now overflow with custom base token, it is certainly in scope to address all the places where an overflow can occur (otherwise there isn't much value in the change).
That makes sense, do you want to add someone else to review this PR or are you checking it internally?
@jrchatruc I've asked @perekopskiy to review and assist you with this PR.
Some general questions/comments
- Do I get it right that
base_fee_per_gas
/l2_fair_gas_price
may overflowu64
if base token price is low?- Do I get it right that
l1_gas_price
may overflowu64
if L1 is not Ethereum but some other chain with cheap base token?u64
is sufficient forgas_per_pubdata
, because all VM gas fits intou32
, so evenu32
is enough. So please revert all changes you've done togas_per_pubdata
- Indeed both
base_fee_per_gas
andl2_fair_gas_price
may overflow 64 bits if the base token price is low. - Yes, I believe gas prices in Ethereum are also technically
U256
, so I think it's a good idea to change it so it matches. - Makes sense, we'll revert them
The change required for this ticket (related to this discussion) was merged into this PR (here a11e12c) since it requires its changes to do it.