Fix escrow payouts (#3261)

portuu3 · flopez7 · web-flow · commit ee21c4d174c6 · 2025-05-21T13:57:56.000+02:00
* fix payouts when fee calculation is truncated

* fix tests and use faker in the root of the repository

* document escrow contract and improve tests

* Delete unnecessary check for BULK_MAX_VALUE

* remove unused modifier

* Undo faker package changes for updating yarn

---------

Co-authored-by: Francisco López &lt;francislopez977@gmail.com&gt;
diff --git a/packages/apps/job-launcher/server/package.json b/packages/apps/job-launcher/server/package.json
@@ -70,7 +70,7 @@
     "zxcvbn": "^4.4.2"
   },
   "devDependencies": {
-    "@faker-js/faker": "^9.5.0",
+    "@faker-js/faker": "^9.8.0",
     "@golevelup/ts-jest": "^0.6.1",
     "@nestjs/cli": "^10.3.2",
     "@nestjs/schematics": "^11.0.2",
diff --git a/packages/apps/reputation-oracle/server/package.json b/packages/apps/reputation-oracle/server/package.json
@@ -72,7 +72,7 @@
     "zxcvbn": "^4.4.2"
   },
   "devDependencies": {
-    "@faker-js/faker": "^9.4.0",
+    "@faker-js/faker": "^9.8.0",
     "@golevelup/ts-jest": "^0.6.1",
     "@nestjs/cli": "^10.3.2",
     "@nestjs/schematics": "^11.0.2",
diff --git a/packages/core/contracts/Escrow.sol b/packages/core/contracts/Escrow.sol
@@ -8,13 +8,18 @@ import '@openzeppelin/contracts/utils/ReentrancyGuard.sol';
 
 import './interfaces/IEscrow.sol';
 
+/**
+ * @title Escrow Contract
+ * @dev This contract manages the lifecycle of an escrow, including funding,
+ * setup, payouts, and completion. It supports trusted handlers and oracles
+ * for managing the escrow process.
+ */
 contract Escrow is IEscrow, ReentrancyGuard {
     bytes4 private constant FUNC_SELECTOR_BALANCE_OF =
         bytes4(keccak256('balanceOf(address)'));
 
     string constant ERROR_ZERO_ADDRESS = 'Escrow: zero address';
 
-    uint256 private constant BULK_MAX_VALUE = 1e9 * (10 ** 18);
     uint32 private constant BULK_MAX_COUNT = 100;
 
     event TrustedHandlerAdded(address _handler);
@@ -73,9 +78,16 @@ contract Escrow is IEscrow, ReentrancyGuard {
     mapping(address => bool) public areTrustedHandlers;
 
     uint256 public remainingFunds;
-
     uint256 public reservedFunds;
 
+    /**
+     * @dev Constructor to initialize the escrow contract.
+     * @param _token Address of the token used in the escrow.
+     * @param _launcher Address of the launcher (creator) of the escrow.
+     * @param _canceler Address of the canceler who can cancel the escrow.
+     * @param _duration Duration of the escrow in seconds.
+     * @param _handlers Array of trusted handler addresses.
+     */
     constructor(
         address _token,
         address _launcher,
@@ -97,23 +109,38 @@ contract Escrow is IEscrow, ReentrancyGuard {
         _addTrustedHandlers(_handlers);
     }
 
+    /**
+     * @dev Returns the balance of the escrow contract for the main token.
+     */
     function getBalance() public view returns (uint256) {
         return getTokenBalance(token);
     }
 
+    /**
+     * @dev Returns the balance of the escrow contract for a specific token.
+     * @param _token Address of the token to check the balance for.
+     */
     function getTokenBalance(address _token) public view returns (uint256) {
         (bool success, bytes memory returnData) = _token.staticcall(
             abi.encodeWithSelector(FUNC_SELECTOR_BALANCE_OF, address(this))
         );
         return success ? abi.decode(returnData, (uint256)) : 0;
     }
 
+    /**
+     * @dev Adds trusted handlers to the contract.
+     * @param _handlers Array of addresses to be added as trusted handlers.
+     */
     function addTrustedHandlers(
         address[] memory _handlers
     ) public override trusted {
         _addTrustedHandlers(_handlers);
     }
 
+    /**
+     * @dev Internal function to add trusted handlers.
+     * @param _handlers Array of addresses to be added as trusted handlers.
+     */
     function _addTrustedHandlers(address[] memory _handlers) internal {
         for (uint256 i = 0; i < _handlers.length; i++) {
             require(_handlers[i] != address(0), ERROR_ZERO_ADDRESS);
@@ -122,9 +149,17 @@ contract Escrow is IEscrow, ReentrancyGuard {
         }
     }
 
-    // The escrower puts the Token in the contract without an agentless
-    // and assigsn a reputation oracle to payout the bounty of size of the
-    // amount specified
+    /**
+     * @dev Sets up the escrow with oracles and manifest details.
+     * @param _reputationOracle Address of the reputation oracle.
+     * @param _recordingOracle Address of the recording oracle.
+     * @param _exchangeOracle Address of the exchange oracle.
+     * @param _reputationOracleFeePercentage Fee percentage for the reputation oracle.
+     * @param _recordingOracleFeePercentage Fee percentage for the recording oracle.
+     * @param _exchangeOracleFeePercentage Fee percentage for the exchange oracle.
+     * @param _url URL of the manifest.
+     * @param _hash Hash of the manifest.
+     */
     function setup(
         address _reputationOracle,
         address _recordingOracle,
@@ -182,6 +217,10 @@ contract Escrow is IEscrow, ReentrancyGuard {
         emit Fund(remainingFunds);
     }
 
+    /**
+     * @dev Cancels the escrow and transfers remaining funds to the canceler.
+     * @return bool indicating success of the cancellation.
+     */
     function cancel()
         public
         override
@@ -195,6 +234,11 @@ contract Escrow is IEscrow, ReentrancyGuard {
         return true;
     }
 
+    /**
+     * @dev Withdraws excess funds from the escrow for a specific token.
+     * @param _token Address of the token to withdraw.
+     * @return bool indicating success of the withdrawal.
+     */
     function withdraw(
         address _token
     ) public override trusted nonReentrant returns (bool) {
@@ -213,15 +257,18 @@ contract Escrow is IEscrow, ReentrancyGuard {
         return true;
     }
 
+    /**
+     * @dev Completes the escrow, transferring remaining funds to the launcher.
+     */
     function complete() external override notExpired trustedOrReputationOracle {
         require(
             status == EscrowStatuses.Paid || status == EscrowStatuses.Partial,
             'Escrow not in Paid or Partial state'
         );
-        _complete();
+        _finalize();
     }
 
-    function _complete() private {
+    function _finalize() private {
         if (remainingFunds > 0) {
             _safeTransfer(token, launcher, remainingFunds);
             remainingFunds = 0;
@@ -236,6 +283,11 @@ contract Escrow is IEscrow, ReentrancyGuard {
         }
     }
 
+    /**
+     * @dev Stores intermediate results during the escrow process.
+     * @param _url URL of the intermediate results.
+     * @param _hash Hash of the intermediate results.
+     */
     function storeResults(
         string memory _url,
         string memory _hash,
@@ -270,22 +322,13 @@ contract Escrow is IEscrow, ReentrancyGuard {
     }
 
     /**
-     * @dev Performs bulk payout to multiple workers
-     * Escrow needs to be completed / cancelled, so that it can be paid out.
-     * Every recipient is paid with the amount after reputation and recording oracle fees taken out.
-     * If the amount is less than the fee, the recipient is not paid.
-     * If the fee is zero, reputation, and recording oracle are not paid.
-     * Payout will fail if any of the transaction fails.
-     * If the escrow is fully paid out, meaning that the balance of the escrow is 0, it'll set as Paid.
-     * If the escrow is partially paid out, meaning that the escrow still has remaining balance, it'll set as Partial.
-     * This contract is only callable if the contract is not broke, not launched, not paid, not expired, by trusted parties.
-     *
-     * @param _recipients Array of recipients
+     * @dev Performs bulk payout to multiple recipients with oracle fees deducted.
+     * @param _recipients Array of recipient addresses.
      * @param _amounts Array of amounts to be paid to each recipient.
-     * @param _url URL storing results as transaction details
-     * @param _hash Hash of the results
-     * @param _txId Transaction ID
-     * @param forceComplete Boolean parameter indicating if remaining balance should be transferred to the escrow creator
+     * @param _url URL storing results as transaction details.
+     * @param _hash Hash of the results.
+     * @param _txId Transaction ID.
+     * @param forceComplete Boolean indicating if remaining balance should be transferred to the launcher.
      */
     function bulkPayOut(
         address[] memory _recipients,
@@ -314,60 +357,80 @@ contract Escrow is IEscrow, ReentrancyGuard {
                 status != EscrowStatuses.Cancelled,
             'Invalid status'
         );
-
-        uint256 aggregatedBulkAmount = 0;
-        for (uint256 i = 0; i < _amounts.length; i++) {
-            uint256 amount = _amounts[i];
-            require(amount > 0, 'Amount should be greater than zero');
-            aggregatedBulkAmount += amount;
-        }
-        require(aggregatedBulkAmount < BULK_MAX_VALUE, 'Bulk value too high');
         require(
-            aggregatedBulkAmount <= reservedFunds,
-            'Not enough reserved funds'
+            bytes(_url).length != 0 && bytes(_hash).length != 0,
+            'URL or hash is empty'
         );
 
-        reservedFunds -= aggregatedBulkAmount;
-        remainingFunds -= aggregatedBulkAmount;
-
-        require(bytes(_url).length != 0, "URL can't be empty");
-        require(bytes(_hash).length != 0, "Hash can't be empty");
+        uint256 totalBulkAmount = 0;
+        uint256 totalReputationOracleFee = 0;
+        uint256 totalRecordingOracleFee = 0;
+        uint256 totalExchangeOracleFee = 0;
 
-        finalResultsUrl = _url;
-        finalResultsHash = _hash;
+        for (uint256 i = 0; i < _recipients.length; i++) {
+            uint256 amount = _amounts[i];
+            require(amount > 0, 'Amount should be greater than zero');
+            totalBulkAmount += amount;
+            totalReputationOracleFee +=
+                (reputationOracleFeePercentage * amount) /
+                100;
+            totalRecordingOracleFee +=
+                (recordingOracleFeePercentage * amount) /
+                100;
+            totalExchangeOracleFee +=
+                (exchangeOracleFeePercentage * amount) /
+                100;
+        }
+        require(totalBulkAmount <= reservedFunds, 'Not enough reserved funds');
 
-        uint256 totalFeePercentage = reputationOracleFeePercentage +
-            recordingOracleFeePercentage +
-            exchangeOracleFeePercentage;
+        uint256 paidReputation = 0;
+        uint256 paidRecording = 0;
+        uint256 paidExchange = 0;
 
         for (uint256 i = 0; i < _recipients.length; i++) {
             uint256 amount = _amounts[i];
-            uint256 amountFee = (totalFeePercentage * amount) / 100;
-            _safeTransfer(token, _recipients[i], amount - amountFee);
-        }
+            uint256 reputationOracleFee = (reputationOracleFeePercentage *
+                amount) / 100;
+            uint256 recordingOracleFee = (recordingOracleFeePercentage *
+                amount) / 100;
+            uint256 exchangeOracleFee = (exchangeOracleFeePercentage * amount) /
+                100;
+
+            if (i == _recipients.length - 1) {
+                reputationOracleFee = totalReputationOracleFee - paidReputation;
+                recordingOracleFee = totalRecordingOracleFee - paidRecording;
+                exchangeOracleFee = totalExchangeOracleFee - paidExchange;
+            }
+
+            paidReputation += reputationOracleFee;
+            paidRecording += recordingOracleFee;
+            paidExchange += exchangeOracleFee;
 
-        // Transfer oracle fees
-        if (reputationOracleFeePercentage > 0) {
             _safeTransfer(
                 token,
-                reputationOracle,
-                (reputationOracleFeePercentage * aggregatedBulkAmount) / 100
+                _recipients[i],
+                amount -
+                    reputationOracleFee -
+                    recordingOracleFee -
+                    exchangeOracleFee
             );
         }
+
+        // Transfer oracle fees
+        if (reputationOracleFeePercentage > 0) {
+            _safeTransfer(token, reputationOracle, totalReputationOracleFee);
+        }
         if (recordingOracleFeePercentage > 0) {
-            _safeTransfer(
-                token,
-                recordingOracle,
-                (recordingOracleFeePercentage * aggregatedBulkAmount) / 100
-            );
+            _safeTransfer(token, recordingOracle, totalRecordingOracleFee);
         }
         if (exchangeOracleFeePercentage > 0) {
-            _safeTransfer(
-                token,
-                exchangeOracle,
-                (exchangeOracleFeePercentage * aggregatedBulkAmount) / 100
-            );
+            _safeTransfer(token, exchangeOracle, totalExchangeOracleFee);
         }
+        remainingFunds -= totalBulkAmount;
+        reservedFunds -= totalBulkAmount;
+
+        finalResultsUrl = _url;
+        finalResultsHash = _hash;
 
         if (remainingFunds == 0 || forceComplete) {
             emit BulkTransferV2(
@@ -377,7 +440,7 @@ contract Escrow is IEscrow, ReentrancyGuard {
                 false,
                 finalResultsUrl
             );
-            _complete();
+            _finalize();
         } else {
             if (status != EscrowStatuses.ToCancel) {
                 status = EscrowStatuses.Partial;
@@ -394,13 +457,11 @@ contract Escrow is IEscrow, ReentrancyGuard {
 
     /**
      * @dev Overloaded function to perform bulk payout with default forceComplete set to false.
-     * Calls the main bulkPayout function with forceComplete as false.
-     *
-     * @param _recipients Array of recipients
+     * @param _recipients Array of recipient addresses.
      * @param _amounts Array of amounts to be paid to each recipient.
-     * @param _url URL storing results as transaction details
-     * @param _hash Hash of the results
-     * @param _txId Transaction ID
+     * @param _url URL storing results as transaction details.
+     * @param _hash Hash of the results.
+     * @param _txId Transaction ID.
      */
     function bulkPayOut(
         address[] memory _recipients,
@@ -412,6 +473,12 @@ contract Escrow is IEscrow, ReentrancyGuard {
         bulkPayOut(_recipients, _amounts, _url, _hash, _txId, false);
     }
 
+    /**
+     * @dev Internal function to safely transfer tokens.
+     * @param _token Address of the token to transfer.
+     * @param to Address of the recipient.
+     * @param value Amount to transfer.
+     */
     function _safeTransfer(address _token, address to, uint256 value) internal {
         SafeERC20.safeTransfer(IERC20(_token), to, value);
     }
diff --git a/packages/core/package.json b/packages/core/package.json
@@ -51,6 +51,7 @@
   ],
   "license": "MIT",
   "devDependencies": {
+    "@faker-js/faker": "^9.8.0",
     "@nomicfoundation/hardhat-chai-matchers": "^2.0.7",
     "@nomicfoundation/hardhat-ethers": "^3.0.5",
     "@nomicfoundation/hardhat-network-helpers": "^1.0.12",
diff --git a/packages/core/test/Escrow-USDT.ts b/packages/core/test/Escrow-USDT.ts
diff --git a/packages/core/test/Escrow.ts b/packages/core/test/Escrow.ts
