From a55aa4bfed0865dbd29642fdbc474b5694461a2e Mon Sep 17 00:00:00 2001 From: Noah Zinsmeister Date: Thu, 23 Jan 2020 12:04:36 -0500 Subject: [PATCH] block.number -> block.timestamp tweak timing tests make tests even better --- contracts/UniswapV2Exchange.sol | 22 ++++++++++----------- contracts/interfaces/IUniswapV2Exchange.sol | 2 +- test/UniswapV2Exchange.spec.ts | 13 +++++++----- test/shared/utilities.ts | 11 +++-------- 4 files changed, 23 insertions(+), 25 deletions(-) diff --git a/contracts/UniswapV2Exchange.sol b/contracts/UniswapV2Exchange.sol index 012af87..e6899c6 100644 --- a/contracts/UniswapV2Exchange.sol +++ b/contracts/UniswapV2Exchange.sol @@ -16,9 +16,9 @@ contract UniswapV2Exchange is IUniswapV2Exchange, UniswapV2ERC20 { address public token0; address public token1; - uint112 private reserve0; // single storage slot, (jointly) access via getReserves - uint112 private reserve1; // single storage slot, (jointly) access via getReserves - uint32 private blockNumberLast; // single storage slot, (jointly) access via getReserves + uint112 private reserve0; // single storage slot, (jointly) access via getReserves + uint112 private reserve1; // single storage slot, (jointly) access via getReserves + uint32 private blockTimestampLast; // single storage slot, (jointly) access via getReserves uint public price0CumulativeLast; uint public price1CumulativeLast; @@ -52,25 +52,25 @@ contract UniswapV2Exchange is IUniswapV2Exchange, UniswapV2ERC20 { token1 = _token1; } - function getReserves() external view returns (uint112 _reserve0, uint112 _reserve1, uint32 _blockNumberLast) { + function getReserves() external view returns (uint112 _reserve0, uint112 _reserve1, uint32 _blockTimestampLast) { _reserve0 = reserve0; _reserve1 = reserve1; - _blockNumberLast = blockNumberLast; + _blockTimestampLast = blockTimestampLast; } // update reserves and, on the first time this function is called per block, price accumulators function _update(uint balance0, uint balance1, uint112 _reserve0, uint112 _reserve1) private { require(balance0 <= uint112(-1) && balance1 <= uint112(-1), "UniswapV2: BALANCE_OVERFLOW"); - uint32 blockNumber = uint32(block.number % 2**32); - uint32 blocksElapsed = blockNumber - blockNumberLast; // overflow is desired - if (blocksElapsed > 0 && _reserve0 != 0 && _reserve1 != 0) { + uint32 blockTimestamp = uint32(block.timestamp % 2**32); + uint32 timeElapsed = blockTimestamp - blockTimestampLast; // overflow is desired + if (timeElapsed > 0 && _reserve0 != 0 && _reserve1 != 0) { // * never overflows, and + overflow is desired - price0CumulativeLast += uint(UQ112x112.encode(_reserve1).uqdiv(_reserve0)) * blocksElapsed; - price1CumulativeLast += uint(UQ112x112.encode(_reserve0).uqdiv(_reserve1)) * blocksElapsed; + price0CumulativeLast += uint(UQ112x112.encode(_reserve1).uqdiv(_reserve0)) * timeElapsed; + price1CumulativeLast += uint(UQ112x112.encode(_reserve0).uqdiv(_reserve1)) * timeElapsed; } reserve0 = uint112(balance0); reserve1 = uint112(balance1); - blockNumberLast = blockNumber; + blockTimestampLast = blockTimestamp; emit Sync(reserve0, reserve1); } diff --git a/contracts/interfaces/IUniswapV2Exchange.sol b/contracts/interfaces/IUniswapV2Exchange.sol index c3eff7a..771ec0d 100644 --- a/contracts/interfaces/IUniswapV2Exchange.sol +++ b/contracts/interfaces/IUniswapV2Exchange.sol @@ -10,7 +10,7 @@ interface IUniswapV2Exchange { function factory() external view returns (address); function token0() external view returns (address); function token1() external view returns (address); - function getReserves() external view returns (uint112 reserve0, uint112 reserve1, uint32 blockNumberLast); + function getReserves() external view returns (uint112 reserve0, uint112 reserve1, uint32 blockTimestampLast); function price0CumulativeLast() external view returns (uint); function price1CumulativeLast() external view returns (uint); diff --git a/test/UniswapV2Exchange.spec.ts b/test/UniswapV2Exchange.spec.ts index 4c85ca8..e0caa49 100644 --- a/test/UniswapV2Exchange.spec.ts +++ b/test/UniswapV2Exchange.spec.ts @@ -3,7 +3,7 @@ import { Contract } from 'ethers' import { solidity, MockProvider, createFixtureLoader } from 'ethereum-waffle' import { BigNumber, bigNumberify } from 'ethers/utils' -import { expandTo18Decimals, mineBlocks } from './shared/utilities' +import { expandTo18Decimals, mineBlock } from './shared/utilities' import { exchangeFixture } from './shared/fixtures' import { AddressZero } from 'ethers/constants' @@ -142,11 +142,13 @@ describe('UniswapV2Exchange', () => { await addLiquidity(token0Amount, token1Amount) // ensure that setting price{0,1}CumulativeLast for the first time doesn't affect our gas math + await mineBlock(provider, 1) await exchange.sync(overrides) const swapAmount = expandTo18Decimals(1) const expectedOutputAmount = bigNumberify('453305446940074565') await token0.transfer(exchange.address, swapAmount) + await mineBlock(provider, 1) const gasCost = await exchange.estimate.swap(token0.address, expectedOutputAmount, wallet.address, overrides) console.log(`Gas required for swap: ${gasCost}`) }) @@ -182,16 +184,17 @@ describe('UniswapV2Exchange', () => { const token1Amount = expandTo18Decimals(3) await addLiquidity(token0Amount, token1Amount) - const blockNumber = (await exchange.getReserves())[2] + const blockTimestamp = (await exchange.getReserves())[2] expect(await exchange.price0CumulativeLast()).to.eq(0) expect(await exchange.price1CumulativeLast()).to.eq(0) + await mineBlock(provider, 1) await exchange.sync(overrides) expect(await exchange.price0CumulativeLast()).to.eq(bigNumberify(2).pow(112)) expect(await exchange.price1CumulativeLast()).to.eq(bigNumberify(2).pow(112)) - expect((await exchange.getReserves())[2]).to.eq(blockNumber + 1) + expect((await exchange.getReserves())[2]).to.eq(blockTimestamp + 1) - await mineBlocks(provider, 8) + await mineBlock(provider, 9) await exchange.sync(overrides) expect(await exchange.price0CumulativeLast()).to.eq( bigNumberify(2) @@ -203,6 +206,6 @@ describe('UniswapV2Exchange', () => { .pow(112) .mul(10) ) - expect((await exchange.getReserves())[2]).to.eq(blockNumber + 10) + expect((await exchange.getReserves())[2]).to.eq(blockTimestamp + 10) }) }) diff --git a/test/shared/utilities.ts b/test/shared/utilities.ts index fea7526..6186696 100644 --- a/test/shared/utilities.ts +++ b/test/shared/utilities.ts @@ -79,10 +79,10 @@ export async function getApprovalDigest( ) } -async function mineBlock(provider: Web3Provider, timestamp?: number): Promise { - await new Promise((resolve, reject) => { +export async function mineBlock(provider: Web3Provider, seconds: number): Promise { + await new Promise(async (resolve, reject) => { ;(provider._web3Provider.sendAsync as any)( - { jsonrpc: '2.0', method: 'evm_mine', params: timestamp ? [timestamp] : [] }, + { jsonrpc: '2.0', method: 'evm_mine', params: [(await provider.getBlock('latest')).timestamp + seconds] }, (error: any, result: any): void => { if (error) { reject(error) @@ -93,8 +93,3 @@ async function mineBlock(provider: Web3Provider, timestamp?: number): Promise { - await Promise.all([...Array(numberOfBlocks - 1)].map(() => mineBlock(provider))) - await mineBlock(provider, timestamp) -}