From 2a876977fd5cd9163fa323475b8ffae726dbb8be Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Wed, 7 Oct 2020 12:45:40 +0300 Subject: [PATCH 01/28] simplify contract --- contracts/schemes/GenericSchemeMultiCall.sol | 66 ++--- test/genericschememulticall.js | 267 ++----------------- 2 files changed, 40 insertions(+), 293 deletions(-) diff --git a/contracts/schemes/GenericSchemeMultiCall.sol b/contracts/schemes/GenericSchemeMultiCall.sol index fdde48fc..75b6906f 100644 --- a/contracts/schemes/GenericSchemeMultiCall.sol +++ b/contracts/schemes/GenericSchemeMultiCall.sol @@ -24,12 +24,11 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf } mapping(bytes32=>MultiCallProposal) public proposals; - IntVoteInterface public votingMachine; bytes32 public voteParams; - mapping(address=>bool) internal contractWhitelist; - address[] public whitelistedContracts; + mapping(address=>bool) public contractsWhitelist; Avatar public avatar; + bytes4 private constant APPROVE_SIGNATURE = 0x095ea7b3;//approve(address,uint256) event NewMultiCallProposal( address indexed _avatar, @@ -61,37 +60,34 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf event ProposalDeleted(address indexed _avatar, bytes32 indexed _proposalId); - /** - * @dev initialize + event WhiteListedContracts(address indexed _avatar, address[] _contractsWhitelist); + + /* @dev initialize * @param _avatar the avatar to mint reputation from * @param _votingMachine the voting machines address to * @param _voteParams voting machine parameters. - * @param _contractWhitelist the contracts the scheme is allowed to interact with + * @param _contractsWhitelist the contracts the scheme is allowed to interact with * */ function initialize( Avatar _avatar, IntVoteInterface _votingMachine, bytes32 _voteParams, - address[] calldata _contractWhitelist + address[] calldata _contractsWhitelist ) external { require(avatar == Avatar(0), "can be called only one time"); require(_avatar != Avatar(0), "avatar cannot be zero"); - require(_contractWhitelist.length > 0, "contractWhitelist cannot be empty"); + require(_contractsWhitelist.length > 0, "contractsWhitelist cannot be empty"); avatar = _avatar; votingMachine = _votingMachine; voteParams = _voteParams; - /* Whitelist controller by default*/ - Controller controller = Controller(_avatar.owner()); - whitelistedContracts.push(address(controller)); - contractWhitelist[address(controller)] = true; - - for (uint i = 0; i < _contractWhitelist.length; i++) { - contractWhitelist[_contractWhitelist[i]] = true; - whitelistedContracts.push(_contractWhitelist[i]); + + for (uint i = 0; i < _contractsWhitelist.length; i++) { + contractsWhitelist[_contractsWhitelist[i]] = true; } + emit WhiteListedContracts(address(avatar), _contractsWhitelist); } /** @@ -130,25 +126,11 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf proposal.exist = false; bytes memory genericCallReturnValue; bool success; - Controller controller = Controller(whitelistedContracts[0]); - + Controller controller = Controller(avatar.owner()); for (uint i = 0; i < proposal.contractsToCall.length; i++) { bytes memory callData = proposal.callsData[i]; - if (proposal.contractsToCall[i] == address(controller)) { - (IERC20 extToken, - address spender, - uint256 valueToSpend - ) = - abi.decode( - callData, - (IERC20, address, uint256) - ); - success = controller.externalTokenApproval(extToken, spender, valueToSpend, avatar); - } else { - (success, genericCallReturnValue) = - controller.genericCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]); - } - + (success, genericCallReturnValue) = + controller.genericCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]); /* Whole transaction will be reverted if at least one call fails*/ require(success, "Proposal call failed"); emit ProposalCallExecuted( @@ -191,17 +173,13 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf "Wrong length of _contractsToCall, _callsDataLens or _values arrays" ); for (uint i = 0; i < _contractsToCall.length; i++) { - require( - contractWhitelist[_contractsToCall[i]], "contractToCall is not whitelisted" - ); - if (_contractsToCall[i] == whitelistedContracts[0]) { - - (, address spender,) = - abi.decode( - _callsData[i], - (IERC20, address, uint256) - ); - require(contractWhitelist[spender], "spender contract not whitelisted"); + if (!contractsWhitelist[_contractsToCall[i]]) { + require( + _callsData[i][0] == APPROVE_SIGNATURE[0] && + _callsData[i][1] == APPROVE_SIGNATURE[1] && + _callsData[i][2] == APPROVE_SIGNATURE[2] && + _callsData[i][3] == APPROVE_SIGNATURE[3], + "allow only approve call for none whitelistedContracts"); } } diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index f60d1150..86011aaf 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -68,257 +68,23 @@ const createCallToActionMock = async function(_avatar,_actionMock) { return await new web3.eth.Contract(_actionMock.abi).methods.test2(_avatar).encodeABI(); }; +const createCallToTokenApproval = async function(_token,_spender,_amount) { + return await new web3.eth.Contract(_token.abi).methods.approve(_spender,_amount).encodeABI(); +}; + contract('GenericSchemeMultiCall', function(accounts) { before(function() { helpers.etherForEveryone(accounts); }); - it("proposeCall log", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[actionMock.address]); - var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address],[callData],[10],"description"); - assert.equal(tx.logs.length, 1); - assert.equal(tx.logs[0].event, "NewMultiCallProposal"); - assert.equal(tx.logs[0].args._callsData[0],callData); - assert.equal(tx.logs[0].args._contractsToCall[0],actionMock.address); - assert.equal(tx.logs[0].args._values[0],10); - assert.equal(tx.logs[0].args._descriptionHash,"description"); - }); - - it("proposeCall log - with invalid array - reverts", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[actionMock.address]); - var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - try { - await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address,actionMock.address],[callData],[0],helpers.NULL_HASH); - assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); - } catch(error) { - helpers.assertVMException(error); - } - try { - await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address,actionMock.address],[callData],[0],helpers.NULL_HASH); - assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); - } catch(error) { - helpers.assertVMException(error); - } - try { - await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address,actionMock.address],[callData],[0,0],helpers.NULL_HASH); - assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); - } catch(error) { - helpers.assertVMException(error); - } - }); - - it("execute proposeCall -no decision - proposal data delete", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[actionMock.address]); - var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address],[callData],[0],helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,0,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - //check organizationsProposals after execution - var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); - assert.equal(proposal.passed,false); - assert.equal(proposal.callData,null); - }); - - it("execute proposeVote -positive decision - proposal data delete", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[actionMock.address]); - var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address],[callData],[0],helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); - await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - //check organizationsProposals after execution - proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); - assert.equal(proposal.callData,null);//new contract address - }); - - it("execute proposeVote -positive decision - destination reverts", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[actionMock.address]); - var callData = await createCallToActionMock(helpers.NULL_ADDRESS,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address],[callData],[0],helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - //actionMock revert because msg.sender is not the _addr param at actionMock though the whole proposal execution will fail. - await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - try { - await testSetup.GenericSchemeMultiCall.execute(proposalId); - assert(false, "Proposal call failed"); - } catch(error) { - helpers.assertVMException(error); - } - }); - - it("execute proposeVote -positive decision - not whitelisted contract", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[accounts[1]]); - var callData = await createCallToActionMock(helpers.NULL_ADDRESS,actionMock); - try { - await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address],[callData],[0],helpers.NULL_HASH); - assert(false, "contractToCall is not whitelisted"); - } catch(error) { - helpers.assertVMException(error); - } - }); - - it("execute proposeVote without return value-positive decision - check action", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[actionMock.address]); - const encodeABI = await new web3.eth.Contract(actionMock.abi).methods.withoutReturnValue(testSetup.org.avatar.address).encodeABI(); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - }); - - it("execute should fail if not executed from votingMachine", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[actionMock.address]); - const encodeABI = await new web3.eth.Contract(actionMock.abi).methods.withoutReturnValue(testSetup.org.avatar.address).encodeABI(); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - try { - await testSetup.GenericSchemeMultiCall.execute( proposalId); - assert(false, "execute should fail if not executed from votingMachine"); - } catch(error) { - helpers.assertVMException(error); - } - - }); - - it("execute proposeVote -positive decision - check action - with GenesisProtocol", async function() { - var actionMock =await ActionMock.new(); - var standardTokenMock = await ERC20Mock.new(accounts[0],1000); - var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); - var value = 123; - var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[value],helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - //transfer some eth to avatar - await web3.eth.sendTransaction({from:accounts[0],to:testSetup.org.avatar.address, value: web3.utils.toWei('1', "ether")}); - assert.equal(await web3.eth.getBalance(actionMock.address),0); - await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - tx = await testSetup.GenericSchemeMultiCall.execute(proposalId); - await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecuted', { - fromBlock: tx.blockNumber, - toBlock: 'latest' - }) - .then(function(events){ - assert.equal(events[0].event,"ProposalExecuted"); - assert.equal(events[0].args._proposalId,proposalId); - }); - assert.equal(await web3.eth.getBalance(actionMock.address),value); - }); - - it("execute proposeVote -negative decision - check action - with GenesisProtocol", async function() { - var actionMock =await ActionMock.new(); - var standardTokenMock = await ERC20Mock.new(accounts[0],1000); - var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); - - var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[0],helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - tx = await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,2,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { - fromBlock: tx.blockNumber, - toBlock: 'latest' - }) - .then(function(events){ - assert.equal(events[0].event,"ProposalExecutedByVotingMachine"); - assert.equal(events[0].args._param,2); - }); - }); - - it("execute proposeVote with multiple calls -positive decision - check action - with GenesisProtocol", async function() { - var actionMock =await ActionMock.new(); - var actionMock2 =await ActionMock.new(); - var standardTokenMock = await ERC20Mock.new(accounts[0],1000); - var testSetup = await setup(accounts,[actionMock.address,actionMock2.address],0,true,standardTokenMock.address); - - var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var callData2 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address,actionMock2.address], - [callData1,callData2], - [0,0], - helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - tx = await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { - fromBlock: tx.blockNumber, - toBlock: 'latest' - }) - .then(function(events){ - assert.equal(events[0].event,"ProposalExecutedByVotingMachine"); - assert.equal(events[0].args._param,1); - }); - }); - - it("execute proposeVote with multiple calls -positive decision - one failed transaction", async function() { - var actionMock =await ActionMock.new(); - var actionMock2 =await ActionMock.new(); - var standardTokenMock = await ERC20Mock.new(accounts[0],1000); - var testSetup = await setup(accounts,[actionMock.address,actionMock2.address],0,true,standardTokenMock.address); - var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var callData2 = await createCallToActionMock(accounts[0],actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address,actionMock2.address], - [callData1,callData2], - [0,0], - helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); - assert.equal(proposal.exist,true); - assert.equal(proposal.passed,false); - await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - try { - await testSetup.GenericSchemeMultiCall.execute(proposalId); - assert(false, "Proposal call failed"); - } catch(error) { - helpers.assertVMException(error); - } - }); - - it("execute proposeVote with multiple calls with votingMachine without whitelisted spender -negative decision", async function() { - var actionMock =await ActionMock.new(); - var standardTokenMock = await ERC20Mock.new(accounts[0],1000); - var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); - var avatarInst = await new web3.eth.Contract(testSetup.org.avatar.abi,testSetup.org.avatar.address); - var controllerAddr = await avatarInst.methods.owner().call(); - var encodedTokenApproval= await web3.eth.abi.encodeParameters(['address','address', 'uint256'], [standardTokenMock.address, accounts[3], 1000]); - var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - try { - await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address,controllerAddr], - [callData1,encodedTokenApproval], - [0,0], - helpers.NULL_HASH); - assert(false, "spender contract not whitelisted"); - } catch(error) { - helpers.assertVMException(error); - } - }); - it("execute proposeVote with multiple calls with votingMachine -positive decision", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); var testSetup = await setup(accounts,[actionMock.address,accounts[3]],0,true,standardTokenMock.address); - var avatarInst = await new web3.eth.Contract(testSetup.org.avatar.abi,testSetup.org.avatar.address); - var controllerAddr = await avatarInst.methods.owner().call(); - var encodedTokenApproval= await web3.eth.abi.encodeParameters(['address','address', 'uint256'], [standardTokenMock.address, accounts[3], 1000]); + var encodedTokenApproval = await createCallToTokenApproval(standardTokenMock,accounts[3], 1000); var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address,controllerAddr], + [actionMock.address,standardTokenMock.address], [callData1,encodedTokenApproval], [0,0], helpers.NULL_HASH); @@ -378,16 +144,19 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); var genericSchemeMultiCall =await GenericSchemeMultiCall.new(); - await genericSchemeMultiCall.initialize( - testSetup.org.avatar.address, - accounts[0], - helpers.SOME_HASH, - [accounts[0],accounts[1],accounts[2],accounts[3]] + var tx = await genericSchemeMultiCall.initialize( + testSetup.org.avatar.address, + accounts[0], + helpers.SOME_HASH, + [accounts[0],accounts[1],accounts[2],accounts[3]] ); - assert.equal(await genericSchemeMultiCall.whitelistedContracts(1),accounts[0]); - assert.equal(await genericSchemeMultiCall.whitelistedContracts(2),accounts[1]); - assert.equal(await genericSchemeMultiCall.whitelistedContracts(3),accounts[2]); - assert.equal(await genericSchemeMultiCall.whitelistedContracts(4),accounts[3]); + assert.equal(tx.logs.length,1); + assert.equal(tx.logs[0].event,"WhiteListedContracts"); + assert.equal(tx.logs[0].args._contractsWhitelist[0],accounts[0]); + assert.equal(tx.logs[0].args._contractsWhitelist[1],accounts[1]); + assert.equal(tx.logs[0].args._contractsWhitelist[2],accounts[2]); + assert.equal(tx.logs[0].args._contractsWhitelist[3],accounts[3]); + }); }); From 685675e19d722e1a5ec37e26b1b6ba6c833345b8 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Wed, 7 Oct 2020 12:49:31 +0300 Subject: [PATCH 02/28] tests --- test/genericschememulticall.js | 217 +++++++++++++++++++++++++++++++++ 1 file changed, 217 insertions(+) diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 86011aaf..3b971312 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -77,6 +77,223 @@ contract('GenericSchemeMultiCall', function(accounts) { helpers.etherForEveryone(accounts); }); + it("proposeCall log", async function() { + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[actionMock.address]); + var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address],[callData],[10],"description"); + assert.equal(tx.logs.length, 1); + assert.equal(tx.logs[0].event, "NewMultiCallProposal"); + assert.equal(tx.logs[0].args._callsData[0],callData); + assert.equal(tx.logs[0].args._contractsToCall[0],actionMock.address); + assert.equal(tx.logs[0].args._values[0],10); + assert.equal(tx.logs[0].args._descriptionHash,"description"); + }); + + it("proposeCall log - with invalid array - reverts", async function() { + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[actionMock.address]); + var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + try { + await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address,actionMock.address],[callData],[0],helpers.NULL_HASH); + assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); + } catch(error) { + helpers.assertVMException(error); + } + try { + await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address,actionMock.address],[callData],[0],helpers.NULL_HASH); + assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); + } catch(error) { + helpers.assertVMException(error); + } + try { + await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address,actionMock.address],[callData],[0,0],helpers.NULL_HASH); + assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); + } catch(error) { + helpers.assertVMException(error); + } + }); + + it("execute proposeCall -no decision - proposal data delete", async function() { + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[actionMock.address]); + var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address],[callData],[0],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,0,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + //check organizationsProposals after execution + var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); + assert.equal(proposal.passed,false); + assert.equal(proposal.callData,null); + }); + + it("execute proposeVote -positive decision - proposal data delete", async function() { + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[actionMock.address]); + var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address],[callData],[0],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); + await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + //check organizationsProposals after execution + proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); + assert.equal(proposal.callData,null);//new contract address + }); + + it("execute proposeVote -positive decision - destination reverts", async function() { + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[actionMock.address]); + var callData = await createCallToActionMock(helpers.NULL_ADDRESS,actionMock); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address],[callData],[0],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + //actionMock revert because msg.sender is not the _addr param at actionMock though the whole proposal execution will fail. + await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + try { + await testSetup.GenericSchemeMultiCall.execute(proposalId); + assert(false, "Proposal call failed"); + } catch(error) { + helpers.assertVMException(error); + } + }); + + it("execute proposeVote -positive decision - not whitelisted contract", async function() { + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[accounts[1]]); + var callData = await createCallToActionMock(helpers.NULL_ADDRESS,actionMock); + try { + await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address],[callData],[0],helpers.NULL_HASH); + assert(false, "contractToCall is not whitelisted"); + } catch(error) { + helpers.assertVMException(error); + } + }); + + it("execute proposeVote without return value-positive decision - check action", async function() { + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[actionMock.address]); + const encodeABI = await new web3.eth.Contract(actionMock.abi).methods.withoutReturnValue(testSetup.org.avatar.address).encodeABI(); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + }); + + it("execute should fail if not executed from votingMachine", async function() { + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[actionMock.address]); + const encodeABI = await new web3.eth.Contract(actionMock.abi).methods.withoutReturnValue(testSetup.org.avatar.address).encodeABI(); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + try { + await testSetup.GenericSchemeMultiCall.execute( proposalId); + assert(false, "execute should fail if not executed from votingMachine"); + } catch(error) { + helpers.assertVMException(error); + } + + }); + + it("execute proposeVote -positive decision - check action - with GenesisProtocol", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); + var value = 123; + var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[value],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + //transfer some eth to avatar + await web3.eth.sendTransaction({from:accounts[0],to:testSetup.org.avatar.address, value: web3.utils.toWei('1', "ether")}); + assert.equal(await web3.eth.getBalance(actionMock.address),0); + await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + tx = await testSetup.GenericSchemeMultiCall.execute(proposalId); + await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecuted', { + fromBlock: tx.blockNumber, + toBlock: 'latest' + }) + .then(function(events){ + assert.equal(events[0].event,"ProposalExecuted"); + assert.equal(events[0].args._proposalId,proposalId); + }); + assert.equal(await web3.eth.getBalance(actionMock.address),value); + }); + + it("execute proposeVote -negative decision - check action - with GenesisProtocol", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); + + var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[0],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + tx = await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,2,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { + fromBlock: tx.blockNumber, + toBlock: 'latest' + }) + .then(function(events){ + assert.equal(events[0].event,"ProposalExecutedByVotingMachine"); + assert.equal(events[0].args._param,2); + }); + }); + + it("execute proposeVote with multiple calls -positive decision - check action - with GenesisProtocol", async function() { + var actionMock =await ActionMock.new(); + var actionMock2 =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address,actionMock2.address],0,true,standardTokenMock.address); + + var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var callData2 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address,actionMock2.address], + [callData1,callData2], + [0,0], + helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + tx = await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { + fromBlock: tx.blockNumber, + toBlock: 'latest' + }) + .then(function(events){ + assert.equal(events[0].event,"ProposalExecutedByVotingMachine"); + assert.equal(events[0].args._param,1); + }); + }); + + it("execute proposeVote with multiple calls -positive decision - one failed transaction", async function() { + var actionMock =await ActionMock.new(); + var actionMock2 =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address,actionMock2.address],0,true,standardTokenMock.address); + var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var callData2 = await createCallToActionMock(accounts[0],actionMock); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address,actionMock2.address], + [callData1,callData2], + [0,0], + helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); + assert.equal(proposal.exist,true); + assert.equal(proposal.passed,false); + await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + try { + await testSetup.GenericSchemeMultiCall.execute(proposalId); + assert(false, "Proposal call failed"); + } catch(error) { + helpers.assertVMException(error); + } + }); + + it("execute proposeVote with multiple calls with votingMachine -positive decision", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); From 6b91201ec1954490b4da180fca9b4d45a9a83572 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Wed, 7 Oct 2020 12:54:31 +0300 Subject: [PATCH 03/28] test approval --- test/genericschememulticall.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 3b971312..4bcda461 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -309,6 +309,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); assert.equal(proposal.exist,true); assert.equal(proposal.passed,false); + assert.equal(await standardTokenMock.allowance(testSetup.org.avatar.address,accounts[3]),0); await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); await testSetup.GenericSchemeMultiCall.execute(proposalId); await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalCallExecuted', { @@ -321,6 +322,7 @@ contract('GenericSchemeMultiCall', function(accounts) { assert.equal(events[1].event,"ProposalCallExecuted"); assert.equal(events[1].args._proposalId,proposalId); }); + assert.equal(await standardTokenMock.allowance(testSetup.org.avatar.address,accounts[3]),1000); }); it("cannot init without contract whitelist", async function() { From feae9ee3ff3697eaae4e57bdd6fdd8d31cde4c71 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Wed, 7 Oct 2020 15:07:34 +0300 Subject: [PATCH 04/28] check spender is white listed --- contracts/schemes/GenericSchemeMultiCall.sol | 18 ++++++++++++++---- test/genericschememulticall.js | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/contracts/schemes/GenericSchemeMultiCall.sol b/contracts/schemes/GenericSchemeMultiCall.sol index 75b6906f..5bd44fb0 100644 --- a/contracts/schemes/GenericSchemeMultiCall.sol +++ b/contracts/schemes/GenericSchemeMultiCall.sol @@ -174,12 +174,22 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf ); for (uint i = 0; i < _contractsToCall.length; i++) { if (!contractsWhitelist[_contractsToCall[i]]) { + address spender; + bytes memory callData = _callsData[i]; require( - _callsData[i][0] == APPROVE_SIGNATURE[0] && - _callsData[i][1] == APPROVE_SIGNATURE[1] && - _callsData[i][2] == APPROVE_SIGNATURE[2] && - _callsData[i][3] == APPROVE_SIGNATURE[3], + callData[0] == APPROVE_SIGNATURE[0] && + callData[1] == APPROVE_SIGNATURE[1] && + callData[2] == APPROVE_SIGNATURE[2] && + callData[3] == APPROVE_SIGNATURE[3], "allow only approve call for none whitelistedContracts"); + //in solidity > 6 this can be replaced by: + //(spender,) = abi.decode(callData[4:], (address, uint)); + //see https://github.com/ethereum/solidity/issues/9439 + // solhint-disable-next-line no-inline-assembly + assembly { + spender := mload(add(callData, 36)) + } + require(contractsWhitelist[spender], "spender contract not whitelisted"); } } diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 4bcda461..4a1158c1 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -378,4 +378,22 @@ contract('GenericSchemeMultiCall', function(accounts) { }); + it("execute proposeVote with multiple calls with votingMachine without whitelisted spender", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); + var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); + var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + try { + await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address], + [callData1,encodedTokenApproval], + [0,0], + helpers.NULL_HASH); + assert(false, "spender contract not whitelisted"); + } catch(error) { + helpers.assertVMException(error); + } + }); + }); From 1a6cd03d24445d93cd38056d09cb7e76b4775710 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Wed, 7 Oct 2020 15:12:37 +0300 Subject: [PATCH 05/28] clean --- contracts/schemes/GenericSchemeMultiCall.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/schemes/GenericSchemeMultiCall.sol b/contracts/schemes/GenericSchemeMultiCall.sol index 5bd44fb0..89b99c27 100644 --- a/contracts/schemes/GenericSchemeMultiCall.sol +++ b/contracts/schemes/GenericSchemeMultiCall.sol @@ -184,7 +184,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf "allow only approve call for none whitelistedContracts"); //in solidity > 6 this can be replaced by: //(spender,) = abi.decode(callData[4:], (address, uint)); - //see https://github.com/ethereum/solidity/issues/9439 + // see https://github.com/ethereum/solidity/issues/9439 // solhint-disable-next-line no-inline-assembly assembly { spender := mload(add(callData, 36)) From e06fe114675803e0dd546f161cd8097c04ec4f69 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Thu, 8 Oct 2020 19:07:27 +0300 Subject: [PATCH 06/28] poc --- contracts/schemes/GenericSchemeMultiCall.sol | 8 +++++++- contracts/schemes/SchemeConstraint.sol | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 contracts/schemes/SchemeConstraint.sol diff --git a/contracts/schemes/GenericSchemeMultiCall.sol b/contracts/schemes/GenericSchemeMultiCall.sol index 89b99c27..ebb0fec7 100644 --- a/contracts/schemes/GenericSchemeMultiCall.sol +++ b/contracts/schemes/GenericSchemeMultiCall.sol @@ -4,6 +4,7 @@ pragma experimental ABIEncoderV2; import "@daostack/infra/contracts/votingMachines/IntVoteInterface.sol"; import "@daostack/infra/contracts/votingMachines/ProposalExecuteInterface.sol"; import "../votingMachines/VotingMachineCallbacks.sol"; +import "./SchemeConstraint.sol"; /** @@ -29,6 +30,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf mapping(address=>bool) public contractsWhitelist; Avatar public avatar; bytes4 private constant APPROVE_SIGNATURE = 0x095ea7b3;//approve(address,uint256) + SchemeConstraint public schemeConstraint; event NewMultiCallProposal( address indexed _avatar, @@ -73,7 +75,8 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf Avatar _avatar, IntVoteInterface _votingMachine, bytes32 _voteParams, - address[] calldata _contractsWhitelist + address[] calldata _contractsWhitelist, + SchemeConstraint _schemeConstraint ) external { @@ -83,6 +86,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf avatar = _avatar; votingMachine = _votingMachine; voteParams = _voteParams; + schemeConstraint = _schemeConstraint; for (uint i = 0; i < _contractsWhitelist.length; i++) { contractsWhitelist[_contractsWhitelist[i]] = true; @@ -129,6 +133,8 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf Controller controller = Controller(avatar.owner()); for (uint i = 0; i < proposal.contractsToCall.length; i++) { bytes memory callData = proposal.callsData[i]; + require(schemeConstraint.isAllowedToCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]), + "call is not allowed"); (success, genericCallReturnValue) = controller.genericCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]); /* Whole transaction will be reverted if at least one call fails*/ diff --git a/contracts/schemes/SchemeConstraint.sol b/contracts/schemes/SchemeConstraint.sol new file mode 100644 index 00000000..f69119d4 --- /dev/null +++ b/contracts/schemes/SchemeConstraint.sol @@ -0,0 +1,17 @@ +pragma solidity 0.5.17; +import "../controller/Avatar.sol"; + + +contract SchemeConstraint { + + function isAllowedToCall( + address _contractToCall, + bytes calldata callData, + Avatar _avatar, + uint256 _ethAmount) external returns(bool) { + //do the logic + return true; + + } + +} From 8872446d232f82f02c5a09da09a864bcb440c417 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Thu, 8 Oct 2020 19:26:05 +0300 Subject: [PATCH 07/28] Add SchemeConstraints interface --- ...straint.sol => DxDaoSchemeConstraints.sol} | 5 +++-- contracts/schemes/GenericSchemeMultiCall.sol | 10 ++++----- contracts/schemes/SchemeConstraints.sol | 14 ++++++++++++ test/genericschememulticall.js | 22 +++++++++++++------ 4 files changed, 37 insertions(+), 14 deletions(-) rename contracts/schemes/{SchemeConstraint.sol => DxDaoSchemeConstraints.sol} (76%) create mode 100644 contracts/schemes/SchemeConstraints.sol diff --git a/contracts/schemes/SchemeConstraint.sol b/contracts/schemes/DxDaoSchemeConstraints.sol similarity index 76% rename from contracts/schemes/SchemeConstraint.sol rename to contracts/schemes/DxDaoSchemeConstraints.sol index f69119d4..51d6107a 100644 --- a/contracts/schemes/SchemeConstraint.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -1,9 +1,10 @@ pragma solidity 0.5.17; import "../controller/Avatar.sol"; +import "./SchemeConstraints.sol"; -contract SchemeConstraint { - +contract DxDaoSchemeConstraints is SchemeConstraints { + function isAllowedToCall( address _contractToCall, bytes calldata callData, diff --git a/contracts/schemes/GenericSchemeMultiCall.sol b/contracts/schemes/GenericSchemeMultiCall.sol index ebb0fec7..e78aa223 100644 --- a/contracts/schemes/GenericSchemeMultiCall.sol +++ b/contracts/schemes/GenericSchemeMultiCall.sol @@ -4,7 +4,7 @@ pragma experimental ABIEncoderV2; import "@daostack/infra/contracts/votingMachines/IntVoteInterface.sol"; import "@daostack/infra/contracts/votingMachines/ProposalExecuteInterface.sol"; import "../votingMachines/VotingMachineCallbacks.sol"; -import "./SchemeConstraint.sol"; +import "./SchemeConstraints.sol"; /** @@ -30,7 +30,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf mapping(address=>bool) public contractsWhitelist; Avatar public avatar; bytes4 private constant APPROVE_SIGNATURE = 0x095ea7b3;//approve(address,uint256) - SchemeConstraint public schemeConstraint; + SchemeConstraints public schemeConstraints; event NewMultiCallProposal( address indexed _avatar, @@ -76,7 +76,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf IntVoteInterface _votingMachine, bytes32 _voteParams, address[] calldata _contractsWhitelist, - SchemeConstraint _schemeConstraint + SchemeConstraints _schemeConstraints ) external { @@ -86,7 +86,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf avatar = _avatar; votingMachine = _votingMachine; voteParams = _voteParams; - schemeConstraint = _schemeConstraint; + schemeConstraints = _schemeConstraints; for (uint i = 0; i < _contractsWhitelist.length; i++) { contractsWhitelist[_contractsWhitelist[i]] = true; @@ -133,7 +133,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf Controller controller = Controller(avatar.owner()); for (uint i = 0; i < proposal.contractsToCall.length; i++) { bytes memory callData = proposal.callsData[i]; - require(schemeConstraint.isAllowedToCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]), + require(schemeConstraints.isAllowedToCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]), "call is not allowed"); (success, genericCallReturnValue) = controller.genericCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]); diff --git a/contracts/schemes/SchemeConstraints.sol b/contracts/schemes/SchemeConstraints.sol new file mode 100644 index 00000000..100f932e --- /dev/null +++ b/contracts/schemes/SchemeConstraints.sol @@ -0,0 +1,14 @@ +pragma solidity 0.5.17; + +import "../controller/Avatar.sol"; + + +interface SchemeConstraints { + + function isAllowedToCall( + address _contractToCall, + bytes calldata callData, + Avatar _avatar, + uint256 _ethAmount) external returns(bool); + +} diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 4a1158c1..ed4c6a68 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -6,6 +6,7 @@ const ControllerCreator = artifacts.require("./ControllerCreator.sol"); const DAOTracker = artifacts.require("./DAOTracker.sol"); const ERC20Mock = artifacts.require("./ERC20Mock.sol"); const ActionMock = artifacts.require("./ActionMock.sol"); +const DxDaoSchemeConstraints = artifacts.require("./DxDaoSchemeConstraints.sol"); export class GenericSchemeParams { constructor() { @@ -18,7 +19,8 @@ const setupGenericSchemeParams = async function( contractWhitelist, genesisProtocol = false, tokenAddress = 0, - avatar + avatar, + schemeConstraints ) { var genericSchemeParams = new GenericSchemeParams(); if (genesisProtocol === true){ @@ -27,7 +29,8 @@ const setupGenericSchemeParams = async function( avatar.address, genericSchemeParams.votingMachine.genesisProtocol.address, genericSchemeParams.votingMachine.params, - contractWhitelist); + contractWhitelist, + schemeConstraints.address); } else { genericSchemeParams.votingMachine = await helpers.setupAbsoluteVote(helpers.NULL_ADDRESS,50,genericScheme.address); @@ -35,7 +38,8 @@ const setupGenericSchemeParams = async function( avatar.address, genericSchemeParams.votingMachine.absoluteVote.address, genericSchemeParams.votingMachine.params, - contractWhitelist); + contractWhitelist, + schemeConstraints.address); } return genericSchemeParams; }; @@ -53,7 +57,8 @@ const setup = async function (accounts,contractsWhitelist,reputationAccount=0,ge } else { testSetup.org = await helpers.setupOrganizationWithArrays(testSetup.daoCreator,[accounts[0],accounts[1],reputationAccount],[1000,1000,1000],testSetup.reputationArray); } - testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.GenericSchemeMultiCall,accounts,contractsWhitelist,genesisProtocol,tokenAddress,testSetup.org.avatar); + testSetup.schemeConstraints = await DxDaoSchemeConstraints.new(); + testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.GenericSchemeMultiCall,accounts,contractsWhitelist,genesisProtocol,tokenAddress,testSetup.org.avatar,testSetup.schemeConstraints); var permissions = "0x00000010"; @@ -335,7 +340,8 @@ contract('GenericSchemeMultiCall', function(accounts) { testSetup.org.avatar.address, accounts[0], helpers.SOME_HASH, - [] + [], + testSetup.schemeConstraints.address ); assert(false, "contractWhitelist cannot be empty"); } catch(error) { @@ -351,7 +357,8 @@ contract('GenericSchemeMultiCall', function(accounts) { testSetup.org.avatar.address, accounts[0], helpers.SOME_HASH, - [accounts[0]] + [accounts[0]], + testSetup.schemeConstraints.address ); assert(false, "cannot init twice"); } catch(error) { @@ -367,7 +374,8 @@ contract('GenericSchemeMultiCall', function(accounts) { testSetup.org.avatar.address, accounts[0], helpers.SOME_HASH, - [accounts[0],accounts[1],accounts[2],accounts[3]] + [accounts[0],accounts[1],accounts[2],accounts[3]], + testSetup.schemeConstraints.address ); assert.equal(tx.logs.length,1); assert.equal(tx.logs[0].event,"WhiteListedContracts"); From 9c9b4abc6392265c33575fa2e724c7243c2767d7 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Thu, 8 Oct 2020 19:49:38 +0300 Subject: [PATCH 08/28] add eth constraint as an example for DxDaoSchemeConstraint --- contracts/schemes/DxDaoSchemeConstraints.sol | 40 ++++++++++++++++++-- test/genericschememulticall.js | 3 +- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index 51d6107a..a1c081e2 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -1,18 +1,52 @@ pragma solidity 0.5.17; -import "../controller/Avatar.sol"; + import "./SchemeConstraints.sol"; contract DxDaoSchemeConstraints is SchemeConstraints { + using SafeMath for uint256; + + uint256 public initialTimestamp; + uint256 public periodSize; + uint256 public periodLimitWei; + + uint256 public periodLimitTokens; + mapping(uint256=>uint256) periodSpendingTokens; + + mapping(address=>uint256) periodLimitToken; + mapping (uint256 => mapping(address => uint256)) public periodSpendingToken; + mapping(uint256=>uint256) periodSpendingWei; + + function initialize( + uint256 _periodSize, + uint256 _periodLimitWei + ) + external { + require(initialTimestamp == 0, "cannot initialize twice"); + require(_periodSize > 0 , "preriod size should be greater than 0"); + periodSize = _periodSize; + periodLimitWei = _periodLimitWei; + initialTimestamp = block.timestamp; + } function isAllowedToCall( address _contractToCall, - bytes calldata callData, + bytes calldata _callData, Avatar _avatar, - uint256 _ethAmount) external returns(bool) { + uint256 _ethAmount) + external + returns(bool) + { + + uint256 observervationIndex = observationIndexOf(block.timestamp); + require(periodSpendingWei[observervationIndex].add(_ethAmount) <= periodLimitWei, "periodSpendingWeiExceeded"); //do the logic return true; } + function observationIndexOf(uint256 _timestamp) public view returns (uint256) { + return uint8((_timestamp-initialTimestamp) / periodSize); + } + } diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index ed4c6a68..c97e0950 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -58,6 +58,7 @@ const setup = async function (accounts,contractsWhitelist,reputationAccount=0,ge testSetup.org = await helpers.setupOrganizationWithArrays(testSetup.daoCreator,[accounts[0],accounts[1],reputationAccount],[1000,1000,1000],testSetup.reputationArray); } testSetup.schemeConstraints = await DxDaoSchemeConstraints.new(); + await testSetup.schemeConstraints.initialize(100000,1000000); testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.GenericSchemeMultiCall,accounts,contractsWhitelist,genesisProtocol,tokenAddress,testSetup.org.avatar,testSetup.schemeConstraints); var permissions = "0x00000010"; @@ -95,7 +96,7 @@ contract('GenericSchemeMultiCall', function(accounts) { assert.equal(tx.logs[0].args._values[0],10); assert.equal(tx.logs[0].args._descriptionHash,"description"); }); - + it("proposeCall log - with invalid array - reverts", async function() { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); From e7cb19716180b657adc90799cc04de1a6c2189e9 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Thu, 8 Oct 2020 19:56:46 +0300 Subject: [PATCH 09/28] + --- contracts/schemes/DxDaoSchemeConstraints.sol | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index a1c081e2..f29fcd90 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -39,8 +39,12 @@ contract DxDaoSchemeConstraints is SchemeConstraints { { uint256 observervationIndex = observationIndexOf(block.timestamp); - require(periodSpendingWei[observervationIndex].add(_ethAmount) <= periodLimitWei, "periodSpendingWeiExceeded"); - //do the logic + periodSpendingWei[observervationIndex] = periodSpendingWei[observervationIndex].add(_ethAmount); + require(periodSpendingWei[observervationIndex] <= periodLimitWei, "periodSpendingWeiExceeded"); + //do other logic : + // constraint approve calls + // constraint token transfer + // ... return true; } From 519f7cb4e2edc5391d4decf69714a68d33b8ae9e Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Sun, 11 Oct 2020 11:29:07 +0300 Subject: [PATCH 10/28] more .. --- contracts/schemes/DxDaoSchemeConstraints.sol | 114 +++++++++++++++---- contracts/schemes/GenericSchemeMultiCall.sol | 48 +++----- contracts/schemes/SchemeConstraints.sol | 18 ++- test/genericschememulticall.js | 47 ++------ 4 files changed, 134 insertions(+), 93 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index f29fcd90..896ff6d7 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -1,4 +1,5 @@ pragma solidity 0.5.17; +pragma experimental ABIEncoderV2; import "./SchemeConstraints.sol"; @@ -6,51 +7,126 @@ import "./SchemeConstraints.sol"; contract DxDaoSchemeConstraints is SchemeConstraints { using SafeMath for uint256; + event WhiteListedContracts(address[] _contractsWhitelist); + uint256 public initialTimestamp; uint256 public periodSize; uint256 public periodLimitWei; - uint256 public periodLimitTokens; - mapping(uint256=>uint256) periodSpendingTokens; + mapping(uint256=>uint256) public periodSpendingTokens; - mapping(address=>uint256) periodLimitToken; + mapping(address=>uint256) public periodLimitToken; mapping (uint256 => mapping(address => uint256)) public periodSpendingToken; - mapping(uint256=>uint256) periodSpendingWei; + mapping(uint256=>uint256) public periodSpendingWei; + mapping(address=>bool) public contractsWhitelist; + bytes4 private constant APPROVE_SIGNATURE = 0x095ea7b3;//approve(address,uint256) + /* @dev initialize + * @param _avatar the avatar to mint reputation from + * @param _votingMachine the voting machines address to + * @param _voteParams voting machine parameters. + * @param _contractsWhitelist the contracts the scheme is allowed to interact with + */ function initialize( uint256 _periodSize, - uint256 _periodLimitWei + uint256 _periodLimitWei, + address[] calldata _periodLimitTokensAddresses, + uint256[] calldata _periodLimitTokensAmounts, + address[] calldata _contractsWhitelist ) external { require(initialTimestamp == 0, "cannot initialize twice"); - require(_periodSize > 0 , "preriod size should be greater than 0"); + require(_periodSize > 0, "preriod size should be greater than 0"); + require(_periodLimitTokensAddresses.length == _periodLimitTokensAmounts.length, + "invalid length _periodLimitTokensAddresses"); periodSize = _periodSize; periodLimitWei = _periodLimitWei; + // solhint-disable-next-line not-rely-on-time initialTimestamp = block.timestamp; + for (uint i = 0; i < _contractsWhitelist.length; i++) { + contractsWhitelist[_contractsWhitelist[i]] = true; + } + emit WhiteListedContracts(_contractsWhitelist); + for (uint i = 0; i < _periodLimitTokensAmounts.length; i++) { + periodLimitToken[_periodLimitTokensAddresses[i]] = _periodLimitTokensAmounts[i]; + } } function isAllowedToCall( - address _contractToCall, - bytes calldata _callData, - Avatar _avatar, - uint256 _ethAmount) + address[] calldata _contractsToCall, + bytes[] calldata _callsData, + uint256[] calldata _values, + Avatar + ) external returns(bool) { - uint256 observervationIndex = observationIndexOf(block.timestamp); - periodSpendingWei[observervationIndex] = periodSpendingWei[observervationIndex].add(_ethAmount); - require(periodSpendingWei[observervationIndex] <= periodLimitWei, "periodSpendingWeiExceeded"); - //do other logic : - // constraint approve calls - // constraint token transfer - // ... + uint256 observervationIndex = observationIndex(); + uint256 totalPeriodSpendingInWei = periodSpendingWei[observervationIndex]; + for (uint i = 0; i < _contractsToCall.length; i++) { + // constraint eth transfer + totalPeriodSpendingInWei = totalPeriodSpendingInWei.add(_values[i]); + bytes memory callData = _callsData[i]; + // constraint approve calls + if (callData[0] == APPROVE_SIGNATURE[0] && + callData[1] == APPROVE_SIGNATURE[1] && + callData[2] == APPROVE_SIGNATURE[2] && + callData[3] == APPROVE_SIGNATURE[3]) { + uint256 amount; + address contractToCall = _contractsToCall[i]; + // solhint-disable-next-line no-inline-assembly + assembly { + amount := calldataload(add(callData, 68)) + } + periodSpendingToken[observervationIndex][contractToCall] = + periodSpendingToken[observervationIndex][contractToCall].add(amount); + require( + periodSpendingToken[observervationIndex][contractToCall] <= periodLimitToken[contractToCall], + "periodSpendingTokensExceeded"); + } + periodSpendingWei[observervationIndex] = + periodSpendingWei[observervationIndex].add(totalPeriodSpendingInWei); + require(periodSpendingWei[observervationIndex] <= periodLimitWei, "periodSpendingWeiExceeded"); + } return true; + } + function isAllowedToPropose( + address[] calldata _contractsToCall, + bytes[] calldata _callsData, + uint256[] calldata, + Avatar) + external + returns(bool) + { + for (uint i = 0; i < _contractsToCall.length; i++) { + // constraint approve calls + if (!contractsWhitelist[_contractsToCall[i]]) { + address spender; + bytes memory callData = _callsData[i]; + require( + callData[0] == APPROVE_SIGNATURE[0] && + callData[1] == APPROVE_SIGNATURE[1] && + callData[2] == APPROVE_SIGNATURE[2] && + callData[3] == APPROVE_SIGNATURE[3], + "allow only approve call for none whitelistedContracts"); + //in solidity > 6 this can be replaced by: + //(spender,) = abi.decode(callData[4:], (address, uint)); + // see https://github.com/ethereum/solidity/issues/9439 + // solhint-disable-next-line no-inline-assembly + assembly { + spender := calldataload(add(callData, 36)) + } + require(contractsWhitelist[spender], "spender contract not whitelisted"); + } + } + return true; } - function observationIndexOf(uint256 _timestamp) public view returns (uint256) { - return uint8((_timestamp-initialTimestamp) / periodSize); + function observationIndex() public view returns (uint256) { + // solhint-disable-next-line not-rely-on-time + return uint8((block.timestamp - initialTimestamp) / periodSize); } } diff --git a/contracts/schemes/GenericSchemeMultiCall.sol b/contracts/schemes/GenericSchemeMultiCall.sol index e78aa223..4122ae3a 100644 --- a/contracts/schemes/GenericSchemeMultiCall.sol +++ b/contracts/schemes/GenericSchemeMultiCall.sol @@ -27,9 +27,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf mapping(bytes32=>MultiCallProposal) public proposals; IntVoteInterface public votingMachine; bytes32 public voteParams; - mapping(address=>bool) public contractsWhitelist; Avatar public avatar; - bytes4 private constant APPROVE_SIGNATURE = 0x095ea7b3;//approve(address,uint256) SchemeConstraints public schemeConstraints; event NewMultiCallProposal( @@ -62,8 +60,6 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf event ProposalDeleted(address indexed _avatar, bytes32 indexed _proposalId); - event WhiteListedContracts(address indexed _avatar, address[] _contractsWhitelist); - /* @dev initialize * @param _avatar the avatar to mint reputation from * @param _votingMachine the voting machines address to @@ -75,23 +71,16 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf Avatar _avatar, IntVoteInterface _votingMachine, bytes32 _voteParams, - address[] calldata _contractsWhitelist, SchemeConstraints _schemeConstraints ) external { require(avatar == Avatar(0), "can be called only one time"); require(_avatar != Avatar(0), "avatar cannot be zero"); - require(_contractsWhitelist.length > 0, "contractsWhitelist cannot be empty"); avatar = _avatar; votingMachine = _votingMachine; voteParams = _voteParams; schemeConstraints = _schemeConstraints; - - for (uint i = 0; i < _contractsWhitelist.length; i++) { - contractsWhitelist[_contractsWhitelist[i]] = true; - } - emit WhiteListedContracts(address(avatar), _contractsWhitelist); } /** @@ -127,14 +116,19 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf MultiCallProposal storage proposal = proposals[_proposalId]; require(proposal.exist, "must be a live proposal"); require(proposal.passed, "proposal must passed by voting machine"); + require( + schemeConstraints.isAllowedToCall( + proposal.contractsToCall, + proposal.callsData, + proposal.values, + avatar), + "call is not allowed"); proposal.exist = false; bytes memory genericCallReturnValue; bool success; Controller controller = Controller(avatar.owner()); for (uint i = 0; i < proposal.contractsToCall.length; i++) { bytes memory callData = proposal.callsData[i]; - require(schemeConstraints.isAllowedToCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]), - "call is not allowed"); (success, genericCallReturnValue) = controller.genericCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]); /* Whole transaction will be reverted if at least one call fails*/ @@ -178,26 +172,14 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf (_contractsToCall.length == _callsData.length) && (_contractsToCall.length == _values.length), "Wrong length of _contractsToCall, _callsDataLens or _values arrays" ); - for (uint i = 0; i < _contractsToCall.length; i++) { - if (!contractsWhitelist[_contractsToCall[i]]) { - address spender; - bytes memory callData = _callsData[i]; - require( - callData[0] == APPROVE_SIGNATURE[0] && - callData[1] == APPROVE_SIGNATURE[1] && - callData[2] == APPROVE_SIGNATURE[2] && - callData[3] == APPROVE_SIGNATURE[3], - "allow only approve call for none whitelistedContracts"); - //in solidity > 6 this can be replaced by: - //(spender,) = abi.decode(callData[4:], (address, uint)); - // see https://github.com/ethereum/solidity/issues/9439 - // solhint-disable-next-line no-inline-assembly - assembly { - spender := mload(add(callData, 36)) - } - require(contractsWhitelist[spender], "spender contract not whitelisted"); - } - } + + require( + schemeConstraints.isAllowedToPropose( + _contractsToCall, + _callsData, + _values, + avatar), + "propose is not allowed"); proposalId = votingMachine.propose(2, voteParams, msg.sender, address(avatar)); diff --git a/contracts/schemes/SchemeConstraints.sol b/contracts/schemes/SchemeConstraints.sol index 100f932e..cea098bd 100644 --- a/contracts/schemes/SchemeConstraints.sol +++ b/contracts/schemes/SchemeConstraints.sol @@ -1,14 +1,22 @@ pragma solidity 0.5.17; - +pragma experimental ABIEncoderV2; import "../controller/Avatar.sol"; interface SchemeConstraints { function isAllowedToCall( - address _contractToCall, - bytes calldata callData, - Avatar _avatar, - uint256 _ethAmount) external returns(bool); + address[] calldata _contractsToCall, + bytes[] calldata _callsData, + uint256[] calldata _values, + Avatar _avatar) + external returns(bool); + + function isAllowedToPropose( + address[] calldata _contractsToCall, + bytes[] calldata _callsData, + uint256[] calldata _values, + Avatar _avatar) + external returns(bool); } diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index c97e0950..d2e9dca1 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -16,7 +16,6 @@ export class GenericSchemeParams { const setupGenericSchemeParams = async function( genericScheme, accounts, - contractWhitelist, genesisProtocol = false, tokenAddress = 0, avatar, @@ -29,7 +28,6 @@ const setupGenericSchemeParams = async function( avatar.address, genericSchemeParams.votingMachine.genesisProtocol.address, genericSchemeParams.votingMachine.params, - contractWhitelist, schemeConstraints.address); } else { @@ -38,13 +36,12 @@ const setupGenericSchemeParams = async function( avatar.address, genericSchemeParams.votingMachine.absoluteVote.address, genericSchemeParams.votingMachine.params, - contractWhitelist, schemeConstraints.address); } return genericSchemeParams; }; -const setup = async function (accounts,contractsWhitelist,reputationAccount=0,genesisProtocol = false,tokenAddress=0) { +const setup = async function (accounts,contractsWhitelist,reputationAccount=0,genesisProtocol = false,tokenAddress=helpers.NULL_ADDRESS) { var testSetup = new helpers.TestSetup(); testSetup.standardTokenMock = await ERC20Mock.new(accounts[1],100); testSetup.GenericSchemeMultiCall = await GenericSchemeMultiCall.new(); @@ -58,8 +55,8 @@ const setup = async function (accounts,contractsWhitelist,reputationAccount=0,ge testSetup.org = await helpers.setupOrganizationWithArrays(testSetup.daoCreator,[accounts[0],accounts[1],reputationAccount],[1000,1000,1000],testSetup.reputationArray); } testSetup.schemeConstraints = await DxDaoSchemeConstraints.new(); - await testSetup.schemeConstraints.initialize(100000,1000000); - testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.GenericSchemeMultiCall,accounts,contractsWhitelist,genesisProtocol,tokenAddress,testSetup.org.avatar,testSetup.schemeConstraints); + await testSetup.schemeConstraints.initialize(100000,1000000,[tokenAddress],[1000],contractsWhitelist); + testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.GenericSchemeMultiCall,accounts,genesisProtocol,tokenAddress,testSetup.org.avatar,testSetup.schemeConstraints); var permissions = "0x00000010"; @@ -96,7 +93,7 @@ contract('GenericSchemeMultiCall', function(accounts) { assert.equal(tx.logs[0].args._values[0],10); assert.equal(tx.logs[0].args._descriptionHash,"description"); }); - + it("proposeCall log - with invalid array - reverts", async function() { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); @@ -331,25 +328,6 @@ contract('GenericSchemeMultiCall', function(accounts) { assert.equal(await standardTokenMock.allowance(testSetup.org.avatar.address,accounts[3]),1000); }); - it("cannot init without contract whitelist", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[actionMock.address]); - var genericSchemeMultiCall =await GenericSchemeMultiCall.new(); - - try { - await genericSchemeMultiCall.initialize( - testSetup.org.avatar.address, - accounts[0], - helpers.SOME_HASH, - [], - testSetup.schemeConstraints.address - ); - assert(false, "contractWhitelist cannot be empty"); - } catch(error) { - helpers.assertVMException(error); - } - }); - it("cannot init twice", async function() { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); @@ -358,7 +336,6 @@ contract('GenericSchemeMultiCall', function(accounts) { testSetup.org.avatar.address, accounts[0], helpers.SOME_HASH, - [accounts[0]], testSetup.schemeConstraints.address ); assert(false, "cannot init twice"); @@ -368,15 +345,13 @@ contract('GenericSchemeMultiCall', function(accounts) { }); it("can init with multiple contracts on whitelist", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[actionMock.address]); - var genericSchemeMultiCall =await GenericSchemeMultiCall.new(); - var tx = await genericSchemeMultiCall.initialize( - testSetup.org.avatar.address, - accounts[0], - helpers.SOME_HASH, - [accounts[0],accounts[1],accounts[2],accounts[3]], - testSetup.schemeConstraints.address + var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); + var tx = await dxDaoSchemeConstraints.initialize( + 1, + 0, + [], + [], + [accounts[0],accounts[1],accounts[2],accounts[3]] ); assert.equal(tx.logs.length,1); assert.equal(tx.logs[0].event,"WhiteListedContracts"); From 221721fecfd91802ff073a23eb7fe46a4e1bfbbd Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Sun, 11 Oct 2020 15:05:16 +0300 Subject: [PATCH 11/28] fix dxdao constraints --- contracts/schemes/DxDaoSchemeConstraints.sol | 10 ++-- test/genericschememulticall.js | 48 ++++++++++++++++++-- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index 896ff6d7..bbcf2cd6 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -14,7 +14,6 @@ contract DxDaoSchemeConstraints is SchemeConstraints { uint256 public periodLimitWei; mapping(uint256=>uint256) public periodSpendingTokens; - mapping(address=>uint256) public periodLimitToken; mapping (uint256 => mapping(address => uint256)) public periodSpendingToken; mapping(uint256=>uint256) public periodSpendingWei; @@ -63,7 +62,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { { uint256 observervationIndex = observationIndex(); - uint256 totalPeriodSpendingInWei = periodSpendingWei[observervationIndex]; + uint256 totalPeriodSpendingInWei; for (uint i = 0; i < _contractsToCall.length; i++) { // constraint eth transfer totalPeriodSpendingInWei = totalPeriodSpendingInWei.add(_values[i]); @@ -85,10 +84,11 @@ contract DxDaoSchemeConstraints is SchemeConstraints { periodSpendingToken[observervationIndex][contractToCall] <= periodLimitToken[contractToCall], "periodSpendingTokensExceeded"); } - periodSpendingWei[observervationIndex] = - periodSpendingWei[observervationIndex].add(totalPeriodSpendingInWei); - require(periodSpendingWei[observervationIndex] <= periodLimitWei, "periodSpendingWeiExceeded"); + } + periodSpendingWei[observervationIndex] = + periodSpendingWei[observervationIndex].add(totalPeriodSpendingInWei); + require(periodSpendingWei[observervationIndex] <= periodLimitWei, "periodSpendingWeiExceeded"); return true; } diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index d2e9dca1..871f53e0 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -55,7 +55,7 @@ const setup = async function (accounts,contractsWhitelist,reputationAccount=0,ge testSetup.org = await helpers.setupOrganizationWithArrays(testSetup.daoCreator,[accounts[0],accounts[1],reputationAccount],[1000,1000,1000],testSetup.reputationArray); } testSetup.schemeConstraints = await DxDaoSchemeConstraints.new(); - await testSetup.schemeConstraints.initialize(100000,1000000,[tokenAddress],[1000],contractsWhitelist); + await testSetup.schemeConstraints.initialize(100000,100000,[tokenAddress],[1000],contractsWhitelist); testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.GenericSchemeMultiCall,accounts,genesisProtocol,tokenAddress,testSetup.org.avatar,testSetup.schemeConstraints); var permissions = "0x00000010"; @@ -207,9 +207,9 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); - var value = 123; + var value = 50000; var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[value],helpers.NULL_HASH); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address,actionMock.address],[callData,callData],[value,value],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); //transfer some eth to avatar await web3.eth.sendTransaction({from:accounts[0],to:testSetup.org.avatar.address, value: web3.utils.toWei('1', "ether")}); @@ -224,7 +224,47 @@ contract('GenericSchemeMultiCall', function(accounts) { assert.equal(events[0].event,"ProposalExecuted"); assert.equal(events[0].args._proposalId,proposalId); }); - assert.equal(await web3.eth.getBalance(actionMock.address),value); + assert.equal(await web3.eth.getBalance(actionMock.address),value*2); + //try to execute another one within the same period should fail + tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address,actionMock.address],[callData,callData],[value,value],helpers.NULL_HASH); + proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + try { + await testSetup.GenericSchemeMultiCall.execute(proposalId); + assert(false, "cannot send more within the same period"); + } catch(error) { + helpers.assertVMException(error); + } + await helpers.increaseTime(100000); + tx = await testSetup.GenericSchemeMultiCall.execute(proposalId); + await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecuted', { + fromBlock: tx.blockNumber, + toBlock: 'latest' + }) + .then(function(events){ + assert.equal(events[0].event,"ProposalExecuted"); + assert.equal(events[0].args._proposalId,proposalId); + }); + }); + + it("schemeconstrains eth value exceed limit", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); + var value = 100001; + var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[value],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + //transfer some eth to avatar + await web3.eth.sendTransaction({from:accounts[0],to:testSetup.org.avatar.address, value: web3.utils.toWei('1', "ether")}); + assert.equal(await web3.eth.getBalance(actionMock.address),0); + await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + try { + await testSetup.GenericSchemeMultiCall.execute(proposalId); + assert(false, "cannot transfer eth amount"); + } catch(error) { + helpers.assertVMException(error); + } }); it("execute proposeVote -negative decision - check action - with GenesisProtocol", async function() { From 8917c30d43323bd69f3d1ba46d5ea451da1466eb Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Sun, 11 Oct 2020 17:33:52 +0300 Subject: [PATCH 12/28] check schemeConstraint exist --- contracts/schemes/GenericSchemeMultiCall.sol | 33 +++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/contracts/schemes/GenericSchemeMultiCall.sol b/contracts/schemes/GenericSchemeMultiCall.sol index 4122ae3a..881fdf51 100644 --- a/contracts/schemes/GenericSchemeMultiCall.sol +++ b/contracts/schemes/GenericSchemeMultiCall.sol @@ -116,13 +116,15 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf MultiCallProposal storage proposal = proposals[_proposalId]; require(proposal.exist, "must be a live proposal"); require(proposal.passed, "proposal must passed by voting machine"); - require( - schemeConstraints.isAllowedToCall( - proposal.contractsToCall, - proposal.callsData, - proposal.values, - avatar), - "call is not allowed"); + if (schemeConstraints != SchemeConstraints(0)) { + require( + schemeConstraints.isAllowedToCall( + proposal.contractsToCall, + proposal.callsData, + proposal.values, + avatar), + "call is not allowed"); + } proposal.exist = false; bytes memory genericCallReturnValue; bool success; @@ -172,14 +174,15 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf (_contractsToCall.length == _callsData.length) && (_contractsToCall.length == _values.length), "Wrong length of _contractsToCall, _callsDataLens or _values arrays" ); - - require( - schemeConstraints.isAllowedToPropose( - _contractsToCall, - _callsData, - _values, - avatar), - "propose is not allowed"); + if (schemeConstraints != SchemeConstraints(0)) { + require( + schemeConstraints.isAllowedToPropose( + _contractsToCall, + _callsData, + _values, + avatar), + "propose is not allowed"); + } proposalId = votingMachine.propose(2, voteParams, msg.sender, address(avatar)); From aad9d93033e35ec52aa95626e0306299555ba492 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Mon, 12 Oct 2020 21:10:46 +0300 Subject: [PATCH 13/28] + comments --- contracts/schemes/DxDaoSchemeConstraints.sol | 51 +++++++++++++------- contracts/schemes/GenericSchemeMultiCall.sol | 3 +- contracts/schemes/SchemeConstraints.sol | 24 ++++++++- test/genericschememulticall.js | 17 +++---- 4 files changed, 66 insertions(+), 29 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index bbcf2cd6..fda6364a 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -7,8 +7,6 @@ import "./SchemeConstraints.sol"; contract DxDaoSchemeConstraints is SchemeConstraints { using SafeMath for uint256; - event WhiteListedContracts(address[] _contractsWhitelist); - uint256 public initialTimestamp; uint256 public periodSize; uint256 public periodLimitWei; @@ -17,21 +15,22 @@ contract DxDaoSchemeConstraints is SchemeConstraints { mapping(address=>uint256) public periodLimitToken; mapping (uint256 => mapping(address => uint256)) public periodSpendingToken; mapping(uint256=>uint256) public periodSpendingWei; - mapping(address=>bool) public contractsWhitelist; + mapping(address=>bool) public contractsWhiteListMap; bytes4 private constant APPROVE_SIGNATURE = 0x095ea7b3;//approve(address,uint256) /* @dev initialize - * @param _avatar the avatar to mint reputation from - * @param _votingMachine the voting machines address to - * @param _voteParams voting machine parameters. - * @param _contractsWhitelist the contracts the scheme is allowed to interact with + * @param _periodSize the time period to limit the tokens and eth spending + * @param _periodLimitWei the limit of eth which can be sent per period + * @param _periodLimitTokensAddresses tokens to limit + * @param _periodLimitTokensAmounts the limit of token which can be sent per period + * @param _contractsWhiteList the contracts the scheme is allowed to interact with */ function initialize( uint256 _periodSize, uint256 _periodLimitWei, address[] calldata _periodLimitTokensAddresses, uint256[] calldata _periodLimitTokensAmounts, - address[] calldata _contractsWhitelist + address[] calldata _contractsWhiteList ) external { require(initialTimestamp == 0, "cannot initialize twice"); @@ -42,15 +41,25 @@ contract DxDaoSchemeConstraints is SchemeConstraints { periodLimitWei = _periodLimitWei; // solhint-disable-next-line not-rely-on-time initialTimestamp = block.timestamp; - for (uint i = 0; i < _contractsWhitelist.length; i++) { - contractsWhitelist[_contractsWhitelist[i]] = true; + for (uint i = 0; i < _contractsWhiteList.length; i++) { + contractsWhiteListMap[_contractsWhiteList[i]] = true; } - emit WhiteListedContracts(_contractsWhitelist); for (uint i = 0; i < _periodLimitTokensAmounts.length; i++) { periodLimitToken[_periodLimitTokensAddresses[i]] = _periodLimitTokensAmounts[i]; } + contractsWhiteList = _contractsWhiteList; } + /* + * @dev isAllowedToCall should be called upon a proposal execution. + * - check that the total spending of tokens within a 'periodSize' does not exceed the periodLimit per token + * - check that the total sending of eth within a 'periodSize' does not exceed the periodLimit + * @param _contractsToCall the contracts to be called + * @param _callsData - The abi encode data for the calls + * @param _values value(ETH) to transfer with the calls + * @param _avatar avatar + * @return bool value true-allowed false not allowed + */ function isAllowedToCall( address[] calldata _contractsToCall, bytes[] calldata _callsData, @@ -76,7 +85,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { address contractToCall = _contractsToCall[i]; // solhint-disable-next-line no-inline-assembly assembly { - amount := calldataload(add(callData, 68)) + amount := mload(add(callData, 68)) } periodSpendingToken[observervationIndex][contractToCall] = periodSpendingToken[observervationIndex][contractToCall].add(amount); @@ -92,6 +101,15 @@ contract DxDaoSchemeConstraints is SchemeConstraints { return true; } + /* + * @dev isAllowedToPropose should be called upon a proposal submition. + * allow only whitelisted target contracts or 'approve' calls which the 'spender' is whitelisted + * @param _contractsToCall the contracts to be called + * @param _callsData - The abi encode data for the calls + * @param _values value(ETH) to transfer with the calls + * @param _avatar avatar + * @return bool value true-allowed false not allowed + */ function isAllowedToPropose( address[] calldata _contractsToCall, bytes[] calldata _callsData, @@ -101,8 +119,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { returns(bool) { for (uint i = 0; i < _contractsToCall.length; i++) { - // constraint approve calls - if (!contractsWhitelist[_contractsToCall[i]]) { + if (!contractsWhiteListMap[_contractsToCall[i]]) { address spender; bytes memory callData = _callsData[i]; require( @@ -112,13 +129,13 @@ contract DxDaoSchemeConstraints is SchemeConstraints { callData[3] == APPROVE_SIGNATURE[3], "allow only approve call for none whitelistedContracts"); //in solidity > 6 this can be replaced by: - //(spender,) = abi.decode(callData[4:], (address, uint)); + //(spender,) = abi.descode(callData[4:], (address, uint)); // see https://github.com/ethereum/solidity/issues/9439 // solhint-disable-next-line no-inline-assembly assembly { - spender := calldataload(add(callData, 36)) + spender := mload(add(callData, 36)) } - require(contractsWhitelist[spender], "spender contract not whitelisted"); + require(contractsWhiteListMap[spender], "spender contract not whitelisted"); } } return true; diff --git a/contracts/schemes/GenericSchemeMultiCall.sol b/contracts/schemes/GenericSchemeMultiCall.sol index 881fdf51..94507367 100644 --- a/contracts/schemes/GenericSchemeMultiCall.sol +++ b/contracts/schemes/GenericSchemeMultiCall.sol @@ -64,8 +64,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf * @param _avatar the avatar to mint reputation from * @param _votingMachine the voting machines address to * @param _voteParams voting machine parameters. - * @param _contractsWhitelist the contracts the scheme is allowed to interact with - * + * @param _schemeConstraints the schemeConstraints contracts. */ function initialize( Avatar _avatar, diff --git a/contracts/schemes/SchemeConstraints.sol b/contracts/schemes/SchemeConstraints.sol index cea098bd..60d5091f 100644 --- a/contracts/schemes/SchemeConstraints.sol +++ b/contracts/schemes/SchemeConstraints.sol @@ -3,8 +3,18 @@ pragma experimental ABIEncoderV2; import "../controller/Avatar.sol"; -interface SchemeConstraints { +contract SchemeConstraints { + address[] public contractsWhiteList; + + /* + * @dev isAllowedToCall should be called upon a proposal execution. + * @param _contractsToCall the contracts to be called + * @param _callsData - The abi encode data for the calls + * @param _values value(ETH) to transfer with the calls + * @param _avatar avatar + * @return bool value true-allowed false not allowed + */ function isAllowedToCall( address[] calldata _contractsToCall, bytes[] calldata _callsData, @@ -12,6 +22,14 @@ interface SchemeConstraints { Avatar _avatar) external returns(bool); + /* + * @dev isAllowedToPropose should be called upon a proposal submition. + * @param _contractsToCall the contracts to be called + * @param _callsData - The abi encode data for the calls + * @param _values value(ETH) to transfer with the calls + * @param _avatar avatar + * @return bool value true-allowed false not allowed + */ function isAllowedToPropose( address[] calldata _contractsToCall, bytes[] calldata _callsData, @@ -19,4 +37,8 @@ interface SchemeConstraints { Avatar _avatar) external returns(bool); + function getContractsWhiteList() external view returns(address[] memory) { + return contractsWhiteList; + } + } diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 871f53e0..da0a3a69 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -41,7 +41,7 @@ const setupGenericSchemeParams = async function( return genericSchemeParams; }; -const setup = async function (accounts,contractsWhitelist,reputationAccount=0,genesisProtocol = false,tokenAddress=helpers.NULL_ADDRESS) { +const setup = async function (accounts,contractsWhiteList,reputationAccount=0,genesisProtocol = false,tokenAddress=helpers.NULL_ADDRESS) { var testSetup = new helpers.TestSetup(); testSetup.standardTokenMock = await ERC20Mock.new(accounts[1],100); testSetup.GenericSchemeMultiCall = await GenericSchemeMultiCall.new(); @@ -55,7 +55,7 @@ const setup = async function (accounts,contractsWhitelist,reputationAccount=0,ge testSetup.org = await helpers.setupOrganizationWithArrays(testSetup.daoCreator,[accounts[0],accounts[1],reputationAccount],[1000,1000,1000],testSetup.reputationArray); } testSetup.schemeConstraints = await DxDaoSchemeConstraints.new(); - await testSetup.schemeConstraints.initialize(100000,100000,[tokenAddress],[1000],contractsWhitelist); + await testSetup.schemeConstraints.initialize(100000,100000,[tokenAddress],[1000],contractsWhiteList); testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.GenericSchemeMultiCall,accounts,genesisProtocol,tokenAddress,testSetup.org.avatar,testSetup.schemeConstraints); var permissions = "0x00000010"; @@ -386,19 +386,18 @@ contract('GenericSchemeMultiCall', function(accounts) { it("can init with multiple contracts on whitelist", async function() { var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); - var tx = await dxDaoSchemeConstraints.initialize( + await dxDaoSchemeConstraints.initialize( 1, 0, [], [], [accounts[0],accounts[1],accounts[2],accounts[3]] ); - assert.equal(tx.logs.length,1); - assert.equal(tx.logs[0].event,"WhiteListedContracts"); - assert.equal(tx.logs[0].args._contractsWhitelist[0],accounts[0]); - assert.equal(tx.logs[0].args._contractsWhitelist[1],accounts[1]); - assert.equal(tx.logs[0].args._contractsWhitelist[2],accounts[2]); - assert.equal(tx.logs[0].args._contractsWhitelist[3],accounts[3]); + var contractsWhiteList = await dxDaoSchemeConstraints.getContractsWhiteList(); + assert.equal(contractsWhiteList[0],accounts[0]); + assert.equal(contractsWhiteList[1],accounts[1]); + assert.equal(contractsWhiteList[2],accounts[2]); + assert.equal(contractsWhiteList[3],accounts[3]); }); From 8a0bd6b306728bcc0c4cc902b64242d79d4a6fd5 Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Tue, 13 Oct 2020 12:23:05 +0300 Subject: [PATCH 14/28] coverage --- test/genericschememulticall.js | 66 ++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 6b8e699b..53c5a808 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -19,7 +19,7 @@ const setupGenericSchemeParams = async function( genesisProtocol = false, tokenAddress = 0, avatar, - schemeConstraints + schemeConstraintsAddress ) { var genericSchemeParams = new GenericSchemeParams(); if (genesisProtocol === true){ @@ -28,7 +28,7 @@ const setupGenericSchemeParams = async function( avatar.address, genericSchemeParams.votingMachine.genesisProtocol.address, genericSchemeParams.votingMachine.params, - schemeConstraints.address); + schemeConstraintsAddress); } else { genericSchemeParams.votingMachine = await helpers.setupAbsoluteVote(helpers.NULL_ADDRESS,50,genericScheme.address); @@ -36,12 +36,17 @@ const setupGenericSchemeParams = async function( avatar.address, genericSchemeParams.votingMachine.absoluteVote.address, genericSchemeParams.votingMachine.params, - schemeConstraints.address); + schemeConstraintsAddress); } return genericSchemeParams; }; -const setup = async function (accounts,contractsWhiteList,reputationAccount=0,genesisProtocol = false,tokenAddress=helpers.NULL_ADDRESS) { +const setup = async function (accounts, + contractsWhiteList, + reputationAccount=0, + genesisProtocol = false, + tokenAddress=helpers.NULL_ADDRESS, + useSchemeConstraint = true) { var testSetup = new helpers.TestSetup(); testSetup.standardTokenMock = await ERC20Mock.new(accounts[1],100); testSetup.GenericSchemeMultiCall = await GenericSchemeMultiCall.new(); @@ -54,9 +59,15 @@ const setup = async function (accounts,contractsWhiteList,reputationAccount=0,ge } else { testSetup.org = await helpers.setupOrganizationWithArrays(testSetup.daoCreator,[accounts[0],accounts[1],reputationAccount],[1000,1000,1000],testSetup.reputationArray); } - testSetup.schemeConstraints = await DxDaoSchemeConstraints.new(); - await testSetup.schemeConstraints.initialize(100000,100000,[tokenAddress],[1000],contractsWhiteList); - testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.GenericSchemeMultiCall,accounts,genesisProtocol,tokenAddress,testSetup.org.avatar,testSetup.schemeConstraints); + var schemeConstraintsAddress; + if (useSchemeConstraint) { + testSetup.schemeConstraints = await DxDaoSchemeConstraints.new(); + schemeConstraintsAddress = testSetup.schemeConstraints.address; + await testSetup.schemeConstraints.initialize(100000,100000,[tokenAddress],[1000],contractsWhiteList); + } else { + schemeConstraintsAddress = helpers.NULL_ADDRESS; + } + testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.GenericSchemeMultiCall,accounts,genesisProtocol,tokenAddress,testSetup.org.avatar,schemeConstraintsAddress); var permissions = "0x00000010"; @@ -419,4 +430,45 @@ contract('GenericSchemeMultiCall', function(accounts) { } }); + + it("none exist schemeConstraints for proposeCall", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address,false); + var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); + var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address,actionMock.address], + [callData1,encodedTokenApproval], + [0,0], + helpers.NULL_HASH); + }); + + it("none exist schemeConstraints for executeCall", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address,false); + var value = 100001; + var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[value],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + //transfer some eth to avatar + await web3.eth.sendTransaction({from:accounts[0],to:testSetup.org.avatar.address, value: web3.utils.toWei('1', "ether")}); + assert.equal(await web3.eth.getBalance(actionMock.address),0); + await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + await testSetup.GenericSchemeMultiCall.execute(proposalId); + }); + + it("execute none exist proposal", async function() { + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[actionMock.address]); + try { + await testSetup.GenericSchemeMultiCall.execute(helpers.SOME_HASH); + assert(false, "cannot execute none exist proposal"); + } catch(error) { + helpers.assertVMException(error); + } + + }); + }); From 09c040ef427c710120137d04495ccab20ab8b040 Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Tue, 13 Oct 2020 20:42:52 +0200 Subject: [PATCH 15/28] Added upgradable contraintLimits --- contracts/schemes/DxDaoSchemeConstraints.sol | 48 ++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index fda6364a..cc2268df 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -7,6 +7,7 @@ import "./SchemeConstraints.sol"; contract DxDaoSchemeConstraints is SchemeConstraints { using SafeMath for uint256; + address public avatar; uint256 public initialTimestamp; uint256 public periodSize; uint256 public periodLimitWei; @@ -19,6 +20,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { bytes4 private constant APPROVE_SIGNATURE = 0x095ea7b3;//approve(address,uint256) /* @dev initialize + * @param avatar the DAOs avatar address * @param _periodSize the time period to limit the tokens and eth spending * @param _periodLimitWei the limit of eth which can be sent per period * @param _periodLimitTokensAddresses tokens to limit @@ -26,6 +28,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { * @param _contractsWhiteList the contracts the scheme is allowed to interact with */ function initialize( + address avatar, uint256 _periodSize, uint256 _periodLimitWei, address[] calldata _periodLimitTokensAddresses, @@ -50,6 +53,51 @@ contract DxDaoSchemeConstraints is SchemeConstraints { contractsWhiteList = _contractsWhiteList; } + /* + * @dev updateContractWhitelist used to let the DAO update whitelisted contracts. + * @param _contractsAddresses - The contract that should be update + * @param _contractsWhitelisted – true adds a contract to the whitelist, false removes it. + */ + function updateContractWhitelist( + address[] _contractsAddresses, + bool[] _contractsWhitelisted + ) + external { + require(msg.sender == avatar, "caller must be avatar"); + require(_contractsAddresses.length == _contractsWhitelisted.length, + "invalid length _periodLimitTokensAddresses"); + for (uint i = 0; i < _contractsAddresses.length; i++) { + contractsWhiteListMap[_contractsAddresses[i]] = contractsWhiteListMap[_contractsWhitelisted[i]]; + } + } + + /* + * @dev updatePeriodLimitTokens lets the dao update limits to token limits. + * @param _tokensAddresses - The token that should be updated + * @param _tokensPeriodLimits – The amount that will be set as a spending limit + */ + function updatePeriodLimitTokens( + address[] _tokensAddresses, + uint256[] _tokensPeriodLimits + ) + external { + require(msg.sender == avatar, "caller must be avatar"); + require(_tokensAddresses.length == _tokensPeriodLimits.length, + "invalid length _tokensPeriodLimits"); + for (uint i = 0; i < _tokensAddresses.length; i++) { + periodLimitToken[_tokensAddresses[i]] = _tokensPeriodLimits[i]; + } + } + + /* + * @dev updatePeriodLimitWei lets the dao update limits to ETH spending limit. + * @param _periodLimitWei - The new spending limit in WEI that should be set. + */ + function updatePeriodLimitWei(uint256 _periodLimitWei) external { + require(msg.sender == avatar, "caller must be avatar"); + periodLimitWei = _periodLimitWei; + } + /* * @dev isAllowedToCall should be called upon a proposal execution. * - check that the total spending of tokens within a 'periodSize' does not exceed the periodLimit per token From 0231a5f48e436c7401fbfa81ca01bb2f3c89a749 Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Tue, 13 Oct 2020 21:16:48 +0200 Subject: [PATCH 16/28] updated genericschmemulticall tests --- contracts/schemes/DxDaoSchemeConstraints.sol | 13 +++++++------ test/genericschememulticall.js | 6 +++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index cc2268df..150fd347 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -28,7 +28,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { * @param _contractsWhiteList the contracts the scheme is allowed to interact with */ function initialize( - address avatar, + address _avatar, uint256 _periodSize, uint256 _periodLimitWei, address[] calldata _periodLimitTokensAddresses, @@ -42,6 +42,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { "invalid length _periodLimitTokensAddresses"); periodSize = _periodSize; periodLimitWei = _periodLimitWei; + avatar = _avatar; // solhint-disable-next-line not-rely-on-time initialTimestamp = block.timestamp; for (uint i = 0; i < _contractsWhiteList.length; i++) { @@ -59,15 +60,15 @@ contract DxDaoSchemeConstraints is SchemeConstraints { * @param _contractsWhitelisted – true adds a contract to the whitelist, false removes it. */ function updateContractWhitelist( - address[] _contractsAddresses, - bool[] _contractsWhitelisted + address[] calldata _contractsAddresses, + bool[] calldata _contractsWhitelisted ) external { require(msg.sender == avatar, "caller must be avatar"); require(_contractsAddresses.length == _contractsWhitelisted.length, "invalid length _periodLimitTokensAddresses"); for (uint i = 0; i < _contractsAddresses.length; i++) { - contractsWhiteListMap[_contractsAddresses[i]] = contractsWhiteListMap[_contractsWhitelisted[i]]; + contractsWhiteListMap[_contractsAddresses[i]] = _contractsWhitelisted[i]; } } @@ -77,8 +78,8 @@ contract DxDaoSchemeConstraints is SchemeConstraints { * @param _tokensPeriodLimits – The amount that will be set as a spending limit */ function updatePeriodLimitTokens( - address[] _tokensAddresses, - uint256[] _tokensPeriodLimits + address[] calldata _tokensAddresses, + uint256[] calldata _tokensPeriodLimits ) external { require(msg.sender == avatar, "caller must be avatar"); diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 53c5a808..245def3f 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -63,7 +63,7 @@ const setup = async function (accounts, if (useSchemeConstraint) { testSetup.schemeConstraints = await DxDaoSchemeConstraints.new(); schemeConstraintsAddress = testSetup.schemeConstraints.address; - await testSetup.schemeConstraints.initialize(100000,100000,[tokenAddress],[1000],contractsWhiteList); + await testSetup.schemeConstraints.initialize(testSetup.org.avatar.address,100000,100000,[tokenAddress],[1000],contractsWhiteList); } else { schemeConstraintsAddress = helpers.NULL_ADDRESS; } @@ -396,8 +396,12 @@ contract('GenericSchemeMultiCall', function(accounts) { }); it("can init with multiple contracts on whitelist", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); await dxDaoSchemeConstraints.initialize( + testSetup.org.avatar.address, 1, 0, [], From d6809a244851551906e6a9952cf76c3886c81b37 Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Thu, 15 Oct 2020 09:11:11 +0200 Subject: [PATCH 17/28] Updated DXdaoSchemeConstraints --- contracts/schemes/DxDaoSchemeConstraints.sol | 41 +++++++++----------- test/genericschememulticall.js | 2 +- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index 150fd347..79d514e0 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -3,7 +3,6 @@ pragma experimental ABIEncoderV2; import "./SchemeConstraints.sol"; - contract DxDaoSchemeConstraints is SchemeConstraints { using SafeMath for uint256; @@ -74,8 +73,8 @@ contract DxDaoSchemeConstraints is SchemeConstraints { /* * @dev updatePeriodLimitTokens lets the dao update limits to token limits. - * @param _tokensAddresses - The token that should be updated - * @param _tokensPeriodLimits – The amount that will be set as a spending limit + * @param _tokensAddresses - The token addresses to be updated + * @param _tokensPeriodLimits – The token amounts to be set as period limit. */ function updatePeriodLimitTokens( address[] calldata _tokensAddresses, @@ -118,10 +117,10 @@ contract DxDaoSchemeConstraints is SchemeConstraints { external returns(bool) { - uint256 observervationIndex = observationIndex(); uint256 totalPeriodSpendingInWei; for (uint i = 0; i < _contractsToCall.length; i++) { + require(contractsWhiteListMap[_contractsToCall[i]], "contract not whitelisted"); // constraint eth transfer totalPeriodSpendingInWei = totalPeriodSpendingInWei.add(_values[i]); bytes memory callData = _callsData[i]; @@ -132,6 +131,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { callData[3] == APPROVE_SIGNATURE[3]) { uint256 amount; address contractToCall = _contractsToCall[i]; + require(contractsWhiteListMap[contractToCall] == true, "tokenNotWhitelisted"); // solhint-disable-next-line no-inline-assembly assembly { amount := mload(add(callData, 68)) @@ -168,31 +168,26 @@ contract DxDaoSchemeConstraints is SchemeConstraints { returns(bool) { for (uint i = 0; i < _contractsToCall.length; i++) { - if (!contractsWhiteListMap[_contractsToCall[i]]) { - address spender; - bytes memory callData = _callsData[i]; - require( - callData[0] == APPROVE_SIGNATURE[0] && - callData[1] == APPROVE_SIGNATURE[1] && - callData[2] == APPROVE_SIGNATURE[2] && - callData[3] == APPROVE_SIGNATURE[3], - "allow only approve call for none whitelistedContracts"); - //in solidity > 6 this can be replaced by: - //(spender,) = abi.descode(callData[4:], (address, uint)); - // see https://github.com/ethereum/solidity/issues/9439 - // solhint-disable-next-line no-inline-assembly - assembly { - spender := mload(add(callData, 36)) - } - require(contractsWhiteListMap[spender], "spender contract not whitelisted"); - } + require(contractsWhiteListMap[_contractsToCall[i]], "contract not whitelisted"); + bytes memory callData = _callsData[i]; + if(callData[0] == APPROVE_SIGNATURE[0] && + callData[1] == APPROVE_SIGNATURE[1] && + callData[2] == APPROVE_SIGNATURE[2] && + callData[3] == APPROVE_SIGNATURE[3]){ + address spender; + // solhint-disable-next-line no-inline-assembly + assembly { + spender := mload(add(callData, 36)) + } + require(contractsWhiteListMap[spender], "spender contract not whitelisted"); + } } return true; } function observationIndex() public view returns (uint256) { // solhint-disable-next-line not-rely-on-time - return uint8((block.timestamp - initialTimestamp) / periodSize); + return ((block.timestamp - initialTimestamp) / periodSize); } } diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 245def3f..4b86c193 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -351,7 +351,7 @@ contract('GenericSchemeMultiCall', function(accounts) { it("execute proposeVote with multiple calls with votingMachine -positive decision", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); - var testSetup = await setup(accounts,[actionMock.address,accounts[3]],0,true,standardTokenMock.address); + var testSetup = await setup(accounts,[actionMock.address,accounts[3],standardTokenMock.address],0,true,standardTokenMock.address); var encodedTokenApproval = await createCallToTokenApproval(standardTokenMock,accounts[3], 1000); var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( From ad547564235e8ef7de6f20ab7a710e22b3526d8c Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Thu, 15 Oct 2020 12:20:15 +0200 Subject: [PATCH 18/28] Added contraint events and smaller updates --- contracts/schemes/DxDaoSchemeConstraints.sol | 29 ++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index 79d514e0..47698a27 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -6,6 +6,20 @@ import "./SchemeConstraints.sol"; contract DxDaoSchemeConstraints is SchemeConstraints { using SafeMath for uint256; + event UpdatedContractsWhitelist( + address _contractAddress, + bool _contractWhitelisted + ); + + event UpdatedPeriodLimitsTokens( + address _tokenAddress, + uint256 _tokenPeriodLimit + ); + + event UpdatedPeriodLimitWei( + uint256 _periodLimitWei + ); + address public avatar; uint256 public initialTimestamp; uint256 public periodSize; @@ -58,7 +72,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { * @param _contractsAddresses - The contract that should be update * @param _contractsWhitelisted – true adds a contract to the whitelist, false removes it. */ - function updateContractWhitelist( + function updateContractsWhitelist( address[] calldata _contractsAddresses, bool[] calldata _contractsWhitelisted ) @@ -68,15 +82,16 @@ contract DxDaoSchemeConstraints is SchemeConstraints { "invalid length _periodLimitTokensAddresses"); for (uint i = 0; i < _contractsAddresses.length; i++) { contractsWhiteListMap[_contractsAddresses[i]] = _contractsWhitelisted[i]; + emit UpdatedContractsWhitelist(_contractsAddresses[i], _contractsWhitelisted[i]); } } /* - * @dev updatePeriodLimitTokens lets the dao update limits to token limits. + * @dev updatePeriodLimitsTokens lets the dao update limits to token limits. * @param _tokensAddresses - The token addresses to be updated * @param _tokensPeriodLimits – The token amounts to be set as period limit. */ - function updatePeriodLimitTokens( + function updatePeriodLimitsTokens( address[] calldata _tokensAddresses, uint256[] calldata _tokensPeriodLimits ) @@ -86,6 +101,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { "invalid length _tokensPeriodLimits"); for (uint i = 0; i < _tokensAddresses.length; i++) { periodLimitToken[_tokensAddresses[i]] = _tokensPeriodLimits[i]; + emit UpdatedPeriodLimitsTokens(_tokensAddresses[i], _tokensPeriodLimits[i]); } } @@ -96,6 +112,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { function updatePeriodLimitWei(uint256 _periodLimitWei) external { require(msg.sender == avatar, "caller must be avatar"); periodLimitWei = _periodLimitWei; + emit UpdatedPeriodLimitWei(_periodLimitWei); } /* @@ -130,12 +147,14 @@ contract DxDaoSchemeConstraints is SchemeConstraints { callData[2] == APPROVE_SIGNATURE[2] && callData[3] == APPROVE_SIGNATURE[3]) { uint256 amount; + address spender; address contractToCall = _contractsToCall[i]; - require(contractsWhiteListMap[contractToCall] == true, "tokenNotWhitelisted"); // solhint-disable-next-line no-inline-assembly assembly { - amount := mload(add(callData, 68)) + spender := mload(add(callData, 36)) + amount := mload(add(callData, 68)) } + require(contractsWhiteListMap[spender], "spender contract not whitelisted"); periodSpendingToken[observervationIndex][contractToCall] = periodSpendingToken[observervationIndex][contractToCall].add(amount); require( From 990ca26a7236c8f389214fe11f99bdb3811af8bf Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Thu, 15 Oct 2020 14:27:36 +0200 Subject: [PATCH 19/28] Added additional tests for DxDaoSchemeConstraints --- test/genericschememulticall.js | 167 ++++++++++++++++++++++++++++++++- 1 file changed, 166 insertions(+), 1 deletion(-) diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 4b86c193..1a24c9cd 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -416,6 +416,24 @@ contract('GenericSchemeMultiCall', function(accounts) { }); + it("execute proposeVote with multiple calls with votingMachine without whitelisted token", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[],0,true,standardTokenMock.address); + var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); + var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + try { + await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address], + [callData1,encodedTokenApproval], + [0,0], + helpers.NULL_HASH); + assert(false, "contract not whitelisted"); + } catch(error) { + helpers.assertVMException(error); + } + }); + it("execute proposeVote with multiple calls with votingMachine without whitelisted spender", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); @@ -434,7 +452,6 @@ contract('GenericSchemeMultiCall', function(accounts) { } }); - it("none exist schemeConstraints for proposeCall", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); @@ -475,4 +492,152 @@ contract('GenericSchemeMultiCall', function(accounts) { }); + it("cannot update constraints from other address then avatar", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); + var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); + await dxDaoSchemeConstraints.initialize( + testSetup.org.avatar.address, + 1, + 0, + [], + [], + [accounts[0]] + ); + try { + await dxDaoSchemeConstraints.updateContractsWhitelist([actionMock.address],[true]) + assert(false, "caller must be avatar"); + } catch(error) { + helpers.assertVMException(error); + } + + try { + await dxDaoSchemeConstraints.updatePeriodLimitsTokens([actionMock.address, standardTokenMock.address],[5000,6000]) + assert(false, "caller must be avatar"); + } catch(error) { + helpers.assertVMException(error); + } + + try { + await dxDaoSchemeConstraints.updatePeriodLimitWei(5000) + assert(false, "caller must be avatar"); + } catch(error) { + helpers.assertVMException(error); + } + + var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); + await dxDaoSchemeConstraints.initialize( + accounts[3], + 1, + 0, + [], + [], + [accounts[0]] + ); + await dxDaoSchemeConstraints.updatePeriodLimitWei(10000,{from:accounts[3]}); + await dxDaoSchemeConstraints.getPastEvents('UpdatedPeriodLimitWei', { + fromBlock: 0, + toBlock: 'latest' + }) + .then(function(events){ + assert.equal(events[0].event,"UpdatedPeriodLimitWei"); + assert.equal(events[0].args._periodLimitWei,10000); + }); + + await dxDaoSchemeConstraints.updatePeriodLimitsTokens([standardTokenMock.address],[10000],{from:accounts[3]}); + await dxDaoSchemeConstraints.getPastEvents('UpdatedPeriodLimitsTokens', { + fromBlock: 0, + toBlock: 'latest' + }) + .then(function(events){ + assert.equal(events[0].event,"UpdatedPeriodLimitsTokens"); + assert.equal(events[0].args._tokenAddress,standardTokenMock.address); + assert.equal(events[0].args._tokenPeriodLimit,10000); + }); + + await dxDaoSchemeConstraints.updateContractsWhitelist([standardTokenMock.address],[true],{from:accounts[3]}); + await dxDaoSchemeConstraints.getPastEvents('UpdatedContractsWhitelist', { + fromBlock: 0, + toBlock: 'latest' + }) + .then(function(events){ + assert.equal(events[0].event,"UpdatedContractsWhitelist"); + assert.equal(events[0].args._contractAddress,standardTokenMock.address); + assert.equal(events[0].args._contractWhitelisted,true); + }); + try { + await dxDaoSchemeConstraints.updateContractsWhitelist([standardTokenMock.address],[true, false],{from:accounts[3]}); + assert(false, "invalid length _periodLimitTokensAddresses"); + } catch(error) { + helpers.assertVMException(error); + } + + try { + await dxDaoSchemeConstraints.updatePeriodLimitsTokens([standardTokenMock.address],[10000, 500],{from:accounts[3]}); + assert(false, "invalid length _tokensPeriodLimits"); + } catch(error) { + helpers.assertVMException(error); + } + + }); + + it("calculates the observationIndex correctly", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); + var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); + + // 10 seconds period + await dxDaoSchemeConstraints.initialize( + testSetup.org.avatar.address, + 10, + 0, + [], + [], + [accounts[0]] + ); + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),0); + await helpers.increaseTime(10); + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),1); + await helpers.increaseTime(3600); // adding 1 hour + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),361); + await helpers.increaseTime(86400); // adding 1 day + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),9001); + await helpers.increaseTime(315360000);// adding 10 year + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),31545001); + var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); + + // 7 days period + await dxDaoSchemeConstraints.initialize( + testSetup.org.avatar.address, + 604800, + 0, + [], + [], + [accounts[0]] + ); + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),0); + await helpers.increaseTime(50); + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),0); + await helpers.increaseTime(604750); + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),1); + await helpers.increaseTime(604800); // adding 7 days + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),2); + await helpers.increaseTime(604800); // adding 7 days + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),3); + await helpers.increaseTime(604800); // adding 7 days + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),4); + await helpers.increaseTime(604800); // adding 7 days + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),5); + await helpers.increaseTime(9072000); // adding 15 days + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),20); + await helpers.increaseTime(6048000); // adding 10 days + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),30); + await helpers.increaseTime(42336000); // adding 70 days + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),100); + await helpers.increaseTime(604800000); // adding 1000 days + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),1100); + }); + }); From bd5a72b22764681d353d77f72712db9d3746dbfc Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Thu, 15 Oct 2020 14:54:53 +0200 Subject: [PATCH 20/28] rebase from arc/master --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9ce12076..632c8d1c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@daostack/arc", - "version": "0.0.1-rc.45", + "version": "0.0.1-rc.46", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 173b2b13..bfd55323 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@daostack/arc", - "version": "0.0.1-rc.45", + "version": "0.0.1-rc.46", "description": "A platform for building DAOs", "files": [ "contracts/", From aad89291e1191be454252454510043ff74e67ac7 Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Thu, 15 Oct 2020 15:15:44 +0200 Subject: [PATCH 21/28] Updated DxDaoSchemeContraints events & general cleanup --- contracts/schemes/DxDaoSchemeConstraints.sol | 16 +++-- test/genericschememulticall.js | 63 +++++++++++++------- 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index 47698a27..207a8f56 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -7,19 +7,23 @@ contract DxDaoSchemeConstraints is SchemeConstraints { using SafeMath for uint256; event UpdatedContractsWhitelist( - address _contractAddress, - bool _contractWhitelisted + address[] _contractsAddresses, + bool[] _contractsWhitelisted ); event UpdatedPeriodLimitsTokens( - address _tokenAddress, - uint256 _tokenPeriodLimit + address[] _tokensAddresses, + uint256[] _tokensPeriodLimits ); event UpdatedPeriodLimitWei( uint256 _periodLimitWei ); + + + + address public avatar; uint256 public initialTimestamp; uint256 public periodSize; @@ -82,7 +86,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { "invalid length _periodLimitTokensAddresses"); for (uint i = 0; i < _contractsAddresses.length; i++) { contractsWhiteListMap[_contractsAddresses[i]] = _contractsWhitelisted[i]; - emit UpdatedContractsWhitelist(_contractsAddresses[i], _contractsWhitelisted[i]); + emit UpdatedContractsWhitelist(_contractsAddresses, _contractsWhitelisted); } } @@ -101,7 +105,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { "invalid length _tokensPeriodLimits"); for (uint i = 0; i < _tokensAddresses.length; i++) { periodLimitToken[_tokensAddresses[i]] = _tokensPeriodLimits[i]; - emit UpdatedPeriodLimitsTokens(_tokensAddresses[i], _tokensPeriodLimits[i]); + emit UpdatedPeriodLimitsTokens(_tokensAddresses, _tokensPeriodLimits); } } diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 1a24c9cd..c4e3a32f 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -492,7 +492,7 @@ contract('GenericSchemeMultiCall', function(accounts) { }); - it("cannot update constraints from other address then avatar", async function() { + it("can only update contraint whitelist & limits from avatar", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); @@ -506,27 +506,27 @@ contract('GenericSchemeMultiCall', function(accounts) { [accounts[0]] ); try { - await dxDaoSchemeConstraints.updateContractsWhitelist([actionMock.address],[true]) + await dxDaoSchemeConstraints.updateContractsWhitelist([actionMock.address],[true]); assert(false, "caller must be avatar"); } catch(error) { helpers.assertVMException(error); } try { - await dxDaoSchemeConstraints.updatePeriodLimitsTokens([actionMock.address, standardTokenMock.address],[5000,6000]) + await dxDaoSchemeConstraints.updatePeriodLimitsTokens([actionMock.address, standardTokenMock.address],[5000,6000]); assert(false, "caller must be avatar"); } catch(error) { helpers.assertVMException(error); } try { - await dxDaoSchemeConstraints.updatePeriodLimitWei(5000) + await dxDaoSchemeConstraints.updatePeriodLimitWei(5000); assert(false, "caller must be avatar"); } catch(error) { helpers.assertVMException(error); } - var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); + dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); await dxDaoSchemeConstraints.initialize( accounts[3], 1, @@ -552,8 +552,8 @@ contract('GenericSchemeMultiCall', function(accounts) { }) .then(function(events){ assert.equal(events[0].event,"UpdatedPeriodLimitsTokens"); - assert.equal(events[0].args._tokenAddress,standardTokenMock.address); - assert.equal(events[0].args._tokenPeriodLimit,10000); + assert.equal(events[0].args._tokensAddresses[0],standardTokenMock.address); + assert.equal(events[0].args._tokensPeriodLimits[0],10000); }); await dxDaoSchemeConstraints.updateContractsWhitelist([standardTokenMock.address],[true],{from:accounts[3]}); @@ -563,9 +563,10 @@ contract('GenericSchemeMultiCall', function(accounts) { }) .then(function(events){ assert.equal(events[0].event,"UpdatedContractsWhitelist"); - assert.equal(events[0].args._contractAddress,standardTokenMock.address); - assert.equal(events[0].args._contractWhitelisted,true); + assert.equal(events[0].args._contractsAddresses[0],standardTokenMock.address); + assert.equal(events[0].args._contractsWhitelisted[0],true); }); + try { await dxDaoSchemeConstraints.updateContractsWhitelist([standardTokenMock.address],[true, false],{from:accounts[3]}); assert(false, "invalid length _periodLimitTokensAddresses"); @@ -582,6 +583,24 @@ contract('GenericSchemeMultiCall', function(accounts) { }); + it("can only update contraint whitelist & limits from avatar with correct array length", async function() { + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var dxDaoSchemeConstraints2 =await DxDaoSchemeConstraints.new(); + try { + await dxDaoSchemeConstraints2.updateContractsWhitelist([standardTokenMock.address],[true, false],{from:accounts[3]}); + assert(false, "invalid length _periodLimitTokensAddresses"); + } catch(error) { + helpers.assertVMException(error); + } + + try { + await dxDaoSchemeConstraints2.updatePeriodLimitsTokens([standardTokenMock.address],[10000, 500],{from:accounts[3]}); + assert(false, "invalid length _tokensPeriodLimits"); + } catch(error) { + helpers.assertVMException(error); + } + }); + it("calculates the observationIndex correctly", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); @@ -606,10 +625,10 @@ contract('GenericSchemeMultiCall', function(accounts) { assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),9001); await helpers.increaseTime(315360000);// adding 10 year assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),31545001); - var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); + var dxDaoSchemeConstraints2 =await DxDaoSchemeConstraints.new(); // 7 days period - await dxDaoSchemeConstraints.initialize( + await dxDaoSchemeConstraints2.initialize( testSetup.org.avatar.address, 604800, 0, @@ -617,27 +636,27 @@ contract('GenericSchemeMultiCall', function(accounts) { [], [accounts[0]] ); - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),0); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),0); await helpers.increaseTime(50); - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),0); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),0); await helpers.increaseTime(604750); - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),1); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),1); await helpers.increaseTime(604800); // adding 7 days - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),2); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),2); await helpers.increaseTime(604800); // adding 7 days - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),3); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),3); await helpers.increaseTime(604800); // adding 7 days - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),4); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),4); await helpers.increaseTime(604800); // adding 7 days - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),5); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),5); await helpers.increaseTime(9072000); // adding 15 days - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),20); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),20); await helpers.increaseTime(6048000); // adding 10 days - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),30); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),30); await helpers.increaseTime(42336000); // adding 70 days - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),100); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),100); await helpers.increaseTime(604800000); // adding 1000 days - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),1100); + assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),1100); }); }); From 1b319f6748ab64ea938f7d4dbb2036ed61af63d7 Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Thu, 15 Oct 2020 15:20:59 +0200 Subject: [PATCH 22/28] solhint cleanup --- contracts/schemes/DxDaoSchemeConstraints.sol | 30 +++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index 207a8f56..03d1500c 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -20,10 +20,6 @@ contract DxDaoSchemeConstraints is SchemeConstraints { uint256 _periodLimitWei ); - - - - address public avatar; uint256 public initialTimestamp; uint256 public periodSize; @@ -191,19 +187,19 @@ contract DxDaoSchemeConstraints is SchemeConstraints { returns(bool) { for (uint i = 0; i < _contractsToCall.length; i++) { - require(contractsWhiteListMap[_contractsToCall[i]], "contract not whitelisted"); - bytes memory callData = _callsData[i]; - if(callData[0] == APPROVE_SIGNATURE[0] && - callData[1] == APPROVE_SIGNATURE[1] && - callData[2] == APPROVE_SIGNATURE[2] && - callData[3] == APPROVE_SIGNATURE[3]){ - address spender; - // solhint-disable-next-line no-inline-assembly - assembly { - spender := mload(add(callData, 36)) - } - require(contractsWhiteListMap[spender], "spender contract not whitelisted"); - } + require(contractsWhiteListMap[_contractsToCall[i]], "contract not whitelisted"); + bytes memory callData = _callsData[i]; + if (callData[0] == APPROVE_SIGNATURE[0] && + callData[1] == APPROVE_SIGNATURE[1] && + callData[2] == APPROVE_SIGNATURE[2] && + callData[3] == APPROVE_SIGNATURE[3]) { + address spender; + // solhint-disable-next-line no-inline-assembly + assembly { + spender := mload(add(callData, 36)) + } + require(contractsWhiteListMap[spender], "spender contract not whitelisted"); + } } return true; } From 32c8403f64809323225ce399320ea2d8803e23c3 Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Thu, 15 Oct 2020 15:28:24 +0200 Subject: [PATCH 23/28] cleanup --- contracts/schemes/DxDaoSchemeConstraints.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index 03d1500c..6c0f1a71 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -82,8 +82,8 @@ contract DxDaoSchemeConstraints is SchemeConstraints { "invalid length _periodLimitTokensAddresses"); for (uint i = 0; i < _contractsAddresses.length; i++) { contractsWhiteListMap[_contractsAddresses[i]] = _contractsWhitelisted[i]; - emit UpdatedContractsWhitelist(_contractsAddresses, _contractsWhitelisted); } + emit UpdatedContractsWhitelist(_contractsAddresses, _contractsWhitelisted); } /* @@ -101,8 +101,8 @@ contract DxDaoSchemeConstraints is SchemeConstraints { "invalid length _tokensPeriodLimits"); for (uint i = 0; i < _tokensAddresses.length; i++) { periodLimitToken[_tokensAddresses[i]] = _tokensPeriodLimits[i]; - emit UpdatedPeriodLimitsTokens(_tokensAddresses, _tokensPeriodLimits); } + emit UpdatedPeriodLimitsTokens(_tokensAddresses, _tokensPeriodLimits); } /* From 58b64b8fcfd5a393043c8b16062f3b5b78bc00b0 Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Thu, 15 Oct 2020 16:50:17 +0200 Subject: [PATCH 24/28] Fixed test issues --- test/genericschememulticall.js | 82 +++++++++++++--------------------- 1 file changed, 32 insertions(+), 50 deletions(-) diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index c41e874b..259867f7 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -49,7 +49,7 @@ const setup = async function (accounts, useSchemeConstraint = true) { var testSetup = new helpers.TestSetup(); testSetup.standardTokenMock = await ERC20Mock.new(accounts[1],100); - testSetup.genericSchemeMultiCall = await GenericSchemeMultiCall.new(); + testSetup.GenericSchemeMultiCall = await GenericSchemeMultiCall.new(); var controllerCreator = await ControllerCreator.new(); var daoTracker = await DAOTracker.new(); testSetup.daoCreator = await DaoCreator.new(controllerCreator.address,daoTracker.address); @@ -72,7 +72,7 @@ const setup = async function (accounts, await testSetup.daoCreator.setSchemes(testSetup.org.avatar.address, - [testSetup.genericSchemeMultiCall.address], + [testSetup.GenericSchemeMultiCall.address], [helpers.NULL_HASH],[permissions],"metaData"); return testSetup; @@ -95,7 +95,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls( + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( [actionMock.address],[callData],[10],"description"); assert.equal(tx.logs.length, 1); assert.equal(tx.logs[0].event, "NewMultiCallProposal"); @@ -110,21 +110,21 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[actionMock.address]); var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); try { - await testSetup.genericSchemeMultiCall.proposeCalls( + await testSetup.GenericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock.address],[callData],[0],helpers.NULL_HASH); assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); } catch(error) { helpers.assertVMException(error); } try { - await testSetup.genericSchemeMultiCall.proposeCalls( + await testSetup.GenericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock.address],[callData],[0],helpers.NULL_HASH); assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); } catch(error) { helpers.assertVMException(error); } try { - await testSetup.genericSchemeMultiCall.proposeCalls( + await testSetup.GenericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock.address],[callData],[0,0],helpers.NULL_HASH); assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); } catch(error) { @@ -136,12 +136,12 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls( + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( [actionMock.address],[callData],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,0,0,helpers.NULL_ADDRESS,{from:accounts[2]}); //check organizationsProposals after execution - var proposal = await testSetup.genericSchemeMultiCall.proposals(proposalId); + var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); assert.equal(proposal.passed,false); assert.equal(proposal.callData,null); }); @@ -150,13 +150,13 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls( + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( [actionMock.address],[callData],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - var proposal = await testSetup.genericSchemeMultiCall.proposals(proposalId); + var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); //check organizationsProposals after execution - proposal = await testSetup.genericSchemeMultiCall.proposals(proposalId); + proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); assert.equal(proposal.callData,null);//new contract address }); @@ -164,13 +164,13 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); var callData = await createCallToActionMock(helpers.NULL_ADDRESS,actionMock); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls( + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( [actionMock.address],[callData],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); //actionMock revert because msg.sender is not the _addr param at actionMock though the whole proposal execution will fail. await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); try { - await testSetup.genericSchemeMultiCall.execute(proposalId); + await testSetup.GenericSchemeMultiCall.execute(proposalId); assert(false, "Proposal call failed"); } catch(error) { helpers.assertVMException(error); @@ -182,7 +182,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[accounts[1]]); var callData = await createCallToActionMock(helpers.NULL_ADDRESS,actionMock); try { - await testSetup.genericSchemeMultiCall.proposeCalls( + await testSetup.GenericSchemeMultiCall.proposeCalls( [actionMock.address],[callData],[0],helpers.NULL_HASH); assert(false, "contractToCall is not whitelisted"); } catch(error) { @@ -194,7 +194,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); const encodeABI = await new web3.eth.Contract(actionMock.abi).methods.withoutReturnValue(testSetup.org.avatar.address).encodeABI(); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); }); @@ -203,10 +203,10 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); const encodeABI = await new web3.eth.Contract(actionMock.abi).methods.withoutReturnValue(testSetup.org.avatar.address).encodeABI(); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); try { - await testSetup.genericSchemeMultiCall.execute( proposalId); + await testSetup.GenericSchemeMultiCall.execute( proposalId); assert(false, "execute should fail if not executed from votingMachine"); } catch(error) { helpers.assertVMException(error); @@ -226,8 +226,8 @@ contract('GenericSchemeMultiCall', function(accounts) { await web3.eth.sendTransaction({from:accounts[0],to:testSetup.org.avatar.address, value: web3.utils.toWei('1', "ether")}); assert.equal(await web3.eth.getBalance(actionMock.address),0); await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - tx = await testSetup.genericSchemeMultiCall.execute(proposalId); - await testSetup.genericSchemeMultiCall.getPastEvents('ProposalExecuted', { + tx = await testSetup.GenericSchemeMultiCall.execute(proposalId); + await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecuted', { fromBlock: tx.blockNumber, toBlock: 'latest' }) @@ -278,31 +278,16 @@ contract('GenericSchemeMultiCall', function(accounts) { } }); - it("schemeconstrains token value exceed limit", async function() { - var standardTokenMock = await ERC20Mock.new(accounts[0],1000); - var testSetup = await setup(accounts,[accounts[3]],0,true,standardTokenMock.address); - var encodedTokenApproval = await createCallToTokenApproval(standardTokenMock,accounts[3], 10001); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls([standardTokenMock.address],[encodedTokenApproval],[0],helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - try { - await testSetup.genericSchemeMultiCall.execute(proposalId); - assert(false, "cannot approve token amount: periodSpendingTokensExceeded"); - } catch(error) { - helpers.assertVMException(error); - } - }); - it("execute proposeVote -negative decision - check action - with GenesisProtocol", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[0],helpers.NULL_HASH); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); tx = await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,2,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - await testSetup.genericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { + await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { fromBlock: tx.blockNumber, toBlock: 'latest' }) @@ -320,14 +305,14 @@ contract('GenericSchemeMultiCall', function(accounts) { var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); var callData2 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls( + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock2.address], [callData1,callData2], [0,0], helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); tx = await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - await testSetup.genericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { + await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { fromBlock: tx.blockNumber, toBlock: 'latest' }) @@ -344,18 +329,18 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[actionMock.address,actionMock2.address],0,true,standardTokenMock.address); var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); var callData2 = await createCallToActionMock(accounts[0],actionMock); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls( + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock2.address], [callData1,callData2], [0,0], helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - var proposal = await testSetup.genericSchemeMultiCall.proposals(proposalId); + var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); assert.equal(proposal.exist,true); assert.equal(proposal.passed,false); await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); try { - await testSetup.genericSchemeMultiCall.execute(proposalId); + await testSetup.GenericSchemeMultiCall.execute(proposalId); assert(false, "Proposal call failed"); } catch(error) { helpers.assertVMException(error); @@ -375,13 +360,13 @@ contract('GenericSchemeMultiCall', function(accounts) { [0,0], helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - var proposal = await testSetup.genericSchemeMultiCall.proposals(proposalId); + var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); assert.equal(proposal.exist,true); assert.equal(proposal.passed,false); assert.equal(await standardTokenMock.allowance(testSetup.org.avatar.address,accounts[3]),0); await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - await testSetup.genericSchemeMultiCall.execute(proposalId); - await testSetup.genericSchemeMultiCall.getPastEvents('ProposalCallExecuted', { + await testSetup.GenericSchemeMultiCall.execute(proposalId); + await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalCallExecuted', { fromBlock: tx.blockNumber, toBlock: 'latest' }) @@ -417,7 +402,6 @@ contract('GenericSchemeMultiCall', function(accounts) { var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); await dxDaoSchemeConstraints.initialize( testSetup.org.avatar.address, - 1, 0, [], @@ -468,7 +452,6 @@ contract('GenericSchemeMultiCall', function(accounts) { } }); - it("none exist schemeConstraints for proposeCall", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); @@ -476,7 +459,6 @@ contract('GenericSchemeMultiCall', function(accounts) { var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address,actionMock.address], [callData1,encodedTokenApproval], [0,0], @@ -603,16 +585,16 @@ contract('GenericSchemeMultiCall', function(accounts) { it("can only update contraint whitelist & limits from avatar with correct array length", async function() { var standardTokenMock = await ERC20Mock.new(accounts[0],1000); - var dxDaoSchemeConstraints2 =await DxDaoSchemeConstraints.new(); + var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); try { - await dxDaoSchemeConstraints2.updateContractsWhitelist([standardTokenMock.address],[true, false],{from:accounts[3]}); + await dxDaoSchemeConstraints.updateContractsWhitelist([standardTokenMock.address],[true, false],{from:accounts[3]}); assert(false, "invalid length _periodLimitTokensAddresses"); } catch(error) { helpers.assertVMException(error); } try { - await dxDaoSchemeConstraints2.updatePeriodLimitsTokens([standardTokenMock.address],[10000, 500],{from:accounts[3]}); + await dxDaoSchemeConstraints.updatePeriodLimitsTokens([standardTokenMock.address],[10000, 500],{from:accounts[3]}); assert(false, "invalid length _tokensPeriodLimits"); } catch(error) { helpers.assertVMException(error); From 75a38d16b361d6683fcdae9791338215448f33ab Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Thu, 15 Oct 2020 21:00:24 +0200 Subject: [PATCH 25/28] updated DxDaoSchemeConstraints.sol --- test/genericschememulticall.js | 208 ++++++++++++++++++++++++--------- 1 file changed, 150 insertions(+), 58 deletions(-) diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 259867f7..cbdccc33 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -49,7 +49,7 @@ const setup = async function (accounts, useSchemeConstraint = true) { var testSetup = new helpers.TestSetup(); testSetup.standardTokenMock = await ERC20Mock.new(accounts[1],100); - testSetup.GenericSchemeMultiCall = await GenericSchemeMultiCall.new(); + testSetup.genericSchemeMultiCall = await GenericSchemeMultiCall.new(); var controllerCreator = await ControllerCreator.new(); var daoTracker = await DAOTracker.new(); testSetup.daoCreator = await DaoCreator.new(controllerCreator.address,daoTracker.address); @@ -67,12 +67,12 @@ const setup = async function (accounts, } else { schemeConstraintsAddress = helpers.NULL_ADDRESS; } - testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.GenericSchemeMultiCall,accounts,genesisProtocol,tokenAddress,testSetup.org.avatar,schemeConstraintsAddress); + testSetup.genericSchemeParams= await setupGenericSchemeParams(testSetup.genericSchemeMultiCall,accounts,genesisProtocol,tokenAddress,testSetup.org.avatar,schemeConstraintsAddress); var permissions = "0x00000010"; await testSetup.daoCreator.setSchemes(testSetup.org.avatar.address, - [testSetup.GenericSchemeMultiCall.address], + [testSetup.genericSchemeMultiCall.address], [helpers.NULL_HASH],[permissions],"metaData"); return testSetup; @@ -95,7 +95,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + var tx = await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address],[callData],[10],"description"); assert.equal(tx.logs.length, 1); assert.equal(tx.logs[0].event, "NewMultiCallProposal"); @@ -110,21 +110,21 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[actionMock.address]); var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); try { - await testSetup.GenericSchemeMultiCall.proposeCalls( + await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock.address],[callData],[0],helpers.NULL_HASH); assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); } catch(error) { helpers.assertVMException(error); } try { - await testSetup.GenericSchemeMultiCall.proposeCalls( + await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock.address],[callData],[0],helpers.NULL_HASH); assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); } catch(error) { helpers.assertVMException(error); } try { - await testSetup.GenericSchemeMultiCall.proposeCalls( + await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock.address],[callData],[0,0],helpers.NULL_HASH); assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); } catch(error) { @@ -136,12 +136,12 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + var tx = await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address],[callData],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,0,0,helpers.NULL_ADDRESS,{from:accounts[2]}); //check organizationsProposals after execution - var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); + var proposal = await testSetup.genericSchemeMultiCall.proposals(proposalId); assert.equal(proposal.passed,false); assert.equal(proposal.callData,null); }); @@ -150,13 +150,13 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + var tx = await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address],[callData],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); + var proposal = await testSetup.genericSchemeMultiCall.proposals(proposalId); await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); //check organizationsProposals after execution - proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); + proposal = await testSetup.genericSchemeMultiCall.proposals(proposalId); assert.equal(proposal.callData,null);//new contract address }); @@ -164,13 +164,13 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); var callData = await createCallToActionMock(helpers.NULL_ADDRESS,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + var tx = await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address],[callData],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); //actionMock revert because msg.sender is not the _addr param at actionMock though the whole proposal execution will fail. await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); try { - await testSetup.GenericSchemeMultiCall.execute(proposalId); + await testSetup.genericSchemeMultiCall.execute(proposalId); assert(false, "Proposal call failed"); } catch(error) { helpers.assertVMException(error); @@ -182,7 +182,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[accounts[1]]); var callData = await createCallToActionMock(helpers.NULL_ADDRESS,actionMock); try { - await testSetup.GenericSchemeMultiCall.proposeCalls( + await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address],[callData],[0],helpers.NULL_HASH); assert(false, "contractToCall is not whitelisted"); } catch(error) { @@ -194,7 +194,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); const encodeABI = await new web3.eth.Contract(actionMock.abi).methods.withoutReturnValue(testSetup.org.avatar.address).encodeABI(); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); + var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); await testSetup.genericSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); }); @@ -203,10 +203,10 @@ contract('GenericSchemeMultiCall', function(accounts) { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); const encodeABI = await new web3.eth.Contract(actionMock.abi).methods.withoutReturnValue(testSetup.org.avatar.address).encodeABI(); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); + var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address],[encodeABI],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); try { - await testSetup.GenericSchemeMultiCall.execute( proposalId); + await testSetup.genericSchemeMultiCall.execute( proposalId); assert(false, "execute should fail if not executed from votingMachine"); } catch(error) { helpers.assertVMException(error); @@ -220,14 +220,14 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); var value = 50000; var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address,actionMock.address],[callData,callData],[value,value],helpers.NULL_HASH); + var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address,actionMock.address],[callData,callData],[value,value],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); //transfer some eth to avatar await web3.eth.sendTransaction({from:accounts[0],to:testSetup.org.avatar.address, value: web3.utils.toWei('1', "ether")}); assert.equal(await web3.eth.getBalance(actionMock.address),0); await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - tx = await testSetup.GenericSchemeMultiCall.execute(proposalId); - await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecuted', { + tx = await testSetup.genericSchemeMultiCall.execute(proposalId); + await testSetup.genericSchemeMultiCall.getPastEvents('ProposalExecuted', { fromBlock: tx.blockNumber, toBlock: 'latest' }) @@ -237,18 +237,18 @@ contract('GenericSchemeMultiCall', function(accounts) { }); assert.equal(await web3.eth.getBalance(actionMock.address),value*2); //try to execute another one within the same period should fail - tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address,actionMock.address],[callData,callData],[value,value],helpers.NULL_HASH); + tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address,actionMock.address],[callData,callData],[value,value],helpers.NULL_HASH); proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); try { - await testSetup.GenericSchemeMultiCall.execute(proposalId); + await testSetup.genericSchemeMultiCall.execute(proposalId); assert(false, "cannot send more within the same period"); } catch(error) { helpers.assertVMException(error); } await helpers.increaseTime(100000); - tx = await testSetup.GenericSchemeMultiCall.execute(proposalId); - await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecuted', { + tx = await testSetup.genericSchemeMultiCall.execute(proposalId); + await testSetup.genericSchemeMultiCall.getPastEvents('ProposalExecuted', { fromBlock: tx.blockNumber, toBlock: 'latest' }) @@ -264,30 +264,45 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); var value = 100001; var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[value],helpers.NULL_HASH); + var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[value],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); //transfer some eth to avatar await web3.eth.sendTransaction({from:accounts[0],to:testSetup.org.avatar.address, value: web3.utils.toWei('1', "ether")}); assert.equal(await web3.eth.getBalance(actionMock.address),0); await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); try { - await testSetup.GenericSchemeMultiCall.execute(proposalId); + await testSetup.genericSchemeMultiCall.execute(proposalId); assert(false, "cannot transfer eth amount"); } catch(error) { helpers.assertVMException(error); } }); + it("schemeconstrains token value exceed limit", async function() { + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[standardTokenMock.address,accounts[3]],0,true,standardTokenMock.address); + var encodedTokenApproval = await createCallToTokenApproval(standardTokenMock,accounts[3], 10001); + var tx = await testSetup.genericSchemeMultiCall.proposeCalls([standardTokenMock.address],[encodedTokenApproval],[0],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + try { + await testSetup.genericSchemeMultiCall.execute(proposalId); + assert(false, "periodSpendingTokensExceeded"); + } catch(error) { + helpers.assertVMException(error); + } + }); + it("execute proposeVote -negative decision - check action - with GenesisProtocol", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[0],helpers.NULL_HASH); + var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); tx = await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,2,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { + await testSetup.genericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { fromBlock: tx.blockNumber, toBlock: 'latest' }) @@ -305,14 +320,14 @@ contract('GenericSchemeMultiCall', function(accounts) { var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); var callData2 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + var tx = await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock2.address], [callData1,callData2], [0,0], helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); tx = await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { + await testSetup.genericSchemeMultiCall.getPastEvents('ProposalExecutedByVotingMachine', { fromBlock: tx.blockNumber, toBlock: 'latest' }) @@ -329,18 +344,18 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[actionMock.address,actionMock2.address],0,true,standardTokenMock.address); var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); var callData2 = await createCallToActionMock(accounts[0],actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + var tx = await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock2.address], [callData1,callData2], [0,0], helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); + var proposal = await testSetup.genericSchemeMultiCall.proposals(proposalId); assert.equal(proposal.exist,true); assert.equal(proposal.passed,false); await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); try { - await testSetup.GenericSchemeMultiCall.execute(proposalId); + await testSetup.genericSchemeMultiCall.execute(proposalId); assert(false, "Proposal call failed"); } catch(error) { helpers.assertVMException(error); @@ -354,19 +369,19 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[actionMock.address,accounts[3],standardTokenMock.address],0,true,standardTokenMock.address); var encodedTokenApproval = await createCallToTokenApproval(standardTokenMock,accounts[3], 1000); var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + var tx = await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address,standardTokenMock.address], [callData1,encodedTokenApproval], [0,0], helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); + var proposal = await testSetup.genericSchemeMultiCall.proposals(proposalId); assert.equal(proposal.exist,true); assert.equal(proposal.passed,false); assert.equal(await standardTokenMock.allowance(testSetup.org.avatar.address,accounts[3]),0); await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - await testSetup.GenericSchemeMultiCall.execute(proposalId); - await testSetup.GenericSchemeMultiCall.getPastEvents('ProposalCallExecuted', { + await testSetup.genericSchemeMultiCall.execute(proposalId); + await testSetup.genericSchemeMultiCall.getPastEvents('ProposalCallExecuted', { fromBlock: tx.blockNumber, toBlock: 'latest' }) @@ -380,20 +395,20 @@ contract('GenericSchemeMultiCall', function(accounts) { }); it("cannot init twice", async function() { - var actionMock =await ActionMock.new(); - var testSetup = await setup(accounts,[actionMock.address]); - try { - await testSetup.GenericSchemeMultiCall.initialize( - testSetup.org.avatar.address, - accounts[0], - helpers.SOME_HASH, - testSetup.schemeConstraints.address - ); - assert(false, "cannot init twice"); - } catch(error) { - helpers.assertVMException(error); - } - }); + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[actionMock.address]); + try { + await testSetup.genericSchemeMultiCall.initialize( + testSetup.org.avatar.address, + accounts[0], + helpers.SOME_HASH, + testSetup.schemeConstraints.address + ); + assert(false, "cannot init twice"); + } catch(error) { + helpers.assertVMException(error); + } + }); it("can init with multiple contracts on whitelist", async function() { var actionMock =await ActionMock.new(); @@ -416,6 +431,65 @@ contract('GenericSchemeMultiCall', function(accounts) { }); + it("cannot initialize contraints with zero period", async function() { + var dxDaoSchemeConstraintsInit =await DxDaoSchemeConstraints.new(); + try { + await dxDaoSchemeConstraintsInit.initialize( + accounts[0], + 0, + 0, + [], + [], + [accounts[0]] + ); + assert(false, "preriod size should be greater than 0"); + } catch(error) { + helpers.assertVMException(error); + } + }); + + it("cannot initialize contraints with invalid array length", async function() { + var dxDaoSchemeConstraintsArray=await DxDaoSchemeConstraints.new(); + try { + await dxDaoSchemeConstraintsArray.initialize( + accounts[0], + 0, + 0, + [accounts[0]], + [100,100], + [accounts[0]] + ); + assert(false, "invalid length _periodLimitTokensAddresses"); + } catch(error) { + helpers.assertVMException(error); + } + }); + + it("cannot initialize contraints twice", async function() { + var dxDaoSchemeConstraintsDouble=await DxDaoSchemeConstraints.new(); + await dxDaoSchemeConstraintsDouble.initialize( + accounts[0], + 3, + 0, + [], + [], + [accounts[0]] + ); + try { + await dxDaoSchemeConstraintsDouble.initialize( + accounts[0], + 3, + 0, + [], + [], + [accounts[0]] + ); + assert(false, "cannot initialize twice"); + } catch(error) { + helpers.assertVMException(error); + } + }); + it("execute proposeVote with multiple calls with votingMachine without whitelisted token", async function() { var actionMock =await ActionMock.new(); var standardTokenMock = await ERC20Mock.new(accounts[0],1000); @@ -423,7 +497,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); try { - await testSetup.GenericSchemeMultiCall.proposeCalls( + await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address], [callData1,encodedTokenApproval], [0,0], @@ -441,7 +515,25 @@ contract('GenericSchemeMultiCall', function(accounts) { var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); try { - await testSetup.GenericSchemeMultiCall.proposeCalls( + await testSetup.genericSchemeMultiCall.proposeCalls( + [actionMock.address], + [callData1,encodedTokenApproval], + [0,0], + helpers.NULL_HASH); + assert(false, "spender contract not whitelisted"); + } catch(error) { + helpers.assertVMException(error); + } + }); + + it("execute proposeVote with multiple calls with votingMachine without whitelisted spender", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); + var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); + var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + try { + await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address], [callData1,encodedTokenApproval], [0,0], @@ -458,7 +550,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address,false); var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - await testSetup.GenericSchemeMultiCall.proposeCalls( + await testSetup.genericSchemeMultiCall.proposeCalls( [actionMock.address,actionMock.address], [callData1,encodedTokenApproval], [0,0], @@ -471,20 +563,20 @@ contract('GenericSchemeMultiCall', function(accounts) { var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address,false); var value = 100001; var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[value],helpers.NULL_HASH); + var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address],[callData],[value],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); //transfer some eth to avatar await web3.eth.sendTransaction({from:accounts[0],to:testSetup.org.avatar.address, value: web3.utils.toWei('1', "ether")}); assert.equal(await web3.eth.getBalance(actionMock.address),0); await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - await testSetup.GenericSchemeMultiCall.execute(proposalId); + await testSetup.genericSchemeMultiCall.execute(proposalId); }); it("execute none exist proposal", async function() { var actionMock =await ActionMock.new(); var testSetup = await setup(accounts,[actionMock.address]); try { - await testSetup.GenericSchemeMultiCall.execute(helpers.SOME_HASH); + await testSetup.genericSchemeMultiCall.execute(helpers.SOME_HASH); assert(false, "cannot execute none exist proposal"); } catch(error) { helpers.assertVMException(error); From d1d85ce22272cba76c1bc868e7acafbee3b302fd Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Fri, 16 Oct 2020 00:46:17 +0300 Subject: [PATCH 26/28] only genericSchemeMultiCall can call isAllowToCall test coverage --- contracts/schemes/DxDaoSchemeConstraints.sol | 8 +- test/genericschememulticall.js | 187 ++++++++++++++----- 2 files changed, 143 insertions(+), 52 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index 477294c0..4424f93e 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -21,6 +21,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { ); address public avatar; + address public genericSchemeMultiCall; uint256 public initialTimestamp; uint256 public periodSize; uint256 public periodLimitWei; @@ -45,13 +46,15 @@ contract DxDaoSchemeConstraints is SchemeConstraints { uint256 _periodLimitWei, address[] calldata _periodLimitTokensAddresses, uint256[] calldata _periodLimitTokensAmounts, - address[] calldata _contractsWhiteList + address[] calldata _contractsWhiteList, + address _genericSchemeMultiCall ) external { require(initialTimestamp == 0, "cannot initialize twice"); require(_periodSize > 0, "preriod size should be greater than 0"); require(_periodLimitTokensAddresses.length == _periodLimitTokensAmounts.length, "invalid length _periodLimitTokensAddresses"); + require(_genericSchemeMultiCall != address(0), "genericSchemeMultiCall cannot be zero"); periodSize = _periodSize; periodLimitWei = _periodLimitWei; avatar = _avatar; @@ -64,6 +67,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { periodLimitToken[_periodLimitTokensAddresses[i]] = _periodLimitTokensAmounts[i]; } contractsWhiteList = _contractsWhiteList; + genericSchemeMultiCall = _genericSchemeMultiCall; } /* @@ -133,7 +137,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { external returns(bool) { - + require(msg.sender == genericSchemeMultiCall, "only genericSchemeMultiCall"); uint256 observervationIndex = observationIndex(); uint256 totalPeriodSpendingInWei; for (uint i = 0; i < _contractsToCall.length; i++) { diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index cbdccc33..a5b91752 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -63,7 +63,14 @@ const setup = async function (accounts, if (useSchemeConstraint) { testSetup.schemeConstraints = await DxDaoSchemeConstraints.new(); schemeConstraintsAddress = testSetup.schemeConstraints.address; - await testSetup.schemeConstraints.initialize(testSetup.org.avatar.address,100000,100000,[tokenAddress],[1000],contractsWhiteList); + //use accounts[4] as the avatar. + await testSetup.schemeConstraints.initialize(accounts[4], + 100000, + 100000, + [tokenAddress], + [1000], + contractsWhiteList, + testSetup.genericSchemeMultiCall.address); } else { schemeConstraintsAddress = helpers.NULL_ADDRESS; } @@ -279,18 +286,18 @@ contract('GenericSchemeMultiCall', function(accounts) { }); it("schemeconstrains token value exceed limit", async function() { - var standardTokenMock = await ERC20Mock.new(accounts[0],1000); - var testSetup = await setup(accounts,[standardTokenMock.address,accounts[3]],0,true,standardTokenMock.address); - var encodedTokenApproval = await createCallToTokenApproval(standardTokenMock,accounts[3], 10001); - var tx = await testSetup.genericSchemeMultiCall.proposeCalls([standardTokenMock.address],[encodedTokenApproval],[0],helpers.NULL_HASH); - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); - await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - try { - await testSetup.genericSchemeMultiCall.execute(proposalId); - assert(false, "periodSpendingTokensExceeded"); - } catch(error) { - helpers.assertVMException(error); - } + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[standardTokenMock.address,accounts[3]],0,true,standardTokenMock.address); + var encodedTokenApproval = await createCallToTokenApproval(standardTokenMock,accounts[3], 10001); + var tx = await testSetup.genericSchemeMultiCall.proposeCalls([standardTokenMock.address],[encodedTokenApproval],[0],helpers.NULL_HASH); + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + try { + await testSetup.genericSchemeMultiCall.execute(proposalId); + assert(false, "periodSpendingTokensExceeded"); + } catch(error) { + helpers.assertVMException(error); + } }); it("execute proposeVote -negative decision - check action - with GenesisProtocol", async function() { @@ -415,13 +422,29 @@ contract('GenericSchemeMultiCall', function(accounts) { var standardTokenMock = await ERC20Mock.new(accounts[0],1000); var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); + try { + await dxDaoSchemeConstraints.initialize( + testSetup.org.avatar.address, + 1, + 0, + [], + [], + [accounts[0],accounts[1],accounts[2],accounts[3]], + helpers.NULL_ADDRESS + ); + assert(false, "cannot init with zero genericSchemeMultiCall"); + } catch(error) { + helpers.assertVMException(error); + } + await dxDaoSchemeConstraints.initialize( testSetup.org.avatar.address, 1, 0, [], [], - [accounts[0],accounts[1],accounts[2],accounts[3]] + [accounts[0],accounts[1],accounts[2],accounts[3]], + testSetup.genericSchemeMultiCall.address ); var contractsWhiteList = await dxDaoSchemeConstraints.getContractsWhiteList(); assert.equal(contractsWhiteList[0],accounts[0]); @@ -440,7 +463,9 @@ contract('GenericSchemeMultiCall', function(accounts) { 0, [], [], - [accounts[0]] + [accounts[0]], + accounts[0] + ); assert(false, "preriod size should be greater than 0"); } catch(error) { @@ -449,15 +474,16 @@ contract('GenericSchemeMultiCall', function(accounts) { }); it("cannot initialize contraints with invalid array length", async function() { - var dxDaoSchemeConstraintsArray=await DxDaoSchemeConstraints.new(); + var dxDaoSchemeConstraintsArray = await DxDaoSchemeConstraints.new(); try { await dxDaoSchemeConstraintsArray.initialize( accounts[0], - 0, + 1, 0, [accounts[0]], [100,100], - [accounts[0]] + [accounts[0]], + accounts[0] ); assert(false, "invalid length _periodLimitTokensAddresses"); } catch(error) { @@ -473,7 +499,8 @@ contract('GenericSchemeMultiCall', function(accounts) { 0, [], [], - [accounts[0]] + [accounts[0]], + accounts[0] ); try { await dxDaoSchemeConstraintsDouble.initialize( @@ -482,7 +509,8 @@ contract('GenericSchemeMultiCall', function(accounts) { 0, [], [], - [accounts[0]] + [accounts[0]], + accounts[0] ); assert(false, "cannot initialize twice"); } catch(error) { @@ -516,25 +544,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); try { await testSetup.genericSchemeMultiCall.proposeCalls( - [actionMock.address], - [callData1,encodedTokenApproval], - [0,0], - helpers.NULL_HASH); - assert(false, "spender contract not whitelisted"); - } catch(error) { - helpers.assertVMException(error); - } - }); - - it("execute proposeVote with multiple calls with votingMachine without whitelisted spender", async function() { - var actionMock =await ActionMock.new(); - var standardTokenMock = await ERC20Mock.new(accounts[0],1000); - var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); - var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); - var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - try { - await testSetup.genericSchemeMultiCall.proposeCalls( - [actionMock.address], + [actionMock.address,actionMock.address], [callData1,encodedTokenApproval], [0,0], helpers.NULL_HASH); @@ -595,7 +605,8 @@ contract('GenericSchemeMultiCall', function(accounts) { 0, [], [], - [accounts[0]] + [accounts[0]], + testSetup.genericSchemeMultiCall.address ); try { await dxDaoSchemeConstraints.updateContractsWhitelist([actionMock.address],[true]); @@ -610,7 +621,7 @@ contract('GenericSchemeMultiCall', function(accounts) { } catch(error) { helpers.assertVMException(error); } - + try { await dxDaoSchemeConstraints.updatePeriodLimitWei(5000); assert(false, "caller must be avatar"); @@ -625,7 +636,8 @@ contract('GenericSchemeMultiCall', function(accounts) { 0, [], [], - [accounts[0]] + [accounts[0]], + testSetup.genericSchemeMultiCall.address ); await dxDaoSchemeConstraints.updatePeriodLimitWei(10000,{from:accounts[3]}); await dxDaoSchemeConstraints.getPastEvents('UpdatedPeriodLimitWei', { @@ -636,7 +648,7 @@ contract('GenericSchemeMultiCall', function(accounts) { assert.equal(events[0].event,"UpdatedPeriodLimitWei"); assert.equal(events[0].args._periodLimitWei,10000); }); - + await dxDaoSchemeConstraints.updatePeriodLimitsTokens([standardTokenMock.address],[10000],{from:accounts[3]}); await dxDaoSchemeConstraints.getPastEvents('UpdatedPeriodLimitsTokens', { fromBlock: 0, @@ -647,7 +659,7 @@ contract('GenericSchemeMultiCall', function(accounts) { assert.equal(events[0].args._tokensAddresses[0],standardTokenMock.address); assert.equal(events[0].args._tokensPeriodLimits[0],10000); }); - + await dxDaoSchemeConstraints.updateContractsWhitelist([standardTokenMock.address],[true],{from:accounts[3]}); await dxDaoSchemeConstraints.getPastEvents('UpdatedContractsWhitelist', { fromBlock: 0, @@ -675,6 +687,62 @@ contract('GenericSchemeMultiCall', function(accounts) { }); + it("update contraints whitelist after proposal call and before execute and check spender", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address,accounts[3],standardTokenMock.address],0,true,standardTokenMock.address); + var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); + var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + + var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address,standardTokenMock.address], + [callData1,encodedTokenApproval], + [0,0], + helpers.NULL_HASH); + + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + + await testSetup.schemeConstraints.updateContractsWhitelist([accounts[3]],[false],{from:accounts[4]}); + + await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + + try { + await testSetup.genericSchemeMultiCall.execute(proposalId); + assert(false, "contract was blacklisted"); + } catch(error) { + helpers.assertVMException(error); + } + await testSetup.schemeConstraints.updateContractsWhitelist([accounts[3]],[true],{from:accounts[4]}); + await testSetup.genericSchemeMultiCall.execute(proposalId); +}); + + it("update contraints whitelist after proposal call and before execute", async function() { + var actionMock =await ActionMock.new(); + var standardTokenMock = await ERC20Mock.new(accounts[0],1000); + var testSetup = await setup(accounts,[actionMock.address,accounts[3],standardTokenMock.address],0,true,standardTokenMock.address); + var encodedTokenApproval= await createCallToTokenApproval(standardTokenMock, accounts[3], 1000); + var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + + var tx = await testSetup.genericSchemeMultiCall.proposeCalls([actionMock.address,standardTokenMock.address], + [callData1,encodedTokenApproval], + [0,0], + helpers.NULL_HASH); + + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); + + await testSetup.schemeConstraints.updateContractsWhitelist([actionMock.address],[false],{from:accounts[4]}); + await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + + try { + await testSetup.genericSchemeMultiCall.execute(proposalId); + assert(false, "contract was blacklisted"); + } catch(error) { + helpers.assertVMException(error); + } + await testSetup.schemeConstraints.updateContractsWhitelist([actionMock.address],[true],{from:accounts[4]}); + await testSetup.genericSchemeMultiCall.execute(proposalId); +}); + + it("can only update contraint whitelist & limits from avatar with correct array length", async function() { var standardTokenMock = await ERC20Mock.new(accounts[0],1000); var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); @@ -698,7 +766,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var standardTokenMock = await ERC20Mock.new(accounts[0],1000); var testSetup = await setup(accounts,[actionMock.address],0,true,standardTokenMock.address); var dxDaoSchemeConstraints =await DxDaoSchemeConstraints.new(); - + // 10 seconds period await dxDaoSchemeConstraints.initialize( testSetup.org.avatar.address, @@ -706,19 +774,20 @@ contract('GenericSchemeMultiCall', function(accounts) { 0, [], [], - [accounts[0]] + [accounts[0]], + testSetup.genericSchemeMultiCall.address ); - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),0); + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),0); await helpers.increaseTime(10); assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),1); await helpers.increaseTime(3600); // adding 1 hour assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),361); await helpers.increaseTime(86400); // adding 1 day - assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),9001); + assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),9001); await helpers.increaseTime(315360000);// adding 10 year assert.equal((await dxDaoSchemeConstraints.observationIndex()).toString(),31545001); var dxDaoSchemeConstraints2 =await DxDaoSchemeConstraints.new(); - + // 7 days period await dxDaoSchemeConstraints2.initialize( testSetup.org.avatar.address, @@ -726,7 +795,8 @@ contract('GenericSchemeMultiCall', function(accounts) { 0, [], [], - [accounts[0]] + [accounts[0]], + testSetup.genericSchemeMultiCall.address ); assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),0); await helpers.increaseTime(50); @@ -751,4 +821,21 @@ contract('GenericSchemeMultiCall', function(accounts) { assert.equal((await dxDaoSchemeConstraints2.observationIndex()).toString(),1100); }); + it("only genericSchemeMultiCall allow to call isAllowedToCall", async function() { + var actionMock =await ActionMock.new(); + var testSetup = await setup(accounts,[actionMock.address]); + const encodeABI = await new web3.eth.Contract(actionMock.abi).methods.withoutReturnValue(testSetup.org.avatar.address).encodeABI(); + + try { + await testSetup.schemeConstraints.isAllowedToCall( + [actionMock.address], + [encodeABI], + [1], + accounts[4]); + assert(false, "only genericSchemeMultiCall allow to call isAllowedToCall"); + } catch(error) { + helpers.assertVMException(error); + } + }); + }); From 924f713481524a143b0f2076d24e2bb32d8c685a Mon Sep 17 00:00:00 2001 From: Oren Sokolowsky Date: Fri, 16 Oct 2020 00:49:46 +0300 Subject: [PATCH 27/28] comments --- contracts/schemes/DxDaoSchemeConstraints.sol | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/contracts/schemes/DxDaoSchemeConstraints.sol b/contracts/schemes/DxDaoSchemeConstraints.sol index 4424f93e..87dea875 100644 --- a/contracts/schemes/DxDaoSchemeConstraints.sol +++ b/contracts/schemes/DxDaoSchemeConstraints.sol @@ -7,12 +7,12 @@ contract DxDaoSchemeConstraints is SchemeConstraints { using SafeMath for uint256; event UpdatedContractsWhitelist( - address[] _contractsAddresses, + address[] _contractsAddresses, bool[] _contractsWhitelisted ); event UpdatedPeriodLimitsTokens( - address[] _tokensAddresses, + address[] _tokensAddresses, uint256[] _tokensPeriodLimits ); @@ -39,6 +39,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { * @param _periodLimitTokensAddresses tokens to limit * @param _periodLimitTokensAmounts the limit of token which can be sent per period * @param _contractsWhiteList the contracts the scheme is allowed to interact with + * @param _genericSchemeMultiCall genericSchemeMultiCall which allowed to call isAllowedToCall */ function initialize( address _avatar, @@ -76,7 +77,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { * @param _contractsWhitelisted – true adds a contract to the whitelist, false removes it. */ function updateContractsWhitelist( - address[] calldata _contractsAddresses, + address[] calldata _contractsAddresses, bool[] calldata _contractsWhitelisted ) external { @@ -95,7 +96,7 @@ contract DxDaoSchemeConstraints is SchemeConstraints { * @param _tokensPeriodLimits – The token amounts to be set as period limit. */ function updatePeriodLimitsTokens( - address[] calldata _tokensAddresses, + address[] calldata _tokensAddresses, uint256[] calldata _tokensPeriodLimits ) external { From 3f6c82adc2227ee9317f7a26d5ddd2d727f7a00f Mon Sep 17 00:00:00 2001 From: Nico Elzer Date: Fri, 16 Oct 2020 10:19:25 +0200 Subject: [PATCH 28/28] Added additional test for GenericSchemeMultiCall --- test/genericschememulticall.js | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index a5b91752..8e48384a 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -415,7 +415,23 @@ contract('GenericSchemeMultiCall', function(accounts) { } catch(error) { helpers.assertVMException(error); } - }); + }); + + it("cannot init with invalid avatar address", async function() { + var genericSchemeMultiCallInitAvatar = await GenericSchemeMultiCall.new(); + try { + await genericSchemeMultiCallInitAvatar.initialize( + helpers.NULL_ADDRESS, + accounts[0], + helpers.SOME_HASH, + accounts[0] + ); + assert(false, "avatar cannot be zero"); + } catch(error) { + helpers.assertVMException(error); + } + }); + it("can init with multiple contracts on whitelist", async function() { var actionMock =await ActionMock.new(); @@ -713,7 +729,7 @@ contract('GenericSchemeMultiCall', function(accounts) { } await testSetup.schemeConstraints.updateContractsWhitelist([accounts[3]],[true],{from:accounts[4]}); await testSetup.genericSchemeMultiCall.execute(proposalId); -}); + }); it("update contraints whitelist after proposal call and before execute", async function() { var actionMock =await ActionMock.new(); @@ -740,7 +756,7 @@ contract('GenericSchemeMultiCall', function(accounts) { } await testSetup.schemeConstraints.updateContractsWhitelist([actionMock.address],[true],{from:accounts[4]}); await testSetup.genericSchemeMultiCall.execute(proposalId); -}); + }); it("can only update contraint whitelist & limits from avatar with correct array length", async function() {