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

Project Idea: Semaphore + Account Abstraction #345

Open
aguzmant103 opened this issue Aug 15, 2023 · 24 comments
Open

Project Idea: Semaphore + Account Abstraction #345

aguzmant103 opened this issue Aug 15, 2023 · 24 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@aguzmant103
Copy link
Contributor

No description provided.

@aguzmant103 aguzmant103 added help wanted Extra attention is needed good first issue Good for newcomers labels Aug 15, 2023
@FoodChain1028
Copy link

Hi @aguzmant103, is there any detail about this project? I would love to build something with this.

@moven0831
Copy link

moven0831 commented Sep 19, 2023

Hi @FoodChain1028, I've got some ideas about applying Semaphore on AA's wallet recovery. Since a contract-based wallet owner can assign multiple addresses as wallet guardians. We can use Semaphore to register the guardians instead of directly listing them on the smart contract. This alternation can provide privacy if the guardian's address needs to be hidden under some scenarios.

@FoodChain1028
Copy link

Hi @FoodChain1028, I've got some ideas about applying Semaphore on AA's wallet recovery. Since a contract-based wallet owner can assign multiple addresses as wallet guardians. We can use Semaphore to register the guardians instead of directly listing them on the smart contract. This alternation can provide privacy if the guardian's address needs to be hidden under some scenarios.

Hi, @moven0831. This idea looks good, yet for now we have to interact the contract-based wallet with another Externally Owned Address; thus the relationship between this address and proof in revealed as a result. A method from me to fix this is using a relayer address for everyone wants to use this service, the user would send their fee (like a transferring fee and everyone need to send the same amount of money here) and proof to the relayer and relayer would do this for the users. To be more specifically, the relayer is more like a mixer box to:

  1. verify the identity in the Merkle Tree (As the owner of this AA contract).
  2. mix the address to make any proof - user EOA - AA contract unrelated, and thus would preserve the privacy.

@cedoor
Copy link
Member

cedoor commented Sep 19, 2023

Hey guys, this could also be useful -> https://github.com/saleel/aa-zk-test

Author: @saleel

@saleel
Copy link
Contributor

saleel commented Sep 19, 2023

Thanks for the tag @cedoor. It was an experiment I did a while back, and need to make some changes in that code to make it work correctly.
I am also in the process of doing a write-up on explaining the whole stuff. Planning to do both in the coming weekend.

@FoodChain1028
Copy link

Hi, @saleel, is there any chance or anything I can help you with?

@saleel
Copy link
Contributor

saleel commented Oct 9, 2023

I wrote about it here - https://saleel.xyz/blog/zk-account-abstraction/
Here is the repo with sample code - https://github.com/saleel/semaphore-wallet

@FoodChain1028 Above is a reference implementation. There are some minor changes needed in semaphore (mentioned in the blog towards the end) before we can implement a production level semaphore wallet. But feel free to use the code as you wish, and I would be happy to help if you have any questions.

@jimmychu0807
Copy link
Contributor

jimmychu0807 commented Sep 4, 2024

Hi @cedoor @saleel

I am PSE Core Taiwan fellow. I read through the issue and want to check:

  • is this issue still relevant today, or is it just not closed?
  • With Add a function to verify proof without storing any nullifier #329 resolved, do you see other major blockers that prevent driving this issue to completion?
  • if you still welcome contribution on this issue, may I pick this up and provide guidance/feedback along the way?

Thank you.

@saleel
Copy link
Contributor

saleel commented Sep 4, 2024

I think we still need to:

Both should be doable (though second one might be tricky as Solidity doesn't support array constants). I will check them again by next week and see if we can update.

@cedoor
Copy link
Member

cedoor commented Sep 20, 2024

Thanks @saleel for listing the missing tasks!

if you still welcome contribution on this issue, may I pick this up and provide guidance/feedback along the way?

@jimmychu0807 of course, I'll assign this issue to you!

@jimmychu0807
Copy link
Contributor

jimmychu0807 commented Oct 1, 2024

Screenshot 2024-10-01 at 10 23 05

@saleel as seen on the above screenshot, top right (TR) is the geth run with the docker command listed in the bundler repo. top left (TL) is the bundler run with yarn run bundler. Seems I have successfully run it.

Now, in BL, I run yarn run runop --deployFactory --network http://localhost:8545/ --entryPoint 0x0000000071727De22E5E9d8BAf0edAc6f37da032 (shown here) all the executions are reverted (shown in TR). I suspect it is not supposed to be like that, right?

Then when I run yarn test in semaphore wallet (BR), it is running the e2e test as you mentioned in the doc, deploying a few contracts, but failing at the "before all" hook saying cannot estimate gas.

I suspect they all come from the same source. Is there some Geth setting I need to update to run these account abstraction tests?

Thanks.

Update 2024-10-01 05:20 UTC

On running semaphore wallet yarn test, I found that running with hardhat node get different result from Geth node. On Geth node, it fails in the semaphoreContract.addMembers() line in the before hook. In hardhat node, there is a better luck and run further till factoryContract.addStake(). Now I will use hardhat node to further run the test, but at some point need to learn how to run Geth node with right configuration.


Another newbie question:

WARN [10-01|03:38:08.080] Served eth_call
conn=192.168.65.1:40046 reqid=102 duration="992.375µs" err="execution reverted"
errdata="\"0x11e6f5a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000109ad9ce884bc7289b35959d0eca1049b52a7aba99785c080ba921af8689db37a\""

Given the Geth dev node returns the above message, how can I trace back from which smart contract and which function is it reverted from? I have an impression there is a way to trace it from the first 4 - 6 bytes. Thanks again


p.s. I don't mind continue the trouble-shooting conversation in this issue thread, but if any of you want to move this discussion somewhere, let me know where is the best place to discuss this. I expect there will be more of it.

@jimmychu0807
Copy link
Contributor

jimmychu0807 commented Oct 1, 2024

@cedoor @saleel

The past few days I started picking up on this issue. I read thru EIP-4337, read account abstraction contract code, try running bundler, and also EIP-6900.

A classmate in the core program, @quanxi1, suggests implementing Semaphore wallet in ERC-6900 way (his deck).

As I believe modular smart contract account (MSCA) is going to be more widely accepted, and the plugin system is indeed a very neat way to integrate "Semaphore" component into any smart contract accounts, I am now gearing toward more on adding the Semaphore capability by writing a MSCA plugin module. This plugin will have the LMTs stored inside its storage, so there will be no problem in reading storage.

I will investigate more to tackle this issue from this angle, but would love to hear from you two what you think about this approach.

@quanxi1 feel free to add any insight, and I will also work with you on this.

Thanks.

But I first need to get saleel Semaphore wallet tests run first :)

@saleel
Copy link
Contributor

saleel commented Oct 1, 2024

@jimmychu0807 Thank you for picking this up. If I remember correctly it used to work with both geth and hardhat. For the full e2e test, you need to run the bundler and use the modified Semaphore contracts inside that repo (should be configured that way already).
That "cannot estimate gas" happens when the contract call would be reverted if its ran. Maybe you can parse the hex return error data here (remove the 0x)

suggests implementing Semaphore wallet in ERC-6900
This plugin will have the LMTs stored inside its storage, so there will be no problem in reading storage.

Yea, that would be great. Also consider ERC-7579.

@cedoor
Copy link
Member

cedoor commented Oct 1, 2024

I don't mind continue the trouble-shooting conversation in this issue thread, but if any of you want to move this discussion somewhere, let me know where is the best place to discuss this. I expect there will be more of it.

I think we can keep it here for now 👍🏽

@jimmychu0807
Copy link
Contributor

jimmychu0807 commented Oct 8, 2024

Just want to report back on current status (2024-10-08)

  • I am still running Semaphore wallet e2e test. Have not run it successfully yet, but progress has been made, and I am tracing code between semaphore wallet and the Bundler RunOp interaction.
  • Updated Semaphore wallet to use Semaphore v4, ethers.js v6, @account-abstraction/contracts v0.7, and @account-abstraction/utils latest on this branch.

@jimmychu0807
Copy link
Contributor

jimmychu0807 commented Oct 8, 2024

@saleel if you have time, I have a favor to ask you. Referring to the Bundler README, could you run on the master branch of Bundler and get to the point to have the simple test below to pass, using Geth dev node?

yarn run runop --deployFactory --network http://localhost:8545/ --entryPoint 0x0000000071727De22E5E9d8BAf0edAc6f37da032

I couldn't, and I am checking with Infinitism team.

Below are the command output and output of geth node (which have executions reverted).

Bundler output:
Screenshot 2024-10-08 at 13 21 04

runOp output:
Screenshot 2024-10-08 at 13 22 39

Geth dev node output:
Screenshot 2024-10-08 at 13 24 26

@saleel
Copy link
Contributor

saleel commented Oct 8, 2024

Did you try to parse the errdata? If its a string message you can decode here

I also see a low balance message in some of your logs..maybe that is the reason?

I tried running the bundler now and faced some issues; looks like there is a missing dependency when deploying contacts (some npm package doesnt have the dist published)..dont have much time to debug; good that you are checking with them. Let me know if you face issues even after setting up geth and bunder with basic tests.

@jimmychu0807
Copy link
Contributor

jimmychu0807 commented Oct 9, 2024

There are two repeated errdata above:

  • errdata="\"0x091cd005a1b9a8ceb62b7ff0190c72c8327ef30713996459410bfc7faf5ef0dd20ce69b6\""
  • errdata="\"0x11e6f5a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001c3f67297df3eb3f12a7d33473e90f58885d044b8dd0c12f44d2b7f2fe83aa9fb\""

The first one seems like a string but not parsable using your tool and onlinestringtools, returning me some symbol and error messages.

The second one seems to start with a function selector 0x11e6f5a0. That's all I can tell.

@jimmychu0807
Copy link
Contributor

Current status (2024-10-15)

Yes there are some detours taken in the past week, but this is a good research for me and hopefully I will get more done in the coming week.

@cedoor
Copy link
Member

cedoor commented Oct 15, 2024

@jimmychu0807 thanks for the updates 🙏🏽 Looks like we need to prioritize the following issues mentioned by Saleel:

remove sub() from gas() calls like here
Vkey points needs to be constants #330

Which seem to be a blocker for another AA <> Semaphore grant. Are you planning to work on them? Other community devs might also work on it.

@jimmychu0807
Copy link
Contributor

@cedoor alright, I will work on these two (sub-) issues first then.

  1. remove sub() from gas() calls like here
  2. Vkey points needs to be constants Change verification key points in contract to constant #330

@cedoor
Copy link
Member

cedoor commented Oct 15, 2024

Thanks @jimmychu0807, can you create an issue for the first one? I'll assign the second one to you, please add a comment there.

@JohnGuilding
Copy link

Hey @jimmychu0807, John here from the ZK Email team at PSE. I have some familiarity building smart account modules as we built our recovery module as an ERC7579 executor. I'm also evaluating the prospective AA <> Semaphore grant @cedoor mentioned.

Just trying to get my head around the approach of integrating Semaphore into a module. Can you explain what the use-case would be? Would this be a recovery module? Would it just be an easy, generic way to use Semaphore for smart accounts? Something else?

@jimmychu0807
Copy link
Contributor

jimmychu0807 commented Oct 19, 2024

Hi @JohnGuilding, thank you for checking. cedoor did point out to me there is a grant proposal of Semaphore Paymaster. This module I want to build is to focus on having Semaphore group members able to sign a transaction of the smart wallet.

In fact, this idea has been explored by Saleel, and is documented in his blogpost. But it was built at a time when the tool stack was still primitive. I aim to formalize this so it becomes an ERC-7579 compatible module and smart account owners can easily install it. Saleel also pointed out two challenges he saw in current implementation (other challenge section of the blogpost) that restrict Semaphore from directly being used in smart account. They will be solved along the way (one is solved already with #875).

There are also two issues I want to investigate first before formalizing my thought into a grant proposal:

  1. Investigate if the semaphore proof/nullifier output can somehow be used by Safe multi-sig module. If this is true, then this can become a semaphore multi-sig wallet. Otherwise it is a 1/N wallet, with N being the number Semaphore group members.

  2. If the above is true, and because a smart account signer is sending a HTTP request to the bundler, and the bundler in turn sends the transaction on-chain with paymaster configured to pay for the tx fee. Does this mean we can build a privacy-preserving multi-sig wallet? Privacy-preserving here means the wallet signer address(es) are not exposed out (this is not the case for Safe wallet now and we can use getOwners() to retrieve the wallet signers) because they are replaced by Semaphore group-generated public key.

I am currently investigating them, but will be glad to hear your thought about this direction, and any potential roadblocks you see I will encounter.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
Status: 🏗 In Progress
Development

No branches or pull requests

9 participants
@jimmychu0807 @cedoor @saleel @JohnGuilding @moven0831 @aguzmant103 @FoodChain1028 and others