From 23f009afd21e2f0fcfa8dd969dcca22cabf1b995 Mon Sep 17 00:00:00 2001 From: Nikesh Nazareth Date: Wed, 30 Apr 2025 12:39:08 +0200 Subject: [PATCH 1/3] Move percentage calculation to a separate library This is cleaner in code and it also makes it possible to provide a configurable percentage --- src/libs/LibPercentage.sol | 29 +++++++++++++++++++++++++++++ src/protocol/BaseProverManager.sol | 24 ++++++++---------------- 2 files changed, 37 insertions(+), 16 deletions(-) create mode 100644 src/libs/LibPercentage.sol diff --git a/src/libs/LibPercentage.sol b/src/libs/LibPercentage.sol new file mode 100644 index 00000000..47f53c0f --- /dev/null +++ b/src/libs/LibPercentage.sol @@ -0,0 +1,29 @@ +// 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 = 10000; + uint256 constant TWO_DECIMALS = 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 scaleBy(uint256 value, uint16 percentage) internal pure returns (uint96) { + return scaleBy(value, percentage, BASIS_POINTS); + } +} diff --git a/src/protocol/BaseProverManager.sol b/src/protocol/BaseProverManager.sol index 547ef373..72dbd486 100644 --- a/src/protocol/BaseProverManager.sol +++ b/src/protocol/BaseProverManager.sol @@ -1,14 +1,15 @@ // 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 uint256; struct Period { // SLOT 1 @@ -94,7 +95,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.scaleBy(_periods[periodId].delayedFeePercentage); } _balances[proposer] -= fee; } @@ -147,7 +148,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.scaleBy(_evictorIncentivePercentage()); _balances[msg.sender] += evictorIncentive; period.stake -= evictorIncentive; @@ -209,7 +210,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager { uint256 delayedPubFee; if (numDelayedPublications > 0) { - uint256 delayedFee = _calculatePercentage(baseFee, period.delayedFeePercentage); + uint96 delayedFee = baseFee.scaleBy(period.delayedFeePercentage); delayedPubFee = numDelayedPublications * delayedFee; } @@ -228,8 +229,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.scaleBy(_rewardPercentage()) : stake; period.stake = 0; } @@ -246,7 +246,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager { Period storage period = _periods[currentPeriod]; fee = period.fee; - delayedFee = _calculatePercentage(fee, period.delayedFeePercentage).toUint96(); + delayedFee = fee.scaleBy(period.delayedFeePercentage); } /// @notice Get the balance of a user @@ -273,7 +273,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.scaleBy(_maxBidPercentage()); require(offeredFee <= requiredMaxFee, "Offered fee not low enough"); } @@ -339,14 +339,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 From b8b45e4453140934ae796a8c71d89a7b37168ba0 Mon Sep 17 00:00:00 2001 From: Nikesh Nazareth Date: Wed, 30 Apr 2025 12:43:07 +0200 Subject: [PATCH 2/3] Use two decimals of precision for the delayed fee percentage This rebalances the bits to be more useful for anticipated values. A uint16 proves 65536 possible possibilities, but using basis points means that the values range from 0-6 with 4 decimals. As a percentage, this can represent up to 655 with two decimals. --- src/libs/LibPercentage.sol | 2 +- src/protocol/BaseProverManager.sol | 20 +++++++++++--------- test/BaseProverManager.t.sol | 15 ++++++++++----- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/libs/LibPercentage.sol b/src/libs/LibPercentage.sol index 47f53c0f..3accf2c5 100644 --- a/src/libs/LibPercentage.sol +++ b/src/libs/LibPercentage.sol @@ -8,7 +8,7 @@ library LibPercentage { // COMMON PRECISION AMOUNTS (https://muens.io/solidity-percentages) uint256 constant BASIS_POINTS = 10000; - uint256 constant TWO_DECIMALS = 100; + 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 diff --git a/src/protocol/BaseProverManager.sol b/src/protocol/BaseProverManager.sol index 72dbd486..679ea530 100644 --- a/src/protocol/BaseProverManager.sol +++ b/src/protocol/BaseProverManager.sol @@ -7,9 +7,12 @@ 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 uint256; + using LibPercentage for uint96; struct Period { // SLOT 1 @@ -18,7 +21,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; @@ -95,7 +98,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager { // Deduct fee from proposer's balance uint96 fee = _periods[periodId].fee; if (isDelayed) { - fee = fee.scaleBy(_periods[periodId].delayedFeePercentage); + fee = fee.scaleBy(_periods[periodId].delayedFeePercentage, LibPercentage.PERCENT); } _balances[proposer] -= fee; } @@ -210,7 +213,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager { uint256 delayedPubFee; if (numDelayedPublications > 0) { - uint96 delayedFee = baseFee.scaleBy(period.delayedFeePercentage); + uint96 delayedFee = baseFee.scaleBy(period.delayedFeePercentage, LibPercentage.PERCENT); delayedPubFee = numDelayedPublications * delayedFee; } @@ -246,7 +249,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager { Period storage period = _periods[currentPeriod]; fee = period.fee; - delayedFee = fee.scaleBy(period.delayedFeePercentage); + delayedFee = fee.scaleBy(period.delayedFeePercentage, LibPercentage.PERCENT); } /// @notice Get the balance of a user @@ -310,10 +313,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 diff --git a/test/BaseProverManager.t.sol b/test/BaseProverManager.t.sol index 23bc553a..f2ce235c 100644 --- a/test/BaseProverManager.t.sol +++ b/test/BaseProverManager.t.sol @@ -24,7 +24,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 +218,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 = _calculatePercentageBPS(stakeBefore, EVICTOR_INCENTIVE_PERCENTAGE); // Evict the prover vm.warp(vm.getBlockTimestamp() + LIVENESS_WINDOW + 1); @@ -848,7 +848,7 @@ abstract contract BaseProverManagerTest is Test { uint256 initialProverBalanceAfter = proverManager.balances(initialProver); uint256 prover1BalanceAfter = proverManager.balances(prover1); - uint256 stakeReward = _calculatePercentage(stakeBefore, REWARD_PERCENTAGE); + uint256 stakeReward = _calculatePercentageBPS(stakeBefore, REWARD_PERCENTAGE); assertEq(prover1BalanceAfter, prover1BalanceBefore + stakeReward, "Prover1 should receive the remaining stake"); assertEq(initialProverBalanceAfter, initialProverBalanceBefore, "Initial prover should receive nothing"); } @@ -974,13 +974,18 @@ 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)); + return uint96(_calculatePercentageBPS(fee, MAX_BID_PERCENTAGE)); } - function _calculatePercentage(uint256 amount, uint16 percentage) internal pure returns (uint256) { + function _calculatePercentageBPS(uint256 amount, uint16 percentage) internal pure returns (uint256) { return amount * percentage / 10_000; } + function _calculatePercentage(uint256 amount, uint16 percentage) internal pure returns (uint256) { + return amount * percentage / 100; + } + + function _exit(address prover) internal { vm.prank(prover); proverManager.exit(); From f1b63015f77ff906747fe432388dd5a6293e038f Mon Sep 17 00:00:00 2001 From: nikeshnazareth Date: Wed, 7 May 2025 12:35:44 +1000 Subject: [PATCH 3/3] Replace 10000 with 10_000 for clarity Co-authored-by: Leo --- src/libs/LibPercentage.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/LibPercentage.sol b/src/libs/LibPercentage.sol index 3accf2c5..0105b21d 100644 --- a/src/libs/LibPercentage.sol +++ b/src/libs/LibPercentage.sol @@ -7,7 +7,7 @@ library LibPercentage { using SafeCast for uint256; // COMMON PRECISION AMOUNTS (https://muens.io/solidity-percentages) - uint256 constant BASIS_POINTS = 10000; + 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