From 9ae64e73127102cf25f0e54d78abb431e456d611 Mon Sep 17 00:00:00 2001 From: Pierrick Turelier Date: Fri, 12 Aug 2022 15:13:29 -0500 Subject: [PATCH 1/3] fix(PrizeTierHistory): check tiers ceiling --- contracts/PrizeTierHistory.sol | 83 +++++++++++++++++----- contracts/interfaces/IPrizeTierHistory.sol | 26 ++++--- test/PrizeTierHistory.test.ts | 77 +++++++++++++++++--- 3 files changed, 148 insertions(+), 38 deletions(-) diff --git a/contracts/PrizeTierHistory.sol b/contracts/PrizeTierHistory.sol index bb971df..f7b8a20 100644 --- a/contracts/PrizeTierHistory.sol +++ b/contracts/PrizeTierHistory.sol @@ -7,14 +7,13 @@ import "./libraries/BinarySearchLib.sol"; /** * @title PoolTogether V4 PrizeTierHistory * @author PoolTogether Inc Team - * @notice The PrizeTierHistory smart contract stores a history of PrizeTier structs linked to - a range of valid Draw IDs. + * @notice The PrizeTierHistory smart contract stores a history of PrizeTier structs linked to + a range of valid Draw IDs. * @dev If the history param has single PrizeTier struct with a "drawId" of 1 all subsequent Draws will use that PrizeTier struct for PrizeDitribution calculations. The BinarySearchLib will find a PrizeTier using a "atOrBefore" range search when supplied drawId input parameter. */ contract PrizeTierHistory is IPrizeTierHistory, Manageable { - // @dev The uint32[] type is extended with a binarySearch(uint32) function. using BinarySearchLib for uint32[]; @@ -22,7 +21,7 @@ contract PrizeTierHistory is IPrizeTierHistory, Manageable { * @notice Ordered array of Draw IDs * @dev The history, with sequentially ordered ids, can be searched using binary search. The binary search will find index of a drawId (atOrBefore) using a specific drawId (at). - When a new Draw ID is added to the history, a corresponding mapping of the ID is + When a new Draw ID is added to the history, a corresponding mapping of the ID is updated in the prizeTiers mapping. */ uint32[] internal history; @@ -34,13 +33,19 @@ contract PrizeTierHistory is IPrizeTierHistory, Manageable { */ mapping(uint32 => PrizeTier) internal prizeTiers; + /** + * @notice Ceiling for the total sum of tiers from the prize distribution. 1e9 = 100%. + * @dev It's fixed point 9 because 1e9 is the largest "1" that fits into 2**32 + */ + uint256 internal constant TIERS_CEILING = 1e9; + constructor(address owner) Ownable(owner) {} // @inheritdoc IPrizeTierHistory function count() external view override returns (uint256) { return history.length; } - + // @inheritdoc IPrizeTierHistory function getOldestDrawId() external view override returns (uint32) { return history[0]; @@ -52,14 +57,19 @@ contract PrizeTierHistory is IPrizeTierHistory, Manageable { } // @inheritdoc IPrizeTierHistory - function getPrizeTier(uint32 drawId) override external view returns (PrizeTier memory) { + function getPrizeTier(uint32 drawId) external view override returns (PrizeTier memory) { require(drawId > 0, "PrizeTierHistory/draw-id-not-zero"); return prizeTiers[history.binarySearch(drawId)]; } // @inheritdoc IPrizeTierHistory - function getPrizeTierList(uint32[] calldata _drawIds) override external view returns (PrizeTier[] memory) { - uint256 _length = _drawIds.length; + function getPrizeTierList(uint32[] calldata _drawIds) + external + view + override + returns (PrizeTier[] memory) + { + uint256 _length = _drawIds.length; PrizeTier[] memory _data = new PrizeTier[](_length); for (uint256 index = 0; index < _length; index++) { _data[index] = prizeTiers[history.binarySearch(_drawIds[index])]; @@ -73,13 +83,18 @@ contract PrizeTierHistory is IPrizeTierHistory, Manageable { } // @inheritdoc IPrizeTierHistory - function push(PrizeTier calldata nextPrizeTier) override external onlyManagerOrOwner { + function push(PrizeTier calldata nextPrizeTier) external override onlyManagerOrOwner { _push(nextPrizeTier); } - + // @inheritdoc IPrizeTierHistory - function popAndPush(PrizeTier calldata newPrizeTier) override external onlyOwner returns (uint32) { - uint length = history.length; + function popAndPush(PrizeTier calldata newPrizeTier) + external + override + onlyOwner + returns (uint32) + { + uint256 length = history.length; require(length > 0, "PrizeTierHistory/history-empty"); require(history[length - 1] == newPrizeTier.drawId, "PrizeTierHistory/invalid-draw-id"); _replace(newPrizeTier); @@ -87,32 +102,66 @@ contract PrizeTierHistory is IPrizeTierHistory, Manageable { } // @inheritdoc IPrizeTierHistory - function replace(PrizeTier calldata newPrizeTier) override external onlyOwner { + function replace(PrizeTier calldata newPrizeTier) external override onlyOwner { _replace(newPrizeTier); } + /** + * @notice Check that the total sum of the tiers is not greater than 1e9 (100%). + * @param _tiers Array of tiers to check + */ + function _checkTiersTotalSum(uint32[16] memory _tiers) internal pure { + uint256 tiersTotalSum; + uint256 tiersLength = _tiers.length; + + for (uint256 index; index < tiersLength; index++) { + tiersTotalSum += _tiers[index]; + } + + require(tiersTotalSum <= TIERS_CEILING, "PrizeTierHistory/tiers-gt-100%"); + } + + /** + * @notice Push PrizeTier struct onto `prizeTiers` array. + * @dev Callable only by the owner or manager. + * @dev `drawId` must be greater than the latest one stored in `history`. + * @param _prizeTier Next PrizeTier struct + */ function _push(PrizeTier memory _prizeTier) internal { uint32 _length = uint32(history.length); + if (_length > 0) { uint32 _id = history[_length - 1]; - require( - _prizeTier.drawId > _id, - "PrizeTierHistory/non-sequential-id" - ); + require(_prizeTier.drawId > _id, "PrizeTierHistory/non-sequential-id"); } + + _checkTiersTotalSum(_prizeTier.tiers); + history.push(_prizeTier.drawId); prizeTiers[_length] = _prizeTier; + emit PrizeTierPushed(_prizeTier.drawId, _prizeTier); } + /** + * @notice Replace PrizeTier struct in `prizeTiers` array. + * @dev Callable only by the owner. + * @param _prizeTier PrizeTier parameters + */ function _replace(PrizeTier calldata _prizeTier) internal { uint256 cardinality = history.length; require(cardinality > 0, "PrizeTierHistory/no-prize-tiers"); + uint32 oldestDrawId = history[0]; require(_prizeTier.drawId >= oldestDrawId, "PrizeTierHistory/draw-id-out-of-range"); + uint32 index = history.binarySearch(_prizeTier.drawId); require(history[index] == _prizeTier.drawId, "PrizeTierHistory/draw-id-must-match"); + + _checkTiersTotalSum(_prizeTier.tiers); + prizeTiers[index] = _prizeTier; + emit PrizeTierSet(_prizeTier.drawId, _prizeTier); } } diff --git a/contracts/interfaces/IPrizeTierHistory.sol b/contracts/interfaces/IPrizeTierHistory.sol index 806ce8b..3bdc30f 100644 --- a/contracts/interfaces/IPrizeTierHistory.sol +++ b/contracts/interfaces/IPrizeTierHistory.sol @@ -36,13 +36,19 @@ interface IPrizeTierHistory { event PrizeTierSet(uint32 indexed drawId, PrizeTier prizeTier); /** - * @notice Push PrizeTierHistory struct onto history array. - * @dev Callable only by owner or manager, - * @param drawPrizeDistribution New PrizeTierHistory struct + * @notice Push PrizeTier struct onto `prizeTiers` array. + * @dev Callable only by the owner or manager. + * @dev `drawId` must be greater than the latest one stored in `history`. + * @param nextPrizeTier Next PrizeTier struct */ - function push(PrizeTier calldata drawPrizeDistribution) external; + function push(PrizeTier calldata nextPrizeTier) external; - function replace(PrizeTier calldata _prizeTier) external; + /** + * @notice Replace PrizeTier struct in `prizeTiers` array. + * @dev Callable only by the owner. + * @param newPrizeTier PrizeTier parameters + */ + function replace(PrizeTier calldata newPrizeTier) external; /** * @notice Read PrizeTierHistory struct from history array. @@ -70,12 +76,12 @@ interface IPrizeTierHistory { returns (PrizeTier[] memory prizeTierList); /** - * @notice Push PrizeTierHistory struct onto history array. - * @dev Callable only by owner. - * @param prizeTier Updated PrizeTierHistory struct - * @return drawId Draw ID linked to PrizeTierHistory + * @notice Pop the latest prize tier stored in the `prizeTiers` array and replace it with the new prize tier. + * @dev Callable only by the owner. + * @param newPrizeTier Updated PrizeTier struct + * @return drawId Draw ID of the PrizeTier that was pushed */ - function popAndPush(PrizeTier calldata prizeTier) external returns (uint32 drawId); + function popAndPush(PrizeTier calldata newPrizeTier) external returns (uint32 drawId); function getPrizeTierAtIndex(uint256 index) external view returns (PrizeTier memory); diff --git a/test/PrizeTierHistory.test.ts b/test/PrizeTierHistory.test.ts index cac46de..e06bfe9 100644 --- a/test/PrizeTierHistory.test.ts +++ b/test/PrizeTierHistory.test.ts @@ -16,12 +16,14 @@ describe('PrizeTierHistory', () => { let prizeTierHistory: Contract; let prizeTierHistoryFactory: ContractFactory; + const tiers = range(16, 0).map((i) => 0); + const prizeTiers = [ { bitRangeSize: 5, drawId: 1, maxPicksPerUser: 10, - tiers: range(16, 0).map((i) => 0), + tiers, expiryDuration: 10000, prize: toWei('10000'), endTimestampOffset: 3000, @@ -30,7 +32,7 @@ describe('PrizeTierHistory', () => { bitRangeSize: 5, drawId: 6, maxPicksPerUser: 10, - tiers: range(16, 0).map((i) => 0), + tiers, expiryDuration: 10000, prize: toWei('10000'), endTimestampOffset: 3000, @@ -39,7 +41,7 @@ describe('PrizeTierHistory', () => { bitRangeSize: 5, drawId: 9, maxPicksPerUser: 10, - tiers: range(16, 0).map((i) => 0), + tiers, expiryDuration: 10000, prize: toWei('10000'), endTimestampOffset: 3000, @@ -48,13 +50,32 @@ describe('PrizeTierHistory', () => { bitRangeSize: 5, drawId: 20, maxPicksPerUser: 10, - tiers: range(16, 0).map((i) => 0), + tiers, expiryDuration: 10000, prize: toWei('10000'), endTimestampOffset: 3000, }, ]; + const tiersGT1e9 = [ + 141787658, + 0, + 0, + 0, + 85072595, + 0, + 0, + 136116152, + 136116152, + 108892921, + 0, + 0, + 217785843, + 174228675, + 174228675, + 0 + ]; + const pushPrizeTiers = async () => { await Promise.all(prizeTiers.map(async (tier) => { await prizeTierHistory.push(tier); @@ -76,19 +97,19 @@ describe('PrizeTierHistory', () => { const count = await prizeTierHistory.count(); expect(count).to.equal(4); }); - + it('should succeed to get oldest Draw Id', async () => { await pushPrizeTiers(); const oldestDrawId = await prizeTierHistory.getOldestDrawId(); expect(oldestDrawId).to.equal(1); }); - + it('should succeed to get newest Draw Id', async () => { await pushPrizeTiers(); const newestDrawId = await prizeTierHistory.getNewestDrawId(); expect(newestDrawId).to.equal(20); }); - + it('should succeed to get a PrizeTier using an index position', async () => { await pushPrizeTiers(); const prizeTier = await prizeTierHistory.getPrizeTierAtIndex(3); @@ -152,9 +173,19 @@ describe('PrizeTierHistory', () => { prizeTierHistory.connect(wallet3 as unknown as Signer).push(prizeTiers[0]), ).to.be.revertedWith('Manageable/caller-not-manager-or-owner'); }); + + it('should fail to push a PrizeTier if the sum of tiers is greater than 1e9', async () => { + prizeTiers[0].tiers = tiersGT1e9; + + await expect( + prizeTierHistory.push(prizeTiers[0]), + ).to.be.revertedWith('PrizeTierHistory/tiers-gt-100%'); + + prizeTiers[0].tiers = tiers; + }); }); - describe('.set()', () => { + describe('.popAndPush()', () => { it('should succeed to set existing PrizeTier in history from Owner wallet.', async () => { await pushPrizeTiers(); const prizeTier = { @@ -208,6 +239,18 @@ describe('PrizeTierHistory', () => { ).popAndPush(prizeTiers[0]), ).to.revertedWith('Ownable/caller-not-owner'); }); + + it('should fail to popAndPush a PrizeTier if the sum of tiers is greater than 1e9', async () => { + await prizeTierHistory.push(prizeTiers[0]); + + prizeTiers[0].tiers = tiersGT1e9; + + await expect( + prizeTierHistory.popAndPush(prizeTiers[0]), + ).to.be.revertedWith('PrizeTierHistory/tiers-gt-100%'); + + prizeTiers[0].tiers = tiers; + }); }); }); @@ -236,14 +279,26 @@ describe('PrizeTierHistory', () => { 'PrizeTierHistory/no-prize-tiers', ); }); - - it('should fail to replace a PrizeTier that is out of rance', async () => { + + it('should fail to replace a PrizeTier that is out of range', async () => { await prizeTierHistory.push(prizeTiers[3]); await expect(prizeTierHistory.replace(prizeTiers[0])).to.be.revertedWith( 'PrizeTierHistory/draw-id-out-of-range', ); }); - + + it('should fail to replace a PrizeTier if the sum of tiers is greater than 1e9', async () => { + await prizeTierHistory.push(prizeTiers[3]); + + prizeTiers[3].tiers = tiersGT1e9; + + await expect( + prizeTierHistory.replace(prizeTiers[3]), + ).to.be.revertedWith('PrizeTierHistory/tiers-gt-100%'); + + prizeTiers[3].tiers = tiers; + }); + it('should fail to replace a non-existent PrizeTier', async () => { await pushPrizeTiers(); const prizeTier = { From c77965b246298addc299b561d50d59576a182c99 Mon Sep 17 00:00:00 2001 From: Pierrick Turelier Date: Fri, 12 Aug 2022 16:06:42 -0500 Subject: [PATCH 2/3] 1.2.6-beta.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a39a7fe..60cef95 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@pooltogether/v4-periphery", - "version": "1.2.5", + "version": "1.2.6-beta.1", "description": "PoolTogether V4 Periphery", "main": "index.js", "license": "GPL-3.0", From a6dce95b9cfd41d8ac733f8d0f70dc71ef4c6a18 Mon Sep 17 00:00:00 2001 From: Pierrick Turelier Date: Thu, 15 Sep 2022 16:28:41 -0500 Subject: [PATCH 3/3] 1.2.6 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 60cef95..3a0ce83 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@pooltogether/v4-periphery", - "version": "1.2.6-beta.1", + "version": "1.2.6", "description": "PoolTogether V4 Periphery", "main": "index.js", "license": "GPL-3.0",