diff --git a/contracts/libs/BytesLib.sol b/contracts/libs/BytesLib.sol new file mode 100644 index 00000000..c4d3557d --- /dev/null +++ b/contracts/libs/BytesLib.sol @@ -0,0 +1,79 @@ +/* + * @title Solidity Bytes Arrays Utils + * @author Gonçalo Sá + * + * @dev Bytes tightly packed arrays utility library for ethereum contracts written in Solidity. + * The library lets you concatenate, slice and type cast bytes arrays both in memory and storage. + */ + +pragma solidity 0.5.17; + + +library BytesLib { + + function slice( + bytes memory _bytes, + uint _start, + uint _length + ) + internal + pure + returns (bytes memory) + // solhint-disable-next-line function-max-lines + { + require(_bytes.length >= (_start + _length)); + + bytes memory tempBytes; + // solhint-disable-next-line no-inline-assembly + assembly { + switch iszero(_length) + case 0 { + // Get a location of some free memory and store it in tempBytes as + // Solidity does for memory variables. + tempBytes := mload(0x40) + + // The first word of the slice result is potentially a partial + // word read from the original array. To read it, we calculate + // the length of that partial word and start copying that many + // bytes into the array. The first word we copy will start with + // data we don't care about, but the last `lengthmod` bytes will + // land at the beginning of the contents of the new array. When + // we're done copying, we overwrite the full first word with + // the actual length of the slice. + let lengthmod := and(_length, 31) + + // The multiplication in the next line is necessary + // because when slicing multiples of 32 bytes (lengthmod == 0) + // the following copy loop was copying the origin's length + // and then ending prematurely not copying everything it should. + let mc := add(add(tempBytes, lengthmod), mul(0x20, iszero(lengthmod))) + let end := add(mc, _length) + + for { + // The multiplication in the next line has the same exact purpose + // as the one above. + let cc := add(add(add(_bytes, lengthmod), mul(0x20, iszero(lengthmod))), _start) + } lt(mc, end) { + mc := add(mc, 0x20) + cc := add(cc, 0x20) + } { + mstore(mc, mload(cc)) + } + + mstore(tempBytes, _length) + + //update free-memory pointer + //allocating the array padded to 32 bytes like the compiler does now + mstore(0x40, and(add(mc, 31), not(31))) + } + //if we want a zero-length slice let's just return a zero-length array + default { + tempBytes := mload(0x40) + + mstore(0x40, add(tempBytes, 0x20)) + } + } + + return tempBytes; + } +} diff --git a/contracts/schemes/GenericSchemeMultiCall.sol b/contracts/schemes/GenericSchemeMultiCall.sol index 52d5a806..cea876b9 100644 --- a/contracts/schemes/GenericSchemeMultiCall.sol +++ b/contracts/schemes/GenericSchemeMultiCall.sol @@ -1,9 +1,10 @@ pragma solidity 0.5.17; -pragma experimental ABIEncoderV2; import "@daostack/infra/contracts/votingMachines/IntVoteInterface.sol"; import "@daostack/infra/contracts/votingMachines/ProposalExecuteInterface.sol"; import "../votingMachines/VotingMachineCallbacks.sol"; +import "../libs/BytesLib.sol"; + /** * @title GenericSchemeMultiCall. @@ -11,11 +12,14 @@ import "../votingMachines/VotingMachineCallbacks.sol"; * on one or multiple contracts on behalf of the organization avatar. */ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterface { + using BytesLib for bytes; + using SafeMath for uint256; // Details of a voting proposal: struct MultiCallProposal { address[] contractsToCall; - bytes[] callsData; + bytes callsData; + uint256[] callsDataLens; uint256[] values; bool exist; bool passed; @@ -32,7 +36,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf event NewMultiCallProposal( address indexed _avatar, bytes32 indexed _proposalId, - bytes[] _callsData, + bytes _callsData, uint256[] _values, string _descriptionHash, address[] _contractsToCall @@ -47,6 +51,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf address indexed _avatar, bytes32 indexed _proposalId, address _contractToCall, + bytes _callsData, bool _success, bytes _callDataReturnValue ); @@ -65,7 +70,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf * @param _votingMachine the voting machines address to * @param _voteParams voting machine parameters. * @param _contractWhitelist the contracts the scheme is allowed to interact with - * + * */ function initialize( Avatar _avatar, @@ -85,7 +90,7 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf 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]); @@ -130,27 +135,30 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf bytes memory genericCallReturnValue; bool success; Controller controller = Controller(whitelistedContracts[0]); + uint256 startIndex = 0; for (uint i = 0; i < proposal.contractsToCall.length; i++) { + bytes memory callData = proposal.callsData.slice(startIndex, proposal.callsDataLens[i]); if (proposal.contractsToCall[i] == address(controller)) { (IERC20 extToken, address spender, uint256 valueToSpend ) = abi.decode( - proposal.callsData[i], + callData, (IERC20, address, uint256) ); (success) = controller.externalTokenApproval(extToken, spender, valueToSpend, avatar); } else { (success, genericCallReturnValue) = - controller.genericCall(proposal.contractsToCall[i], proposal.callsData[i], avatar, proposal.values[i]); + controller.genericCall(proposal.contractsToCall[i], callData, avatar, proposal.values[i]); } - + startIndex = startIndex.add(proposal.callsDataLens[i]); emit ProposalCallExecuted( address(avatar), _proposalId, proposal.contractsToCall[i], + callData, success, genericCallReturnValue ); @@ -164,15 +172,17 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf /** * @dev propose to call one or multiple contracts on behalf of the _avatar * The function trigger NewMultiCallProposal event - * @param _contractsToCall the contracts to be called + * @param _contractsToCall the contracts to be called * @param _callsData - The abi encode data for the calls + * @param _callsDataLens the length of each callData * @param _values value(ETH) to transfer with the calls * @param _descriptionHash proposal description hash * @return an id which represents the proposal */ function proposeCalls( address[] memory _contractsToCall, - bytes[] memory _callsData, + bytes memory _callsData, + uint256[] memory _callsDataLens, uint256[] memory _values, string memory _descriptionHash ) @@ -180,31 +190,33 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf returns(bytes32 proposalId) { require( - (_contractsToCall.length == _callsData.length) && (_contractsToCall.length == _values.length), - "Wrong length of _contractsToCall, _callsData or _values arrays" + (_contractsToCall.length == _callsDataLens.length) && (_contractsToCall.length == _values.length), + "Wrong length of _contractsToCall, _callsDataLens or _values arrays" ); Controller controller = Controller(whitelistedContracts[0]); + uint256 startIndex = 0; for (uint i = 0; i < _contractsToCall.length; i++) { require( contractWhitelist[_contractsToCall[i]], "contractToCall is not whitelisted" ); if (_contractsToCall[i] == address(controller)) { - (IERC20 extToken, - address spender, - uint256 valueToSpend - ) = + + bytes memory callData = _callsData.slice(startIndex, _callsDataLens[i]); + (, address spender,) = abi.decode( - _callsData[i], + callData, (IERC20, address, uint256) ); require(contractWhitelist[spender], "spender contract not whitelisted"); } + startIndex = startIndex.add(_callsDataLens[i]); } proposalId = votingMachine.propose(2, voteParams, msg.sender, address(avatar)); proposals[proposalId] = MultiCallProposal({ contractsToCall: _contractsToCall, callsData: _callsData, + callsDataLens: _callsDataLens, values: _values, exist: true, passed: false @@ -217,4 +229,4 @@ contract GenericSchemeMultiCall is VotingMachineCallbacks, ProposalExecuteInterf emit NewMultiCallProposal(address(avatar), proposalId, _callsData, _values, _descriptionHash, _contractsToCall); } -} \ No newline at end of file +} diff --git a/package-lock.json b/package-lock.json index 2db2731f..1bb6dec1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@daostack/arc", - "version": "0.0.1-rc.44", + "version": "0.0.1-rc.45", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/test/genericschememulticall.js b/test/genericschememulticall.js index 62bc15db..4387fd06 100644 --- a/test/genericschememulticall.js +++ b/test/genericschememulticall.js @@ -12,6 +12,14 @@ export class GenericSchemeParams { } } +const getBytesLength = function (bytes) { + return Number(web3.utils.toBN(Number(bytes.slice(2).length) / 2)); +}; + +const concatBytes = function (bytes1, bytes2) { + return bytes1 + (bytes2.slice(2)); +}; + const setupGenericSchemeParams = async function( genericScheme, accounts, @@ -78,7 +86,7 @@ contract('GenericSchemeMultiCall', function(accounts) { 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); + [actionMock.address],callData,[getBytesLength(callData)],[0],helpers.NULL_HASH); assert.equal(tx.logs.length, 1); assert.equal(tx.logs[0].event, "NewMultiCallProposal"); }); @@ -89,22 +97,22 @@ contract('GenericSchemeMultiCall', function(accounts) { 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, _callData or _value arrays"); + [actionMock.address,actionMock.address],callData,[getBytesLength(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,callData],[0],helpers.NULL_HASH); - assert(false, "Wrong length of _contractsToCall, _callData or _value arrays"); + [actionMock.address,actionMock.address],callData,[getBytesLength(callData),getBytesLength(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, _callData or _value arrays"); + [actionMock.address,actionMock.address],callData,[getBytesLength(callData)],[0,0],helpers.NULL_HASH); + assert(false, "Wrong length of _contractsToCall, _callsDataLens or _value arrays"); } catch(error) { helpers.assertVMException(error); } @@ -115,7 +123,7 @@ contract('GenericSchemeMultiCall', function(accounts) { 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); + [actionMock.address],callData,[getBytesLength(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 @@ -129,7 +137,7 @@ contract('GenericSchemeMultiCall', function(accounts) { 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); + [actionMock.address],callData,[getBytesLength(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]}); @@ -143,7 +151,7 @@ contract('GenericSchemeMultiCall', function(accounts) { 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); + [actionMock.address],callData,[getBytesLength(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]}); @@ -163,7 +171,7 @@ contract('GenericSchemeMultiCall', function(accounts) { var callData = await createCallToActionMock(helpers.NULL_ADDRESS,actionMock); try { await testSetup.GenericSchemeMultiCall.proposeCalls( - [actionMock.address],[callData],[0],helpers.NULL_HASH); + [actionMock.address],callData,[getBytesLength(callData)],[0],helpers.NULL_HASH); assert(false, "contractToCall is not whitelisted"); } catch(error) { helpers.assertVMException(error); @@ -174,7 +182,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,[getBytesLength(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]}); }); @@ -183,7 +191,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,[getBytesLength(encodeABI)],[0],helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); try { await testSetup.GenericSchemeMultiCall.execute( proposalId); @@ -200,7 +208,7 @@ contract('GenericSchemeMultiCall', function(accounts) { 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 tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address],callData,[getBytesLength(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")}); @@ -223,7 +231,7 @@ contract('GenericSchemeMultiCall', function(accounts) { 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,[getBytesLength(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', { @@ -242,9 +250,15 @@ contract('GenericSchemeMultiCall', function(accounts) { var standardTokenMock = await ERC20Mock.new(accounts[0],1000); var testSetup = await setup(accounts,[actionMock.address,actionMock2.address],0,true,standardTokenMock.address); - var callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + 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],[callData,callData2],[0,0],helpers.NULL_HASH); + var callData = concatBytes(callData1,callData2); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address,actionMock2.address], + callData, + [getBytesLength(callData1),getBytesLength(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', { @@ -262,9 +276,15 @@ contract('GenericSchemeMultiCall', function(accounts) { 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 callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + 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],[callData,callData2],[0,0],helpers.NULL_HASH); + var callData = concatBytes(callData1,callData2); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address,actionMock2.address], + callData, + [getBytesLength(callData1),getBytesLength(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); @@ -289,9 +309,15 @@ contract('GenericSchemeMultiCall', function(accounts) { 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 callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var callData = concatBytes(callData1,encodedTokenApproval); try { - await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address,controllerAddr],[callData,encodedTokenApproval],[0,0],helpers.NULL_HASH); + await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address,controllerAddr], + callData, + [getBytesLength(callData1),getBytesLength(encodedTokenApproval)], + [0,0], + helpers.NULL_HASH); assert(false, "spender contract not whitelisted"); } catch(error) { helpers.assertVMException(error); @@ -305,8 +331,14 @@ contract('GenericSchemeMultiCall', function(accounts) { 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 callData = await createCallToActionMock(testSetup.org.avatar.address,actionMock); - var tx = await testSetup.GenericSchemeMultiCall.proposeCalls([actionMock.address,controllerAddr],[callData,encodedTokenApproval],[0,0],helpers.NULL_HASH); + var callData1 = await createCallToActionMock(testSetup.org.avatar.address,actionMock); + var callData = concatBytes(callData1,encodedTokenApproval); + var tx = await testSetup.GenericSchemeMultiCall.proposeCalls( + [actionMock.address,controllerAddr], + callData, + [getBytesLength(callData1),getBytesLength(encodedTokenApproval)], + [0,0], + helpers.NULL_HASH); var proposalId = await helpers.getValueFromLogs(tx, '_proposalId'); var proposal = await testSetup.GenericSchemeMultiCall.proposals(proposalId); assert.equal(proposal.exist,true);