diff --git a/contracts/bridges/Accounting.sol b/contracts/bridges/Accounting.sol index fe3a2110..4e518400 100644 --- a/contracts/bridges/Accounting.sol +++ b/contracts/bridges/Accounting.sol @@ -46,7 +46,7 @@ abstract contract Accounting is Ownable, ReentrancyGuard { _; } - /// @dev Used by parent contract to ensure that the Bonder is solvent at the end of the transaction. + /// @dev Used by child contract to ensure that the Bonder is solvent at the end of the transaction. modifier requirePositiveBalance { _; require(getCredit(msg.sender) >= getDebitAndAdditionalDebit(msg.sender), "ACT: Not enough available credit"); @@ -54,6 +54,7 @@ abstract contract Accounting is Ownable, ReentrancyGuard { /// @dev Sets the Bonder addresses constructor(IBonderRegistry registry) public { + require(registry != IBonderRegistry(0), "ACT: Cannot set registry to zero address"); _registry = registry; } @@ -65,7 +66,7 @@ abstract contract Accounting is Ownable, ReentrancyGuard { function _transferToBridge(address from, uint256 amount) internal virtual; /** - * @dev This function can be optionally overridden by a parent contract to track any additional + * @dev This function can be optionally overridden by a child contract to track any additional * debit balance in an alternative way. */ function _additionalDebit(address /*bonder*/) internal view virtual returns (uint256) { diff --git a/contracts/bridges/BonderRegistry.sol b/contracts/bridges/BonderRegistry.sol index 225f2350..16568142 100644 --- a/contracts/bridges/BonderRegistry.sol +++ b/contracts/bridges/BonderRegistry.sol @@ -19,6 +19,7 @@ contract BonderRegistry is IBonderRegistry, Ownable { constructor(address[] memory bonders) public { for (uint256 i = 0; i < bonders.length; i++) { + require(bonders[i] != address(0), "BR: Cannot add zero address bonder"); isBonder[bonders[i]] = true; } } @@ -37,6 +38,7 @@ contract BonderRegistry is IBonderRegistry, Ownable { * @param bonder The address being added as a Bonder */ function addBonder(address bonder) external onlyOwner { + require(bonder != address(0), "BR: Cannot add zero address bonder"); require(isBonder[bonder] == false, "BR: Address is already bonder"); isBonder[bonder] = true; diff --git a/contracts/bridges/Bridge.sol b/contracts/bridges/Bridge.sol index 11f6a64f..c68830de 100644 --- a/contracts/bridges/Bridge.sol +++ b/contracts/bridges/Bridge.sol @@ -100,7 +100,7 @@ abstract contract Bridge is Accounting, SwapDataConsumer { } /** - * @notice getChainId can be overridden by subclasses if needed for compatibility or testing purposes. + * @notice getChainId can be overridden by child contracts if needed for compatibility or testing purposes. * @dev Get the current chainId * @return chainId The current chainId */ @@ -269,7 +269,7 @@ abstract contract Bridge is Accounting, SwapDataConsumer { bytes32 transferRootId = getTransferRootId(rootHash, transferRootTotalAmount); uint256 amount = _bondedWithdrawalAmounts[bonder][transferId]; - require(amount > 0, "L2_BRG: transferId has no bond"); + require(amount > 0, "BRG: transferId has no bond"); _bondedWithdrawalAmounts[bonder][transferId] = 0; _addToAmountWithdrawn(transferRootId, amount); @@ -300,6 +300,7 @@ abstract contract Bridge is Accounting, SwapDataConsumer { _bondedWithdrawalAmounts[bonder][transferIds[i]] = 0; } } + require(totalBondsSettled > 0, "BRG: No transfer bonds to settle"); bytes32 rootHash = Lib_MerkleTree.getMerkleRoot(transferIds); bytes32 transferRootId = getTransferRootId(rootHash, totalAmount); diff --git a/contracts/bridges/L1_Bridge.sol b/contracts/bridges/L1_Bridge.sol index 264fe226..21387851 100644 --- a/contracts/bridges/L1_Bridge.sol +++ b/contracts/bridges/L1_Bridge.sol @@ -8,6 +8,20 @@ import "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; import "./Bridge.sol"; import "./L2_Bridge.sol"; +contract IL2_Bridge is SwapDataConsumer { + // Redeclare distribute as payable allow value to be forwarded to xDomainConnector + function distribute( + address recipient, + uint256 amount, + SwapData calldata swapData, + address relayer, + uint256 relayerFee + ) + external + payable + {} +} + /** * @dev L1_Bridge is responsible for the bonding and challenging of TransferRoots. All TransferRoots * originate in the L1_Bridge through `bondTransferRoot` and are propagated up to destination L2s. @@ -100,6 +114,7 @@ abstract contract L1_Bridge is Bridge { public Bridge(_registry) { + require(_token != address(0), "L1_BRG: Cannot set token to zero address"); token = _token; } @@ -145,7 +160,7 @@ abstract contract L1_Bridge is Bridge { forwardedValue = msg.value; } - L2_Bridge(xDomainConnector).distribute( + IL2_Bridge(xDomainConnector).distribute{value: forwardedValue}( recipient, amount, swapData, @@ -328,7 +343,7 @@ abstract contract L1_Bridge is Bridge { address bonder = transferBond.bonder; timeSlotToAmountBonded[timeSlot][bonder] = timeSlotToAmountBonded[timeSlot][bonder].sub(bondAmount); - _addDebit(transferBond.bonder, bondAmount); + _addDebit(bonder, bondAmount); // Get stake for challenge uint256 challengeStakeAmount = getChallengeAmountForTransferAmount(originalAmount); diff --git a/contracts/bridges/L2_AmmWrapper.sol b/contracts/bridges/L2_AmmWrapper.sol index 90b43098..3316f6aa 100644 --- a/contracts/bridges/L2_AmmWrapper.sol +++ b/contracts/bridges/L2_AmmWrapper.sol @@ -4,12 +4,14 @@ pragma solidity 0.6.12; pragma experimental ABIEncoderV2; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; import "../saddle/Swap.sol"; import "./L2_Bridge.sol"; import "../interfaces/IWETH.sol"; import "./SwapDataConsumer.sol"; contract L2_AmmWrapper is SwapDataConsumer { + using SafeERC20 for IERC20; L2_Bridge public immutable bridge; IERC20 public immutable l2CanonicalToken; @@ -27,6 +29,11 @@ contract L2_AmmWrapper is SwapDataConsumer { ) public { + require(_bridge != L2_Bridge(0), "L2_AMM_W: Cannot set bridge to zero address"); + require(_l2CanonicalToken != IERC20(0), "L2_AMM_W: Cannot set l2CanonicalToken to zero address"); + require(_hToken != IERC20(0), "L2_AMM_W: Cannot set hToken to zero address"); + require(_exchangeAddress != Swap(0), "L2_AMM_W: Cannot set exchangeAddress to zero address"); + bridge = _bridge; l2CanonicalToken = _l2CanonicalToken; l2CanonicalTokenIsEth = _l2CanonicalTokenIsEth; @@ -50,15 +57,17 @@ contract L2_AmmWrapper is SwapDataConsumer { payable { require(amount >= bonderFee, "L2_AMM_W: Bonder fee cannot exceed amount"); + require(recipient != address(0), "L2_AMM_W: Recipient cannot be zero address"); + require(bonder != address(0), "L2_AMM_W: Bonder cannot be zero address"); if (l2CanonicalTokenIsEth) { require(msg.value == amount, "L2_AMM_W: Value does not match amount"); IWETH(address(l2CanonicalToken)).deposit{value: amount}(); } else { - require(l2CanonicalToken.transferFrom(msg.sender, address(this), amount), "L2_AMM_W: TransferFrom failed"); + l2CanonicalToken.safeTransferFrom(msg.sender, address(this), amount); } - require(l2CanonicalToken.approve(address(exchangeAddress), amount), "L2_AMM_W: Approve failed"); + l2CanonicalToken.safeApprove(address(exchangeAddress), amount); uint256 swapAmount = Swap(exchangeAddress).swap( swapData.tokenIndex, 0, @@ -84,8 +93,10 @@ contract L2_AmmWrapper is SwapDataConsumer { ) external { - require(hToken.transferFrom(msg.sender, address(this), amount), "L2_AMM_W: TransferFrom failed"); - require(hToken.approve(address(exchangeAddress), amount), "L2_AMM_W: Approve failed"); + require(recipient != address(0), "L2_AMM_W: Recipient cannot be zero address"); + + hToken.safeTransferFrom(msg.sender, address(this), amount); + hToken.safeApprove(address(exchangeAddress), amount); uint256 amountOut = 0; try Swap(exchangeAddress).swap( @@ -100,7 +111,7 @@ contract L2_AmmWrapper is SwapDataConsumer { if (amountOut == 0) { // Transfer hToken to recipient if swap fails - require(hToken.transfer(recipient, amount), "L2_AMM_W: Transfer failed"); + hToken.safeTransfer(recipient, amount); return; } @@ -109,7 +120,7 @@ contract L2_AmmWrapper is SwapDataConsumer { (bool success, ) = recipient.call{value: amountOut}(new bytes(0)); require(success, 'L2_AMM_W: ETH transfer failed'); } else { - require(l2CanonicalToken.transfer(recipient, amountOut), "L2_AMM_W: Transfer failed"); + l2CanonicalToken.safeTransfer(recipient, amountOut); } } } diff --git a/contracts/bridges/L2_Bridge.sol b/contracts/bridges/L2_Bridge.sol index c9cbc48b..2364b5ee 100644 --- a/contracts/bridges/L2_Bridge.sol +++ b/contracts/bridges/L2_Bridge.sol @@ -10,6 +10,7 @@ import "./Bridge.sol"; import "./HopBridgeToken.sol"; import "../libraries/Lib_MerkleTree.sol"; import "./L2_AmmWrapper.sol"; +import "./L1_Bridge.sol"; /** * @dev The L2_Bridge is responsible for aggregating pending Transfers into TransferRoots. Each newly @@ -21,7 +22,7 @@ abstract contract L2_Bridge is Bridge { using SafeERC20 for IERC20; HopBridgeToken public immutable hToken; - address public l1BridgeConnector; + address public bridgeConnector; L2_AmmWrapper public ammWrapper; mapping(uint256 => bool) public activeChainIds; uint256 public minimumForceCommitDelay = 1 days; @@ -29,13 +30,13 @@ abstract contract L2_Bridge is Bridge { uint256 public minBonderBps = 2; uint256 public minBonderFeeAbsolute = 0; - // destnation chain Id -> bonder address -> pending transfer Ids + // destination chain Id -> bonder address -> pending transfer Ids mapping(uint256 => mapping(address => bytes32[])) public pendingTransferIds; - // destnation chain Id -> bonder address -> pending amount + // destination chain Id -> bonder address -> pending amount mapping(uint256 => mapping(address => uint256)) public pendingAmount; - // destnation chain Id -> bonder address -> last commit time + // destination chain Id -> bonder address -> last commit time mapping(uint256 => mapping(address => uint256)) public lastCommitTime; - // destnation chain Id -> bonder address -> root index + // destination chain Id -> bonder address -> root index mapping(uint256 => mapping(address => uint256)) public rootIndex; uint256 public transferNonceIncrementer; @@ -76,7 +77,7 @@ abstract contract L2_Bridge is Bridge { ); modifier onlyL1Bridge { - require(msg.sender == l1BridgeConnector, "L2_BRG: xDomain caller must be L1 Bridge"); + require(msg.sender == bridgeConnector, "L2_BRG: xDomain caller must be L1 Bridge"); _; } @@ -88,6 +89,7 @@ abstract contract L2_Bridge is Bridge { public Bridge(registry) { + require(_hToken != HopBridgeToken(0), "L2_BRG: Cannot set hToken to zero address"); hToken = _hToken; for (uint256 i = 0; i < _activeChainIds.length; i++) { @@ -280,21 +282,16 @@ abstract contract L2_Bridge is Bridge { lastCommitTime[destinationChainId][bonder] = block.timestamp; rootIndex[destinationChainId][bonder]++; + pendingAmount[destinationChainId][bonder] = 0; + delete pendingTransferIds[destinationChainId][bonder]; - bytes memory confirmTransferRootMessage = abi.encodeWithSignature( - "confirmTransferRoot(uint256,bytes32,uint256,uint256,uint256)", + L1_Bridge(bridgeConnector).confirmTransferRoot( getChainId(), rootHash, destinationChainId, totalAmount, rootCommittedAt ); - - pendingAmount[destinationChainId][bonder] = 0; - delete pendingTransferIds[destinationChainId][bonder]; - - (bool success,) = l1BridgeConnector.call(confirmTransferRootMessage); - require(success, "L2_BRG: Call to L1 bridge failed"); } function _distribute( @@ -337,11 +334,13 @@ abstract contract L2_Bridge is Bridge { /* ========== External Config Management Functions ========== */ function setAmmWrapper(L2_AmmWrapper _ammWrapper) external onlyOwner { + require(_ammWrapper != L2_AmmWrapper(0), "L2_BRG: Cannot set ammWrapper to zero address"); ammWrapper = _ammWrapper; } - function setL1BridgeConnector(address _l1BridgeConnector) external onlyOwner { - l1BridgeConnector = _l1BridgeConnector; + function setBridgeConnector(address _bridgeConnector) external onlyOwner { + require(_bridgeConnector != address(0), "L2_BRG: Cannot set bridgeConnector to zero address"); + bridgeConnector = _bridgeConnector; } function addActiveChainIds(uint256[] calldata chainIds) external onlyOwner { diff --git a/contracts/bridges/SwapDataConsumer.sol b/contracts/bridges/SwapDataConsumer.sol index 1f545f99..95f162e2 100644 --- a/contracts/bridges/SwapDataConsumer.sol +++ b/contracts/bridges/SwapDataConsumer.sol @@ -3,7 +3,7 @@ pragma solidity 0.6.12; pragma experimental ABIEncoderV2; -contract SwapDataConsumer { +abstract contract SwapDataConsumer { struct SwapData { uint8 tokenIndex; uint256 amountOutMin; diff --git a/contracts/connectors/Connector.sol b/contracts/connectors/Connector.sol index d9d3b896..6734d6c1 100644 --- a/contracts/connectors/Connector.sol +++ b/contracts/connectors/Connector.sol @@ -3,31 +3,33 @@ pragma solidity 0.6.12; abstract contract Connector { - address public owner; - address public xDomainAddress; + address public localAddress; + address public xDomainConnector; - constructor(address _owner) public { - owner = _owner; + constructor(address _localAddress) public { + require(_localAddress != address(0), "CNR: localAddress cannot be zero address"); + localAddress = _localAddress; } - fallback () external { - if (msg.sender == owner) { + fallback () external payable { + if (msg.sender == localAddress) { _forwardCrossDomainMessage(); } else { _verifySender(); - (bool success,) = owner.call(msg.data); + (bool success,) = localAddress.call(msg.data); require(success, "CNR: Failed to forward message"); } } /** * @dev Sets the l2BridgeConnectorAddress - * @param _xDomainAddress The new bridge connector address + * @param _xDomainConnector The new bridge connector address */ - function setXDomainAddress(address _xDomainAddress) external { - require(xDomainAddress == address(0), "CNR: Connector address has already been set"); - xDomainAddress = _xDomainAddress; + function setxDomainConnector(address _xDomainConnector) external { + require(_xDomainConnector != address(0), "CNR: Cannot set xDomainConnector to zero address"); + require(xDomainConnector == address(0), "CNR: xDomainConnector has already been set"); + xDomainConnector = _xDomainConnector; } /* ========== Virtual functions ========== */ diff --git a/contracts/connectors/L1_ArbitrumConnector.sol b/contracts/connectors/L1_ArbitrumConnector.sol index a0c842d7..f6863172 100644 --- a/contracts/connectors/L1_ArbitrumConnector.sol +++ b/contracts/connectors/L1_ArbitrumConnector.sol @@ -6,16 +6,17 @@ import "./Connector.sol"; contract L1_ArbitrumConnector is Connector { constructor ( - address _owner + address _localAddress ) public - Connector(_owner) + Connector(_localAddress) {} /* ========== Override Functions ========== */ function _forwardCrossDomainMessage() internal override { // ToDo not implemented + // ToDo pass msg.value to Inbox to pay for L2 fee } function _verifySender() internal override { diff --git a/contracts/connectors/L1_OptimismConnector.sol b/contracts/connectors/L1_OptimismConnector.sol index 1ecd8396..cc2bef6f 100644 --- a/contracts/connectors/L1_OptimismConnector.sol +++ b/contracts/connectors/L1_OptimismConnector.sol @@ -10,12 +10,12 @@ contract L1_OptimismConnector is Connector { uint256 public immutable defaultGasLimit; constructor ( - address _owner, + address _localAddress, iOVM_L1CrossDomainMessenger _l1MessengerAddress, uint32 _defaultGasLimit ) public - Connector(_owner) + Connector(_localAddress) { l1MessengerAddress = _l1MessengerAddress; defaultGasLimit = _defaultGasLimit; @@ -25,7 +25,7 @@ contract L1_OptimismConnector is Connector { function _forwardCrossDomainMessage() internal override { l1MessengerAddress.sendMessage( - xDomainAddress, + xDomainConnector, msg.data, uint32(defaultGasLimit) ); @@ -33,7 +33,7 @@ contract L1_OptimismConnector is Connector { function _verifySender() internal override { require(msg.sender == address(l1MessengerAddress), "L1_OVM_CNR: Caller is not l1MessengerAddress"); - // Verify that cross-domain sender is xDomainAddress - require(l1MessengerAddress.xDomainMessageSender() == xDomainAddress, "L1_OVM_CNR: Invalid cross-domain sender"); + // Verify that cross-domain sender is xDomainConnector + require(l1MessengerAddress.xDomainMessageSender() == xDomainConnector, "L1_OVM_CNR: Invalid cross-domain sender"); } } diff --git a/contracts/connectors/L1_PolygonConnector.sol b/contracts/connectors/L1_PolygonConnector.sol index 8e39cdd6..e8685372 100644 --- a/contracts/connectors/L1_PolygonConnector.sol +++ b/contracts/connectors/L1_PolygonConnector.sol @@ -5,10 +5,10 @@ pragma solidity 0.7.3; import "../polygon/tunnel/FxBaseRootTunnel.sol"; contract L1_PolygonConnector is FxBaseRootTunnel { - address public owner; + address public localAddress; constructor ( - address _owner, + address _localAddress, address _checkpointManager, address _fxRoot, address _fxChildTunnel @@ -16,19 +16,19 @@ contract L1_PolygonConnector is FxBaseRootTunnel { public FxBaseRootTunnel(_checkpointManager, _fxRoot) { - owner = _owner; + localAddress = _localAddress; setFxChildTunnel(_fxChildTunnel); } fallback () external { - require(msg.sender == owner, "L1_PLGN_CNR: Only owner can forward messages"); + require(msg.sender == localAddress, "L1_PLGN_CNR: Only localAddress can forward messages"); _sendMessageToChild(msg.data); } /* ========== Override Functions ========== */ function _processMessageFromChild(bytes memory message) internal override { - (bool success,) = owner.call(message); - require(success, "L1_PLGN_CNR: Call to owner failed"); + (bool success,) = localAddress.call(message); + require(success, "L1_PLGN_CNR: Call to localAddress failed"); } } diff --git a/contracts/connectors/L1_XDaiConnector.sol b/contracts/connectors/L1_XDaiConnector.sol index 167cf52b..dc1eadf6 100644 --- a/contracts/connectors/L1_XDaiConnector.sol +++ b/contracts/connectors/L1_XDaiConnector.sol @@ -13,13 +13,13 @@ contract L1_XDaiConnector is Connector { uint256 public immutable defaultGasLimit; constructor ( - address _owner, + address _localAddress, IArbitraryMessageBridge _l1MessengerAddress, uint256 _defaultGasLimit, uint256 _l2ChainId ) public - Connector(_owner) + Connector(_localAddress) { l1MessengerAddress = _l1MessengerAddress; defaultGasLimit = _defaultGasLimit; @@ -30,7 +30,7 @@ contract L1_XDaiConnector is Connector { function _forwardCrossDomainMessage() internal override { l1MessengerAddress.requireToPassMessage( - xDomainAddress, + xDomainConnector, msg.data, defaultGasLimit ); @@ -38,7 +38,7 @@ contract L1_XDaiConnector is Connector { function _verifySender() internal override { require(msg.sender == address(l1MessengerAddress), "L2_XDAI_CNR: Caller is not the expected sender"); - require(l1MessengerAddress.messageSender() == xDomainAddress, "L2_XDAI_CNR: Invalid cross-domain sender"); + require(l1MessengerAddress.messageSender() == xDomainConnector, "L2_XDAI_CNR: Invalid cross-domain sender"); // With the xDai AMB, it is best practice to also check the source chainId // https://docs.tokenbridge.net/amb-bridge/how-to-develop-xchain-apps-by-amb#receive-a-method-call-from-the-amb-bridge diff --git a/contracts/connectors/L2_ArbitrumConnector.sol b/contracts/connectors/L2_ArbitrumConnector.sol index 4ef1b6f4..0346a535 100644 --- a/contracts/connectors/L2_ArbitrumConnector.sol +++ b/contracts/connectors/L2_ArbitrumConnector.sol @@ -6,10 +6,10 @@ import "./Connector.sol"; contract L2_ArbitrumConnector is Connector { constructor ( - address _owner + address _localAddress ) public - Connector(_owner) + Connector(_localAddress) {} /* ========== Override Functions ========== */ diff --git a/contracts/connectors/L2_OptimismConnector.sol b/contracts/connectors/L2_OptimismConnector.sol index e1a2b654..26f79418 100644 --- a/contracts/connectors/L2_OptimismConnector.sol +++ b/contracts/connectors/L2_OptimismConnector.sol @@ -10,12 +10,12 @@ contract L2_OptimismConnector is Connector { uint32 public defaultGasLimit; constructor ( - address _owner, + address _localAddress, iOVM_L2CrossDomainMessenger _messenger, uint32 _defaultGasLimit ) public - Connector(_owner) + Connector(_localAddress) { messenger = _messenger; defaultGasLimit = _defaultGasLimit; @@ -25,7 +25,7 @@ contract L2_OptimismConnector is Connector { function _forwardCrossDomainMessage() internal override { messenger.sendMessage( - xDomainAddress, + xDomainConnector, msg.data, defaultGasLimit ); @@ -34,6 +34,6 @@ contract L2_OptimismConnector is Connector { function _verifySender() internal override { require(msg.sender == address(messenger), "L2_OVM_CNR: Caller is not the expected sender"); // Verify that cross-domain sender is expectedSender - require(messenger.xDomainMessageSender() == xDomainAddress, "L2_OVM_CNR: Invalid cross-domain sender"); + require(messenger.xDomainMessageSender() == xDomainConnector, "L2_OVM_CNR: Invalid cross-domain sender"); } } diff --git a/contracts/connectors/L2_PolygonConnector.sol b/contracts/connectors/L2_PolygonConnector.sol index 3b324ff0..cb9c78a7 100644 --- a/contracts/connectors/L2_PolygonConnector.sol +++ b/contracts/connectors/L2_PolygonConnector.sol @@ -5,20 +5,20 @@ pragma solidity 0.7.3; import "../polygon/tunnel/FxBaseChildTunnel.sol"; contract L2_PolygonConnector is FxBaseChildTunnel { - address public owner; + address public localAddress; constructor ( - address _owner, + address _localAddress, address _fxChild ) public FxBaseChildTunnel(_fxChild) { - owner = _owner; + localAddress = _localAddress; } fallback () external { - require(msg.sender == owner, "L2_PLGN_CNR: Only owner can forward messages"); + require(msg.sender == localAddress, "L2_PLGN_CNR: Only localAddress can forward messages"); _sendMessageToRoot(msg.data); } @@ -33,7 +33,7 @@ contract L2_PolygonConnector is FxBaseChildTunnel { override validateSender(sender) { - (bool success,) = owner.call(data); + (bool success,) = localAddress.call(data); require(success, "L2_PLGN_CNR: Failed to proxy message"); } } diff --git a/contracts/connectors/L2_XDaiConnector.sol b/contracts/connectors/L2_XDaiConnector.sol index 499d18ab..6548aa98 100644 --- a/contracts/connectors/L2_XDaiConnector.sol +++ b/contracts/connectors/L2_XDaiConnector.sol @@ -12,13 +12,13 @@ contract L2_XDaiConnector is Connector { uint256 public defaultGasLimit; constructor ( - address _owner, + address _localAddress, IArbitraryMessageBridge _messenger, uint256 _l1ChainId, uint256 _defaultGasLimit ) public - Connector(_owner) + Connector(_localAddress) { messenger = _messenger; l1ChainId = bytes32(_l1ChainId); @@ -29,14 +29,14 @@ contract L2_XDaiConnector is Connector { function _forwardCrossDomainMessage() internal override { messenger.requireToPassMessage( - xDomainAddress, + xDomainConnector, msg.data, defaultGasLimit ); } function _verifySender() internal override { - require(messenger.messageSender() == xDomainAddress, "L2_XDAI_CNR: Invalid cross-domain sender"); + require(messenger.messageSender() == xDomainConnector, "L2_XDAI_CNR: Invalid cross-domain sender"); require(msg.sender == address(messenger), "L2_XDAI_CNR: Caller is not the expected sender"); // With the xDai AMB, it is best practice to also check the source chainId diff --git a/hardhat.config.ts b/hardhat.config.ts index c7a5a85d..4edbad52 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -16,11 +16,6 @@ const desiredAccounts: string[] = [ process.env.GOVERNANCE_PRIVATE_KEY ] -const isOptimizerEnabled: boolean = true -// 50k for normal, 1 for Optimism -// const numOptimizerRuns: number = 1 -const numOptimizerRuns: number = 50000 - // You have to export an object to set up your config // This object can have the following optional entries: // defaultNetwork, networks, solc, and paths. @@ -107,84 +102,30 @@ export default { solidity: { compilers: [ { - settings: { - optimizer: { - enabled: isOptimizerEnabled, - runs: numOptimizerRuns - } - }, version: '0.7.3' }, { - settings: { - optimizer: { - enabled: isOptimizerEnabled, - runs: numOptimizerRuns - } - }, version: '0.7.0' }, { - settings: { - optimizer: { - enabled: isOptimizerEnabled, - runs: numOptimizerRuns - } - }, version: '0.6.12' }, { - settings: { - optimizer: { - enabled: isOptimizerEnabled, - runs: numOptimizerRuns - } - }, version: '0.6.11' }, { - settings: { - optimizer: { - enabled: isOptimizerEnabled, - runs: numOptimizerRuns - } - }, version: '0.6.6' }, { - settings: { - optimizer: { - enabled: isOptimizerEnabled, - runs: numOptimizerRuns - } - }, version: '0.5.17' }, { - settings: { - optimizer: { - enabled: isOptimizerEnabled, - runs: numOptimizerRuns - } - }, version: '0.5.16' }, { - settings: { - optimizer: { - enabled: isOptimizerEnabled, - runs: numOptimizerRuns - } - }, version: '0.5.11' }, { - settings: { - optimizer: { - enabled: isOptimizerEnabled, - runs: numOptimizerRuns - } - }, version: '0.4.25' } ] diff --git a/scripts/contractState/getUpdateContractStateMessage.ts b/scripts/contractState/getUpdateContractStateMessage.ts index 18bc9018..678eb929 100644 --- a/scripts/contractState/getUpdateContractStateMessage.ts +++ b/scripts/contractState/getUpdateContractStateMessage.ts @@ -7,7 +7,7 @@ import { getRemoveBonderMessage, getSetL1GovernanceMessage, getSetAmmWrapperMessage, - getSetL1BridgeConnectorMessage, + getSetBridgeConnectorMessage, getSetL1CallerMessage, getAddActiveChainIdsMessage, getRemoveActiveChainIdsMessage, @@ -26,7 +26,7 @@ const FUNCTIONS = { REMOVE_BONDER: 'removeBonder', SET_L1_GOVERNANCE: 'setL1Governance', SET_AMM_WRAPPER: 'setAmmWrapper', - SET_L1_BRIDGE_ADDRESS: 'setL1BridgeConnector', + SET_L1_BRIDGE_ADDRESS: 'setBridgeConnector', SET_L1_BRIDGE_CALLER: 'setL1Caller', ADD_ACTIVE_CHAIN_IDS: 'addActiveChainIds', REMOVE_ACTIVE_CHAIN_IDS: 'removeActiveChainIds', @@ -102,7 +102,7 @@ const getMessageToSend = ( return getSetAmmWrapperMessage(input) } case FUNCTIONS.SET_L1_BRIDGE_ADDRESS.toLowerCase(): { - return getSetL1BridgeConnectorMessage(input) + return getSetBridgeConnectorMessage(input) } case FUNCTIONS.SET_L1_BRIDGE_CALLER.toLowerCase(): { return getSetL1CallerMessage(input) diff --git a/scripts/other/sendMessageOverHopBridge.ts b/scripts/other/sendMessageOverHopBridge.ts index c39af5c9..4b83ca29 100644 --- a/scripts/other/sendMessageOverHopBridge.ts +++ b/scripts/other/sendMessageOverHopBridge.ts @@ -9,7 +9,7 @@ import { import { executeCanonicalMessengerSendMessage, - getSetL1BridgeConnectorMessage, + getSetBridgeConnectorMessage, getAddBonderMessage, } from '../../test/shared/contractFunctionWrappers' diff --git a/test/bridges/L1_Bridge.test.ts b/test/bridges/L1_Bridge.test.ts index 83702c0e..5461515d 100644 --- a/test/bridges/L1_Bridge.test.ts +++ b/test/bridges/L1_Bridge.test.ts @@ -1912,7 +1912,7 @@ describe('L1_Bridge', () => { it('Should not allow a transfer to L2 via sendToL2 if the user did not approve the token transfer to the L1 Bridge', async () => { const expectedErrorMsg: string = - ' ERC20: transfer amount exceeds allowance' + 'ERC20: transfer amount exceeds allowance' const tokenAmount = await l1_canonicalToken.balanceOf( await user.getAddress() ) @@ -2292,7 +2292,7 @@ describe('L1_Bridge', () => { }) it('Should not let an arbitrary bonder settle a bonded withdrawal', async () => { - const expectedErrorMsg: string = 'L2_BRG: transferId has no bond' + const expectedErrorMsg: string = 'BRG: transferId has no bond' await executeL1BridgeSendToL2( l1_canonicalToken, @@ -2351,9 +2351,55 @@ describe('L1_Bridge', () => { describe('settleBondedWithdrawals', async () => { it('Should not settle bonded withdrawals because the transfer root is not found', async () => { + await executeL1BridgeSendToL2( + l1_canonicalToken, + l1_bridge, + l2_hopBridgeToken, + l2_canonicalToken, + l2_messenger, + l2_swap, + transfer.sender, + transfer.recipient, + relayer, + transfer.amount.mul(2), + transfer.amountOutMin, + transfer.deadline, + defaultRelayerFee, + l2ChainId + ) + + await executeL2BridgeSend(l2_hopBridgeToken, l2_bridge, transfer) + + await executeBridgeBondWithdrawal( + l1_canonicalToken, + l1_bridge, + l2_bridge, + transfer, + bonder + ) + + const transferIndex: BigNumber = BigNumber.from('1') + await executeL2BridgeSend(l2_hopBridgeToken, l2_bridge, transfer, transferIndex) + + await executeBridgeBondWithdrawal( + l1_canonicalToken, + l1_bridge, + l2_bridge, + transfer, + bonder, + transferIndex + ) + + const numTransfers: BigNumber = BigNumber.from('2') + let totalTransferAmount: BigNumber = BigNumber.from('0') + let transferIds: Buffer[] = [] + for (let i = 0; i < numTransfers.toNumber(); i++) { + const transferNonce: string = await getTransferNonceFromEvent(l2_bridge, BigNumber.from(i)) + transferIds.push(await transfer.getTransferId(transferNonce)) + totalTransferAmount = totalTransferAmount.add(transfers[i].amount) + } + const expectedErrorMsg: string = 'BRG: Transfer root not found' - const transferIds: string[] = [ARBITRARY_ROOT_HASH] - const totalTransferAmount: BigNumber = BigNumber.from('123') await expect( l1_bridge @@ -2610,7 +2656,7 @@ describe('L1_Bridge', () => { // Unset the supported chainId for this test await l1_bridge.connect(governance).setXDomainConnector( l2Transfer.chainId, - ZERO_ADDRESS + DEAD_ADDRESS ) await expect(l1_messenger.relayNextMessage()).to.be.revertedWith( @@ -3794,13 +3840,18 @@ describe('L1_Bridge', () => { const creditBefore = await l1_bridge.getCredit(await bonder.getAddress()) // Send tx with arbitrary bonder address - await l1_bridge - .connect(bonder) - .settleBondedWithdrawals( - await user.getAddress(), - transferIds, - totalTransferAmount - ) + + const expectedErrorMsg: string = 'BRG: No transfer bonds to settle' + + await expect( + l1_bridge + .connect(bonder) + .settleBondedWithdrawals( + await user.getAddress(), + transferIds, + totalTransferAmount + ) + ).to.be.revertedWith(expectedErrorMsg) // Validate state after transaction const currentTime: number = Math.floor(Date.now() / 1000) @@ -3836,7 +3887,7 @@ describe('L1_Bridge', () => { }) it('Should settle bonded withdrawals and update state. Should then try to settle a single withdrawal and fail.', async () => { - const expectedErrorMsg: string = 'L2_BRG: transferId has no bond' + const expectedErrorMsg: string = 'BRG: transferId has no bond' await executeL1BridgeSendToL2( l1_canonicalToken, l1_bridge, @@ -4237,16 +4288,21 @@ describe('L1_Bridge', () => { const transferNonce: string = await getTransferNonceFromEvent(l2_bridge) const transferId: Buffer = await transfer.getTransferId(transferNonce) const totalTransferAmount: BigNumber = transfer.amount - await l1_bridge - .connect(bonder) - .settleBondedWithdrawals( - await bonder.getAddress(), - [transferId], - totalTransferAmount - ) - const credit = await l1_bridge.getCredit(await otherUser.getAddress()) - expect(credit).to.eq(BigNumber.from('0')) + const expectedErrorMsg: string = 'BRG: No transfer bonds to settle' + + await expect( + l1_bridge + .connect(bonder) + .settleBondedWithdrawals( + await bonder.getAddress(), + [transferId], + totalTransferAmount + ) + ).to.be.revertedWith(expectedErrorMsg) + + const credit = await l1_bridge.getCredit(await otherUser.getAddress()) + expect(credit).to.eq(BigNumber.from('0')) }) it('Should settle bonded withdrawals and update state with 3, 5, 7, 11, 12, and 15 transfers.', async () => { diff --git a/test/bridges/L2_AmmWrapper.test.ts b/test/bridges/L2_AmmWrapper.test.ts index e5359b5b..2703c2c0 100644 --- a/test/bridges/L2_AmmWrapper.test.ts +++ b/test/bridges/L2_AmmWrapper.test.ts @@ -30,7 +30,7 @@ import { executeL2BridgeBondWithdrawalAndDistribute, executeCanonicalMessengerSendMessage, getSetAmmWrapperMessage, - getSetL1BridgeConnectorMessage, + getSetBridgeConnectorMessage, getSetL1CallerMessage, getAddActiveChainIdsMessage, getRemoveActiveChainIdsMessage, diff --git a/test/bridges/L2_Bridge.test.ts b/test/bridges/L2_Bridge.test.ts index 46c57fd2..68891773 100644 --- a/test/bridges/L2_Bridge.test.ts +++ b/test/bridges/L2_Bridge.test.ts @@ -30,7 +30,7 @@ import { executeL2BridgeBondWithdrawalAndDistribute, executeCanonicalMessengerSendMessage, getSetAmmWrapperMessage, - getSetL1BridgeConnectorMessage, + getSetBridgeConnectorMessage, getSetL1CallerMessage, getAddActiveChainIdsMessage, getRemoveActiveChainIdsMessage, @@ -240,14 +240,14 @@ describe('L2_Bridge', () => { }) it('Should set the L1 bridge connector arbitrarily', async () => { - const expectedL1BridgeConnectorAddress: string = ONE_ADDRESS + const expectedBridgeConnectorAddress: string = ONE_ADDRESS - await l2_bridge.connect(governance).setL1BridgeConnector( - expectedL1BridgeConnectorAddress + await l2_bridge.connect(governance).setBridgeConnector( + expectedBridgeConnectorAddress ) - const l1BridgeAddress: string = await l2_bridge.l1BridgeConnector() - expect(l1BridgeAddress).to.eq(expectedL1BridgeConnectorAddress) + const l1BridgeAddress: string = await l2_bridge.bridgeConnector() + expect(l1BridgeAddress).to.eq(expectedBridgeConnectorAddress) }) it('Should add support for a new chainId', async () => { @@ -620,7 +620,7 @@ describe('L2_Bridge', () => { const expectedErrorMsg: string = 'Ownable: caller is not the owner' const expectedL1BridgeAddress: string = ONE_ADDRESS - const message: string = getSetL1BridgeConnectorMessage( + const message: string = getSetBridgeConnectorMessage( expectedL1BridgeAddress ) await expect( diff --git a/test/shared/contractFunctionWrappers.ts b/test/shared/contractFunctionWrappers.ts index 00f2e0a9..f930fdd4 100644 --- a/test/shared/contractFunctionWrappers.ts +++ b/test/shared/contractFunctionWrappers.ts @@ -1306,11 +1306,11 @@ export const getSetL1GovernanceMessage = (l1_governanceAddress: string) => { return ethersInterface.encodeFunctionData('setL1Governance', [l1_governanceAddress]) } -export const getSetL1BridgeConnectorMessage = (l1_bridge: Contract | string) => { +export const getSetBridgeConnectorMessage = (l1_bridge: Contract | string) => { const address = getAddressFromContractOrString(l1_bridge) - const ABI = ['function setL1BridgeConnector(address _l1BridgeAddress)'] + const ABI = ['function setBridgeConnector(address _l1BridgeAddress)'] const ethersInterface = new ethersUtils.Interface(ABI) - return ethersInterface.encodeFunctionData('setL1BridgeConnector', [address]) + return ethersInterface.encodeFunctionData('setBridgeConnector', [address]) } export const getSetL1CallerMessage = ( diff --git a/test/shared/fixtures.ts b/test/shared/fixtures.ts index 74eb00bf..d65ce8d3 100644 --- a/test/shared/fixtures.ts +++ b/test/shared/fixtures.ts @@ -211,8 +211,8 @@ export async function fixture ( ) if (!isChainIdPolygon(l2ChainId)) { - await l1_messengerWrapper.setXDomainAddress(l2_bridgeConnector.address) - await l2_bridgeConnector.setXDomainAddress(l1_messengerWrapper.address) + await l1_messengerWrapper.setxDomainConnector(l2_bridgeConnector.address) + await l2_bridgeConnector.setxDomainConnector(l1_messengerWrapper.address) } // Deploy AMM contracts diff --git a/test/shared/utils.ts b/test/shared/utils.ts index d5b0591c..e543d45b 100644 --- a/test/shared/utils.ts +++ b/test/shared/utils.ts @@ -25,7 +25,7 @@ import { executeCanonicalMessengerSendMessage, executeCanonicalBridgeSendTokens, executeL1BridgeSendToL2, - getSetL1BridgeConnectorMessage, + getSetBridgeConnectorMessage, getSetL1CallerMessage, getSetAmmWrapperMessage } from './contractFunctionWrappers' @@ -151,7 +151,7 @@ export const setUpL1AndL2Bridges = async (fixture: IFixture, opts: any) => { l1_messengerWrapper.address ) // Set up L2 - await l2_bridge.connect(governance).setL1BridgeConnector(l2_bridgeConnector.address) + await l2_bridge.connect(governance).setBridgeConnector(l2_bridgeConnector.address) await l2_bridge.connect(governance).setAmmWrapper(l2_ammWrapper.address) } diff --git a/test/wrappers/ArbitrumMessengerWrapper.test.ts b/test/wrappers/ArbitrumMessengerWrapper.test.ts index 9f665bed..a9e7079d 100644 --- a/test/wrappers/ArbitrumMessengerWrapper.test.ts +++ b/test/wrappers/ArbitrumMessengerWrapper.test.ts @@ -44,7 +44,7 @@ describe.skip('Arbitrum Messenger Wrapper', () => { it('Should set the correct values in the constructor', async () => { const expectedL1BridgeAddress: string = l1_bridge.address - const l1BridgeAddress: string = await l1_messengerWrapper.owner() + const l1BridgeAddress: string = await l1_messengerWrapper.localAddress() expect(expectedL1BridgeAddress).to.eq(l1BridgeAddress) }) diff --git a/test/wrappers/OptimismMessengerWrapper.test.ts b/test/wrappers/OptimismMessengerWrapper.test.ts index 57191c11..837eaf11 100644 --- a/test/wrappers/OptimismMessengerWrapper.test.ts +++ b/test/wrappers/OptimismMessengerWrapper.test.ts @@ -38,17 +38,17 @@ describe('Optimism Messenger Wrapper', () => { it('Should set the correct values in the constructor ', async () => { const expectedL1BridgeAddress: string = l1_bridge.address - const expectedXDomainAddress: string = l2_bridgeConnector.address + const expectedxDomainConnector: string = l2_bridgeConnector.address const expectedDefaultGasLimit: number = DEFAULT_MESSENGER_WRAPPER_GAS_LIMIT const expectedL1MessengerAddress: string = l1_messenger.address - const l1BridgeAddress: string = await l1_messengerWrapper.owner() - const xDomainAddress: string = await l1_messengerWrapper.xDomainAddress() + const l1BridgeAddress: string = await l1_messengerWrapper.localAddress() + const xDomainConnector: string = await l1_messengerWrapper.xDomainConnector() const defaultGasLimit: string = await l1_messengerWrapper.defaultGasLimit() const l1MessengerAddress: string = await l1_messengerWrapper.l1MessengerAddress() expect(expectedL1BridgeAddress).to.eq(l1BridgeAddress) - expect(expectedXDomainAddress).to.eq(xDomainAddress) + expect(expectedxDomainConnector).to.eq(xDomainConnector) expect(expectedDefaultGasLimit).to.eq(defaultGasLimit) expect(expectedL1MessengerAddress).to.eq(l1MessengerAddress) }) diff --git a/test/wrappers/PolygonWrapper.test.ts b/test/wrappers/PolygonWrapper.test.ts index 1da20330..388ddf86 100644 --- a/test/wrappers/PolygonWrapper.test.ts +++ b/test/wrappers/PolygonWrapper.test.ts @@ -16,7 +16,7 @@ import { } from '../../config/utils' import { - getSetL1BridgeConnectorMessage + getSetBridgeConnectorMessage } from '../shared/contractFunctionWrappers' export const MAX_NUM_SENDS_BEFORE_COMMIT = 10 @@ -62,7 +62,7 @@ describe.skip('Polygon Wrapper', () => { const expectedFxRoot: string = fxRoot.address const expectedFxChildTunnel: string = l2_bridgeConnector.address - const l1BridgeAddress: string = await l1_messengerWrapper.owner() + const l1BridgeAddress: string = await l1_messengerWrapper.localAddress() const checkpointManager: string = await l1_messengerWrapper.checkpointManager() const fxRootAddress: string = await l1_messengerWrapper.fxRoot() const fxChildTunnel: string = await l1_messengerWrapper.fxChildTunnel() @@ -74,7 +74,7 @@ describe.skip('Polygon Wrapper', () => { }) it('Should allow anyone to send a cross domain message', async () => { - const message: string = getSetL1BridgeConnectorMessage(ONE_ADDRESS) + const message: string = getSetBridgeConnectorMessage(ONE_ADDRESS) await l1_messengerWrapper.connect(user).sendCrossDomainMessage(message) const messengerWrapperMessage: string = ethersUtils.defaultAbiCoder.encode( diff --git a/test/wrappers/XDaiMessengerWrapper.test.ts b/test/wrappers/XDaiMessengerWrapper.test.ts index a3e0b209..2023d9a8 100644 --- a/test/wrappers/XDaiMessengerWrapper.test.ts +++ b/test/wrappers/XDaiMessengerWrapper.test.ts @@ -51,7 +51,7 @@ describe('XDai Messenger Wrapper', () => { const expectedDefaultGasLimit: number = 1000000 const expectedL2ChainId: string = ethersUtils.hexZeroPad(l2ChainId.toHexString(), 32) - const l1BridgeAddress: string = await l1_messengerWrapper.owner() + const l1BridgeAddress: string = await l1_messengerWrapper.localAddress() const l1MessengerAddress: string = await l1_messengerWrapper.l1MessengerAddress() const defaultGasLimit: number = await l1_messengerWrapper.defaultGasLimit() const actualL2ChainId: string = await l1_messengerWrapper.l2ChainId()