Skip to content

Commit 373c610

Browse files
kronosapienskronosapiens
committed
Add _strict flag to makeArbitraryTransaction
1 parent 7a82029 commit 373c610

File tree

12 files changed

+59
-39
lines changed

12 files changed

+59
-39
lines changed

‎contracts/colony/ColonyArbitraryTransaction.sol

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ contract ColonyArbitraryTransaction is ColonyStorage {
3636

3737
function makeArbitraryTransaction(
3838
address _to,
39-
bytes memory _action
39+
bytes memory _action,
40+
bool _strict
4041
) public stoppable auth returns (bool) {
4142
// Prevent transactions to network contracts
4243
require(_to != address(this), "colony-cannot-target-self");
@@ -78,6 +79,8 @@ contract ColonyArbitraryTransaction is ColonyStorage {
7879

7980
emit ArbitraryTransaction(_to, _action, res);
8081

82+
require(res || !_strict, "colony-arbitrary-transaction-failed");
83+
8184
return res;
8285
}
8386

‎contracts/colony/ColonyAuthority.sol

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ contract ColonyAuthority is CommonAuthority {
9898
addRoleCapability(ROOT_ROLE, "upgradeExtension(bytes32,uint256)");
9999
addRoleCapability(ROOT_ROLE, "deprecateExtension(bytes32,bool)");
100100
addRoleCapability(ROOT_ROLE, "uninstallExtension(bytes32)");
101-
addRoleCapability(ROOT_ROLE, "makeArbitraryTransaction(address,bytes)");
101+
addRoleCapability(ROOT_ROLE, "makeArbitraryTransaction(address,bytes)"); // Deprecated
102102
addRoleCapability(ROOT_ROLE, "emitDomainReputationReward(uint256,address,int256)");
103103
addRoleCapability(ROOT_ROLE, "emitSkillReputationReward(uint256,address,int256)");
104104
addRoleCapability(ARBITRATION_ROLE, "transferStake(uint256,uint256,address,address,uint256,uint256,address)");
@@ -134,6 +134,9 @@ contract ColonyAuthority is CommonAuthority {
134134
addRoleCapability(ARBITRATION_ROLE, "finalizeExpenditureViaArbitration(uint256,uint256,uint256)");
135135
addRoleCapability(ROOT_ROLE, "setColonyBridgeAddress(address)");
136136
addRoleCapability(ROOT_ROLE, "initialiseReputationMining(uint256,bytes32,uint256)");
137+
138+
// Added in colony v17 (jade-lwss)
139+
addRoleCapability(ROOT_ROLE, "makeArbitraryTransaction(address,bytes,bool)");
137140
}
138141

139142
function addRoleCapability(uint8 role, bytes memory sig) private {

‎contracts/colony/IColony.sol

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,12 @@ interface IColony is IDSAuth, ColonyDataTypes, IRecovery, IBasicMetaTransaction,
5454
/// @notice Executes an arbitrary transaction
5555
/// @param _target Contract to receive the function call
5656
/// @param _action Bytes array encoding the function call and arguments
57+
/// @param _strict Boolean indicating whether the transaction must succeed
5758
/// @return success Boolean indicating whether the transactions succeeded
5859
function makeArbitraryTransaction(
5960
address _target,
60-
bytes memory _action
61+
bytes memory _action,
62+
bool _strict
6163
) external returns (bool success);
6264

6365
/// @notice Emit a metadata string for a transaction

‎docs/interfaces/icolony.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1049,7 +1049,7 @@ Lock the colony's token. Can only be called by a network-managed extension.
10491049
|---|---|---|
10501050
|timesLocked|uint256|The amount of times the token was locked
10511051

1052-
### ▸ `makeArbitraryTransaction(address _target, bytes memory _action):bool success`
1052+
### ▸ `makeArbitraryTransaction(address _target, bytes memory _action, bool _strict):bool success`
10531053

10541054
Executes an arbitrary transaction
10551055

@@ -1060,6 +1060,7 @@ Executes an arbitrary transaction
10601060
|---|---|---|
10611061
|_target|address|Contract to receive the function call
10621062
|_action|bytes|Bytes array encoding the function call and arguments
1063+
|_strict|bool|Boolean indicating whether the transaction must succeed
10631064

10641065
**Return Parameters**
10651066

‎docs/interfaces/imetacolony.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1087,7 +1087,7 @@ Lock the colony's token. Can only be called by a network-managed extension.
10871087
|---|---|---|
10881088
|timesLocked|uint256|The amount of times the token was locked
10891089

1090-
### ▸ `makeArbitraryTransaction(address _target, bytes memory _action):bool success`
1090+
### ▸ `makeArbitraryTransaction(address _target, bytes memory _action, bool _strict):bool success`
10911091

10921092
Executes an arbitrary transaction
10931093

@@ -1098,6 +1098,7 @@ Executes an arbitrary transaction
10981098
|---|---|---|
10991099
|_target|address|Contract to receive the function call
11001100
|_action|bytes|Bytes array encoding the function call and arguments
1101+
|_strict|bool|Boolean indicating whether the transaction must succeed
11011102

11021103
**Return Parameters**
11031104

‎packages/metatransaction-broadcaster/MetatransactionBroadcaster.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ class MetatransactionBroadcaster {
208208

209209
// Add the old makeSingleArbitraryTransaction and makeArbitraryTransactions to the abi
210210
const iface = new ethers.utils.Interface([
211+
"function makeArbitraryTransaction(address,bytes)",
211212
"function makeSingleArbitraryTransaction(address,bytes)",
212213
"function makeArbitraryTransactions(address[],bytes[],bool)",
213214
]);
@@ -223,6 +224,7 @@ class MetatransactionBroadcaster {
223224
// If it's an arbitrary transaction...
224225
if (
225226
tx.signature === "makeArbitraryTransaction(address,bytes)" ||
227+
tx.signature === "makeArbitraryTransaction(address,bytes,bool)" ||
226228
tx.signature === "makeSingleArbitraryTransaction(address,bytes)" ||
227229
tx.signature === "makeArbitraryTransactions(address[],bytes[],bool)"
228230
) {

‎test/contracts-network/colony-arbitrary-transactions.js

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ contract("Colony Arbitrary Transactions", (accounts) => {
4040
const action = await encodeTxData(token, "mint", [WAD]);
4141
const balancePre = await token.balanceOf(colony.address);
4242

43-
const tx = await colony.makeArbitraryTransaction(token.address, action);
43+
const tx = await colony.makeArbitraryTransaction(token.address, action, false);
4444

4545
await expectEvent(tx, "ArbitraryTransaction(address,bytes,bool)", [token.address, action, true]);
4646

@@ -53,8 +53,8 @@ contract("Colony Arbitrary Transactions", (accounts) => {
5353
const action2 = await encodeTxData(token, "mint", [WAD.muln(2)]);
5454
const balancePre = await token.balanceOf(colony.address);
5555

56-
const arbitraryAction = await encodeTxData(colony, "makeArbitraryTransaction", [token.address, action]);
57-
const arbitraryAction2 = await encodeTxData(colony, "makeArbitraryTransaction", [token.address, action2]);
56+
const arbitraryAction = await encodeTxData(colony, "makeArbitraryTransaction", [token.address, action, false]);
57+
const arbitraryAction2 = await encodeTxData(colony, "makeArbitraryTransaction", [token.address, action2, false]);
5858

5959
const tx = await colony.multicall([arbitraryAction, arbitraryAction2]);
6060

@@ -68,15 +68,15 @@ contract("Colony Arbitrary Transactions", (accounts) => {
6868
it("should not be able to make arbitrary transactions if not root", async () => {
6969
const action = await encodeTxData(token, "mint", [WAD]);
7070

71-
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action, { from: USER1 }), "ds-auth-unauthorized");
71+
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action, false, { from: USER1 }), "ds-auth-unauthorized");
7272
});
7373

7474
it("should not be able to make arbitrary transactions to a colony itself", async () => {
75-
await checkErrorRevert(colony.makeArbitraryTransaction(colony.address, "0x0"), "colony-cannot-target-self");
75+
await checkErrorRevert(colony.makeArbitraryTransaction(colony.address, 0x0, false), "colony-cannot-target-self");
7676
});
7777

7878
it("should not be able to make arbitrary transactions to a user address", async () => {
79-
await checkErrorRevert(colony.makeArbitraryTransaction(accounts[0], "0x0"), "colony-to-must-be-contract");
79+
await checkErrorRevert(colony.makeArbitraryTransaction(accounts[0], 0x0, false), "colony-to-must-be-contract");
8080
});
8181

8282
it("should not be able to make arbitrary transactions to network or token locking", async () => {
@@ -86,14 +86,14 @@ contract("Colony Arbitrary Transactions", (accounts) => {
8686
const action1 = await encodeTxData(colonyNetwork, "addSkill", [0]);
8787
const action2 = await encodeTxData(tokenLocking, "lockToken", [token.address]);
8888

89-
await checkErrorRevert(colony.makeArbitraryTransaction(colonyNetwork.address, action1), "colony-cannot-target-network");
90-
await checkErrorRevert(colony.makeArbitraryTransaction(tokenLocking.address, action2), "colony-cannot-target-token-locking");
89+
await checkErrorRevert(colony.makeArbitraryTransaction(colonyNetwork.address, action1, false), "colony-cannot-target-network");
90+
await checkErrorRevert(colony.makeArbitraryTransaction(tokenLocking.address, action2, false), "colony-cannot-target-token-locking");
9191
});
9292

9393
it("if an arbitrary transaction is made to approve tokens, then tokens needed for approval cannot be moved out of the main pot", async () => {
9494
await fundColonyWithTokens(colony, token, 100);
9595
const action1 = await encodeTxData(token, "approve", [USER0, 50]);
96-
await colony.makeArbitraryTransaction(token.address, action1);
96+
await colony.makeArbitraryTransaction(token.address, action1, false);
9797
await colony.moveFundsBetweenPots(1, UINT256_MAX, 1, UINT256_MAX, UINT256_MAX, 1, 0, 50, token.address);
9898
await checkErrorRevert(
9999
colony.moveFundsBetweenPots(1, UINT256_MAX, 1, UINT256_MAX, UINT256_MAX, 1, 0, 50, token.address),
@@ -109,7 +109,7 @@ contract("Colony Arbitrary Transactions", (accounts) => {
109109
tokens can only be moved from main pot that weren't part of the allowance`, async () => {
110110
await fundColonyWithTokens(colony, token, 100);
111111
const action1 = await encodeTxData(token, "approve", [USER0, 20]);
112-
await colony.makeArbitraryTransaction(token.address, action1);
112+
await colony.makeArbitraryTransaction(token.address, action1, false);
113113
// Use allowance
114114
await token.transferFrom(colony.address, USER0, 20, { from: USER0 });
115115
// Approval tracking still thinks it has to reserve 20
@@ -141,9 +141,9 @@ contract("Colony Arbitrary Transactions", (accounts) => {
141141
await fundColonyWithTokens(colony, token, 100);
142142
const action1 = await encodeTxData(token, "approve", [USER0, 300]);
143143
// Not enough tokens at all
144-
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action1), "colony-approval-exceeds-balance");
144+
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action1, false), "colony-approval-exceeds-balance");
145145
await fundColonyWithTokens(colony, token, 1000);
146-
await colony.makeArbitraryTransaction(token.address, action1);
146+
await colony.makeArbitraryTransaction(token.address, action1, false);
147147
// They are now approved for 300.
148148
let approval = await colony.getTokenApproval(token.address, USER0);
149149
expect(approval).to.be.eq.BN(300);
@@ -152,25 +152,25 @@ contract("Colony Arbitrary Transactions", (accounts) => {
152152

153153
const action2 = await encodeTxData(token, "approve", [USER0, 900]);
154154
// 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
155-
await colony.makeArbitraryTransaction(token.address, action2);
155+
await colony.makeArbitraryTransaction(token.address, action2, false);
156156
approval = await colony.getTokenApproval(token.address, USER0);
157157
expect(approval).to.be.eq.BN(900);
158158
allApprovals = await colony.getTotalTokenApproval(token.address);
159159
expect(allApprovals).to.be.eq.BN(900);
160160

161161
// Set them back to 300
162-
await colony.makeArbitraryTransaction(token.address, action1);
162+
await colony.makeArbitraryTransaction(token.address, action1, false);
163163
approval = await colony.getTokenApproval(token.address, USER0);
164164
expect(approval).to.be.eq.BN(300);
165165
allApprovals = await colony.getTotalTokenApproval(token.address);
166166
expect(allApprovals).to.be.eq.BN(300);
167167

168168
// Cannot approve someone else for 900
169169
const action3 = await encodeTxData(token, "approve", [USER1, 900]);
170-
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action3), "colony-approval-exceeds-balance");
170+
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action3, false), "colony-approval-exceeds-balance");
171171
// But can for 800
172172
const action4 = await encodeTxData(token, "approve", [USER1, 800]);
173-
await colony.makeArbitraryTransaction(token.address, action4);
173+
await colony.makeArbitraryTransaction(token.address, action4, false);
174174
approval = await colony.getTokenApproval(token.address, USER1);
175175
expect(approval).to.be.eq.BN(800);
176176
allApprovals = await colony.getTotalTokenApproval(token.address);
@@ -212,47 +212,47 @@ contract("Colony Arbitrary Transactions", (accounts) => {
212212
0,
213213
]);
214214

215-
await checkErrorRevert(colony.makeArbitraryTransaction(oneTxPayment.address, action), "colony-cannot-target-extensions");
215+
await checkErrorRevert(colony.makeArbitraryTransaction(oneTxPayment.address, action, false), "colony-cannot-target-extensions");
216216

217217
// But other colonies can
218218
const { colony: otherColony } = await setupRandomColony(colonyNetwork);
219219
await colony.setUserRoles(1, UINT256_MAX, otherColony.address, 1, ROLES);
220220

221-
await otherColony.makeArbitraryTransaction(oneTxPayment.address, action);
221+
await otherColony.makeArbitraryTransaction(oneTxPayment.address, action, false);
222222
});
223223

224224
it("when burning tokens, can burn own tokens with burn(amount) up to the amount unspoken for in root pot", async () => {
225225
await fundColonyWithTokens(colony, token, 100);
226226
const action1 = await encodeTxData(token, "approve", [USER0, 60]);
227-
await colony.makeArbitraryTransaction(token.address, action1);
227+
await colony.makeArbitraryTransaction(token.address, action1, false);
228228
let potBalance = await colony.getFundingPotBalance(1, token.address);
229229
expect(potBalance).to.be.eq.BN(100);
230230

231231
const action2 = await encodeTxData(token, "burn", [100]);
232232
// Can't burn 100 as 60 are reserved
233-
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action2), "colony-not-enough-tokens");
233+
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action2, false), "colony-not-enough-tokens");
234234

235235
// Can burn 40
236236
const action3 = await encodeTxData(token, "burn", [40]);
237-
await colony.makeArbitraryTransaction(token.address, action3);
237+
await colony.makeArbitraryTransaction(token.address, action3, false);
238238
potBalance = await colony.getFundingPotBalance(1, token.address);
239239
expect(potBalance).to.be.eq.BN(60);
240240
});
241241

242242
it("when transferring tokens, can transfer own tokens with transfer(dst, amount) up to the amount unspoken for in root pot", async () => {
243243
await fundColonyWithTokens(colony, token, 100);
244244
const action1 = await encodeTxData(token, "approve", [USER0, 60]);
245-
await colony.makeArbitraryTransaction(token.address, action1);
245+
await colony.makeArbitraryTransaction(token.address, action1, false);
246246
let potBalance = await colony.getFundingPotBalance(1, token.address);
247247
expect(potBalance).to.be.eq.BN(100);
248248

249249
const action2 = await encodeTxData(token, "transfer", [USER0, 100]);
250250
// Can't transfer 100 as 60 are reserved
251-
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action2), "colony-not-enough-tokens");
251+
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action2, false), "colony-not-enough-tokens");
252252

253253
// Can transfer 40
254254
const action3 = await encodeTxData(token, "transfer", [USER0, 40]);
255-
await colony.makeArbitraryTransaction(token.address, action3);
255+
await colony.makeArbitraryTransaction(token.address, action3, false);
256256
potBalance = await colony.getFundingPotBalance(1, token.address);
257257
expect(potBalance).to.be.eq.BN(60);
258258
const userBalance = await token.balanceOf(USER0);
@@ -263,7 +263,7 @@ contract("Colony Arbitrary Transactions", (accounts) => {
263263
await token.mint(100);
264264
await token.approve(colony.address, 100);
265265
const action1 = await encodeTxData(token, "burn", [USER0, 60]);
266-
await colony.makeArbitraryTransaction(token.address, action1);
266+
await colony.makeArbitraryTransaction(token.address, action1, false);
267267

268268
const userBalance = await token.balanceOf(USER0);
269269
expect(userBalance).to.be.eq.BN(40);
@@ -273,7 +273,7 @@ contract("Colony Arbitrary Transactions", (accounts) => {
273273
await token.mint(100);
274274
await token.approve(colony.address, 100);
275275
const action1 = await encodeTxData(token, "transferFrom", [USER0, colony.address, 60]);
276-
await colony.makeArbitraryTransaction(token.address, action1);
276+
await colony.makeArbitraryTransaction(token.address, action1, false);
277277

278278
const userBalance = await token.balanceOf(USER0);
279279
expect(userBalance).to.be.eq.BN(40);
@@ -283,11 +283,11 @@ contract("Colony Arbitrary Transactions", (accounts) => {
283283

284284
it("cannot burn own tokens with burn(guy, amount)", async () => {
285285
const action = await encodeTxData(token, "burn", [colony.address, 60]);
286-
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action), "colony-cannot-spend-own-allowance");
286+
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action, false), "colony-cannot-spend-own-allowance");
287287
});
288288

289289
it("cannot transfer own tokens with transferFrom(from, to, amount)", async () => {
290290
const action = await encodeTxData(token, "transferFrom", [colony.address, USER0, 60]);
291-
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action), "colony-cannot-spend-own-allowance");
291+
await checkErrorRevert(colony.makeArbitraryTransaction(token.address, action, false), "colony-cannot-spend-own-allowance");
292292
});
293293
});

‎test/contracts-network/colony-permissions.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ contract("ColonyPermissions", (accounts) => {
287287
await checkErrorRevert(colony.installExtension(HASHZERO, ADDRESS_ZERO, { from: USER1 }), "ds-auth-unauthorized");
288288
await checkErrorRevert(colony.addLocalSkill({ from: USER1 }), "ds-auth-unauthorized");
289289
await checkErrorRevert(colony.deprecateLocalSkill(0, true, { from: USER2 }), "ds-auth-unauthorized");
290-
await checkErrorRevert(colony.makeArbitraryTransaction(ADDRESS_ZERO, HASHZERO, { from: USER1 }), "ds-auth-unauthorized");
290+
await checkErrorRevert(colony.makeArbitraryTransaction(ADDRESS_ZERO, HASHZERO, false, { from: USER1 }), "ds-auth-unauthorized");
291291
await checkErrorRevert(colony.startNextRewardPayout(ADDRESS_ZERO, HASHZERO, HASHZERO, 0, [HASHZERO], { from: USER1 }), "ds-auth-unauthorized");
292292
});
293293

‎test/contracts-network/colony-recovery.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ contract("Colony Recovery", (accounts) => {
203203
await checkErrorRevert(metaColony.burnTokens(ADDRESS_ZERO, 0), "colony-in-recovery-mode");
204204
await checkErrorRevert(metaColony.registerColonyLabel("", ""), "colony-in-recovery-mode");
205205
await checkErrorRevert(metaColony.updateColonyOrbitDB(""), "colony-in-recovery-mode");
206-
await checkErrorRevert(metaColony.makeArbitraryTransaction(ADDRESS_ZERO, HASHZERO), "colony-in-recovery-mode");
206+
await checkErrorRevert(metaColony.makeArbitraryTransaction(ADDRESS_ZERO, HASHZERO, false), "colony-in-recovery-mode");
207207
await checkErrorRevert(metaColony.updateApprovalAmount(ADDRESS_ZERO, ADDRESS_ZERO), "colony-in-recovery-mode");
208208
await checkErrorRevert(metaColony.finalizeRewardPayout(1), "colony-in-recovery-mode");
209209
});

‎test/cross-chain/cross-chain.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ contract("Cross-chain", (accounts) => {
440440

441441
console.log("tx to home bridge address:", homeBridge.address);
442442

443-
const tx = await homeColony.makeArbitraryTransaction(homeBridge.address, txDataToBeSentToAMB);
443+
const tx = await homeColony.makeArbitraryTransaction(homeBridge.address, txDataToBeSentToAMB, true);
444444
await tx.wait();
445445
await p;
446446
// Check balances

‎test/extensions/voting-rep.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2111,7 +2111,7 @@ contract("Voting Reputation", (accounts) => {
21112111
});
21122112

21132113
it("transactions that try to execute a forbidden method on Reputation Voting extension are rejected by the MTX broadcaster", async function () {
2114-
const action = await encodeTxData(colony, "makeArbitraryTransactions", [[colony.address], ["0x00"], true]);
2114+
const action = await encodeTxData(colony, "makeArbitraryTransactions(address[],bytes[],bool)", [[colony.address], ["0x00"], true]);
21152115

21162116
await voting.createMotion(1, UINT256_MAX, ADDRESS_ZERO, action, domain1Key, domain1Value, domain1Mask, domain1Siblings);
21172117
motionId = await voting.getMotionCount();

0 commit comments

Comments
 (0)