From ea5e63c427bb54510445c3e7bf9cf241da709d82 Mon Sep 17 00:00:00 2001 From: Noah Zinsmeister Date: Wed, 22 Jan 2020 12:16:55 -0500 Subject: [PATCH] improve code quality of tests --- test/UniswapV2ERC20.spec.ts | 5 ++-- test/UniswapV2Exchange.spec.ts | 44 ++++++++++++++++------------------ test/UniswapV2Factory.spec.ts | 15 ++++++------ test/shared/fixtures.ts | 13 +++++----- test/shared/utilities.ts | 40 +++++++++++++------------------ 5 files changed, 53 insertions(+), 64 deletions(-) diff --git a/test/UniswapV2ERC20.spec.ts b/test/UniswapV2ERC20.spec.ts index a88381f..f625e68 100644 --- a/test/UniswapV2ERC20.spec.ts +++ b/test/UniswapV2ERC20.spec.ts @@ -1,8 +1,8 @@ -import chai from 'chai' -import { solidity, MockProvider, deployContract } from 'ethereum-waffle' +import chai, { expect } from 'chai' import { Contract } from 'ethers' import { MaxUint256 } from 'ethers/constants' import { bigNumberify, hexlify, keccak256, defaultAbiCoder, toUtf8Bytes } from 'ethers/utils' +import { solidity, MockProvider, deployContract } from 'ethereum-waffle' import { ecsign } from 'ethereumjs-util' import { expandTo18Decimals, getApprovalDigest } from './shared/utilities' @@ -10,7 +10,6 @@ import { expandTo18Decimals, getApprovalDigest } from './shared/utilities' import GenericERC20 from '../build/GenericERC20.json' chai.use(solidity) -const { expect } = chai const TOTAL_SUPPLY = expandTo18Decimals(10000) const TEST_AMOUNT = expandTo18Decimals(10) diff --git a/test/UniswapV2Exchange.spec.ts b/test/UniswapV2Exchange.spec.ts index 1a2bc97..4c85ca8 100644 --- a/test/UniswapV2Exchange.spec.ts +++ b/test/UniswapV2Exchange.spec.ts @@ -1,14 +1,13 @@ -import chai from 'chai' -import { solidity, MockProvider, createFixtureLoader } from 'ethereum-waffle' +import chai, { expect } from 'chai' import { Contract } from 'ethers' +import { solidity, MockProvider, createFixtureLoader } from 'ethereum-waffle' import { BigNumber, bigNumberify } from 'ethers/utils' import { expandTo18Decimals, mineBlocks } from './shared/utilities' -import { exchangeFixture, ExchangeFixture } from './shared/fixtures' +import { exchangeFixture } from './shared/fixtures' import { AddressZero } from 'ethers/constants' chai.use(solidity) -const { expect } = chai const overrides = { gasLimit: 1000000 @@ -27,12 +26,10 @@ describe('UniswapV2Exchange', () => { let token1: Contract let exchange: Contract beforeEach(async () => { - const { token0: _token0, token1: _token1, exchange: _exchange }: ExchangeFixture = await loadFixture( - exchangeFixture as any - ) - token0 = _token0 - token1 = _token1 - exchange = _exchange + const fixture = await loadFixture(exchangeFixture) + token0 = fixture.token0 + token1 = fixture.token1 + exchange = fixture.exchange }) it('mint', async () => { @@ -42,7 +39,7 @@ describe('UniswapV2Exchange', () => { await token1.transfer(exchange.address, token1Amount) const expectedLiquidity = expandTo18Decimals(2) - await expect(exchange.connect(wallet).mint(wallet.address, overrides)) + await expect(exchange.mint(wallet.address, overrides)) .to.emit(exchange, 'Transfer') .withArgs(AddressZero, wallet.address, expectedLiquidity) .to.emit(exchange, 'Sync') @@ -62,7 +59,7 @@ describe('UniswapV2Exchange', () => { async function addLiquidity(token0Amount: BigNumber, token1Amount: BigNumber) { await token0.transfer(exchange.address, token0Amount) await token1.transfer(exchange.address, token1Amount) - await exchange.connect(wallet).mint(wallet.address, overrides) + await exchange.mint(wallet.address, overrides) } it('getInputPrice', async () => { @@ -81,12 +78,11 @@ describe('UniswapV2Exchange', () => { for (let testCase of testCases) { await addLiquidity(testCase[1], testCase[2]) await token0.transfer(exchange.address, testCase[0]) - await expect(exchange.connect(wallet).swap(token0.address, testCase[3].add(1), wallet.address, overrides)).to.be - .reverted // UniswapV2: K_VIOLATED - await exchange.connect(wallet).swap(token0.address, testCase[3], wallet.address, overrides) + await expect(exchange.swap(token0.address, testCase[3].add(1), wallet.address, overrides)).to.be.reverted // UniswapV2: K_VIOLATED + await exchange.swap(token0.address, testCase[3], wallet.address, overrides) const totalSupply = await exchange.totalSupply() - await exchange.connect(wallet).transfer(exchange.address, totalSupply) - await exchange.connect(wallet).burn(wallet.address, overrides) + await exchange.transfer(exchange.address, totalSupply) + await exchange.burn(wallet.address, overrides) } }) @@ -98,7 +94,7 @@ describe('UniswapV2Exchange', () => { const swapAmount = expandTo18Decimals(1) const expectedOutputAmount = bigNumberify('1662497915624478906') await token0.transfer(exchange.address, swapAmount) - await expect(exchange.connect(wallet).swap(token0.address, expectedOutputAmount, wallet.address, overrides)) + await expect(exchange.swap(token0.address, expectedOutputAmount, wallet.address, overrides)) .to.emit(exchange, 'Sync') .withArgs(token0Amount.add(swapAmount), token1Amount.sub(expectedOutputAmount)) .to.emit(exchange, 'Swap') @@ -123,7 +119,7 @@ describe('UniswapV2Exchange', () => { const swapAmount = expandTo18Decimals(1) const expectedOutputAmount = bigNumberify('453305446940074565') await token1.transfer(exchange.address, swapAmount) - await expect(exchange.connect(wallet).swap(token1.address, expectedOutputAmount, wallet.address, overrides)) + await expect(exchange.swap(token1.address, expectedOutputAmount, wallet.address, overrides)) .to.emit(exchange, 'Sync') .withArgs(token0Amount.sub(expectedOutputAmount), token1Amount.add(swapAmount)) .to.emit(exchange, 'Swap') @@ -146,7 +142,7 @@ describe('UniswapV2Exchange', () => { await addLiquidity(token0Amount, token1Amount) // ensure that setting price{0,1}CumulativeLast for the first time doesn't affect our gas math - await exchange.connect(wallet).sync(overrides) + await exchange.sync(overrides) const swapAmount = expandTo18Decimals(1) const expectedOutputAmount = bigNumberify('453305446940074565') @@ -161,9 +157,9 @@ describe('UniswapV2Exchange', () => { await addLiquidity(token0Amount, token1Amount) const expectedLiquidity = expandTo18Decimals(3) - await exchange.connect(wallet).transfer(exchange.address, expectedLiquidity) + await exchange.transfer(exchange.address, expectedLiquidity) // this test is bugged, it catches the token{0,1} transfers before the lp transfers - await expect(exchange.connect(wallet).burn(wallet.address, overrides)) + await expect(exchange.burn(wallet.address, overrides)) // .to.emit(exchange, 'Transfer') // .withArgs(exchange.address, AddressZero, expectedLiquidity) .to.emit(exchange, 'Burn') @@ -190,13 +186,13 @@ describe('UniswapV2Exchange', () => { expect(await exchange.price0CumulativeLast()).to.eq(0) expect(await exchange.price1CumulativeLast()).to.eq(0) - await exchange.connect(wallet).sync(overrides) + 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) await mineBlocks(provider, 8) - await exchange.connect(wallet).sync(overrides) + await exchange.sync(overrides) expect(await exchange.price0CumulativeLast()).to.eq( bigNumberify(2) .pow(112) diff --git a/test/UniswapV2Factory.spec.ts b/test/UniswapV2Factory.spec.ts index de53c36..25e7670 100644 --- a/test/UniswapV2Factory.spec.ts +++ b/test/UniswapV2Factory.spec.ts @@ -1,16 +1,15 @@ -import chai from 'chai' -import { solidity, MockProvider, createFixtureLoader } from 'ethereum-waffle' +import chai, { expect } from 'chai' import { Contract } from 'ethers' +import { AddressZero } from 'ethers/constants' import { bigNumberify } from 'ethers/utils' +import { solidity, MockProvider, createFixtureLoader } from 'ethereum-waffle' import { getCreate2Address } from './shared/utilities' -import { factoryFixture, FactoryFixture } from './shared/fixtures' +import { factoryFixture } from './shared/fixtures' import UniswapV2Exchange from '../build/UniswapV2Exchange.json' -import { AddressZero } from 'ethers/constants' chai.use(solidity) -const { expect } = chai const TEST_ADDRESSES = { token0: '0x1000000000000000000000000000000000000000', @@ -24,12 +23,12 @@ describe('UniswapV2Factory', () => { gasLimit: 9999999 }) const [wallet, other] = provider.getWallets() - const loadFixture = createFixtureLoader(provider, [wallet]) + const loadFixture = createFixtureLoader(provider, [wallet, other]) let factory: Contract beforeEach(async () => { - const { factory: _factory }: FactoryFixture = await loadFixture(factoryFixture as any) - factory = _factory + const fixture = await loadFixture(factoryFixture) + factory = fixture.factory }) it('feeToSetter, feeTo, exchangesCount', async () => { diff --git a/test/shared/fixtures.ts b/test/shared/fixtures.ts index 3af3710..6b65ba6 100644 --- a/test/shared/fixtures.ts +++ b/test/shared/fixtures.ts @@ -1,4 +1,5 @@ -import { providers, Wallet, Contract } from 'ethers' +import { Contract, Wallet } from 'ethers' +import { Web3Provider } from 'ethers/providers' import { deployContract } from 'ethereum-waffle' import { expandTo18Decimals } from './utilities' @@ -7,22 +8,22 @@ import GenericERC20 from '../../build/GenericERC20.json' import UniswapV2Factory from '../../build/UniswapV2Factory.json' import UniswapV2Exchange from '../../build/UniswapV2Exchange.json' -export interface FactoryFixture { +interface FactoryFixture { factory: Contract } -export async function factoryFixture(_: providers.Web3Provider, [wallet]: Wallet[]): Promise { +export async function factoryFixture(_: Web3Provider, [wallet]: Wallet[]): Promise { const factory = await deployContract(wallet, UniswapV2Factory, [wallet.address]) return { factory } } -export interface ExchangeFixture extends FactoryFixture { +interface ExchangeFixture extends FactoryFixture { token0: Contract token1: Contract exchange: Contract } -export async function exchangeFixture(provider: providers.Web3Provider, [wallet]: Wallet[]): Promise { +export async function exchangeFixture(provider: Web3Provider, [wallet]: Wallet[]): Promise { const { factory } = await factoryFixture(provider, [wallet]) const tokenA = await deployContract(wallet, GenericERC20, [expandTo18Decimals(10000)]) @@ -30,7 +31,7 @@ export async function exchangeFixture(provider: providers.Web3Provider, [wallet] await factory.createExchange(tokenA.address, tokenB.address) const exchangeAddress = await factory.getExchange(tokenA.address, tokenB.address) - const exchange = new Contract(exchangeAddress, JSON.stringify(UniswapV2Exchange.abi), provider) + const exchange = new Contract(exchangeAddress, JSON.stringify(UniswapV2Exchange.abi), provider).connect(wallet) const token0Address = (await exchange.token0()).address const token0 = tokenA.address === token0Address ? tokenA : tokenB diff --git a/test/shared/utilities.ts b/test/shared/utilities.ts index 1a4750a..fea7526 100644 --- a/test/shared/utilities.ts +++ b/test/shared/utilities.ts @@ -1,4 +1,5 @@ -import { providers, Contract } from 'ethers' +import { Contract } from 'ethers' +import { Web3Provider } from 'ethers/providers' import { BigNumber, bigNumberify, @@ -9,16 +10,15 @@ import { solidityPack } from 'ethers/utils' -export function expandTo18Decimals(n: number): BigNumber { - return bigNumberify(n).mul(bigNumberify(10).pow(18)) -} - const PERMIT_TYPEHASH = keccak256( toUtf8Bytes('Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)') ) -const GET_DOMAIN_SEPARATOR = async (token: Contract) => { - const name = await token.name() +export function expandTo18Decimals(n: number): BigNumber { + return bigNumberify(n).mul(bigNumberify(10).pow(18)) +} + +function getDomainSeparator(name: string, tokenAddress: string) { return keccak256( defaultAbiCoder.encode( ['bytes32', 'bytes32', 'bytes32', 'uint256', 'address'], @@ -27,7 +27,7 @@ const GET_DOMAIN_SEPARATOR = async (token: Contract) => { keccak256(toUtf8Bytes(name)), keccak256(toUtf8Bytes('1')), 1, - token.address + tokenAddress ] ) ) @@ -45,24 +45,22 @@ export function getCreate2Address( keccak256(solidityPack(['address', 'address'], [token0Address, token1Address])), keccak256(bytecode) ] - const sanitizedInputs = `0x${create2Inputs.map(i => i.slice(2)).join('')}` - return getAddress(`0x${keccak256(sanitizedInputs).slice(-40)}`) } -interface Approve { - owner: string - spender: string - value: BigNumber -} export async function getApprovalDigest( token: Contract, - approve: Approve, + approve: { + owner: string + spender: string + value: BigNumber + }, nonce: BigNumber, expiration: BigNumber ): Promise { - const DOMAIN_SEPARATOR = await GET_DOMAIN_SEPARATOR(token) + const name = await token.name() + const DOMAIN_SEPARATOR = getDomainSeparator(name, token.address) return keccak256( solidityPack( ['bytes1', 'bytes1', 'bytes32', 'bytes32'], @@ -81,7 +79,7 @@ export async function getApprovalDigest( ) } -async function mineBlock(provider: providers.Web3Provider, timestamp?: number): Promise { +async function mineBlock(provider: Web3Provider, timestamp?: number): Promise { await new Promise((resolve, reject) => { ;(provider._web3Provider.sendAsync as any)( { jsonrpc: '2.0', method: 'evm_mine', params: timestamp ? [timestamp] : [] }, @@ -96,11 +94,7 @@ async function mineBlock(provider: providers.Web3Provider, timestamp?: number): }) } -export async function mineBlocks( - provider: providers.Web3Provider, - numberOfBlocks: number, - timestamp?: number -): Promise { +export async function mineBlocks(provider: Web3Provider, numberOfBlocks: number, timestamp?: number): Promise { await Promise.all([...Array(numberOfBlocks - 1)].map(() => mineBlock(provider))) await mineBlock(provider, timestamp) }