Merge pull request #18 from OpenBazaar/aw-audit-fixes

adjusting specs and rewards contract as per solidified audit

contracts/escrow/EscrowSpec.md

@@ -4,7 +4,7 @@
 
 OpenBazaar facilitates trades between arbitrary third parties on the internet. Currently, only UTXO-based cryptocurrencies can be used as a medium of exchange on OpenBazaar. The escrow contract is intended to be used as a way to shoehorn Ethereum functionality into OpenBazaar's existing framework so that users can trade using ETH and ERC20 tokens as their medium of exchange.
 
-IMPORTANT: This contract supports only ETH and _compliant_ ERC20 tokens. Use of the Escrow contract with non-compliant ERC20 tokens may result in permanent loss of tokens. In particular, if the token does not return `true` upon a successful call to `token.transfer` or `token.transferFrom` you should not use the token with this escrow contract. See [this article](https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca) for a deeper explanation. We will never present non-complaint tokens as a payment option in the OpenBazaar UI, but it is still possible to send (and permanently lose) such tokens by interacting with the Escrow contract through a third-party UI.
+**IMPORTANT:** This contract supports only ETH and _compliant_ ERC20 tokens. Use of the Escrow contract with non-compliant ERC20 tokens may result in permanent loss of tokens. In particular, if the token does not return `true` upon a successful call to `token.transfer` or `token.transferFrom` you should not use the token with this escrow contract. See [this article](https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca) for a deeper explanation. We will never present non-complaint tokens as a payment option in the OpenBazaar UI, but it is still possible to send (and permanently lose) such tokens by interacting with the Escrow contract through a third-party UI.
 
 ### How OpenBazaar Trades Currently Work (in UTXO land)
 

contracts/rewards/OBRewards.sol

@@ -33,9 +33,13 @@ contract OBRewards is Ownable {
     //before X + timeWindow.
     uint256 public timeWindow;
 
-    //Mapping of seller versus all buyers who received rewards by purchasing
+    //Mapping of seller to all buyers who received rewards by purchasing
     //from that seller.
-    mapping(address => address[]) sellerVsBuyers;
+    mapping(address => address[]) sellerVsBuyersArray;
+
+    //Mapping of seller and buyer to a bool indicating whether the buyers has
+    //claimed any rewards from that seller.
+    mapping(address => mapping(address => bool)) sellerVsBuyersBool;
 
     //Given a seller and a buyer, this will return the amount of tokens that
     //have been rewarded to the buyer for purchasing from the seller.
@@ -143,6 +147,10 @@ contract OBRewards is Ownable {
     function addPromotedSellers(address[] sellers) external onlyOwner {
 
         for (uint256 i = 0; i < sellers.length; i++) {
+            require(
+                sellers[i] != address(0),
+                "Zero address cannot be a promoted seller"
+            );
 
             require(
                 !promotedSellers[sellers[i]],
@@ -180,7 +188,7 @@ contract OBRewards is Ownable {
         view
         returns (address[] buyers)
     {
-        buyers = sellerVsBuyers[seller];
+        buyers = sellerVsBuyersArray[seller];
         return buyers;
     }
 
@@ -268,7 +276,7 @@ contract OBRewards is Ownable {
         view
         returns (uint256 size)
     {
-        size = sellerVsBuyers[seller].length;
+        size = sellerVsBuyersArray[seller].length;
         return size;
     }
 
@@ -286,10 +294,10 @@ contract OBRewards is Ownable {
         returns (address buyer)
     {
         require(
-            sellerVsBuyers[seller].length > index,
+            sellerVsBuyersArray[seller].length > index,
             "Array index out of bound"
         );
-        buyer = sellerVsBuyers[seller][index];
+        buyer = sellerVsBuyersArray[seller][index];
         return buyer;
     }
 
@@ -442,7 +450,10 @@ contract OBRewards is Ownable {
             }
 
             //6. Update state
-            sellerVsBuyers[seller].push(buyer);
+            if (!sellerVsBuyersBool[seller][buyer]) {
+                sellerVsBuyersBool[seller][buyer] = true;
+                sellerVsBuyersArray[seller].push(buyer);
+            }
 
             sellerVsBuyerRewards[seller][buyer] = sellerVsBuyerRewards[
                 seller

contracts/rewards/RewardsSpec.md

@@ -8,7 +8,7 @@ OB1 will occasionally hold 'promotions' where users who buy goods from "promoted
 
 When a buyer purchases from a promoted seller they become eligible to receive up to 50 OBT from the rewards contract. The buyer has a fixed amount of time (`timeWindow` seconds) after the completion of the sale to claim their reward tokens from the contract.
 
-The promotion as a whole has an `endDate`, which is set (and changeble) by the owner. After the promotion's `endDate` has come to pass, buyers can no longer claim any rewards.
+The promotion as a whole has an `endDate`, which is set (and changeable) by the owner. After the promotion's `endDate` has come to pass, buyers can no longer claim any rewards.
 
 ## Claiming Rewards
 
@@ -20,7 +20,7 @@ The buyer can claim tokens for which she is eligible in on of two ways:
 
 ## Limits on Reward Amounts
 
-Each buyer may be rewarded tokens for purchasing from a given promoted seller only once per promotion. That is, if buyer Bob buys from promoted seller Sally, he'll be eligible for up to 50 reward tokens, but if he buys from her again during the same promotion, he will not be eligible for an additional 50 reward tokens. If Bob wants to earn more tokens during the same promotion, he'd have to complete a purchase from some other promoted seller.
+Each buyer may be rewarded up to 50 reward tokens for purchasing from a given promoted seller. That is, if buyer Bob buys from promoted seller Sally, he'll be eligible for up to 50 reward tokens, but if he buys from her again during the same promotion, he will not be eligible for an additional 50 reward tokens. If Bob wants to earn more tokens during the same promotion, he'd have to complete a purchase from some other promoted seller.
 
 Additionally, the owner of the contract sets a maximum total number of tokens that can be rewarded for purchasing from any given promoted seller (`maxRewardPerSeller`). For example, suppose `maxRewardPerSeller` is 500 OBT and that each buyer is eligible to receive up to 50 OBT for purchasing from a given promoted seller. Then if Alice is a promoted seller, at most 10 buyers can receive rewards from purchasing from Alice.