Skip to content

feat: batching for reads and smart-account writes #773

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

Merged
merged 8 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
68 changes: 68 additions & 0 deletions sdk/src/services/BackendWalletService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,72 @@ export class BackendWalletService {
});
}

/**
* Send a batch of raw transactions atomically
* Send a batch of raw transactions in a single UserOp. Can only be used with smart wallets.
* @param chain A chain ID ("137") or slug ("polygon-amoy-testnet"). Chain ID is preferred.
* @param xBackendWalletAddress Backend wallet address
* @param requestBody
* @param simulateTx Simulates the transaction before adding it to the queue, returning an error if it fails simulation. Note: This step is less performant and recommended only for debugging purposes.
* @param xIdempotencyKey Transactions submitted with the same idempotency key will be de-duplicated. Only the last 100000 transactions are compared.
* @param xAccountAddress Smart account address
* @param xAccountFactoryAddress Smart account factory address. If omitted, Engine will try to resolve it from the contract.
* @param xAccountSalt Smart account salt as string or hex. This is used to predict the smart account address. Useful when creating multiple accounts with the same admin and only needed when deploying the account as part of a userop.
* @returns any Default Response
* @throws ApiError
*/
public sendTransactionsAtomic(
chain: string,
xBackendWalletAddress: string,
requestBody: {
transactions: Array<{
/**
* A contract or wallet address
*/
toAddress?: string;
data: string;
value: string;
}>;
},
simulateTx: boolean = false,
xIdempotencyKey?: string,
xAccountAddress?: string,
xAccountFactoryAddress?: string,
xAccountSalt?: string,
): CancelablePromise<{
result: {
/**
* Queue ID
*/
queueId: string;
};
}> {
return this.httpRequest.request({
method: 'POST',
url: '/backend-wallet/{chain}/send-transactions-atomic',
path: {
'chain': chain,
},
headers: {
'x-backend-wallet-address': xBackendWalletAddress,
'x-idempotency-key': xIdempotencyKey,
'x-account-address': xAccountAddress,
'x-account-factory-address': xAccountFactoryAddress,
'x-account-salt': xAccountSalt,
},
query: {
'simulateTx': simulateTx,
},
body: requestBody,
mediaType: 'application/json',
errors: {
400: `Bad Request`,
404: `Not Found`,
500: `Internal Server Error`,
},
});
}

/**
* Sign a transaction
* Sign a transaction
Expand Down Expand Up @@ -833,6 +899,7 @@ export class BackendWalletService {
onchainStatus: ('success' | 'reverted' | null);
effectiveGasPrice: (string | null);
cumulativeGasUsed: (string | null);
batchOperations: null;
}>;
};
}> {
Expand Down Expand Up @@ -928,6 +995,7 @@ export class BackendWalletService {
onchainStatus: ('success' | 'reverted' | null);
effectiveGasPrice: (string | null);
cumulativeGasUsed: (string | null);
batchOperations: null;
} | string);
}>;
}> {
Expand Down
44 changes: 44 additions & 0 deletions sdk/src/services/ContractService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,50 @@ export class ContractService {
});
}

/**
* Batch read from multiple contracts
* Execute multiple contract read operations in a single call using Multicall
* @param chain
* @param requestBody
* @returns any Default Response
* @throws ApiError
*/
public readBatch(
chain: string,
requestBody: {
calls: Array<{
contractAddress: string;
functionName: string;
functionAbi?: string;
args?: Array<any>;
}>;
/**
* Address of the multicall contract to use. If omitted, multicall3 contract will be used (0xcA11bde05977b3631167028862bE2a173976CA11).
*/
multicallAddress?: string;
},
): CancelablePromise<{
results: Array<{
success: boolean;
result: any;
}>;
}> {
return this.httpRequest.request({
method: 'POST',
url: '/contract/{chain}/read-batch',
path: {
'chain': chain,
},
body: requestBody,
mediaType: 'application/json',
errors: {
400: `Bad Request`,
404: `Not Found`,
500: `Internal Server Error`,
},
});
}

/**
* Write to contract
* Call a write function on a contract.
Expand Down
3 changes: 3 additions & 0 deletions sdk/src/services/TransactionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export class TransactionService {
onchainStatus: ('success' | 'reverted' | null);
effectiveGasPrice: (string | null);
cumulativeGasUsed: (string | null);
batchOperations: null;
}>;
totalCount: number;
};
Expand Down Expand Up @@ -162,6 +163,7 @@ export class TransactionService {
onchainStatus: ('success' | 'reverted' | null);
effectiveGasPrice: (string | null);
cumulativeGasUsed: (string | null);
batchOperations: null;
};
}> {
return this.httpRequest.request({
Expand Down Expand Up @@ -245,6 +247,7 @@ export class TransactionService {
onchainStatus: ('success' | 'reverted' | null);
effectiveGasPrice: (string | null);
cumulativeGasUsed: (string | null);
batchOperations: null;
}>;
totalCount: number;
};
Expand Down
2 changes: 1 addition & 1 deletion src/scripts/generate-sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import path from "node:path";
import { kill } from "node:process";

const ENGINE_OPENAPI_URL = "https://demo.web3api.thirdweb.com/json";
// const ENGINE_OPENAPI_URL = "http://localhost:3005/json";
// const ENGINE_OPENAPI_URL = "http://127.0.0.1:3005/json";
const REPLACE_LOG_FILE = "sdk/replacement_log.txt";

type BasicOpenAPISpec = {
Expand Down
151 changes: 151 additions & 0 deletions src/server/routes/backend-wallet/send-transactions-atomic.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
import { Type, type Static } from "@sinclair/typebox";
import type { FastifyInstance } from "fastify";
import { StatusCodes } from "http-status-codes";
import type { Address, Hex } from "thirdweb";
import { insertTransaction } from "../../../shared/utils/transaction/insert-transaction";
import { AddressSchema } from "../../schemas/address";
import {
requestQuerystringSchema,
standardResponseSchema,
transactionWritesResponseSchema,
} from "../../schemas/shared-api-schemas";
import {
maybeAddress,
walletChainParamSchema,
walletWithAAHeaderSchema,
} from "../../schemas/wallet";
import { getChainIdFromChain } from "../../utils/chain";
import {
getWalletDetails,
isSmartBackendWallet,
type ParsedWalletDetails,
WalletDetailsError,
} from "../../../shared/db/wallets/get-wallet-details";
import { createCustomError } from "../../middleware/error";

const requestBodySchema = Type.Object({
transactions: Type.Array(
Type.Object({
toAddress: Type.Optional(AddressSchema),
data: Type.String({
Copy link
Contributor

Choose a reason for hiding this comment

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

Use HexSchema

examples: ["0x..."],
}),
value: Type.String({
Copy link
Contributor

Choose a reason for hiding this comment

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

Use WeiAmountStringSchema

examples: ["10000000"],
}),
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: These are the minimal args of a "raw transaction" and may be useful to DRY since it's used in a few places. Then we can add descriptions too.

),
});

export async function sendTransactionsAtomicRoute(fastify: FastifyInstance) {
fastify.route<{
Params: Static<typeof walletChainParamSchema>;
Body: Static<typeof requestBodySchema>;
Reply: Static<typeof transactionWritesResponseSchema>;
Querystring: Static<typeof requestQuerystringSchema>;
}>({
method: "POST",
url: "/backend-wallet/:chain/send-transactions-atomic",
Copy link
Contributor

Choose a reason for hiding this comment

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

There's already a /send-transactions-batch. IMO introducing /send-transactions-atomic will confuse the dev.

WDYT about reusing the same handler and branching the logic if SBW? In the docs we'd then need to clarify "Order is guaranteed for SBW only. If non-SBW, order is not guaranteed".

If you still want two separate paths, I'd prefer avoiding a new term (atomic) and using simpler language, like /send-transaction-batch-ordered. Or /send-userop-batch to make it clear it's for smart wallets.

Copy link
Member Author

Choose a reason for hiding this comment

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

answered why not the same handler in a question above, Joaquim had a similar question.

I do agree the name for the other path could be improved, but I ran out of ideas.

send-userop-batch could be misleading since we're creating a single UserOp rather than batching multiple UserOps.

I do like keeping "atomic" in the endpoint name since it's a widely understood term beyond web3 - developers regularly encounter it in databases, filesystems, and other contexts where operations need to succeed or fail as one unit, and it's more precise than "ordered".

I was initially thinking /send-transaction-batch-atomic to go along with the existing /send-transaction-batch, but felt it would be too long.

Copy link
Member

Choose a reason for hiding this comment

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

maybe send-batch-transaction ? we kinda use that language already in the sdk and docs

Copy link
Member Author

Choose a reason for hiding this comment

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

We already have a send-transaction-batch endpoint which just queues up all the transactions and sends them non atomically

Copy link
Member

Choose a reason for hiding this comment

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

Why don't you edit that endpoint then? Just do an actual batch if it's a smart backend wallet

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered modifying the send-transaction-batch endpoint, but decided against it, because:

  1. There's legitimate use-cases for non-atomic batching, even when using smart backend wallets. Batching atomically you might run into block gas limit, no limits on non-atomic batching though

  2. Because of above reason, we shouldn't auto-force atomic batch on SBW. This might break user expectations, so instead having a batch?: boolean option would be good. This isn't easy to add, because existing endpoint takes an expects an array as the transaction body, no easy way to add to request body.
    (We can still add to queryParam)

  3. Another argument against automatic atomic batching when SBW: we also want to support atomic batching for regular EOA + Account Address style smart accounts.

  4. There's also a couple of things don't make sense to share between the two batching strategies. Eg, idempotency-key. Non-atomic batching returns multiple queue-ids (queueIds: string[]), so an idempotency key doesn't work there, but it does make sense to include when using atomic batching, since there's only one queue-id returned.

  5. shouldSimulate is not available as a query-param for non-atomic batch, due to latency of individually simulating transactions. But we can allow this for atomic batch.

  6. As mentioned before: non atomic batch returns an array of queueIds, and atomic batch only returns a single queueId. If we automatically move to atomic batching on the same endpoint, and start returning only a single queueId in the array, client code that expects transactions.length == queueIds.length might break

Given the number of special cases to handle if we were to merge endpoints, I think having them separate is best.

Copy link
Contributor

Choose a reason for hiding this comment

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

You listed enough differences to justify a different endpoint rather than overloading one. 👍

More thoughts on names:

  • /send-ordered-transactions
  • /send-userop-batch
  • /send-transaction-sequence
    I don't have a strong opinion against atomic, seems clear too.

Copy link
Member

Choose a reason for hiding this comment

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

ugh i see. is anyone actually using the non-atomic batch endpoint though? seems very niche when you can just send 2 api calls

schema: {
summary: "Send a batch of raw transactions atomically",
description:
"Send a batch of raw transactions in a single UserOp. Can only be used with smart wallets.",
tags: ["Backend Wallet"],
operationId: "sendTransactionsAtomic",
params: walletChainParamSchema,
body: requestBodySchema,
headers: Type.Omit(walletWithAAHeaderSchema, ["x-transaction-mode"]),
querystring: requestQuerystringSchema,
response: {
...standardResponseSchema,
[StatusCodes.OK]: transactionWritesResponseSchema,
},
},
handler: async (request, reply) => {
const { chain } = request.params;
const {
"x-backend-wallet-address": fromAddress,
"x-idempotency-key": idempotencyKey,
"x-account-address": accountAddress,
"x-account-factory-address": accountFactoryAddress,
"x-account-salt": accountSalt,
} = request.headers as Static<typeof walletWithAAHeaderSchema>;
const chainId = await getChainIdFromChain(chain);
const shouldSimulate = request.query.simulateTx ?? false;
const transactionRequests = request.body.transactions;

const hasSmartHeaders = !!accountAddress;

// check that we either use SBW, or send using EOA with smart wallet headers
if (!hasSmartHeaders) {
let backendWallet: ParsedWalletDetails | undefined;

try {
backendWallet = await getWalletDetails({
address: fromAddress,
});
} catch (e: unknown) {
if (e instanceof WalletDetailsError) {
throw createCustomError(
`Failed to get wallet details for backend wallet ${fromAddress}. ${e.message}`,
StatusCodes.BAD_REQUEST,
"WALLET_DETAILS_ERROR",
);
}
}

if (!backendWallet) {
throw createCustomError(
"Failed to get wallet details for backend wallet. See: https://portal.thirdweb.com/engine/troubleshooting",
StatusCodes.INTERNAL_SERVER_ERROR,
"WALLET_DETAILS_ERROR",
);
}

if (!isSmartBackendWallet(backendWallet)) {
throw createCustomError(
"Backend wallet is not a smart wallet, and x-account-address is not provided. Either use a smart backend wallet or provide x-account-address. This endpoint can only be used with smart wallets.",
StatusCodes.BAD_REQUEST,
"BACKEND_WALLET_NOT_SMART",
);
}
}

if (transactionRequests.length === 0) {
throw createCustomError(
"No transactions provided",
StatusCodes.BAD_REQUEST,
"NO_TRANSACTIONS_PROVIDED",
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to assert this in TypeBox: { minItems: 1 }


const queueId = await insertTransaction({
insertedTransaction: {
transactionMode: undefined,
isUserOp: false,
chainId,
from: fromAddress as Address,
accountAddress: maybeAddress(accountAddress, "x-account-address"),
accountFactoryAddress: maybeAddress(
accountFactoryAddress,
"x-account-factory-address",
),
accountSalt: accountSalt,
batchOperations: transactionRequests.map((transactionRequest) => ({
to: transactionRequest.toAddress as Address | undefined,
data: transactionRequest.data as Hex,
value: BigInt(transactionRequest.value),
})),
},
shouldSimulate,
idempotencyKey,
});

reply.status(StatusCodes.OK).send({
result: {
queueId,
},
});
},
});
}
Loading
Loading