Add utilities for CrossChain messaging

contracts/access/AccessControlCrossChain.sol

@@ -14,7 +14,7 @@ import "../crosschain/CrossChainEnabled.sol";
  * if an address `x` has role `SOME_ROLE`, it would be able to call `myFunction` directly.
  * A wallet or contract at the same address on another chain would however not be able
  * to call this function. In order to do so, it would require to have the role
- * `_aliasRole(SOME_ROLE)`.
+ * `_crossChainRoleAlias(SOME_ROLE)`.
  *
  * This aliasing is required to protect against multiple contracts living at the same
  * address on different chains but controlled by conflicting entities.
@@ -29,7 +29,7 @@ abstract contract AccessControlCrossChain is AccessControl, CrossChainEnabled {
      */
     function _checkRole(bytes32 role) internal view virtual override {
         if (_isCrossChain()) {
-            _checkRole(_aliasRole(role), _crossChainSender());
+            _checkRole(_crossChainRoleAlias(role), _crossChainSender());
         } else {
             super._checkRole(role);
         }
@@ -38,7 +38,7 @@ abstract contract AccessControlCrossChain is AccessControl, CrossChainEnabled {
     /**
      * @dev Returns the aliased role corresponding to `role`.
      */
-    function _aliasRole(bytes32 role) internal pure virtual returns (bytes32) {
+    function _crossChainRoleAlias(bytes32 role) internal pure virtual returns (bytes32) {
         return role ^ CROSSCHAIN_ALIAS;
     }
 }

contracts/crosschain/CrossChainEnabled.sol

@@ -16,7 +16,7 @@ pragma solidity ^0.8.0;
  */
 abstract contract CrossChainEnabled {
     error NotCrossChainCall();
-    error InvalidCrossChainSender(address sender, address expected);
+    error InvalidCrossChainSender(address actual, address expected);
 
     /**
      * @dev Throws if the current function call is not the result of a
@@ -31,9 +31,9 @@ abstract contract CrossChainEnabled {
      * @dev Throws if the current function call is not the result of a
      * cross-chain execution initiated by `account`.
      */
-    modifier onlyCrossChainSender(address account) {
-        address sender = _crossChainSender();
-        if (account != sender) revert InvalidCrossChainSender(sender, account);
+    modifier onlyCrossChainSender(address expected) {
+        address actual = _crossChainSender();
+        if (expected != actual) revert InvalidCrossChainSender(actual, expected);
         _;
     }
 
@@ -47,8 +47,8 @@ abstract contract CrossChainEnabled {
      * @dev Returns the address of the sender of the cross-chain message that
      * triggered the current function call.
      *
-     * NOTE: Should revert if the current function call is not the result of a
-     * cross-chain message.
+     * IMPORTANT: Should revert with `NotCrossChainCall` if the current function
+     * call is not the result of a cross-chain message.
      */
     function _crossChainSender() internal view virtual returns (address);
 }

contracts/crosschain/amb/CrossChainEnabledAMB.sol

@@ -23,8 +23,10 @@ import "./LibAMB.sol";
  * _Available since v4.6._
  */
 contract CrossChainEnabledAMB is CrossChainEnabled {
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address private immutable _bridge;
 
+    /// @custom:oz-upgrades-unsafe-allow constructor
     constructor(address bridge) {
         _bridge = bridge;
     }

contracts/crosschain/arbitrum/CrossChainEnabledArbitrumL1.sol

@@ -19,8 +19,10 @@ import "./LibArbitrumL1.sol";
  * _Available since v4.6._
  */
 abstract contract CrossChainEnabledArbitrumL1 is CrossChainEnabled {
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address private immutable _inbox;
 
+    /// @custom:oz-upgrades-unsafe-allow constructor
     constructor(address inbox) {
         _inbox = inbox;
     }

contracts/crosschain/arbitrum/CrossChainEnabledArbitrumL2.sol

@@ -22,13 +22,13 @@ abstract contract CrossChainEnabledArbitrumL2 is CrossChainEnabled {
      * @dev see {CrossChainEnabled-_isCrossChain}
      */
     function _isCrossChain() internal view virtual override returns (bool) {
-        return LibArbitrumL2.isCrossChain(address(LibArbitrumL2.ARBSYS));
+        return LibArbitrumL2.isCrossChain(LibArbitrumL2.ARBSYS);
     }
 
     /**
      * @dev see {CrossChainEnabled-_crossChainSender}
      */
     function _crossChainSender() internal view virtual override onlyCrossChain returns (address) {
-        return LibArbitrumL2.crossChainSender(address(LibArbitrumL2.ARBSYS));
+        return LibArbitrumL2.crossChainSender(LibArbitrumL2.ARBSYS);
     }
 }

contracts/crosschain/arbitrum/LibArbitrumL1.sol

@@ -16,20 +16,23 @@ import {IOutbox as ArbitrumL1_Outbox} from "../../vendor/arbitrum/IOutbox.sol";
 library LibArbitrumL1 {
     /**
      * @dev Returns whether the current function call is the result of a
-     * cross-chain message relayed by `bridge`.
+     * cross-chain message relayed by the bridge attached to `inbox`.
      */
     function isCrossChain(address inbox) internal view returns (bool) {
         return msg.sender == ArbitrumL1_Inbox(inbox).bridge();
     }
 
     /**
      * @dev Returns the address of the sender that triggered the current
-     * cross-chain message through `bridge`.
+     * cross-chain message through the bridge attached to `inbox`.
      *
      * NOTE: {isCrossChain} should be checked before trying to recover the
      * sender.
      */
     function crossChainSender(address inbox) internal view returns (address) {
-        return ArbitrumL1_Outbox(ArbitrumL1_Bridge(ArbitrumL1_Inbox(inbox).bridge()).activeOutbox()).l2ToL1Sender();
+        address sender = ArbitrumL1_Outbox(ArbitrumL1_Bridge(ArbitrumL1_Inbox(inbox).bridge()).activeOutbox())
+            .l2ToL1Sender();
+        require(sender != address(0), "LibArbitrumL1: system messages without sender");
+        return sender;
     }
 }

contracts/crosschain/arbitrum/LibArbitrumL2.sol

@@ -14,25 +14,25 @@ import {IArbSys as ArbitrumL2_Bridge} from "../../vendor/arbitrum/IArbSys.sol";
 library LibArbitrumL2 {
     /**
      * @dev Returns whether the current function call is the result of a
-     * cross-chain message relayed by `bridge`.
+     * cross-chain message relayed by `arbsys`.
      */
-    ArbitrumL2_Bridge public constant ARBSYS = ArbitrumL2_Bridge(0x0000000000000000000000000000000000000064);
+    address public constant ARBSYS = 0x0000000000000000000000000000000000000064;
 
-    function isCrossChain(address bridge) internal view returns (bool) {
-        return ArbitrumL2_Bridge(bridge).isTopLevelCall();
+    function isCrossChain(address arbsys) internal view returns (bool) {
+        return ArbitrumL2_Bridge(arbsys).isTopLevelCall();
     }
 
     /**
      * @dev Returns the address of the sender that triggered the current
-     * cross-chain message through `bridge`.
+     * cross-chain message through `arbsys`.
      *
      * NOTE: {isCrossChain} should be checked before trying to recover the
      * sender.
      */
-    function crossChainSender(address bridge) internal view returns (address) {
+    function crossChainSender(address arbsys) internal view returns (address) {
         return
-            ArbitrumL2_Bridge(bridge).wasMyCallersAddressAliased()
-                ? ArbitrumL2_Bridge(bridge).myCallersAddressWithoutAliasing()
+            ArbitrumL2_Bridge(arbsys).wasMyCallersAddressAliased()
+                ? ArbitrumL2_Bridge(arbsys).myCallersAddressWithoutAliasing()
                 : msg.sender;
     }
 }

contracts/crosschain/optimism/CrossChainEnabledOptimism.sol

@@ -9,30 +9,32 @@ import "./LibOptimism.sol";
  * @dev [Optimism](https://www.optimism.io/) specialization or the
  * {CrossChainEnabled} abstraction.
  *
- * The bridge (`CrossDomainMessenger`) contract is provided and maintained by
+ * The messenger (`CrossDomainMessenger`) contract is provided and maintained by
  * the optimism team. You can find the address of this contract on mainnet and
  * kovan in the [deployments section of Optimism monorepo](https://github.com/ethereum-optimism/optimism/tree/develop/packages/contracts/deployments).
  *
  * _Available since v4.6._
  */
 abstract contract CrossChainEnabledOptimism is CrossChainEnabled {
-    address private immutable _bridge;
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
+    address private immutable _messenger;
 
-    constructor(address bridge) {
-        _bridge = bridge;
+    /// @custom:oz-upgrades-unsafe-allow constructor
+    constructor(address messenger) {
+        _messenger = messenger;
     }
 
     /**
      * @dev see {CrossChainEnabled-_isCrossChain}
      */
     function _isCrossChain() internal view virtual override returns (bool) {
-        return LibOptimism.isCrossChain(_bridge);
+        return LibOptimism.isCrossChain(_messenger);
     }
 
     /**
      * @dev see {CrossChainEnabled-_crossChainSender}
      */
     function _crossChainSender() internal view virtual override onlyCrossChain returns (address) {
-        return LibOptimism.crossChainSender(_bridge);
+        return LibOptimism.crossChainSender(_messenger);
     }
 }

contracts/crosschain/optimism/LibOptimism.sol

@@ -5,26 +5,27 @@ pragma solidity ^0.8.0;
 import {ICrossDomainMessenger as Optimism_Bridge} from "../../vendor/optimism/ICrossDomainMessenger.sol";
 
 /**
- * @dev Primitives for cross-chain aware contracts for
- * [Optimism](https://www.optimism.io/).
+ * @dev Primitives for cross-chain aware contracts for [Optimism](https://www.optimism.io/).
+ * See the [documentation](https://community.optimism.io/docs/developers/bridge/messaging/#accessing-msg-sender)
+ * for the functionality used here.
  */
 library LibOptimism {
     /**
      * @dev Returns whether the current function call is the result of a
-     * cross-chain message relayed by `bridge`.
+     * cross-chain message relayed by `messenger`.
      */
-    function isCrossChain(address bridge) internal view returns (bool) {
-        return msg.sender == bridge;
+    function isCrossChain(address messenger) internal view returns (bool) {
+        return msg.sender == messenger;
     }
 
     /**
      * @dev Returns the address of the sender that triggered the current
-     * cross-chain message through `bridge`.
+     * cross-chain message through `messenger`.
      *
      * NOTE: {isCrossChain} should be checked before trying to recover the
      * sender.
      */
-    function crossChainSender(address bridge) internal view returns (address) {
-        return Optimism_Bridge(bridge).xDomainMessageSender();
+    function crossChainSender(address messenger) internal view returns (address) {
+        return Optimism_Bridge(messenger).xDomainMessageSender();
     }
 }

contracts/crosschain/polygon/CrossChainEnabledPolygonChild.sol

@@ -23,9 +23,11 @@ address constant DEFAULT_SENDER = 0x000000000000000000000000000000000000dEaD;
  * _Available since v4.6._
  */
 abstract contract CrossChainEnabledPolygonChild is IFxMessageProcessor, CrossChainEnabled, ReentrancyGuard {
+    /// @custom:oz-upgrades-unsafe-allow state-variable-immutable
     address private immutable _fxChild;
     address private _sender = DEFAULT_SENDER;
 
+    /// @custom:oz-upgrades-unsafe-allow constructor
     constructor(address fxChild) {
         _fxChild = fxChild;
     }

contracts/mocks/AccessControlCrossChainMock.sol

@@ -16,7 +16,7 @@ contract AccessControlCrossChainMock is AccessControlCrossChain, CrossChainEnabl
 
     function senderProtected(bytes32 roleId) public onlyRole(roleId) {}
 
-    function aliasRole(bytes32 role) public pure virtual returns (bytes32) {
-        return _aliasRole(role);
+    function crossChainRoleAlias(bytes32 role) public pure virtual returns (bytes32) {
+        return _crossChainRoleAlias(role);
     }
 }

test/access/AccessControlCrossChain.test.js

@@ -5,7 +5,7 @@ const {
   shouldBehaveLikeAccessControl,
 } = require('./AccessControl.behavior.js');
 
-const aliasRole = (role) => web3.utils.leftPad(
+const crossChainRoleAlias = (role) => web3.utils.leftPad(
   web3.utils.toHex(web3.utils.toBN(role).xor(web3.utils.toBN(web3.utils.soliditySha3('CROSSCHAIN_ALIAS')))),
   64,
 );
@@ -26,11 +26,11 @@ contract('AccessControl', function (accounts) {
   describe('CrossChain enabled', function () {
     beforeEach(async function () {
       await this.accessControl.grantRole(ROLE, accounts[0], { from: accounts[0] });
-      await this.accessControl.grantRole(aliasRole(ROLE), accounts[1], { from: accounts[0] });
+      await this.accessControl.grantRole(crossChainRoleAlias(ROLE), accounts[1], { from: accounts[0] });
     });
 
     it('check alliassing', async function () {
-      expect(await this.accessControl.aliasRole(ROLE)).to.be.bignumber.equal(aliasRole(ROLE));
+      expect(await this.accessControl.crossChainRoleAlias(ROLE)).to.be.bignumber.equal(crossChainRoleAlias(ROLE));
     });
 
     it('Crosschain calls not authorized to non-aliased addresses', async function () {
@@ -41,7 +41,7 @@ contract('AccessControl', function (accounts) {
           'senderProtected',
           [ ROLE ],
         ),
-        `AccessControl: account ${accounts[0].toLowerCase()} is missing role ${aliasRole(ROLE)}`,
+        `AccessControl: account ${accounts[0].toLowerCase()} is missing role ${crossChainRoleAlias(ROLE)}`,
       );
     });
 

test/crosschain/CrossChainEnabled.test.js

@@ -1,6 +1,18 @@
-const { expectRevert } = require('@openzeppelin/test-helpers');
+const { expect } = require('chai');
 const CrossChainHelper = require('../helpers/crosschain');
 
+/** Revert handler that supports custom errors. */
+async function expectRevert (promise, reason) {
+  try {
+    await promise;
+    expect.fail('Expected promise to throw but it didn\'t');
+  } catch (error) {
+    if (reason) {
+      expect(error.message).to.include(reason);
+    }
+  }
+}
+
 function randomAddress () {
   return web3.utils.toChecksumAddress(web3.utils.randomHex(20));
 }
@@ -13,20 +25,21 @@ const CrossChainEnabledPolygonChildMock = artifacts.require('CrossChainEnabledPo
 
 function shouldBehaveLikeReceiver (sender = randomAddress()) {
   it('should reject same-chain calls', async function () {
-    await expectRevert.unspecified(
+    await expectRevert(
       this.receiver.crossChainRestricted(),
-    ); // TODO: check custom error
+      'NotCrossChainCall()',
+    );
   });
 
   it('should restrict to cross-chain call from a invalid sender', async function () {
-    await expectRevert.unspecified(CrossChainHelper.call(
+    await expectRevert(CrossChainHelper.call(
       sender,
       this.receiver,
       'crossChainOwnerRestricted()',
-    )); // TODO: check custom error
+    ));
   });
 
-  it('should grant access to cross-chain call from a the owner', async function () {
+  it('should grant access to cross-chain call from the owner', async function () {
     await CrossChainHelper.call(
       await this.receiver.owner(),
       this.receiver,