Skip to content

NOJIRA: Try refactoring the nonce handling #2565

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

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
9 changes: 0 additions & 9 deletions packages/passport/sdk/src/guardian/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,11 @@ describe('Guardian', () => {
revertOnError: true,
to: mockUserZkEvm.zkEvm.ethAddress,
value: '0x00',
nonce: 5,
},
{
revertOnError: true,
to: '0x123',
value: '0x',
nonce: 5,
},
],
}),
Expand Down Expand Up @@ -200,7 +198,6 @@ describe('Guardian', () => {
revertOnError: true,
to: mockUserZkEvm.zkEvm.ethAddress,
value: '0x00',
nonce: 5,
},
],
});
Expand Down Expand Up @@ -250,7 +247,6 @@ describe('Guardian', () => {
revertOnError: true,
to: mockUserZkEvm.zkEvm.ethAddress,
value: '0x00',
nonce: 5,
},
],
isBackgroundTransaction: true,
Expand Down Expand Up @@ -302,7 +298,6 @@ describe('Guardian', () => {
revertOnError: true,
to: mockUserZkEvm.zkEvm.ethAddress,
value: '0x00',
nonce: 1,
},
],
isBackgroundTransaction: false,
Expand Down Expand Up @@ -354,7 +349,6 @@ describe('Guardian', () => {
revertOnError: true,
to: mockUserZkEvm.zkEvm.ethAddress,
value: '0x00',
nonce: 1,
},
],
});
Expand Down Expand Up @@ -415,7 +409,6 @@ describe('Guardian', () => {
revertOnError: true,
to: mockUserZkEvm.zkEvm.ethAddress,
value: '0x00',
nonce: 5,
},
],
});
Expand Down Expand Up @@ -450,13 +443,11 @@ describe('Guardian', () => {
revertOnError: true,
to: mockUserZkEvm.zkEvm.ethAddress,
value: '0x00',
nonce: 5,
},
{
revertOnError: true,
to: '0x123',
value: '0x00',
nonce: 5,
},
],
}),
Expand Down
2 changes: 1 addition & 1 deletion packages/passport/sdk/src/guardian/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type GuardianERC191MessageEvaluationParams = {
const transactionRejectedCrossSdkBridgeError = 'Transaction requires confirmation but this functionality is not'
+ ' supported in this environment. Please contact Immutable support if you need to enable this feature.';

export const convertBigNumberishToString = (
const convertBigNumberishToString = (
value: BigNumberish,
): string => BigInt(value).toString();

Expand Down
10 changes: 2 additions & 8 deletions packages/passport/sdk/src/zkEvm/transactionHelpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ describe('transactionHelpers', () => {
{
to: transactionRequest.to,
data: transactionRequest.data,
nonce,
value: transactionRequest.value,
revertOnError: true,
},
Expand Down Expand Up @@ -171,7 +170,6 @@ describe('transactionHelpers', () => {
revertOnError: true,
to: transactionRequest.to,
value: '0x00',
nonce: expect.any(BigInt),
}),
]),
}),
Expand Down Expand Up @@ -206,13 +204,11 @@ describe('transactionHelpers', () => {
revertOnError: true,
to: transactionRequest.to,
value: '0x00',
nonce: expect.any(BigInt),
}),
expect.objectContaining({
to: '0x7331',
value: expect.any(BigInt),
revertOnError: true,
nonce: expect.any(BigInt),
}),
]),
}),
Expand All @@ -225,17 +221,15 @@ describe('transactionHelpers', () => {
revertOnError: true,
to: transactionRequest.to,
value: '0x00',
nonce: expect.any(BigInt),
}),
expect.objectContaining({
to: '0x7331',
value: expect.any(BigInt),
revertOnError: true,
nonce: expect.any(BigInt),
}),
]),
expect.any(BigInt),
expect.any(BigInt),
nonce,
chainId,
zkEvmAddress,
ethSigner,
);
Expand Down
106 changes: 49 additions & 57 deletions packages/passport/sdk/src/zkEvm/transactionHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Flow } from '@imtbl/metrics';
import {
Signer, TransactionRequest, JsonRpcProvider,
BigNumberish,
} from 'ethers';
import {
getEip155ChainId,
Expand All @@ -11,7 +10,7 @@ import {
getNonce,
} from './walletHelpers';
import { RelayerClient } from './relayerClient';
import GuardianClient, { convertBigNumberishToString } from '../guardian';
import GuardianClient from '../guardian';
import {
FeeOption,
MetaTransaction,
Expand Down Expand Up @@ -71,51 +70,38 @@ const getFeeOption = async (
*/
const buildMetaTransactions = async (
transactionRequest: TransactionRequest,
rpcProvider: JsonRpcProvider,
relayerClient: RelayerClient,
zkevmAddress: string,
nonceSpace?: bigint,
): Promise<[MetaTransaction, ...MetaTransaction[]]> => {
flow: Flow,
): Promise<MetaTransaction[]> => {
if (!transactionRequest.to) {
throw new JsonRpcError(
RpcErrorCode.INVALID_PARAMS,
'eth_sendTransaction requires a "to" field',
);
}

const metaTransaction: MetaTransaction = {
const metaTransactions: MetaTransaction[] = [{
to: transactionRequest.to.toString(),
data: transactionRequest.data,
nonce: BigInt(0), // NOTE: We don't need a valid nonce to estimate the fee
value: transactionRequest.value,
revertOnError: true,
};
}];

// Estimate the fee and get the nonce from the smart wallet
const [nonce, feeOption] = await Promise.all([
getNonce(rpcProvider, zkevmAddress, nonceSpace),
getFeeOption(metaTransaction, zkevmAddress, relayerClient),
]);

// Build the meta transactions array with a valid nonce and fee transaction
const metaTransactions: [MetaTransaction, ...MetaTransaction[]] = [
{
...metaTransaction,
nonce,
},
];
const feeOption = await getFeeOption(metaTransactions[0], zkevmAddress, relayerClient);

// Add a fee transaction if the fee is non-zero
const feeValue = BigInt(feeOption.tokenPrice);
if (feeValue !== BigInt(0)) {
metaTransactions.push({
nonce,
to: feeOption.recipientAddress,
value: feeValue,
revertOnError: true,
});
}

flow.addEvent('endBuildMetaTransactions');
return metaTransactions;
};

Expand Down Expand Up @@ -162,6 +148,12 @@ export const pollRelayerTransaction = async (
return relayerTransaction;
};

const detectNetwork = async (rpcProvider: JsonRpcProvider, flow: Flow) => {
const network = await rpcProvider.getNetwork();
flow.addEvent('endDetectNetwork');
return network;
};

export const prepareAndSignTransaction = async ({
transactionRequest,
ethSigner,
Expand All @@ -173,30 +165,23 @@ export const prepareAndSignTransaction = async ({
nonceSpace,
isBackgroundTransaction,
}: TransactionParams & { transactionRequest: TransactionRequest }) => {
const { chainId } = await rpcProvider.getNetwork();
const chainIdBigNumber = BigInt(chainId);
flow.addEvent('endDetectNetwork');

const metaTransactions = await buildMetaTransactions(
transactionRequest,
rpcProvider,
relayerClient,
zkEvmAddress,
nonceSpace,
);
flow.addEvent('endBuildMetaTransactions');

const { nonce } = metaTransactions[0];
if (typeof nonce === 'undefined') {
throw new Error('Failed to retrieve nonce from the smart wallet');
}
const [metaTransactions, nonce, network] = await Promise.all([
buildMetaTransactions(
transactionRequest,
relayerClient,
zkEvmAddress,
flow,
),
getNonce(rpcProvider, zkEvmAddress, nonceSpace),
detectNetwork(rpcProvider, flow),
]);

// Parallelize the validation and signing of the transaction
// without waiting for the validation to complete
const validateTransaction = async () => {
await guardianClient.validateEVMTransaction({
chainId: getEip155ChainId(Number(chainId)),
nonce: convertBigNumberishToString(nonce),
chainId: getEip155ChainId(Number(network.chainId)),
nonce: nonce.toString(),
metaTransactions,
isBackgroundTransaction,
});
Expand All @@ -209,7 +194,7 @@ export const prepareAndSignTransaction = async ({
const signed = await signMetaTransactions(
metaTransactions,
nonce,
chainIdBigNumber,
network.chainId,
zkEvmAddress,
ethSigner,
);
Expand All @@ -230,37 +215,42 @@ export const prepareAndSignTransaction = async ({

const buildMetaTransactionForEjection = async (
transactionRequest: TransactionRequest,
): Promise<[MetaTransaction, ...MetaTransaction[]]> => {
): Promise<MetaTransaction> => {
if (!transactionRequest.to) {
throw new JsonRpcError(
RpcErrorCode.INVALID_PARAMS,
'im_signEjectionTransaction requires a "to" field',
);
}

if (typeof transactionRequest.nonce === 'undefined') {
const metaTransaction: MetaTransaction = {
to: transactionRequest.to.toString(),
data: transactionRequest.data,
value: transactionRequest.value,
revertOnError: true,
};

return metaTransaction;
};

const parseNonce = (transactionRequest: TransactionRequest): bigint => {
if (typeof transactionRequest.nonce === 'undefined' || transactionRequest.nonce === null) {
throw new JsonRpcError(
RpcErrorCode.INVALID_PARAMS,
'im_signEjectionTransaction requires a "nonce" field',
);
}
return BigInt(transactionRequest.nonce);
};

if (!transactionRequest.chainId) {
const parseChainId = (transactionRequest: TransactionRequest): bigint => {
if (typeof transactionRequest.chainId === 'undefined' || transactionRequest.chainId === null) {
throw new JsonRpcError(
RpcErrorCode.INVALID_PARAMS,
'im_signEjectionTransaction requires a "chainId" field',
);
}

const metaTransaction: MetaTransaction = {
to: transactionRequest.to.toString(),
data: transactionRequest.data,
nonce: transactionRequest.nonce ?? undefined,
value: transactionRequest.value,
revertOnError: true,
};

return [metaTransaction];
return BigInt(transactionRequest.chainId);
};

export const prepareAndSignEjectionTransaction = async ({
Expand All @@ -269,15 +259,17 @@ export const prepareAndSignEjectionTransaction = async ({
zkEvmAddress,
flow,
}: EjectionTransactionParams & { transactionRequest: TransactionRequest }): Promise<EjectionTransactionResponse> => {
const nonce = parseNonce(transactionRequest);
const chainId = parseChainId(transactionRequest);
const metaTransaction = await buildMetaTransactionForEjection(
transactionRequest,
);
flow.addEvent('endBuildMetaTransactions');

const signedTransaction = await signMetaTransactions(
metaTransaction,
transactionRequest.nonce as BigNumberish,
BigInt(transactionRequest.chainId ?? 0),
[metaTransaction],
nonce,
chainId,
zkEvmAddress,
ethSigner,
);
Expand Down
1 change: 0 additions & 1 deletion packages/passport/sdk/src/zkEvm/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export interface MetaTransaction {
to: string;
value?: BigNumberish | null;
data?: string | null;
nonce?: BigNumberish;
gasLimit?: BigNumberish;
delegateCall?: boolean;
revertOnError?: boolean;
Expand Down
6 changes: 3 additions & 3 deletions packages/passport/sdk/src/zkEvm/walletHelpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ describe('signMetaTransactions', () => {
data: '0x',
},
];
const nonce = 0;
const chainId = 1779;
const nonce = 0n;
const chainId = 1779n;

const signature = await signMetaTransactions(
transactions,
nonce,
BigInt(chainId),
chainId,
walletAddress,
signer,
);
Expand Down
2 changes: 1 addition & 1 deletion packages/passport/sdk/src/zkEvm/walletHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export const encodeMessageSubDigest = (chainId: bigint, walletAddress: string, d

export const signMetaTransactions = async (
metaTransactions: MetaTransaction[],
nonce: BigNumberish,
nonce: bigint,
chainId: bigint,
walletAddress: string,
signer: Signer,
Expand Down