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

Use network specific config for tests with permit2 #772

Open
pakim249CAL opened this issue Apr 17, 2023 · 13 comments
Open

Use network specific config for tests with permit2 #772

pakim249CAL opened this issue Apr 17, 2023 · 13 comments
Labels

Comments

@pakim249CAL
Copy link
Contributor

No description provided.

@Rubilmax Rubilmax self-assigned this Apr 18, 2023
@Rubilmax
Copy link
Contributor

Rubilmax commented Apr 18, 2023

There's currently no Permit2 deployed on avalanche. What shall we do?
Besides, it seems the address is identical on all EVMs on which Uniswap is deployed

@Rubilmax Rubilmax removed their assignment Apr 18, 2023
@pakim249CAL
Copy link
Contributor Author

pakim249CAL commented Apr 18, 2023

We should deploy it on avalanche! XD

Not sure what better option there is actually. I guess we can just skip any test files that use permit 2 in tests that aren't on eth for now. I think we can tackle this problem when we get closer to deploying on avalanche.

@MerlinEgalite
Copy link
Contributor

Yes let's skip tests on avalanche for now

@MerlinEgalite
Copy link
Contributor

Should we keep this one open?

@Rubilmax
Copy link
Contributor

Yes, it is still an active issue:

  • either we get rid of Avalanche config (because we don't & can't run tests on Avalanche, there's no Permit2 there)
  • or we find a solution to run tests on Avalanche

@pakim249CAL
Copy link
Contributor Author

I think it's important to keep the avalanche config, as that is currently the only network where rewards can be tested on v3. Maybe we can use a setCode cheatcode. I'll look into it.

@pakim249CAL
Copy link
Contributor Author

After trying some ideas, I don't think there's a neat way to test Permit2 on avalanche. Permit2 requires --via-ir to compile, and therefore it is not possible to obtain a permit2 bytecode artifact without impacting the time it takes for the testing suite to compile. I thought about just using a precompiled artifact, but this is also problematic because the chain id and address of the contract is used in the constructor of EIP712, and so a precompiled artifact cannot be adjusted for this constructor. Deploying our own permit2 to any other address would conflict with the fact that Permit2Lib hardcodes the address.

So in my view, we have these options:

  1. Do nothing and just don't do permit2 tests on avalanche, or any network that doesn't have Permit2 deployed. We should maybe just ask the Uniswap team to deploy it there.
  2. Change our testing suite to compile --via-ir
  3. Make a special pre-compiled Permit2 artifact and use that exclusively for avalanche tests

Overall, I think solutions 2 & 3 are just not very practical.

@MerlinEgalite
Copy link
Contributor

I'm for solution 1.

@Rubilmax
Copy link
Contributor

Then how do we skip Permit2 tests on Avalanche now?

@MerlinEgalite
Copy link
Contributor

Then how do we skip Permit2 tests on Avalanche now?

I think you can just add command that check if the network is avalanche you skip compilation of Permit2 in it? It's not possible?

@Rubilmax
Copy link
Contributor

I don't think the issue is the Permit2 compilation, but rather the tests in which Permit2 is involved
And I don't know how to skip specific tests programmatically

@MerlinEgalite
Copy link
Contributor

Just by using forge test --no-match-test regex I think.

Doc

@Rubilmax
Copy link
Contributor

I thought we didn't know the network until runtime, hence why I was looking for a programmatic way of skipping tests
But indeed, we know the network before hand so we could just skip tests from the command line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants