From 82dcb577af03d3cbf0016c698a178314af6ede3b Mon Sep 17 00:00:00 2001 From: Kames Geraghty Date: Tue, 5 Oct 2021 12:16:55 -0400 Subject: [PATCH 1/2] update(TwabLib): uniform uint types for calculations --- contracts/libraries/ExtendedSafeCastLib.sol | 15 ++++++++++ contracts/libraries/TwabLib.sol | 14 +++++---- test/DrawCalculator.test.ts | 2 +- test/libraries/OverflowSafeComparator.test.ts | 30 +++++++++++++++---- 4 files changed, 50 insertions(+), 11 deletions(-) diff --git a/contracts/libraries/ExtendedSafeCastLib.sol b/contracts/libraries/ExtendedSafeCastLib.sol index 3eda606e..1c1716d7 100644 --- a/contracts/libraries/ExtendedSafeCastLib.sol +++ b/contracts/libraries/ExtendedSafeCastLib.sol @@ -32,4 +32,19 @@ library ExtendedSafeCastLib { require(value <= type(uint208).max, "SafeCast: value doesn't fit in 208 bits"); return uint208(value); } + + /** + * @dev Returns the downcasted uint224 from uint256, reverting on + * overflow (when the input is greater than largest uint224). + * + * Counterpart to Solidity's `uint224` operator. + * + * Requirements: + * + * - input must fit into 224 bits + */ + function toUint224(uint256 value) internal pure returns (uint224) { + require(value <= type(uint224).max, "SafeCast: value doesn't fit in 224 bits"); + return uint224(value); + } } diff --git a/contracts/libraries/TwabLib.sol b/contracts/libraries/TwabLib.sol index 00fb81e5..190aaaad 100644 --- a/contracts/libraries/TwabLib.sol +++ b/contracts/libraries/TwabLib.sol @@ -134,7 +134,9 @@ library TwabLib { ObservationLib.Observation[MAX_CARDINALITY] storage _twabs, AccountDetails memory _accountDetails ) internal view returns (uint24 index, ObservationLib.Observation memory twab) { - index = uint24(RingBufferLib.mostRecentIndex(_accountDetails.nextTwabIndex, MAX_CARDINALITY)); + index = uint24( + RingBufferLib.mostRecentIndex(_accountDetails.nextTwabIndex, MAX_CARDINALITY) + ); twab = _twabs[index]; } @@ -311,15 +313,17 @@ library TwabLib { /// @return New TWAB that was recorded. function _computeNextTwab( ObservationLib.Observation memory _currentTwab, - uint256 _currentBalance, + uint224 _currentBalance, uint32 _time ) private pure returns (ObservationLib.Observation memory) { // New twab amount = last twab amount (or zero) + (current amount * elapsed seconds) return ObservationLib.Observation({ - amount: (uint256(_currentTwab.amount) + - (_currentBalance * (_time.checkedSub(_currentTwab.timestamp, _time)))) - .toUint208(), + amount: uint256( + _currentTwab.amount + + _currentBalance * + (_time.checkedSub(_currentTwab.timestamp, _time)) + ).toUint224(), timestamp: _time }); } diff --git a/test/DrawCalculator.test.ts b/test/DrawCalculator.test.ts index e8e2e535..de499d5d 100644 --- a/test/DrawCalculator.test.ts +++ b/test/DrawCalculator.test.ts @@ -337,7 +337,7 @@ describe('DrawCalculator', () => { const userRandomNumber = '0x369ddb959b07c1d22a9bada1f3420961d0e0252f73c0f5b2173d7f7c6fe12b70'; // intentionally same as winning random number - const prizeDistributionIndex: BigNumber = + const prizeDistributionIndex: BigNumber = await drawCalculator.calculateDistributionIndex( userRandomNumber, winningRandomNumber, diff --git a/test/libraries/OverflowSafeComparator.test.ts b/test/libraries/OverflowSafeComparator.test.ts index 9389b6fd..9fd48170 100644 --- a/test/libraries/OverflowSafeComparator.test.ts +++ b/test/libraries/OverflowSafeComparator.test.ts @@ -70,7 +70,11 @@ describe('overflowSafeComparatorLib', () => { const timestampB = currentTimestamp - 100; expect( - await overflowSafeComparatorLib.lteHarness(timestampA, timestampB, currentTimestamp), + await overflowSafeComparatorLib.lteHarness( + timestampA, + timestampB, + currentTimestamp, + ), ).to.equal(true); }); @@ -79,7 +83,11 @@ describe('overflowSafeComparatorLib', () => { const timestampB = timestampA; expect( - await overflowSafeComparatorLib.lteHarness(timestampA, timestampB, currentTimestamp), + await overflowSafeComparatorLib.lteHarness( + timestampA, + timestampB, + currentTimestamp, + ), ).to.equal(true); }); @@ -88,7 +96,11 @@ describe('overflowSafeComparatorLib', () => { const timestampB = currentTimestamp + 1000; expect( - await overflowSafeComparatorLib.lteHarness(timestampA, timestampB, currentTimestamp), + await overflowSafeComparatorLib.lteHarness( + timestampA, + timestampB, + currentTimestamp, + ), ).to.equal(false); }); @@ -97,7 +109,11 @@ describe('overflowSafeComparatorLib', () => { const timestampB = currentTimestamp - 1000; expect( - await overflowSafeComparatorLib.lteHarness(timestampA, timestampB, currentTimestamp), + await overflowSafeComparatorLib.lteHarness( + timestampA, + timestampB, + currentTimestamp, + ), ).to.equal(true); }); @@ -106,7 +122,11 @@ describe('overflowSafeComparatorLib', () => { const timestampB = timestampA; expect( - await overflowSafeComparatorLib.lteHarness(timestampA, timestampB, currentTimestamp), + await overflowSafeComparatorLib.lteHarness( + timestampA, + timestampB, + currentTimestamp, + ), ).to.equal(true); }); }); From bdc767e367db253b6bbbd3ad050c5c6e8594dee2 Mon Sep 17 00:00:00 2001 From: Brendan Asselstine Date: Tue, 5 Oct 2021 19:29:56 -0700 Subject: [PATCH 2/2] Removed extraneous cast --- contracts/libraries/TwabLib.sol | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/contracts/libraries/TwabLib.sol b/contracts/libraries/TwabLib.sol index 190aaaad..a3c8725e 100644 --- a/contracts/libraries/TwabLib.sol +++ b/contracts/libraries/TwabLib.sol @@ -319,11 +319,9 @@ library TwabLib { // New twab amount = last twab amount (or zero) + (current amount * elapsed seconds) return ObservationLib.Observation({ - amount: uint256( - _currentTwab.amount + + amount: _currentTwab.amount + _currentBalance * - (_time.checkedSub(_currentTwab.timestamp, _time)) - ).toUint224(), + (_time.checkedSub(_currentTwab.timestamp, _time)), timestamp: _time }); }