Skip to content

Commit 5d2b6df

Browse files
authored
perf(bot): parallelize independent bot factory setup steps (#24581)
Parallelizes the independent setup steps in the bot factory (`@aztec/bot`, `bot/src/factory.ts`). This is production bot code that runs against live networks, so the changes are strictly behavior-preserving: same contracts at the same (salt-derived) addresses, same amounts, same final state. Only the ordering/concurrency of provably-independent setup steps changes. The bot factory is the single largest cost in the `single-node/bot` e2e suite: each `*.create` runs the production sequencer at 12s L2 slots (`PIPELINING_SETUP_OPTS`) and deploys/mints one tx per slot, fully serial. `AmmBot.create` alone was ~74s (7 serial txs). ## Dependency graph per bot type Edges below are "must be mined before", verified against the Noir contract source (`token_contract`, `amm_contract`). ### Transaction bot -- `setup()` (unchanged) `setupAccount` -> register recipient -> `ensureFeeJuiceBalance` -> deploy token -> mint. This chain is fully data-dependent: funding gates the deploy, the deploy gates the mint. The recipient (`createSchnorrAccount`) is register-only (no tx), and the private+public mints are already batched into one tx. Nothing is independent, so `setup()` is left serial. ### AMM bot -- `setupAmm()` Steps: deploy token0 (D0), token1 (D1), LP token (DL), AMM (DA); `set_minter` granting the AMM rights over the LP token (SM); mint token0+token1 to the provider (M, one batched tx); `add_liquidity` (AL). - D0, D1, DL: independent -- distinct contracts, no cross-references. - DA: the AMM constructor only stores the three token addresses (no calls into the tokens). Those addresses are salt-derived, so DA needs only the (pre-derived) addresses, not the token deploys mined. - SM: `Token::set_minter` just writes `minters[amm] = true`; it does not call or validate the AMM contract. It needs the LP token deployed and the (derivable) AMM address only -- not the AMM deployed. The deployer is authorized because the Token constructor sets `minters[admin] = true`. - M: `mint_to_private`/`mint_to_public` require the caller be a minter; the deployer is admin=minter from the constructor, so the token0/token1 mints need only those tokens deployed (no `set_minter`). - AL: `add_liquidity` transfers token0/token1 from the provider (needs M for balances), mints LP tokens (needs SM so the AMM is an LP minter), and targets the AMM (needs DA). Restructured from 7 serial txs into 3 slot-phases: - Phase 1: `Promise.all([D0, D1, DL])` - Phase 2: `Promise.all([DA, SM, M])` - Phase 3: `AL` ### Cross-chain bot -- `setupCrossChain()` Steps: deploy TestContract (an L2 tx), seed N L1->L2 messages (L1 txs), wait for the first message ready. - The seeds reference only the L2 recipient (TestContract) address, which is salt-derived. L1->L2 messages are queued on L1 and do not require the L2 contract to exist yet; they are consumed later in `bot.run()`, after setup completes. So the L2 deploy and the L1 seeding are independent. - The L2 deploy pays via the L2 wallet/PXE account; the seeds use the bot's L1 client -- different accounts, so there is no L1 nonce interaction between them. Restructured to overlap the deploy with the seed loop: `Promise.all([deploy TestContract, seed loop])`, then the message-ready wait. ## What stayed serial, and why - The whole transaction-bot `setup()` (data-dependent chain, nothing independent). - `ensureFeeJuiceBalance` before every deploy (funding must precede spending). - `add_liquidity` after its mints + minter grant + AMM deploy. - The individual L1->L2 seeds inside the loop: they share the bot's single L1 account, so concurrent sends would race on the L1 nonce. The suite already documents this nonce hazard. Only the loop as a whole overlaps the (L2) TestContract deploy. ## Production safety / idempotent re-entry - Every deploy still goes through `registerOrDeployContract`, which checks `getContractMetadata(address).isContractPublished` per contract and only registers (no tx) if already deployed. If one parallel deploy fails and the others succeed, the next `*.create` recovers: the succeeded contracts register-only, the failed one redeploys. Addresses are salt-derived and identical across runs. - `set_minter` is idempotent (writes `true` again). The AMM path's mint and `add_liquidity` always run (no balance guard) -- unchanged from the previous `fundAmm`. - Same-sender concurrent `.send()`s are safe by construction: the PXE serializes simulate/prove through its `SerialQueue` and setup txs pay with random tx nonces. The bot runs the same `EmbeddedWallet` -> PXE path in the e2e suite and in production. - Deliberate, benign failure-path change: because Phase 2 runs DA/SM/M concurrently, a failure of one no longer prevents the others from being sent. Their effects (a deployed-but-unused AMM, a minter grant, an extra mint) are all idempotent-safe, and `add_liquidity` uses fixed amounts, so re-entry converges to the same final state. - `withNoMinTxsPerBlock` (the `flushSetupTransactions` save/zero/restore wrapper around each setup tx) was not safe to re-enter concurrently: interleaved save/zero/restore could read the already-zeroed value and "restore" `minTxsPerBlock` to 0 permanently. It now reference-counts entrants -- the first saves and zeroes, the last restores -- with unit tests covering the overlapping and failure paths (`bot/src/factory.test.ts`). Serial callers see the exact same RPC sequence as before. Final state is identical in every path: same contract addresses, same minter grant, same minted amounts, same liquidity. ## Local evidence Two full local runs of `single-node/bot/bot.test.ts` (production sequencer, 12s L2 slots), all 10 tests passing in both. Hook durations from factory log timestamps, against the round-4 local baseline measured on the same machine at the same cadence (findings from #24534): - `Bot.create` (transaction-bot hook, untouched): 31.8s vs 32.2s baseline -- unchanged, as intended. - `AmmBot.create`: 57.2s vs 73.9s baseline (-16.7s). The three token deploy txs go out within 300ms of each other; the AMM deploy, `set_minter` and the mint batch all go out within the following slot. - `CrossChainBot.create`: 24.0s vs 43.5s baseline (-19.5s). The TestContract deploy overlaps both L1 seeds; the first message is ready almost immediately after the deploy is mined. - Suite-level `beforeHooksMs`: 116.9s vs 153.7s baseline (-36.8s), matching the per-hook deltas. - Unit: `bot/src/factory.test.ts` (4 tests) covering the `withNoMinTxsPerBlock` reentrancy fix. ## Expected effect on CI - `AmmBot.create`: 7 serial txs -> 3 slot-phases (~87s on CI per #24534's spans -> ~55-60s). - `CrossChainBot.create`: deploy overlaps the seed pipeline (~43s -> ~25s). - Transaction bot: unchanged. - Aggregate: roughly -35 to -45s on the bot suite's beforeAll wall time. - Fix-phase proof metric (once #24534's `setup:bot` instrumentation is on this base): the amm-bot and cross-chain `setup:bot` occurrences drop per the numbers above. Measured CI numbers to be appended when a full run lands. ## Measured impact The bot suite (a single shard) drops 216.3s → 118.3s total (**−98.0s, −45%**); beforeHooks 215.9s → 118.0s. - No `setup:bot` spans exist on this base (that instrumentation lives in the unmerged #24534), so the metric is the suite-level hook fold, which covers all three nested bot factory setups (Bot, AmmBot, CrossChainBot). - Noise context from the same run pair: untouched light suites move −2 to −3s per shard (private_initialization −2.9s/shard over 20 shards, deploy_method −2.1s/shard over 14), and the largest counter-move is validators_sentinel +24s (multi-node variance). A −98s single-shard delta is far outside that envelope. - The CI delta exceeds the local −36.8s because the CI baseline pays more missed-slot penalties per serial tx (baseline beforeHooks 215.9s on CI vs 153.7s locally at the same 12s cadence); collapsing 7 serial txs into 3 slot-phases removes proportionally more where slots are more often missed. Baseline CI run 1783393028773172 (merge-base 30966a4, full). PR CI run 1783427250198215 (x-fast). Fixes A-1408
1 parent 1321e98 commit 5d2b6df

2 files changed

Lines changed: 211 additions & 107 deletions

File tree

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { promiseWithResolvers } from '@aztec/foundation/promise';
2+
import type { AztecNode, AztecNodeAdmin, AztecNodeAdminConfig } from '@aztec/stdlib/interfaces/client';
3+
import type { EmbeddedWallet } from '@aztec/wallets/embedded';
4+
5+
import { mock } from 'jest-mock-extended';
6+
7+
import { type BotConfig, getBotDefaultConfig } from './config.js';
8+
import { BotFactory } from './factory.js';
9+
import type { BotStore } from './store/index.js';
10+
11+
class TestBotFactory extends BotFactory {
12+
public override withNoMinTxsPerBlock<T>(fn: () => Promise<T>): Promise<T> {
13+
return super.withNoMinTxsPerBlock(fn);
14+
}
15+
}
16+
17+
describe('BotFactory.withNoMinTxsPerBlock', () => {
18+
let minTxsPerBlock: number | undefined;
19+
let setConfigCalls: (number | undefined)[];
20+
let factory: TestBotFactory;
21+
22+
beforeEach(() => {
23+
minTxsPerBlock = 3;
24+
setConfigCalls = [];
25+
26+
const aztecNodeAdmin = mock<AztecNodeAdmin>();
27+
aztecNodeAdmin.getConfig.mockImplementation(() => Promise.resolve({ minTxsPerBlock } as AztecNodeAdminConfig));
28+
aztecNodeAdmin.setConfig.mockImplementation(config => {
29+
setConfigCalls.push(config.minTxsPerBlock);
30+
minTxsPerBlock = config.minTxsPerBlock;
31+
return Promise.resolve();
32+
});
33+
34+
const config: BotConfig = { ...getBotDefaultConfig(), flushSetupTransactions: true };
35+
const wallet = mock<EmbeddedWallet>();
36+
factory = new TestBotFactory(config, wallet, mock<BotStore>(), mock<AztecNode>(), aztecNodeAdmin);
37+
});
38+
39+
it('zeroes minTxsPerBlock around a call and restores it after', async () => {
40+
await factory.withNoMinTxsPerBlock(() => {
41+
expect(minTxsPerBlock).toBe(0);
42+
return Promise.resolve();
43+
});
44+
45+
expect(setConfigCalls).toEqual([0, 3]);
46+
expect(minTxsPerBlock).toBe(3);
47+
});
48+
49+
it('zeroes once and restores once across overlapping calls', async () => {
50+
const gates = [promiseWithResolvers<void>(), promiseWithResolvers<void>(), promiseWithResolvers<void>()];
51+
const calls = gates.map(gate => factory.withNoMinTxsPerBlock(() => gate.promise));
52+
53+
gates[0].resolve();
54+
await calls[0];
55+
expect(minTxsPerBlock).toBe(0);
56+
57+
gates[1].resolve();
58+
await calls[1];
59+
expect(minTxsPerBlock).toBe(0);
60+
61+
gates[2].resolve();
62+
await calls[2];
63+
expect(setConfigCalls).toEqual([0, 3]);
64+
expect(minTxsPerBlock).toBe(3);
65+
});
66+
67+
it('restores minTxsPerBlock when the wrapped call fails', async () => {
68+
await expect(factory.withNoMinTxsPerBlock(() => Promise.reject(new Error('boom')))).rejects.toThrow('boom');
69+
70+
expect(setConfigCalls).toEqual([0, 3]);
71+
expect(minTxsPerBlock).toBe(3);
72+
});
73+
74+
it('restores minTxsPerBlock when one of several overlapping calls fails', async () => {
75+
const gate = promiseWithResolvers<void>();
76+
const failing = factory.withNoMinTxsPerBlock(() => Promise.reject(new Error('boom')));
77+
const succeeding = factory.withNoMinTxsPerBlock(() => gate.promise);
78+
79+
await expect(failing).rejects.toThrow('boom');
80+
expect(minTxsPerBlock).toBe(0);
81+
82+
gate.resolve();
83+
await succeeding;
84+
expect(setConfigCalls).toEqual([0, 3]);
85+
expect(minTxsPerBlock).toBe(3);
86+
});
87+
});

0 commit comments

Comments
 (0)