Merge pull request #14 from unknownunknown1/master

fix(Escrow): remove whitelist, small fixes

contracts/MultipleArbitrableTokenTransactionWithAppeals.sol

@@ -1,12 +1,13 @@
+// SPDX-License-Identifier: MIT
+
 /**
  *  @authors: [@unknownunknown1, @fnanni-0, @shalzz]
  *  @reviewers: [@ferittuncer*, @epiqueras*, @nix1g*, @unknownunknown1, @alcercu*, @fnanni-0*]
  *  @auditors: []
  *  @bounties: []
  */
 
-pragma solidity ^0.8;
-pragma experimental ABIEncoderV2;
+pragma solidity 0.8.9;
 
 import "@kleros/erc-792/contracts/IArbitrable.sol";
 import "@kleros/erc-792/contracts/IArbitrator.sol";
@@ -20,6 +21,11 @@ import "./libraries/CappedMath.sol";
  *  This can be used for buying goods, services and for paying freelancers.
  *  Parties are identified as "sender" and "receiver".
  *  This version of the contract supports appeal crowdfunding.
+ *  Note that the contract expects the tokens to have standard ERC20 behaviour.
+ *  The tokens that don't conform to this type of behaviour should be filtered by the UI.
+ *  Tokens should not reenter or allow recipients to refuse the transfer.
+ *  Also note that for ETH send() function is used deliberately instead of transfer() 
+ *  to avoid blocking the flow with reverting fallback.
  */
 contract MultipleArbitrableTokenTransactionWithAppeals is IArbitrable, IEvidence {
     using CappedMath for uint256;

contracts/MultipleArbitrableTokenTransactionWithFee.sol

@@ -1,12 +1,13 @@
+// SPDX-License-Identifier: MIT
+
 /**
  *  @authors: [@unknownunknown1, @fnanni-0, @shalzz, @remedcu]
  *  @reviewers: []
  *  @auditors: []
  *  @bounties: []
  */
 
-pragma solidity ^0.8;
-pragma experimental ABIEncoderV2;
+pragma solidity 0.8.9;
 
 import "@kleros/erc-792/contracts/IArbitrable.sol";
 import "@kleros/erc-792/contracts/IArbitrator.sol";
@@ -21,6 +22,11 @@ import "./libraries/CappedMath.sol";
  *  Parties are identified as "sender" and "receiver".
  *  This version of the contract supports appeal crowdfunding and platform fees.
  *  A fee is associated with each successful payment to receiver.
+ *  Note that the contract expects the tokens to have standard ERC20 behaviour.
+ *  The tokens that don't conform to this type of behaviour should be filtered by the UI.
+ *  Tokens should not reenter or allow recipients to refuse the transfer.
+ *  Also note that for ETH send() function is used deliberately instead of transfer() 
+ *  to avoid blocking the flow with reverting fallback.
  */
 contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
     using CappedMath for uint256;
@@ -115,9 +121,6 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
     /// @dev Stores the hashes of all transactions.
     bytes32[] public transactionHashes;
 
-    /// @dev Tokens that were whitelisted by deployer and feeRecipient.
-    mapping(IERC20 => bool) public whitelisted;
-
     /// @dev Maps a transactionID to its respective appeal rounds.
     mapping(uint256 => Round[]) public roundsByTransactionID;
 
@@ -161,12 +164,6 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
      */
     event FeeBasisPointChanged(uint16 _oldFeeRecipientBasisPoint, uint16 _newFeeRecipientBasisPoint);
 
-    /** @dev To be emitted when the whitelist was changed.
-     *  @param _token The token that was either added or removed from whitelist.
-     *  @param _allow Whether added or removed.
-     */
-    event WhitelistChanged(IERC20 _token, bool _allow);
-
     /** @dev Indicate that a party has to pay a fee or would otherwise be considered as losing.
      *  @param _transactionID The index of the transaction.
      *  @param _party The party who has to pay.
@@ -225,7 +222,6 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
      *  @param _arbitratorExtraData Extra data for the arbitrator.
      *  @param _feeRecipient Address which receives a share of receiver payment.
      *  @param _feeRecipientBasisPoint The share of fee to be received by the feeRecipient.
-     *  @param _whitelistedTokens Array of the default whitelisted tokens.
      *  @param _feeTimeout Arbitration fee timeout for the parties.
      *  @param _settlementTimeout Settlement timeout for the parties.
      *  @param _sharedStakeMultiplier Multiplier of the appeal cost that the
@@ -241,7 +237,6 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
         bytes memory _arbitratorExtraData,
         address _feeRecipient,
         uint16 _feeRecipientBasisPoint,
-        IERC20[] memory _whitelistedTokens,
         uint256 _feeTimeout,
         uint256 _settlementTimeout,
         uint256 _sharedStakeMultiplier,
@@ -259,13 +254,6 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
         sharedStakeMultiplier = _sharedStakeMultiplier;
         winnerStakeMultiplier = _winnerStakeMultiplier;
         loserStakeMultiplier = _loserStakeMultiplier;
-
-        IERC20 token;
-        for (uint256 i = 0; i < _whitelistedTokens.length; i++) {
-            token = _whitelistedTokens[i];
-            whitelisted[token] = true;
-            emit WhitelistChanged(token, true);
-        }
     }
 
     modifier onlyValidTransaction(uint256 _transactionID, Transaction memory _transaction) {
@@ -306,22 +294,6 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
         feeRecipientData.feeRecipientBasisPoint = _newFeeRecipientBasisPoint;      
     }
 
-    /** @dev Add/remove tokens from whitelist.
-     *  @param _tokens Array of tokens.
-     *  @param _allow Whether to add or remove from whitelist.
-     */
-    function changeWhitelist(IERC20[] memory _tokens, bool _allow) external {
-        require(msg.sender == feeRecipientData.feeRecipient, "The caller must be the current Fee Recipient");
-        IERC20 token;
-        for (uint256 i = 0; i < _tokens.length; i++) {
-            token = _tokens[i];
-            if (whitelisted[token] != _allow) {
-                whitelisted[token] = _allow;
-                emit WhitelistChanged(token, _allow);
-            }
-        }
-    }
-
     /** @dev Create a transaction. UNTRUSTED.
      *  @param _amount The amount of tokens in this transaction.
      *  @param _token The ERC20 token contract.
@@ -337,7 +309,6 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
         address payable _receiver,
         string calldata _metaEvidence
     ) external returns (uint256 transactionID) {
-        require(whitelisted[_token], "Token is not whitelisted");
         // Transfers token from sender wallet to contract.
         require(
             _token.transferFrom(msg.sender, address(this), _amount),
@@ -386,7 +357,10 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
 
         uint256 feeAmount = calculateFeeRecipientAmount(_amount);
         // Tokens should not reenter or allow recipients to refuse the transfer.
-        _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount); // It is the responsibility of the feeRecipient to accept Token.
+        require(
+            _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount),
+            "The `transfer` function must not fail."
+        );
 
         require(
             _transaction.token.transfer(_transaction.receiver, _amount - feeAmount),
@@ -444,7 +418,10 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
         transactionHashes[_transactionID - 1] = hashTransactionState(_transaction);
 
         uint256 feeAmount = calculateFeeRecipientAmount(amount);
-        _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount);
+        require(
+            _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount),
+            "The `transfer` function must not fail."
+        );
 
         require(
             _transaction.token.transfer(_transaction.receiver, amount - feeAmount),
@@ -533,7 +510,10 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
         transactionHashes[_transactionID - 1] = hashTransactionState(_transaction); // solhint-disable-line
 
         uint256 feeAmount = calculateFeeRecipientAmount(settlementAmount);
-        _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount);
+        require(
+            _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount),
+            "The `transfer` function must not fail."
+        );
 
         require(
             _transaction.token.transfer(_transaction.sender, remainingAmount),
@@ -726,7 +706,10 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
         transactionHashes[_transactionID - 1] = hashTransactionState(_transaction); // solhint-disable-line
 
         uint256 feeAmount = calculateFeeRecipientAmount(amount);
-        _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount);
+        require(
+            _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount),
+            "The `transfer` function must not fail."
+        );
         require(
             _transaction.token.transfer(_transaction.receiver, amount - feeAmount),
             "The `transfer` function must not fail."
@@ -913,7 +896,7 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
         if (!round.hasPaid[_side]) {
             reward = round.contributions[_beneficiary][_side];
         } else if (!round.hasPaid[_finalRuling]) {
-            // Reimburse unspent fees proportionally if the ultimate winner didn't pay appeal fees fully.
+            // When the ultimate winner didn't fully pay appeal fees, proportionally reimburse unspent fees of the fully funded sides.
             // Note that if only one side is funded it will become a winner and this part of the condition won't be reached.
             reward = round.fundedSides.length > 1
                 ? (round.contributions[_beneficiary][_side] * round.feeRewards) /
@@ -1072,7 +1055,10 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
             if (settlementSender != 0) {
                 feeAmount = calculateFeeRecipientAmount(settlementSender);
                 // Tokens should not reenter or allow recipients to refuse the transfer.
-                _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount); // It is the responsibility of the feeRecipient to accept Token.
+                require(
+                    _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount),
+                    "The `transfer` function must not fail."
+                );
                 require(
                     _transaction.token.transfer(_transaction.sender, amount - settlementSender),
                     "The `transfer` function must not fail."
@@ -1096,7 +1082,10 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
             if (settlementReceiver != 0) {
                 feeAmount = calculateFeeRecipientAmount(settlementReceiver);
                 // Tokens should not reenter or allow recipients to refuse the transfer.
-                _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount); // It is the responsibility of the feeRecipient to accept Token.
+                require(
+                    _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount),
+                    "The `transfer` function must not fail."
+                );
                 require(
                     _transaction.token.transfer(_transaction.sender, amount - settlementReceiver),
                     "The `transfer` function must not fail."
@@ -1108,7 +1097,10 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
                 emit FeeRecipientPaymentInToken(_transactionID, feeAmount, _transaction.token);
             } else {
                 feeAmount = calculateFeeRecipientAmount(amount);
-                _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount);
+                require(
+                    _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount),
+                    "The `transfer` function must not fail."
+                );
                 require(
                     _transaction.token.transfer(_transaction.receiver, amount - feeAmount),
                     "The `transfer` function must not fail."
@@ -1125,7 +1117,10 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
             uint256 splitAmount = amount / 2;
 
             feeAmount = calculateFeeRecipientAmount(splitAmount);
-            _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount);
+            require(
+                _transaction.token.transfer(feeRecipientData.feeRecipient, feeAmount),
+                "The `transfer` function must not fail."
+            );
             require(
                 _transaction.token.transfer(_transaction.receiver, splitAmount - feeAmount),
                 "The `transfer` function must not fail."
@@ -1178,7 +1173,7 @@ contract MultipleArbitrableTokenTransactionWitFee is IArbitrable, IEvidence {
             if (!round.hasPaid[uint256(_side)]) {
                 total += round.contributions[_beneficiary][uint256(_side)];
             } else if (!round.hasPaid[finalRuling]) {
-                // Reimburse unspent fees proportionally if the ultimate winner didn't pay appeal fees fully.
+                // When the ultimate winner didn't fully pay appeal fees, proportionally reimburse unspent fees of the fully funded sides.
                 // Note that if only one side is funded it will become a winner and this part of the condition won't be reached.
                 total += round.fundedSides.length > 1
                 ? (round.contributions[_beneficiary][uint256(_side)] * round.feeRewards) /

contracts/MultipleArbitrableTransactionWithAppeals.sol

@@ -1,18 +1,25 @@
+// SPDX-License-Identifier: MIT
+
 /**
  *  @authors: [@unknownunknown1, @fnanni-0, @shalzz]
  *  @reviewers: [@ferittuncer*, @epiqueras*, @nix1g*, @unknownunknown1, @alcercu*, @fnanni-0*]
  *  @auditors: []
  *  @bounties: []
  */
 
-pragma solidity ^0.8;
-pragma experimental ABIEncoderV2;
+pragma solidity 0.8.9;
 
 import "@kleros/erc-792/contracts/IArbitrable.sol";
 import "@kleros/erc-792/contracts/IArbitrator.sol";
 import "@kleros/erc-792/contracts/erc-1497/IEvidence.sol";
 import "./libraries/CappedMath.sol";
 
+/**
+ *  @title MultipleArbitrableTransactionWithAppeals
+ *  @dev Escrow contract with appeal support.
+ *  Note that send() function is used deliberately instead of transfer() 
+ *  to avoid blocking the flow with reverting fallback.
+ */
 contract MultipleArbitrableTransactionWithAppeals is IArbitrable, IEvidence {
     using CappedMath for uint256;
     // **************************** //
@@ -436,7 +443,7 @@ contract MultipleArbitrableTransactionWithAppeals is IArbitrable, IEvidence {
 
         uint256 arbitrationCost = arbitrator.arbitrationCost(arbitratorExtraData);
         _transaction.senderFee += msg.value;
-        // Require that the total pay at least the arbitration cost.
+        // Require that the total paid to be at least the arbitration cost.
         require(
             _transaction.senderFee >= arbitrationCost,
             "The sender fee must cover arbitration costs."

contracts/MultipleArbitrableTransactionWithFee.sol

@@ -1,12 +1,13 @@
+// SPDX-License-Identifier: MIT
+
 /**
  *  @authors: [@unknownunknown1, @fnanni-0, @shalzz, @remedcu]
  *  @reviewers: []
  *  @auditors: []
  *  @bounties: []
  */
 
-pragma solidity ^0.8;
-pragma experimental ABIEncoderV2;
+pragma solidity 0.8.9;
 
 import "@kleros/erc-792/contracts/IArbitrable.sol";
 import "@kleros/erc-792/contracts/IArbitrator.sol";
@@ -17,6 +18,8 @@ import "./libraries/CappedMath.sol";
  *  @title MultipleArbitrableTransactionWithFee
  *  @dev Escrow contract that demands platform fees. Also supports appeals.
  *  A fee is associated with each successful payment to receiver.
+ *  Note that send() function is used deliberately instead of transfer() 
+ *  to avoid blocking the flow with reverting fallback.
  */
 
 contract MultipleArbitrableTransactionWithFee is IArbitrable, IEvidence {
@@ -506,7 +509,7 @@ contract MultipleArbitrableTransactionWithFee is IArbitrable, IEvidence {
 
         uint256 arbitrationCost = arbitrator.arbitrationCost(arbitratorExtraData);
         _transaction.senderFee += msg.value;
-        // Require that the total pay at least the arbitration cost.
+        // Require that the total paid to be at least the arbitration cost.
         require(
             _transaction.senderFee >= arbitrationCost,
             "The sender fee must cover arbitration costs."
@@ -823,7 +826,7 @@ contract MultipleArbitrableTransactionWithFee is IArbitrable, IEvidence {
         if (!round.hasPaid[_side]) {
             reward = round.contributions[_beneficiary][_side];
         } else if (!round.hasPaid[_finalRuling]) {
-            // Reimburse unspent fees proportionally if the ultimate winner didn't pay appeal fees fully.
+            // When the ultimate winner didn't fully pay appeal fees, proportionally reimburse unspent fees of the fully funded sides.
             // Note that if only one side is funded it will become a winner and this part of the condition won't be reached.
             reward = round.fundedSides.length > 1
                 ? (round.contributions[_beneficiary][_side] * round.feeRewards) /
@@ -1049,7 +1052,7 @@ contract MultipleArbitrableTransactionWithFee is IArbitrable, IEvidence {
             if (!round.hasPaid[uint256(_side)]) {
                 total += round.contributions[_beneficiary][uint256(_side)];
             } else if (!round.hasPaid[finalRuling]) {
-                // Reimburse unspent fees proportionally if the ultimate winner didn't pay appeal fees fully.
+                // When the ultimate winner didn't fully pay appeal fees, proportionally reimburse unspent fees of the fully funded sides.
                 // Note that if only one side is funded it will become a winner and this part of the condition won't be reached.
                 total += round.fundedSides.length > 1
                 ? (round.contributions[_beneficiary][uint256(_side)] * round.feeRewards) /