Add _strict flag to makeArbitraryTransaction

kronosapiens · kronosapiens · commit 373c610be4a3 · 2024-10-31T13:40:07.000-07:00
diff --git a/contracts/colony/ColonyArbitraryTransaction.sol b/contracts/colony/ColonyArbitraryTransaction.sol
@@ -36,7 +36,8 @@ contract ColonyArbitraryTransaction is ColonyStorage {
 
   function makeArbitraryTransaction(
     address _to,
-    bytes memory _action
+    bytes memory _action,
+    bool _strict
   ) public stoppable auth returns (bool) {
     // Prevent transactions to network contracts
     require(_to != address(this), "colony-cannot-target-self");
@@ -78,6 +79,8 @@ contract ColonyArbitraryTransaction is ColonyStorage {
 
     emit ArbitraryTransaction(_to, _action, res);
 
+    require(res || !_strict, "colony-arbitrary-transaction-failed");
+
     return res;
   }
 
diff --git a/contracts/colony/ColonyAuthority.sol b/contracts/colony/ColonyAuthority.sol
@@ -98,7 +98,7 @@ contract ColonyAuthority is CommonAuthority {
     addRoleCapability(ROOT_ROLE, "upgradeExtension(bytes32,uint256)");
     addRoleCapability(ROOT_ROLE, "deprecateExtension(bytes32,bool)");
     addRoleCapability(ROOT_ROLE, "uninstallExtension(bytes32)");
-    addRoleCapability(ROOT_ROLE, "makeArbitraryTransaction(address,bytes)");
+    addRoleCapability(ROOT_ROLE, "makeArbitraryTransaction(address,bytes)"); // Deprecated
     addRoleCapability(ROOT_ROLE, "emitDomainReputationReward(uint256,address,int256)");
     addRoleCapability(ROOT_ROLE, "emitSkillReputationReward(uint256,address,int256)");
     addRoleCapability(ARBITRATION_ROLE, "transferStake(uint256,uint256,address,address,uint256,uint256,address)");
@@ -134,6 +134,9 @@ contract ColonyAuthority is CommonAuthority {
     addRoleCapability(ARBITRATION_ROLE, "finalizeExpenditureViaArbitration(uint256,uint256,uint256)");
     addRoleCapability(ROOT_ROLE, "setColonyBridgeAddress(address)");
     addRoleCapability(ROOT_ROLE, "initialiseReputationMining(uint256,bytes32,uint256)");
+
+    // Added in colony v17 (jade-lwss)
+    addRoleCapability(ROOT_ROLE, "makeArbitraryTransaction(address,bytes,bool)");
   }
 
   function addRoleCapability(uint8 role, bytes memory sig) private {
diff --git a/contracts/colony/IColony.sol b/contracts/colony/IColony.sol
@@ -54,10 +54,12 @@ interface IColony is IDSAuth, ColonyDataTypes, IRecovery, IBasicMetaTransaction,
   /// @notice Executes an arbitrary transaction
   /// @param _target Contract to receive the function call
   /// @param _action Bytes array encoding the function call and arguments
+  /// @param _strict Boolean indicating whether the transaction must succeed
   /// @return success Boolean indicating whether the transactions succeeded
   function makeArbitraryTransaction(
     address _target,
-    bytes memory _action
+    bytes memory _action,
+    bool _strict
   ) external returns (bool success);
 
   /// @notice Emit a metadata string for a transaction
diff --git a/docs/interfaces/icolony.md b/docs/interfaces/icolony.md
@@ -1049,7 +1049,7 @@ Lock the colony's token. Can only be called by a network-managed extension.
 |---|---|---|
 |timesLocked|uint256|The amount of times the token was locked
 
-### ▸ `makeArbitraryTransaction(address _target, bytes memory _action):bool success`
+### ▸ `makeArbitraryTransaction(address _target, bytes memory _action, bool _strict):bool success`
 
 Executes an arbitrary transaction
 
@@ -1060,6 +1060,7 @@ Executes an arbitrary transaction
 |---|---|---|
 |_target|address|Contract to receive the function call
 |_action|bytes|Bytes array encoding the function call and arguments
+|_strict|bool|Boolean indicating whether the transaction must succeed
 
 **Return Parameters**
 
diff --git a/docs/interfaces/imetacolony.md b/docs/interfaces/imetacolony.md
@@ -1087,7 +1087,7 @@ Lock the colony's token. Can only be called by a network-managed extension.
 |---|---|---|
 |timesLocked|uint256|The amount of times the token was locked
 
-### ▸ `makeArbitraryTransaction(address _target, bytes memory _action):bool success`
+### ▸ `makeArbitraryTransaction(address _target, bytes memory _action, bool _strict):bool success`
 
 Executes an arbitrary transaction
 
@@ -1098,6 +1098,7 @@ Executes an arbitrary transaction
 |---|---|---|
 |_target|address|Contract to receive the function call
 |_action|bytes|Bytes array encoding the function call and arguments
+|_strict|bool|Boolean indicating whether the transaction must succeed
 
 **Return Parameters**
 
diff --git a/packages/metatransaction-broadcaster/MetatransactionBroadcaster.js b/packages/metatransaction-broadcaster/MetatransactionBroadcaster.js
@@ -208,6 +208,7 @@ class MetatransactionBroadcaster {
 
     // Add the old makeSingleArbitraryTransaction and makeArbitraryTransactions to the abi
     const iface = new ethers.utils.Interface([
+      "function makeArbitraryTransaction(address,bytes)",
       "function makeSingleArbitraryTransaction(address,bytes)",
       "function makeArbitraryTransactions(address[],bytes[],bool)",
     ]);
@@ -223,6 +224,7 @@ class MetatransactionBroadcaster {
       // If it's an arbitrary transaction...
       if (
         tx.signature === "makeArbitraryTransaction(address,bytes)" ||
+        tx.signature === "makeArbitraryTransaction(address,bytes,bool)" ||
         tx.signature === "makeSingleArbitraryTransaction(address,bytes)" ||
         tx.signature === "makeArbitraryTransactions(address[],bytes[],bool)"
       ) {
diff --git a/test/contracts-network/colony-arbitrary-transactions.js b/test/contracts-network/colony-arbitrary-transactions.js
@@ -40,7 +40,7 @@ contract("Colony Arbitrary Transactions", (accounts) => {
     const action = await encodeTxData(token, "mint", [WAD]);
     const balancePre = await token.balanceOf(colony.address);
 
-    const tx = await colony.makeArbitraryTransaction(token.address, action);
+    const tx = await colony.makeArbitraryTransaction(token.address, action, false);
 
     await expectEvent(tx, "ArbitraryTransaction(address,bytes,bool)", [token.address, action, true]);
 
@@ -53,8 +53,8 @@ contract("Colony Arbitrary Transactions", (accounts) => {
     const action2 = await encodeTxData(token, "mint", [WAD.muln(2)]);
     const balancePre = await token.balanceOf(colony.address);
 
-    const arbitraryAction = await encodeTxData(colony, "makeArbitraryTransaction", [token.address, action]);
-    const arbitraryAction2 = await encodeTxData(colony, "makeArbitraryTransaction", [token.address, action2]);
+    const arbitraryAction = await encodeTxData(colony, "makeArbitraryTransaction", [token.address, action, false]);
+    const arbitraryAction2 = await encodeTxData(colony, "makeArbitraryTransaction", [token.address, action2, false]);
 
     const tx = await colony.multicall([arbitraryAction, arbitraryAction2]);
 
@@ -68,15 +68,15 @@ contract("Colony Arbitrary Transactions", (accounts) => {
   it("should not be able to make arbitrary transactions if not root", async () => {
     const action = await encodeTxData(token, "mint", [WAD]);
 
-    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action, { from: USER1 }), "ds-auth-unauthorized");
+    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action, false, { from: USER1 }), "ds-auth-unauthorized");
   });
 
   it("should not be able to make arbitrary transactions to a colony itself", async () => {
-    await checkErrorRevert(colony.makeArbitraryTransaction(colony.address, "0x0"), "colony-cannot-target-self");
+    await checkErrorRevert(colony.makeArbitraryTransaction(colony.address, 0x0, false), "colony-cannot-target-self");
   });
 
   it("should not be able to make arbitrary transactions to a user address", async () => {
-    await checkErrorRevert(colony.makeArbitraryTransaction(accounts[0], "0x0"), "colony-to-must-be-contract");
+    await checkErrorRevert(colony.makeArbitraryTransaction(accounts[0], 0x0, false), "colony-to-must-be-contract");
   });
 
   it("should not be able to make arbitrary transactions to network or token locking", async () => {
@@ -86,14 +86,14 @@ contract("Colony Arbitrary Transactions", (accounts) => {
     const action1 = await encodeTxData(colonyNetwork, "addSkill", [0]);
     const action2 = await encodeTxData(tokenLocking, "lockToken", [token.address]);
 
-    await checkErrorRevert(colony.makeArbitraryTransaction(colonyNetwork.address, action1), "colony-cannot-target-network");
-    await checkErrorRevert(colony.makeArbitraryTransaction(tokenLocking.address, action2), "colony-cannot-target-token-locking");
+    await checkErrorRevert(colony.makeArbitraryTransaction(colonyNetwork.address, action1, false), "colony-cannot-target-network");
+    await checkErrorRevert(colony.makeArbitraryTransaction(tokenLocking.address, action2, false), "colony-cannot-target-token-locking");
   });
 
   it("if an arbitrary transaction is made to approve tokens, then tokens needed for approval cannot be moved out of the main pot", async () => {
     await fundColonyWithTokens(colony, token, 100);
     const action1 = await encodeTxData(token, "approve", [USER0, 50]);
-    await colony.makeArbitraryTransaction(token.address, action1);
+    await colony.makeArbitraryTransaction(token.address, action1, false);
     await colony.moveFundsBetweenPots(1, UINT256_MAX, 1, UINT256_MAX, UINT256_MAX, 1, 0, 50, token.address);
     await checkErrorRevert(
       colony.moveFundsBetweenPots(1, UINT256_MAX, 1, UINT256_MAX, UINT256_MAX, 1, 0, 50, token.address),
@@ -109,7 +109,7 @@ contract("Colony Arbitrary Transactions", (accounts) => {
    tokens can only be moved from main pot that weren't part of the allowance`, async () => {
     await fundColonyWithTokens(colony, token, 100);
     const action1 = await encodeTxData(token, "approve", [USER0, 20]);
-    await colony.makeArbitraryTransaction(token.address, action1);
+    await colony.makeArbitraryTransaction(token.address, action1, false);
     // Use allowance
     await token.transferFrom(colony.address, USER0, 20, { from: USER0 });
     // Approval tracking still thinks it has to reserve 20
@@ -141,9 +141,9 @@ contract("Colony Arbitrary Transactions", (accounts) => {
     await fundColonyWithTokens(colony, token, 100);
     const action1 = await encodeTxData(token, "approve", [USER0, 300]);
     // Not enough tokens at all
-    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action1), "colony-approval-exceeds-balance");
+    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action1, false), "colony-approval-exceeds-balance");
     await fundColonyWithTokens(colony, token, 1000);
-    await colony.makeArbitraryTransaction(token.address, action1);
+    await colony.makeArbitraryTransaction(token.address, action1, false);
     // They are now approved for 300.
     let approval = await colony.getTokenApproval(token.address, USER0);
     expect(approval).to.be.eq.BN(300);
@@ -152,25 +152,25 @@ contract("Colony Arbitrary Transactions", (accounts) => {
 
     const action2 = await encodeTxData(token, "approve", [USER0, 900]);
     // User was approved for 300, we now approve them for 900. There are enough tokens to cover this, even though 900 + 300 > 1100, the balance of the pot
-    await colony.makeArbitraryTransaction(token.address, action2);
+    await colony.makeArbitraryTransaction(token.address, action2, false);
     approval = await colony.getTokenApproval(token.address, USER0);
     expect(approval).to.be.eq.BN(900);
     allApprovals = await colony.getTotalTokenApproval(token.address);
     expect(allApprovals).to.be.eq.BN(900);
 
     // Set them back to 300
-    await colony.makeArbitraryTransaction(token.address, action1);
+    await colony.makeArbitraryTransaction(token.address, action1, false);
     approval = await colony.getTokenApproval(token.address, USER0);
     expect(approval).to.be.eq.BN(300);
     allApprovals = await colony.getTotalTokenApproval(token.address);
     expect(allApprovals).to.be.eq.BN(300);
 
     // Cannot approve someone else for 900
     const action3 = await encodeTxData(token, "approve", [USER1, 900]);
-    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action3), "colony-approval-exceeds-balance");
+    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action3, false), "colony-approval-exceeds-balance");
     // But can for 800
     const action4 = await encodeTxData(token, "approve", [USER1, 800]);
-    await colony.makeArbitraryTransaction(token.address, action4);
+    await colony.makeArbitraryTransaction(token.address, action4, false);
     approval = await colony.getTokenApproval(token.address, USER1);
     expect(approval).to.be.eq.BN(800);
     allApprovals = await colony.getTotalTokenApproval(token.address);
@@ -212,47 +212,47 @@ contract("Colony Arbitrary Transactions", (accounts) => {
       0,
     ]);
 
-    await checkErrorRevert(colony.makeArbitraryTransaction(oneTxPayment.address, action), "colony-cannot-target-extensions");
+    await checkErrorRevert(colony.makeArbitraryTransaction(oneTxPayment.address, action, false), "colony-cannot-target-extensions");
 
     // But other colonies can
     const { colony: otherColony } = await setupRandomColony(colonyNetwork);
     await colony.setUserRoles(1, UINT256_MAX, otherColony.address, 1, ROLES);
 
-    await otherColony.makeArbitraryTransaction(oneTxPayment.address, action);
+    await otherColony.makeArbitraryTransaction(oneTxPayment.address, action, false);
   });
 
   it("when burning tokens, can burn own tokens with burn(amount) up to the amount unspoken for in root pot", async () => {
     await fundColonyWithTokens(colony, token, 100);
     const action1 = await encodeTxData(token, "approve", [USER0, 60]);
-    await colony.makeArbitraryTransaction(token.address, action1);
+    await colony.makeArbitraryTransaction(token.address, action1, false);
     let potBalance = await colony.getFundingPotBalance(1, token.address);
     expect(potBalance).to.be.eq.BN(100);
 
     const action2 = await encodeTxData(token, "burn", [100]);
     // Can't  burn 100 as 60 are reserved
-    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action2), "colony-not-enough-tokens");
+    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action2, false), "colony-not-enough-tokens");
 
     // Can burn 40
     const action3 = await encodeTxData(token, "burn", [40]);
-    await colony.makeArbitraryTransaction(token.address, action3);
+    await colony.makeArbitraryTransaction(token.address, action3, false);
     potBalance = await colony.getFundingPotBalance(1, token.address);
     expect(potBalance).to.be.eq.BN(60);
   });
 
   it("when transferring tokens, can transfer own tokens with transfer(dst, amount) up to the amount unspoken for in root pot", async () => {
     await fundColonyWithTokens(colony, token, 100);
     const action1 = await encodeTxData(token, "approve", [USER0, 60]);
-    await colony.makeArbitraryTransaction(token.address, action1);
+    await colony.makeArbitraryTransaction(token.address, action1, false);
     let potBalance = await colony.getFundingPotBalance(1, token.address);
     expect(potBalance).to.be.eq.BN(100);
 
     const action2 = await encodeTxData(token, "transfer", [USER0, 100]);
     // Can't transfer 100 as 60 are reserved
-    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action2), "colony-not-enough-tokens");
+    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action2, false), "colony-not-enough-tokens");
 
     // Can transfer 40
     const action3 = await encodeTxData(token, "transfer", [USER0, 40]);
-    await colony.makeArbitraryTransaction(token.address, action3);
+    await colony.makeArbitraryTransaction(token.address, action3, false);
     potBalance = await colony.getFundingPotBalance(1, token.address);
     expect(potBalance).to.be.eq.BN(60);
     const userBalance = await token.balanceOf(USER0);
@@ -263,7 +263,7 @@ contract("Colony Arbitrary Transactions", (accounts) => {
     await token.mint(100);
     await token.approve(colony.address, 100);
     const action1 = await encodeTxData(token, "burn", [USER0, 60]);
-    await colony.makeArbitraryTransaction(token.address, action1);
+    await colony.makeArbitraryTransaction(token.address, action1, false);
 
     const userBalance = await token.balanceOf(USER0);
     expect(userBalance).to.be.eq.BN(40);
@@ -273,7 +273,7 @@ contract("Colony Arbitrary Transactions", (accounts) => {
     await token.mint(100);
     await token.approve(colony.address, 100);
     const action1 = await encodeTxData(token, "transferFrom", [USER0, colony.address, 60]);
-    await colony.makeArbitraryTransaction(token.address, action1);
+    await colony.makeArbitraryTransaction(token.address, action1, false);
 
     const userBalance = await token.balanceOf(USER0);
     expect(userBalance).to.be.eq.BN(40);
@@ -283,11 +283,11 @@ contract("Colony Arbitrary Transactions", (accounts) => {
 
   it("cannot burn own tokens with burn(guy, amount)", async () => {
     const action = await encodeTxData(token, "burn", [colony.address, 60]);
-    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action), "colony-cannot-spend-own-allowance");
+    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action, false), "colony-cannot-spend-own-allowance");
   });
 
   it("cannot transfer own tokens with transferFrom(from, to, amount)", async () => {
     const action = await encodeTxData(token, "transferFrom", [colony.address, USER0, 60]);
-    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action), "colony-cannot-spend-own-allowance");
+    await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action, false), "colony-cannot-spend-own-allowance");
   });
 });
diff --git a/test/contracts-network/colony-permissions.js b/test/contracts-network/colony-permissions.js
@@ -287,7 +287,7 @@ contract("ColonyPermissions", (accounts) => {
       await checkErrorRevert(colony.installExtension(HASHZERO, ADDRESS_ZERO, { from: USER1 }), "ds-auth-unauthorized");
       await checkErrorRevert(colony.addLocalSkill({ from: USER1 }), "ds-auth-unauthorized");
       await checkErrorRevert(colony.deprecateLocalSkill(0, true, { from: USER2 }), "ds-auth-unauthorized");
-      await checkErrorRevert(colony.makeArbitraryTransaction(ADDRESS_ZERO, HASHZERO, { from: USER1 }), "ds-auth-unauthorized");
+      await checkErrorRevert(colony.makeArbitraryTransaction(ADDRESS_ZERO, HASHZERO, false, { from: USER1 }), "ds-auth-unauthorized");
       await checkErrorRevert(colony.startNextRewardPayout(ADDRESS_ZERO, HASHZERO, HASHZERO, 0, [HASHZERO], { from: USER1 }), "ds-auth-unauthorized");
     });
 
diff --git a/test/contracts-network/colony-recovery.js b/test/contracts-network/colony-recovery.js
@@ -203,7 +203,7 @@ contract("Colony Recovery", (accounts) => {
       await checkErrorRevert(metaColony.burnTokens(ADDRESS_ZERO, 0), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.registerColonyLabel("", ""), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.updateColonyOrbitDB(""), "colony-in-recovery-mode");
-      await checkErrorRevert(metaColony.makeArbitraryTransaction(ADDRESS_ZERO, HASHZERO), "colony-in-recovery-mode");
+      await checkErrorRevert(metaColony.makeArbitraryTransaction(ADDRESS_ZERO, HASHZERO, false), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.updateApprovalAmount(ADDRESS_ZERO, ADDRESS_ZERO), "colony-in-recovery-mode");
       await checkErrorRevert(metaColony.finalizeRewardPayout(1), "colony-in-recovery-mode");
     });
diff --git a/test/cross-chain/cross-chain.js b/test/cross-chain/cross-chain.js
@@ -440,7 +440,7 @@ contract("Cross-chain", (accounts) => {
 
       console.log("tx to home bridge address:", homeBridge.address);
 
-      const tx = await homeColony.makeArbitraryTransaction(homeBridge.address, txDataToBeSentToAMB);
+      const tx = await homeColony.makeArbitraryTransaction(homeBridge.address, txDataToBeSentToAMB, true);
       await tx.wait();
       await p;
       // Check balances
diff --git a/test/extensions/voting-rep.js b/test/extensions/voting-rep.js
@@ -2111,7 +2111,7 @@ contract("Voting Reputation", (accounts) => {
     });
 
     it("transactions that try to execute a forbidden method on Reputation Voting extension are rejected by the MTX broadcaster", async function () {
-      const action = await encodeTxData(colony, "makeArbitraryTransactions", [[colony.address], ["0x00"], true]);
+      const action = await encodeTxData(colony, "makeArbitraryTransactions(address[],bytes[],bool)", [[colony.address], ["0x00"], true]);
 
       await voting.createMotion(1, UINT256_MAX, ADDRESS_ZERO, action, domain1Key, domain1Value, domain1Mask, domain1Siblings);
       motionId = await voting.getMotionCount();
diff --git a/test/packages/metaTransactionBroadcaster.js b/test/packages/metaTransactionBroadcaster.js
