Feature/#1 rewrite v2 tests using forge#23
Open
duelinggalois wants to merge 13 commits intomasterfrom
Open
Conversation
Added foundry to repo to use forge for tests. Modified the default source and build paths to match the current file structure. Forge uses a git submodule to manage `lib/forge-std` which contains the test dependancies closes #6
There were a number of issues with inheritance that changed from version 0.5.16 to 0.8.13 that needed to be addressed. For the most part the use of override on public state variables that where overriding interface functions. Some redundant functions in the pair interface were removed as they existed in the ERC20 interface. One stack to deep error was addressed by adding a struct to hold the parameters passed to the function. closes #8
Using Factory is non deterministic in ordering of tokens so I changed the nonce of the test contract in order to get the token0 and token1 addresses in the expected order to better match previous tests.
Added // SPDX-License-Identifier: UNLICENSED on all uniswap v2 source files for solc compiler 0.8.13 to be able to compile the uniswap contract files Removed separate mint test file feature-3-mint.t.sol
Finished writing remaining pair tests. Did not delete current typescript
tests. Gas tests for swap did not come anywhere close to previous test
numbers. I did look at some of the gas reports available with foundry
and am assuming the difference in forge tests, the gas is measured as
additional gas added by swap transaction, but not what a swap would cost
as a standalone transaction. Clearly I need to learn more about gas.
Gas changes are described in test:
```
assertEq(gasStart - gasEnd, 20311, "<- yarn to forge <- 74721 <- update to 0.8.13 <- 73462");
```
Gas report seems to match assertion.
```
$ forge test --gas-report
...
╭────────────────────────┬─────────────────┬────────┬────────┬────────┬─────────╮
│ UniswapV2Pair contract ┆ ┆ ┆ ┆ ┆ │
╞════════════════════════╪═════════════════╪════════╪════════╪════════╪═════════╡
│ Deployment Cost ┆ Deployment Size ┆ ┆ ┆ ┆ │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 1849143 ┆ 9167 ┆ ┆ ┆ ┆ │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name ┆ min ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ balanceOf ┆ 581 ┆ 581 ┆ 581 ┆ 581 ┆ 2 │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ burn ┆ 20445 ┆ 37467 ┆ 20445 ┆ 71513 ┆ 3 │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ getReserves ┆ 526 ┆ 526 ┆ 526 ┆ 526 ┆ 7 │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ initialize ┆ 44900 ┆ 44900 ┆ 44900 ┆ 44900 ┆ 19 │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ mint ┆ 133174 ┆ 134710 ┆ 133666 ┆ 151297 ┆ 19 │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ price0CumulativeLast ┆ 384 ┆ 384 ┆ 384 ┆ 384 ┆ 3 │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ price1CumulativeLast ┆ 406 ┆ 406 ┆ 406 ┆ 406 ┆ 3 │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ swap ┆ 10489 ┆ 13440 ┆ 15241 ┆ 16892 ┆ 28 │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ sync ┆ 6264 ┆ 21436 ┆ 7576 ┆ 50469 ┆ 3 │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ totalSupply ┆ 429 ┆ 429 ┆ 429 ┆ 429 ┆ 4 │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ transfer ┆ 20260 ┆ 20260 ┆ 20260 ┆ 20260 ┆ 3 │
╰────────────────────────┴─────────────────┴────────┴────────┴────────┴─────────╯
```
. converted some functions in utilities.ts with ./shared/utilities.sol . converted all testing points in UniswapV2FactoryTest.spec.ts . clean up code . made a function createPair to be called by tests . wrapped initial fee test into function testInitialStates
Required adding a resolution to hold back a dependency of solc to an earlier version that supported node 10.
Understood UniswapERC20 specific permit feature having reentrancy attack protected by introducing extra hash calculation : DOMAIN_SEPARATOR and PERMIT_TYPEHASH Understood vm.chainId, the default chainId is 31337 and that can be reset before calling token deploy
for removing visibility warning in UniswapV2ERC20Test.sol need upgrade foundry to forge 0.2.0 (3fc1491 2022-06-06T00:12:33.657656525Z)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.