chore(refactor): removing measure and ranges to scope down contract complexity

bitbeckers · bitbeckers · commit 2abf4be9794a · 2020-10-13T16:01:44.000+02:00
diff --git a/contracts/SinglePlayerCommit.sol b/contracts/SinglePlayerCommit.sol
@@ -7,8 +7,6 @@ import "@openzeppelin/contracts/access/Ownable.sol";
 import "@openzeppelin/contracts/math/SafeMath.sol";
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 
-//TODO Do we want console.log logging, start with Activity constructor?..
-
 contract SinglePlayerCommit is Ownable {
     using SafeMath for uint256;
 
@@ -21,26 +19,18 @@ contract SinglePlayerCommit is Ownable {
     /***************
     DATA TYPES
     ***************/
-    struct Measure {
-        string name;
-        bool allowed;
-    }
-
     struct Activity {
-        string name; // e.g. "cycling"
-        bytes32[] measures; // keys from allowedMeasures
-        uint256[2][] ranges; // array of [min,max] goal values
+        string name; // e.g. "cycling" with list scoped to activities supported by Strava
         address oracle;
         bool allowed;
     }
 
     struct Commitment {
         address committer; // user
-        bytes32 activity; // key from allowedActivities
-        bytes32 measure; // key from allowedMeasures
-        uint256 goalValue; // must be within range of Activity.measures[measureIndex]
-        uint256 start;
-        uint256 end;
+        bytes32 activityKey;
+        uint256 goal;
+        uint256 startTime;
+        uint256 endTime;
         uint256 stake; // amount of token staked, scaled by token decimals
         bool exists; // flag to help check if commitment exists
         uint256 reportedValue; // as reported by oracle
@@ -52,8 +42,8 @@ contract SinglePlayerCommit is Ownable {
     ***************/
     event NewCommitment(
         address committer,
-        string activity,
-        string measure,
+        string activityName,
+        uint256 goal,
         uint256 startTime,
         uint256 endTime,
         uint256 stake
@@ -68,11 +58,8 @@ contract SinglePlayerCommit is Ownable {
     mapping(bytes32 => Activity) public allowedActivities;
     bytes32[] public activityList;
 
-    mapping(bytes32 => Measure) public allowedMeasures;
-    bytes32[] public measureList;
-
     mapping(address => Commitment) public commitments; // active commitments
-    // address[] public committers; // addresses with active commitments
+    address[] public userCommitments; // addresses with active commitments
 
     mapping(address => uint256) public balances; // current token balances
     uint256 public committerBalance; // sum of current token balances
@@ -82,71 +69,35 @@ contract SinglePlayerCommit is Ownable {
     ********/
     // constructor
     constructor(
-        string memory _activity,
-        string[] memory _measures,
-        uint256[2][] memory _ranges,
-        address _oracle,
+        string[] memory _activityList,
+        address _oracleAddress,
         address _token
     ) public {
         console.log("Constructor called for SinglePlayerCommit contract");
         // set up token interface
         token = IERC20(_token);
+        require(_activityList.length >= 1, "SPC::constructor - activityList empty");
 
-        // need to create fixed length bytes32 array to pass to _addActivity
-        uint256 len = _measures.length;
-        bytes32[] memory measureKeys = new bytes32[](len);
-
-        // register measures
-        for (uint256 i = 0; i < len; i++) {
-            // register the measure
-            bytes32 measureKey = _addMeasure(_measures[i]);
-            // add its key to the array to be passed to _addActivity
-            measureKeys[i] = measureKey;
-        }
-
-        // register activity
-        _addActivity(_activity, measureKeys, _ranges, _oracle);
+        // register allowed activities with corresponding oracle
+        _addActivities(_activityList, _oracleAddress);
     }
 
-    // fallback function (if exists)
-    // TODO
-
     // view functions
 
     function getActivityName(bytes32 _activityKey) public view returns (string memory) {
         return allowedActivities[_activityKey].name;
     }
 
-    function getActivityMeasures(bytes32 _activityKey) public view returns (string[] memory measureNames) {
-        bytes32[] memory measures = allowedActivities[_activityKey].measures;
-        uint256 len = measures.length;
-        measureNames = new string[](len);
-        for (uint256 i = 0; i < len; i++) {
-            measureNames[i] = getMeasureName(measures[i]);
-        }
-
-        return measureNames;
-    }
-
-    function getMeasureName(bytes32 _measureKey) public view returns (string memory) {
-        return allowedMeasures[_measureKey].name;
-    }
-
     // other public functions
     function depositAndCommit(
-        bytes32 _activity,
-        uint256 _measureIndex,
+        bytes32 _activityKey,
         uint256 _goal,
         uint256 _startTime,
         uint256 _stake,
         uint256 _depositAmount
     ) public returns (bool) {
         require(deposit(_depositAmount), "SPC::depositAndCommit - deposit failed");
-
-        require(
-            makeCommitment(_activity, _measureIndex, _goal, _startTime, _stake),
-            "SPC::depositAndCommit - commitment failed"
-        );
+        require(makeCommitment(_activityKey, _goal, _startTime, _stake), "SPC::depositAndCommit - commitment failed");
 
         return true;
     }
@@ -165,39 +116,32 @@ contract SinglePlayerCommit is Ownable {
     }
 
     function makeCommitment(
-        bytes32 _activity,
-        uint256 _measureIndex, // index of the Activity.measures array
+        bytes32 _activityKey,
+        // uint256 _measureIndex, // index of the Activity.measures array
         uint256 _goal,
         uint256 _startTime,
         uint256 _stake
     ) public returns (bool) {
         console.log("makeCommitment called by %s", msg.sender);
 
         require(!commitments[msg.sender].exists, "SPC::makeCommitment - msg.sender already has a commitment");
-        require(allowedActivities[_activity].allowed, "SPC::makeCommitment - activity doesn't exist or isn't allowed");
-        require(allowedActivities[_activity].measures.length >= _measureIndex+1, "SPC::makeCommitment - measure index out of bounds");
-
-        bytes32 measure = allowedActivities[_activity].measures[_measureIndex];
-
-        require(allowedMeasures[measure].allowed, "SPC::makeCommitment - measure doesn't exist or isn't allowed");
+        require(
+            allowedActivities[_activityKey].allowed,
+            "SPC::makeCommitment - activity doesn't exist or isn't allowed"
+        );
         require(_startTime > block.timestamp, "SPC::makeCommitment - commitment cannot start in the past");
-
-        uint256[2] storage range = allowedActivities[_activity].ranges[_measureIndex];
-        require(_goal >= range[0], "SPC::makeCommitment - goal is too low");
-        require(_goal <= range[1], "SPC::makeCommitment - goal is too high");
-
+        require(_goal >= 1, "SPC::makeCommitment - goal is too low");
         require(balances[msg.sender] >= _stake, "SPC::makeCommitment - insufficient token balance");
 
         uint256 endTime = _startTime.add(7 days);
 
         // create commitment...
         Commitment memory commitment = Commitment({
             committer: msg.sender,
-            activity: _activity,
-            measure: measure,
-            goalValue: _goal,
-            start: _startTime,
-            end: endTime,
+            activityKey: _activityKey,
+            goal: _goal,
+            startTime: _startTime,
+            endTime: endTime,
             stake: _stake,
             exists: true,
             reportedValue: 0,
@@ -206,16 +150,8 @@ contract SinglePlayerCommit is Ownable {
 
         // ...and add it to storage
         commitments[msg.sender] = commitment;
-        // committers.push(msg.sender);
-
-        emit NewCommitment(
-            msg.sender,
-            allowedActivities[_activity].name,
-            allowedMeasures[measure].name,
-            _startTime,
-            endTime,
-            _stake
-        );
+
+        emit NewCommitment(msg.sender, allowedActivities[_activityKey].name, _goal, _startTime, endTime, _stake);
 
         return true;
     }
@@ -247,16 +183,13 @@ contract SinglePlayerCommit is Ownable {
         Commitment memory commitment = commitments[committer];
 
         // check if commitment has ended
-        require(commitment.end < block.timestamp, "SPC::processCommitment - commitment is still active");
+        require(commitment.endTime < block.timestamp, "SPC::processCommitment - commitment is still active");
 
         bool met = commitment.met;
         uint256 stake = commitment.stake;
 
         // "delete" the expired commitment
         commitments[committer].exists = false;
-        // remove the committer from the list of committers
-        // committers[committer] = committers[committers.length.sub(1)];
-        // committers.pop();
 
         uint256 penalty;
 
@@ -271,58 +204,46 @@ contract SinglePlayerCommit is Ownable {
         emit CommitmentEnded(committer, met, penalty);
     }
 
-    // function processCommitments() public returns (bool) {
-    //     for (uint256 i = 0; i < committers.length; i.add(1)) {
-    //         processCommitment(committers[i]);
-    //     }
-
-    //     return true;
-    // }
-
     function ownerWithdraw(uint256 amount) public onlyOwner returns (bool) {
         uint256 available = token.balanceOf(address(this)).sub(committerBalance);
 
         require(amount <= available, "SPC::ownerWithdraw - not enough available balance");
-
         require(token.transfer(msg.sender, amount), "SPC::ownerWithdraw - token transfer failed");
 
         return true;
     }
 
     // internal functions
+    function _addActivities(string[] memory _activityList, address oracleAddress) internal {
+        uint256 arrayLength = _activityList.length;
 
-    function _addMeasure(string memory _name) internal returns (bytes32 measureKey) {
-        Measure memory measure = Measure({ name: _name, allowed: true });
-
-        measureKey = keccak256(abi.encode(_name));
-        allowedMeasures[measureKey] = measure;
-        measureList.push(measureKey);
+        for (uint256 i = 0; i < arrayLength; i++) {
+            _addActivity(_activityList[i], oracleAddress);
+        }
 
-        return measureKey;
+        console.log("All provided activities added");
     }
 
-    function _addActivity(
-        string memory _name,
-        bytes32[] memory _measures,
-        uint256[2][] memory _ranges,
-        address _oracle
-    ) internal returns (bytes32 activityKey) {
-        uint256 measuresLength = _measures.length;
-        require(measuresLength == _ranges.length, "SPC::_addActivity - measures and ranges must have same length");
-
-        Activity memory activity;
-
-        activity.name = _name;
-        activity.oracle = _oracle;
-        activity.measures = _measures;
-        activity.ranges = _ranges;
+    function _addActivity(string memory _activityName, address _oracleAddress) internal returns (bytes32 activityKey) {
+        bytes memory activityNameBytes = bytes(_activityName);
+        require(activityNameBytes.length > 0, "SPC::_addActivity - _activityName empty");
+
+        bytes32 _activityKey = keccak256(abi.encode(_activityName));
+
+        Activity storage activity = allowedActivities[_activityKey];
+        activity.name = _activityName;
+        activity.oracle = _oracleAddress;
         activity.allowed = true;
 
-        activityKey = keccak256(abi.encode(_name));
-        allowedActivities[activityKey] = activity;
-        activityList.push(activityKey);
+        console.log(
+            "Registered activity %s, oracle %s, allowed %s",
+            allowedActivities[_activityKey].name,
+            allowedActivities[_activityKey].oracle,
+            allowedActivities[_activityKey].allowed
+        );
 
-        return activityKey;
+        activityList.push(_activityKey);
+        return _activityKey;
     }
 
     function _changeCommitterBalance(uint256 amount, bool add) internal returns (bool) {
diff --git a/test/SinglePlayerCommit.deploy.ts b/test/SinglePlayerCommit.deploy.ts
@@ -1,44 +1,46 @@
 import { expect } from "chai";
-import { BytesLike } from "ethers/lib/utils";
+import { BytesLike, solidityKeccak256 } from "ethers/lib/utils";
 
 export function shouldDeployWithInitialParameters(): void {
 
-  it("has the 'biking' activity and it is allowed", async function () {
-    const activityKey: BytesLike = await this.singlePlayerCommit.activityList(0);
-    const _activityName: string = await this.singlePlayerCommit.getActivityName(activityKey);
- 
+  it("has the 'biking' and 'cycling' activity and it is allowed", async function () {
+    //Check biking
+    const _activityName: string = "biking";
+    const _firstKey: BytesLike = await this.singlePlayerCommit.activityList(0);
 
-    const _activity = await this.singlePlayerCommit.allowedActivities(activityKey);
+    //TODO Find the way to get the expected Keccak256 output matching the first key in the list
+    // const _activityKey: BytesLike = solidityKeccak256(["string"], [_activityName]);
+    // console.log("Activitykey: ", _activityKey )
 
-    expect(_activityName).to.equal('biking');
+    const _activity = await this.singlePlayerCommit.allowedActivities(_firstKey);
+    console.log("Activity returned: ", _activity)
+
+    //Check running
+    const _activityName2: string = "running";
+    const _secondKey: BytesLike = await this.singlePlayerCommit.activityList(1);
+
+    const _activityKey2: BytesLike = solidityKeccak256(["string"], [_activityName2]);
+    console.log("Activitykey: ", _activityKey2 )
+
+    const _activity2 = await this.singlePlayerCommit.allowedActivities(_secondKey);
+    console.log("Activity returned: ", _activity2)
+
+    //Validate
     expect(_activity['name']).to.equal(_activityName);
-    // expect(_activity['measures'][0]).to.equal('km');
-    // expect(_activity['ranges']).to.equal([2,1024]);
     expect(_activity['oracle']).to.be.properAddress;
     expect(_activity['allowed']).to.be.true;
-    // expect('getActivityName').to.be.calledOnContract(this.singlePlayerCommit);
-  });
-
-  it("has no other activities", async function () {
-    await expect(
-      this.singlePlayerCommit.activityList(1),
-    ).to.be.revertedWith("Transaction reverted without a reason")
-    
-  });
 
-  it("has the 'km' measure and it is allowed", async function () {
-    const measureKey: BytesLike = await this.singlePlayerCommit.measureList(0);
-    const activityMeasure: string[] = await this.singlePlayerCommit.allowedMeasures(measureKey); 
-
-    expect(activityMeasure[0]).to.equal('km');
-    expect(activityMeasure[1]).to.be.true;
+    expect(_activity2['name']).to.equal(_activityName2);
+    expect(_activity2['oracle']).to.be.properAddress;
+    expect(_activity2['allowed']).to.be.true;
 
   });
 
-  it("has no other measures", async function () {
+  it("has no other activities", async function () {
     await expect(
-      this.singlePlayerCommit.activityList(1),
+      this.singlePlayerCommit.activityList(2),
     ).to.be.revertedWith("Transaction reverted without a reason")
+    
   });
 
 }
diff --git a/test/SinglePlayerCommit.ts b/test/SinglePlayerCommit.ts
