From 73c0c9b2d942567a1a4093f368af8d6e34204a16 Mon Sep 17 00:00:00 2001 From: Trevor Richard Date: Fri, 5 Apr 2024 14:45:23 +0000 Subject: [PATCH 1/5] update lib --- lib/forge-std | 2 +- lib/openzeppelin-contracts | 2 +- lib/prb-math | 2 +- lib/pt-v5-twab-controller | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/forge-std b/lib/forge-std index f73c73d..bb4ceea 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit f73c73d2018eb6a111f35e4dae7b4f27401e9421 +Subproject commit bb4ceea94d6f10eeb5b41dc2391c6c8bf8e734ef diff --git a/lib/openzeppelin-contracts b/lib/openzeppelin-contracts index 9329cfa..dc44c9f 160000 --- a/lib/openzeppelin-contracts +++ b/lib/openzeppelin-contracts @@ -1 +1 @@ -Subproject commit 9329cfacd4c7d20bcb43d772d947ff9e39b65df9 +Subproject commit dc44c9f1a4c3b10af99492eed84f83ed244203f6 diff --git a/lib/prb-math b/lib/prb-math index 77fa88e..9dc0651 160000 --- a/lib/prb-math +++ b/lib/prb-math @@ -1 +1 @@ -Subproject commit 77fa88eda4a4a91b3f3e9431df291292c26b6c71 +Subproject commit 9dc06519f3b9f1659fec7d396da634fe690f660c diff --git a/lib/pt-v5-twab-controller b/lib/pt-v5-twab-controller index 258ab89..81f9274 160000 --- a/lib/pt-v5-twab-controller +++ b/lib/pt-v5-twab-controller @@ -1 +1 @@ -Subproject commit 258ab89ee5da09a494d5721c18d27133ad844b63 +Subproject commit 81f9274850294c5c47c879c27e299dbc9cf5fdf9 From a84d5eba1a51ba173ba79fef31d31fc90cee4bf6 Mon Sep 17 00:00:00 2001 From: Trevor Richard Date: Fri, 5 Apr 2024 14:47:57 +0000 Subject: [PATCH 2/5] update twab controller lib --- .gitmodules | 1 - lib/pt-v5-twab-controller | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitmodules b/.gitmodules index a7a6acf..3100659 100644 --- a/.gitmodules +++ b/.gitmodules @@ -21,4 +21,3 @@ [submodule "lib/pt-v5-twab-controller"] path = lib/pt-v5-twab-controller url = https://github.com/generationsoftware/pt-v5-twab-controller - branch = prod-deploy-2 diff --git a/lib/pt-v5-twab-controller b/lib/pt-v5-twab-controller index 81f9274..817ca2d 160000 --- a/lib/pt-v5-twab-controller +++ b/lib/pt-v5-twab-controller @@ -1 +1 @@ -Subproject commit 81f9274850294c5c47c879c27e299dbc9cf5fdf9 +Subproject commit 817ca2df9fd85cf6d6bbddbd58e8377febd9b1d7 From 03d4db8e3b9e93addf8870bf2b18f2230d301235 Mon Sep 17 00:00:00 2001 From: Trevor Richard Date: Fri, 5 Apr 2024 20:29:15 +0000 Subject: [PATCH 3/5] use integer math for prize tokens per share --- src/PrizePool.sol | 5 +- src/abstract/TieredLiquidityDistributor.sol | 134 ++++++++---------- src/libraries/UD34x4.sol | 70 --------- test/PrizePool.t.sol | 20 +-- .../abstract/TieredLiquidityDistributor.t.sol | 7 +- .../TieredLiquidityDistributorWrapper.sol | 9 +- .../TieredLiquidityDistributorFuzzHarness.sol | 21 ++- test/libraries/UD34x4.t.sol | 46 ------ 8 files changed, 85 insertions(+), 227 deletions(-) delete mode 100644 src/libraries/UD34x4.sol delete mode 100644 test/libraries/UD34x4.t.sol diff --git a/src/PrizePool.sol b/src/PrizePool.sol index fc296e3..3c0608c 100644 --- a/src/PrizePool.sol +++ b/src/PrizePool.sol @@ -8,7 +8,6 @@ import { SD59x18, convert, sd } from "prb-math/SD59x18.sol"; import { SD1x18, unwrap, UNIT } from "prb-math/SD1x18.sol"; import { TwabController } from "pt-v5-twab-controller/TwabController.sol"; -import { UD34x4, intoUD60x18 as fromUD34x4toUD60x18 } from "./libraries/UD34x4.sol"; import { DrawAccumulatorLib, Observation } from "./libraries/DrawAccumulatorLib.sol"; import { TieredLiquidityDistributor, Tier } from "./abstract/TieredLiquidityDistributor.sol"; import { TierCalculationLib } from "./libraries/TierCalculationLib.sol"; @@ -194,7 +193,7 @@ contract PrizePool is TieredLiquidityDistributor { uint8 lastNumTiers, uint8 numTiers, uint104 reserve, - UD34x4 prizeTokensPerShare, + uint128 prizeTokensPerShare, uint48 drawOpenedAt ); @@ -668,7 +667,7 @@ contract PrizePool is TieredLiquidityDistributor { (uint104 newReserve, ) = _computeNewDistributions( _numTiers, lastAwardedDrawId_ == 0 ? _numTiers : computeNextNumberOfTiers(claimCount), - fromUD34x4toUD60x18(prizeTokenPerShare), + prizeTokenPerShare, getTotalContributedBetween(lastAwardedDrawId_ + 1, getDrawIdToAward()) ); diff --git a/src/abstract/TieredLiquidityDistributor.sol b/src/abstract/TieredLiquidityDistributor.sol index d730cea..9fa1551 100644 --- a/src/abstract/TieredLiquidityDistributor.sol +++ b/src/abstract/TieredLiquidityDistributor.sol @@ -6,7 +6,6 @@ import { SafeCast } from "openzeppelin/utils/math/SafeCast.sol"; import { SD59x18, sd } from "prb-math/SD59x18.sol"; import { UD60x18, ud, convert } from "prb-math/UD60x18.sol"; -import { UD34x4, fromUD60x18 as fromUD60x18toUD34x4, intoUD60x18 as fromUD34x4toUD60x18 } from "../libraries/UD34x4.sol"; import { TierCalculationLib } from "../libraries/TierCalculationLib.sol"; /// @notice Struct that tracks tier liquidity information. @@ -16,7 +15,7 @@ import { TierCalculationLib } from "../libraries/TierCalculationLib.sol"; struct Tier { uint24 drawId; uint104 prizeSize; - UD34x4 prizeTokenPerShare; + uint128 prizeTokenPerShare; } /// @notice Emitted when the number of tiers is less than the minimum number of tiers. @@ -117,7 +116,7 @@ contract TieredLiquidityDistributor { /// @dev This is an ever-increasing exchange rate that is used to calculate the prize liquidity for each tier. /// @dev Each tier holds a separate tierPrizeTokenPerShare; the delta between the tierPrizeTokenPerShare and /// the prizeTokenPerShare * tierShares is the available liquidity they have. - UD34x4 public prizeTokenPerShare; + uint128 public prizeTokenPerShare; /// @notice The number of tiers for the last awarded draw. The last tier is the canary tier. uint8 public numberOfTiers; @@ -222,12 +221,11 @@ contract TieredLiquidityDistributor { } uint8 numTiers = numberOfTiers; - UD34x4 _prizeTokenPerShare = prizeTokenPerShare; - UD60x18 _prizeTokenPerShareUD60x18 = fromUD34x4toUD60x18(_prizeTokenPerShare); - (uint96 deltaReserve, UD60x18 newPrizeTokenPerShare) = _computeNewDistributions( + uint128 _prizeTokenPerShare = prizeTokenPerShare; + (uint96 deltaReserve, uint128 newPrizeTokenPerShare) = _computeNewDistributions( numTiers, _nextNumberOfTiers, - _prizeTokenPerShareUD60x18, + _prizeTokenPerShare, _prizeTokenLiquidity ); @@ -240,13 +238,13 @@ contract TieredLiquidityDistributor { prizeSize: _computePrizeSize( i, _nextNumberOfTiers, - _prizeTokenPerShareUD60x18, + _prizeTokenPerShare, newPrizeTokenPerShare ) }); } - prizeTokenPerShare = fromUD60x18toUD34x4(newPrizeTokenPerShare); + prizeTokenPerShare = newPrizeTokenPerShare; numberOfTiers = _nextNumberOfTiers; _lastAwardedDrawId = _awardingDraw; lastAwardedDrawAwardedAt = uint48(block.timestamp); @@ -263,30 +261,30 @@ contract TieredLiquidityDistributor { function _computeNewDistributions( uint8 _numberOfTiers, uint8 _nextNumberOfTiers, - UD60x18 _currentPrizeTokenPerShare, + uint128 _currentPrizeTokenPerShare, uint256 _prizeTokenLiquidity - ) internal view returns (uint96 deltaReserve, UD60x18 newPrizeTokenPerShare) { - UD60x18 reclaimedLiquidity; + ) internal view returns (uint96 deltaReserve, uint128 newPrizeTokenPerShare) { + uint256 reclaimedLiquidity; { // need to redistribute to the canary tier and any new tiers (if expanding) uint8 start = _computeReclamationStart(_numberOfTiers, _nextNumberOfTiers); uint8 end = _numberOfTiers; for (uint8 i = start; i < end; i++) { - reclaimedLiquidity = reclaimedLiquidity.add( + reclaimedLiquidity = reclaimedLiquidity + ( _getTierRemainingLiquidity( - fromUD34x4toUD60x18(_tiers[i].prizeTokenPerShare), + _tiers[i].prizeTokenPerShare, _currentPrizeTokenPerShare, - i + _numShares(i, _numberOfTiers) ) ); } } - uint256 totalNewLiquidity = _prizeTokenLiquidity + convert(reclaimedLiquidity); + uint256 totalNewLiquidity = _prizeTokenLiquidity + reclaimedLiquidity; uint256 nextTotalShares = computeTotalShares(_nextNumberOfTiers); uint256 deltaPrizeTokensPerShare = totalNewLiquidity / nextTotalShares; - newPrizeTokenPerShare = _currentPrizeTokenPerShare.add(convert(deltaPrizeTokensPerShare)); + newPrizeTokenPerShare = SafeCast.toUint128(_currentPrizeTokenPerShare + deltaPrizeTokensPerShare); deltaReserve = SafeCast.toUint96( // reserve portion of new liquidity @@ -328,8 +326,8 @@ contract TieredLiquidityDistributor { tier.prizeSize = _computePrizeSize( _tier, _numberOfTiers, - fromUD34x4toUD60x18(tier.prizeTokenPerShare), - fromUD34x4toUD60x18(prizeTokenPerShare) + tier.prizeTokenPerShare, + prizeTokenPerShare ); } return tier; @@ -364,13 +362,12 @@ contract TieredLiquidityDistributor { /// @param _tier The tier number /// @param _liquidity The amount of liquidity to consume function _consumeLiquidity(Tier memory _tierStruct, uint8 _tier, uint104 _liquidity) internal { + uint8 _tierShares = _numShares(_tier, numberOfTiers); uint104 remainingLiquidity = SafeCast.toUint104( - convert( - _getTierRemainingLiquidity( - fromUD34x4toUD60x18(_tierStruct.prizeTokenPerShare), - fromUD34x4toUD60x18(prizeTokenPerShare), - _tier - ) + _getTierRemainingLiquidity( + _tierStruct.prizeTokenPerShare, + prizeTokenPerShare, + _tierShares ) ); @@ -388,10 +385,16 @@ contract TieredLiquidityDistributor { emit ReserveConsumed(excess); _tierStruct.prizeTokenPerShare = prizeTokenPerShare; } else { - _tierStruct.prizeTokenPerShare = UD34x4.wrap( - UD34x4.unwrap(_tierStruct.prizeTokenPerShare) + - UD34x4.unwrap(fromUD60x18toUD34x4(convert(_liquidity).div(convert(_numShares(_tier, numberOfTiers))))) - ); + uint8 _remainder = uint8(_liquidity % _tierShares); + uint8 _overConsumption = _remainder == 0 ? 0 : _tierShares - _remainder; + if (_overConsumption > 0) { + // We must round up our tier prize token per share value to ensure we don't over-award the tier's + // liquidity, but any over-consumption can be contributed to the reserve so every wei is accounted + // for. + _reserve += _overConsumption; + } + + _tierStruct.prizeTokenPerShare += SafeCast.toUint104(uint256(_liquidity) + _overConsumption) / _tierShares; } _tiers[_tier] = _tierStruct; @@ -406,18 +409,19 @@ contract TieredLiquidityDistributor { function _computePrizeSize( uint8 _tier, uint8 _numberOfTiers, - UD60x18 _tierPrizeTokenPerShare, - UD60x18 _prizeTokenPerShare + uint128 _tierPrizeTokenPerShare, + uint128 _prizeTokenPerShare ) internal view returns (uint104) { - uint256 prizeSize; - if (_prizeTokenPerShare.gt(_tierPrizeTokenPerShare)) { - prizeSize = _computePrizeSize( - _tierPrizeTokenPerShare, - _prizeTokenPerShare, - convert(TierCalculationLib.prizeCount(_tier)), - _numShares(_tier, _numberOfTiers) - ); - } + uint256 prizeCount = TierCalculationLib.prizeCount(_tier); + uint256 remainingTierLiquidity = _getTierRemainingLiquidity( + _tierPrizeTokenPerShare, + _prizeTokenPerShare, + _numShares(_tier, _numberOfTiers) + ); + + uint256 prizeSize = convert( + convert(remainingTierLiquidity).mul(tierLiquidityUtilizationRate).div(convert(prizeCount)) + ); return prizeSize > type(uint104).max ? type(uint104).max : uint104(prizeSize); } @@ -438,41 +442,17 @@ contract TieredLiquidityDistributor { return result; } - /// @notice Computes the prize size with the given parameters. - /// @param _tierPrizeTokenPerShare The prizeTokenPerShare of the Tier struct - /// @param _prizeTokenPerShare The global prizeTokenPerShare - /// @param _fractionalPrizeCount The prize count as UD60x18 - /// @param _shares The number of shares that the tier has - /// @return The prize size - function _computePrizeSize( - UD60x18 _tierPrizeTokenPerShare, - UD60x18 _prizeTokenPerShare, - UD60x18 _fractionalPrizeCount, - uint8 _shares - ) internal view returns (uint256) { - return - convert( - _prizeTokenPerShare.sub(_tierPrizeTokenPerShare).mul(convert(_shares)).mul(tierLiquidityUtilizationRate).div( - _fractionalPrizeCount - ) - ); - } - /// @notice Computes the remaining liquidity available to a tier. /// @param _tier The tier to compute the liquidity for /// @return The remaining liquidity function getTierRemainingLiquidity(uint8 _tier) public view returns (uint256) { uint8 _numTiers = numberOfTiers; if (TierCalculationLib.isValidTier(_tier, _numTiers)) { - UD60x18 remaining = _getTierRemainingLiquidity( - fromUD34x4toUD60x18(_getTier(_tier, _numTiers).prizeTokenPerShare), - fromUD34x4toUD60x18(prizeTokenPerShare), - _tier - ); - uint result = convert( - remaining + return _getTierRemainingLiquidity( + _getTier(_tier, _numTiers).prizeTokenPerShare, + prizeTokenPerShare, + _numShares(_tier, _numTiers) ); - return result; } else { return 0; } @@ -481,17 +461,17 @@ contract TieredLiquidityDistributor { /// @notice Computes the remaining tier liquidity. /// @param _tierPrizeTokenPerShare The prizeTokenPerShare of the Tier struct /// @param _prizeTokenPerShare The global prizeTokenPerShare + /// @param _tierShares The number of shares for the tier /// @return The remaining available liquidity function _getTierRemainingLiquidity( - UD60x18 _tierPrizeTokenPerShare, - UD60x18 _prizeTokenPerShare, - uint8 _tier - ) internal view returns (UD60x18) { - uint8 numShares = _numShares(_tier, numberOfTiers); - UD60x18 result = - _tierPrizeTokenPerShare.gte(_prizeTokenPerShare) - ? ud(0) - : _prizeTokenPerShare.sub(_tierPrizeTokenPerShare).mul(convert(numShares)); + uint128 _tierPrizeTokenPerShare, + uint128 _prizeTokenPerShare, + uint8 _tierShares + ) internal pure returns (uint256) { + uint256 result = + _tierPrizeTokenPerShare >= _prizeTokenPerShare + ? 0 + : uint256(_prizeTokenPerShare - _tierPrizeTokenPerShare) * _tierShares; return result; } diff --git a/src/libraries/UD34x4.sol b/src/libraries/UD34x4.sol deleted file mode 100644 index bf3a2a7..0000000 --- a/src/libraries/UD34x4.sol +++ /dev/null @@ -1,70 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.19; - -import { UD60x18 } from "prb-math/UD60x18.sol"; - -type UD34x4 is uint128; - -/// @notice Emitted when converting a basic integer to the fixed-point format overflows UD34x4. -error PRBMath_UD34x4_Convert_Overflow(uint128 x); -error PRBMath_UD34x4_fromUD60x18_Convert_Overflow(uint256 x); - -/// @dev The maximum value an UD34x4 number can have. -uint128 constant uMAX_UD34x4 = 340282366920938463463374607431768211455; - -uint128 constant uUNIT = 1e4; - -/// @notice Casts an UD34x4 number into UD60x18. -/// @dev Requirements: -/// - x must be less than or equal to `uMAX_UD2x18`. -function intoUD60x18(UD34x4 x) pure returns (UD60x18) { - return UD60x18.wrap(uint256(UD34x4.unwrap(x)) * uint256(1e14)); -} - -/// @notice Casts an UD60x18 number into UD34x4 -/// @dev Requirements: -/// - x must be less than or equal to `uMAX_UD34x4`. -/// @param x The value to be converted -/// @return The value as UD34x4 -function fromUD60x18(UD60x18 x) pure returns (UD34x4) { - uint256 xUint = UD60x18.unwrap(x) / 1e14; - if (xUint > uMAX_UD34x4) { - revert PRBMath_UD34x4_fromUD60x18_Convert_Overflow(x.unwrap()); - } - return UD34x4.wrap(uint128(xUint)); -} - -/// @notice Converts an UD34x4 number to a simple integer by dividing it by `UNIT`. Rounds towards zero in the process. -/// @dev Rounds down in the process. -/// @param x The UD34x4 number to convert. -/// @return result The same number in basic integer form. -function convert(UD34x4 x) pure returns (uint128 result) { - result = UD34x4.unwrap(x) / uUNIT; -} - -/// @notice Converts a simple integer to UD34x4 by multiplying it by `UNIT`. -/// -/// @dev Requirements: -/// - x must be less than or equal to `MAX_UD34x4` divided by `UNIT`. -/// -/// @param x The basic integer to convert. -/// @param result The same number converted to UD34x4. -function convert(uint128 x) pure returns (UD34x4 result) { - if (x > uMAX_UD34x4 / uUNIT) { - revert PRBMath_UD34x4_Convert_Overflow(x); - } - unchecked { - result = UD34x4.wrap(x * uUNIT); - } -} - -/// @notice Alias for the `convert` function defined above. -function fromUD34x4(UD34x4 x) pure returns (uint128 result) { - result = convert(x); -} - -/// @notice Alias for the `convert` function defined above. -function toUD34x4(uint128 x) pure returns (UD34x4 result) { - result = convert(x); -} diff --git a/test/PrizePool.t.sol b/test/PrizePool.t.sol index 559529f..0f41ea1 100644 --- a/test/PrizePool.t.sol +++ b/test/PrizePool.t.sol @@ -6,7 +6,6 @@ import "forge-std/Test.sol"; import { ERC20 } from "openzeppelin/token/ERC20/ERC20.sol"; import { IERC20 } from "openzeppelin/token/ERC20/IERC20.sol"; import { sd, SD59x18 } from "prb-math/SD59x18.sol"; -import { UD34x4, fromUD34x4 } from "../src/libraries/UD34x4.sol"; import { UD2x18, ud2x18 } from "prb-math/UD2x18.sol"; import { SD1x18, sd1x18 } from "prb-math/SD1x18.sol"; import { TwabController } from "pt-v5-twab-controller/TwabController.sol"; @@ -88,7 +87,7 @@ contract PrizePoolTest is Test { uint8 lastNumTiers, uint8 numTiers, uint104 reserve, - UD34x4 prizeTokensPerShare, + uint128 prizeTokensPerShare, uint48 drawOpenedAt ); event SetDrawManager(address indexed drawManager); @@ -637,7 +636,7 @@ contract PrizePoolTest is Test { uint24 expectedDrawId = 1; vm.expectEmit(true, true, true, false); - emit DrawAwarded(expectedDrawId, 12345, 3, 3, 0, UD34x4.wrap(0), firstDrawOpensAt); + emit DrawAwarded(expectedDrawId, 12345, 3, 3, 0, 0, firstDrawOpensAt); vm.warp(prizePool.drawClosesAt(prizePool.getOpenDrawId())); uint24 closedDrawId = prizePool.awardDraw(12345); @@ -692,7 +691,7 @@ contract PrizePoolTest is Test { uint256 remainder = 100e18 - liquidityPerShare * prizePool.getTotalShares(); assertEq( - fromUD34x4(prizePool.prizeTokenPerShare()), + prizePool.prizeTokenPerShare(), liquidityPerShare, "prize token per share" ); @@ -951,7 +950,7 @@ contract PrizePoolTest is Test { MINIMUM_NUMBER_OF_TIERS, MINIMUM_NUMBER_OF_TIERS+1, 45454545454545576 /*reserve from output*/, - UD34x4.wrap(45454545454545450000) /*prize tokens per share from output*/, + 4545454545454545 /*prize tokens per share from output*/, firstDrawOpensAt + drawPeriodSeconds ); awardDraw(245); @@ -976,7 +975,7 @@ contract PrizePoolTest is Test { MINIMUM_NUMBER_OF_TIERS+1, MINIMUM_NUMBER_OF_TIERS, 78125000000000236 /*reserve from output*/, - UD34x4.wrap(78124999999999990000) /*prize tokens per share from output*/, + 7812499999999999 /*prize tokens per share from output*/, firstDrawOpensAt + drawPeriodSeconds*2 ); awardDraw(245); @@ -1023,7 +1022,7 @@ contract PrizePoolTest is Test { function testAwardDraw_emitsEvent() public { vm.expectEmit(); - emit DrawAwarded(1, 12345, MINIMUM_NUMBER_OF_TIERS, MINIMUM_NUMBER_OF_TIERS, 0, UD34x4.wrap(0), firstDrawOpensAt); + emit DrawAwarded(1, 12345, MINIMUM_NUMBER_OF_TIERS, MINIMUM_NUMBER_OF_TIERS, 0, 0, firstDrawOpensAt); awardDraw(12345); } @@ -1062,14 +1061,17 @@ contract PrizePoolTest is Test { uint256 tierLiquidity = TIER_SHARES * (100e18 / prizePool.getTotalShares()); assertEq(prizePool.getTierRemainingLiquidity(1), tierLiquidity, "second tier"); + + // Get the initial reserve to compare any additions after claim + uint256 initialReserve = prizePool.reserve(); mockTwab(address(this), sender1, 1); uint256 prize = prizePool.getTierPrizeSize(1); assertEq(claimPrize(sender1, 1, 0), prize, "second tier prize 1"); - // reduce by prize + // reduced by prize assertEq( - prizePool.getTierRemainingLiquidity(1), + prizePool.getTierRemainingLiquidity(1) + (prizePool.reserve() - initialReserve), // rounding errors are dumped in the reserve tierLiquidity - prize, "second tier liquidity post claim 1" ); diff --git a/test/abstract/TieredLiquidityDistributor.t.sol b/test/abstract/TieredLiquidityDistributor.t.sol index e0d47f0..cd282a3 100644 --- a/test/abstract/TieredLiquidityDistributor.t.sol +++ b/test/abstract/TieredLiquidityDistributor.t.sol @@ -12,7 +12,6 @@ import { TierLiquidityUtilizationRateGreaterThanOne, TierLiquidityUtilizationRateCannotBeZero, InsufficientLiquidity, - fromUD34x4toUD60x18, convert, SD59x18, sd, @@ -276,9 +275,9 @@ contract TieredLiquidityDistributorTest is Test { // uint96 amount = 100e18; distributor.awardDraw(15, amount); - UD60x18 prizeTokenPerShare = fromUD34x4toUD60x18(distributor.prizeTokenPerShare()); - uint256 total = convert( - prizeTokenPerShare.mul(convert(distributor.getTotalShares() - distributor.reserveShares())) + uint128 prizeTokenPerShare = distributor.prizeTokenPerShare(); + uint256 total = ( + prizeTokenPerShare * (distributor.getTotalShares() - distributor.reserveShares()) ) + distributor.reserve(); assertEq(total, amount, "prize token per share against total shares"); diff --git a/test/abstract/helper/TieredLiquidityDistributorWrapper.sol b/test/abstract/helper/TieredLiquidityDistributorWrapper.sol index 2482d5d..c651b48 100644 --- a/test/abstract/helper/TieredLiquidityDistributorWrapper.sol +++ b/test/abstract/helper/TieredLiquidityDistributorWrapper.sol @@ -3,8 +3,7 @@ pragma solidity ^0.8.19; import "forge-std/console2.sol"; -import { TieredLiquidityDistributor, Tier, fromUD34x4toUD60x18, convert } from "../../../src/abstract/TieredLiquidityDistributor.sol"; -import { UD60x18 } from "prb-math/UD60x18.sol"; +import { TieredLiquidityDistributor, Tier } from "../../../src/abstract/TieredLiquidityDistributor.sol"; contract TieredLiquidityDistributorWrapper is TieredLiquidityDistributor { constructor( @@ -30,10 +29,10 @@ contract TieredLiquidityDistributorWrapper is TieredLiquidityDistributor { function computeNewDistributions( uint8 _numberOfTiers, uint8 _nextNumberOfTiers, - UD60x18 _currentPrizeTokenPerShare, + uint128 _currentPrizeTokenPerShare, uint256 _prizeTokenLiquidity - ) external view returns (uint96, UD60x18) { - (uint96 newReserve, UD60x18 newPrizeTokenPerShare) = _computeNewDistributions( + ) external view returns (uint96, uint128) { + (uint96 newReserve, uint128 newPrizeTokenPerShare) = _computeNewDistributions( _numberOfTiers, _nextNumberOfTiers, _currentPrizeTokenPerShare, diff --git a/test/invariants/helpers/TieredLiquidityDistributorFuzzHarness.sol b/test/invariants/helpers/TieredLiquidityDistributorFuzzHarness.sol index 04a6ee9..4d47e76 100644 --- a/test/invariants/helpers/TieredLiquidityDistributorFuzzHarness.sol +++ b/test/invariants/helpers/TieredLiquidityDistributorFuzzHarness.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.19; import { TieredLiquidityDistributor, Tier, - fromUD34x4toUD60x18, convert, MINIMUM_NUMBER_OF_TIERS } from "../../../src/abstract/TieredLiquidityDistributor.sol"; @@ -31,12 +30,10 @@ contract TieredLiquidityDistributorFuzzHarness is TieredLiquidityDistributor { for (uint8 i = 0; i < numberOfTiers; i++) { Tier memory tier = _getTier(i, numberOfTiers); - availableLiquidity += convert( - _getTierRemainingLiquidity( - fromUD34x4toUD60x18(tier.prizeTokenPerShare), - fromUD34x4toUD60x18(prizeTokenPerShare), - i - ) + availableLiquidity += _getTierRemainingLiquidity( + tier.prizeTokenPerShare, + prizeTokenPerShare, + _numShares(i, numberOfTiers) ); } @@ -50,12 +47,10 @@ contract TieredLiquidityDistributorFuzzHarness is TieredLiquidityDistributor { Tier memory tier_ = _getTier(tier, numberOfTiers); uint104 liq = uint104( - convert( - _getTierRemainingLiquidity( - fromUD34x4toUD60x18(tier_.prizeTokenPerShare), - fromUD34x4toUD60x18(prizeTokenPerShare), - _tier - ) + _getTierRemainingLiquidity( + tier_.prizeTokenPerShare, + prizeTokenPerShare, + _numShares(_tier, numberOfTiers) ) ); diff --git a/test/libraries/UD34x4.t.sol b/test/libraries/UD34x4.t.sol deleted file mode 100644 index 4cb1fd6..0000000 --- a/test/libraries/UD34x4.t.sol +++ /dev/null @@ -1,46 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; - -import "forge-std/Test.sol"; - -import { UD60x18, convert } from "prb-math/UD60x18.sol"; -import { UD34x4, toUD34x4, fromUD34x4, intoUD60x18, fromUD60x18, uUNIT, PRBMath_UD34x4_Convert_Overflow, PRBMath_UD34x4_fromUD60x18_Convert_Overflow, uMAX_UD34x4 } from "../../src/libraries/UD34x4.sol"; - -contract UD34x4Test is Test { - function testToUD34x4_withUintMax() public { - uint128 legalMax = type(uint128).max / uUNIT; - UD34x4 result = toUD34x4(legalMax); - assertEq(UD34x4.unwrap(result), legalMax * uUNIT); - } - - function testToUD34x4_overflow() public { - uint128 legalMax = uMAX_UD34x4 / uUNIT; - vm.expectRevert(abi.encodeWithSelector(PRBMath_UD34x4_Convert_Overflow.selector, legalMax + 1)); - toUD34x4(legalMax + 1); - } - - function testIntoUD60x18() public { - UD34x4 x = UD34x4.wrap(100e4); - UD60x18 result = intoUD60x18(x); - assertEq(result.unwrap(), 100e18); - } - - function testIntoUD60x18_large() public pure { - UD34x4 x = UD34x4.wrap(6004291579826925202373984590); - intoUD60x18(x); - } - - function testFromUD60x18_normal() public { - UD60x18 x = UD60x18.wrap(100.1234e18); - UD34x4 result = fromUD60x18(x); - assertEq(UD34x4.unwrap(result), 100.1234e4); - } - - function testFromUD60x18_overflow() public { - UD60x18 x = convert(uMAX_UD34x4); - vm.expectRevert( - abi.encodeWithSelector(PRBMath_UD34x4_fromUD60x18_Convert_Overflow.selector, x.unwrap()) - ); - fromUD60x18(x); - } -} From 633865ae4265e779fcc9231141d9e3ec753d3aec Mon Sep 17 00:00:00 2001 From: Trevor Richard Date: Fri, 5 Apr 2024 20:44:02 +0000 Subject: [PATCH 4/5] add regression test --- .../abstract/TieredLiquidityDistributor.t.sol | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/abstract/TieredLiquidityDistributor.t.sol b/test/abstract/TieredLiquidityDistributor.t.sol index cd282a3..00b8e35 100644 --- a/test/abstract/TieredLiquidityDistributor.t.sol +++ b/test/abstract/TieredLiquidityDistributor.t.sol @@ -21,6 +21,9 @@ import { } from "../../src/abstract/TieredLiquidityDistributor.sol"; contract TieredLiquidityDistributorTest is Test { + + event ReserveConsumed(uint256 amount); + TieredLiquidityDistributorWrapper public distributor; uint24 grandPrizePeriodDraws; @@ -161,6 +164,38 @@ contract TieredLiquidityDistributorTest is Test { assertEq(distributor.getTierRemainingLiquidity(3), 5e18, "tier 3"); } + // regression test to see if there are any unaccounted rounding errors on consumeLiquidity + function testConsumeLiquidity_roundingErrors() public { + distributor = new TieredLiquidityDistributorWrapper( + tierLiquidityUtilizationRate, + numberOfTiers, + 100, + 9, + 0, + grandPrizePeriodDraws + ); + distributor.awardDraw(MINIMUM_NUMBER_OF_TIERS, 218e18); // 100 for each tier + 9 for each canary + + uint256 reserveBefore = distributor.reserve(); + + // There is 9e18 liquidity available for tier 3. + // Each time we consume 1 liquidity we will lose 0.00001 to rounding errors in + // the tier.prizeTokenPerShare value. Over time, this will accumulate and lead + // to the tier thinking is has more remainingLiquidity than it actually does. + for (uint i = 1; i <= 10000; i++) { + distributor.consumeLiquidity(3, 1); + assertEq(distributor.getTierRemainingLiquidity(3) + (distributor.reserve() - reserveBefore), 9e18 - i); + } + + // Test that we can still consume the rest of the liquidity even it if dips in the reserve + assertEq(distributor.getTierRemainingLiquidity(3), 9e18 - 90000); // 10000 consumed + 10000 rounding errors, rounding up by 8 each time + assertEq(distributor.reserve(), 80000); + vm.expectEmit(); + emit ReserveConsumed(80000); // equal to the rounding errors (8 for each one) + distributor.consumeLiquidity(3, 9e18 - 10000); // we only consumed 10000, so we should still be able to consume the rest by dipping into reserve + assertEq(distributor.getTierRemainingLiquidity(3), 0); + } + function testConsumeLiquidity_partial() public { distributor.awardDraw(MINIMUM_NUMBER_OF_TIERS, 220e18); distributor.consumeLiquidity(1, 50e18); // consume full liq for tier 1 From b076f59a28790dac4b192c0af08cbfd7384d4d1c Mon Sep 17 00:00:00 2001 From: Trevor Richard Date: Fri, 5 Apr 2024 21:44:14 +0000 Subject: [PATCH 5/5] update rounding up comments and var name --- src/abstract/TieredLiquidityDistributor.sol | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/abstract/TieredLiquidityDistributor.sol b/src/abstract/TieredLiquidityDistributor.sol index 9fa1551..7fa0a6a 100644 --- a/src/abstract/TieredLiquidityDistributor.sol +++ b/src/abstract/TieredLiquidityDistributor.sol @@ -386,15 +386,17 @@ contract TieredLiquidityDistributor { _tierStruct.prizeTokenPerShare = prizeTokenPerShare; } else { uint8 _remainder = uint8(_liquidity % _tierShares); - uint8 _overConsumption = _remainder == 0 ? 0 : _tierShares - _remainder; - if (_overConsumption > 0) { + uint8 _roundUpConsumption = _remainder == 0 ? 0 : _tierShares - _remainder; + if (_roundUpConsumption > 0) { // We must round up our tier prize token per share value to ensure we don't over-award the tier's - // liquidity, but any over-consumption can be contributed to the reserve so every wei is accounted - // for. - _reserve += _overConsumption; + // liquidity, but any extra rounded up consumption can be contributed to the reserve so every wei + // is accounted for. + _reserve += _roundUpConsumption; } - _tierStruct.prizeTokenPerShare += SafeCast.toUint104(uint256(_liquidity) + _overConsumption) / _tierShares; + // We know that the rounded up `liquidity` won't exceed the `remainingLiquidity` since the `remainingLiquidity` + // is an integer multiple of `_tierShares` and we check above that `_liquidity <= remainingLiquidity`. + _tierStruct.prizeTokenPerShare += SafeCast.toUint104(uint256(_liquidity) + _roundUpConsumption) / _tierShares; } _tiers[_tier] = _tierStruct;