From c310aeb096baa05c94a3ef19d7c444e23ec00931 Mon Sep 17 00:00:00 2001 From: Ben Date: Thu, 14 Nov 2019 17:01:00 +0200 Subject: [PATCH 1/6] Remove concat from DAOFactory contract --- contracts/utils/DAOFactory.sol | 12 ---------- package-lock.json | 41 +++++++++++++++++++++++++--------- test/contributionreward.js | 8 +++---- test/daofactory.js | 31 ++++++++++--------------- test/helpers.js | 8 +++++++ 5 files changed, 53 insertions(+), 47 deletions(-) diff --git a/contracts/utils/DAOFactory.sol b/contracts/utils/DAOFactory.sol index 164106cc..581a6d07 100644 --- a/contracts/utils/DAOFactory.sol +++ b/contracts/utils/DAOFactory.sol @@ -137,18 +137,6 @@ contract DAOFactory is Initializable { _schemesInitilizeDataLens, _permissions, _metaData); - } - - //this function is an helper function to concate 2 bytes vars and return its length. - //todo: implement that offlince and remove it from the contract - function bytesConcat(bytes calldata _preBytes, bytes calldata _postBytes) - external - pure - returns (bytes memory, uint256, uint256) { - if (_postBytes.length == 0) { - return (_preBytes, _preBytes.length, 0); - } - return (_preBytes.concat(_postBytes), _preBytes.length, _postBytes.length); } /** diff --git a/package-lock.json b/package-lock.json index e3b89124..584c69c5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4009,7 +4009,8 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "aproba": { "version": "1.2.0", @@ -4030,12 +4031,14 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, + "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -4050,17 +4053,20 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -4177,7 +4183,8 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -4189,6 +4196,7 @@ "version": "1.0.0", "bundled": true, "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4203,6 +4211,7 @@ "version": "3.0.4", "bundled": true, "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -4210,12 +4219,14 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "minipass": { "version": "2.3.5", "bundled": true, "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -4234,6 +4245,7 @@ "version": "0.5.1", "bundled": true, "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -4314,7 +4326,8 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -4326,6 +4339,7 @@ "version": "1.4.0", "bundled": true, "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -4411,7 +4425,8 @@ "safe-buffer": { "version": "5.1.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "safer-buffer": { "version": "2.1.2", @@ -4447,6 +4462,7 @@ "version": "1.0.2", "bundled": true, "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -4466,6 +4482,7 @@ "version": "3.0.1", "bundled": true, "dev": true, + "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -4509,12 +4526,14 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "yallist": { "version": "3.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true } } }, diff --git a/test/contributionreward.js b/test/contributionreward.js index 649a437a..e0b9027f 100644 --- a/test/contributionreward.js +++ b/test/contributionreward.js @@ -91,14 +91,12 @@ const setup = async function (accounts,genesisProtocol = false,tokenAddress=0) { tokenAddress, testSetup.org.avatar.address); var permissions = "0x00000000"; - - var bytesConcate = await registration.daoFactory.bytesConcat(testSetup.contributionRewardParams.initdata,"0x"); - + var tx = await registration.daoFactory.setSchemes( testSetup.org.avatar.address, [web3.utils.fromAscii("ContributionReward")], - bytesConcate[0], - [bytesConcate[1]], + testSetup.contributionRewardParams.initdata, + [helpers.getBytesLength(testSetup.contributionRewardParams.initdata)], [permissions], "metaData",{from:testSetup.proxyAdmin}); diff --git a/test/daofactory.js b/test/daofactory.js index 4f1f4609..7e051641 100644 --- a/test/daofactory.js +++ b/test/daofactory.js @@ -84,13 +84,11 @@ contract('DaoFactory', function(accounts) { .methods .initialize(avatar.address,2) .encodeABI(); - var bytesConcate = await registration.daoFactory.bytesConcat(schemeMockData1,schemeMockData2); - var tx = await registration.daoFactory.setSchemes( avatar.address, [web3.utils.fromAscii("SchemeMock"),web3.utils.fromAscii("SchemeMock")], - bytesConcate[0], - [bytesConcate[1],bytesConcate[2]], + helpers.concatBytes(schemeMockData1,schemeMockData2), + [helpers.getBytesLength(schemeMockData1), helpers.getBytesLength(schemeMockData2)], ["0x0000000F","0x0000000F"], "metaData"); assert.equal(tx.logs.length, 5); @@ -116,14 +114,13 @@ contract('DaoFactory', function(accounts) { .methods .initialize(avatar.address,2) .encodeABI(); - var bytesConcate = await registration.daoFactory.bytesConcat(schemeMockData1,schemeMockData2); try { await registration.daoFactory.setSchemes( avatar.address, [web3.utils.fromAscii("SchemeMock"),web3.utils.fromAscii("SchemeMock")], - bytesConcate[0], - [bytesConcate[1],bytesConcate[2]], + helpers.concatBytes(schemeMockData1, schemeMockData2), + [helpers.getBytesLength(schemeMockData1), helpers.getBytesLength(schemeMockData2)], ["0x0000000F","0x0000000F"], "metaData",{from:accounts[1]}); assert(false,"should fail because accounts[1] does not hold the lock"); @@ -141,13 +138,12 @@ contract('DaoFactory', function(accounts) { .methods .initialize(avatar.address,1) .encodeABI(); - var bytesConcate = await registration.daoFactory.bytesConcat(schemeMockData1,"0x"); var tx = await registration.daoFactory.setSchemes( avatar.address, [web3.utils.fromAscii("SchemeMock")], - bytesConcate[0], - [bytesConcate[1]], + schemeMockData1, + [helpers.getBytesLength(schemeMockData1)], ["0x0000000F"], "metaData"); controllerAddress = await avatar.owner({from:accounts[1]}); @@ -169,13 +165,12 @@ contract('DaoFactory', function(accounts) { .methods .initialize(avatar.address,1) .encodeABI(); - var bytesConcate = await registration.daoFactory.bytesConcat(schemeMockData1,"0x"); await registration.daoFactory.setSchemes( avatar.address, [web3.utils.fromAscii("SchemeMock")], - bytesConcate[0], - [bytesConcate[1]], + schemeMockData1, + [helpers.getBytesLength(schemeMockData1)], ["0x0000000F"], "metaData"); isSchemeRegistered = await controller.isSchemeRegistered(registration.daoFactory.address,{from:accounts[1]}); @@ -189,13 +184,12 @@ contract('DaoFactory', function(accounts) { .methods .initialize(avatar.address,1) .encodeABI(); - var bytesConcate = await registration.daoFactory.bytesConcat(schemeMockData1,"0x"); await registration.daoFactory.setSchemes( avatar.address, [web3.utils.fromAscii("SchemeMock")], - bytesConcate[0], - [bytesConcate[1]], + schemeMockData1, + [helpers.getBytesLength(schemeMockData1)], ["0x0000000F"], "metaData"); try { @@ -271,13 +265,12 @@ contract('DaoFactory', function(accounts) { .methods .initialize(avatar.address,1) .encodeABI(); - var bytesConcate = await registration.daoFactory.bytesConcat(schemeMockData1,"0x"); var tx = await registration.daoFactory.setSchemes( avatar.address, [web3.utils.fromAscii("SchemeMock")], - bytesConcate[0], - [bytesConcate[1]], + schemeMockData1, + [helpers.getBytesLength(schemeMockData1)], ["0x0000000F"], "metaData"); assert.equal(tx.logs.length, 3); diff --git a/test/helpers.js b/test/helpers.js index d6b5c218..c7a4e379 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -355,3 +355,11 @@ export const increaseTime = async function(duration) { }); }); }; + +export const concatBytes = function (bytes1, bytes2) { + return bytes1 + (bytes2.slice(2)); +}; + +export const getBytesLength = function (bytes) { + return web3.utils.toBN(Number(bytes.slice(2).length) / 2); +}; From f4c321e801b4d21d4791c5b73bee3c25dfc8f8ae Mon Sep 17 00:00:00 2001 From: Ben Date: Thu, 14 Nov 2019 17:12:51 +0200 Subject: [PATCH 2/6] Fix --- contracts/utils/DAOFactory.sol | 2 +- test/daofactory.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/utils/DAOFactory.sol b/contracts/utils/DAOFactory.sol index 581a6d07..b1b77274 100644 --- a/contracts/utils/DAOFactory.sol +++ b/contracts/utils/DAOFactory.sol @@ -137,7 +137,7 @@ contract DAOFactory is Initializable { _schemesInitilizeDataLens, _permissions, _metaData); - } + } /** * @dev Creates a new proxy for the given contract and forwards a function call to it. diff --git a/test/daofactory.js b/test/daofactory.js index 7e051641..521f3b68 100644 --- a/test/daofactory.js +++ b/test/daofactory.js @@ -196,8 +196,8 @@ contract('DaoFactory', function(accounts) { await registration.daoFactory.setSchemes( avatar.address, [web3.utils.fromAscii("SchemeMock")], - bytesConcate[0], - [bytesConcate[1]], + schemeMockData1, + [helpers.getBytesLength(schemeMockData1)], ["0x0000000F"], "metaData"); assert(false,"should fail because lock for account[0] suppose to be deleted by the first call"); From 013dc8f81151655b9785ee762b2101c6cdc9ed1e Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Sun, 17 Nov 2019 11:39:01 +0200 Subject: [PATCH 3/6] test fail. --- test/daofactory.js | 17 +++++++++++++---- test/helpers.js | 3 +++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/test/daofactory.js b/test/daofactory.js index 521f3b68..4b357150 100644 --- a/test/daofactory.js +++ b/test/daofactory.js @@ -80,16 +80,24 @@ contract('DaoFactory', function(accounts) { .methods .initialize(avatar.address,1) .encodeABI(); + var schemeMockData2 = await new web3.eth.Contract(registration.schemeMock.abi) .methods .initialize(avatar.address,2) .encodeABI(); + var walletData = await new web3.eth.Contract(registration.wallet.abi) + .methods + .initialize(avatar.address) + .encodeABI(); + var tx = await registration.daoFactory.setSchemes( avatar.address, - [web3.utils.fromAscii("SchemeMock"),web3.utils.fromAscii("SchemeMock")], - helpers.concatBytes(schemeMockData1,schemeMockData2), - [helpers.getBytesLength(schemeMockData1), helpers.getBytesLength(schemeMockData2)], - ["0x0000000F","0x0000000F"], + [web3.utils.fromAscii("Wallet"), + web3.utils.fromAscii("SchemeMock"), + web3.utils.fromAscii("SchemeMock")], + helpers.concatBytes(helpers.concatBytes(walletData,schemeMockData1),schemeMockData2), + [helpers.getBytesLength(walletData), helpers.getBytesLength(schemeMockData1),helpers.getBytesLength(schemeMockData2)], + ["0x0000000F","0x0000000F","0x0000000F"], "metaData"); assert.equal(tx.logs.length, 5); assert.equal(tx.logs[4].event, "InitialSchemesSet"); @@ -100,6 +108,7 @@ contract('DaoFactory', function(accounts) { var scheme2Instance = new SchemeMock(tx.logs[3].args._scheme); assert.equal(await scheme1Instance.testData({from:accounts[1]}), 1); assert.equal(await scheme2Instance.testData({from:accounts[1]}), 2); + assert.equal(await walletInstance.owner({from:accounts[1]}), avatar.address); }); diff --git a/test/helpers.js b/test/helpers.js index c7a4e379..7ebf33f4 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -10,6 +10,7 @@ const constants = require('./constants'); const GenesisProtocol = artifacts.require("./GenesisProtocol.sol"); const DAOFactory = artifacts.require("./DAOFactory.sol"); const SchemeMock = artifacts.require('./test/SchemeMock.sol'); +const Wallet = artifacts.require('./test/Wallet.sol'); const DAOTracker = artifacts.require("./DAOTracker.sol"); const App = artifacts.require("./App.sol"); const Package = artifacts.require("./Package.sol"); @@ -143,12 +144,14 @@ export const registrationAddVersionToPackege = async function (registration,vers registration.avatar = await Avatar.new(); registration.controller = await Controller.new(); registration.schemeMock = await SchemeMock.new(); + registration.wallet = await Wallet.new(); registration.contributionReward = await ContributionReward.new(); await implementationDirectory.setImplementation("DAOToken",registration.daoToken.address); await implementationDirectory.setImplementation("Reputation",registration.reputation.address); await implementationDirectory.setImplementation("Avatar",registration.avatar.address); await implementationDirectory.setImplementation("Controller",registration.controller.address); await implementationDirectory.setImplementation("SchemeMock",registration.schemeMock.address); + await implementationDirectory.setImplementation("Wallet",registration.wallet.address); await implementationDirectory.setImplementation("ContributionReward",registration.contributionReward.address); return registration; }; From 381b222f715f5406894cffc88531f0576a4f8061 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Wed, 20 Nov 2019 21:06:03 +0200 Subject: [PATCH 4/6] fix bug in setSchemes --- contracts/utils/DAOFactory.sol | 4 +++- test/daofactory.js | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/contracts/utils/DAOFactory.sol b/contracts/utils/DAOFactory.sol index b1b77274..e8bd4fe6 100644 --- a/contracts/utils/DAOFactory.sol +++ b/contracts/utils/DAOFactory.sol @@ -8,10 +8,12 @@ import "@openzeppelin/upgrades/contracts/upgradeability/AdminUpgradeabilityProxy import "solidity-bytes-utils/contracts/BytesLib.sol"; import "../controller/Controller.sol"; import "../utils/DAOTracker.sol"; +import "@openzeppelin/contracts-ethereum-package/contracts/math/SafeMath.sol"; contract DAOFactory is Initializable { using BytesLib for bytes; + using SafeMath for uint256; event NewOrg (address indexed _avatar); event InitialSchemesSet (address indexed _avatar); @@ -200,7 +202,7 @@ contract DAOFactory is Initializable { _schemesData.slice(startIndex, _schemesInitilizeDataLens[i]))); emit SchemeInstance(scheme, bytes32ToStr(_schemesNames[i])); controller.registerScheme(scheme, _permissions[i]); - startIndex = _schemesInitilizeDataLens[i]; + startIndex = startIndex.add(_schemesInitilizeDataLens[i]); } controller.metaData(_metaData); // Unregister self: diff --git a/test/daofactory.js b/test/daofactory.js index 4b357150..37158855 100644 --- a/test/daofactory.js +++ b/test/daofactory.js @@ -5,7 +5,7 @@ const Reputation = artifacts.require("./Reputation.sol"); const Avatar = artifacts.require("./Avatar.sol"); const Controller = artifacts.require("./Controller.sol"); const SchemeMock = artifacts.require('./test/SchemeMock.sol'); - +const Wallet = artifacts.require('./test/Wallet.sol'); var avatar; var daoToken; @@ -73,7 +73,7 @@ contract('DaoFactory', function(accounts) { assert.equal(controllerReputationAddress,reputationAddress); }); - it("setSchemes", async function() { + it("setSchemes", async function() { var amountToMint = 10; await setup(accounts,amountToMint,amountToMint); var schemeMockData1 = await new web3.eth.Contract(registration.schemeMock.abi) @@ -112,6 +112,7 @@ contract('DaoFactory', function(accounts) { }); + it("setSchemes from account that does not hold the lock", async function() { var amountToMint = 10; await setup(accounts,amountToMint,amountToMint); From f49ca83fb28f23a50fc6182496456357b1163e8d Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Wed, 20 Nov 2019 22:03:29 +0200 Subject: [PATCH 5/6] update package-lock.json --- package-lock.json | 47 ++++++++++++++--------------------------------- 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/package-lock.json b/package-lock.json index 584c69c5..2d59b4ab 100644 --- a/package-lock.json +++ b/package-lock.json @@ -368,9 +368,9 @@ } }, "@types/node": { - "version": "12.12.9", - "resolved": "https://registry.npmjs.org/@types/node/-/node-12.12.9.tgz", - "integrity": "sha512-kV3w4KeLsRBW+O2rKhktBwENNJuqAUQHS3kf4ia2wIaF/MN6U7ANgTsx7tGremcA0Pk3Yh0Hl0iKiLPuBdIgmw==" + "version": "12.12.11", + "resolved": "https://registry.npmjs.org/@types/node/-/node-12.12.11.tgz", + "integrity": "sha512-O+x6uIpa6oMNTkPuHDa9MhMMehlxLAd5QcOvKRjAFsBVpeFWTOPnXbDvILvFgFFZfQ1xh1EZi1FbXxUix+zpsQ==" }, "@types/web3": { "version": "1.2.2", @@ -4009,8 +4009,7 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "aproba": { "version": "1.2.0", @@ -4031,14 +4030,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -4053,20 +4050,17 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -4183,8 +4177,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -4196,7 +4189,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4211,7 +4203,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -4219,14 +4210,12 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.3.5", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -4245,7 +4234,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -4326,8 +4314,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -4339,7 +4326,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -4425,8 +4411,7 @@ "safe-buffer": { "version": "5.1.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "safer-buffer": { "version": "2.1.2", @@ -4462,7 +4447,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -4482,7 +4466,6 @@ "version": "3.0.1", "bundled": true, "dev": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -4526,14 +4509,12 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "yallist": { "version": "3.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true } } }, From df27c745bf47473783649cca8eb1ff5a073a9a2d Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Wed, 20 Nov 2019 22:18:36 +0200 Subject: [PATCH 6/6] update test --- test/daofactory.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/daofactory.js b/test/daofactory.js index 37158855..9d6cda17 100644 --- a/test/daofactory.js +++ b/test/daofactory.js @@ -99,17 +99,16 @@ contract('DaoFactory', function(accounts) { [helpers.getBytesLength(walletData), helpers.getBytesLength(schemeMockData1),helpers.getBytesLength(schemeMockData2)], ["0x0000000F","0x0000000F","0x0000000F"], "metaData"); - assert.equal(tx.logs.length, 5); - assert.equal(tx.logs[4].event, "InitialSchemesSet"); - assert.equal(tx.logs[4].args._avatar, avatar.address); - + assert.equal(tx.logs.length, 7); + assert.equal(tx.logs[6].event, "InitialSchemesSet"); + assert.equal(tx.logs[6].args._avatar, avatar.address); assert.equal(tx.logs[1].event, "SchemeInstance"); - var scheme1Instance = new SchemeMock(tx.logs[1].args._scheme); - var scheme2Instance = new SchemeMock(tx.logs[3].args._scheme); + var walletInstance = new Wallet(tx.logs[1].args._scheme); + var scheme1Instance = new SchemeMock(tx.logs[3].args._scheme); + var scheme2Instance = new SchemeMock(tx.logs[5].args._scheme); assert.equal(await scheme1Instance.testData({from:accounts[1]}), 1); assert.equal(await scheme2Instance.testData({from:accounts[1]}), 2); assert.equal(await walletInstance.owner({from:accounts[1]}), avatar.address); - });