Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Safe AnonAadhaar Plugin #223

Merged
merged 20 commits into from
Jul 5, 2024
Merged

Safe AnonAadhaar Plugin #223

merged 20 commits into from
Jul 5, 2024

Conversation

porco-rosso-j
Copy link
Contributor

@porco-rosso-j porco-rosso-j commented Mar 27, 2024

What is this PR doing?

Let me apologize first, as this may not be an appropriate PR. I created this PR not to expect to make any change to the current codebase but, by exhibiting the examples, to hear the WAX team's impression/opinions on adding AnonAadhaar, a zk solution developed by another PSE team, to the current Wax stack as a showcase of an additional "signature validation" scheme.

I've added two files in packages/plugins/src/safe that are SafeAnonAadhaarPlugin.sol and SafeAnonAadhaarFactory.sol. This plugin allows the owner of the Aadhaar card, a biometric identity card issued by the Indian government, to control 4337 wallets via the Safe plugin. Its recovery example can also be added for sure.

The functionality of this validation scheme has already been proven as I've built a 4337 wallet PoC with AnonAadhaar called Banganoir. Demo is here. Btw, I'm not from AnonAadhaar but an external contributor.

Let me know if you have any questions.

How can these changes be manually tested?

No test in this PR. But if Wax team finds this integration valuable, I can work on adding tests and other necessary improvements.

Checklist

  • I have manually tested these changes
  • Post a link to the PR in the group chat

Guidelines

  • If your PR is not ready, mark it as a draft
  • The resolve conversation button is for reviewers, not authors
    • (But add a 'done' comment or similar)

Copy link
Contributor

@jacque006 jacque006 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, this is very cool ❤️

The main addition I would want to see is an integration test (happy path) showing this working. This would also be helpful for developers who want to use the plugin. We usually do this as a Hardhat test (see existing tests), but Foundry should work as well if that is your preference. Feel free to ping us on Discord if you need help with that.

Some other things worth mentioning:

  • @JohnGuilding is updating our 4337 version to 0.7.0 . That may go in before this, but I don't think it will have too much impact.
  • John has also been looking into more natively integrating with Safe in a variety of ways, which may make it possible to instead write this plugin as just a ERC-1271 contract that is a Safe owner since you are only validating ops and not recovering the account. This would also make it more compatible with other non-Safe components in the AA/4337 ecosystem.

@porco-rosso-j
Copy link
Contributor Author

#40eb6b9: Working on SafeAnonAadhaarPlugin test. Currently, it still fails due to an on-chain verification error caused by a bug in @anon-aadhaar/core. This will be resolved after this PR is merged and a fixed version is released.

@porco-rosso-j
Copy link
Contributor Author

Test completed & natspec added. Maybe ready to be merged once here if it looks fine and makes sense.

future work:

Note: it seems like some commits made unnecessary changes to files like .gitmodules and userOpUtils.ts. Please ignore them when merging.

@jacque006
Copy link
Contributor

jacque006 commented Jun 17, 2024

Hey @porco-rosso-j , thanks for updating this contribution/PR, overall looks good 👍 Apologies for the extensive delay in reviewing again.

Before I can merge, need two things resolved:
image

  • Looks like the ZK verifier may need to be updated, or there is some other issue.
  • Resolve packages/plugins/yarn.lock conflict.

If you need help with either of these things, let me know I can PR into your fork branch.

porco-rosso-j and others added 4 commits July 2, 2024 20:52
Rename AnonAadhaar verifier contract so it does not conflict with ZKEmail verfier in Typechain.
Add AnonAadhaar to remappings.
Add recommended dependency compiler to hardhat from AnonAadhaar docs.
jacque006 and others added 5 commits July 3, 2024 18:57
Remove yarn install for forge CI.
Update most core deps to latest versions.
Copy needed @anon-aadhaar/contracts to workaround issues with typechain generation.
Minor updates to some e2e tests with deployments & timing.
Use correct verfier for AnonAadhaar
@jacque006 jacque006 merged commit 1cc5e79 into getwax:main Jul 5, 2024
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants