Skip to content

Use two decimals for delayed scale factor #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: price-delayed-publications
Choose a base branch
from
Open

Use two decimals for delayed scale factor #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23f009a
Move percentage calculation to a separate library
nikeshnazareth Apr 30, 2025
b8b45e4
Use two decimals of precision for the delayed fee percentage
nikeshnazareth Apr 30, 2025
f1b6301
Replace 10000 with 10_000 for clarity
nikeshnazareth May 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/libs/LibPercentage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 = 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 scaleBy(uint256 value, uint16 percentage) internal pure returns (uint96) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hesitant to name the two functions the same. it might be easy to get confused by anyone using the library.
What do you think about this approach?

  • scaleBy remains the common function that takes a precision
  • scaleByBps is the bps implementation
  • scaleByPercentage is the percentage implementation

It is more verbose, but I believe it is less error prone

return scaleBy(value, percentage, BASIS_POINTS);
}
}
34 changes: 14 additions & 20 deletions src/protocol/BaseProverManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
// 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
Expand All @@ -17,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;
Expand Down Expand Up @@ -94,7 +98,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, LibPercentage.PERCENT);
}
_balances[proposer] -= fee;
}
Expand Down Expand Up @@ -147,7 +151,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;

Expand Down Expand Up @@ -209,7 +213,7 @@ abstract contract BaseProverManager is IProposerFees, IProverManager {
uint256 delayedPubFee;

if (numDelayedPublications > 0) {
uint256 delayedFee = _calculatePercentage(baseFee, period.delayedFeePercentage);
uint96 delayedFee = baseFee.scaleBy(period.delayedFeePercentage, LibPercentage.PERCENT);
delayedPubFee = numDelayedPublications * delayedFee;
}

Expand All @@ -228,8 +232,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;
}

Expand All @@ -246,7 +249,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, LibPercentage.PERCENT);
}

/// @notice Get the balance of a user
Expand All @@ -273,7 +276,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");
}

Expand Down Expand Up @@ -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
Expand All @@ -339,14 +341,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
Expand Down
15 changes: 10 additions & 5 deletions test/BaseProverManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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;
}


Comment on lines +980 to +988
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the tests just use the library? I don't think we are trying to check if the library works properly on these tests(we can create a new unit test for LibPercentage if we wanted to do this)

function _exit(address prover) internal {
vm.prank(prover);
proverManager.exit();
Expand Down
Loading