diff --git a/backend/src/__tests__/shared/bundle-builder.test.ts b/backend/src/__tests__/shared/bundle-builder.test.ts index 421080c..ca60a3b 100644 --- a/backend/src/__tests__/shared/bundle-builder.test.ts +++ b/backend/src/__tests__/shared/bundle-builder.test.ts @@ -1,5 +1,16 @@ import { describe, it, expect, beforeEach } from "vitest"; import { ethers } from "ethers"; + +const { mockEstimateGas } = vi.hoisted(() => ({ + mockEstimateGas: vi.fn(), +})); + +vi.mock("../../providers/chain.provider", () => ({ + getProvider: vi.fn(() => ({ + estimateGas: mockEstimateGas, + })), +})); + import { BundleBuilder, ADAPTER_SELECTORS, @@ -73,6 +84,91 @@ describe("BundleBuilder", () => { builder = new BundleBuilder(CHAIN_ID); }); + +describe("buildWithGas", () => { + const USER = "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266"; + + beforeEach(() => { + mockEstimateGas.mockReset(); + mockEstimateGas.mockResolvedValue(100_000n); + }); + + it("estimates gas for a single-step bundle", async () => { + builder.addExecute( + PROTOCOL_ID, + ADAPTER_SELECTORS.SWAP, + [], + DEADLINE, + "0x", + 0n, + EXECUTOR, + "Swap" + ); + + const bundle = await builder.buildWithGas("Single step", USER); + + expect(mockEstimateGas).toHaveBeenCalledTimes(1); + expect(bundle.steps[0].gas).toBe("0x1fbd0"); + }); + + it("estimates only the first step of a multi-step bundle", async () => { + builder + .addApproveIfNeeded( + TOKEN_IN, + EXECUTOR, + 0n, + 1000n, + "Approve WETH" + ) + .addExecute( + PROTOCOL_ID, + ADAPTER_SELECTORS.SWAP, + [{ token: TOKEN_IN, amount: 1000n }], + DEADLINE, + "0x", + 0n, + EXECUTOR, + "Swap" + ); + + const bundle = await builder.buildWithGas( + "Approve + Swap", + USER + ); + + expect(bundle.steps).toHaveLength(2); + expect(mockEstimateGas).toHaveBeenCalledTimes(1); + + expect(bundle.steps[0].gas).toBe("0x1fbd0"); + expect(bundle.steps[1].gas).toBeUndefined(); + }); + + it("returns the first step without gas when estimation fails", async () => { + mockEstimateGas.mockRejectedValueOnce( + new Error("estimation failed") + ); + + builder.addExecute( + PROTOCOL_ID, + ADAPTER_SELECTORS.SWAP, + [], + DEADLINE, + "0x", + 0n, + EXECUTOR, + "Swap" + ); + + const bundle = await builder.buildWithGas( + "Failed estimate", + USER + ); + + expect(bundle.steps).toHaveLength(1); + expect(bundle.steps[0].gas).toBeUndefined(); + }); +}); + // ── addApproveIfNeeded ───────────────────────────────────────────────────── describe("addApproveIfNeeded", () => { diff --git a/backend/src/shared/bundle-builder.ts b/backend/src/shared/bundle-builder.ts index 72f2e66..53e9d57 100644 --- a/backend/src/shared/bundle-builder.ts +++ b/backend/src/shared/bundle-builder.ts @@ -128,47 +128,94 @@ export class BundleBuilder { }; } - /** - * Builds the bundle and estimates gas for each step using the chain's RPC provider. - * This avoids MetaMask needing to call eth_estimateGas (which can be rate-limited). - * Adds a 30% buffer to the estimate to prevent out-of-gas failures. - */ - async buildWithGas(summary: string, fromAddress: string): Promise { - const chainName = CHAIN_ID_TO_NAME[this.chainId]; - if (!chainName) { - logger.warn({ chainId: this.chainId }, "Unknown chainId for gas estimation, returning without gas"); - return this.build(summary); +/** + * Builds the bundle and estimates gas where the estimate is valid against + * current on-chain state. + * + * Only the first step can be safely estimated generically. Later steps may + * depend on state transitions produced by earlier transactions in the bundle + * (for example approve -> execute). eth_estimateGas does not persist state + * changes from previous simulations, so independently estimating every step + * can produce false reverts. + * + * Adds a 30% buffer to successful estimates. + */ +async buildWithGas( + summary: string, + fromAddress: string +): Promise { + const chainName = CHAIN_ID_TO_NAME[this.chainId]; + + if (!chainName) { + logger.warn( + { chainId: this.chainId }, + "Unknown chainId for gas estimation, returning without gas" + ); + return this.build(summary); + } + + if (this.steps.length === 0) { + return this.build(summary); + } + + const provider = getProvider(chainName); + const stepsWithGas: PreparedTransaction[] = []; + + for (let index = 0; index < this.steps.length; index++) { + const step = this.steps[index]; + + /* + * Only the first transaction is guaranteed to be estimable against the + * current chain state. Subsequent transactions may depend on state changes + * made by previous bundle steps. + */ + if (index > 0) { + logger.debug( + { + step: step.description, + stepIndex: index, + totalSteps: this.steps.length, + }, + "Skipping gas estimation for state-dependent bundle step" + ); + + stepsWithGas.push(step); + continue; } - const provider = getProvider(chainName); - const stepsWithGas = await Promise.all( - this.steps.map(async (step) => { - try { - const estimate = await provider.estimateGas({ - from: fromAddress, - to: step.to, - data: step.data, - value: BigInt(step.value || "0"), - }); - // 30% buffer - const buffered = (estimate * 130n) / 100n; - return { ...step, gas: `0x${buffered.toString(16)}` }; - } catch (err) { - logger.warn( - { step: step.description, error: err instanceof Error ? err.message : "unknown" }, - "Gas estimation failed for step, returning without gas" - ); - return step; - } - }) - ); + try { + const estimate = await provider.estimateGas({ + from: fromAddress, + to: step.to, + data: step.data, + value: BigInt(step.value || "0"), + }); - return { - steps: stepsWithGas, - totalSteps: stepsWithGas.length, - summary, - }; + const buffered = (estimate * 130n) / 100n; + + stepsWithGas.push({ + ...step, + gas: `0x${buffered.toString(16)}`, + }); + } catch (err) { + logger.warn( + { + step: step.description, + error: err instanceof Error ? err.message : "unknown", + }, + "Gas estimation failed for first bundle step, returning without gas" + ); + + stepsWithGas.push(step); + } } + + return { + steps: stepsWithGas, + totalSteps: stepsWithGas.length, + summary, + }; +} } const CHAIN_ID_TO_NAME: Record = {