Commit ad0b4d12 authored by Maurelian's avatar Maurelian Committed by GitHub

Merge pull request #5527 from ethereum-optimism/fix/max-deposit-data

parents fad2f411 fb2adc42
This diff is collapsed.
This diff is collapsed.
...@@ -22,25 +22,24 @@ CrossDomainOwnable3_Test:test_transferOwnershipNoLocal_succeeds() (gas: 48610) ...@@ -22,25 +22,24 @@ CrossDomainOwnable3_Test:test_transferOwnershipNoLocal_succeeds() (gas: 48610)
CrossDomainOwnable3_Test:test_transferOwnership_noLocalZeroAddress_reverts() (gas: 12015) CrossDomainOwnable3_Test:test_transferOwnership_noLocalZeroAddress_reverts() (gas: 12015)
CrossDomainOwnable3_Test:test_transferOwnership_notOwner_reverts() (gas: 13437) CrossDomainOwnable3_Test:test_transferOwnership_notOwner_reverts() (gas: 13437)
CrossDomainOwnable3_Test:test_transferOwnership_zeroAddress_reverts() (gas: 12081) CrossDomainOwnable3_Test:test_transferOwnership_zeroAddress_reverts() (gas: 12081)
CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner_succeeds() (gas: 81559) CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner_succeeds() (gas: 81416)
CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10597) CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10597)
CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34883) CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34883)
DeployerWhitelist_Test:test_owner_succeeds() (gas: 7582) DeployerWhitelist_Test:test_owner_succeeds() (gas: 7582)
DeployerWhitelist_Test:test_storageSlots_succeeds() (gas: 33395) DeployerWhitelist_Test:test_storageSlots_succeeds() (gas: 33395)
DoS_Test:test_findMaxCalldataSize_allBitsSet_success() (gas: 19139333096) DoS_Test:test_findMaxCalldataSize_zeros_success() (gas: 2121077658)
DoS_Test:test_findMaxCalldataSize_zeros_success() (gas: 34646492848)
FeeVault_Test:test_constructor_succeeds() (gas: 10736) FeeVault_Test:test_constructor_succeeds() (gas: 10736)
FeeVault_Test:test_minWithdrawalAmount_succeeds() (gas: 10713) FeeVault_Test:test_minWithdrawalAmount_succeeds() (gas: 10713)
GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 352106) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 352135)
GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2950313) GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2950342)
GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 537885) GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 537914)
GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4050078) GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 4050107)
GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 439314) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 439343)
GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3485041) GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 3485070)
GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 40409) GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 40409)
GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 88513) GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 88513)
GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 75196) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 75225)
GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 75633) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 75662)
GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 169237) GasBenchMark_OptimismPortal:test_proveWithdrawalTransaction_benchmark() (gas: 169237)
GasPriceOracle_Test:test_baseFee_succeeds() (gas: 8325) GasPriceOracle_Test:test_baseFee_succeeds() (gas: 8325)
GasPriceOracle_Test:test_decimals_succeeds() (gas: 6167) GasPriceOracle_Test:test_decimals_succeeds() (gas: 6167)
...@@ -81,32 +80,32 @@ L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 74244) ...@@ -81,32 +80,32 @@ L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 74244)
L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 56518) L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 56518)
L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 12365) L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 12365)
L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 31063) L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 31063)
L1CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 390794) L1CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 390823)
L1CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 1666628) L1CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 1666686)
L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 84686) L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 84686)
L1CrossDomainMessenger_Test:test_xDomainSender_notSet_reverts() (gas: 24253) L1CrossDomainMessenger_Test:test_xDomainSender_notSet_reverts() (gas: 24253)
L1ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 52707) L1ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 52707)
L1ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 27310) L1ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 27310)
L1ERC721Bridge_Test:test_bridgeERC721To_succeeds() (gas: 445243) L1ERC721Bridge_Test:test_bridgeERC721To_succeeds() (gas: 445272)
L1ERC721Bridge_Test:test_bridgeERC721To_wrongOwner_reverts() (gas: 60934) L1ERC721Bridge_Test:test_bridgeERC721To_wrongOwner_reverts() (gas: 60934)
L1ERC721Bridge_Test:test_bridgeERC721_fromContract_reverts() (gas: 25666) L1ERC721Bridge_Test:test_bridgeERC721_fromContract_reverts() (gas: 25666)
L1ERC721Bridge_Test:test_bridgeERC721_localTokenZeroAddress_reverts() (gas: 50564) L1ERC721Bridge_Test:test_bridgeERC721_localTokenZeroAddress_reverts() (gas: 50564)
L1ERC721Bridge_Test:test_bridgeERC721_remoteTokenZeroAddress_reverts() (gas: 25124) L1ERC721Bridge_Test:test_bridgeERC721_remoteTokenZeroAddress_reverts() (gas: 25124)
L1ERC721Bridge_Test:test_bridgeERC721_succeeds() (gas: 442823) L1ERC721Bridge_Test:test_bridgeERC721_succeeds() (gas: 442852)
L1ERC721Bridge_Test:test_bridgeERC721_wrongOwner_reverts() (gas: 60830) L1ERC721Bridge_Test:test_bridgeERC721_wrongOwner_reverts() (gas: 60830)
L1ERC721Bridge_Test:test_constructor_succeeds() (gas: 10200) L1ERC721Bridge_Test:test_constructor_succeeds() (gas: 10200)
L1ERC721Bridge_Test:test_finalizeBridgeERC721_notEscrowed_reverts() (gas: 22119) L1ERC721Bridge_Test:test_finalizeBridgeERC721_notEscrowed_reverts() (gas: 22119)
L1ERC721Bridge_Test:test_finalizeBridgeERC721_notFromRemoteMessenger_reverts() (gas: 19797) L1ERC721Bridge_Test:test_finalizeBridgeERC721_notFromRemoteMessenger_reverts() (gas: 19797)
L1ERC721Bridge_Test:test_finalizeBridgeERC721_notViaLocalMessenger_reverts() (gas: 16049) L1ERC721Bridge_Test:test_finalizeBridgeERC721_notViaLocalMessenger_reverts() (gas: 16049)
L1ERC721Bridge_Test:test_finalizeBridgeERC721_selfToken_reverts() (gas: 17615) L1ERC721Bridge_Test:test_finalizeBridgeERC721_selfToken_reverts() (gas: 17615)
L1ERC721Bridge_Test:test_finalizeBridgeERC721_succeeds() (gas: 414335) L1ERC721Bridge_Test:test_finalizeBridgeERC721_succeeds() (gas: 414364)
L1StandardBridge_BridgeETHTo_Test:test_bridgeETHTo_succeeds() (gas: 510387) L1StandardBridge_BridgeETHTo_Test:test_bridgeETHTo_succeeds() (gas: 510416)
L1StandardBridge_BridgeETH_Test:test_bridgeETH_succeeds() (gas: 497608) L1StandardBridge_BridgeETH_Test:test_bridgeETH_succeeds() (gas: 497637)
L1StandardBridge_DepositERC20To_Test:test_depositERC20To_succeeds() (gas: 715691) L1StandardBridge_DepositERC20To_Test:test_depositERC20To_succeeds() (gas: 715720)
L1StandardBridge_DepositERC20_Test:test_depositERC20_succeeds() (gas: 713392) L1StandardBridge_DepositERC20_Test:test_depositERC20_succeeds() (gas: 713421)
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: 510464) L1StandardBridge_DepositETHTo_Test:test_depositETHTo_succeeds() (gas: 510493)
L1StandardBridge_DepositETH_Test:test_depositETH_succeeds() (gas: 497702) L1StandardBridge_DepositETH_Test:test_depositETH_succeeds() (gas: 497731)
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: 34204) L1StandardBridge_FinalizeBridgeETH_TestFail:test_finalizeBridgeETH_incorrectValue_reverts() (gas: 34204)
...@@ -118,7 +117,7 @@ L1StandardBridge_FinalizeERC20Withdrawal_TestFail:test_finalizeERC20Withdrawal_n ...@@ -118,7 +117,7 @@ L1StandardBridge_FinalizeERC20Withdrawal_TestFail:test_finalizeERC20Withdrawal_n
L1StandardBridge_FinalizeETHWithdrawal_Test:test_finalizeETHWithdrawal_succeeds() (gas: 61722) L1StandardBridge_FinalizeETHWithdrawal_Test:test_finalizeETHWithdrawal_succeeds() (gas: 61722)
L1StandardBridge_Getter_Test:test_getters_succeeds() (gas: 32173) L1StandardBridge_Getter_Test:test_getters_succeeds() (gas: 32173)
L1StandardBridge_Initialize_Test:test_initialize_succeeds() (gas: 22050) L1StandardBridge_Initialize_Test:test_initialize_succeeds() (gas: 22050)
L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 610690) L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 610719)
L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8434) L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8434)
L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 163755) L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 163755)
L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 48938) L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 48938)
...@@ -284,23 +283,24 @@ OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayPro ...@@ -284,23 +283,24 @@ OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayPro
OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_succeeds() (gas: 180486) OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_succeeds() (gas: 180486)
OptimismPortal_Test:test_constructor_succeeds() (gas: 19402) OptimismPortal_Test:test_constructor_succeeds() (gas: 19402)
OptimismPortal_Test:test_depositTransaction_contractCreation_reverts() (gas: 14342) OptimismPortal_Test:test_depositTransaction_contractCreation_reverts() (gas: 14342)
OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract_succeeds() (gas: 76869) OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract_succeeds() (gas: 76726)
OptimismPortal_Test:test_depositTransaction_createWithZeroValueForEOA_succeeds() (gas: 77235) OptimismPortal_Test:test_depositTransaction_createWithZeroValueForEOA_succeeds() (gas: 77264)
OptimismPortal_Test:test_depositTransaction_noValueContract_succeeds() (gas: 76865) OptimismPortal_Test:test_depositTransaction_largeData_reverts() (gas: 512221)
OptimismPortal_Test:test_depositTransaction_noValueEOA_succeeds() (gas: 77232) OptimismPortal_Test:test_depositTransaction_noValueContract_succeeds() (gas: 76916)
OptimismPortal_Test:test_depositTransaction_noValueEOA_succeeds() (gas: 77261)
OptimismPortal_Test:test_depositTransaction_smallGasLimit_reverts() (gas: 14556) OptimismPortal_Test:test_depositTransaction_smallGasLimit_reverts() (gas: 14556)
OptimismPortal_Test:test_depositTransaction_withEthValueAndContractContractCreation_succeeds() (gas: 83893) OptimismPortal_Test:test_depositTransaction_withEthValueAndContractContractCreation_succeeds() (gas: 83750)
OptimismPortal_Test:test_depositTransaction_withEthValueAndEOAContractCreation_succeeds() (gas: 75877) OptimismPortal_Test:test_depositTransaction_withEthValueAndEOAContractCreation_succeeds() (gas: 75906)
OptimismPortal_Test:test_depositTransaction_withEthValueFromContract_succeeds() (gas: 83573) OptimismPortal_Test:test_depositTransaction_withEthValueFromContract_succeeds() (gas: 83602)
OptimismPortal_Test:test_depositTransaction_withEthValueFromEOA_succeeds() (gas: 84167) OptimismPortal_Test:test_depositTransaction_withEthValueFromEOA_succeeds() (gas: 84218)
OptimismPortal_Test:test_isOutputFinalized_succeeds() (gas: 121831) OptimismPortal_Test:test_isOutputFinalized_succeeds() (gas: 121831)
OptimismPortal_Test:test_minimumGasLimit_succeeds() (gas: 17650) OptimismPortal_Test:test_minimumGasLimit_succeeds() (gas: 17650)
OptimismPortal_Test:test_pause_onlyGuardian_reverts() (gas: 22196) OptimismPortal_Test:test_pause_onlyGuardian_reverts() (gas: 22196)
OptimismPortal_Test:test_pause_succeeds() (gas: 42175) OptimismPortal_Test:test_pause_succeeds() (gas: 42175)
OptimismPortal_Test:test_receive_succeeds() (gas: 127484) OptimismPortal_Test:test_receive_succeeds() (gas: 127513)
OptimismPortal_Test:test_simple_isOutputFinalized_succeeds() (gas: 32949) OptimismPortal_Test:test_simple_isOutputFinalized_succeeds() (gas: 32971)
OptimismPortal_Test:test_unpause_onlyGuardian_reverts() (gas: 46076) OptimismPortal_Test:test_unpause_onlyGuardian_reverts() (gas: 46098)
OptimismPortal_Test:test_unpause_succeeds() (gas: 31738) OptimismPortal_Test:test_unpause_succeeds() (gas: 31756)
ProxyAdmin_Test:test_chugsplashChangeProxyAdmin_succeeds() (gas: 35586) ProxyAdmin_Test:test_chugsplashChangeProxyAdmin_succeeds() (gas: 35586)
ProxyAdmin_Test:test_chugsplashGetProxyAdmin_succeeds() (gas: 15675) ProxyAdmin_Test:test_chugsplashGetProxyAdmin_succeeds() (gas: 15675)
ProxyAdmin_Test:test_chugsplashGetProxyImplementation_succeeds() (gas: 51084) ProxyAdmin_Test:test_chugsplashGetProxyImplementation_succeeds() (gas: 51084)
......
...@@ -189,7 +189,9 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -189,7 +189,9 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
/** /**
* @notice Computes the minimum gas limit for a deposit. The minimum gas limit * @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 * linearly increases based on the size of the calldata. This is to prevent
* users from creating L2 resource usage without paying for it. * users from creating L2 resource usage without paying for it. This function
* can be used when interacting with the portal to ensure forwards compatibility.
*
*/ */
function minimumGasLimit(uint64 _byteCount) public pure returns (uint64) { function minimumGasLimit(uint64 _byteCount) public pure returns (uint64) {
return _byteCount * 16 + 21000; return _byteCount * 16 + 21000;
...@@ -445,12 +447,19 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver { ...@@ -445,12 +447,19 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
); );
} }
// Prevent depositing transactions that have too small of a gas limit. // Prevent depositing transactions that have too small of a gas limit. Users should pay
// more for more resource usage.
require( require(
_gasLimit >= minimumGasLimit(uint64(_data.length)), _gasLimit >= minimumGasLimit(uint64(_data.length)),
"OptimismPortal: gas limit too small" "OptimismPortal: gas limit too small"
); );
// Prevent the creation of deposit transactions that have too much calldata. This gives an
// upper limit on the size of unsafe blocks over the p2p network. 120kb is chosen to ensure
// that the transaction can fit into the p2p network policy of 128kb even though deposit
// transactions are not gossipped over the p2p network.
require(_data.length <= 120_000, "OptimismPortal: data too large");
// 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) {
......
...@@ -110,6 +110,23 @@ contract OptimismPortal_Test is Portal_Initializer { ...@@ -110,6 +110,23 @@ contract OptimismPortal_Test is Portal_Initializer {
op.depositTransaction(address(1), 1, 0, true, hex""); op.depositTransaction(address(1), 1, 0, true, hex"");
} }
/**
* @notice Prevent deposits from being too large to have a sane upper bound
* on unsafe blocks sent over the p2p network.
*/
function test_depositTransaction_largeData_reverts() external {
uint256 size = 120_001;
uint64 gasLimit = op.minimumGasLimit(uint64(size));
vm.expectRevert("OptimismPortal: data too large");
op.depositTransaction({
_to: address(0),
_value: 0,
_gasLimit: gasLimit,
_isCreation: false,
_data: new bytes(size)
});
}
/** /**
* @notice Prevent gasless deposits from being force processed in L2 by * @notice Prevent gasless deposits from being force processed in L2 by
* ensuring that they have a large enough gas limit set. * ensuring that they have a large enough gas limit set.
......
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