Commit ec8e9b0c authored by mergify[bot]'s avatar mergify[bot] Committed by GitHub

Merge pull request #4912 from ethereum-optimism/fix/deposit-tx-gaslimit

contracts-bedrock: prevent deposits with too small gaslimit
parents 2c892d5b 2a2e1d2d
This diff is collapsed.
This diff is collapsed.
GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 261344) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 261376)
GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 75851) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 75883)
GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 348296) GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 348328)
GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 112728) GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 112760)
GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 348318) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 348350)
GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 112749) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 112781)
GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 40853) GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 40853)
GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 88513) GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 88513)
GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 74998) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 75030)
GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 36156) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 36188)
GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 167187) GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 167187)
Bytes_slice_Test:test_slice_acrossMultipleWords_works() (gas: 9357) Bytes_slice_Test:test_slice_acrossMultipleWords_works() (gas: 9357)
Bytes_slice_Test:test_slice_acrossWords_works() (gas: 1396) Bytes_slice_Test:test_slice_acrossWords_works() (gas: 1396)
...@@ -17,7 +17,7 @@ Bytes_toNibbles_Test:test_toNibbles_expectedResult128Bytes_works() (gas: 129830) ...@@ -17,7 +17,7 @@ Bytes_toNibbles_Test:test_toNibbles_expectedResult128Bytes_works() (gas: 129830)
Bytes_toNibbles_Test:test_toNibbles_expectedResult5Bytes_works() (gas: 6088) Bytes_toNibbles_Test:test_toNibbles_expectedResult5Bytes_works() (gas: 6088)
Bytes_toNibbles_Test:test_toNibbles_zeroLengthInput_works() (gas: 944) Bytes_toNibbles_Test:test_toNibbles_zeroLengthInput_works() (gas: 944)
CrossDomainMessenger_BaseGas_Test:test_baseGas_succeeds() (gas: 20120) CrossDomainMessenger_BaseGas_Test:test_baseGas_succeeds() (gas: 20120)
CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner_succeeds() (gas: 61882) CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner_succeeds() (gas: 72436)
CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10530) CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10530)
CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34861) CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34861)
CrossDomainOwnable2_Test:test_onlyOwner_notMessenger_reverts() (gas: 8416) CrossDomainOwnable2_Test:test_onlyOwner_notMessenger_reverts() (gas: 8416)
...@@ -71,34 +71,34 @@ L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 73558) ...@@ -71,34 +71,34 @@ L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 73558)
L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 65827) L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 65827)
L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 19500) L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 19500)
L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 38220) L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 38220)
L1CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 299678) L1CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 299710)
L1CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 1490502) L1CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 1490566)
L1CrossDomainMessenger_Test:test_unpause_callerIsNotOwner_reverts() (gas: 24516) L1CrossDomainMessenger_Test:test_unpause_callerIsNotOwner_reverts() (gas: 24516)
L1CrossDomainMessenger_Test:test_unpause_succeeds() (gas: 45185) L1CrossDomainMessenger_Test:test_unpause_succeeds() (gas: 45185)
L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 84020) L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 84020)
L1CrossDomainMessenger_Test:test_xDomainSender_notSet_reverts() (gas: 24274) L1CrossDomainMessenger_Test:test_xDomainSender_notSet_reverts() (gas: 24274)
L1ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 52730) L1ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 52730)
L1ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 27332) L1ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 27332)
L1ERC721Bridge_Test:test_bridgeERC721To_succeeds() (gas: 354722) L1ERC721Bridge_Test:test_bridgeERC721To_succeeds() (gas: 354754)
L1ERC721Bridge_Test:test_bridgeERC721To_wrongOwner_reverts() (gas: 60956) L1ERC721Bridge_Test:test_bridgeERC721To_wrongOwner_reverts() (gas: 60956)
L1ERC721Bridge_Test:test_bridgeERC721_fromContract_reverts() (gas: 25689) L1ERC721Bridge_Test:test_bridgeERC721_fromContract_reverts() (gas: 25689)
L1ERC721Bridge_Test:test_bridgeERC721_localTokenZeroAddress_reverts() (gas: 50565) L1ERC721Bridge_Test:test_bridgeERC721_localTokenZeroAddress_reverts() (gas: 50565)
L1ERC721Bridge_Test:test_bridgeERC721_remoteTokenZeroAddress_reverts() (gas: 25167) L1ERC721Bridge_Test:test_bridgeERC721_remoteTokenZeroAddress_reverts() (gas: 25167)
L1ERC721Bridge_Test:test_bridgeERC721_succeeds() (gas: 352302) L1ERC721Bridge_Test:test_bridgeERC721_succeeds() (gas: 352334)
L1ERC721Bridge_Test:test_bridgeERC721_wrongOwner_reverts() (gas: 60786) L1ERC721Bridge_Test:test_bridgeERC721_wrongOwner_reverts() (gas: 60786)
L1ERC721Bridge_Test:test_constructor_succeeds() (gas: 10156) L1ERC721Bridge_Test:test_constructor_succeeds() (gas: 10156)
L1ERC721Bridge_Test:test_finalizeBridgeERC721_notEscrowed_reverts() (gas: 22075) L1ERC721Bridge_Test:test_finalizeBridgeERC721_notEscrowed_reverts() (gas: 22075)
L1ERC721Bridge_Test:test_finalizeBridgeERC721_notFromRemoteMessenger_reverts() (gas: 19820) L1ERC721Bridge_Test:test_finalizeBridgeERC721_notFromRemoteMessenger_reverts() (gas: 19820)
L1ERC721Bridge_Test:test_finalizeBridgeERC721_notViaLocalMessenger_reverts() (gas: 16093) L1ERC721Bridge_Test:test_finalizeBridgeERC721_notViaLocalMessenger_reverts() (gas: 16093)
L1ERC721Bridge_Test:test_finalizeBridgeERC721_selfToken_reverts() (gas: 17593) L1ERC721Bridge_Test:test_finalizeBridgeERC721_selfToken_reverts() (gas: 17593)
L1ERC721Bridge_Test:test_finalizeBridgeERC721_succeeds() (gas: 323814) L1ERC721Bridge_Test:test_finalizeBridgeERC721_succeeds() (gas: 323846)
L1StandardBridge_BridgeETHTo_Test:test_bridgeETHTo_succeeds() (gas: 419609) L1StandardBridge_BridgeETHTo_Test:test_bridgeETHTo_succeeds() (gas: 419641)
L1StandardBridge_BridgeETH_Test:test_bridgeETH_succeeds() (gas: 406785) L1StandardBridge_BridgeETH_Test:test_bridgeETH_succeeds() (gas: 406817)
L1StandardBridge_DepositERC20To_Test:test_depositERC20To_succeeds() (gas: 625056) L1StandardBridge_DepositERC20To_Test:test_depositERC20To_succeeds() (gas: 624916)
L1StandardBridge_DepositERC20_Test:test_depositERC20_succeeds() (gas: 622735) L1StandardBridge_DepositERC20_Test:test_depositERC20_succeeds() (gas: 622595)
L1StandardBridge_DepositERC20_TestFail:test_depositERC20_notEoa_reverts() (gas: 22320) L1StandardBridge_DepositERC20_TestFail:test_depositERC20_notEoa_reverts() (gas: 22320)
L1StandardBridge_DepositETHTo_Test:test_depositETHTo_succeeds() (gas: 419642) L1StandardBridge_DepositETHTo_Test:test_depositETHTo_succeeds() (gas: 419674)
L1StandardBridge_DepositETH_Test:test_depositETH_succeeds() (gas: 406880) L1StandardBridge_DepositETH_Test:test_depositETH_succeeds() (gas: 406912)
L1StandardBridge_DepositETH_TestFail:test_depositETH_notEoa_reverts() (gas: 40780) L1StandardBridge_DepositETH_TestFail:test_depositETH_notEoa_reverts() (gas: 40780)
L1StandardBridge_FinalizeBridgeETH_Test:test_finalizeBridgeETH_succeeds() (gas: 51674) L1StandardBridge_FinalizeBridgeETH_Test:test_finalizeBridgeETH_succeeds() (gas: 51674)
L1StandardBridge_FinalizeBridgeETH_TestFail:test_finalizeBridgeETH_incorrectValue_reverts() (gas: 34207) L1StandardBridge_FinalizeBridgeETH_TestFail:test_finalizeBridgeETH_incorrectValue_reverts() (gas: 34207)
...@@ -110,7 +110,7 @@ L1StandardBridge_FinalizeERC20Withdrawal_TestFail:test_finalizeERC20Withdrawal_n ...@@ -110,7 +110,7 @@ L1StandardBridge_FinalizeERC20Withdrawal_TestFail:test_finalizeERC20Withdrawal_n
L1StandardBridge_FinalizeETHWithdrawal_Test:test_finalizeETHWithdrawal_succeeds() (gas: 62166) L1StandardBridge_FinalizeETHWithdrawal_Test:test_finalizeETHWithdrawal_succeeds() (gas: 62166)
L1StandardBridge_Getter_Test:test_getters_succeeds() (gas: 32151) L1StandardBridge_Getter_Test:test_getters_succeeds() (gas: 32151)
L1StandardBridge_Initialize_Test:test_initialize_succeeds() (gas: 22005) L1StandardBridge_Initialize_Test:test_initialize_succeeds() (gas: 22005)
L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 520338) L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 520198)
L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8389) L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8389)
L2CrossDomainMessenger_Test:test_pause_notOwner_reverts() (gas: 10837) L2CrossDomainMessenger_Test:test_pause_notOwner_reverts() (gas: 10837)
L2CrossDomainMessenger_Test:test_pause_succeeds() (gas: 31846) L2CrossDomainMessenger_Test:test_pause_succeeds() (gas: 31846)
...@@ -273,17 +273,18 @@ OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayPro ...@@ -273,17 +273,18 @@ OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayPro
OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_succeeds() (gas: 178317) OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_succeeds() (gas: 178317)
OptimismPortal_Test:test_constructor_succeeds() (gas: 17277) OptimismPortal_Test:test_constructor_succeeds() (gas: 17277)
OptimismPortal_Test:test_depositTransaction_contractCreation_reverts() (gas: 14273) OptimismPortal_Test:test_depositTransaction_contractCreation_reverts() (gas: 14273)
OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract_succeeds() (gas: 76662) OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract_succeeds() (gas: 76717)
OptimismPortal_Test:test_depositTransaction_createWithZeroValueForEOA_succeeds() (gas: 77029) OptimismPortal_Test:test_depositTransaction_createWithZeroValueForEOA_succeeds() (gas: 77061)
OptimismPortal_Test:test_depositTransaction_noValueContract_succeeds() (gas: 76681) OptimismPortal_Test:test_depositTransaction_noValueContract_succeeds() (gas: 76691)
OptimismPortal_Test:test_depositTransaction_noValueEOA_succeeds() (gas: 77198) OptimismPortal_Test:test_depositTransaction_noValueEOA_succeeds() (gas: 77058)
OptimismPortal_Test:test_depositTransaction_withEthValueAndContractContractCreation_succeeds() (gas: 83664) OptimismPortal_Test:test_depositTransaction_smallGasLimit_reverts() (gas: 14241)
OptimismPortal_Test:test_depositTransaction_withEthValueAndEOAContractCreation_succeeds() (gas: 75871) OptimismPortal_Test:test_depositTransaction_withEthValueAndContractContractCreation_succeeds() (gas: 83719)
OptimismPortal_Test:test_depositTransaction_withEthValueFromContract_succeeds() (gas: 83389) OptimismPortal_Test:test_depositTransaction_withEthValueAndEOAContractCreation_succeeds() (gas: 75903)
OptimismPortal_Test:test_depositTransaction_withEthValueFromEOA_succeeds() (gas: 84133) OptimismPortal_Test:test_depositTransaction_withEthValueFromContract_succeeds() (gas: 83421)
OptimismPortal_Test:test_depositTransaction_withEthValueFromEOA_succeeds() (gas: 84037)
OptimismPortal_Test:test_isOutputFinalized_succeeds() (gas: 119474) OptimismPortal_Test:test_isOutputFinalized_succeeds() (gas: 119474)
OptimismPortal_Test:test_receive_succeeds() (gas: 127522) OptimismPortal_Test:test_receive_succeeds() (gas: 127554)
OptimismPortal_Test:test_simple_isOutputFinalized_succeeds() (gas: 24165) OptimismPortal_Test:test_simple_isOutputFinalized_succeeds() (gas: 24188)
Proxy_Test:test_delegatesToImpl_succeeds() (gas: 45184) Proxy_Test:test_delegatesToImpl_succeeds() (gas: 45184)
Proxy_Test:test_implementationKey_succeeds() (gas: 20886) Proxy_Test:test_implementationKey_succeeds() (gas: 20886)
Proxy_Test:test_implementation_isZeroAddress_reverts() (gas: 47648) Proxy_Test:test_implementation_isZeroAddress_reverts() (gas: 47648)
......
...@@ -371,6 +371,9 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -371,6 +371,9 @@ 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");
// Transform the from-address to its alias if the caller is a contract. // Transform the from-address to its alias if the caller is a contract.
address from = msg.sender; address from = msg.sender;
if (msg.sender != tx.origin) { if (msg.sender != tx.origin) {
......
...@@ -52,13 +52,13 @@ contract CrossDomainOwnableThroughPortal_Test is Portal_Initializer { ...@@ -52,13 +52,13 @@ contract CrossDomainOwnableThroughPortal_Test is Portal_Initializer {
vm.recordLogs(); vm.recordLogs();
vm.prank(alice); vm.prank(alice);
op.depositTransaction( op.depositTransaction({
address(setter), _to: address(setter),
0, _value: 0,
10000, _gasLimit: 21_000,
false, _isCreation: false,
abi.encodeWithSelector(XDomainSetter.set.selector, 1) _data: abi.encodeWithSelector(XDomainSetter.set.selector, 1)
); });
// Simulate the operation of the `op-node` by parsing data // Simulate the operation of the `op-node` by parsing data
// from logs // from logs
......
...@@ -37,6 +37,21 @@ contract OptimismPortal_Test is Portal_Initializer { ...@@ -37,6 +37,21 @@ contract OptimismPortal_Test is Portal_Initializer {
op.depositTransaction(address(1), 1, 0, true, hex""); op.depositTransaction(address(1), 1, 0, true, hex"");
} }
/**
* @notice Prevent gasless deposits from being force processed in L2 by
* 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");
op.depositTransaction({
_to: address(1),
_value: 0,
_gasLimit: 0,
_isCreation: false,
_data: hex""
});
}
// Test: depositTransaction should emit the correct log when an EOA deposits a tx with 0 value // Test: depositTransaction should emit the correct log when an EOA deposits a tx with 0 value
function test_depositTransaction_noValueEOA_succeeds() external { function test_depositTransaction_noValueEOA_succeeds() external {
// EOA emulation // EOA emulation
......
...@@ -264,7 +264,7 @@ transaction are determined by the corresponding `TransactionDeposited` event emi ...@@ -264,7 +264,7 @@ transaction are determined by the corresponding `TransactionDeposited` event emi
- In case of a contract creation (cf. `isCreation`), this address is always zero. - In case of a contract creation (cf. `isCreation`), this address is always zero.
3. `mint` is set to the emitted value. 3. `mint` is set to the emitted value.
4. `value` is set to the emitted value. 4. `value` is set to the emitted value.
5. `gaslimit` is unchanged from the emitted value. 5. `gaslimit` is unchanged from the emitted value. It must be at least 21000.
6. `isCreation` is set to `true` if the transaction is a contract creation, `false` otherwise. 6. `isCreation` is set to `true` if the transaction is a contract creation, `false` otherwise.
7. `data` is unchanged from the emitted value. Depending on the value of `isCreation` it is handled 7. `data` is unchanged from the emitted value. Depending on the value of `isCreation` it is handled
as either calldata or contract initialization code. as either calldata or contract initialization code.
......
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