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

Deploy P256 verifier in setup of P256 and WebAuthn tests #34

Merged

Conversation

doublespending
Copy link
Contributor

Motivation

The uint tests in P256.t.sol and WebAuthn.sol will be fail because of miss deploying P256Verifier contract.
截屏2023-12-07 11 33 47

Solution

Deploy P256Verifier in the setup of P256 and WebAuthn tests.

@nalinbhardwaj
Copy link
Contributor

nalinbhardwaj commented Dec 7, 2023

Amazing, this can close #22. Would you also mind changing the CI commands to drop --fork-url "https://base-goerli.publicnode.com" here (we added it originally to handle this deployment): https://github.com/daimo-eth/p256-verifier/blob/master/.github/workflows/ci.yml#L30-L31. Then, you can also regenerate the .lcov file (as in the CI).

Thanks for the contribution

@doublespending
Copy link
Contributor Author

Amazing, this can close #22. Would you also mind changing the CI commands to drop --fork-url "https://base-goerli.publicnode.com" here (we added it originally to handle this deployment): https://github.com/daimo-eth/p256-verifier/blob/master/.github/workflows/ci.yml#L30-L31. Then, you can also regenerate the .lcov file (as in the CI).

Thanks for the contribution

Sure! Check it at 804cfdd and a3577ef.

Copy link
Contributor

@nalinbhardwaj nalinbhardwaj left a comment

Choose a reason for hiding this comment

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

looks great; thanks again!

@nalinbhardwaj nalinbhardwaj merged commit 29475ae into daimo-eth:master Dec 8, 2023
6 checks passed
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.

2 participants