claim proceeds takes a shareholder argument instead of using the msg.sender

Author: nuevoalex

source/contracts/trading/ClaimTradingProceeds.sol

@@ -22,7 +22,7 @@ import 'libraries/Extractable.sol';
 contract ClaimTradingProceeds is CashAutoConverter, Extractable, ReentrancyGuard, MarketValidator, IClaimTradingProceeds {
     using SafeMathUint256 for uint256;
 
-    function claimTradingProceeds(IMarket _market) marketIsLegit(_market) convertToAndFromCash onlyInGoodTimes nonReentrant external returns(bool) {
+    function claimTradingProceeds(IMarket _market, address _shareHolder) marketIsLegit(_market) onlyInGoodTimes nonReentrant external returns(bool) {
         if (!_market.isFinalized()) {
             _market.finalize();
         }
@@ -33,7 +33,7 @@ contract ClaimTradingProceeds is CashAutoConverter, Extractable, ReentrancyGuard
 
         for (uint8 _outcome = 0; _outcome < _market.getNumberOfOutcomes(); ++_outcome) {
             IShareToken _shareToken = _market.getShareToken(_outcome);
-            uint256 _numberOfShares = _shareToken.balanceOf(msg.sender);
+            uint256 _numberOfShares = _shareToken.balanceOf(_shareHolder);
             var (_proceeds, _shareHolderShare, _creatorShare, _reporterShare) = divideUpWinnings(_market, _outcome, _numberOfShares);
 
             if (_proceeds > 0) {
@@ -42,11 +42,12 @@ contract ClaimTradingProceeds is CashAutoConverter, Extractable, ReentrancyGuard
 
             // always destroy shares as it gives a minor gas refund and is good for the network
             if (_numberOfShares > 0) {
-                _shareToken.destroyShares(msg.sender, _numberOfShares);
-                logTradingProceedsClaimed(_market, _shareToken, msg.sender, _numberOfShares, _shareHolderShare);
+                _shareToken.destroyShares(_shareHolder, _numberOfShares);
+                logTradingProceedsClaimed(_market, _shareToken, _shareHolder, _numberOfShares, _shareHolderShare);
             }
             if (_shareHolderShare > 0) {
-                require(_denominationToken.transferFrom(_market, msg.sender, _shareHolderShare));
+                require(_denominationToken.transferFrom(_market, this, _shareHolderShare));
+                _denominationToken.withdrawEtherTo(_shareHolder, _shareHolderShare);
             }
             if (_creatorShare > 0) {
                 require(_denominationToken.transferFrom(_market, _market.getMarketCreatorMailbox(), _creatorShare));

tests/test_gas_costs.py

@@ -86,7 +86,7 @@ def test_winningShareRedmption(localFixture, cash, market):
     finalizeMarket(localFixture, market, [0,market.getNumTicks()])
 
     with PrintGasUsed(localFixture, "ClaimTradingProceeds:claimTradingProceeds", CLAIM_PROCEEDS):
-        claimTradingProceeds.claimTradingProceeds(market.address, sender = tester.k1)
+        claimTradingProceeds.claimTradingProceeds(market.address, tester.a1)
 
 def test_initial_report(localFixture, universe, cash, market):
     proceedToDesignatedReporting(localFixture, market)

tests/trading/test_claimTradingProceeds.py

@@ -100,10 +100,10 @@ def test_redeem_shares_in_binary_market(kitchenSinkFixture, universe, cash, mark
         with TokenDelta(cash, expectedReporterFees, universe.getOrCreateNextFeeWindow(), "Reporter fees not paid"):
             # redeem shares with a1
             initialLongHolderETH = kitchenSinkFixture.chain.head_state.get_balance(tester.a1)
-            claimTradingProceeds.claimTradingProceeds(market.address, sender = tester.k1)
+            claimTradingProceeds.claimTradingProceeds(market.address, tester.a1)
             # redeem shares with a2
             initialShortHolderETH = kitchenSinkFixture.chain.head_state.get_balance(tester.a2)
-            claimTradingProceeds.claimTradingProceeds(market.address, sender = tester.k2)
+            claimTradingProceeds.claimTradingProceeds(market.address, tester.a2)
 
     # Confirm claim proceeds logging works correctly
     assert len(logs) == 6
@@ -146,10 +146,10 @@ def test_redeem_shares_in_categorical_market(kitchenSinkFixture, universe, cash,
 
     # redeem shares with a1
     initialLongHolderETH = kitchenSinkFixture.chain.head_state.get_balance(tester.a1)
-    claimTradingProceeds.claimTradingProceeds(market.address, sender = tester.k1)
+    claimTradingProceeds.claimTradingProceeds(market.address, tester.a1)
     # redeem shares with a2
     initialShortHolderETH = kitchenSinkFixture.chain.head_state.get_balance(tester.a2)
-    claimTradingProceeds.claimTradingProceeds(market.address, sender = tester.k2)
+    claimTradingProceeds.claimTradingProceeds(market.address, tester.a2)
 
     # assert a1 ends up with cash (minus fees) and a2 does not
     assert kitchenSinkFixture.chain.head_state.get_balance(tester.a1) == initialLongHolderETH + expectedPayout + 1 # rounding errors from fees
@@ -183,10 +183,10 @@ def test_redeem_shares_in_scalar_market(kitchenSinkFixture, universe, cash, scal
 
     # redeem shares with a1
     initialLongHolderETH = kitchenSinkFixture.chain.head_state.get_balance(tester.a1)
-    claimTradingProceeds.claimTradingProceeds(market.address, sender = tester.k1)
+    claimTradingProceeds.claimTradingProceeds(market.address, tester.a1)
     # redeem shares with a2
     initialShortHolderETH = kitchenSinkFixture.chain.head_state.get_balance(tester.a2)
-    claimTradingProceeds.claimTradingProceeds(market.address, sender = tester.k2)
+    claimTradingProceeds.claimTradingProceeds(market.address, tester.a2)
 
     # assert a1 ends up with cash (minus fees) and a2 does not
     assert kitchenSinkFixture.chain.head_state.get_balance(tester.a1) == initialLongHolderETH + expectedPayout * 3 / 4
@@ -214,14 +214,14 @@ def test_reedem_failure(kitchenSinkFixture, cash, market):
 
     # market not finalized
     with raises(TransactionFailed):
-        claimTradingProceeds.claimTradingProceeds(market.address, sender = tester.k1)
+        claimTradingProceeds.claimTradingProceeds(market.address, tester.a1)
     # finalize the market
     assert market.finalize()
     # waiting period not over
     with raises(TransactionFailed):
-        claimTradingProceeds.claimTradingProceeds(market.address, sender = tester.k1)
+        claimTradingProceeds.claimTradingProceeds(market.address, tester.a1)
 
     # set timestamp to 3 days later (waiting period)
     kitchenSinkFixture.contracts["Time"].incrementTimestamp(long(timedelta(days = 3, seconds = 1).total_seconds()))
     # validate that everything else is OK
-    assert claimTradingProceeds.claimTradingProceeds(market.address, sender = tester.k1)
+    assert claimTradingProceeds.claimTradingProceeds(market.address, tester.a1)