Merge pull request #1112 from JoinColony/maint/never-float-hexlify

Fix MTX broadcaster intermittent bug; pay for bridge transactions
JoinColony · Jan 27, 2023 · 4595b4d · 4595b4d
2 parents 193242a + feb6764
commit 4595b4d
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 11 deletions.
diff --git a/packages/metatransaction-broadcaster/Dockerfile b/packages/metatransaction-broadcaster/Dockerfile
@@ -3,8 +3,8 @@ COPY ./packages ./packages
 COPY ./package.json ./
 COPY ./package-lock.json ./
 COPY ./build ./build
-RUN npm ci
+RUN yarn
 RUN cd ./packages/metatransaction-broadcaster/ && npm i
 RUN cd ./packages/package-utils/ && npm i
 EXPOSE 3000
-CMD node $NODE_ARGS packages/metatransaction-broadcaster/bin/index.js --colonyNetworkAddress $COLONYNETWORK_ADDRESS --privateKey $PRIVATE_KEY --gasLimit $GASLIMIT $ARGS
+CMD node $NODE_ARGS packages/metatransaction-broadcaster/bin/index.js --colonyNetworkAddress $COLONYNETWORK_ADDRESS --privateKey $PRIVATE_KEY $ARGS
diff --git a/packages/metatransaction-broadcaster/MetatransactionBroadcaster.js b/packages/metatransaction-broadcaster/MetatransactionBroadcaster.js
@@ -6,6 +6,10 @@ const queue = require("express-queue");
 const NonceManager = require("./ExtendedNonceManager");
 const { colonyIOCors, ConsoleAdapter, updateGasEstimate } = require("../package-utils");
 
+const ETHEREUM_BRIDGE_ADDRESS = "0x75Df5AF045d91108662D8080fD1FEFAd6aA0bb59";
+const BINANCE_BRIDGE_ADDRESS = "0x162E898bD0aacB578C8D5F8d6ca588c13d2A383F";
+const REQUIRE_TO_PASS_MESSAGE_SIG = ethers.utils.keccak256(ethers.utils.toUtf8Bytes("requireToPassMessage(address,bytes,uint256)")).slice(0, 10);
+
 class MetatransactionBroadcaster {
   /**
    * Constructor for MetatransactionBroadcaster
@@ -194,14 +198,35 @@ class MetatransactionBroadcaster {
     const possibleColony = new ethers.Contract(target, colonyDef.abi, this.wallet);
     try {
       const tx = possibleColony.interface.parseTransaction({ data: txData });
-      if (tx.signature === "makeArbitraryTransaction(address,bytes)") {
-        return false;
-      }
-      if (tx.signature === "makeArbitraryTransactions(address[],bytes[],bool)") {
-        return false;
-      }
-      if (tx.signature === "makeSingleArbitraryTransaction(address,bytes)") {
-        return false;
+      // If it's an arbitrary transaction...
+      if (
+        tx.signature === "makeArbitraryTransaction(address,bytes)" ||
+        tx.signature === "makeSingleArbitraryTransaction(address,bytes)" ||
+        tx.signature === "makeArbitraryTransactions(address[],bytes[],bool)"
+      ) {
+        // We allow it if these transactions are going only to known bridges.
+        let addresses = tx.args[0];
+        let calls = tx.args[1];
+
+        if (!Array.isArray(addresses)) {
+          addresses = [addresses];
+        }
+        if (!Array.isArray(calls)) {
+          calls = [calls];
+        }
+        if (addresses.length !== calls.length) {
+          // If the arrays don't line up, return false.
+          return false;
+        }
+
+        for (let i = 0; i < addresses.length; i += 1) {
+          if (
+            !MetatransactionBroadcaster.isBridgeAddress(addresses[i]) || // It's not going to a bridge address
+            calls[i].slice(0, 10) !== REQUIRE_TO_PASS_MESSAGE_SIG // Or it is, but not calling the function we allow
+          ) {
+            return false;
+          }
+        }
       }
     } catch (err) {
       // Not a colony related transaction (we recognise)
@@ -256,6 +281,19 @@ class MetatransactionBroadcaster {
     return true;
   }
 
+  static isBridgeAddress(targetOrTargets) {
+    if (targetOrTargets === ETHEREUM_BRIDGE_ADDRESS) {
+      return true;
+    }
+    if (targetOrTargets === BINANCE_BRIDGE_ADDRESS) {
+      return true;
+    }
+    if (Array.isArray(targetOrTargets)) {
+      return targetOrTargets.map((x) => MetatransactionBroadcaster.isBridgeAddress(x)).every((x) => x === true);
+    }
+    return false;
+  }
+
   async isValidSetAuthorityTransaction(tx, userAddress) {
     // Get the most recent metatx this user sent on colonyNetwork
     let logs = await this.provider.getLogs({

diff --git a/packages/package-utils/updateGasEstimate.js b/packages/package-utils/updateGasEstimate.js
@@ -40,7 +40,7 @@ const updateGasEstimate = async function (_type, chainId, adapter) {
     const gasEstimates = await axios.request(options);
     let gasPrice;
     if (gasEstimates[type]) {
-      gasPrice = ethers.utils.hexlify((gasEstimates[type] / factor) * 1e9);
+      gasPrice = ethers.utils.hexlify(ethers.BigNumber.from(gasEstimates[type] * 1e9).div(factor));
     } else {
       gasPrice = defaultGasPrice;
     }

diff --git a/test/packages/metaTransactionBroadcaster.js b/test/packages/metaTransactionBroadcaster.js
@@ -118,6 +118,34 @@ contract("Metatransaction broadcaster", (accounts) => {
       expect(valid).to.be.equal(false);
     });
 
+    it(`transactions to the forbidden arbitrary transaction methods are allowed only if
+      going to a bridge calling the right function`, async function () {
+      const ETHEREUM_BRIDGE_ADDRESS = "0x75Df5AF045d91108662D8080fD1FEFAd6aA0bb59";
+      const BINANCE_BRIDGE_ADDRESS = "0x162E898bD0aacB578C8D5F8d6ca588c13d2A383F";
+
+      const AMBABI = ["function requireToPassMessage(address,bytes,uint256)"];
+      const AMBInterface = new ethers.utils.Interface(AMBABI);
+
+      const ambCall = AMBInterface.encodeFunctionData("requireToPassMessage", ["0x75Df5AF045d91108662D8080fD1FEFAd6aA0bb59", "0x00000000", 1000000]);
+
+      let txData = await colony.contract.methods.makeArbitraryTransaction(ETHEREUM_BRIDGE_ADDRESS, ambCall).encodeABI();
+      let valid = await broadcaster.isColonyFamilyTransactionAllowed(colony.address, txData);
+      expect(valid).to.be.equal(true);
+
+      txData = await colony.contract.methods.makeArbitraryTransactions([BINANCE_BRIDGE_ADDRESS], [ambCall], false).encodeABI();
+      valid = await broadcaster.isColonyFamilyTransactionAllowed(colony.address, txData);
+      expect(valid).to.be.equal(true);
+
+      txData = await colony.contract.methods.makeSingleArbitraryTransaction(BINANCE_BRIDGE_ADDRESS, ambCall).encodeABI();
+      valid = await broadcaster.isColonyFamilyTransactionAllowed(colony.address, txData);
+      expect(valid).to.be.equal(true);
+
+      // Going to a bridge, but not the right function call
+      txData = await colony.contract.methods.makeSingleArbitraryTransaction(BINANCE_BRIDGE_ADDRESS, "0x00000000").encodeABI();
+      valid = await broadcaster.isColonyFamilyTransactionAllowed(colony.address, txData);
+      expect(valid).to.be.equal(false);
+    });
+
     it("transactions to a token are not accepted based on address", async function () {
       const tokenAddress = await colony.getToken();