Use two decimals for delayed scale factor (#114)

Co-authored-by: Leo <[email protected]>
Co-authored-by: Gustavo Gonzalez <[email protected]>

Author: 3 people

.github/workflows/test.yml

@@ -10,9 +10,6 @@ env:
 
 jobs:
   check:
-    strategy:
-      fail-fast: true
-
     name: Foundry project
     runs-on: ubuntu-latest
     steps:
@@ -23,7 +20,7 @@ jobs:
       - name: Install Foundry
         uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly
+          version: stable
 
       - name: Show Forge version
         run: |

src/libs/LibPercentage.sol

@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.28;
+
+import "@openzeppelin/contracts/utils/math/SafeCast.sol";
+
+library LibPercentage {
+    using SafeCast for uint256;
+
+    // COMMON PRECISION AMOUNTS (https://muens.io/solidity-percentages)
+    uint256 constant BASIS_POINTS = 10_000;
+    uint256 constant PERCENT = 100;
+
+    /// @dev Calculates the percentage of a given value scaling by `precision` to limit rounding loss
+    /// @param value The number to scale
+    /// @param percentage The percentage expressed in `precision` units.
+    /// @param precision The precision of `percentage` (e.g. percentage 5000 with BASIS_POINTS precision is 50%).
+    /// @return _ The scaled value
+    function scaleBy(uint256 value, uint16 percentage, uint256 precision) internal pure returns (uint96) {
+        return (value * percentage / precision).toUint96();
+    }
+
+    /// @dev Calculates the percentage (represented in basis points) of a given value
+    /// @param value The number to scale
+    /// @param percentage The percentage expressed in basis points
+    /// @return _ The scaled value
+    function scaleByBPS(uint256 value, uint16 percentage) internal pure returns (uint96) {
+        return scaleBy(value, percentage, BASIS_POINTS);
+    }
+
+    /// @dev Calculates the percentage of a given value
+    /// @param value The number to scale
+    /// @param percentage The percentage to single-percentage precision (e.g. percentage 50 is 50%)
+    /// @return _ The scaled value
+    function scaleByPercentage(uint256 value, uint16 percentage) internal pure returns (uint96) {
+        return scaleBy(value, percentage, BASIS_POINTS);
+    }
+}

src/protocol/BaseProverManager.sol

@@ -1,14 +1,17 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.28;
 
+import {LibPercentage} from "../libs/LibPercentage.sol";
 import {ICheckpointTracker} from "./ICheckpointTracker.sol";
 import {IProposerFees} from "./IProposerFees.sol";
 import {IProverManager} from "./IProverManager.sol";
 import {IPublicationFeed} from "./IPublicationFeed.sol";
+
 import "@openzeppelin/contracts/utils/math/SafeCast.sol";
 
 abstract contract BaseProverManager is IProposerFees, IProverManager {
     using SafeCast for uint256;
+    using LibPercentage for uint96;
 
     struct Period {
         // SLOT 1
@@ -17,7 +20,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager {
         // SLOT 2
         // the fee that the prover is willing to charge for proving each publication
         uint96 fee;
-        // the percentage (in bps) of the fee that is charged for delayed publications.
+        // the percentage (with two decimals precision) of the fee that is charged for delayed publications.
         uint16 delayedFeePercentage;
         // the timestamp of the end of the period. Default to zero while the period is open.
         uint40 end;
@@ -94,7 +97,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager {
         // Deduct fee from proposer's balance
         uint96 fee = _periods[periodId].fee;
         if (isDelayed) {
-            fee = _calculatePercentage(fee, _periods[periodId].delayedFeePercentage).toUint96();
+            fee = fee.scaleByPercentage(_periods[periodId].delayedFeePercentage);
         }
         _balances[proposer] -= fee;
     }
@@ -147,7 +150,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager {
         (uint40 end,) = _closePeriod(period, _exitDelay(), 0);
 
         // Reward the evictor and slash the prover
-        uint96 evictorIncentive = _calculatePercentage(period.stake, _evictorIncentivePercentage()).toUint96();
+        uint96 evictorIncentive = period.stake.scaleByBPS(_evictorIncentivePercentage());
         _balances[msg.sender] += evictorIncentive;
         period.stake -= evictorIncentive;
 
@@ -209,7 +212,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager {
         uint256 delayedPubFee;
 
         if (numDelayedPublications > 0) {
-            uint256 delayedFee = _calculatePercentage(baseFee, period.delayedFeePercentage);
+            uint96 delayedFee = baseFee.scaleByPercentage(period.delayedFeePercentage);
             delayedPubFee = numDelayedPublications * delayedFee;
         }
 
@@ -228,8 +231,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager {
         require(provenPublication.timestamp > period.end, "Publication must be after period");
 
         uint96 stake = period.stake;
-        _balances[period.prover] +=
-            period.pastDeadline ? _calculatePercentage(stake, _rewardPercentage()).toUint96() : stake;
+        _balances[period.prover] += period.pastDeadline ? stake.scaleByBPS(_rewardPercentage()) : stake;
         period.stake = 0;
     }
 
@@ -246,7 +248,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager {
 
         Period storage period = _periods[currentPeriod];
         fee = period.fee;
-        delayedFee = _calculatePercentage(fee, period.delayedFeePercentage).toUint96();
+        delayedFee = fee.scaleByPercentage(period.delayedFeePercentage);
     }
 
     /// @notice Get the balance of a user
@@ -273,7 +275,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager {
     /// @param fee The fee to be outbid (either the current period's fee or next period's winning fee)
     /// @param offeredFee The new bid
     function _ensureSufficientUnderbid(uint96 fee, uint96 offeredFee) internal view virtual {
-        uint256 requiredMaxFee = _calculatePercentage(fee, _maxBidPercentage());
+        uint96 requiredMaxFee = fee.scaleByBPS(_maxBidPercentage());
         require(offeredFee <= requiredMaxFee, "Offered fee not low enough");
     }
 
@@ -310,10 +312,9 @@ abstract contract BaseProverManager is IProposerFees, IProverManager {
     /// @return _ The reward percentage
     function _rewardPercentage() internal view virtual returns (uint16);
 
-    /// @dev The percentage (in bps) of the fee that is charged for delayed publications
-    /// @dev It is recommended to set this to >10,000 bps since delayed publications should usually be charged at a
-    /// higher rate
-    /// @return _ The multiplier expressed in basis points. This value should usually be greater than 10,000 bps(100%).
+    /// @dev The percentage of the fee that is charged for delayed publications
+    /// @dev It is recommended to set this to >100 since delayed publications should usually be charged at a higher rate
+    /// @return _ The multiplier as a percentage (two decimals). This value should usually be greater than 100 (100%).
     function _delayedFeePercentage() internal view virtual returns (uint16);
 
     /// @dev Increases `user`'s balance by `amount` and emits a `Deposit` event
@@ -339,14 +340,6 @@ abstract contract BaseProverManager is IProposerFees, IProverManager {
         _updatePeriod(nextPeriod, prover, fee, _livenessBond());
     }
 
-    /// @dev Calculates the percentage of a given numerator scaling up to avoid precision loss
-    /// @param amount The number to calculate the percentage of
-    /// @param bps The percentage expressed in basis points(https://muens.io/solidity-percentages)
-    /// @return _ The calculated percentage of the given numerator
-    function _calculatePercentage(uint256 amount, uint256 bps) private pure returns (uint256) {
-        return (amount * bps) / 10_000;
-    }
-
     /// @dev Updates a period with prover information and transfers the liveness bond
     /// @param period The period to update
     /// @param prover The address of the prover

test/BaseProverManager.t.sol

@@ -11,6 +11,7 @@ import {ICheckpointTracker} from "src/protocol/ICheckpointTracker.sol";
 import {IPublicationFeed} from "src/protocol/IPublicationFeed.sol";
 import {PublicationFeed} from "src/protocol/PublicationFeed.sol";
 
+import {LibPercentage} from "src/libs/LibPercentage.sol";
 import {MockCheckpointTracker} from "test/mocks/MockCheckpointTracker.sol";
 import {NullVerifier} from "test/mocks/NullVerifier.sol";
 
@@ -24,7 +25,7 @@ uint96 constant LIVENESS_BOND = 1 ether;
 uint16 constant EVICTOR_INCENTIVE_PERCENTAGE = 500; // 5%
 uint16 constant REWARD_PERCENTAGE = 9000; // 90%
 uint96 constant INITIAL_FEE = 0.1 ether;
-uint16 constant DELAYED_FEE_PERCENTAGE = 15_000; // 150%
+uint16 constant DELAYED_FEE_PERCENTAGE = 150; // 150%
 uint256 constant INITIAL_PERIOD = 1;
 
 abstract contract BaseProverManagerTest is Test {
@@ -218,7 +219,7 @@ abstract contract BaseProverManagerTest is Test {
         // Capture current period stake before eviction
         BaseProverManager.Period memory periodBefore = proverManager.getPeriod(1);
         uint256 stakeBefore = periodBefore.stake;
-        uint256 incentive = _calculatePercentage(stakeBefore, EVICTOR_INCENTIVE_PERCENTAGE);
+        uint256 incentive = LibPercentage.scaleByBPS(stakeBefore, EVICTOR_INCENTIVE_PERCENTAGE);
 
         // Evict the prover
         vm.warp(vm.getBlockTimestamp() + LIVENESS_WINDOW + 1);
@@ -504,7 +505,7 @@ abstract contract BaseProverManagerTest is Test {
 
         uint256 proverBalanceAfter = proverManager.balances(initialProver);
         uint256 expectedBalance = proverBalanceBefore + INITIAL_FEE * numRegularPublications
-            + _calculatePercentage(INITIAL_FEE, DELAYED_FEE_PERCENTAGE) * numDelayedPublications;
+            + LibPercentage.scaleByPercentage(INITIAL_FEE, DELAYED_FEE_PERCENTAGE) * numDelayedPublications;
         assertEq(proverBalanceAfter, expectedBalance, "Prover should receive fees");
     }
 
@@ -848,7 +849,7 @@ abstract contract BaseProverManagerTest is Test {
 
         uint256 initialProverBalanceAfter = proverManager.balances(initialProver);
         uint256 prover1BalanceAfter = proverManager.balances(prover1);
-        uint256 stakeReward = _calculatePercentage(stakeBefore, REWARD_PERCENTAGE);
+        uint256 stakeReward = LibPercentage.scaleByBPS(stakeBefore, REWARD_PERCENTAGE);
         assertEq(prover1BalanceAfter, prover1BalanceBefore + stakeReward, "Prover1 should receive the remaining stake");
         assertEq(initialProverBalanceAfter, initialProverBalanceBefore, "Initial prover should receive nothing");
     }
@@ -905,7 +906,7 @@ abstract contract BaseProverManagerTest is Test {
         assertEq(fee, INITIAL_FEE, "Fee should be the initial fee");
         assertEq(
             delayedFee,
-            _calculatePercentage(INITIAL_FEE, DELAYED_FEE_PERCENTAGE),
+            LibPercentage.scaleByPercentage(INITIAL_FEE, DELAYED_FEE_PERCENTAGE),
             "Delayed fee should be the initial fee"
         );
     }
@@ -926,7 +927,7 @@ abstract contract BaseProverManagerTest is Test {
         assertEq(fee, bidFee, "Fee should be the bid fee");
         assertEq(
             delayedFee,
-            uint96(_calculatePercentage(bidFee, DELAYED_FEE_PERCENTAGE)),
+            uint96(LibPercentage.scaleByPercentage(bidFee, DELAYED_FEE_PERCENTAGE)),
             "Delayed fee should be the bid fee"
         );
     }
@@ -942,8 +943,9 @@ abstract contract BaseProverManagerTest is Test {
         internal
         returns (IPublicationFeed.PublicationHeader[] memory)
     {
-        uint256 depositAmount =
-            delayed ? _calculatePercentage(fee, DELAYED_FEE_PERCENTAGE) * numPublications : fee * numPublications;
+        uint256 depositAmount = delayed
+            ? LibPercentage.scaleByPercentage(fee, DELAYED_FEE_PERCENTAGE) * numPublications
+            : fee * numPublications;
         _deposit(proposer, depositAmount);
 
         IPublicationFeed.PublicationHeader[] memory headers = new IPublicationFeed.PublicationHeader[](numPublications);
@@ -974,11 +976,7 @@ abstract contract BaseProverManagerTest is Test {
     function _deposit(address user, uint256 amount) internal virtual;
 
     function _maxAllowedFee(uint96 fee) internal pure returns (uint96) {
-        return uint96(_calculatePercentage(fee, MAX_BID_PERCENTAGE));
-    }
-
-    function _calculatePercentage(uint256 amount, uint16 percentage) internal pure returns (uint256) {
-        return amount * percentage / 10_000;
+        return uint96(LibPercentage.scaleByBPS(fee, MAX_BID_PERCENTAGE));
     }
 
     function _exit(address prover) internal {