diff --git a/docs/PRICE_ORACLE.md b/docs/PRICE_ORACLE.md index 8fa8a6ca..24aa017a 100644 --- a/docs/PRICE_ORACLE.md +++ b/docs/PRICE_ORACLE.md @@ -69,10 +69,8 @@ Manually sets the price (for initialization or emergency). **Logic**: ```solidity -currentPriceUpScaled = _price << 10 // upscale by 2^10 -if (currentPriceUpScaled < minimumPriceUpscaled) { - currentPriceUpScaled = minimumPriceUpscaled -} +_currentPriceUpScaled = uint64(_price) << 10 // upscale by 2^10 +currentPriceUpScaled = _clampPriceUpscaled(_currentPriceUpScaled) // Update PostageStamp PostageStamp.setPrice(currentPrice()) emit PriceUpdate(currentPrice()) @@ -94,7 +92,7 @@ Automatically adjusts price based on redundancy (called by Redistribution). 2. Cap redundancy at `targetRedundancy + maxConsideredExtraRedundancy` 3. Apply change rate based on redundancy - target 4. Apply maximum penalty for skipped rounds -5. Enforce minimum price +5. Clamp price to `[MINIMUM_PRICE_UPSCALED, MAX_CURRENT_PRICE_UPSCALED]` 6. Update PostageStamp 7. Emit event @@ -124,7 +122,10 @@ Pauses or unpauses price adjustments. Returns the current price (downscaled by 2^10). #### minimumPrice() -Returns the minimum price floor. +Returns the minimum price floor (compile-time constant derived from `MINIMUM_PRICE_UPSCALED`). + +#### MINIMUM_PRICE_UPSCALED() / MAX_CURRENT_PRICE_UPSCALED() +Public constant getters for the upscaled price bounds used by `_clampPriceUpscaled()`. #### currentRound() Returns the current round number: `block.number / 152` @@ -134,7 +135,8 @@ Returns the current round number: `block.number / 152` ```solidity uint16 targetRedundancy = 4; // Target chunks per node uint16 maxConsideredExtraRedundancy = 4; // Cap on extra redundancy -uint32 minimumPriceUpscaled = 24000 << 10; // ~23.44 (downscaled) +uint64 constant MINIMUM_PRICE_UPSCALED = 24000 << 10; // ~24000 (downscaled) +uint64 constant MAX_CURRENT_PRICE_UPSCALED = uint64(type(uint32).max) << 10; uint32 priceBase = 1048576; // Base for change rate (2^20) ``` @@ -186,16 +188,24 @@ newPrice = (1049417 * 1000000) / 1048576 = 1000800 // Increase of ~0.08% ``` -### Minimum Price Enforcement +### Price Bounds Enforcement + +Prices are clamped via `_clampPriceUpscaled()` after every manual or automatic update: -Prices are bounded from below: ```solidity -if (currentPriceUpScaled < minimumPriceUpscaled) { - currentPriceUpScaled = minimumPriceUpscaled +function _clampPriceUpscaled(uint64 priceUpScaled) private pure returns (uint64) { + if (priceUpScaled < MINIMUM_PRICE_UPSCALED) { + priceUpScaled = MINIMUM_PRICE_UPSCALED; + } + if (priceUpScaled > MAX_CURRENT_PRICE_UPSCALED) { + priceUpScaled = MAX_CURRENT_PRICE_UPSCALED; + } + return priceUpScaled; } ``` -This prevents prices from becoming too low and disincentivizing storage. +- **Minimum floor** prevents prices from becoming too low and disincentivizing storage. +- **Maximum ceiling** ensures `(currentPriceUpScaled >> 10)` fits in `uint32`, so `currentPrice()` cannot silently truncate and under-report relative to the stored upscaled value. ## Integration with Other Contracts @@ -264,10 +274,11 @@ error UnexpectedZero(); // Redundancy must be > 0 ## Security Considerations 1. Minimum price floor prevents race-to-bottom pricing -2. Maximum extra redundancy cap prevents excessive price increases -3. One adjustment per round prevents manipulation -4. Pausable for emergency stops -5. Failed PostageStamp updates don't prevent oracle updates +2. Maximum upscaled price cap keeps `currentPrice()` consistent with stored state +3. Maximum extra redundancy cap prevents excessive price increases +4. One adjustment per round prevents manipulation +5. Pausable for emergency stops +6. Failed PostageStamp updates don't prevent oracle updates ## Pause Mechanism @@ -300,9 +311,8 @@ function adjustPrice(redundancy): for each skipped round: newPrice = (changeRate[0] * newPrice) / priceBase - // Enforce minimum - if (newPrice < minimumPriceUpscaled): - newPrice = minimumPriceUpscaled + // Clamp to upscaled bounds + newPrice = _clampPriceUpscaled(newPrice) currentPriceUpScaled = newPrice lastAdjustedRound = currentRoundNum diff --git a/src/PriceOracle.sol b/src/PriceOracle.sol index 5675f4f9..5b741edf 100644 --- a/src/PriceOracle.sol +++ b/src/PriceOracle.sol @@ -24,14 +24,9 @@ contract PriceOracle is AccessControl { // The number of the last round price adjusting happend uint64 public lastAdjustedRound; - // The minimum price allowed - uint32 public minimumPriceUpscaled = 24000 << 10; // we upscale it by 2^10 - // The priceBase to modulate the price uint32 public priceBase = 1048576; - uint64 public currentPriceUpScaled = minimumPriceUpscaled; - // Constants used to modulate the price, see below usage uint32[9] public changeRate = [1049417, 1049206, 1048996, 1048786, 1048576, 1048366, 1048156, 1047946, 1047736]; @@ -41,6 +36,16 @@ contract PriceOracle is AccessControl { // The length of a round in blocks. uint8 private constant ROUND_LENGTH = 152; + /// @dev Minimum upscaled price (24000 PLUR × 2^10). + uint64 public constant MINIMUM_PRICE_UPSCALED = 24000 << 10; + + /// @dev Upper bound for upscaled price so `(currentPriceUpScaled >> 10)` fits in `uint32`. + /// Without this, `currentPrice()`'s `uint32(... >> 10)` truncates and can disagree with + /// `currentPriceUpScaled` and under-report vs `minimumPrice()`. + uint64 public constant MAX_CURRENT_PRICE_UPSCALED = uint64(type(uint32).max) << 10; + + uint64 public currentPriceUpScaled = MINIMUM_PRICE_UPSCALED; + // ----------------------------- Events ------------------------------ /** @@ -78,13 +83,9 @@ contract PriceOracle is AccessControl { revert CallerNotAdmin(); } - uint64 _currentPriceUpScaled = _price << 10; - uint64 _minimumPriceUpscaled = minimumPriceUpscaled; - - // Enforce minimum price - if (_currentPriceUpScaled < _minimumPriceUpscaled) { - _currentPriceUpScaled = _minimumPriceUpscaled; - } + // Cast before shifting to avoid uint32 overflow/truncation. + uint64 _currentPriceUpScaled = uint64(_price) << 10; + _currentPriceUpScaled = _clampPriceUpscaled(_currentPriceUpScaled); currentPriceUpScaled = _currentPriceUpScaled; // Check if the setting of price in postagestamp succeded @@ -124,7 +125,6 @@ contract PriceOracle is AccessControl { } uint64 _currentPriceUpScaled = currentPriceUpScaled; - uint64 _minimumPriceUpscaled = minimumPriceUpscaled; uint32 _priceBase = priceBase; // Set the number of rounds that were skipped, we substract 1 as lastAdjustedRound is set below and default result is 1 @@ -142,10 +142,7 @@ contract PriceOracle is AccessControl { } } - // Enforce minimum price - if (_currentPriceUpScaled < _minimumPriceUpscaled) { - _currentPriceUpScaled = _minimumPriceUpscaled; - } + _currentPriceUpScaled = _clampPriceUpscaled(_currentPriceUpScaled); currentPriceUpScaled = _currentPriceUpScaled; lastAdjustedRound = currentRoundNumber; @@ -182,6 +179,17 @@ contract PriceOracle is AccessControl { // STATE READING // //////////////////////////////////////// + /// @notice Clamp upscaled price to [MINIMUM_PRICE_UPSCALED, MAX_CURRENT_PRICE_UPSCALED]. + function _clampPriceUpscaled(uint64 priceUpScaled) private pure returns (uint64) { + if (priceUpScaled < MINIMUM_PRICE_UPSCALED) { + priceUpScaled = MINIMUM_PRICE_UPSCALED; + } + if (priceUpScaled > MAX_CURRENT_PRICE_UPSCALED) { + priceUpScaled = MAX_CURRENT_PRICE_UPSCALED; + } + return priceUpScaled; + } + /** * @notice Return the number of the current round. */ @@ -203,8 +211,8 @@ contract PriceOracle is AccessControl { /** * @notice Return the price downscaled */ - function minimumPrice() public view returns (uint32) { + function minimumPrice() public pure returns (uint32) { // We downcasted to uint32 and bitshift it by 2^10 - return uint32((minimumPriceUpscaled) >> 10); + return uint32(MINIMUM_PRICE_UPSCALED >> 10); } } diff --git a/src/echidna/EchidnaPriceOracleHarness.sol b/src/echidna/EchidnaPriceOracleHarness.sol index ff28623b..4675509a 100644 --- a/src/echidna/EchidnaPriceOracleHarness.sol +++ b/src/echidna/EchidnaPriceOracleHarness.sol @@ -220,7 +220,7 @@ contract EchidnaPriceOracleHarness { } function echidna_price_never_below_minimum() external view returns (bool) { - return oracle.currentPriceUpScaled() >= oracle.minimumPriceUpscaled(); + return oracle.currentPriceUpScaled() >= oracle.MINIMUM_PRICE_UPSCALED(); } function echidna_lastAdjustedRound_not_in_future() external view returns (bool) { @@ -298,7 +298,7 @@ contract EchidnaPriceOracleHarness { } } - uint256 minUp = uint256(oracle.minimumPriceUpscaled()); + uint256 minUp = uint256(oracle.MINIMUM_PRICE_UPSCALED()); if (price < minUp) price = minUp; if (price > type(uint64).max) return (false, 0); return (true, uint64(price));