diff --git a/contracts/Reserve.sol b/contracts/Reserve.sol index f4c0b0b0..aac52a14 100644 --- a/contracts/Reserve.sol +++ b/contracts/Reserve.sol @@ -8,6 +8,7 @@ import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import "./interfaces/IReserve.sol"; import "./libraries/ObservationLib.sol"; +import "./libraries/RingBufferLib.sol"; /** * @title PoolTogether V4 Reserve @@ -29,14 +30,16 @@ contract Reserve is IReserve, Manageable { /// @notice Total withdraw amount from reserve uint224 public withdrawAccumulator; + uint32 private _gap; + + uint24 internal nextIndex; + uint24 internal cardinality; /// @notice The maximum number of twab entries - uint24 internal constant MAX_CARDINALITY = 16777215; // 2**24 + uint24 internal constant MAX_CARDINALITY = 16777215; // 2**24 - 1 ObservationLib.Observation[MAX_CARDINALITY] internal reserveAccumulators; - uint24 internal cardinality; - /* ============ Events ============ */ event Deployed(IERC20 indexed token); @@ -74,18 +77,16 @@ contract Reserve is IReserve, Manageable { { require(_startTimestamp < _endTimestamp, "Reserve/start-less-then-end"); uint24 _cardinality = cardinality; + uint24 _nextIndex = nextIndex; - ObservationLib.Observation memory _newestObservation; - - if (_cardinality > 0) { - _newestObservation = reserveAccumulators[_cardinality - 1]; - } - - ObservationLib.Observation memory _oldestObservation = reserveAccumulators[0]; + (uint24 _newestIndex, ObservationLib.Observation memory _newestObservation) = _getNewestObservation(_nextIndex); + (uint24 _oldestIndex, ObservationLib.Observation memory _oldestObservation) = _getOldestObservation(_nextIndex); uint224 _start = _getReserveAccumulatedAt( _newestObservation, _oldestObservation, + _newestIndex, + _oldestIndex, _cardinality, _startTimestamp ); @@ -93,6 +94,8 @@ contract Reserve is IReserve, Manageable { uint224 _end = _getReserveAccumulatedAt( _newestObservation, _oldestObservation, + _newestIndex, + _oldestIndex, _cardinality, _endTimestamp ); @@ -118,6 +121,8 @@ contract Reserve is IReserve, Manageable { * @dev Uses binary search if target timestamp is within ring buffer range. * @param _newestObservation ObservationLib.Observation * @param _oldestObservation ObservationLib.Observation + * @param _newestIndex The index of the newest observation + * @param _oldestIndex The index of the oldest observation * @param _cardinality RingBuffer Range * @param _timestamp Timestamp target * @@ -126,6 +131,8 @@ contract Reserve is IReserve, Manageable { function _getReserveAccumulatedAt( ObservationLib.Observation memory _newestObservation, ObservationLib.Observation memory _oldestObservation, + uint24 _newestIndex, + uint24 _oldestIndex, uint24 _cardinality, uint32 _timestamp ) internal view returns (uint224) { @@ -168,8 +175,8 @@ contract Reserve is IReserve, Manageable { ObservationLib.Observation memory atOrAfter ) = ObservationLib.binarySearch( reserveAccumulators, - _cardinality - 1, - 0, + _newestIndex, + _oldestIndex, _timestamp, _cardinality, timeNow @@ -188,34 +195,37 @@ contract Reserve is IReserve, Manageable { /// @notice Records the currently accrued reserve amount. function _checkpoint() internal { uint24 _cardinality = cardinality; + uint24 _nextIndex = nextIndex; uint256 _balanceOfReserve = token.balanceOf(address(this)); uint224 _withdrawAccumulator = withdrawAccumulator; //sload - ObservationLib.Observation memory _newestObservation = _getNewestObservation(_cardinality); + (uint24 newestIndex, ObservationLib.Observation memory newestObservation) = _getNewestObservation(_nextIndex); /** * IF tokens have been deposited into Reserve contract since the last checkpoint * create a new Reserve balance checkpoint. The will will update multiple times in a single block. */ - if (_balanceOfReserve + _withdrawAccumulator > _newestObservation.amount) { + if (_balanceOfReserve + _withdrawAccumulator > newestObservation.amount) { uint32 nowTime = uint32(block.timestamp); // checkpointAccumulator = currentBalance + totalWithdraws uint224 newReserveAccumulator = uint224(_balanceOfReserve) + _withdrawAccumulator; - // IF _newestObservation IS NOT in the current block. + // IF newestObservation IS NOT in the current block. // CREATE observation in the accumulators ring buffer. - if (_newestObservation.timestamp != nowTime) { - reserveAccumulators[_cardinality] = ObservationLib.Observation({ + if (newestObservation.timestamp != nowTime) { + reserveAccumulators[_nextIndex] = ObservationLib.Observation({ amount: newReserveAccumulator, timestamp: nowTime }); - - cardinality++; + nextIndex = uint24(RingBufferLib.nextIndex(_nextIndex, MAX_CARDINALITY)); + if (_cardinality < MAX_CARDINALITY) { + cardinality = _cardinality + 1; + } } - // ELSE IF _newestObservation IS in the current block. + // ELSE IF newestObservation IS in the current block. // UPDATE the checkpoint previously created in block history. else { - reserveAccumulators[_cardinality - 1] = ObservationLib.Observation({ + reserveAccumulators[newestIndex] = ObservationLib.Observation({ amount: newReserveAccumulator, timestamp: nowTime }); @@ -225,12 +235,28 @@ contract Reserve is IReserve, Manageable { } } + function _getOldestObservation(uint24 _nextIndex) + internal + view + returns (uint24 index, ObservationLib.Observation memory observation) + { + index = _nextIndex; + observation = reserveAccumulators[index]; + + // If the TWAB is not initialized we go to the beginning of the TWAB circular buffer at index 0 + if (observation.timestamp == 0) { + index = 0; + observation = reserveAccumulators[0]; + } + } + /// @notice Retrieves the newest observation - function _getNewestObservation(uint24 _cardinality) + function _getNewestObservation(uint24 _nextIndex) internal view - returns (ObservationLib.Observation memory _observation) + returns (uint24 index, ObservationLib.Observation memory observation) { - if (_cardinality > 0) _observation = reserveAccumulators[_cardinality - 1]; + index = uint24(RingBufferLib.newestIndex(_nextIndex, MAX_CARDINALITY)); + observation = reserveAccumulators[index]; } } diff --git a/contracts/test/ReserveHarness.sol b/contracts/test/ReserveHarness.sol index 5e4e83ea..87022590 100644 --- a/contracts/test/ReserveHarness.sol +++ b/contracts/test/ReserveHarness.sol @@ -8,27 +8,13 @@ import "./ERC20Mintable.sol"; contract ReserveHarness is Reserve { constructor(address _owner, IERC20 _token) Reserve(_owner, _token) {} - function getReserveAccumulatedAt( - ObservationLib.Observation memory _newestObservation, - ObservationLib.Observation memory _oldestObservation, - uint24 _cardinality, - uint32 timestamp - ) public view returns (uint224) { - return - _getReserveAccumulatedAt( - _newestObservation, - _oldestObservation, - _cardinality, - timestamp - ); - } - function setObservationsAt(ObservationLib.Observation[] calldata observations) external { for (uint256 i = 0; i < observations.length; i++) { reserveAccumulators[i] = observations[i]; } - cardinality = uint16(observations.length); + nextIndex = uint24(observations.length); + cardinality = uint24(observations.length); } function doubleCheckpoint(ERC20Mintable _token, uint256 _amount) external { diff --git a/test/Reserve.test.ts b/test/Reserve.test.ts index 05704478..f977cb8d 100644 --- a/test/Reserve.test.ts +++ b/test/Reserve.test.ts @@ -254,19 +254,6 @@ describe('Reserve', () => { }); }); - describe('_getReserveAccumulatedAt()', () => { - it('should return 0 if cardinality is equal to 0', async () => { - const newestObservation = { timestamp: 8, amount: 72 }; - const oldestObservation = { timestamp: 5, amount: 70 }; - - await reserve.setObservationsAt([oldestObservation, newestObservation]); - - expect( - await reserve.getReserveAccumulatedAt(newestObservation, oldestObservation, 0, 5), - ).to.equal(0); - }); - }); - describe('withdrawTo()', () => { it('should emit Checkpoint, Transfer and Withdrawn events', async () => { await ticket.mint(reserve.address, toWei('100'));