Migrate TickBitmap tests from HardHat to Foundry #320

gitcoindev · 2023-07-26T10:29:36Z

Related Issue

This pull request migrates TickBitmap tests from HardHat to Foundry (35 test cases), a part of #315 .

Description of changes

migrated 35 TickBitmap tests to Foundry
removed HardHat implementation after migration

Running 35 tests for test/foundry-tests/TickBitmap.t.sol:TickBitmapTestTest
[PASS] test_flipTick_flipsOnlyTheSpecifiedTick() (gas: 37394)
[PASS] test_flipTick_gasCostOfFlippingATickThatResultsInDeletingAWord() (gas: 19391)
[PASS] test_flipTick_gasCostOfFlippingFirstTickInWordToInitialized() (gas: 28230)
[PASS] test_flipTick_gasCostOfFlippingSecondTickInWordToInitialized() (gas: 29492)
[PASS] test_flipTick_revertsOnlyItself() (gas: 83555)
[PASS] test_isInitialized_isFalseAtFirst() (gas: 8323)
[PASS] test_isInitialized_isFlippedBackByFlapTick() (gas: 20466)
[PASS] test_isInitialized_isFlippedByFlapTick() (gas: 29793)
[PASS] test_isInitialized_isNotChangedByAnotherFlipToADifferentTick() (gas: 29549)
[PASS] test_isInitialized_isNotChangedByAnotherFlipToADifferentTickOnAnotherWord() (gas: 33421)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_doesNotExceedBoundary() (gas: 65229)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_gasCostForEntireWord() (gas: 65074)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_gasCostJustBelowBoundary() (gas: 63073)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_gasCostOnBoundary() (gas: 65074)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_returnsTheNextInitializedTickFromTheNextWord() (gas: 86853)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_returnsTheNextWordsInitializedTickIfOnTheRightBoundary() (gas: 65242)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_returnsTheNextWordsInitializedTickIfOnTheRightBoundary2() (gas: 63663)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_returnsTheTickDirectlyToTheRight() (gas: 63610)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_returnsTheTickDirectlyToTheRight2() (gas: 63700)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_returnsTickToRightIfAtInitializedTick() (gas: 63641)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_returnsTickToRightIfAtInitializedTick2() (gas: 63681)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_skipsEntireWord() (gas: 65240)
[PASS] test_nextInitializedTickWithinOneWord_lteFalse_skipsHalfWord() (gas: 65208)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_atTheWordBoundary() (gas: 65226)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_boundaryIsInitialized() (gas: 86703)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_entireEmptyWord() (gas: 65203)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_gasCostForEntireWord() (gas: 65081)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_gasCostJustBelowBoundary() (gas: 63379)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_gasCostOnBoundary() (gas: 65070)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_halfwayThroughEmptyWord() (gas: 65214)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_returnsSameTickIfInitialized() (gas: 63512)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_returnsTickDirectlyToTheLeftOfInputTickIfNotInitialized() (gas: 63503)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_willNotExceedTheWordBoundary() (gas: 65216)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_wordBoundary() (gas: 65256)
[PASS] test_nextInitializedTickWithinOneWord_lteTrue_wordBoundaryLess1nextInitializedTickInNextWord() (gas: 63506)
Test result: ok. 35 passed; 0 failed; finished in 7.48ms

nagrarohit

function test_isInitialized_isFlippedByFlapTick()**
function test_isInitialized_isFlippedBackByFlapTick()**
I think the name should be Flip instead of Flap.

gitcoindev · 2023-07-28T19:57:54Z

function test_isInitialized_isFlippedByFlapTick()**

function test_isInitialized_isFlippedBackByFlapTick()**

I think the name should be Flip instead of Flap.

good catch!

gitcoindev · 2023-07-30T19:15:28Z

@ewilz I fixed typos and refactored tests to use forge-gas-snapshot, PR is ready for the review.

gitcoindev · 2023-07-31T14:57:27Z

The PR needs the same changes as #317 , in progress...

gitcoindev · 2023-07-31T15:43:14Z

Hi @ewilz this PR is ready for another review round

Ref: Uniswap#315

False) to Foundry. Ref: Uniswap#315

… tuple in one-liner. Ref: Uniswap#315

True) to Foundry. Ref: Uniswap#315

Ref: Uniswap#315

… test_isInitialized_isFlippedBackByFlipTick.

The test contract was a middle layer between library and was used by javascript / HardHat tests. Due to the middle layer using Foundry, gas cost usage was calculated incorrectly. Generate correct gas snapshots using direct library calls. Delete unnecessary comments. Ref: Uniswap#315

Ref: Uniswap#315

nagrarohit

LGTM. But i missed the following comments :-
// #isInitialized
// #flipTick
// #nextInitializedTickWithinOneWord

ewilz · 2023-08-07T19:34:47Z

hardhat snapshots need to be deleted

test/foundry-tests/TickBitmap.t.sol

ewilz · 2023-08-07T19:57:46Z

Can delete TickBitmapTest.sol and rename foundry test TickBitmapTest hence getting rid of the extra "Test" on there

test/foundry-tests/TickBitmap.t.sol

ewilz · 2023-08-07T20:09:00Z

test migration looks good, not sure if you've ever written forge fuzz tests, with the recently committed FullMath conversion for instance, we took the echidna fuzz tests and converted them to forge fuzz tests, need to convert checkNextInitializedTickWithinOneWordInvariants Happy to merge without that though and just do it later..

Follow structure of other forge tests.

gitcoindev · 2023-08-07T20:26:34Z

Hi @ewilz thank you for the review, I already pushed changes. I can have a look at fuzzing later this week after merging.

ewilz · 2023-08-07T23:14:28Z

woops sorry didn't think I had write access to your branch

gitcoindev · 2023-08-08T16:41:40Z

Yes I enabled write access, thanks for merging!

nagrarohit suggested changes Jul 28, 2023

View reviewed changes

gitcoindev force-pushed the feature/migrate_TickBitmap_toFoundry branch from b9fbe79 to 287ffd8 Compare July 30, 2023 19:14

gitcoindev force-pushed the feature/migrate_TickBitmap_toFoundry branch from 287ffd8 to 4426e47 Compare July 31, 2023 15:02

gitcoindev mentioned this pull request Jul 31, 2023

Migrate SqrtPriceMath tests from HardHat to Foundry #317

Merged

ewilz mentioned this pull request Jul 30, 2023

Migrate All tests to Foundry #315

Closed

6 tasks

gitcoindev added 10 commits July 31, 2023 23:48

foundry-test: Migrate TickBitmap#isInitialized to Foundry.

1d0f140

Ref: Uniswap#315

foundry-test: Migrate TickBitmap#flipTick to Foundry.

759a857

Ref: Uniswap#315

foundry-test: Migrate TickBitmap#nextInitializedTickWithinOneWord (lte =

573098c

False) to Foundry. Ref: Uniswap#315

foundry-test: TickBitmap#nextInitializedTickWithinOneWord init return…

d2aeae5

… tuple in one-liner. Ref: Uniswap#315

foundry-test: Migrate TickBitmap#nextInitializedTickWithinOneWord (lte =

0dbdbaa

True) to Foundry. Ref: Uniswap#315

test: Remove migrated TickBitmap from HardHat to Foundry.

3729588

Ref: Uniswap#315

foundry-test: Fix typos in test_isInitialized_isFlippedByFlipTick and…

2d6d691

… test_isInitialized_isFlippedBackByFlipTick.

foundry-test: TickBitmap: Use forge-gas-snapshot to snapshot gas used.

73e7eac

test: Update TickBitmap Foundry gas snapshots.

11beab3

Ref: Uniswap#315

gitcoindev force-pushed the feature/migrate_TickBitmap_toFoundry branch from 726eba1 to 11beab3 Compare July 31, 2023 21:48

gitcoindev requested a review from nagrarohit August 7, 2023 09:56

nagrarohit approved these changes Aug 7, 2023

View reviewed changes

ewilz reviewed Aug 7, 2023

View reviewed changes

test/foundry-tests/TickBitmap.t.sol Outdated Show resolved Hide resolved

ewilz reviewed Aug 7, 2023

View reviewed changes

test/foundry-tests/TickBitmap.t.sol Outdated Show resolved Hide resolved

gitcoindev added 2 commits August 7, 2023 22:05

TickBitmap: Remove superfluous console log import.

56fdbef

TickBitmap: Remove HardHat snapshots.

61eb89f

Merge branch 'main' into feature/migrate_TickBitmap_toFoundry

6fc5d89

gitcoindev added 3 commits August 7, 2023 22:18

foundry: Remove old TickBitmapTest contract.

b266208

TickBitmap: Rename TickBitmapTestTest to TickBitmapTest.

0a661fe

TickBitmap: Move setUpSomeTicks towards bottom of the test file.

70005a5

Follow structure of other forge tests.

gitcoindev requested a review from ewilz August 7, 2023 20:29

fuzz_test

3275eb0

ewilz added 6 commits August 7, 2023 19:16

smol cleanup

06d7efc

fix comment

b8f399d

Delete contracts/test/TickBitmapEchidnaTest.sol

99c46c2

flip tick in setup for accurate gas

f392389

better gas tests which require dirty slots

f7135ad

smol cleanup

6b8b8d6

ewilz approved these changes Aug 7, 2023

View reviewed changes

ewilz merged commit 3b72450 into Uniswap:main Aug 7, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate TickBitmap tests from HardHat to Foundry #320

Migrate TickBitmap tests from HardHat to Foundry #320

gitcoindev commented Jul 26, 2023

nagrarohit left a comment

gitcoindev commented Jul 28, 2023

gitcoindev commented Jul 30, 2023 •

edited

Loading

gitcoindev commented Jul 31, 2023

gitcoindev commented Jul 31, 2023

nagrarohit left a comment

ewilz commented Aug 7, 2023

ewilz commented Aug 7, 2023

ewilz commented Aug 7, 2023

gitcoindev commented Aug 7, 2023

ewilz commented Aug 7, 2023

gitcoindev commented Aug 8, 2023

Migrate TickBitmap tests from HardHat to Foundry #320

Migrate TickBitmap tests from HardHat to Foundry #320

Conversation

gitcoindev commented Jul 26, 2023

Related Issue

Description of changes

nagrarohit left a comment

Choose a reason for hiding this comment

gitcoindev commented Jul 28, 2023

gitcoindev commented Jul 30, 2023 • edited Loading

gitcoindev commented Jul 31, 2023

gitcoindev commented Jul 31, 2023

nagrarohit left a comment

Choose a reason for hiding this comment

ewilz commented Aug 7, 2023

ewilz commented Aug 7, 2023

ewilz commented Aug 7, 2023

gitcoindev commented Aug 7, 2023

ewilz commented Aug 7, 2023

gitcoindev commented Aug 8, 2023

gitcoindev commented Jul 30, 2023 •

edited

Loading