Supporting ERC4337 #15

eerkaijun · 2023-08-18T09:42:45Z

Work in progress for spinning up ERC4337 bundler and paymaster.

bundler.py

eerkaijun · 2023-08-31T13:32:25Z

Managed to spin up a bundler and verifying paymaster, tested them as well (currently tested on L1 devnet, when L2 is ready, we should test those too, as I believe the gas estimation will be slightly different).

An open ended question is private key management: there are multiple instances where devs will need to use a signer.

Account abstraction contract deployments
Bundler signer
Paymaster signer

Should we assume that people will use the same account for all three of these instances (thus only ask for the private key once), or should we ask three times (so that devs can put in different private keys if they want to)?

paymaster/package-lock.json

norswap · 2023-08-30T09:09:45Z

paymaster/tsconfig.json

+{
+    "compilerOptions": {
+      "target": "ES6",
+      "module": "commonjs",


Is there a way we can use ESM or do we pull in stuff that won't work with it?
(Also a huge nit, not urgent, and not a blocker for merging this.)

Let me double check

Could you add an issue for it if you don't end up looking into it? If you do and it's bothersome, I think it's good to add a comment about it.

We should probably add a "non-priority" tag for stuff like that, I just want to keep track of it, makes for good batch spring cleanup.

norswap · 2023-09-01T08:32:59Z

bundler.py

+    help="disable ANSI escape codes for terminal manipulation",
+    default=True,
+    dest="use_ansi_esc",
+    action="store_false")


Should this be folded into roll.py?
Not really important rn.

Can you add an issue for this & link it here?

Can you add an issue for this and link it here?

bundler.py

paymaster/src/abis.ts

paymaster/src/server.ts

paymaster/test/tests.ts

norswap · 2023-09-01T09:55:13Z

Should we assume that people will use the same account for all three of these instances (thus only ask for the private key once), or should we ask three times (so that devs can put in different private keys if they want to)?

Yes, we should support using different ones. See my comment above, probably want to include these as different config fields. If the user does not provide them, prompt. What we can do is give an easy alias for reusing a key. (e.g. you can type "deployer" to reuse to the deployment key)

norswap · 2023-09-01T09:56:02Z

Also, L2 should be working now! (I need to push the change that uses HTTP instead of WebSocket on the op-node, but that's the only fix necessary)

eerkaijun · 2023-09-05T09:02:19Z

@norswap okie made some changes, have also now tested it with L2

bundler.py

norswap · 2023-09-18T08:34:35Z

bundler.py

+    help="disable ANSI escape codes for terminal manipulation",
+    default=True,
+    dest="use_ansi_esc",
+    action="store_false")


Can you add an issue for this & link it here?

bundler.py

norswap · 2023-09-18T08:41:34Z

bundler.py

+    help="disable ANSI escape codes for terminal manipulation",
+    default=True,
+    dest="use_ansi_esc",
+    action="store_false")


Can you add an issue for this and link it here?

norswap · 2023-09-18T08:43:39Z

paymaster/tsconfig.json

+{
+    "compilerOptions": {
+      "target": "ES6",
+      "module": "commonjs",


Could you add an issue for it if you don't end up looking into it? If you do and it's bothersome, I think it's good to add a comment about it.

We should probably add a "non-priority" tag for stuff like that, I just want to keep track of it, makes for good batch spring cleanup.

norswap

This is shaping up great.

My suggestion here would be to run through the review comments, change whatever can be done quickly, and collect a list of what can't (I can compile it if you want), then merge this and do the rest in a subsequent PR.

I want to try to decrease time-to-merge, esp. when there's ongoing parallel work (not a problem atm).

Sorry for taking so long to review!

norswap · 2023-09-18T09:08:02Z

paymaster/README.md

+1. Users send a JSON RPC request to the paymaster server with the corresponding UserOp.
+2. The paymaster server signs the hash of the UserOp if it decides to sponsor the transaction. The signature is encoded in the `paymasterAndData` field of the userOp.
+3. The paymaster server sends the updated UserOp back to the user.
+4. The user can now send the UserOp to the bundler and the transaction will be sponsored by the paymaster.


Oh I see, there is no mempool involved!

This is good, below is just me spitballing on what how I thought this would be + how it could be:

Is there a way in which the paymaster could submit the transaction to the bundler directly? I guess the updated userop needs to be signed by the user right?

Before seeing this, my very naive flow assumption was:

Users submit userOp to mempool (functionally equivalent, as its a RPC call, just that here we don't put it in the mempool)

Batchers pick userOp, check if they can be sponsored (using the whole 4337 dance) and if so submit them.

The sponsorship check is on-chain, the paymaster contract basically always sponsors non-reverting tx (is it even possible to check this in 4337???) to whitelisted contracts. Could also involve rate limiting (though update usage would have to live in the sponsored tx, NOT in the validation logic which can't write state).

I think the ideal flow for what we're doing would be to:

Send to server that is both a paymaster & bundler. (Optionally drop to mempool, no need for now.)

Sign userOp, then wrap in a tx that

Writes to the paymaster contract, authorizing userOp

Submits the userOp to the entrypoint contract (paymaster validation then checks the authorization we just wrote)

This would be an ideal one-request flow from the user's perspective.

You mean users send the signed UserOp to paymaster, and the paymaster submit to the entrypoint contract directly?

Yes exactly!

paymaster/README.md

norswap · 2023-09-18T09:09:30Z

paymaster/src/abis.ts

+    } catch (error) {
+        console.error('Error reading/parsing JSON file:', error);
+    }
+}


Why not reuse the read_json_file from libroll.py here?

This file is in Typescript.

norswap · 2023-09-18T09:10:31Z

libroll.py

+    """
+    run(descr, f"git clone {url}")
+    print(f"Succeeded: {descr}")
+


Could you extend the usage of this to other parts of the codebase where we clone? Or add an issue for it is equally good!

Already done :)

norswap · 2023-09-18T09:12:05Z

paymaster/src/rpcMethods.ts

@@ -0,0 +1,54 @@
+import { BigNumberish, ethers } from 'ethers';


I could switch to viem later if I wanted to, right?

Yes, that shouldn't be a problem

paymaster/src/rpcMethods.ts

norswap · 2023-09-18T09:17:45Z

paymaster/src/rpcMethods.ts

+    );
+
+    /// NOTE: Define additional transaction sponsor logic here if needed
+    /// For example, it could be a list of whitelisted addresses


Do you think you could implement this as a configuration value that is either None/undefined (in which case there is not check) or a list (in which case only whitelisted contracts are allowed, nothing allowed if list is empty)?

Probably good for next PR.

^ for this one though, .env won't cut it :)

Yeah, I think this would be a good enhancement. Added a new issue: #39

bundler.py

norswap · 2023-09-18T09:36:26Z

bundler.py

+    log_file = open(log_file_path, "w")
+    PROCESS_MGR.start(
+        "start bundler", 
+        "stackup-bundler start --mode private",


Just curious, what does --mode private do?

Means it's a private mempool for the bundler (i.e. only our own bundler will be able to see the transactions sent to this RPC endpoints as they are not broadcasted)

norswap · 2023-09-18T09:40:47Z

config.py

+        """
+        Private key to use for deploying 4337 contracts and paymaster signature (None by default).
+        Will be used if set, otherwise will prompt users to enter private key.
+        """


Is it good to always make the deployer the same as the paymaster signature address? Haven't really thought about this in earnest.

I know people like to talk about "secure signers" as option for holding live keys (e.g. paymaster signer) and it might be a bit of a pain to use such a signer for deployment? Is that a good reason to separate them?

This is a good question, let's revisit it: #40

norswap · 2023-09-18T09:50:10Z

Oh and I think you need to run make check to reformat the python code (and maybe make sure it actually runs on python files inside paymaster!

norswap reviewed Aug 18, 2023

View reviewed changes

bundler.py Outdated Show resolved Hide resolved

eerkaijun force-pushed the kai/account-abstraction branch 2 times, most recently from 2070b2c to 949c100 Compare August 26, 2023 14:50

eerkaijun force-pushed the kai/account-abstraction branch from b7ebfe8 to 88714a5 Compare August 31, 2023 11:04

eerkaijun marked this pull request as ready for review August 31, 2023 13:33

norswap reviewed Sep 1, 2023

View reviewed changes

eerkaijun added 16 commits September 5, 2023 12:58

initial bundler setup script

6155758

account abstraction contracts deployment

3f6bb86

local opstack deployment

6cc8ee9

set env for bundler

7233251

use existing helper functions

8471c7b

bundler works

4d2da3f

add logging and cleanup

7617c6c

barebore paymaster server

23a7118

methods to sign paymaster data

c6a8cba

verifying paymaster signer

aa74975

paymaster rpc server and tests

616b390

update end to end test

3f62152

paymaster first need to stake

aef5e96

set paymaster environment variable

7a41f50

linting

d796319

change paymaster to pnpm and some other minor changes

9a0c218

eerkaijun force-pushed the kai/account-abstraction branch from 7c883b4 to 9a0c218 Compare September 5, 2023 05:33

eerkaijun added 2 commits September 5, 2023 16:12

private keys in L2Config

36dd9be

automate contract addresses and abis

eeade6b

paymaster documentation

d173aa2

norswap reviewed Sep 18, 2023

View reviewed changes

norswap approved these changes Sep 18, 2023

View reviewed changes

cleanup

c211fc8

eerkaijun merged commit 26c2c67 into master Sep 18, 2023
2 checks passed

This was referenced Sep 20, 2023

Should we separate ERC4337 contracts deployment private key and paymaster server private key? #40

Closed

Ability to define paymaster sponsor logic in a configuration file #39

Closed

eerkaijun deleted the kai/account-abstraction branch September 24, 2023 11:07

norswap mentioned this pull request Sep 26, 2023

Spin up EIP-4337 infrastructure #4

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Supporting ERC4337 #15

Supporting ERC4337 #15

eerkaijun commented Aug 18, 2023

eerkaijun commented Aug 31, 2023

norswap Aug 30, 2023

eerkaijun Sep 5, 2023

norswap Sep 18, 2023

eerkaijun Sep 18, 2023

norswap Sep 1, 2023

norswap Sep 18, 2023

norswap Sep 18, 2023

eerkaijun Sep 18, 2023

norswap commented Sep 1, 2023

norswap commented Sep 1, 2023

eerkaijun commented Sep 5, 2023

norswap Sep 18, 2023

norswap Sep 18, 2023

norswap Sep 18, 2023

norswap left a comment

norswap Sep 18, 2023

eerkaijun Sep 18, 2023

norswap Sep 20, 2023

norswap Sep 18, 2023

eerkaijun Sep 18, 2023

norswap Sep 20, 2023

norswap Sep 18, 2023

eerkaijun Sep 18, 2023

norswap Sep 18, 2023

eerkaijun Sep 18, 2023

norswap Sep 18, 2023

norswap Sep 18, 2023

eerkaijun Sep 18, 2023

norswap Sep 18, 2023

eerkaijun Sep 18, 2023

norswap Sep 18, 2023

eerkaijun Sep 18, 2023

norswap commented Sep 18, 2023

		@@ -0,0 +1,54 @@
		import { BigNumberish, ethers } from 'ethers';

Supporting ERC4337 #15

Supporting ERC4337 #15

Conversation

eerkaijun commented Aug 18, 2023

eerkaijun commented Aug 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

norswap commented Sep 1, 2023

norswap commented Sep 1, 2023

eerkaijun commented Sep 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

norswap left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

norswap commented Sep 18, 2023