PoC test case passing, combined mappings to save gas

Author: johnhforrest

contracts/ERC900/ERC900.sol

@@ -1,5 +1,6 @@
 pragma solidity ^0.4.24;
 
+
 /**
  * @title ERC900 interface
  * @dev see https://github.com/ethereum/EIPs/issues/900
@@ -17,7 +18,7 @@ contract ERC900 {
   function supportsHistory() public pure returns (bool);
 
   // optional
-  function lastStakedFor(address addr) public view returns (uint256);
-  function totalStakedForAt(address addr, uint256 blockNumber) public view returns (uint256);
-  function totalStakedAt(uint256 blockNumber) public view returns (uint256);
+  // function lastStakedFor(address addr) public view returns (uint256);
+  // function totalStakedForAt(address addr, uint256 blockNumber) public view returns (uint256);
+  // function totalStakedAt(uint256 blockNumber) public view returns (uint256);
 }

contracts/ERC900/ERC900BasicStakeContainer.sol

@@ -5,6 +5,7 @@ import "../ERC20/ERC20.sol";
 
 import "../library/SafeMath.sol";
 
+
 /**
  * @title ERC900BasicStakeContainer
  */
@@ -13,30 +14,42 @@ contract ERC900BasicStakeContainer is ERC900 {
 
   ERC20 stakingToken;
 
-  mapping (address => Stake) stakes;
-
-  // TODO: This data structure should change to represent "weight" instead of amount
-  mapping (address => uint256) amountStakedFor;
+  mapping (address => StakeContainer) addresses;
 
   struct Stake {
     uint256 blockNumber;
     uint256 amount;
     bool exists;
   }
 
+  // To save on gas, rather than create a separate mapping for amountStakedFor & personalStake,
+  //  both data structures are stored in a single mapping for a given addresses.
+  //
+  // amountStakedFor consists of all tokens staked for a given address.
+  // personalStake is the stake made by a given address.
+  //
+  // It's possible to have a non-existing personalStake, but have tokens in amountStakedFor
+  //  if other users are staking on behalf of a given address.
+  struct StakeContainer {
+    // TODO: This data structure should change to represent "weight" instead of amount
+    uint256 amountStakedFor;
+
+    Stake personalStake;
+  }
+
   constructor(ERC20 _stakingToken) public {
     stakingToken = _stakingToken;
   }
 
   function stake(uint256 _amount, bytes _data) public {
-    require(!stakes[msg.sender].exists, "Stake already exists");
+    require(!addresses[msg.sender].personalStake.exists, "Stake already exists");
 
     require(
       stakingToken.transferFrom(msg.sender, this, _amount),
       "Stake required");
 
-    stakes[msg.sender] = Stake(block.number, _amount, true);
-    amountStakedFor[msg.sender].add(_amount);
+    addresses[msg.sender].personalStake = Stake(block.number, _amount, true);
+    addresses[msg.sender].amountStakedFor.add(_amount);
 
     emit Staked(
       msg.sender,
@@ -46,17 +59,17 @@ contract ERC900BasicStakeContainer is ERC900 {
   }
 
   function stakeFor(address _user, uint256 _amount, bytes _data) public {
-    require(!stakes[msg.sender].exists, "Stake already exists");
+    require(!addresses[msg.sender].personalStake.exists, "Stake already exists");
 
     require(
       stakingToken.transferFrom(msg.sender, this, _amount),
       "Stake required");
 
-    stakes[msg.sender] = Stake(block.number, _amount, true);
+    addresses[msg.sender].personalStake = Stake(block.number, _amount, true);
 
     // Notice here that we are increasing the staked amount for _user
     //  instead of msg.sender
-    amountStakedFor[_user].add(_amount);
+    addresses[_user].amountStakedFor.add(_amount);
 
     emit Staked(
       _user,
@@ -66,7 +79,7 @@ contract ERC900BasicStakeContainer is ERC900 {
   }
 
   function unstake(uint256 _amount, bytes _data) public {
-    require(stakes[msg.sender].exists, "Stake doesn't exist");
+    require(addresses[msg.sender].personalStake.exists, "Stake doesn't exist");
 
     // Transfer the staked tokens from this contract back tot he sender
     // Notice that we are using transfer instead of transferFrom here, so
@@ -77,11 +90,11 @@ contract ERC900BasicStakeContainer is ERC900 {
 
     // If this was a complete withdrawal, then delete the previous stake to reset
     //  the block number and exists flag
-    if (stakes[msg.sender].amount == 0) {
-      delete stakes[msg.sender];
-      amountStakedFor[msg.sender] = 0;
+    if (addresses[msg.sender].personalStake.amount == 0) {
+      delete addresses[msg.sender].personalStake;
+      addresses[msg.sender].amountStakedFor = 0;
     } else {
-      amountStakedFor[msg.sender].sub(_amount);
+      addresses[msg.sender].amountStakedFor.sub(_amount);
     }
 
     emit Unstaked(
@@ -92,7 +105,7 @@ contract ERC900BasicStakeContainer is ERC900 {
   }
 
   function totalStakedFor(address _address) public view returns (uint256) {
-    return amountStakedFor[_address];
+    return addresses[_address].amountStakedFor;
   }
 
   function totalStaked() public view returns (uint256) {

contracts/ERC900/ERC900StakeContainer.sol

@@ -2,6 +2,7 @@ pragma solidity ^0.4.24;
 
 import "./ERC900BasicStakeContainer.sol";
 
+
 /**
  * @title ERC900StakeContainer
  */
@@ -10,11 +11,12 @@ contract ERC900StakeContainer is ERC900BasicStakeContainer {
     ERC900BasicStakeContainer(_stakingToken) {
   }
 
-  /*
+
   function supportsHistory() public pure returns (bool) {
     return true;
   }
 
+  /*
   function lastStakedFor(address addr) public view returns (uint256) {
 
   }

migrations/3_deploy_stakecontainer.js

@@ -0,0 +1,6 @@
+const CodexToken = artifacts.require('./CodexToken.sol')
+const ERC900StakeContainer = artifacts.require('./ERC900StakeContainer.sol')
+
+module.exports = (deployer) => {
+  deployer.deploy(ERC900StakeContainer, CodexToken.address)
+}

migrations/3_deploy_721token.js renamed to migrations/4_deploy_721token.js

@@ -1,5 +1,5 @@
 const CodexTitle = artifacts.require('./CodexTitle.sol')
 
-module.exports = (deployer, network, accounts) => {
+module.exports = (deployer) => {
   deployer.deploy(CodexTitle)
 }

migrations/4_deploy_proxy.js renamed to migrations/5_deploy_proxy.js

@@ -15,5 +15,7 @@ module.exports = async (deployer, network, accounts) => {
     })
     .catch((error) => {
       console.log(error)
+
+      throw error
     })
 }

migrations/5_initialize_721token.js renamed to migrations/6_initialize_721token.js

@@ -3,16 +3,10 @@ const CodexTitle = artifacts.require('./CodexTitle.sol')
 const CodexTitleProxy = artifacts.require('./CodexTitleProxy.sol')
 
 module.exports = async (deployer, network, accounts) => {
+  const proxiedCodexTitle = CodexTitle.at(CodexTitleProxy.address)
 
   deployer
     .then(async () => {
-      const codexTitleProxy = await CodexTitleProxy.deployed()
-      const proxiedCodexTitle = CodexTitle.at(codexTitleProxy.address)
-
-      return proxiedCodexTitle
-    })
-    .then(async (proxiedCodexTitle) => {
-
       let initialFees
       let erc20TokenAddress
 
@@ -38,10 +32,8 @@ module.exports = async (deployer, network, accounts) => {
 
       console.log(`Setting the fees to ${initialFees} at ERC-20 token address: ${erc20TokenAddress}`)
       await proxiedCodexTitle.setFees(erc20TokenAddress, accounts[0], initialFees)
-
-      return proxiedCodexTitle
     })
-    .then(async (proxiedCodexTitle) => {
+    .then(async () => {
 
       let tokenURIPrefix
 
@@ -70,5 +62,7 @@ module.exports = async (deployer, network, accounts) => {
     })
     .catch((error) => {
       console.log(error)
+
+      throw error
     })
 }

migrations/6_transfer_ownership.js renamed to migrations/7_transfer_ownership.js

@@ -50,5 +50,7 @@ module.exports = async (deployer, network, accounts) => {
     })
     .catch((error) => {
       console.error(error)
+
+      throw error
     })
 }

test/token/behaviors/CodexTitle.behavior.js

@@ -3,7 +3,7 @@ import modifyMetadataHashesUnbound from '../../helpers/modifyMetadataHashes'
 
 const { BigNumber } = web3
 const CodexToken = artifacts.require('CodexToken.sol')
-const ERC900BasicStakeContainer = artifacts.require('ERC900BasicStakeContainer.sol')
+const ERC900StakeContainer = artifacts.require('ERC900StakeContainer.sol')
 
 require('chai')
   .use(require('chai-as-promised'))
@@ -148,7 +148,7 @@ export default function shouldBehaveLikeCodexTitle(accounts) {
           let stakeContainer
 
           beforeEach(async function () {
-            stakeContainer = await ERC900BasicStakeContainer.new(codexToken.address)
+            stakeContainer = await ERC900StakeContainer.new(codexToken.address)
 
             await this.token.setStakeContainer(stakeContainer.address)