From 43406fb6d4561c80104521ace7ba3c1369a48e27 Mon Sep 17 00:00:00 2001 From: Oren Date: Thu, 14 Mar 2019 08:33:31 +0200 Subject: [PATCH 1/3] VestingScheme : mint tokens only when collect or cancel. --- contracts/universalSchemes/VestingScheme.sol | 27 +++++-- test/vestingscheme.js | 76 ++++++++++++-------- 2 files changed, 67 insertions(+), 36 deletions(-) diff --git a/contracts/universalSchemes/VestingScheme.sol b/contracts/universalSchemes/VestingScheme.sol index d1c72fae..66581aff 100644 --- a/contracts/universalSchemes/VestingScheme.sol +++ b/contracts/universalSchemes/VestingScheme.sol @@ -39,6 +39,8 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu uint256 signaturesReqToCancel; uint256 collectedPeriods; uint256 signaturesReceivedCounter; + Avatar avatar; + uint256 totalAmount; mapping(address=>bool) signers; mapping(address=>bool) signaturesReceived; } @@ -87,9 +89,8 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu // Check if vote was successful: if (_param == 1) { // Define controller and mint tokens, check minting actually took place: - ControllerInterface controller = ControllerInterface(avatar.owner()); - uint256 tokensToMint = proposedAgreement.amountPerPeriod.mul(proposedAgreement.numOfAgreedPeriods); - require(controller.mintTokens(tokensToMint, address(this), address(avatar))); + proposedAgreement.totalAmount = proposedAgreement.amountPerPeriod.mul(proposedAgreement.numOfAgreedPeriods); + proposedAgreement.avatar = avatar; agreements[agreementsCounter] = proposedAgreement; agreementsCounter++; // Log the new agreement: @@ -192,6 +193,7 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu address(_token).safeTransferFrom(msg.sender, address(this), totalAmount); // Write parameters: + agreements[agreementsCounter].totalAmount = totalAmount; agreements[agreementsCounter].token = _token; agreements[agreementsCounter].beneficiary = _beneficiary; agreements[agreementsCounter].returnOnCancelAddress = _returnOnCancelAddress; @@ -306,8 +308,14 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu // Transfer: uint256 tokensToTransfer = periodsToPay.mul(agreement.amountPerPeriod); - address(agreement.token).safeTransfer(agreement.beneficiary, tokensToTransfer); - + agreement.totalAmount = agreement.totalAmount.sub(tokensToTransfer); + if (agreement.avatar != Avatar(0)) { + ControllerInterface controller = ControllerInterface(agreement.avatar.owner()); + require(controller.mintTokens(tokensToTransfer, agreement.beneficiary, address(agreement.avatar))); + } else { + //the agreement was created directly. not via the DAO. + address(agreement.token).safeTransfer(agreement.beneficiary, tokensToTransfer); + } // Log collecting: emit Collect(_agreementId); } @@ -321,7 +329,14 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu delete agreements[_agreementId]; uint256 periodsLeft = agreement.numOfAgreedPeriods.sub(agreement.collectedPeriods); uint256 tokensLeft = periodsLeft.mul(agreement.amountPerPeriod); - address(agreement.token).safeTransfer(agreement.returnOnCancelAddress, tokensLeft); + agreement.totalAmount = agreement.totalAmount.sub(tokensLeft); + if (agreement.avatar != Avatar(0)) { + ControllerInterface controller = ControllerInterface(agreement.avatar.owner()); + require(controller.mintTokens(tokensLeft, agreement.returnOnCancelAddress, address(agreement.avatar))); + } else { + //the agreement was created directly. not via the DAO. + address(agreement.token).safeTransfer(agreement.returnOnCancelAddress, tokensLeft); + } // Log canceling agreement: emit AgreementCancel(_agreementId); } diff --git a/test/vestingscheme.js b/test/vestingscheme.js index 0c5733a3..49c79b33 100644 --- a/test/vestingscheme.js +++ b/test/vestingscheme.js @@ -231,35 +231,38 @@ contract('VestingScheme', accounts => { assert.equal(organizationProposal[0],0x0000000000000000000000000000000000000000);//new contract address }); - it("execute proposeVestingAgreement controller -yes - check minting", async function() { - var testSetup = await setup(accounts); - var amountPerPeriod =3; - var numberOfAgreedPeriods = 7; - var blockNumber = await (web3.utils.toBN(await web3.eth.getBlockNumber())); - - - var tx = await testSetup.vestingScheme.proposeVestingAgreement(accounts[0], - accounts[1], - blockNumber, - amountPerPeriod, - 2, - numberOfAgreedPeriods, - 11, - 0, - [], - testSetup.org.avatar.address,helpers.NULL_HASH); - //Vote with reputation to trigger execution - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1); - //check organizationsProposals before execution - var organizationProposal = await testSetup.vestingScheme.organizationsProposals(testSetup.org.avatar.address,proposalId); - assert.equal(organizationProposal[0],testSetup.org.token.address); - assert.equal(await testSetup.org.token.balanceOf(testSetup.vestingScheme.address),0); - await testSetup.vestingSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - //check organizationsProposals after execution - organizationProposal = await testSetup.vestingScheme.organizationsProposals(testSetup.org.avatar.address,proposalId); - assert.equal(organizationProposal[0],0x0000000000000000000000000000000000000000);//new contract address - assert.equal(await testSetup.org.token.balanceOf(testSetup.vestingScheme.address),amountPerPeriod*numberOfAgreedPeriods); - }); + it("execute proposeVestingAgreement controller -yes - check set total amount", async function() { + var testSetup = await setup(accounts); + var amountPerPeriod =3; + var numberOfAgreedPeriods = 7; + var blockNumber = await (web3.utils.toBN(await web3.eth.getBlockNumber())); + + + var tx = await testSetup.vestingScheme.proposeVestingAgreement(accounts[0], + accounts[1], + blockNumber, + amountPerPeriod, + 2, + numberOfAgreedPeriods, + 11, + 0, + [], + testSetup.org.avatar.address,helpers.NULL_HASH); + //Vote with reputation to trigger execution + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1); + //check organizationsProposals before execution + var organizationProposal = await testSetup.vestingScheme.organizationsProposals(testSetup.org.avatar.address,proposalId); + assert.equal(organizationProposal[0],testSetup.org.token.address); + assert.equal(await testSetup.org.token.balanceOf(testSetup.vestingScheme.address),0); + await testSetup.vestingSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + //check organizationsProposals after execution + organizationProposal = await testSetup.vestingScheme.organizationsProposals(testSetup.org.avatar.address,proposalId); + assert.equal(organizationProposal[0],0x0000000000000000000000000000000000000000);//new contract address + var agreementsCounter = await testSetup.vestingScheme.agreementsCounter() - 1; + var agreement = await testSetup.vestingScheme.agreements(agreementsCounter); + assert.equal(agreement[11],testSetup.org.avatar.address);//avatar + assert.equal(agreement[12],amountPerPeriod*numberOfAgreedPeriods);//totalAmount + }); it("createVestedAgreement check agreement id ", async function() { var testSetup = await setup(accounts); @@ -285,6 +288,13 @@ contract('VestingScheme', accounts => { assert.equal(agreementId,0); blockNumber = await (web3.utils.toBN(await web3.eth.getBlockNumber())); + + var agreementsCounter = await testSetup.vestingScheme.agreementsCounter() - 1; + var agreement = await testSetup.vestingScheme.agreements(agreementsCounter); + assert.equal(agreement[11],0);//avatar + assert.equal(agreement[12],amountPerPeriod*numberOfAgreedPeriods);//totalAmount + + tx = await testSetup.vestingScheme.createVestedAgreement( testSetup.standardTokenMock.address, accounts[0], accounts[1], @@ -300,6 +310,12 @@ contract('VestingScheme', accounts => { assert.equal(tx.logs[0].event, "NewVestedAgreement"); agreementId = await helpers.getValueFromLogs(tx, '_agreementId',1); assert.equal(agreementId,1); + + agreementsCounter = await testSetup.vestingScheme.agreementsCounter() - 1; + agreement = await testSetup.vestingScheme.agreements(agreementsCounter); + assert.equal(agreement[11],0);//avatar + assert.equal(agreement[12],amountPerPeriod*numberOfAgreedPeriods);//totalAmount + }); it("createVestedAgreement check periodLength==0 ", async function() { @@ -914,7 +930,7 @@ contract('VestingScheme', accounts => { assert.equal(balance.toNumber(),numberOfAgreedPeriods*amountPerPeriod); } } - }); + }); }); From f92bf1b4728675a08d95af35dfec446c8a648136 Mon Sep 17 00:00:00 2001 From: Oren Date: Thu, 14 Mar 2019 10:10:46 +0200 Subject: [PATCH 2/3] Verify the agreement avatar is register as scheme --- contracts/universalSchemes/VestingScheme.sol | 31 ++++---- test/vestingscheme.js | 76 ++++++++------------ 2 files changed, 42 insertions(+), 65 deletions(-) diff --git a/contracts/universalSchemes/VestingScheme.sol b/contracts/universalSchemes/VestingScheme.sol index 66581aff..191d8aa9 100644 --- a/contracts/universalSchemes/VestingScheme.sol +++ b/contracts/universalSchemes/VestingScheme.sol @@ -40,7 +40,6 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu uint256 collectedPeriods; uint256 signaturesReceivedCounter; Avatar avatar; - uint256 totalAmount; mapping(address=>bool) signers; mapping(address=>bool) signaturesReceived; } @@ -89,7 +88,9 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu // Check if vote was successful: if (_param == 1) { // Define controller and mint tokens, check minting actually took place: - proposedAgreement.totalAmount = proposedAgreement.amountPerPeriod.mul(proposedAgreement.numOfAgreedPeriods); + ControllerInterface controller = ControllerInterface(avatar.owner()); + uint256 tokensToMint = proposedAgreement.amountPerPeriod.mul(proposedAgreement.numOfAgreedPeriods); + require(controller.mintTokens(tokensToMint, address(this), address(avatar))); proposedAgreement.avatar = avatar; agreements[agreementsCounter] = proposedAgreement; agreementsCounter++; @@ -193,7 +194,6 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu address(_token).safeTransferFrom(msg.sender, address(this), totalAmount); // Write parameters: - agreements[agreementsCounter].totalAmount = totalAmount; agreements[agreementsCounter].token = _token; agreements[agreementsCounter].beneficiary = _beneficiary; agreements[agreementsCounter].returnOnCancelAddress = _returnOnCancelAddress; @@ -293,6 +293,9 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu */ function collect(uint256 _agreementId) public onlyBeneficiary(_agreementId) { Agreement memory agreement = agreements[_agreementId]; + if (agreement.avatar != Avatar(0)) { + require(ControllerInterface(agreement.avatar.owner()).isSchemeRegistered(address(this), address(agreement.avatar))); + } uint256 periodsFromStartingBlock = (block.number.sub(agreement.startingBlock)).div(agreement.periodLength); require(periodsFromStartingBlock >= agreement.cliffInPeriods); @@ -308,14 +311,8 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu // Transfer: uint256 tokensToTransfer = periodsToPay.mul(agreement.amountPerPeriod); - agreement.totalAmount = agreement.totalAmount.sub(tokensToTransfer); - if (agreement.avatar != Avatar(0)) { - ControllerInterface controller = ControllerInterface(agreement.avatar.owner()); - require(controller.mintTokens(tokensToTransfer, agreement.beneficiary, address(agreement.avatar))); - } else { - //the agreement was created directly. not via the DAO. - address(agreement.token).safeTransfer(agreement.beneficiary, tokensToTransfer); - } + address(agreement.token).safeTransfer(agreement.beneficiary, tokensToTransfer); + // Log collecting: emit Collect(_agreementId); } @@ -327,16 +324,12 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu function cancelAgreement(uint256 _agreementId) internal { Agreement memory agreement = agreements[_agreementId]; delete agreements[_agreementId]; - uint256 periodsLeft = agreement.numOfAgreedPeriods.sub(agreement.collectedPeriods); - uint256 tokensLeft = periodsLeft.mul(agreement.amountPerPeriod); - agreement.totalAmount = agreement.totalAmount.sub(tokensLeft); if (agreement.avatar != Avatar(0)) { - ControllerInterface controller = ControllerInterface(agreement.avatar.owner()); - require(controller.mintTokens(tokensLeft, agreement.returnOnCancelAddress, address(agreement.avatar))); - } else { - //the agreement was created directly. not via the DAO. - address(agreement.token).safeTransfer(agreement.returnOnCancelAddress, tokensLeft); + require(ControllerInterface(agreement.avatar.owner()).isSchemeRegistered(address(this), address(agreement.avatar))); } + uint256 periodsLeft = agreement.numOfAgreedPeriods.sub(agreement.collectedPeriods); + uint256 tokensLeft = periodsLeft.mul(agreement.amountPerPeriod); + address(agreement.token).safeTransfer(agreement.returnOnCancelAddress, tokensLeft); // Log canceling agreement: emit AgreementCancel(_agreementId); } diff --git a/test/vestingscheme.js b/test/vestingscheme.js index 49c79b33..0c5733a3 100644 --- a/test/vestingscheme.js +++ b/test/vestingscheme.js @@ -231,38 +231,35 @@ contract('VestingScheme', accounts => { assert.equal(organizationProposal[0],0x0000000000000000000000000000000000000000);//new contract address }); - it("execute proposeVestingAgreement controller -yes - check set total amount", async function() { - var testSetup = await setup(accounts); - var amountPerPeriod =3; - var numberOfAgreedPeriods = 7; - var blockNumber = await (web3.utils.toBN(await web3.eth.getBlockNumber())); - - - var tx = await testSetup.vestingScheme.proposeVestingAgreement(accounts[0], - accounts[1], - blockNumber, - amountPerPeriod, - 2, - numberOfAgreedPeriods, - 11, - 0, - [], - testSetup.org.avatar.address,helpers.NULL_HASH); - //Vote with reputation to trigger execution - var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1); - //check organizationsProposals before execution - var organizationProposal = await testSetup.vestingScheme.organizationsProposals(testSetup.org.avatar.address,proposalId); - assert.equal(organizationProposal[0],testSetup.org.token.address); - assert.equal(await testSetup.org.token.balanceOf(testSetup.vestingScheme.address),0); - await testSetup.vestingSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); - //check organizationsProposals after execution - organizationProposal = await testSetup.vestingScheme.organizationsProposals(testSetup.org.avatar.address,proposalId); - assert.equal(organizationProposal[0],0x0000000000000000000000000000000000000000);//new contract address - var agreementsCounter = await testSetup.vestingScheme.agreementsCounter() - 1; - var agreement = await testSetup.vestingScheme.agreements(agreementsCounter); - assert.equal(agreement[11],testSetup.org.avatar.address);//avatar - assert.equal(agreement[12],amountPerPeriod*numberOfAgreedPeriods);//totalAmount - }); + it("execute proposeVestingAgreement controller -yes - check minting", async function() { + var testSetup = await setup(accounts); + var amountPerPeriod =3; + var numberOfAgreedPeriods = 7; + var blockNumber = await (web3.utils.toBN(await web3.eth.getBlockNumber())); + + + var tx = await testSetup.vestingScheme.proposeVestingAgreement(accounts[0], + accounts[1], + blockNumber, + amountPerPeriod, + 2, + numberOfAgreedPeriods, + 11, + 0, + [], + testSetup.org.avatar.address,helpers.NULL_HASH); + //Vote with reputation to trigger execution + var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1); + //check organizationsProposals before execution + var organizationProposal = await testSetup.vestingScheme.organizationsProposals(testSetup.org.avatar.address,proposalId); + assert.equal(organizationProposal[0],testSetup.org.token.address); + assert.equal(await testSetup.org.token.balanceOf(testSetup.vestingScheme.address),0); + await testSetup.vestingSchemeParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]}); + //check organizationsProposals after execution + organizationProposal = await testSetup.vestingScheme.organizationsProposals(testSetup.org.avatar.address,proposalId); + assert.equal(organizationProposal[0],0x0000000000000000000000000000000000000000);//new contract address + assert.equal(await testSetup.org.token.balanceOf(testSetup.vestingScheme.address),amountPerPeriod*numberOfAgreedPeriods); + }); it("createVestedAgreement check agreement id ", async function() { var testSetup = await setup(accounts); @@ -288,13 +285,6 @@ contract('VestingScheme', accounts => { assert.equal(agreementId,0); blockNumber = await (web3.utils.toBN(await web3.eth.getBlockNumber())); - - var agreementsCounter = await testSetup.vestingScheme.agreementsCounter() - 1; - var agreement = await testSetup.vestingScheme.agreements(agreementsCounter); - assert.equal(agreement[11],0);//avatar - assert.equal(agreement[12],amountPerPeriod*numberOfAgreedPeriods);//totalAmount - - tx = await testSetup.vestingScheme.createVestedAgreement( testSetup.standardTokenMock.address, accounts[0], accounts[1], @@ -310,12 +300,6 @@ contract('VestingScheme', accounts => { assert.equal(tx.logs[0].event, "NewVestedAgreement"); agreementId = await helpers.getValueFromLogs(tx, '_agreementId',1); assert.equal(agreementId,1); - - agreementsCounter = await testSetup.vestingScheme.agreementsCounter() - 1; - agreement = await testSetup.vestingScheme.agreements(agreementsCounter); - assert.equal(agreement[11],0);//avatar - assert.equal(agreement[12],amountPerPeriod*numberOfAgreedPeriods);//totalAmount - }); it("createVestedAgreement check periodLength==0 ", async function() { @@ -930,7 +914,7 @@ contract('VestingScheme', accounts => { assert.equal(balance.toNumber(),numberOfAgreedPeriods*amountPerPeriod); } } - }); + }); }); From 7079d034664240a9f4cc312b850adb430067d9a2 Mon Sep 17 00:00:00 2001 From: Oren Date: Thu, 14 Mar 2019 10:45:41 +0200 Subject: [PATCH 3/3] lint --- contracts/universalSchemes/VestingScheme.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/universalSchemes/VestingScheme.sol b/contracts/universalSchemes/VestingScheme.sol index 191d8aa9..cbddae9d 100644 --- a/contracts/universalSchemes/VestingScheme.sol +++ b/contracts/universalSchemes/VestingScheme.sol @@ -294,7 +294,8 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu function collect(uint256 _agreementId) public onlyBeneficiary(_agreementId) { Agreement memory agreement = agreements[_agreementId]; if (agreement.avatar != Avatar(0)) { - require(ControllerInterface(agreement.avatar.owner()).isSchemeRegistered(address(this), address(agreement.avatar))); + require(ControllerInterface(agreement.avatar.owner()) + .isSchemeRegistered(address(this), address(agreement.avatar))); } uint256 periodsFromStartingBlock = (block.number.sub(agreement.startingBlock)).div(agreement.periodLength); require(periodsFromStartingBlock >= agreement.cliffInPeriods); @@ -325,7 +326,8 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu Agreement memory agreement = agreements[_agreementId]; delete agreements[_agreementId]; if (agreement.avatar != Avatar(0)) { - require(ControllerInterface(agreement.avatar.owner()).isSchemeRegistered(address(this), address(agreement.avatar))); + require(ControllerInterface(agreement.avatar.owner()) + .isSchemeRegistered(address(this), address(agreement.avatar))); } uint256 periodsLeft = agreement.numOfAgreedPeriods.sub(agreement.collectedPeriods); uint256 tokensLeft = periodsLeft.mul(agreement.amountPerPeriod);