Lock delegated tokens in a separate contract

contracts/TestDAO.sol

@@ -24,6 +24,7 @@ contract TestDAO is DelegatedShareholderAssociation {
         requiredSharesToBeBoardMember = REQUIRED_SHARES_TO_BE_BOARD_MEMBER;
         minimumQuorum = 1000 * (10 ** TOKEN_DECIMALS);
         debatingPeriodInMinutes = 0;
+        tokenLocker = new TokenLocker(sharesAddress);
     }
 
 }

contracts/WyvernDAO.sol

@@ -26,6 +26,7 @@ contract WyvernDAO is DelegatedShareholderAssociation {
         requiredSharesToBeBoardMember = REQUIRED_SHARES_TO_BE_BOARD_MEMBER;
         minimumQuorum = MINIMUM_QUORUM;
         debatingPeriodInMinutes = DEBATE_PERIOD_MINUTES;
+        tokenLocker = new TokenLocker(sharesAddress);
     }
 
 }

contracts/common/TokenLocker.sol

@@ -0,0 +1,44 @@
+/*
+
+  Contract to allow an owning contract to receive tokens (ERC20, not ERC223), transfer them at will, and do absolutely nothing else.
+  
+  Used to allow DAO shareholders to lock tokens for vote delegation but prevent the DAO from doing anything with the locked tokens.
+
+  Much thanks to @adamkolar on Github - https://github.com/ProjectWyvern/wyvern-ethereum/issues/4
+
+*/
+
+pragma solidity 0.4.18;
+
+import "zeppelin-solidity/contracts/token/ERC20.sol";
+
+/**
+ * @title TokenLocker
+ * @author Project Wyvern Developers
+ */
+contract TokenLocker {
+
+    address public owner;
+
+    ERC20 public token;
+
+    /**
+     * @dev Create a new TokenLocker contract
+     * @param tokenAddr ERC20 token this contract will be used to lock
+     */
+    function TokenLocker (ERC20 tokenAddr) public {
+        owner = msg.sender;
+        token = tokenAddr;
+    }
+
+    /** 
+     *  @dev Call the ERC20 `transfer` function on the underlying token contract
+     *  @param dest Token destination
+     *  @param amount Amount of tokens to be transferred
+     */
+    function transfer(address dest, uint amount) public returns (bool) {
+        require(msg.sender == owner);
+        return token.transfer(dest, amount);
+    }
+
+}

contracts/dao/DelegatedShareholderAssociation.sol

@@ -46,6 +46,7 @@ pragma solidity 0.4.18;
 
 import "zeppelin-solidity/contracts/token/ERC20.sol";
 import "../common/TokenRecipient.sol";
+import "../common/TokenLocker.sol";
 
 /**
  * @title DelegatedShareholderAssociation
@@ -74,6 +75,9 @@ contract DelegatedShareholderAssociation is TokenRecipient {
     /* Threshold for the ability to create proposals. */
     uint public requiredSharesToBeBoardMember;
 
+    /* Token Locker contract. */
+    TokenLocker public tokenLocker;
+
     /* Events for all state changes. */
 
     event ProposalAdded(uint proposalID, address recipient, uint amount, bytes metadataHash);
@@ -114,6 +118,12 @@ contract DelegatedShareholderAssociation is TokenRecipient {
         _;
     }
 
+    /* Any account except the DAO itself can execute a function with this modifier. */
+    modifier notSelf {
+        require(msg.sender != address(this));
+        _;
+    }
+
     /* Only a shareholder who has *not* delegated his vote can execute a function with this modifier. */
     modifier onlyUndelegated {
         require(delegatesByDelegator[msg.sender] == address(0));
@@ -140,12 +150,18 @@ contract DelegatedShareholderAssociation is TokenRecipient {
       * @param tokensToLock number of tokens to be locked (sending address must have at least this many tokens)
       * @param delegate the address to which votes equal to the number of tokens locked will be delegated
       */
-    function setDelegateAndLockTokens(uint tokensToLock, address delegate) public onlyShareholders onlyUndelegated {
+    function setDelegateAndLockTokens(uint tokensToLock, address delegate)
+        public
+        onlyShareholders
+        onlyUndelegated
+        notSelf
+    {
         lockedDelegatingTokens[msg.sender] = tokensToLock;
         delegatedAmountsByDelegate[delegate] += tokensToLock;
         totalLockedTokens += tokensToLock;
         delegatesByDelegator[msg.sender] = delegate;
-        require(ERC20(sharesTokenAddress).transferFrom(msg.sender, address(this), tokensToLock));
+        require(sharesTokenAddress.transferFrom(msg.sender, tokenLocker, tokensToLock));
+        require(sharesTokenAddress.balanceOf(tokenLocker) == totalLockedTokens);
         TokensDelegated(msg.sender, tokensToLock, delegate);
     }
 
@@ -156,14 +172,20 @@ contract DelegatedShareholderAssociation is TokenRecipient {
      * @dev Can only be called by a sending address currently delegating tokens, will transfer all locked tokens back to the sender
      * @return The number of tokens previously locked, now released
      */
-    function clearDelegateAndUnlockTokens() public onlyDelegated returns (uint lockedTokens) {
+    function clearDelegateAndUnlockTokens()
+        public
+        onlyDelegated
+        notSelf
+        returns (uint lockedTokens)
+    {
         address delegate = delegatesByDelegator[msg.sender];
         lockedTokens = lockedDelegatingTokens[msg.sender];
         lockedDelegatingTokens[msg.sender] = 0;
         delegatedAmountsByDelegate[delegate] -= lockedTokens;
         totalLockedTokens -= lockedTokens;
         delete delegatesByDelegator[msg.sender];
-        require(ERC20(sharesTokenAddress).transfer(msg.sender, lockedTokens));
+        require(tokenLocker.transfer(msg.sender, lockedTokens));
+        require(sharesTokenAddress.balanceOf(tokenLocker) == totalLockedTokens);
         TokensUndelegated(msg.sender, lockedTokens, delegate);
         return lockedTokens;
     }
@@ -179,7 +201,10 @@ contract DelegatedShareholderAssociation is TokenRecipient {
      * @param minutesForDebate the minimum amount of delay between when a proposal is made and when it can be executed
      * @param sharesToBeBoardMember the minimum number of shares required to create proposals
      */
-    function changeVotingRules(uint minimumSharesToPassAVote, uint minutesForDebate, uint sharesToBeBoardMember) public onlySelf {
+    function changeVotingRules(uint minimumSharesToPassAVote, uint minutesForDebate, uint sharesToBeBoardMember)
+        public
+        onlySelf
+    {
         if (minimumSharesToPassAVote == 0 ) {
             minimumSharesToPassAVote = 1;
         }
@@ -207,8 +232,11 @@ contract DelegatedShareholderAssociation is TokenRecipient {
     )
         public
         onlyBoardMembers
+        notSelf
         returns (uint proposalID)
     {
+        /* Proposals cannot be directed to the token locking contract. */
+        require(beneficiary != address(tokenLocker));
         proposalID = proposals.length++;
         Proposal storage p = proposals[proposalID];
         p.recipient = beneficiary;
@@ -222,7 +250,6 @@ contract DelegatedShareholderAssociation is TokenRecipient {
         p.numberOfVotes = 0;
         ProposalAdded(proposalID, beneficiary, weiAmount, jobMetadataHash);
         numProposals = proposalID+1;
-
         return proposalID;
     }
 
@@ -262,6 +289,7 @@ contract DelegatedShareholderAssociation is TokenRecipient {
     )
         public
         onlyShareholders
+        notSelf
         returns (uint voteID)
     {
         Proposal storage p = proposals[proposalNumber];
@@ -315,7 +343,10 @@ contract DelegatedShareholderAssociation is TokenRecipient {
      * @param proposalNumber proposal number
      * @param transactionBytecode optional: if the transaction contained a bytecode, you need to send it
      */
-    function executeProposal(uint proposalNumber, bytes transactionBytecode) public {
+    function executeProposal(uint proposalNumber, bytes transactionBytecode)
+        public
+        notSelf
+    {
         Proposal storage p = proposals[proposalNumber];
 
         /* If at or past deadline, not already finalized, and code is correct, keep going. */
@@ -337,9 +368,6 @@ contract DelegatedShareholderAssociation is TokenRecipient {
             /* Execute the function. */
             require(p.recipient.call.value(p.amount)(transactionBytecode));
 
-            /* Prevent the DAO from sending the locked shares tokens (and thus potentially being unable to release locked tokens to delegating shareholders). */
-            require(ERC20(sharesTokenAddress).balanceOf(address(this)) >= totalLockedTokens);
-
         } else {
             /* Proposal failed. */
             p.proposalPassed = false;

test/test-dao.js

@@ -105,6 +105,21 @@ contract('TestDAO', (accounts) => {
       })
   })
 
+  it('should not allow proposal creation targeting the tokenLocker contract', () => {
+    return TestDAO
+      .deployed()
+      .then(daoInstance => {
+        return daoInstance.tokenLocker.call().then(tokenLocker => {
+          return daoInstance.newProposal.call(tokenLocker, 0, '0x', '0x')
+            .then(() => {
+              assert.equal(true, false, 'Proposal creation was allowed targeting the tokenLocker contract')
+            }).catch(err => {
+              assert.equal(err.message, 'VM Exception while processing transaction: revert', 'Incorrect error')
+            })
+        })
+      })
+  })
+
   it('should allow voting, count votes correctly, then allow proposal execution', () => {
     const amount = new BigNumber(Math.pow(10, 18 + 7))
     return TestDAO