Commit ee5c0ed1 authored by Mark Tyneway's avatar Mark Tyneway

contracts-bedrock: update portal min gas limit

Prevent users from sending cross domain messages with too small
of a gas limit. Users must pay for resources they consume. The
L2 network must store deposit transactions in their history and
they are also included in the p2p gossip of unsafe blocks.
A linearly increasing requirement based on the calldata size
is introduced. A base of 21k minimum gas limit is required when
there is no gas limit, the formula `calldata.length * 16 + 21000`
is used to enforce a min gas limit.

The larger the gaslimit, the more ether that is burnt on L1.
parent 3006ef0c
This source diff could not be displayed because it is too large. You can view the blob instead.
This diff is collapsed.
This diff is collapsed.
......@@ -140,7 +140,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
}
/**
* @custom:semver 1.4.0
* @custom:semver 1.5.0
*
* @param _l2Oracle Address of the L2OutputOracle contract.
* @param _guardian Address that can pause deposits and withdrawals.
......@@ -152,7 +152,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
address _guardian,
bool _paused,
SystemConfig _config
) Semver(1, 4, 0) {
) Semver(1, 5, 0) {
L2_ORACLE = _l2Oracle;
GUARDIAN = _guardian;
SYSTEM_CONFIG = _config;
......@@ -186,6 +186,15 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
emit Unpaused(msg.sender);
}
/**
* @notice Computes the minimum gas limit for a deposit. The minimum gas limit
* linearly increases based on the size of the calldata. This is to prevent
* users from creating L2 resource usage without paying for it.
*/
function minimumGasLimit(uint64 _byteCount) public pure returns (uint64) {
return _byteCount * 16 + 21000;
}
/**
* @notice Accepts value so that users can send ETH directly to this contract and have the
* funds be deposited to their address on L2. This is intended as a convenience
......@@ -437,7 +446,10 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
}
// Prevent depositing transactions that have too small of a gas limit.
require(_gasLimit >= 21_000, "OptimismPortal: gas limit must cover instrinsic gas cost");
require(
_gasLimit >= minimumGasLimit(uint64(_data.length)),
"OptimismPortal: gas limit too small"
);
// Transform the from-address to its alias if the caller is a contract.
address from = msg.sender;
......
......@@ -56,7 +56,7 @@ contract CrossDomainOwnableThroughPortal_Test is Portal_Initializer {
op.depositTransaction({
_to: address(setter),
_value: 0,
_gasLimit: 21_000,
_gasLimit: 30_000,
_isCreation: false,
_data: abi.encodeWithSelector(XDomainSetter.set.selector, 1)
});
......
......@@ -115,7 +115,7 @@ contract OptimismPortal_Test is Portal_Initializer {
* ensuring that they have a large enough gas limit set.
*/
function test_depositTransaction_smallGasLimit_reverts() external {
vm.expectRevert("OptimismPortal: gas limit must cover instrinsic gas cost");
vm.expectRevert("OptimismPortal: gas limit too small");
op.depositTransaction({
_to: address(1),
_value: 0,
......@@ -125,6 +125,40 @@ contract OptimismPortal_Test is Portal_Initializer {
});
}
/**
* @notice Fuzz for too small of gas limits
*/
function testFuzz_depositTransaction_smallGasLimit_succeeds(
bytes memory _data,
bool _shouldFail
) external {
vm.assume(_data.length <= type(uint64).max);
uint64 gasLimit = op.minimumGasLimit(uint64(_data.length));
if (_shouldFail) {
gasLimit = uint64(bound(gasLimit, 0, gasLimit - 1));
vm.expectRevert("OptimismPortal: gas limit too small");
}
op.depositTransaction({
_to: address(0x40),
_value: 0,
_gasLimit: gasLimit,
_isCreation: false,
_data: _data
});
}
/**
* @notice Ensure that the 0 calldata case is covered and there is a linearly
* increasing gas limit for larger calldata sizes.
*/
function test_minimumGasLimit_succeeds() external {
assertEq(op.minimumGasLimit(0), 21_000);
assertTrue(op.minimumGasLimit(2) > op.minimumGasLimit(1));
assertTrue(op.minimumGasLimit(3) > op.minimumGasLimit(2));
}
// Test: depositTransaction should emit the correct log when an EOA deposits a tx with 0 value
function test_depositTransaction_noValueEOA_succeeds() external {
// EOA emulation
......
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