Fix proposalInfo mapping (#597)

* Fix proposalInfo mapping * Fix UController return in removeGlobalConstraint * Bump version
daostack · Feb 13, 2019 · 5fd969d · 5fd969d
1 parent e08efd4
commit 5fd969d
Show file tree

Hide file tree

Showing 12 changed files with 57 additions and 70 deletions.
diff --git a/contracts/controller/UController.sol b/contracts/controller/UController.sol
@@ -334,7 +334,7 @@ contract UController is ControllerInterface {
             (when == GlobalConstraintInterface.CallPhase.PreAndPost)) {
             removeGlobalConstraintPost(_globalConstraint, _avatar);
         }
-        return false;
+        return true;
     }
 
   /**

diff --git a/contracts/test/ARCGenesisProtocolCallbacksMock.sol b/contracts/test/ARCGenesisProtocolCallbacksMock.sol
@@ -6,10 +6,9 @@ import "../votingMachines/VotingMachineCallbacks.sol";
 contract ARCVotingMachineCallbacksMock is VotingMachineCallbacks {
 
     function propose(bytes32 _proposalId, Avatar _avatar, address _votingMachine) public {
-        proposalsInfo[_proposalId] = ProposalInfo({
+        proposalsInfo[_votingMachine][_proposalId] = ProposalInfo({
             blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:_votingMachine
+            avatar:_avatar
         });
     }
 }
diff --git a/contracts/universalSchemes/ContributionReward.sol b/contracts/universalSchemes/ContributionReward.sol
@@ -84,7 +84,7 @@ contract ContributionReward is UniversalScheme, VotingMachineCallbacks, Proposal
     * @param _param a parameter of the voting result, 1 yes and 2 is no.
     */
     function executeProposal(bytes32 _proposalId, int256 _param) external onlyVotingMachine(_proposalId) returns(bool) {
-        ProposalInfo memory proposal = proposalsInfo[_proposalId];
+        ProposalInfo memory proposal = proposalsInfo[msg.sender][_proposalId];
         require(organizationsProposals[address(proposal.avatar)][_proposalId].executionTime == 0);
         require(organizationsProposals[address(proposal.avatar)][_proposalId].beneficiary != address(0));
         // Check if vote was successful:
@@ -203,10 +203,9 @@ contract ContributionReward is UniversalScheme, VotingMachineCallbacks, Proposal
             beneficiary
         );
 
-        proposalsInfo[contributionId] = ProposalInfo({
+        proposalsInfo[address(controllerParams.intVote)][contributionId] = ProposalInfo({
             blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:address(controllerParams.intVote)
+            avatar:_avatar
         });
         return contributionId;
     }

diff --git a/contracts/universalSchemes/GenericScheme.sol b/contracts/universalSchemes/GenericScheme.sol
@@ -62,14 +62,14 @@ contract GenericScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu
     external
     onlyVotingMachine(_proposalId)
     returns(bool) {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
+        Avatar avatar = proposalsInfo[msg.sender][_proposalId].avatar;
         CallProposal storage proposal = organizationsProposals[address(avatar)][_proposalId];
         require(proposal.exist, "must be a live proposal");
         require(proposal.passed == false, "cannot execute twice");
 
         if (_decision == 1) {
             proposal.passed = true;
-            execute(_proposalId);
+            execute(avatar, _proposalId);
         } else {
             delete organizationsProposals[address(avatar)][_proposalId];
             emit ProposalDeleted(address(avatar), _proposalId);
@@ -83,21 +83,20 @@ contract GenericScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu
     * @dev execution of proposals after it has been decided by the voting machine
     * @param _proposalId the ID of the voting in the voting machine
     */
-    function execute(bytes32 _proposalId) public {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
-        Parameters memory params = parameters[getParametersFromController(avatar)];
-        CallProposal storage proposal = organizationsProposals[address(avatar)][_proposalId];
+    function execute(Avatar _avatar, bytes32 _proposalId) public {
+        Parameters memory params = parameters[getParametersFromController(_avatar)];
+        CallProposal storage proposal = organizationsProposals[address(_avatar)][_proposalId];
         require(proposal.exist, "must be a live proposal");
         require(proposal.passed, "proposal must passed by voting machine");
         proposal.exist = false;
         bytes memory genericCallReturnValue;
         bool success;
-        ControllerInterface controller = ControllerInterface(Avatar(avatar).owner());
-        (success, genericCallReturnValue) = controller.genericCall(params.contractToCall, proposal.callData, avatar);
+        ControllerInterface controller = ControllerInterface(_avatar.owner());
+        (success, genericCallReturnValue) = controller.genericCall(params.contractToCall, proposal.callData, _avatar);
         if (success) {
-            delete organizationsProposals[address(avatar)][_proposalId];
-            emit ProposalDeleted(address(avatar), _proposalId);
-            emit ProposalExecuted(address(avatar), _proposalId, genericCallReturnValue);
+            delete organizationsProposals[address(_avatar)][_proposalId];
+            emit ProposalDeleted(address(_avatar), _proposalId);
+            emit ProposalExecuted(address(_avatar), _proposalId, genericCallReturnValue);
         } else {
             proposal.exist = true;
         }
@@ -159,10 +158,9 @@ contract GenericScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu
             exist: true,
             passed: false
         });
-        proposalsInfo[proposalId] = ProposalInfo({
+        proposalsInfo[address(params.intVote)][proposalId] = ProposalInfo({
             blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:address(params.intVote)
+            avatar:_avatar
         });
         emit NewCallProposal(address(_avatar), proposalId, _callData, _descriptionHash);
         return proposalId;

diff --git a/contracts/universalSchemes/GlobalConstraintRegistrar.sol b/contracts/universalSchemes/GlobalConstraintRegistrar.sol
@@ -62,7 +62,7 @@ contract GlobalConstraintRegistrar is UniversalScheme, VotingMachineCallbacks, P
     * @return bool which represents a successful of the function.
     */
     function executeProposal(bytes32 _proposalId, int256 _param) external onlyVotingMachine(_proposalId) returns(bool) {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
+        Avatar avatar = proposalsInfo[msg.sender][_proposalId].avatar;
         bool retVal = true;
         // Check if vote was successful:
         GCProposal memory proposal = organizationsProposals[address(avatar)][_proposalId];
@@ -161,10 +161,9 @@ contract GlobalConstraintRegistrar is UniversalScheme, VotingMachineCallbacks, P
             _voteToRemoveParams,
             _descriptionHash
         );
-        proposalsInfo[proposalId] = ProposalInfo({
+        proposalsInfo[address(intVote)][proposalId] = ProposalInfo({
             blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:address(intVote)
+            avatar:_avatar
         });
         return proposalId;
     }
@@ -197,10 +196,9 @@ contract GlobalConstraintRegistrar is UniversalScheme, VotingMachineCallbacks, P
 
         organizationsProposals[address(_avatar)][proposalId] = proposal;
         emit RemoveGlobalConstraintsProposal(address(_avatar), proposalId, address(intVote), _gc, _descriptionHash);
-        proposalsInfo[proposalId] = ProposalInfo({
-            blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:address(intVote)
+        proposalsInfo[address(intVote)][proposalId] = ProposalInfo({
+            blockNumber: block.number,
+            avatar: _avatar
         });
         return proposalId;
     }

diff --git a/contracts/universalSchemes/SchemeRegistrar.sol b/contracts/universalSchemes/SchemeRegistrar.sol
@@ -58,7 +58,7 @@ contract SchemeRegistrar is UniversalScheme, VotingMachineCallbacks, ProposalExe
     * @param _param a parameter of the voting result, 1 yes and 2 is no.
     */
     function executeProposal(bytes32 _proposalId, int256 _param) external onlyVotingMachine(_proposalId) returns(bool) {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
+        Avatar avatar = proposalsInfo[msg.sender][_proposalId].avatar;
         SchemeProposal memory proposal = organizationsProposals[address(avatar)][_proposalId];
         require(proposal.scheme != address(0));
         delete organizationsProposals[address(avatar)][_proposalId];
@@ -157,10 +157,9 @@ contract SchemeRegistrar is UniversalScheme, VotingMachineCallbacks, ProposalExe
             _descriptionHash
         );
         organizationsProposals[address(_avatar)][proposalId] = proposal;
-        proposalsInfo[proposalId] = ProposalInfo({
+        proposalsInfo[address(controllerParams.intVote)][proposalId] = ProposalInfo({
             blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:address(controllerParams.intVote)
+            avatar:_avatar
         });
         return proposalId;
     }
@@ -184,10 +183,9 @@ contract SchemeRegistrar is UniversalScheme, VotingMachineCallbacks, ProposalExe
         bytes32 proposalId = intVote.propose(2, params.voteRemoveParams, msg.sender, address(_avatar));
         organizationsProposals[address(_avatar)][proposalId].scheme = _scheme;
         emit RemoveSchemeProposal(address(_avatar), proposalId, address(intVote), _scheme, _descriptionHash);
-        proposalsInfo[proposalId] = ProposalInfo({
+        proposalsInfo[address(params.intVote)][proposalId] = ProposalInfo({
             blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:address(params.intVote)
+            avatar:_avatar
         });
         return proposalId;
     }

diff --git a/contracts/universalSchemes/UpgradeScheme.sol b/contracts/universalSchemes/UpgradeScheme.sol
@@ -57,7 +57,7 @@ contract UpgradeScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu
     * @param _param a parameter of the voting result, 1 yes and 2 is no.
     */
     function executeProposal(bytes32 _proposalId, int256 _param) external onlyVotingMachine(_proposalId) returns(bool) {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
+        Avatar avatar = proposalsInfo[msg.sender][_proposalId].avatar;
         UpgradeProposal memory proposal = organizationsProposals[address(avatar)][_proposalId];
         require(proposal.proposalType != 0);
         delete organizationsProposals[address(avatar)][_proposalId];
@@ -138,10 +138,9 @@ contract UpgradeScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu
         _newController,
         _descriptionHash
         );
-        proposalsInfo[proposalId] = ProposalInfo({
+        proposalsInfo[address(params.intVote)][proposalId] = ProposalInfo({
             blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:address(params.intVote)
+            avatar:_avatar
         });
         return proposalId;
     }
@@ -183,10 +182,9 @@ contract UpgradeScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu
             _params,
             _descriptionHash
         );
-        proposalsInfo[proposalId] = ProposalInfo({
+        proposalsInfo[address(intVote)][proposalId] = ProposalInfo({
             blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:address(intVote)
+            avatar:_avatar
         });
         return proposalId;
     }

diff --git a/contracts/universalSchemes/VestingScheme.sol b/contracts/universalSchemes/VestingScheme.sol
@@ -78,7 +78,7 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu
     * @return bool which represents a successful of the function
     */
     function executeProposal(bytes32 _proposalId, int256 _param) external onlyVotingMachine(_proposalId) returns(bool) {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
+        Avatar avatar = proposalsInfo[msg.sender][_proposalId].avatar;
         Agreement memory proposedAgreement = organizationsProposals[address(avatar)][_proposalId];
         require(proposedAgreement.periodLength > 0);
         delete organizationsProposals[address(avatar)][_proposalId];
@@ -151,10 +151,9 @@ contract VestingScheme is UniversalScheme, VotingMachineCallbacks, ProposalExecu
         organizationsProposals[address(_avatar)][proposalId].cliffInPeriods = _cliffInPeriods;
         organizationsProposals[address(_avatar)][proposalId].signaturesReqToCancel = _signaturesReqToCancel;
 
-        proposalsInfo[proposalId] = ProposalInfo({
+        proposalsInfo[address(params.intVote)][proposalId] = ProposalInfo({
             blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:address(params.intVote)
+            avatar:_avatar
         });
         emit AgreementProposal(address(_avatar), proposalId, _descriptionHash);
         return proposalId;

diff --git a/contracts/universalSchemes/VoteInOrganizationScheme.sol b/contracts/universalSchemes/VoteInOrganizationScheme.sol
@@ -50,7 +50,7 @@ contract VoteInOrganizationScheme is UniversalScheme, VotingMachineCallbacks, Pr
     * @return bool which represents a successful of the function
     */
     function executeProposal(bytes32 _proposalId, int256 _param) external onlyVotingMachine(_proposalId) returns(bool) {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
+        Avatar avatar = proposalsInfo[msg.sender][_proposalId].avatar;
         // Save proposal to memory and delete from storage:
         VoteProposal memory proposal = organizationsProposals[address(avatar)][_proposalId];
         require(proposal.exist);
@@ -151,10 +151,9 @@ contract VoteInOrganizationScheme is UniversalScheme, VotingMachineCallbacks, Pr
             _vote,
             _descriptionHash
         );
-        proposalsInfo[proposalId] = ProposalInfo({
+        proposalsInfo[address(intVote)][proposalId] = ProposalInfo({
             blockNumber:block.number,
-            avatar:_avatar,
-            votingMachine:address(intVote)
+            avatar:_avatar
         });
         return proposalId;
     }

diff --git a/contracts/votingMachines/VotingMachineCallbacks.sol b/contracts/votingMachines/VotingMachineCallbacks.sol
@@ -9,23 +9,22 @@ contract VotingMachineCallbacks is VotingMachineCallbacksInterface {
     struct ProposalInfo {
         uint256 blockNumber; // the proposal's block number
         Avatar avatar; // the proposal's avatar
-        address votingMachine;
     }
 
     modifier onlyVotingMachine(bytes32 _proposalId) {
-        require(msg.sender == proposalsInfo[_proposalId].votingMachine, "only VotingMachine");
+        require(proposalsInfo[msg.sender][_proposalId].avatar != Avatar(address(0)), "only VotingMachine");
         _;
     }
 
-            //proposalId ->     ProposalInfo
-    mapping(bytes32      =>     ProposalInfo    ) public proposalsInfo;
+    // VotingMaching  ->  proposalId  ->  ProposalInfo
+    mapping(address => mapping(bytes32 => ProposalInfo)) public proposalsInfo;
 
     function mintReputation(uint256 _amount, address _beneficiary, bytes32 _proposalId)
     external
     onlyVotingMachine(_proposalId)
     returns(bool)
     {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
+        Avatar avatar = proposalsInfo[msg.sender][_proposalId].avatar;
         if (avatar == Avatar(0)) {
             return false;
         }
@@ -37,7 +36,7 @@ contract VotingMachineCallbacks is VotingMachineCallbacksInterface {
     onlyVotingMachine(_proposalId)
     returns(bool)
     {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
+        Avatar avatar = proposalsInfo[msg.sender][_proposalId].avatar;
         if (avatar == Avatar(0)) {
             return false;
         }
@@ -53,31 +52,31 @@ contract VotingMachineCallbacks is VotingMachineCallbacksInterface {
     onlyVotingMachine(_proposalId)
     returns(bool)
     {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
+        Avatar avatar = proposalsInfo[msg.sender][_proposalId].avatar;
         if (avatar == Avatar(0)) {
             return false;
         }
         return ControllerInterface(avatar.owner()).externalTokenTransfer(_stakingToken, _beneficiary, _amount, avatar);
     }
 
     function balanceOfStakingToken(IERC20 _stakingToken, bytes32 _proposalId) external view returns(uint256) {
-        Avatar avatar = proposalsInfo[_proposalId].avatar;
-        if (proposalsInfo[_proposalId].avatar == Avatar(0)) {
+        Avatar avatar = proposalsInfo[msg.sender][_proposalId].avatar;
+        if (proposalsInfo[msg.sender][_proposalId].avatar == Avatar(0)) {
             return 0;
         }
         return _stakingToken.balanceOf(address(avatar));
     }
 
     function getTotalReputationSupply(bytes32 _proposalId) external view returns(uint256) {
-        ProposalInfo memory proposal = proposalsInfo[_proposalId];
+        ProposalInfo memory proposal = proposalsInfo[msg.sender][_proposalId];
         if (proposal.avatar == Avatar(0)) {
             return 0;
         }
         return proposal.avatar.nativeReputation().totalSupplyAt(proposal.blockNumber);
     }
 
     function reputationOf(address _owner, bytes32 _proposalId) external view returns(uint256) {
-        ProposalInfo memory proposal = proposalsInfo[_proposalId];
+        ProposalInfo memory proposal = proposalsInfo[msg.sender][_proposalId];
         if (proposal.avatar == Avatar(0)) {
             return 0;
         }

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@daostack/arc",
-  "version": "0.0.1-rc.8",
+  "version": "0.0.1-rc.9",
   "description": "A platform for building DAOs",
   "files": [
     "contracts/",

diff --git a/test/genericscheme.js b/test/genericscheme.js
@@ -130,7 +130,7 @@ contract('genericScheme', function(accounts) {
        assert.equal(organizationProposal.exist,true);//new contract address
        assert.equal(organizationProposal.passed,true);//new contract address
        //can call execute
-       await testSetup.genericScheme.execute(proposalId);
+       await testSetup.genericScheme.execute(testSetup.org.avatar.address, proposalId);
 
     });
 
@@ -150,15 +150,15 @@ contract('genericScheme', function(accounts) {
        assert.equal(organizationProposal.exist,true);//new contract address
        assert.equal(organizationProposal.passed,true);//new contract address
        //can call execute
-       await testSetup.genericScheme.execute(proposalId);
+       await testSetup.genericScheme.execute(testSetup.org.avatar.address, proposalId);
        await helpers.increaseTime(1001);
-       await testSetup.genericScheme.execute(proposalId);
+       await testSetup.genericScheme.execute(testSetup.org.avatar.address, proposalId);
 
        organizationProposal = await testSetup.genericScheme.organizationsProposals(testSetup.org.avatar.address,proposalId);
        assert.equal(organizationProposal.exist,false);//new contract address
        assert.equal(organizationProposal.passed,false);//new contract address
        try {
-         await testSetup.genericScheme.execute(proposalId);
+         await testSetup.genericScheme.execute(testSetup.org.avatar.address, proposalId);
          assert(false, "cannot call execute after it been executed");
        } catch(error) {
          helpers.assertVMException(error);
@@ -186,7 +186,7 @@ contract('genericScheme', function(accounts) {
        var proposalId = await helpers.getValueFromLogs(tx, '_proposalId');
 
        try {
-         await testSetup.genericScheme.execute(proposalId);
+         await testSetup.genericScheme.execute(testSetup.org.avatar.address, proposalId);
          assert(false, "execute should fail if not executed from votingMachine");
        } catch(error) {
          helpers.assertVMException(error);
@@ -244,7 +244,7 @@ contract('genericScheme', function(accounts) {
          await testSetup.genericSchemeParams.votingMachine.genesisProtocol.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]});
          assert.equal(await web3.eth.getBalance(wallet.address),web3.utils.toWei('1', "ether"));
          await wallet.transferOwnership(testSetup.org.avatar.address);
-         await testSetup.genericScheme.execute(proposalId);
+         await testSetup.genericScheme.execute(testSetup.org.avatar.address, proposalId);
          assert.equal(await web3.eth.getBalance(wallet.address),0);
       });