diff --git a/.changeset/wild-months-matter.md b/.changeset/wild-months-matter.md new file mode 100644 index 0000000000000000000000000000000000000000..aa4c01a6e5c8e2966718eec07054bf730a61f9a7 --- /dev/null +++ b/.changeset/wild-months-matter.md @@ -0,0 +1,5 @@ +--- +'@eth-optimism/contracts': patch +--- + +Token gateways pass additional information: sender and arbitrary data. diff --git a/integration-tests/test/native-eth.spec.ts b/integration-tests/test/native-eth.spec.ts index fc11c82c8be6bed84eee9db84af7f75bd4664339..f152d0a85c139f01a5d9dab1173c3d6ec1510161 100644 --- a/integration-tests/test/native-eth.spec.ts +++ b/integration-tests/test/native-eth.spec.ts @@ -5,6 +5,11 @@ import { Direction } from './shared/watcher-utils' import { PROXY_SEQUENCER_ENTRYPOINT_ADDRESS } from './shared/utils' import { OptimismEnv } from './shared/env' +const DEFAULT_TEST_GAS_L1 = 230_000 +const DEFAULT_TEST_GAS_L2 = 825_000 +// TX size enforced by CTC: +const MAX_ROLLUP_TX_SIZE = 50_000 + describe('Native ETH Integration Tests', async () => { let env: OptimismEnv let l1Bob: Wallet @@ -50,8 +55,8 @@ describe('Native ETH Integration Tests', async () => { it('Should estimate gas for ETH withdraw', async () => { const amount = utils.parseEther('0.5') - const gas = await env.ovmEth.estimateGas.withdraw(amount) - expect(gas).to.be.deep.eq(BigNumber.from(61400489396)) + const gas = await env.ovmEth.estimateGas.withdraw(amount, 0, '0xFFFF') + expect(gas).to.be.deep.eq(BigNumber.from(0x0f5203e9bf)) }) }) @@ -59,7 +64,10 @@ describe('Native ETH Integration Tests', async () => { const depositAmount = 10 const preBalances = await getBalances(env) const { tx, receipt } = await env.waitForXDomainTransaction( - env.gateway.deposit({ value: depositAmount }), + env.gateway.deposit(DEFAULT_TEST_GAS_L2, '0xFFFF', { + value: depositAmount, + gasLimit: DEFAULT_TEST_GAS_L1, + }), Direction.L1ToL2 ) @@ -81,8 +89,9 @@ describe('Native ETH Integration Tests', async () => { const depositAmount = 10 const preBalances = await getBalances(env) const depositReceipts = await env.waitForXDomainTransaction( - env.gateway.depositTo(l2Bob.address, { + env.gateway.depositTo(l2Bob.address, DEFAULT_TEST_GAS_L2, '0xFFFF', { value: depositAmount, + gasLimit: DEFAULT_TEST_GAS_L1, }), Direction.L1ToL2 ) @@ -102,6 +111,50 @@ describe('Native ETH Integration Tests', async () => { ) }) + it('deposit passes with a large data argument', async () => { + const ASSUMED_L2_GAS_LIMIT = 8_000_000 + const depositAmount = 10 + const preBalances = await getBalances(env) + + // Set data length slightly less than MAX_ROLLUP_TX_SIZE + // to allow for encoding and other arguments + const data = `0x` + 'ab'.repeat(MAX_ROLLUP_TX_SIZE - 500) + const { tx, receipt } = await env.waitForXDomainTransaction( + env.gateway.deposit(ASSUMED_L2_GAS_LIMIT, data, { + value: depositAmount, + gasLimit: 4_000_000, + }), + Direction.L1ToL2 + ) + + const l1FeePaid = receipt.gasUsed.mul(tx.gasPrice) + const postBalances = await getBalances(env) + + expect(postBalances.l1GatewayBalance).to.deep.eq( + preBalances.l1GatewayBalance.add(depositAmount) + ) + expect(postBalances.l2UserBalance).to.deep.eq( + preBalances.l2UserBalance.add(depositAmount) + ) + expect(postBalances.l1UserBalance).to.deep.eq( + preBalances.l1UserBalance.sub(l1FeePaid.add(depositAmount)) + ) + }) + + it('deposit fails with a TOO large data argument', async () => { + const depositAmount = 10 + + const data = `0x` + 'ab'.repeat(MAX_ROLLUP_TX_SIZE + 1) + await expect( + env.gateway.deposit(DEFAULT_TEST_GAS_L2, data, { + value: depositAmount, + gasLimit: 4_000_000, + }) + ).to.be.revertedWith( + 'Transaction data size exceeds maximum for rollup transaction.' + ) + }) + it('withdraw', async () => { const withdrawAmount = BigNumber.from(3) const preBalances = await getBalances(env) @@ -111,7 +164,7 @@ describe('Native ETH Integration Tests', async () => { ) const receipts = await env.waitForXDomainTransaction( - env.ovmEth.withdraw(withdrawAmount), + env.ovmEth.withdraw(withdrawAmount, DEFAULT_TEST_GAS_L1, '0xFFFF'), Direction.L2ToL1 ) const fee = receipts.tx.gasLimit.mul(receipts.tx.gasPrice) @@ -140,7 +193,12 @@ describe('Native ETH Integration Tests', async () => { ) const receipts = await env.waitForXDomainTransaction( - env.ovmEth.withdrawTo(l1Bob.address, withdrawAmount), + env.ovmEth.withdrawTo( + l1Bob.address, + withdrawAmount, + DEFAULT_TEST_GAS_L1, + '0xFFFF' + ), Direction.L2ToL1 ) const fee = receipts.tx.gasLimit.mul(receipts.tx.gasPrice) @@ -162,8 +220,9 @@ describe('Native ETH Integration Tests', async () => { // 1. deposit const amount = utils.parseEther('1') await env.waitForXDomainTransaction( - env.gateway.deposit({ + env.gateway.deposit(DEFAULT_TEST_GAS_L2, '0xFFFF', { value: amount, + gasLimit: DEFAULT_TEST_GAS_L1, }), Direction.L1ToL2 ) @@ -180,7 +239,9 @@ describe('Native ETH Integration Tests', async () => { // 3. do withdrawal const withdrawnAmount = utils.parseEther('0.95') const receipts = await env.waitForXDomainTransaction( - env.ovmEth.connect(other).withdraw(withdrawnAmount), + env.ovmEth + .connect(other) + .withdraw(withdrawnAmount, DEFAULT_TEST_GAS_L1, '0xFFFF'), Direction.L2ToL1 ) diff --git a/integration-tests/test/shared/utils.ts b/integration-tests/test/shared/utils.ts index cded6058cf0077da3a86b22ebb870e2c2ba842fe..33f69c6b63233fa4cbfacc2d136ccde801ac3ee9 100644 --- a/integration-tests/test/shared/utils.ts +++ b/integration-tests/test/shared/utils.ts @@ -101,8 +101,9 @@ export const fundUser = async ( ) => { const value = BigNumber.from(amount) const tx = recipient - ? gateway.depositTo(recipient, { value }) - : gateway.deposit({ value }) + ? gateway.depositTo(recipient, 1_000_000, '0xFFFF', { value }) + : gateway.deposit(1_000_000, '0xFFFF', { value }) + await waitForXDomainTransaction(watcher, tx, Direction.L1ToL2) } diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/Abs_L1TokenGateway.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/Abs_L1TokenGateway.sol index db4ba0bd9aa819c12853a29ceb559b96ec7dd8e6..a5e147bf833a9358380a98e27ea2e13106f0e773 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/Abs_L1TokenGateway.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/Abs_L1TokenGateway.sol @@ -52,13 +52,9 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab * Overridable Accounting logic * ********************************/ - // Default gas value which can be overridden if more complex logic runs on L2. - uint32 internal constant DEFAULT_FINALIZE_DEPOSIT_L2_GAS = 1200000; - /** * @dev Core logic to be performed when a withdrawal is finalized on L1. * In most cases, this will simply send locked funds to the withdrawer. - * * param _to Address being withdrawn to. * param _amount Amount being withdrawn. */ @@ -75,7 +71,6 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab /** * @dev Core logic to be performed when a deposit is initiated on L1. * In most cases, this will simply send locked funds to the withdrawer. - * * param _from Address being deposited from on L1. * param _to Address being deposited into on L2. * param _amount Amount being deposited. @@ -91,54 +86,50 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab revert("Implement me in child contracts"); } - /** - * @dev Overridable getter for the L2 gas limit, in the case it may be - * dynamic, and the above public constant does not suffice. - * - */ - function getFinalizeDepositL2Gas() - public - view - virtual - returns( - uint32 - ) - { - return DEFAULT_FINALIZE_DEPOSIT_L2_GAS; - } - /************** * Depositing * **************/ /** - * @dev deposit an amount of the ERC20 to the caller's balance on L2 + * @dev deposit an amount of the ERC20 to the caller's balance on L2. * @param _amount Amount of the ERC20 to deposit + * @param _l2Gas Gas limit required to complete the deposit on L2. + * @param _data Optional data to forward to L2. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function deposit( - uint _amount + uint256 _amount, + uint32 _l2Gas, + bytes calldata _data ) external override virtual { - _initiateDeposit(msg.sender, msg.sender, _amount); + _initiateDeposit(msg.sender, msg.sender, _amount, _l2Gas, _data); } /** - * @dev deposit an amount of ERC20 to a recipients's balance on L2 - * @param _to L2 address to credit the withdrawal to - * @param _amount Amount of the ERC20 to deposit + * @dev deposit an amount of ERC20 to a recipient's balance on L2. + * @param _to L2 address to credit the withdrawal to. + * @param _amount Amount of the ERC20 to deposit. + * @param _l2Gas Gas limit required to complete the deposit on L2. + * @param _data Optional data to forward to L2. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function depositTo( address _to, - uint _amount + uint256 _amount, + uint32 _l2Gas, + bytes calldata _data ) external override virtual { - _initiateDeposit(msg.sender, _to, _amount); + _initiateDeposit(msg.sender, _to, _amount, _l2Gas, _data); } /** @@ -148,11 +139,17 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab * @param _from Account to pull the deposit from on L1 * @param _to Account to give the deposit to on L2 * @param _amount Amount of the ERC20 to deposit. + * @param _l2Gas Gas limit required to complete the deposit on L2. + * @param _data Optional data to forward to L2. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function _initiateDeposit( address _from, address _to, - uint _amount + uint256 _amount, + uint32 _l2Gas, + bytes calldata _data ) internal { @@ -164,20 +161,23 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab ); // Construct calldata for l2DepositedToken.finalizeDeposit(_to, _amount) - bytes memory data = abi.encodeWithSelector( + bytes memory message = abi.encodeWithSelector( iOVM_L2DepositedToken.finalizeDeposit.selector, + _from, _to, - _amount + _amount, + _data ); // Send calldata into L2 sendCrossDomainMessage( l2DepositedToken, - data, - getFinalizeDepositL2Gas() + _l2Gas, + message ); - emit DepositInitiated(_from, _to, _amount); + // We omit _data here because events only support bytes32 types. + emit DepositInitiated(_from, _to, _amount, _data); } /************************* @@ -189,12 +189,18 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab * L1 ERC20 token. * This call will fail if the initialized withdrawal from L2 has not been finalized. * - * @param _to L1 address to credit the withdrawal to - * @param _amount Amount of the ERC20 to withdraw + * @param _from L2 address initiating the transfer. + * @param _to L1 address to credit the withdrawal to. + * @param _amount Amount of the ERC20 to deposit. + * @param _data Data provided by the sender on L2. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function finalizeWithdrawal( + address _from, address _to, - uint _amount + uint256 _amount, + bytes calldata _data ) external override @@ -206,7 +212,6 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab _to, _amount ); - - emit WithdrawalFinalized(_to, _amount); + emit WithdrawalFinalized(_from, _to, _amount, _data); } } diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/Abs_L2DepositedToken.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/Abs_L2DepositedToken.sol index 338220be2dbf74527fbcd5a5b6c9876ed32e29e8..f71dc6667a29919d1965b14e7f7104d3a1238a10 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/Abs_L2DepositedToken.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/Abs_L2DepositedToken.sol @@ -40,7 +40,7 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain ********************************/ /** - * @param _l2CrossDomainMessenger L1 Messenger address being used for cross-chain communications. + * @param _l2CrossDomainMessenger L2 Messenger address being used for cross-chain communications. */ constructor( address _l2CrossDomainMessenger @@ -50,9 +50,10 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain /** * @dev Initialize this contract with the L1 token gateway address. - * The flow: 1) this contract gets deployed on L2, 2) the L1 - * gateway is deployed with addr from (1), 3) L1 gateway address passed here. - * + * The flow: + * 1) this contract is deployed on L2, + * 2) the L1 gateway is deployed with addr from (1), + * 3) L1 gateway address passed here. * @param _l1TokenGateway Address of the corresponding L1 gateway deployed to the main chain */ function init( @@ -80,19 +81,15 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain * Overridable Accounting logic * ********************************/ - // Default gas value which can be overridden if more complex logic runs on L1. - uint32 internal constant DEFAULT_FINALIZE_WITHDRAWAL_L1_GAS = 100000; - /** * @dev Core logic to be performed when a withdrawal from L2 is initialized. * In most cases, this will simply burn the withdrawn L2 funds. - * - * param _to Address being withdrawn to - * param _amount Amount being withdrawn + * param _to Address being withdrawn to. + * param _amount Amount being withdrawn. */ function _handleInitiateWithdrawal( address, // _to, - uint // _amount + uint256 // _amount ) internal virtual @@ -103,13 +100,12 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain /** * @dev Core logic to be performed when a deposit from L2 is finalized on L2. * In most cases, this will simply _mint() to credit L2 funds to the recipient. - * - * param _to Address being deposited to on L2 - * param _amount Amount which was deposited on L1 + * param _to Address being deposited to on L2. + * param _amount Amount which was deposited on L1. */ function _handleFinalizeDeposit( address, // _to - uint // _amount + uint256 // _amount ) internal virtual @@ -117,67 +113,82 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain revert("Accounting must be implemented by child contract."); } - /** - * @dev Overridable getter for the *L1* gas limit of settling the withdrawal, in the case it may be - * dynamic, and the above public constant does not suffice. - */ - function getFinalizeWithdrawalL1Gas() - public - view - virtual - returns( - uint32 - ) - { - return DEFAULT_FINALIZE_WITHDRAWAL_L1_GAS; - } - - /*************** * Withdrawing * ***************/ /** * @dev initiate a withdraw of some tokens to the caller's account on L1 - * @param _amount Amount of the token to withdraw + * @param _amount Amount of the token to withdraw. + * param _l1Gas Unused, but included for potential forward compatibility considerations. + * @param _data Optional data to forward to L1. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function withdraw( - uint _amount + uint256 _amount, + uint32, // _l1Gas, + bytes calldata _data ) external override virtual onlyInitialized() { - _initiateWithdrawal(msg.sender, _amount); + _initiateWithdrawal( + msg.sender, + msg.sender, + _amount, + 0, + _data + ); } /** - * @dev initiate a withdraw of some token to a recipient's account on L1 - * @param _to L1 adress to credit the withdrawal to - * @param _amount Amount of the token to withdraw + * @dev initiate a withdraw of some token to a recipient's account on L1. + * @param _to L1 adress to credit the withdrawal to. + * @param _amount Amount of the token to withdraw. + * param _l1Gas Unused, but included for potential forward compatibility considerations. + * @param _data Optional data to forward to L1. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function withdrawTo( address _to, - uint _amount + uint256 _amount, + uint32, // _l1Gas, + bytes calldata _data ) external override virtual onlyInitialized() { - _initiateWithdrawal(_to, _amount); + _initiateWithdrawal( + msg.sender, + _to, + _amount, + 0, + _data + ); } /** * @dev Performs the logic for deposits by storing the token and informing the L2 token Gateway of the deposit. - * - * @param _to Account to give the withdrawal to on L1 - * @param _amount Amount of the token to withdraw + * @param _from Account to pull the deposit from on L2. + * @param _to Account to give the withdrawal to on L1. + * @param _amount Amount of the token to withdraw. + * param _l1Gas Unused, but included for potential forward compatibility considerations. + * @param _data Optional data to forward to L1. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function _initiateWithdrawal( + address _from, address _to, - uint _amount + uint256 _amount, + uint32, // _l1Gas, + bytes calldata _data ) internal { @@ -185,20 +196,22 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain _handleInitiateWithdrawal(_to, _amount); // Construct calldata for l1TokenGateway.finalizeWithdrawal(_to, _amount) - bytes memory data = abi.encodeWithSelector( + bytes memory message = abi.encodeWithSelector( iOVM_L1TokenGateway.finalizeWithdrawal.selector, + _from, _to, - _amount + _amount, + _data ); // Send message up to L1 gateway sendCrossDomainMessage( address(l1TokenGateway), - data, - getFinalizeWithdrawalL1Gas() + 0, + message ); - emit WithdrawalInitiated(msg.sender, _to, _amount); + emit WithdrawalInitiated(msg.sender, _to, _amount, _data); } /************************************ @@ -209,13 +222,18 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain * @dev Complete a deposit from L1 to L2, and credits funds to the recipient's balance of this * L2 token. * This call will fail if it did not originate from a corresponding deposit in OVM_l1TokenGateway. - * + * @param _from Account to pull the deposit from on L2. * @param _to Address to receive the withdrawal at * @param _amount Amount of the token to withdraw + * @param _data Data provider by the sender on L1. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function finalizeDeposit( + address _from, address _to, - uint _amount + uint256 _amount, + bytes calldata _data ) external override @@ -224,6 +242,6 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain onlyFromCrossDomainAccount(address(l1TokenGateway)) { _handleFinalizeDeposit(_to, _amount); - emit DepositFinalized(_to, _amount); + emit DepositFinalized(_from, _to, _amount, _data); } } diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L1ERC20Gateway.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L1ERC20Gateway.sol index 0d97a7a24c3fa46d19aaad5fa8727aaed7abc9e4..23834445b8b38bef8a072ee9ab7f0d3204cc412d 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L1ERC20Gateway.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L1ERC20Gateway.sol @@ -35,8 +35,8 @@ contract OVM_L1ERC20Gateway is Abs_L1TokenGateway { ***************/ /** - * @param _l1ERC20 L1 ERC20 address this contract stores deposits for - * @param _l2DepositedERC20 L2 Gateway address on the chain being deposited into + * @param _l1ERC20 L1 ERC20 address this contract stores deposits for. + * @param _l2DepositedERC20 L2 Gateway address on the chain being deposited into. */ constructor( iOVM_ERC20 _l1ERC20, @@ -58,11 +58,11 @@ contract OVM_L1ERC20Gateway is Abs_L1TokenGateway { /** * @dev When a deposit is initiated on L1, the L1 Gateway - * transfers the funds to itself for future withdrawals + * transfers the funds to itself for future withdrawals. * - * @param _from L1 address ETH is being deposited from - * param _to L2 address that the ETH is being deposited to - * @param _amount Amount of ERC20 to send + * @param _from L1 address ETH is being deposited from. + * param _to L2 address that the ETH is being deposited to. + * @param _amount Amount of ERC20 to send. */ function _handleInitiateDeposit( address _from, @@ -82,14 +82,14 @@ contract OVM_L1ERC20Gateway is Abs_L1TokenGateway { /** * @dev When a withdrawal is finalized on L1, the L1 Gateway - * transfers the funds to the withdrawer + * transfers the funds to the withdrawer. * - * @param _to L1 address that the ERC20 is being withdrawn to - * @param _amount Amount of ERC20 to send + * @param _to L1 address that the ERC20 is being withdrawn to. + * @param _amount Amount of ERC20 to send. */ function _handleFinalizeWithdrawal( address _to, - uint _amount + uint256 _amount ) internal override diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L1ETHGateway.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L1ETHGateway.sol index 1080e56918953d5a38e74121afaacc96b3479aae..d3e475e43ceb46c80a56c2c9c18e722d030ce240 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L1ETHGateway.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L1ETHGateway.sol @@ -21,12 +21,6 @@ import { Lib_AddressManager } from "../../../libraries/resolver/Lib_AddressManag */ contract OVM_L1ETHGateway is iOVM_L1ETHGateway, OVM_CrossDomainEnabled, Lib_AddressResolver { - /******************** - * Public Constants * - ********************/ - - uint32 public constant override getFinalizeDepositL2Gas = 1200000; - /******************************** * External Contract References * ********************************/ @@ -41,7 +35,6 @@ contract OVM_L1ETHGateway is iOVM_L1ETHGateway, OVM_CrossDomainEnabled, Lib_Addr constructor() OVM_CrossDomainEnabled(address(0)) Lib_AddressResolver(address(0)) - public {} /****************** @@ -68,66 +61,102 @@ contract OVM_L1ETHGateway is iOVM_L1ETHGateway, OVM_CrossDomainEnabled, Lib_Addr * Depositing * **************/ + /** + * @dev This function can be called with no data + * to deposit an amount of ETH to the caller's balance on L2. + * Since the receive function doesn't take data, a conservative + * default of 1.2 Million gas is forwarded to L2. + */ receive() external payable { - _initiateDeposit(msg.sender, msg.sender); + _initiateDeposit(msg.sender, msg.sender, 1_200_000, bytes("")); } /** - * @dev deposit an amount of the ETH to the caller's balance on L2 + * @dev Deposit an amount of the ETH to the caller's balance on L2. + * @param _l2Gas Gas limit required to complete the deposit on L2. + * @param _data Optional data to forward to L2. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ - function deposit() + function deposit( + uint32 _l2Gas, + bytes calldata _data + ) external override payable { - _initiateDeposit(msg.sender, msg.sender); + _initiateDeposit( + msg.sender, + msg.sender, + _l2Gas, + _data + ); } /** - * @dev deposit an amount of ETH to a recipients's balance on L2 - * @param _to L2 address to credit the withdrawal to + * @dev Deposit an amount of ETH to a recipient's balance on L2. + * @param _to L2 address to credit the withdrawal to. + * @param _l2Gas Gas limit required to complete the deposit on L2. + * @param _data Optional data to forward to L2. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function depositTo( - address _to + address _to, + uint32 _l2Gas, + bytes calldata _data ) external override payable { - _initiateDeposit(msg.sender, _to); + _initiateDeposit( + msg.sender, + _to, + _l2Gas, + _data + ); } /** * @dev Performs the logic for deposits by storing the ETH and informing the L2 ETH Gateway of the deposit. - * - * @param _from Account to pull the deposit from on L1 - * @param _to Account to give the deposit to on L2 + * @param _from Account to pull the deposit from on L1. + * @param _to Account to give the deposit to on L2. + * @param _l2Gas Gas limit required to complete the deposit on L2. + * @param _data Optional data to forward to L2. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function _initiateDeposit( address _from, - address _to + address _to, + uint32 _l2Gas, + bytes memory _data ) internal { // Construct calldata for l2ETHGateway.finalizeDeposit(_to, _amount) - bytes memory data = + bytes memory message = abi.encodeWithSelector( iOVM_L2DepositedToken.finalizeDeposit.selector, + _from, _to, - msg.value + msg.value, + _data ); // Send calldata into L2 sendCrossDomainMessage( ovmEth, - data, - getFinalizeDepositL2Gas + _l2Gas, + message ); - emit DepositInitiated(_from, _to, msg.value); + emit DepositInitiated(_from, _to, msg.value, _data); } /************************* @@ -138,13 +167,18 @@ contract OVM_L1ETHGateway is iOVM_L1ETHGateway, OVM_CrossDomainEnabled, Lib_Addr * @dev Complete a withdrawal from L2 to L1, and credit funds to the recipient's balance of the * L1 ETH token. * Since only the xDomainMessenger can call this function, it will never be called before the withdrawal is finalized. - * - * @param _to L1 address to credit the withdrawal to - * @param _amount Amount of the ETH to withdraw + * @param _from L2 address initiating the transfer. + * @param _to L1 address to credit the withdrawal to. + * @param _amount Amount of the ERC20 to deposit. + * @param _data Optional data to forward to L2. This data is provided + * solely as a convenience for external contracts. Aside from enforcing a maximum + * length, these contracts provide no guarantees about its content. */ function finalizeWithdrawal( + address _from, address _to, - uint256 _amount + uint256 _amount, + bytes calldata _data ) external override @@ -152,7 +186,7 @@ contract OVM_L1ETHGateway is iOVM_L1ETHGateway, OVM_CrossDomainEnabled, Lib_Addr { _safeTransferETH(_to, _amount); - emit WithdrawalFinalized(_to, _amount); + emit WithdrawalFinalized(_from, _to, _amount, _data); } /********************************** @@ -162,8 +196,8 @@ contract OVM_L1ETHGateway is iOVM_L1ETHGateway, OVM_CrossDomainEnabled, Lib_Addr /** * @dev Internal accounting function for moving around L1 ETH. * - * @param _to L1 address to transfer ETH to - * @param _value Amount of ETH to send to + * @param _to L1 address to transfer ETH to. + * @param _value Amount of ETH to transfer. */ function _safeTransferETH( address _to, @@ -180,8 +214,8 @@ contract OVM_L1ETHGateway is iOVM_L1ETHGateway, OVM_CrossDomainEnabled, Lib_Addr *****************************/ /** - * @dev Migrates entire ETH balance to another gateway - * @param _to Gateway Proxy address to migrate ETH to + * @dev Migrates entire ETH balance to another gateway. + * @param _to Gateway Proxy address to migrate ETH to. */ function migrateEth(address payable _to) external { address owner = Lib_AddressManager(libAddressManager).owner(); @@ -192,7 +226,7 @@ contract OVM_L1ETHGateway is iOVM_L1ETHGateway, OVM_CrossDomainEnabled, Lib_Addr /** * @dev Adds ETH balance to the account. This is meant to allow for ETH - * to be migrated from an old gateway to a new gateway + * to be migrated from an old gateway to a new gateway. */ function donateETH() external payable {} } diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L2DepositedERC20.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L2DepositedERC20.sol index e1fd3112493b22720da8a9ee05b0608e82001648..b7e741636caf3d7a21eeee90e414a155b0897903 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L2DepositedERC20.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/tokens/OVM_L2DepositedERC20.sol @@ -31,8 +31,8 @@ contract OVM_L2DepositedERC20 is Abs_L2DepositedToken, UniswapV2ERC20 { /** * @param _l2CrossDomainMessenger Cross-domain messenger used by this contract. - * @param _name ERC20 name - * @param _symbol ERC20 symbol + * @param _name ERC20 name. + * @param _symbol ERC20 symbol. */ constructor( address _l2CrossDomainMessenger, @@ -46,7 +46,7 @@ contract OVM_L2DepositedERC20 is Abs_L2DepositedToken, UniswapV2ERC20 { // When a withdrawal is initiated, we burn the withdrawer's funds to prevent subsequent L2 usage. function _handleInitiateWithdrawal( address, // _to, - uint _amount + uint256 _amount ) internal override @@ -57,7 +57,7 @@ contract OVM_L2DepositedERC20 is Abs_L2DepositedToken, UniswapV2ERC20 { // When a deposit is finalized, we credit the account on L2 with the same amount of tokens. function _handleFinalizeDeposit( address _to, - uint _amount + uint256 _amount ) internal override diff --git a/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L1ETHGateway.sol b/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L1ETHGateway.sol index 32a9918d9cca8cfd089c8e6f528542a74289ce5a..44c834b89da57e9fda0c40dd490d9b1e0abef43b 100644 --- a/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L1ETHGateway.sol +++ b/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L1ETHGateway.sol @@ -13,25 +13,33 @@ interface iOVM_L1ETHGateway { event DepositInitiated( address indexed _from, - address _to, - uint256 _amount + address indexed _to, + uint256 _amount, + bytes _data ); event WithdrawalFinalized( + address indexed _from, address indexed _to, - uint256 _amount + uint256 _amount, + bytes _data ); /******************** * Public Functions * ********************/ - function deposit() + function deposit( + uint32 _l2Gas, + bytes calldata _data + ) external payable; function depositTo( - address _to + address _to, + uint32 _l2Gas, + bytes calldata _data ) external payable; @@ -41,15 +49,11 @@ interface iOVM_L1ETHGateway { *************************/ function finalizeWithdrawal( + address _from, address _to, - uint _amount + uint _amount, + bytes calldata _data ) external; - function getFinalizeDepositL2Gas() - external - view - returns( - uint32 - ); } diff --git a/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L1TokenGateway.sol b/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L1TokenGateway.sol index 087684e340266ecec1c0af27c65c4a9f6d0225f8..402aefbcc669c389286e184c0440c8f737438c69 100644 --- a/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L1TokenGateway.sol +++ b/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L1TokenGateway.sol @@ -13,13 +13,16 @@ interface iOVM_L1TokenGateway { event DepositInitiated( address indexed _from, - address _to, - uint256 _amount + address indexed _to, + uint256 _amount, + bytes _data ); event WithdrawalFinalized( + address indexed _from, address indexed _to, - uint256 _amount + uint256 _amount, + bytes _data ); @@ -28,13 +31,17 @@ interface iOVM_L1TokenGateway { ********************/ function deposit( - uint _amount + uint _amount, + uint32 _l2Gas, + bytes calldata _data ) external; function depositTo( address _to, - uint _amount + uint _amount, + uint32 _l2Gas, + bytes calldata _data ) external; @@ -44,8 +51,11 @@ interface iOVM_L1TokenGateway { *************************/ function finalizeWithdrawal( + address _from, address _to, - uint _amount + uint _amount, + bytes calldata _data ) external; + } diff --git a/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L2DepositedToken.sol b/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L2DepositedToken.sol index 40088bfc2d48d87c36a526fc3e6671c229a8b2d5..14d12db81a9a87d54a85c91740dd285ba561eb35 100644 --- a/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L2DepositedToken.sol +++ b/packages/contracts/contracts/optimistic-ethereum/iOVM/bridge/tokens/iOVM_L2DepositedToken.sol @@ -13,13 +13,16 @@ interface iOVM_L2DepositedToken { event WithdrawalInitiated( address indexed _from, - address _to, - uint256 _amount + address indexed _to, + uint256 _amount, + bytes _data ); event DepositFinalized( + address indexed _from, address indexed _to, - uint256 _amount + uint256 _amount, + bytes _data ); @@ -28,13 +31,17 @@ interface iOVM_L2DepositedToken { ********************/ function withdraw( - uint _amount + uint _amount, + uint32 _l1Gas, + bytes calldata _data ) external; function withdrawTo( address _to, - uint _amount + uint _amount, + uint32 _l1Gas, + bytes calldata _data ) external; @@ -44,8 +51,11 @@ interface iOVM_L2DepositedToken { *************************/ function finalizeDeposit( + address _from, address _to, - uint _amount + uint _amount, + bytes calldata _data ) external; + } diff --git a/packages/contracts/contracts/optimistic-ethereum/libraries/bridge/OVM_CrossDomainEnabled.sol b/packages/contracts/contracts/optimistic-ethereum/libraries/bridge/OVM_CrossDomainEnabled.sol index 56f55799f10205026ece10f8e5c773daefde2b60..784ecfb71094b63fe95f4596d7c0d65e874e97e2 100644 --- a/packages/contracts/contracts/optimistic-ethereum/libraries/bridge/OVM_CrossDomainEnabled.sol +++ b/packages/contracts/contracts/optimistic-ethereum/libraries/bridge/OVM_CrossDomainEnabled.sol @@ -23,7 +23,7 @@ contract OVM_CrossDomainEnabled { /*************** * Constructor * - ***************/ + ***************/ /** * @param _messenger Address of the CrossDomainMessenger on the current layer. @@ -68,7 +68,7 @@ contract OVM_CrossDomainEnabled { /** * Gets the messenger, usually from storage. This function is exposed in case a child contract * needs to override. - * @return The address of the cross-domain messenger contract which should be used. + * @return The address of the cross-domain messenger contract which should be used. */ function getCrossDomainMessenger() internal @@ -83,17 +83,17 @@ contract OVM_CrossDomainEnabled { /** * Sends a message to an account on another domain * @param _crossDomainTarget The intended recipient on the destination domain - * @param _data The data to send to the target (usually calldata to a function with + * @param _message The data to send to the target (usually calldata to a function with * `onlyFromCrossDomainAccount()`) * @param _gasLimit The gasLimit for the receipt of the message on the target domain. */ function sendCrossDomainMessage( address _crossDomainTarget, - bytes memory _data, - uint32 _gasLimit + uint32 _gasLimit, + bytes memory _message ) internal { - getCrossDomainMessenger().sendMessage(_crossDomainTarget, _data, _gasLimit); + getCrossDomainMessenger().sendMessage(_crossDomainTarget, _message, _gasLimit); } } diff --git a/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L1ERC20Gateway.spec.ts b/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L1ERC20Gateway.spec.ts index a3a305862774f5e752548ba54b66cb9213a79b8a..a7fb4394a311849659701e1b318c87f17daf04d1 100644 --- a/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L1ERC20Gateway.spec.ts +++ b/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L1ERC20Gateway.spec.ts @@ -6,9 +6,10 @@ import { Signer, ContractFactory, Contract, constants } from 'ethers' import { smockit, MockContract, smoddit } from '@eth-optimism/smock' /* Internal Imports */ -import { NON_ZERO_ADDRESS } from '../../../../helpers' +import { NON_NULL_BYTES32, NON_ZERO_ADDRESS } from '../../../../helpers' const INITIAL_TOTAL_L1_SUPPLY = 3000 +const FINALIZATION_GAS = 1_200_000 const ERR_INVALID_MESSENGER = 'OVM_XCHAIN: messenger contract unauthenticated' const ERR_INVALID_X_DOMAIN_MSG_SENDER = @@ -46,7 +47,6 @@ describe('OVM_L1ERC20Gateway', () => { let OVM_L1ERC20Gateway: Contract let Mock__OVM_L1CrossDomainMessenger: MockContract - let finalizeDepositGasLimit: number beforeEach(async () => { // Create a special signer which will enable us to send messages from the L1Messenger contract let l1MessengerImpersonator: Signer @@ -65,8 +65,6 @@ describe('OVM_L1ERC20Gateway', () => { Mock__OVM_L2DepositedERC20.address, Mock__OVM_L1CrossDomainMessenger.address ) - - finalizeDepositGasLimit = await OVM_L1ERC20Gateway.getFinalizeDepositL2Gas() }) describe('finalizeWithdrawal', () => { @@ -81,7 +79,12 @@ describe('OVM_L1ERC20Gateway', () => { ) await expect( - OVM_L1ERC20Gateway.finalizeWithdrawal(constants.AddressZero, 1) + OVM_L1ERC20Gateway.finalizeWithdrawal( + constants.AddressZero, + constants.AddressZero, + 1, + NON_NULL_BYTES32 + ) ).to.be.revertedWith(ERR_INVALID_MESSENGER) }) @@ -91,9 +94,15 @@ describe('OVM_L1ERC20Gateway', () => { ) await expect( - OVM_L1ERC20Gateway.finalizeWithdrawal(constants.AddressZero, 1, { - from: Mock__OVM_L1CrossDomainMessenger.address, - }) + OVM_L1ERC20Gateway.finalizeWithdrawal( + constants.AddressZero, + constants.AddressZero, + 1, + NON_NULL_BYTES32, + { + from: Mock__OVM_L1CrossDomainMessenger.address, + } + ) ).to.be.revertedWith(ERR_INVALID_X_DOMAIN_MSG_SENDER) }) @@ -109,8 +118,10 @@ describe('OVM_L1ERC20Gateway', () => { await L1ERC20.transfer(OVM_L1ERC20Gateway.address, withdrawalAmount) const res = await OVM_L1ERC20Gateway.finalizeWithdrawal( + NON_ZERO_ADDRESS, NON_ZERO_ADDRESS, withdrawalAmount, + NON_NULL_BYTES32, { from: Mock__OVM_L1CrossDomainMessenger.address } ) @@ -125,8 +136,6 @@ describe('OVM_L1ERC20Gateway', () => { const OVM_L2DepositedERC20 = await ( await ethers.getContractFactory('OVM_L2DepositedERC20') ).deploy(constants.AddressZero, '', '') - const defaultFinalizeWithdrawalGas = await OVM_L2DepositedERC20.getFinalizeWithdrawalL1Gas() - await expect(gasUsed.gt((defaultFinalizeWithdrawalGas * 11) / 10)) }) it.skip('finalizeWithdrawalAndCall(): should should credit funds to the withdrawer, and forward from and data', async () => { @@ -171,7 +180,11 @@ describe('OVM_L1ERC20Gateway', () => { it('deposit() escrows the deposit amount and sends the correct deposit message', async () => { // alice calls deposit on the gateway and the L1 gateway calls transferFrom on the token - await OVM_L1ERC20Gateway.deposit(depositAmount) + await OVM_L1ERC20Gateway.deposit( + depositAmount, + FINALIZATION_GAS, + NON_NULL_BYTES32 + ) const depositCallToMessenger = Mock__OVM_L1CrossDomainMessenger.smocked.sendMessage.calls[0] @@ -195,16 +208,21 @@ describe('OVM_L1ERC20Gateway', () => { expect(depositCallToMessenger._message).to.equal( await Mock__OVM_L2DepositedERC20.interface.encodeFunctionData( 'finalizeDeposit', - [depositer, depositAmount] + [depositer, depositer, depositAmount, NON_NULL_BYTES32] ) ) - expect(depositCallToMessenger._gasLimit).to.equal(finalizeDepositGasLimit) + expect(depositCallToMessenger._gasLimit).to.equal(FINALIZATION_GAS) }) it('depositTo() escrows the deposit amount and sends the correct deposit message', async () => { // depositor calls deposit on the gateway and the L1 gateway calls transferFrom on the token const bobsAddress = await bob.getAddress() - await OVM_L1ERC20Gateway.depositTo(bobsAddress, depositAmount) + await OVM_L1ERC20Gateway.depositTo( + bobsAddress, + depositAmount, + FINALIZATION_GAS, + NON_NULL_BYTES32 + ) const depositCallToMessenger = Mock__OVM_L1CrossDomainMessenger.smocked.sendMessage.calls[0] @@ -228,10 +246,10 @@ describe('OVM_L1ERC20Gateway', () => { expect(depositCallToMessenger._message).to.equal( await Mock__OVM_L2DepositedERC20.interface.encodeFunctionData( 'finalizeDeposit', - [bobsAddress, depositAmount] + [depositer, bobsAddress, depositAmount, NON_NULL_BYTES32] ) ) - expect(depositCallToMessenger._gasLimit).to.equal(finalizeDepositGasLimit) + expect(depositCallToMessenger._gasLimit).to.equal(FINALIZATION_GAS) }) }) }) diff --git a/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L1ETHGateway.spec.ts b/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L1ETHGateway.spec.ts index 98d64620cfad40c8263bc164967094ba1cda476a..731cf073f43266a945de9f2e9dccf59fb878083c 100644 --- a/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L1ETHGateway.spec.ts +++ b/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L1ETHGateway.spec.ts @@ -6,7 +6,11 @@ import { Signer, Contract, constants } from 'ethers' import { smockit, MockContract } from '@eth-optimism/smock' /* Internal Imports */ -import { NON_ZERO_ADDRESS, makeAddressManager } from '../../../../helpers' +import { + NON_ZERO_ADDRESS, + makeAddressManager, + NON_NULL_BYTES32, +} from '../../../../helpers' const L1_MESSENGER_NAME = 'Proxy__OVM_L1CrossDomainMessenger' @@ -14,6 +18,7 @@ const ERR_INVALID_MESSENGER = 'OVM_XCHAIN: messenger contract unauthenticated' const ERR_INVALID_X_DOMAIN_MSG_SENDER = 'OVM_XCHAIN: wrong sender of cross-domain message' const ERR_ALREADY_INITIALIZED = 'Contract has already been initialized.' +const FINALIZATION_GAS = 1_200_000 describe('OVM_L1ETHGateway', () => { // init signers @@ -38,7 +43,6 @@ describe('OVM_L1ETHGateway', () => { let OVM_L1ETHGateway: Contract let Mock__OVM_L1CrossDomainMessenger: MockContract - let finalizeDepositGasLimit: number beforeEach(async () => { // Get a new mock L1 messenger Mock__OVM_L1CrossDomainMessenger = await smockit( @@ -54,8 +58,6 @@ describe('OVM_L1ETHGateway', () => { AddressManager.address, Mock__OVM_L2DepositedERC20.address ) - - finalizeDepositGasLimit = await OVM_L1ETHGateway.getFinalizeDepositL2Gas() }) describe('initialize', () => { @@ -75,7 +77,9 @@ describe('OVM_L1ETHGateway', () => { await expect( OVM_L1ETHGateway.connect(alice).finalizeWithdrawal( constants.AddressZero, - 1 + constants.AddressZero, + 1, + NON_NULL_BYTES32 ) ).to.be.revertedWith(ERR_INVALID_MESSENGER) }) @@ -99,7 +103,12 @@ describe('OVM_L1ETHGateway', () => { ) await expect( - OVM_L1ETHGateway.finalizeWithdrawal(constants.AddressZero, 1) + OVM_L1ETHGateway.finalizeWithdrawal( + constants.AddressZero, + constants.AddressZero, + 1, + NON_NULL_BYTES32 + ) ).to.be.revertedWith(ERR_INVALID_X_DOMAIN_MSG_SENDER) }) @@ -115,14 +124,20 @@ describe('OVM_L1ETHGateway', () => { ) // thanks Alice - await OVM_L1ETHGateway.connect(alice).deposit({ - value: ethers.utils.parseEther('1.0'), - gasPrice: 0, - }) + await OVM_L1ETHGateway.connect(alice).deposit( + FINALIZATION_GAS, + NON_NULL_BYTES32, + { + value: ethers.utils.parseEther('1.0'), + gasPrice: 0, + } + ) const res = await OVM_L1ETHGateway.finalizeWithdrawal( + NON_ZERO_ADDRESS, NON_ZERO_ADDRESS, withdrawalAmount, + NON_NULL_BYTES32, { from: Mock__OVM_L1CrossDomainMessenger.address } ) @@ -138,12 +153,6 @@ describe('OVM_L1ETHGateway', () => { const OVM_L2DepositedERC20 = await ( await ethers.getContractFactory('OVM_L2DepositedERC20') ).deploy(constants.AddressZero, '', '') - - await expect( - gasUsed.gt( - ((await OVM_L2DepositedERC20.getFinalizeWithdrawalL1Gas()) * 11) / 10 - ) - ) }) it.skip('finalizeWithdrawalAndCall(): should should credit funds to the withdrawer, and forward from and data', async () => { @@ -182,10 +191,14 @@ describe('OVM_L1ETHGateway', () => { const initialBalance = await ethers.provider.getBalance(depositer) // alice calls deposit on the gateway and the L1 gateway calls transferFrom on the token - await OVM_L1ETHGateway.connect(alice).deposit({ - value: depositAmount, - gasPrice: 0, - }) + await OVM_L1ETHGateway.connect(alice).deposit( + FINALIZATION_GAS, + NON_NULL_BYTES32, + { + value: depositAmount, + gasPrice: 0, + } + ) const depositCallToMessenger = Mock__OVM_L1CrossDomainMessenger.smocked.sendMessage.calls[0] @@ -211,10 +224,10 @@ describe('OVM_L1ETHGateway', () => { expect(depositCallToMessenger._message).to.equal( await Mock__OVM_L2DepositedERC20.interface.encodeFunctionData( 'finalizeDeposit', - [depositer, depositAmount] + [depositer, depositer, depositAmount, NON_NULL_BYTES32] ) ) - expect(depositCallToMessenger._gasLimit).to.equal(finalizeDepositGasLimit) + expect(depositCallToMessenger._gasLimit).to.equal(FINALIZATION_GAS) }) it('depositTo() escrows the deposit amount and sends the correct deposit message', async () => { @@ -223,10 +236,15 @@ describe('OVM_L1ETHGateway', () => { const aliceAddress = await alice.getAddress() const initialBalance = await ethers.provider.getBalance(aliceAddress) - await OVM_L1ETHGateway.connect(alice).depositTo(bobsAddress, { - value: depositAmount, - gasPrice: 0, - }) + await OVM_L1ETHGateway.connect(alice).depositTo( + bobsAddress, + FINALIZATION_GAS, + NON_NULL_BYTES32, + { + value: depositAmount, + gasPrice: 0, + } + ) const depositCallToMessenger = Mock__OVM_L1CrossDomainMessenger.smocked.sendMessage.calls[0] @@ -250,12 +268,13 @@ describe('OVM_L1ETHGateway', () => { expect(depositCallToMessenger._message).to.equal( await Mock__OVM_L2DepositedERC20.interface.encodeFunctionData( 'finalizeDeposit', - [bobsAddress, depositAmount] + [aliceAddress, bobsAddress, depositAmount, NON_NULL_BYTES32] ) ) - expect(depositCallToMessenger._gasLimit).to.equal(finalizeDepositGasLimit) + expect(depositCallToMessenger._gasLimit).to.equal(FINALIZATION_GAS) }) }) + describe('migrating ETH', () => { const migrateAmount = 1_000 diff --git a/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L2DepositedERC20.spec.ts b/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L2DepositedERC20.spec.ts index 94e22562931e75a90f430374a9e0ea48cfced82e..e404a951a2bb6b6d3c329b84de6e61fe5fccace6 100644 --- a/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L2DepositedERC20.spec.ts +++ b/packages/contracts/test/contracts/OVM/bridge/assets/OVM_L2DepositedERC20.spec.ts @@ -11,7 +11,9 @@ import { } from '@eth-optimism/smock' /* Internal Imports */ -import { NON_ZERO_ADDRESS } from '../../../../helpers' +import { NON_NULL_BYTES32, NON_ZERO_ADDRESS } from '../../../../helpers' + +const FINALIZATION_GAS = 1_200_000 const ERR_INVALID_MESSENGER = 'OVM_XCHAIN: messenger contract unauthenticated' const ERR_INVALID_X_DOMAIN_MSG_SENDER = @@ -32,7 +34,6 @@ describe('OVM_L2DepositedERC20', () => { let OVM_L2DepositedERC20: Contract let Mock__OVM_L2CrossDomainMessenger: MockContract - let finalizeWithdrawalGasLimit: number beforeEach(async () => { // Create a special signer which will enable us to send messages from the L2Messenger contract let l2MessengerImpersonator: Signer @@ -52,8 +53,6 @@ describe('OVM_L2DepositedERC20', () => { // initialize the L2 Gateway with the L1G ateway addrss await OVM_L2DepositedERC20.init(MOCK_L1GATEWAY_ADDRESS) - - finalizeWithdrawalGasLimit = await OVM_L2DepositedERC20.getFinalizeWithdrawalL1Gas() }) // test the transfer flow of moving a token from L2 to L1 @@ -66,7 +65,12 @@ describe('OVM_L2DepositedERC20', () => { await OVM_L2DepositedERC20.init(NON_ZERO_ADDRESS) await expect( - OVM_L2DepositedERC20.finalizeDeposit(constants.AddressZero, 0) + OVM_L2DepositedERC20.finalizeDeposit( + constants.AddressZero, + constants.AddressZero, + 0, + NON_NULL_BYTES32 + ) ).to.be.revertedWith(ERR_INVALID_MESSENGER) }) @@ -76,9 +80,15 @@ describe('OVM_L2DepositedERC20', () => { ) await expect( - OVM_L2DepositedERC20.finalizeDeposit(constants.AddressZero, 0, { - from: Mock__OVM_L2CrossDomainMessenger.address, - }) + OVM_L2DepositedERC20.finalizeDeposit( + constants.AddressZero, + constants.AddressZero, + 0, + NON_NULL_BYTES32, + { + from: Mock__OVM_L2CrossDomainMessenger.address, + } + ) ).to.be.revertedWith(ERR_INVALID_X_DOMAIN_MSG_SENDER) }) @@ -89,8 +99,10 @@ describe('OVM_L2DepositedERC20', () => { ) await OVM_L2DepositedERC20.finalizeDeposit( + NON_ZERO_ADDRESS, await alice.getAddress(), depositAmount, + NON_NULL_BYTES32, { from: Mock__OVM_L2CrossDomainMessenger.address } ) @@ -124,7 +136,11 @@ describe('OVM_L2DepositedERC20', () => { }) it('withdraw() burns and sends the correct withdrawal message', async () => { - await SmoddedL2Gateway.withdraw(withdrawAmount) + await SmoddedL2Gateway.withdraw( + withdrawAmount, + FINALIZATION_GAS, + NON_NULL_BYTES32 + ) const withdrawalCallToMessenger = Mock__OVM_L2CrossDomainMessenger.smocked.sendMessage.calls[0] @@ -149,17 +165,37 @@ describe('OVM_L2DepositedERC20', () => { expect(withdrawalCallToMessenger._message).to.equal( await Factory__OVM_L1ERC20Gateway.interface.encodeFunctionData( 'finalizeWithdrawal', - [await alice.getAddress(), withdrawAmount] + [ + await alice.getAddress(), + await alice.getAddress(), + withdrawAmount, + NON_NULL_BYTES32, + ] ) ) - // Hardcoded gaslimit should be correct - expect(withdrawalCallToMessenger._gasLimit).to.equal( - finalizeWithdrawalGasLimit + // gaslimit should be correct + expect(withdrawalCallToMessenger._gasLimit).to.equal(0) + }) + + it('withdraw() uses the user provided gas limit if it is larger than the default value ', async () => { + await SmoddedL2Gateway.withdraw( + withdrawAmount, + FINALIZATION_GAS, + NON_NULL_BYTES32 ) + const withdrawalCallToMessenger = + Mock__OVM_L2CrossDomainMessenger.smocked.sendMessage.calls[0] + // gas value is ignored and set to 0. + expect(withdrawalCallToMessenger._gasLimit).to.equal(0) }) it('withdrawTo() burns and sends the correct withdrawal message', async () => { - await SmoddedL2Gateway.withdrawTo(await bob.getAddress(), withdrawAmount) + await SmoddedL2Gateway.withdrawTo( + await bob.getAddress(), + withdrawAmount, + FINALIZATION_GAS, + NON_NULL_BYTES32 + ) const withdrawalCallToMessenger = Mock__OVM_L2CrossDomainMessenger.smocked.sendMessage.calls[0] @@ -184,13 +220,30 @@ describe('OVM_L2DepositedERC20', () => { expect(withdrawalCallToMessenger._message).to.equal( await Factory__OVM_L1ERC20Gateway.interface.encodeFunctionData( 'finalizeWithdrawal', - [await bob.getAddress(), withdrawAmount] + [ + await alice.getAddress(), + await bob.getAddress(), + withdrawAmount, + NON_NULL_BYTES32, + ] ) ) - // Hardcoded gaslimit should be correct - expect(withdrawalCallToMessenger._gasLimit).to.equal( - finalizeWithdrawalGasLimit + // gas value is ignored and set to 0. + expect(withdrawalCallToMessenger._gasLimit).to.equal(0) + }) + + it('withdrawTo() uses the user provided gas limit if it is larger than the default value ', async () => { + await SmoddedL2Gateway.withdrawTo( + await bob.getAddress(), + withdrawAmount, + FINALIZATION_GAS, + NON_NULL_BYTES32 ) + const withdrawalCallToMessenger = + Mock__OVM_L2CrossDomainMessenger.smocked.sendMessage.calls[0] + + // gas value is ignored and set to 0. + expect(withdrawalCallToMessenger._gasLimit).to.equal(0) }) })