Support multiple inboxes per domain (#349)

* Support multiple inboxes per domain

* Rename in comment

* PR review

* PR review

rust/tools/balance-exporter/src/main.rs

@@ -139,7 +139,7 @@ impl Sample {
 #[tokio::test]
 #[should_panic]
 async fn mainnet_works() {
-    // query ethereum instance of XAppConnectionManager and asserts the balance is nonzero.
+    // query ethereum instance of AbacusConnectionManager and asserts the balance is nonzero.
     let sample = poll_once(
         &Input {
             contracts: vec![ChainSetup {

solidity/core/contracts/AbacusConnectionManager.sol

@@ -1,12 +1,16 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 pragma solidity >=0.6.11;
+pragma abicoder v2;
 
 // ============ Internal Imports ============
 import {IOutbox} from "../interfaces/IOutbox.sol";
 import {IInterchainGasPaymaster} from "../interfaces/IInterchainGasPaymaster.sol";
 import {IAbacusConnectionManager} from "../interfaces/IAbacusConnectionManager.sol";
+
 // ============ External Imports ============
 import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
+import {SafeMath} from "@openzeppelin/contracts/math/SafeMath.sol";
+import {EnumerableSet} from "@openzeppelin/contracts/utils/EnumerableSet.sol";
 
 /**
  * @title AbacusConnectionManager
@@ -15,6 +19,9 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
  * domains.
  */
 contract AbacusConnectionManager is IAbacusConnectionManager, Ownable {
+    using SafeMath for uint256;
+    using EnumerableSet for EnumerableSet.AddressSet;
+
     // ============ Public Storage ============
 
     // Outbox contract
@@ -26,8 +33,8 @@ contract AbacusConnectionManager is IAbacusConnectionManager, Ownable {
     IInterchainGasPaymaster public override interchainGasPaymaster;
     // local Inbox address => remote Outbox domain
     mapping(address => uint32) public inboxToDomain;
-    // remote Outbox domain => local Inbox address
-    mapping(uint32 => address) public domainToInbox;
+    // remote Outbox domain => local Inbox addresses
+    mapping(uint32 => EnumerableSet.AddressSet) domainToInboxes;
 
     // ============ Events ============
 
@@ -85,11 +92,10 @@ contract AbacusConnectionManager is IAbacusConnectionManager, Ownable {
      * @param _inbox the address of the Inbox
      */
     function enrollInbox(uint32 _domain, address _inbox) external onlyOwner {
-        // un-enroll any existing inbox
-        _unenrollInbox(_inbox);
+        require(!isInbox(_inbox), "already inbox");
         // add inbox and domain to two-way mapping
         inboxToDomain[_inbox] = _domain;
-        domainToInbox[_domain] = _inbox;
+        domainToInboxes[_domain].add(_inbox);
         emit InboxEnrolled(_domain, _inbox);
     }
 
@@ -109,6 +115,26 @@ contract AbacusConnectionManager is IAbacusConnectionManager, Ownable {
         return outbox.localDomain();
     }
 
+    /**
+     * @notice Returns the Inbox addresses for a given remote domain
+     * @return inboxes An array of addresses of the Inboxes
+     */
+    function getInboxes(uint32 remoteDomain)
+        external
+        view
+        returns (address[] memory)
+    {
+        EnumerableSet.AddressSet storage _inboxes = domainToInboxes[
+            remoteDomain
+        ];
+        uint256 length = _inboxes.length();
+        address[] memory ret = new address[](length);
+        for (uint256 i = 0; i < length; i = i.add(1)) {
+            ret[i] = _inboxes.at(i);
+        }
+        return ret;
+    }
+
     // ============ Public Functions ============
 
     /**
@@ -155,7 +181,7 @@ contract AbacusConnectionManager is IAbacusConnectionManager, Ownable {
      */
     function _unenrollInbox(address _inbox) internal {
         uint32 _currentDomain = inboxToDomain[_inbox];
-        domainToInbox[_currentDomain] = address(0);
+        domainToInboxes[_currentDomain].remove(_inbox);
         inboxToDomain[_inbox] = 0;
         emit InboxUnenrolled(_currentDomain, _inbox);
     }

solidity/core/test/abacusConnectionManager.test.ts

@@ -190,9 +190,9 @@ describe('AbacusConnectionManager', async () => {
       connectionManager.enrollInbox(newRemoteDomain, newInbox.address),
     ).to.emit(connectionManager, 'InboxEnrolled');
 
-    expect(await connectionManager.domainToInbox(newRemoteDomain)).to.equal(
+    expect(await connectionManager.getInboxes(newRemoteDomain)).to.eql([
       newInbox.address,
-    );
+    ]);
     expect(await connectionManager.inboxToDomain(newInbox.address)).to.equal(
       newRemoteDomain,
     );
@@ -200,6 +200,9 @@ describe('AbacusConnectionManager', async () => {
   });
 
   it('Owner can unenroll a inbox', async () => {
+    expect(await connectionManager.getInboxes(remoteDomain)).to.eql([
+      enrolledInbox.address,
+    ]);
     await expect(
       connectionManager.unenrollInbox(enrolledInbox.address),
     ).to.emit(connectionManager, 'InboxUnenrolled');
@@ -207,9 +210,47 @@ describe('AbacusConnectionManager', async () => {
     expect(
       await connectionManager.inboxToDomain(enrolledInbox.address),
     ).to.equal(0);
-    expect(await connectionManager.domainToInbox(localDomain)).to.equal(
-      ethers.constants.AddressZero,
-    );
+    expect(await connectionManager.getInboxes(remoteDomain)).to.eql([]);
     expect(await connectionManager.isInbox(enrolledInbox.address)).to.be.false;
   });
+
+  it('Owner can enroll multiple inboxes per domain', async () => {
+    const newRemoteDomain = 3000;
+    const inboxFactory = new TestInbox__factory(signer);
+    const newInbox1 = await inboxFactory.deploy(localDomain);
+    const newInbox2 = await inboxFactory.deploy(localDomain);
+
+    // Assert new inbox not considered inbox before enrolled
+    expect(await connectionManager.isInbox(newInbox1.address)).to.be.false;
+    expect(await connectionManager.isInbox(newInbox2.address)).to.be.false;
+
+    await expect(
+      connectionManager.enrollInbox(newRemoteDomain, newInbox1.address),
+    ).to.emit(connectionManager, 'InboxEnrolled');
+    await expect(
+      connectionManager.enrollInbox(newRemoteDomain, newInbox2.address),
+    ).to.emit(connectionManager, 'InboxEnrolled');
+
+    expect(await connectionManager.inboxToDomain(newInbox1.address)).to.equal(
+      newRemoteDomain,
+    );
+    expect(await connectionManager.inboxToDomain(newInbox2.address)).to.equal(
+      newRemoteDomain,
+    );
+
+    expect(await connectionManager.isInbox(newInbox1.address)).to.be.true;
+    expect(await connectionManager.isInbox(newInbox2.address)).to.be.true;
+
+    expect(await connectionManager.getInboxes(newRemoteDomain)).to.eql([
+      newInbox1.address,
+      newInbox2.address,
+    ]);
+  });
+
+  it('Owner cannot enroll an inbox twice', async () => {
+    const newRemoteDomain = 3000;
+    await expect(
+      connectionManager.enrollInbox(newRemoteDomain, enrolledInbox.address),
+    ).to.be.revertedWith('already inbox');
+  });
 });

typescript/infra/src/core/check.ts

@@ -221,11 +221,11 @@ export class AbacusCoreChecker extends AbacusAppChecker<
     const contracts = this.app.mustGetContracts(domain);
     for (const remote of this.app.remoteDomainNumbers(domain)) {
       // inbox is enrolled in abacusConnectionManager
-      const enrolledInbox =
-        await contracts.abacusConnectionManager.domainToInbox(remote);
-      expect(enrolledInbox).to.equal(
+      const enrolledInboxes =
+        await contracts.abacusConnectionManager.getInboxes(remote);
+      expect(enrolledInboxes).to.eql([
         this.app.mustGetInbox(remote, domain).address,
-      );
+      ]);
     }
     // Outbox is set on abacusConnectionManager
     const outbox = await contracts.abacusConnectionManager.outbox();