Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Part 1 of 3] v4-core dependency + gas snapshot + ci runs tweak (s52) #74

Merged
merged 5 commits into from
Jan 27, 2025

Conversation

ChefMist
Copy link
Collaborator

@ChefMist ChefMist commented Jan 26, 2025

As https://github.com/pancakeswap/pancake-v4-periphery/pull/66 is getting messy, this 3 part PR series aim to split the PR into a few chunks thats easy to review

This part will

  • update v4-core library and s52: migrate from gas snapshot to native snapshot
  • lower run counts for local (so locally forge test --isolate will not take too long)
  • lower run count for CI (so we will not timeout - created https://github.com/pancakeswap/pancake-v4-periphery/issues/73 to track)
  • set stable for foundry version

Comment on lines -29 to -30
call_override = false # Override calls
fail_on_revert = false # Fail the test if the contract reverts
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this 2 is already false from foundry https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md

@@ -16,23 +16,17 @@ evm_version = 'cancun'
bytecode_hash = "none"

[fuzz]
runs = 1000 # change this for higher number of fuzz/invariant locally
runs = 5 # change this for higher number of fuzz runs locally
Copy link
Collaborator Author

@ChefMist ChefMist Jan 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set lower fuzz/invariant runs locally, so dev local forge test is faster by default. they can always override and set higher if required

@@ -23,7 +23,7 @@ jobs:
- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: nightly-471e4ac317858b3419faaee58ade30c0671021e0 # Nightly (2024-10-03)
version: stable
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use stable as local foundry is stable too


[profile.ci_main.fuzz]
runs = 100000
runs = 1000
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lower for now - otherwise ci will fail (timeout) - https://github.com/pancakeswap/pancake-v4-periphery/issues/73

we can visit that issue again if we have time and see whats a suitable number

@ChefMist ChefMist changed the title [Part 1 of x][WIP] feat [Part 1 of x][WIP] v4-core dependency + gas snapshot + ci runs tweak Jan 26, 2025
@ChefMist ChefMist changed the title [Part 1 of x][WIP] v4-core dependency + gas snapshot + ci runs tweak [Part 1 of 3] v4-core dependency + gas snapshot + ci runs tweak Jan 26, 2025
@ChefMist ChefMist changed the title [Part 1 of 3] v4-core dependency + gas snapshot + ci runs tweak [Part 1 of 3] v4-core dependency + gas snapshot + ci runs tweak (s52) Jan 26, 2025
chefburger
chefburger previously approved these changes Jan 27, 2025
ChefSnoopy
ChefSnoopy previously approved these changes Jan 27, 2025
* feat: increase optimizer runs

* [Part 3 of 3] Code factoring (s43, s48, s50) (#76)

* s43: interface doc update

* s48: use directive with global

* s50: use internal keyword to make visibility obvious
@ChefMist ChefMist dismissed stale reviews from ChefSnoopy and chefburger via 69ce492 January 27, 2025 04:53
@ChefMist ChefMist merged commit d7820fb into main Jan 27, 2025
2 checks passed
@ChefMist ChefMist deleted the feat/part-1-gas-snapshot branch January 27, 2025 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants