Update AddressAliasHelper library to work with solidity 0.8 version (#58

)

* test: Add test that bricks the executor with an underflowing address

* fix: Updates the AddressAliasHelper library with 0.8 solidity version

contracts/dependencies/arbitrum/AddressAliasHelper.sol

@@ -1,39 +1,29 @@
-// SPDX-License-Identifier: Apache-2.0
+// Copyright 2021-2022, Offchain Labs, Inc.
+// For license information, see https://github.com/nitro/blob/master/LICENSE
+// SPDX-License-Identifier: BUSL-1.1
 
-/*
- * Copyright 2019-2021, Offchain Labs, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-pragma solidity >=0.7.5;
+pragma solidity ^0.8.0;
 
 library AddressAliasHelper {
-  uint160 constant offset = uint160(0x1111000000000000000000000000000000001111);
+    uint160 internal constant OFFSET = uint160(0x1111000000000000000000000000000000001111);
 
-  /// @notice Utility function that converts the address in the L1 that submitted a tx to
-  /// the inbox to the msg.sender viewed in the L2
-  /// @param l1Address the address in the L1 that triggered the tx to L2
-  /// @return l2Address L2 address as viewed in msg.sender
-  function applyL1ToL2Alias(address l1Address) internal pure returns (address l2Address) {
-    l2Address = address(uint160(l1Address) + offset);
-  }
+    /// @notice Utility function that converts the address in the L1 that submitted a tx to
+    /// the inbox to the msg.sender viewed in the L2
+    /// @param l1Address the address in the L1 that triggered the tx to L2
+    /// @return l2Address L2 address as viewed in msg.sender
+    function applyL1ToL2Alias(address l1Address) internal pure returns (address l2Address) {
+        unchecked {
+            l2Address = address(uint160(l1Address) + OFFSET);
+        }
+    }
 
-  /// @notice Utility function that converts the msg.sender viewed in the L2 to the
-  /// address in the L1 that submitted a tx to the inbox
-  /// @param l2Address L2 address as viewed in msg.sender
-  /// @return l1Address the address in the L1 that triggered the tx to L2
-  function undoL1ToL2Alias(address l2Address) internal pure returns (address l1Address) {
-    l1Address = address(uint160(l2Address) - offset);
-  }
-}
+    /// @notice Utility function that converts the msg.sender viewed in the L2 to the
+    /// address in the L1 that submitted a tx to the inbox
+    /// @param l2Address L2 address as viewed in msg.sender
+    /// @return l1Address the address in the L1 that triggered the tx to L2
+    function undoL1ToL2Alias(address l2Address) internal pure returns (address l1Address) {
+        unchecked {
+            l1Address = address(uint160(l2Address) - OFFSET);
+        }
+    }
+}

helpers/arbitrum-helpers.ts

@@ -1,8 +1,18 @@
 import { ethers, BigNumber } from 'ethers';
+import { ZERO_ADDRESS } from './constants';
+
+export const ALIASING_OFFSET = '0x1111000000000000000000000000000000001111';
 
 export const applyL1ToL2Alias = (l1Address: string) => {
-  const offset = BigNumber.from('0x1111000000000000000000000000000000001111');
-  return ethers.utils.getAddress(
-    BigNumber.from(l1Address).add(offset).mod(BigNumber.from(2).pow(160)).toHexString()
-  );
+  const offset = BigNumber.from(ALIASING_OFFSET);
+  const l2Address = BigNumber.from(l1Address).add(offset).mod(BigNumber.from(2).pow(160));
+  if (l2Address.eq(0)) return ZERO_ADDRESS;
+  return ethers.utils.getAddress(l2Address.toHexString());
+};
+
+export const undoL1ToL2Alias = (l2Address: string) => {
+  const offset = BigNumber.from(ALIASING_OFFSET);
+  const l1Address = BigNumber.from(l2Address).sub(offset).mod(BigNumber.from(2).pow(160));
+  if (l1Address.eq(0)) return ZERO_ADDRESS;
+  return ethers.utils.getAddress(l1Address.toHexString());
 };

test/ArbitrumBridgeExecutor.spec.ts

@@ -18,10 +18,12 @@ import {
   setBlocktime,
   timeLatest,
   setCode,
+  getImpersonatedSigner,
 } from '../helpers/misc-utils';
 import { ONE_ADDRESS, ZERO_ADDRESS } from '../helpers/constants';
 import { ActionsSetState, ExecutorErrors } from './helpers/executor-helpers';
-import { applyL1ToL2Alias } from '../helpers/arbitrum-helpers';
+import { ALIASING_OFFSET, applyL1ToL2Alias, undoL1ToL2Alias } from '../helpers/arbitrum-helpers';
+import { parseEther } from 'ethers/lib/utils';
 
 chai.use(solidity);
 
@@ -635,5 +637,72 @@ describe('ArbitrumBridgeExecutor', async function () {
         NEW_ETHEREUM_GOVERNANCE_EXECUTOR_ADDRESS
       );
     });
+
+    it('Update the Ethereum Governance Executor with an l1Address that underflow when undoing Arbitrum aliasing', async () => {
+      expect(await bridgeExecutor.getEthereumGovernanceExecutor()).to.be.equal(
+        ethereumGovernanceExecutor.address
+      );
+
+      const upperBoundAddress = ethers.utils.getAddress(
+        BigNumber.from(2).pow(160).sub(1).toHexString() // 0xFF
+      );
+      const underflowingL2Address = ethers.utils.getAddress(
+        BigNumber.from(ALIASING_OFFSET).sub(1).toHexString()
+      );
+      const underflowingL1Address = undoL1ToL2Alias(underflowingL2Address);
+      expect(underflowingL1Address).to.be.eq(upperBoundAddress);
+
+      const NEW_ETHEREUM_GOVERNANCE_EXECUTOR_ADDRESS = underflowingL1Address;
+
+      const { data, encodedData } = encodeSimpleActionsSet(
+        bridgeExecutor,
+        bridgeExecutor.address,
+        'updateEthereumGovernanceExecutor(address)',
+        [NEW_ETHEREUM_GOVERNANCE_EXECUTOR_ADDRESS]
+      );
+      const retryableTicket = getSimpleRetryableTicket(bridgeExecutor.address, encodedData);
+
+      const tx = await arbitrumInbox.createRetryableTicket(
+        retryableTicket.destAddr,
+        retryableTicket.arbTxCallValue,
+        retryableTicket.maxSubmissionCost,
+        retryableTicket.submissionRefundAddress,
+        retryableTicket.valueRefundAddress,
+        retryableTicket.maxGas,
+        retryableTicket.gasPriceBid,
+        retryableTicket.data,
+        {
+          gasLimit: 12000000,
+        }
+      );
+
+      const executionTime = (await timeLatest()).add(DELAY);
+      expect(tx)
+        .to.emit(bridgeExecutor, 'ActionsSetQueued')
+        .withArgs(0, data[0], data[1], data[2], data[3], data[4], executionTime);
+
+      await setBlocktime(executionTime.add(1).toNumber());
+      await advanceBlocks(1);
+
+      expect(await bridgeExecutor.execute(0))
+        .to.emit(bridgeExecutor, 'ActionsSetExecuted')
+        .withArgs(0, user.address, ['0x'])
+        .to.emit(bridgeExecutor, 'EthereumGovernanceExecutorUpdate')
+        .withArgs(ethereumGovernanceExecutor.address, NEW_ETHEREUM_GOVERNANCE_EXECUTOR_ADDRESS);
+      expect(await bridgeExecutor.getEthereumGovernanceExecutor()).to.be.equal(
+        NEW_ETHEREUM_GOVERNANCE_EXECUTOR_ADDRESS
+      );
+
+      // Impersonate ethereum gov executor (with the l2 alias already applied)
+      const ethGovExecutor = await getImpersonatedSigner(underflowingL2Address);
+      await user.sendTransaction({ to: underflowingL2Address, value: parseEther('1') });
+
+      // Queues a new action set without issues
+      expect(
+        await bridgeExecutor
+          .connect(ethGovExecutor)
+          .queue([ZERO_ADDRESS], [0], ['mock()'], ['0x'], [false])
+      );
+    });
   });
 });