fix(protocol): Remove enableDestChain functionality (#12341)

* rm enableDestChainid, fix tests, update ABIs * fix taikol2 abi revert * fix taiko l1 abi revert * revert bridge abi * enable dest chain in genesis tests
taikoxyz · Jan 10, 2023 · 362d083 · 362d083
1 parent bad2144
commit 362d083
Show file tree

Hide file tree

Showing 13 changed files with 335 additions and 252 deletions.
diff --git a/packages/bridge-ui/src/constants/abi/Bridge.ts b/packages/bridge-ui/src/constants/abi/Bridge.ts
@@ -128,7 +128,7 @@ export default [
       },
       {
         indexed: false,
-        internalType: "enum LibBridgeData.MessageStatus",
+        internalType: "enum LibBridgeStatus.MessageStatus",
         name: "status",
         type: "uint8",
       },
@@ -217,24 +217,6 @@ export default [
     stateMutability: "view",
     type: "function",
   },
-  {
-    inputs: [
-      {
-        internalType: "uint256",
-        name: "_chainId",
-        type: "uint256",
-      },
-      {
-        internalType: "bool",
-        name: "enabled",
-        type: "bool",
-      },
-    ],
-    name: "enableDestChain",
-    outputs: [],
-    stateMutability: "nonpayable",
-    type: "function",
-  },
   {
     inputs: [
       {
@@ -246,7 +228,7 @@ export default [
     name: "getMessageStatus",
     outputs: [
       {
-        internalType: "enum LibBridgeData.MessageStatus",
+        internalType: "enum LibBridgeStatus.MessageStatus",
         name: "",
         type: "uint8",
       },
@@ -616,7 +598,7 @@ export default [
       },
       {
         internalType: "bool",
-        name: "lastAttempt",
+        name: "isLastAttempt",
         type: "bool",
       },
     ],

diff --git a/packages/protocol/contracts/bridge/Bridge.sol b/packages/protocol/contracts/bridge/Bridge.sol
@@ -99,17 +99,6 @@ contract Bridge is EssentialContract, IBridge {
             });
     }
 
-    function enableDestChain(
-        uint256 _chainId,
-        bool enabled
-    ) external nonReentrant {
-        LibBridgeSend.enableDestChain({
-            state: state,
-            chainId: _chainId,
-            enabled: enabled
-        });
-    }
-
     /*********************
      * Public Functions  *
      *********************/
@@ -169,6 +158,7 @@ contract Bridge is EssentialContract, IBridge {
     }
 
     function isDestChainEnabled(uint256 _chainId) public view returns (bool) {
-        return state.destChains[_chainId];
+        return
+            LibBridgeSend.isDestChainEnabled(AddressResolver(this), _chainId);
     }
 }
diff --git a/packages/protocol/contracts/bridge/libs/LibBridgeData.sol b/packages/protocol/contracts/bridge/libs/LibBridgeData.sol
@@ -20,11 +20,9 @@ import "../IBridge.sol";
  */
 library LibBridgeData {
     struct State {
-        // chainId => isEnabled
-        mapping(uint256 => bool) destChains;
         uint256 nextMessageId;
         IBridge.Context ctx; // 3 slots
-        uint256[45] __gap;
+        uint256[46] __gap;
     }
 
     bytes32 internal constant SIGNAL_PLACEHOLDER = bytes32(uint256(1));

diff --git a/packages/protocol/contracts/bridge/libs/LibBridgeSend.sol b/packages/protocol/contracts/bridge/libs/LibBridgeSend.sol
@@ -26,7 +26,7 @@ library LibBridgeSend {
      *
      * @param message Specifies the `depositValue`, `callValue`,
      * and `processingFee`. These must sum to `msg.value`. It also specifies the
-     * `destChainId` which must be first enabled via `enableDestChain`,
+     * `destChainId` which must have a `bridge` address set on the AddressResolver
      * and differ from the current chain ID.
      *
      * @return signal The message is hashed, stored, and emitted as a signal.
@@ -42,7 +42,7 @@ library LibBridgeSend {
         require(message.owner != address(0), "B:owner");
         require(
             message.destChainId != block.chainid &&
-                state.destChains[message.destChainId],
+                isDestChainEnabled(resolver, message.destChainId),
             "B:destChainId"
         );
 
@@ -69,16 +69,10 @@ library LibBridgeSend {
         emit LibBridgeData.MessageSent(signal, message);
     }
 
-    /**
-     * Enable a destination chain ID for bridge transactions.
-     */
-    function enableDestChain(
-        LibBridgeData.State storage state,
-        uint256 chainId,
-        bool enabled
-    ) internal {
-        require(chainId > 0 && chainId != block.chainid, "B:chainId");
-        state.destChains[chainId] = enabled;
-        emit LibBridgeData.DestChainEnabled(chainId, enabled);
+    function isDestChainEnabled(
+        AddressResolver resolver,
+        uint256 chainId
+    ) internal view returns (bool) {
+        return resolver.resolve(chainId, "bridge") != address(0);
     }
 }
diff --git a/packages/protocol/contracts/test/bridge/libs/TestLibBridgeSend.sol b/packages/protocol/contracts/test/bridge/libs/TestLibBridgeSend.sol
@@ -23,12 +23,4 @@ contract TestLibBridgeSend is EssentialContract {
     ) public payable returns (bytes32 signal) {
         return LibBridgeSend.sendMessage(state, AddressResolver(this), message);
     }
-
-    function enableDestChain(uint256 chainId, bool enabled) public {
-        LibBridgeSend.enableDestChain(state, chainId, enabled);
-    }
-
-    function getDestChainStatus(uint256 chainId) public view returns (bool) {
-        return state.destChains[chainId];
-    }
 }
diff --git a/packages/protocol/docs/bridge/Bridge.md b/packages/protocol/docs/bridge/Bridge.md
@@ -75,12 +75,6 @@ function processMessage(struct IBridge.Message message, bytes proof) external
 function retryMessage(struct IBridge.Message message, bool isLastAttempt) external
 ```
 
-### enableDestChain
-
-```solidity
-function enableDestChain(uint256 _chainId, bool enabled) external
-```
-
 ### isMessageSent
 
 ```solidity

diff --git a/packages/protocol/test/bridge/Bridge.test.ts b/packages/protocol/test/bridge/Bridge.test.ts
@@ -57,8 +57,6 @@ async function deployBridge(
 
     await bridge.connect(signer).init(addressManager.address);
 
-    await bridge.connect(signer).enableDestChain(destChain, true);
-
     const etherVault: EtherVault = await (
         await ethers.getContractFactory("EtherVault")
     )
@@ -107,6 +105,11 @@ describe("Bridge", function () {
                 srcChainId
             );
 
+        await addressManager.setAddress(
+            `${enabledDestChainId}.bridge`,
+            "0x0000000000000000000000000000000000000001" // dummy address so chain is "enabled"
+        );
+
         // deploy protocol contract
         return {
             owner,

diff --git a/packages/protocol/test/bridge/libs/LibBridgeSend.test.ts b/packages/protocol/test/bridge/libs/LibBridgeSend.test.ts
@@ -27,6 +27,11 @@ describe("LibBridgeSend", function () {
         ).deploy();
         await addressManager.init();
 
+        await addressManager.setAddress(
+            `${enabledDestChainId}.bridge`,
+            "0x0000000000000000000000000000000000000001" // dummy address so chain is "enabled"
+        );
+
         const etherVault: EtherVault = await (
             await ethers.getContractFactory("EtherVault")
         )
@@ -51,26 +56,6 @@ describe("LibBridgeSend", function () {
             .authorize(libSend.address, true);
     });
 
-    describe("enableDestChain()", async function () {
-        it("should throw when chainId <= 0", async function () {
-            await expect(libSend.enableDestChain(0, true)).to.be.revertedWith(
-                "B:chainId"
-            );
-        });
-
-        it("should throw when chainId == block.chainId", async function () {
-            await expect(
-                libSend.enableDestChain(blockChainId, true)
-            ).to.be.revertedWith("B:chainId");
-        });
-
-        it("should emit DestChainEnabled() event", async function () {
-            expect(
-                await libSend.enableDestChain(enabledDestChainId, true)
-            ).to.emit(libSend, "DestChainEnabled");
-        });
-    });
-
     describe("sendMessage()", async function () {
         it("should throw when message.owner == address(0)", async function () {
             const nonEnabledDestChain = 2;
@@ -143,8 +128,6 @@ describe("LibBridgeSend", function () {
         });
 
         it("should throw when expectedAmount != msg.value", async function () {
-            await libSend.enableDestChain(enabledDestChainId, true);
-
             const message: Message = {
                 id: 1,
                 sender: owner.address,
@@ -167,8 +150,6 @@ describe("LibBridgeSend", function () {
         });
 
         it("should emit MessageSent() event and signal should be hashed correctly", async function () {
-            await libSend.enableDestChain(enabledDestChainId, true);
-
             const message: Message = {
                 id: 1,
                 sender: owner.address,

diff --git a/packages/protocol/test/genesis/generate_genesis.test.ts b/packages/protocol/test/genesis/generate_genesis.test.ts
@@ -97,7 +97,7 @@ action("Generate Genesis", function () {
             const addressManager = new hre.ethers.Contract(
                 addressManagerAlloc.address,
                 require("../../artifacts/contracts/thirdparty/AddressManager.sol/AddressManager.json").abi,
-                provider
+                signer
             );
 
             const owner = await addressManager.owner();
@@ -110,11 +110,11 @@ action("Generate Genesis", function () {
 
             expect(bridge).to.be.equal(getContractAlloc("Bridge").address);
 
-            const tokenValut = await addressManager.getAddress(
+            const tokenVault = await addressManager.getAddress(
                 `${testConfig.chainId}.token_vault`
             );
 
-            expect(tokenValut).to.be.equal(
+            expect(tokenVault).to.be.equal(
                 getContractAlloc("TokenVault").address
             );
 
@@ -237,8 +237,6 @@ action("Generate Genesis", function () {
 
             expect(owner).to.be.equal(testConfig.contractOwner);
 
-            await expect(Bridge.enableDestChain(1, true)).not.to.reverted;
-
             await expect(
                 Bridge.processMessage(
                     {
@@ -273,6 +271,19 @@ action("Generate Genesis", function () {
 
             expect(owner).to.be.equal(testConfig.contractOwner);
 
+            const addressManager = new hre.ethers.Contract(
+                getContractAlloc("AddressManager").address,
+                require("../../artifacts/contracts/thirdparty/AddressManager.sol/AddressManager.json").abi,
+                signer
+            );
+
+            await expect(
+                addressManager.setAddress(
+                    "1.bridge",
+                    getContractAlloc("Bridge").address
+                )
+            ).not.to.be.reverted;
+
             await expect(
                 TokenVault.sendEther(
                     1,

diff --git a/packages/protocol/test/libs/LibTrieProof.test.ts b/packages/protocol/test/libs/LibTrieProof.test.ts
@@ -26,11 +26,18 @@ describe("integration:LibTrieProof", function () {
 
         const { chainId } = await ethers.provider.getNetwork();
 
+        const enabledDestChainId = chainId + 1;
+
         await addressManager.setAddress(
             `${chainId}.ether_vault`,
             "0xEA3dD11036f668F08940E13e3bcB097C93b09E07"
         );
 
+        await addressManager.setAddress(
+            `${enabledDestChainId}.bridge`,
+            "0x0000000000000000000000000000000000000001" // dummy address so chain is "enabled"
+        );
+
         const libBridgeRetry = await (
             await ethers.getContractFactory("LibBridgeRetry")
         ).deploy();
@@ -57,23 +64,21 @@ describe("integration:LibTrieProof", function () {
 
         const [owner] = await ethers.getSigners();
 
-        return { owner, testLibTreProof, bridge };
+        return { owner, testLibTreProof, bridge, enabledDestChainId };
     }
     describe("verify()", function () {
         it("verifies", async function () {
-            const { owner, testLibTreProof, bridge } =
+            const { owner, testLibTreProof, bridge, enabledDestChainId } =
                 await deployLibTrieProofFixture();
 
             const { chainId } = await ethers.provider.getNetwork();
             const srcChainId = chainId;
-            const destChainId = srcChainId + 1;
-            await (await bridge.enableDestChain(destChainId, true)).wait();
 
             const message: Message = {
                 id: 1,
                 sender: owner.address,
                 srcChainId: srcChainId,
-                destChainId: destChainId,
+                destChainId: enabledDestChainId,
                 owner: owner.address,
                 to: owner.address,
                 refundAddress: owner.address,