From ff7e8b8db06bf49a23fcc2849215cdd6b9e2325a Mon Sep 17 00:00:00 2001 From: Brendan Asselstine Date: Wed, 6 Oct 2021 15:08:54 -0700 Subject: [PATCH] PR comments --- contracts/DrawBeacon.sol | 4 +- contracts/DrawCalculator.sol | 10 +- .../libraries/OverflowSafeComparatorLib.sol | 4 +- .../OverflowSafeComparatorLibHarness.sol | 2 +- test/Ticket.test.ts | 96 ++++++++++--------- 5 files changed, 59 insertions(+), 57 deletions(-) diff --git a/contracts/DrawBeacon.sol b/contracts/DrawBeacon.sol index ac190b34..88bdef8b 100644 --- a/contracts/DrawBeacon.sol +++ b/contracts/DrawBeacon.sol @@ -191,7 +191,7 @@ contract DrawBeacon is IDrawBeacon, Ownable { } /// @inheritdoc IDrawBeacon - function calculateNextBeaconPeriodStartTime(uint256 _currentTime) + function calculateNextBeaconPeriodStartTime(uint256 __currentTime) external view override @@ -201,7 +201,7 @@ contract DrawBeacon is IDrawBeacon, Ownable { _calculateNextBeaconPeriodStartTime( beaconPeriodStartedAt, beaconPeriodSeconds, - uint64(_currentTime) + uint64(__currentTime) ); } diff --git a/contracts/DrawCalculator.sol b/contracts/DrawCalculator.sol index 213b584e..765e3568 100644 --- a/contracts/DrawCalculator.sol +++ b/contracts/DrawCalculator.sol @@ -245,12 +245,10 @@ contract DrawCalculator is IDrawCalculator, Ownable { // generate timestamps with draw cutoff offsets included for (uint32 i = 0; i < _draws.length; i++) { unchecked { - _timestampsWithStartCutoffTimes[i] = ( - _draws[i].timestamp - _prizeDistributions[i].startTimestampOffset - ); - _timestampsWithEndCutoffTimes[i] = ( - _draws[i].timestamp - _prizeDistributions[i].endTimestampOffset - ); + _timestampsWithStartCutoffTimes[i] = + _draws[i].timestamp - _prizeDistributions[i].startTimestampOffset; + _timestampsWithEndCutoffTimes[i] = + _draws[i].timestamp - _prizeDistributions[i].endTimestampOffset; } } diff --git a/contracts/libraries/OverflowSafeComparatorLib.sol b/contracts/libraries/OverflowSafeComparatorLib.sol index dc22dd7a..4e0b9a51 100644 --- a/contracts/libraries/OverflowSafeComparatorLib.sol +++ b/contracts/libraries/OverflowSafeComparatorLib.sol @@ -2,8 +2,6 @@ pragma solidity 0.8.6; -import "hardhat/console.sol"; - /// @title OverflowSafeComparatorLib library to share comparator functions between contracts /// @dev Code taken from Uniswap V3 Oracle.sol: https://github.com/Uniswap/v3-core/blob/3e88af408132fc957e3e406f65a0ce2b1ca06c3d/contracts/libraries/Oracle.sol /// @author PoolTogether Inc. @@ -38,7 +36,7 @@ library OverflowSafeComparatorLib { uint32 _a, uint32 _b, uint32 _timestamp - ) internal view returns (bool) { + ) internal pure returns (bool) { // No need to adjust if there hasn't been an overflow if (_a <= _timestamp && _b <= _timestamp) return _a <= _b; diff --git a/contracts/test/libraries/OverflowSafeComparatorLibHarness.sol b/contracts/test/libraries/OverflowSafeComparatorLibHarness.sol index fefacefc..64f79192 100644 --- a/contracts/test/libraries/OverflowSafeComparatorLibHarness.sol +++ b/contracts/test/libraries/OverflowSafeComparatorLibHarness.sol @@ -19,7 +19,7 @@ contract OverflowSafeComparatorLibHarness { uint32 _a, uint32 _b, uint32 _timestamp - ) external view returns (bool) { + ) external pure returns (bool) { return _a.lte(_b, _timestamp); } diff --git a/test/Ticket.test.ts b/test/Ticket.test.ts index e7a3f0b5..f68b10f8 100644 --- a/test/Ticket.test.ts +++ b/test/Ticket.test.ts @@ -58,7 +58,7 @@ async function printTwabs( debugLog(`Twab ${index} { amount: ${twab.amount}, timestamp: ${twab.timestamp}}`); }); - return twabs + return twabs; } describe('Ticket', () => { @@ -84,10 +84,10 @@ describe('Ticket', () => { ); // delegate for each of the users - await ticket.delegate(wallet1.address) - await ticket.connect(wallet2).delegate(wallet2.address) - await ticket.connect(wallet3).delegate(wallet3.address) - await ticket.connect(wallet4).delegate(wallet4.address) + await ticket.delegate(wallet1.address); + await ticket.connect(wallet2).delegate(wallet2.address); + await ticket.connect(wallet3).delegate(wallet3.address); + await ticket.connect(wallet4).delegate(wallet4.address); }); describe('constructor()', () => { @@ -102,7 +102,7 @@ describe('Ticket', () => { expect(await ticket.name()).to.equal(ticketName); expect(await ticket.symbol()).to.equal(ticketSymbol); expect(await ticket.decimals()).to.equal(ticketDecimals); - expect(await ticket.controller()).to.equal(wallet1.address) + expect(await ticket.controller()).to.equal(wallet1.address); }); it('should fail if token decimal is not greater than 0', async () => { @@ -345,13 +345,13 @@ describe('Ticket', () => { const twabs = await printTwabs(ticket, wallet1, debug); const matchingTwabs = twabs.reduce((all: any, twab: any) => { - debug(`TWAB timestamp ${twab.timestamp}, timestamp: ${timestamp}`) - debug(twab) + debug(`TWAB timestamp ${twab.timestamp}, timestamp: ${timestamp}`); + debug(twab); if (twab.timestamp.toString() == timestamp.toString()) { - all.push(twab) + all.push(twab); } - return all - }, []) + return all; + }, []); expect(matchingTwabs.length).to.equal(1); expect(await ticket.totalSupply()).to.equal(mintAmount.mul(2)); @@ -865,9 +865,7 @@ describe('Ticket', () => { await ticket.mint(wallet1.address, toWei('100')); await ticket.delegate(wallet2.address); - await expect( - ticket.delegate(wallet2.address) - ).to.not.emit(ticket, 'Delegated') + await expect(ticket.delegate(wallet2.address)).to.not.emit(ticket, 'Delegated'); }); it('should allow the delegate to be reset by passing zero', async () => { @@ -942,54 +940,62 @@ describe('Ticket', () => { it('should allow somone to delegate with a signature', async () => { // @ts-ignore const { user, delegate, nonce, deadline, v, r, s } = await delegateSignature({ - ticket, userWallet: wallet1, delegate: wallet2.address - }) + ticket, + userWallet: wallet1, + delegate: wallet2.address, + }); - await ticket.connect(wallet3).delegateWithSignature(user, delegate, deadline, v, r, s) + await ticket.connect(wallet3).delegateWithSignature(user, delegate, deadline, v, r, s); - expect(await ticket.delegateOf(wallet1.address)).to.equal(wallet2.address) - }) - }) + expect(await ticket.delegateOf(wallet1.address)).to.equal(wallet2.address); + }); + }); describe('controllerDelegateFor', () => { it('should allow the controller to delegate for a user', async () => { - await ticket.controllerDelegateFor(wallet2.address, wallet3.address) + await ticket.controllerDelegateFor(wallet2.address, wallet3.address); - expect(await ticket.delegateOf(wallet2.address)).to.equal(wallet3.address) - }) + expect(await ticket.delegateOf(wallet2.address)).to.equal(wallet3.address); + }); it('should not allow anyone else to delegate', async () => { await expect( - ticket.connect(wallet2).controllerDelegateFor(wallet1.address, wallet3.address) - ).to.be.revertedWith('ControlledToken/only-controller') - - }) - }) + ticket.connect(wallet2).controllerDelegateFor(wallet1.address, wallet3.address), + ).to.be.revertedWith('ControlledToken/only-controller'); + }); + }); context('when the timestamp overflows', () => { - - let overflowMintTimestamp: number + let overflowMintTimestamp: number; beforeEach(async () => { - await ticket.mint(wallet1.address, toWei('100')) - const timestamp = (await ethers.provider.getBlock('latest')).timestamp - const timeUntilOverflow = (2**32 - timestamp) - await increaseTime(timeUntilOverflow) - await ticket.mint(wallet1.address, toWei('100')) - overflowMintTimestamp = (await ethers.provider.getBlock('latest')).timestamp - await increaseTime(100) - }) + await ticket.mint(wallet1.address, toWei('100')); + const timestamp = (await ethers.provider.getBlock('latest')).timestamp; + const timeUntilOverflow = 2 ** 32 - timestamp; + await increaseTime(timeUntilOverflow); + await ticket.mint(wallet1.address, toWei('100')); + overflowMintTimestamp = (await ethers.provider.getBlock('latest')).timestamp; + await increaseTime(100); + }); describe('getAverageBalanceBetween()', () => { it('should function across overflow boundary', async () => { - expect(await ticket.getAverageBalanceBetween(wallet1.address, overflowMintTimestamp-100, overflowMintTimestamp+100)).to.equal(toWei('150')) - }) - }) + expect( + await ticket.getAverageBalanceBetween( + wallet1.address, + overflowMintTimestamp - 100, + overflowMintTimestamp + 100, + ), + ).to.equal(toWei('150')); + }); + }); describe('getBalanceAt', () => { it('should function across overflow boundary', async () => { - expect(await ticket.getBalanceAt(wallet1.address, overflowMintTimestamp)).to.equal(toWei('200')) - }) - }) - }) + expect(await ticket.getBalanceAt(wallet1.address, overflowMintTimestamp)).to.equal( + toWei('200'), + ); + }); + }); + }); });