Commit 2ffabba6 authored by Mark Tyneway's avatar Mark Tyneway

contracts-bedrock: fix arithmetic, add test coverage

parent 75080ba4
...@@ -140,7 +140,7 @@ abstract contract ResourceMetering is Initializable { ...@@ -140,7 +140,7 @@ abstract contract ResourceMetering is Initializable {
// type(uint64).max * type(uint128).max < type(uint256).max // type(uint64).max * type(uint128).max < type(uint256).max
uint256 resourceCost; uint256 resourceCost;
unchecked { unchecked {
resourceCost = _amount * params.prevBaseFee; resourceCost = uint256(_amount) * uint256(params.prevBaseFee);
} }
// We currently charge for this ETH amount as an L1 gas burn, so we convert the ETH amount // We currently charge for this ETH amount as an L1 gas burn, so we convert the ETH amount
......
// SPDX-License-Identifier: MIT // SPDX-License-Identifier: MIT
pragma solidity 0.8.15; pragma solidity 0.8.15;
import { CommonTest } from "./CommonTest.t.sol"; import { Test } from "forge-std/Test.sol";
import { ResourceMetering } from "../L1/ResourceMetering.sol"; import { ResourceMetering } from "../L1/ResourceMetering.sol";
import { Proxy } from "../universal/Proxy.sol"; import { Proxy } from "../universal/Proxy.sol";
...@@ -15,14 +15,21 @@ contract MeterUser is ResourceMetering { ...@@ -15,14 +15,21 @@ contract MeterUser is ResourceMetering {
} }
function use(uint64 _amount) public metered(_amount) {} function use(uint64 _amount) public metered(_amount) {}
function set(uint128 _prevBaseFee, uint64 _prevBoughtGas, uint64 _prevBlockNum) public {
params = ResourceMetering.ResourceParams({
prevBaseFee: _prevBaseFee,
prevBoughtGas: _prevBoughtGas,
prevBlockNum: _prevBlockNum
});
}
} }
contract ResourceMetering_Test is CommonTest { contract ResourceMetering_Test is Test {
MeterUser internal meter; MeterUser internal meter;
uint64 initialBlockNum; uint64 initialBlockNum;
function setUp() public virtual override { function setUp() public {
super.setUp();
meter = new MeterUser(); meter = new MeterUser();
initialBlockNum = uint64(block.number); initialBlockNum = uint64(block.number);
} }
...@@ -109,6 +116,34 @@ contract ResourceMetering_Test is CommonTest { ...@@ -109,6 +116,34 @@ contract ResourceMetering_Test is CommonTest {
meter.use(target * elasticity + 1); meter.use(target * elasticity + 1);
} }
/**
* @notice The max resource limit should be able to be used when the L1
* deposit base fee is at its max value. This previously would
* revert because prevBaseFee is a uint128 and checked math when
* multiplying against a uint64 _amount can result in an overflow
* even though its assigning to a uint256. The values MUST be casted
* to uint256 when doing the multiplication to prevent overflows.
*/
function test_meter_useMaxWithMaxBaseFee_success() external {
uint128 _prevBaseFee = uint128(uint256(meter.MAXIMUM_BASE_FEE()));
uint64 _prevBoughtGas = 0;
uint64 _prevBlockNum = uint64(block.number);
meter.set({
_prevBaseFee: _prevBaseFee,
_prevBoughtGas: _prevBoughtGas,
_prevBlockNum: _prevBlockNum
});
(uint128 prevBaseFee, uint64 prevBoughtGas, uint64 prevBlockNum) = meter.params();
assertEq(prevBaseFee, _prevBaseFee);
assertEq(prevBoughtGas, _prevBoughtGas);
assertEq(prevBlockNum, _prevBlockNum);
uint64 gasRequested = uint64(uint256(meter.MAX_RESOURCE_LIMIT()));
meter.use(gasRequested);
}
// Demonstrates that the resource metering arithmetic can tolerate very large gaps between // Demonstrates that the resource metering arithmetic can tolerate very large gaps between
// deposits. // deposits.
function testFuzz_meter_largeBlockDiff_succeeds(uint64 _amount, uint256 _blockDiff) external { function testFuzz_meter_largeBlockDiff_succeeds(uint64 _amount, uint256 _blockDiff) external {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment