-
Notifications
You must be signed in to change notification settings - Fork 30
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
feat: account abstraction and paymaster syntax #1
Changes from 13 commits
e106637
d3dcf2f
480bb35
8b99a1c
a3530de
b394da8
934569f
083197d
2d8300c
7e59111
39c8d94
1120160
1b5ede8
4c2b3b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
export * as utils from "./utils"; | ||
export * as types from "./types"; | ||
export { EIP712Signer, Signer, L1Signer } from "./signer"; | ||
export { Wallet } from "./wallet"; | ||
export { Wallet, AbstractWallet, signingFunction } from "./wallet"; | ||
export { BrowserProvider, Provider } from "./provider"; | ||
export { ContractFactory, Contract } from "./contract"; | ||
export { Paymaster } from "./paymaster"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import { getAddress } from "ethers"; | ||
import { ApprovalBasedPaymasterInput, GeneralPaymasterInput } from "./types"; | ||
import { encodeData, getPaymasterParams } from "./utils"; | ||
|
||
export class Paymaster { | ||
type: string; | ||
address: string; | ||
paymasterInput: ApprovalBasedPaymasterInput | GeneralPaymasterInput; | ||
|
||
constructor( | ||
type: "ApprovalBased" | "General", | ||
address: string, | ||
tokenAddress: string = "", | ||
minimalAllowance: number = 1, | ||
innerInput: any[] = [], | ||
) { | ||
this.type = type; | ||
this.address = getAddress(address); | ||
const encodedInnerInput = encodeData(innerInput); | ||
this.paymasterInput = | ||
type == "ApprovalBased" | ||
? { | ||
type, | ||
token: getAddress(tokenAddress), | ||
minimalAllowance, | ||
innerInput: encodedInnerInput, | ||
} | ||
: { | ||
type, | ||
innerInput: encodedInnerInput, | ||
}; | ||
} | ||
|
||
getPaymasterParams() { | ||
return getPaymasterParams(this.address, this.paymasterInput); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,7 @@ | ||
import { AbiCoder, BigNumberish, BytesLike, ethers, SignatureLike } from "ethers"; | ||
import { IERC20__factory, IL1Bridge__factory } from "../typechain"; | ||
import { Provider } from "./provider"; | ||
import { EIP712Signer } from "./signer"; | ||
import { | ||
Address, | ||
DeploymentInfo, | ||
|
@@ -7,11 +10,8 @@ import { | |
PaymasterParams, | ||
PriorityOpTree, | ||
PriorityQueueType, | ||
TransactionLike, | ||
TransactionLike | ||
} from "./types"; | ||
import { Provider } from "./provider"; | ||
import { EIP712Signer } from "./signer"; | ||
import { IERC20__factory, IL1Bridge__factory } from "../typechain"; | ||
|
||
export * from "./paymaster-utils"; | ||
|
||
|
@@ -594,3 +594,24 @@ export async function estimateCustomBridgeDepositL2Gas( | |
l2Value: l2Value, | ||
}); | ||
} | ||
|
||
export function encodeData(data: any[]): string { | ||
const types = data.map(item => { | ||
if (typeof item === 'number') { | ||
return 'uint256'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if I need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good question. I thought the same while setting up this function too (another reason why I left this PR as draft). I would think now then that it is not possible to deduce these lower levels types without the developer specifying them either through an ABI or some form of input. This changes the syntax for constructing both the |
||
} else if (typeof item === 'string') { | ||
return 'string'; | ||
} else if (typeof item === 'boolean') { | ||
return 'bool'; | ||
} else if (Array.isArray(item)) { | ||
throw new Error('Nested arrays are not supported'); | ||
} else if (typeof item === 'object' && item !== null) { | ||
throw new Error('Nested objects are not supported'); | ||
} | ||
throw new Error(`Unsupported data type: ${typeof item}`); | ||
}); | ||
|
||
const coder = new AbiCoder(); | ||
const encodedData = coder.encode(types, data); | ||
return encodedData; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,10 @@ | ||
import { EIP712Signer } from "./signer"; | ||
import { ProgressCallback, ethers } from "ethers"; | ||
import { AdapterL1, AdapterL2 } from "./adapters"; | ||
import { Paymaster } from "./paymaster"; | ||
import { Provider } from "./provider"; | ||
import { EIP712_TX_TYPE, serializeEip712 } from "./utils"; | ||
import { ethers, ProgressCallback } from "ethers"; | ||
import { EIP712Signer } from "./signer"; | ||
import { TransactionLike, TransactionRequest, TransactionResponse } from "./types"; | ||
import { AdapterL1, AdapterL2 } from "./adapters"; | ||
import { DEFAULT_GAS_PER_PUBDATA_LIMIT, EIP712_TX_TYPE, serializeEip712 } from "./utils"; | ||
|
||
export class Wallet extends AdapterL2(AdapterL1(ethers.Wallet)) { | ||
// @ts-ignore | ||
|
@@ -124,3 +125,52 @@ export class Wallet extends AdapterL2(AdapterL1(ethers.Wallet)) { | |
return await this.provider.broadcastTransaction(await this.signTransaction(populatedTx)); | ||
} | ||
} | ||
|
||
interface SigningFunction { | ||
(this: AbstractWallet, transaction: TransactionRequest): Promise<string>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does it need access to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could otherwise accept the wallet as a parameter as: (abstractWallet: AbstractWallet, transaction: TransactionRequest): Promise<string>; Both approaches are fine by me |
||
} | ||
|
||
export class AbstractWallet extends Wallet { | ||
readonly accountAddress: string; | ||
paymaster: Paymaster; | ||
customSigningFunction: SigningFunction; | ||
|
||
constructor( | ||
accountAddress: string, | ||
privateKey: string, | ||
providerL2?: Provider, | ||
providerL1?: ethers.Provider, | ||
paymaster: Paymaster | null = null, | ||
customSigningFunction: SigningFunction = signingFunction, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a default here? Or should we just make it required? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find it makes the syntax for declaration simpler by adding a default here. I prefer it because of that. I can imagine some developers might not be interested in overriding the signing function and only seek to use AA for the custom logic you can add to accounts on-chain. In these cases I can imagine they'd be happy not having to be concerned with the signing logic, or paymaster logic either (if they do not wish to use a paymaster). |
||
) { | ||
super(privateKey, providerL2); | ||
this.accountAddress = accountAddress; | ||
this.paymaster = paymaster; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
...
transaction.customData = this.paymaster ? {
gasPerPubdata: DEFAULT_GAS_PER_PUBDATA_LIMIT,
paymasterParams: this.paymaster.getPaymasterParams(),
} : {};
... But I think it would be better to change it actually to: ...
transaction.customData ??= this.paymaster ? {
gasPerPubdata: DEFAULT_GAS_PER_PUBDATA_LIMIT,
paymasterParams: this.paymaster.getPaymasterParams(),
} : {};
... Considering the developer might want to specify some transactions on the frontend where a paymaster is applied, and some others not. But if the developer wants a paymaster to be used in all transactions, then they can simply declare the paymaster in this constructor here. |
||
this.providerL1 = providerL1; | ||
this.customSigningFunction = customSigningFunction.bind(this); | ||
} | ||
|
||
override transfer(transaction: { to: string; amount: ethers.BigNumberish; token?: string; overrides?: ethers.Overrides; }): Promise<TransactionResponse> { | ||
transaction.overrides = { type: EIP712_TX_TYPE }; | ||
return super.transfer(transaction); | ||
} | ||
|
||
override getAddress(): Promise<string> { | ||
return Promise.resolve(this.accountAddress); | ||
} | ||
|
||
override async signTransaction(transaction: TransactionRequest): Promise<string> { | ||
return this.customSigningFunction(transaction); | ||
} | ||
} | ||
|
||
export async function signingFunction(this: AbstractWallet, transaction: TransactionRequest): Promise<string> { | ||
transaction.customData = this.paymaster ? { | ||
gasPerPubdata: DEFAULT_GAS_PER_PUBDATA_LIMIT, | ||
paymasterParams: this.paymaster.getPaymasterParams(), | ||
} : {}; | ||
transaction.customData = this._fillCustomData(transaction.customData); | ||
transaction.customData.customSignature = await this.eip712.sign(transaction); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how does it work? What key is used to sign it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will use the I find it also tricky to know what exactly is going on with the class as it extends another class about 5 times.. But this approach adds the least lines of code too. |
||
// @ts-ignore | ||
return (0, serializeEip712)(transaction); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this class if we simply want to build paymaster params? Shouldn't it be a util instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I also notice this method will require adding parameters as those are required for the
ApprovalBased
paymasters that accept a quantity of tokens to cover the transaction cost.As for making this a class, this class will also include a method to enrich a transaction with the data for
paymasterParams
which is useful when developers only want to sponsor some transactions. Something like.addParamsToTransaction(trx)
that callsgetPaymasterParams
to fill in the transaction object. This removes the need for the developer to manipulate the transaction object manually.Not precious about this either though, so if you think strongly of keeping these as functions then we could go that way but I find this:
Less appealing than: