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.

Sign up for GitHub

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: add a nonce key override to support parallel nonces #462

Merged
merged 1 commit into from
Feb 19, 2024
Merged

feat: add a nonce key override to support parallel nonces #462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/core/src/account/smartContractAccount.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export type SmartContractAccount<
typedDataDefinition: TypedDataDefinition<typedData, primaryType>
) => Promise<Hex>;
encodeUpgradeToAndCall: (params: UpgradeToAndCallParams) => Promise<Hex>;
getNonce(): Promise<bigint>;
getNonce(nonceKey?: bigint): Promise<bigint>;
getInitCode: () => Promise<Hex>;
isAccountDeployed: () => Promise<boolean>;
getFactoryAddress: () => Address;
Expand Down Expand Up @@ -244,12 +244,12 @@ export async function toSmartContractAccount<
return initCode === "0x";
};

const getNonce = async () => {
const getNonce = async (nonceKey = 0n) => {
if (!(await isAccountDeployed())) {
return 0n;
}

return entryPointContract.read.getNonce([accountAddress_, BigInt(0)]);
return entryPointContract.read.getNonce([accountAddress_, nonceKey]);
};

const account = toAccount({
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/actions/smartAccount/buildUserOperation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const buildUserOperation: <
uo: {
initCode: account.getInitCode(),
sender: account.address,
nonce: account.getNonce(),
nonce: account.getNonce(overrides?.nonceKey),
callData: Array.isArray(uo)
? account.encodeBatchExecute(uo)
: typeof uo === "string"
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ export type UserOperationOverrides = Partial<{
| UserOperationStruct["verificationGasLimit"]
| Percentage;
paymasterAndData: UserOperationStruct["paymasterAndData"];
/**
* This can be used to override the key used when calling `entryPoint.getNonce`
* It is useful when you want to use parallel nonces for user operations
*
* NOTE: not all bundlers fully support this feature and it could be that your bundler will still only include
* one user operation for your account in a bundle
*/
nonceKey: bigint;
Copy link
Contributor

Choose a reason for hiding this comment

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

so by differentiating nonceKey, you can send parallel nonces. But our bundler supports only up to 4 for unstaked accounts. (Unstaked senders are only allowed to have 4 UOs in the mempool. This is already possible today with parallel nonces).

  1. we should put doc explaining about these nonce keys, their usage, context, etc.
  2. this is an easy quick solution, but I think not really falls into "override". Would be better to have our sendUO api to support parallel nonces more natively.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right now our bundler doesn't really support parallel nonces (it does but users don't get the full benefit since a bundle will only contain 1 UO from a given sender). I think once our bundler supports it more fully then we can add much better docs.

  1. yea I can update the UserOperationOverride docs for this override.
  2. I disagree about it not being a override, but agree there's probably a better way to do it. It's an override because other methods use sendUserOperation under the hood. Take a look at the plugin gen'd methods for example. Those all call send UO under the hood. This overrides the default behaviour of buildUserOperation, and such falls in the mental model of an override. There is a world where getNonce is actually a middleware instead, and then users provide custom nonce middleware (like ethers-rs / alloy). But until we understand how users plan to use this (ie. is baking this into the entryPoint.getNonce() method not enough), I don't think we should over optimize

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added a comment here since the docs for UserOperationOverrides just link you to this part of the code anyways

Copy link
Contributor

Choose a reason for hiding this comment

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

for now not blocking.

Copy link
Contributor

Choose a reason for hiding this comment

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

@moldy530 parallel nonces means there are multiple UOs in the bundler mempool, different from batching which is just one UO. My understanding is that our bundler does support multiple parallel nonces from a single sender, up to 4 for unstaked users. Am I understanding wrong? cc: @dancoombs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Our bundler does support submitting parallel nonces and it won't reject the UOs, but it will still only include one UO per bundle (at least was my understanding). That's what I was trying to get at in the comment that it might not be "fully supported". The full benefit of the parallel nonces is when all of your UOs can be included in the bundle

}>;

// represents the request as it needs to be formatted for RPC requests
Expand Down
Loading