Commit 1a55f640 authored by Kristijan Rebernisak's avatar Kristijan Rebernisak Committed by GitHub

Fix bridge contracts upgradeability (#533)

- Change Abs_L1TokenGateway.DEFAULT_FINALIZE_DEPOSIT_L2_GAS and Abs_L2DepositedToken.DEFAULT_FINALIZE_WITHDRAWAL_L1_GAS to internal constants
- Add virtual declaration to Abs_L1TokenGateway.getFinalizeDepositL2Gas fn
- Add virtual declaration to few more bridge gateway fn to improve extensibility
parent 8d4aae40
---
"@eth-optimism/contracts": patch
---
Fix bridge contracts upgradeability by changing `Abs_L1TokenGateway.DEFAULT_FINALIZE_DEPOSIT_L2_GAS` from a storage var to an internal constant.
Additionally, make some bridge functions virtual so they could be overriden in child contracts.
// SPDX-License-Identifier: MIT // SPDX-License-Identifier: MIT
// @unsupported: ovm // @unsupported: ovm
pragma solidity >0.5.0 <0.8.0; pragma solidity >0.5.0 <0.8.0;
pragma experimental ABIEncoderV2; pragma experimental ABIEncoderV2;
...@@ -16,7 +16,7 @@ import { OVM_CrossDomainEnabled } from "../../../libraries/bridge/OVM_CrossDomai ...@@ -16,7 +16,7 @@ import { OVM_CrossDomainEnabled } from "../../../libraries/bridge/OVM_CrossDomai
* It synchronizes a corresponding L2 representation of the "deposited token", informing it * It synchronizes a corresponding L2 representation of the "deposited token", informing it
* of new deposits and releasing L1 funds when there are newly finalized withdrawals. * of new deposits and releasing L1 funds when there are newly finalized withdrawals.
* *
* NOTE: This abstract contract gives all the core functionality of an L1 token gateway, * NOTE: This abstract contract gives all the core functionality of an L1 token gateway,
* but provides easy hooks in case developers need extensions in child contracts. * but provides easy hooks in case developers need extensions in child contracts.
* In many cases, the default OVM_L1ERC20Gateway will suffice. * In many cases, the default OVM_L1ERC20Gateway will suffice.
* *
...@@ -41,7 +41,7 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab ...@@ -41,7 +41,7 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab
*/ */
constructor( constructor(
address _l2DepositedToken, address _l2DepositedToken,
address _l1messenger address _l1messenger
) )
OVM_CrossDomainEnabled(_l1messenger) OVM_CrossDomainEnabled(_l1messenger)
{ {
...@@ -53,7 +53,7 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab ...@@ -53,7 +53,7 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab
********************************/ ********************************/
// Default gas value which can be overridden if more complex logic runs on L2. // Default gas value which can be overridden if more complex logic runs on L2.
uint32 public DEFAULT_FINALIZE_DEPOSIT_L2_GAS = 1200000; uint32 internal constant DEFAULT_FINALIZE_DEPOSIT_L2_GAS = 1200000;
/** /**
* @dev Core logic to be performed when a withdrawal is finalized on L1. * @dev Core logic to be performed when a withdrawal is finalized on L1.
...@@ -96,10 +96,10 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab ...@@ -96,10 +96,10 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab
* dynamic, and the above public constant does not suffice. * dynamic, and the above public constant does not suffice.
* *
*/ */
function getFinalizeDepositL2Gas() function getFinalizeDepositL2Gas()
public public
view view
virtual
returns( returns(
uint32 uint32
) )
...@@ -118,8 +118,9 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab ...@@ -118,8 +118,9 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab
function deposit( function deposit(
uint _amount uint _amount
) )
public external
override override
virtual
{ {
_initiateDeposit(msg.sender, msg.sender, _amount); _initiateDeposit(msg.sender, msg.sender, _amount);
} }
...@@ -133,8 +134,9 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab ...@@ -133,8 +134,9 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab
address _to, address _to,
uint _amount uint _amount
) )
public external
override override
virtual
{ {
_initiateDeposit(msg.sender, _to, _amount); _initiateDeposit(msg.sender, _to, _amount);
} }
...@@ -183,9 +185,9 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab ...@@ -183,9 +185,9 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab
*************************/ *************************/
/** /**
* @dev Complete a withdrawal from L2 to L1, and credit funds to the recipient's balance of the * @dev Complete a withdrawal from L2 to L1, and credit funds to the recipient's balance of the
* L1 ERC20 token. * L1 ERC20 token.
* This call will fail if the initialized withdrawal from L2 has not been finalized. * This call will fail if the initialized withdrawal from L2 has not been finalized.
* *
* @param _to L1 address to credit the withdrawal to * @param _to L1 address to credit the withdrawal to
* @param _amount Amount of the ERC20 to withdraw * @param _amount Amount of the ERC20 to withdraw
...@@ -195,7 +197,8 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab ...@@ -195,7 +197,8 @@ abstract contract Abs_L1TokenGateway is iOVM_L1TokenGateway, OVM_CrossDomainEnab
uint _amount uint _amount
) )
external external
override override
virtual
onlyFromCrossDomainAccount(l2DepositedToken) onlyFromCrossDomainAccount(l2DepositedToken)
{ {
// Call our withdrawal accounting handler implemented by child contracts. // Call our withdrawal accounting handler implemented by child contracts.
......
...@@ -55,7 +55,6 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain ...@@ -55,7 +55,6 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain
* *
* @param _l1TokenGateway Address of the corresponding L1 gateway deployed to the main chain * @param _l1TokenGateway Address of the corresponding L1 gateway deployed to the main chain
*/ */
function init( function init(
iOVM_L1TokenGateway _l1TokenGateway iOVM_L1TokenGateway _l1TokenGateway
) )
...@@ -64,7 +63,7 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain ...@@ -64,7 +63,7 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain
require(address(l1TokenGateway) == address(0), "Contract has already been initialized"); require(address(l1TokenGateway) == address(0), "Contract has already been initialized");
l1TokenGateway = _l1TokenGateway; l1TokenGateway = _l1TokenGateway;
emit Initialized(l1TokenGateway); emit Initialized(l1TokenGateway);
} }
...@@ -81,8 +80,8 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain ...@@ -81,8 +80,8 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain
* Overridable Accounting logic * * Overridable Accounting logic *
********************************/ ********************************/
// Default gas value which can be overridden if more complex logic runs on L2. // Default gas value which can be overridden if more complex logic runs on L1.
uint32 constant DEFAULT_FINALIZE_WITHDRAWAL_L1_GAS = 100000; uint32 internal constant DEFAULT_FINALIZE_WITHDRAWAL_L1_GAS = 100000;
/** /**
* @dev Core logic to be performed when a withdrawal from L2 is initialized. * @dev Core logic to be performed when a withdrawal from L2 is initialized.
...@@ -91,7 +90,6 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain ...@@ -91,7 +90,6 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain
* param _to Address being withdrawn to * param _to Address being withdrawn to
* param _amount Amount being withdrawn * param _amount Amount being withdrawn
*/ */
function _handleInitiateWithdrawal( function _handleInitiateWithdrawal(
address, // _to, address, // _to,
uint // _amount uint // _amount
...@@ -122,9 +120,7 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain ...@@ -122,9 +120,7 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain
/** /**
* @dev Overridable getter for the *L1* gas limit of settling the withdrawal, in the case it may be * @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. * dynamic, and the above public constant does not suffice.
*
*/ */
function getFinalizeWithdrawalL1Gas() function getFinalizeWithdrawalL1Gas()
public public
view view
...@@ -150,6 +146,7 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain ...@@ -150,6 +146,7 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain
) )
external external
override override
virtual
onlyInitialized() onlyInitialized()
{ {
_initiateWithdrawal(msg.sender, _amount); _initiateWithdrawal(msg.sender, _amount);
...@@ -166,6 +163,7 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain ...@@ -166,6 +163,7 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain
) )
external external
override override
virtual
onlyInitialized() onlyInitialized()
{ {
_initiateWithdrawal(_to, _amount); _initiateWithdrawal(_to, _amount);
...@@ -208,9 +206,9 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain ...@@ -208,9 +206,9 @@ 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 * @dev Complete a deposit from L1 to L2, and credits funds to the recipient's balance of this
* L2 token. * L2 token.
* This call will fail if it did not originate from a corresponding deposit in OVM_l1TokenGateway. * This call will fail if it did not originate from a corresponding deposit in OVM_l1TokenGateway.
* *
* @param _to Address to receive the withdrawal at * @param _to Address to receive the withdrawal at
* @param _amount Amount of the token to withdraw * @param _amount Amount of the token to withdraw
...@@ -220,7 +218,8 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain ...@@ -220,7 +218,8 @@ abstract contract Abs_L2DepositedToken is iOVM_L2DepositedToken, OVM_CrossDomain
uint _amount uint _amount
) )
external external
override override
virtual
onlyInitialized() onlyInitialized()
onlyFromCrossDomainAccount(address(l1TokenGateway)) onlyFromCrossDomainAccount(address(l1TokenGateway))
{ {
......
...@@ -66,7 +66,7 @@ describe('OVM_L1ERC20Gateway', () => { ...@@ -66,7 +66,7 @@ describe('OVM_L1ERC20Gateway', () => {
Mock__OVM_L1CrossDomainMessenger.address Mock__OVM_L1CrossDomainMessenger.address
) )
finalizeDepositGasLimit = await OVM_L1ERC20Gateway.DEFAULT_FINALIZE_DEPOSIT_L2_GAS() finalizeDepositGasLimit = await OVM_L1ERC20Gateway.getFinalizeDepositL2Gas()
}) })
describe('finalizeWithdrawal', () => { describe('finalizeWithdrawal', () => {
......
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