Skip to content
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

Extend base_fee_per_gas type to uint256 #2548

Closed
wants to merge 1 commit into from

Conversation

mkalinin
Copy link
Collaborator

@mkalinin mkalinin commented Aug 12, 2021

As it's been well spotted by @protolambda base_fee_per_gas: uint64 is prone to overflow. Suppose base_fee_per_gas = 32 Gwei, miners will need to produce full blocks for ~150 in a row to overflow this value and eventually brick the Merge.

Thus, data structures, constants and arithmetics is update to use extended uint256 type for base_fee_per_gas.

UPD
This attack requires 924,730 ETH to be burned which is of the very high cost taking current ETH price. If the starting point of base_fee_per_gas = 1024 Gwei then the cost drops to 81,947 ETH.

@djrtwo
Copy link
Contributor

djrtwo commented Aug 12, 2021

Another option is to call it a bytes32 and to not do any validation on the execution-layer base fee within the consensus layer. This would remove the uint256 computational requirement and keep the validations where they currently are

@mkalinin
Copy link
Collaborator Author

Another option is to call it a bytes32 and to not do any validation on the execution-layer base fee within the consensus layer. This would remove the uint256 computational requirement and keep the validations where they currently are

Right, but eventually we'd like to move more checks to the consensus layer and this option defers the introduction of uint256 into the spec, though, it is viable and might make sense short term.

@potuz
Copy link
Contributor

potuz commented Aug 12, 2021

But after the merge the overflow becomes a non-issue since it's very unlikely to get too many blocks in a row under control. So it's better to leave as 32 bytes and not introduce ever the 256bit arithmetic

@protolambda
Copy link
Collaborator

Related issue: in the sharding spec we have base-fees as well, and cannot push those in execution layer. Either shards will have an upper bound and/or lower precision reduction on fee fields, or we introduce uint256 there anyway.

@hwwhww hwwhww added the Bellatrix CL+EL Merge label Aug 12, 2021
@Arachnid
Copy link

2^64 wei is 18 ether. Filling a 30M gas block at that base fee would require over 500 million ether. Surely this is a nonissue? Everyone will run out of ether way before hitting this.

@protolambda
Copy link
Collaborator

protolambda commented Aug 12, 2021

@Arachnid there are ways to decrease that 30M gas to just 21000 (minimum for a single tx to fill a block with), and then there's a minimum 5000 gas limit which avoids a 0 limit, and thus avoids base-fees being manipulated upwards for free (while this 5000 minimum is way older than EIP 1559). You're right that it's a silly issue, but not entirely a non-issue until you consider ways like this around it.

@protolambda
Copy link
Collaborator

protolambda commented Aug 12, 2021

Another thing to consider is that as attacker you don't even need the full 64 bits base fee: the base_fee_per_gas is multiplied with other uint64 numbers. So cost may decrease by another 1000x. Eth1 uses big-nums for these computations, here we use uint64, which will overflow / error if safe-math.

@mkalinin
Copy link
Collaborator Author

Closing in favour of #2550

@mkalinin mkalinin closed this Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bellatrix CL+EL Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants