Skip to content

Commit 96d47a0

Browse files
authored
fix(aztec-node): pipelining-aware slot and fee simulation in simulatePublicCalls (#24031)
## Motivation `AztecNodeService.simulatePublicCalls` built globals for a fictional next block using "the L2 slot at the next L1 slot" with no pipelining offset and no L1 state overrides. Under proposer pipelining this simulates one slot too early and computes the mana min fee against the current L1 state, ignoring the gossiped proposed checkpoint that will land before the simulated block does — producing wrong L1-to-L2 message sets and wrong fees for wallet-side estimation. ## Approach The simulation now mirrors how the sequencer picks the build slot and fee inputs (`sequencer.ts` / `checkpoint_proposal_job.ts` / the checkpoint simulation overrides builder), split into two cases: - **Mid-checkpoint continuation** (proposed blocks past the last checkpoint proposal): every block in a checkpoint shares the same checkpoint-wide globals, so the next block's globals are copied verbatim from the latest proposed block header with only the block number bumped. No L1 reads, no L1-to-L2 message insertion. A missing header fails the request rather than falling through, since the fork already contains the ongoing checkpoint's messages and a fall-through would insert them twice. - **Opening a new checkpoint**: the target slot is the sequencer's formula (`next-L1-slot L2 slot + PROPOSER_PIPELINING_SLOT_OFFSET`), maxed with `proposedCheckpointSlot + 1` when a checkpoint proposal is pending L1. The same `SimulationOverridesPlan` the sequencer uses is always applied: parent archive/temp-log/fee-header overrides derived from the proposed checkpoint when pipelining, tips pinned to the rollback target when the pending chain is invalid, and tips pinned to the checkpointed tip otherwise (neutralizing prunes in fee computation). Both the target slot and the plan derive from a single `ProposedCheckpointData` read so they cannot disagree about the parent. The simulator trusts archiver tips verbatim — no staleness guards. Stale proposed blocks and checkpoints are the archiver's responsibility (orphan pruning and L1-authoritative eviction), so a stale tip mis-simulates only transiently. The logic moves out of `AztecNodeService` into a new `NodePublicCallsSimulator` class so the slot/globals selection is unit-testable without standing up a node. Supersedes #23389. Caching of the L1 fee queries introduced here is tracked separately in A-1208. **Structured for commit-by-commit review:** 1. `refactor(aztec-node)`: pure move of the existing `simulatePublicCalls` code (and its tests) into `NodePublicCallsSimulator` — no behavior change. 2. `fix(aztec-node)`: the actual behavior changes described above. 3. `refactor`: moves the checkpoint simulation overrides builder from sequencer-client to `@aztec/stdlib/checkpoint` so the node consumes it from a shared home. ## API changes - `GlobalVariableBuilder.buildGlobalVariables` is removed from the stdlib interface and its implementations (sequencer-client, TXE); slot selection is now the caller's job and only `buildCheckpointGlobalVariables` remains. - `buildCheckpointSimulationOverridesPlan` and `computePipelinedParentFeeHeader` move from sequencer-client internals to `@aztec/stdlib/checkpoint`, shared by the sequencer and the node simulator. - `AztecNodeService` constructor takes an optional `RollupContract`; environments that never see a proposed checkpoint or an invalid pending chain (TXE) may omit it, and the simulator fails loudly if those paths are reached without it. ## Changes - **aztec-node**: new `NodePublicCallsSimulator` with the two-case globals selection and always-on overrides plan; `simulatePublicCalls` reduced to a thin delegate. - **aztec-node (tests)**: 12 unit tests covering verbatim mid-checkpoint continuation, the missing-header guard, the pipelining offset, parent-slot/override derivation from the proposed checkpoint data, torn-snapshot fallback, invalid-pending-chain tips pinning, message insertion semantics, and the rollup-contract invariant. - **stdlib**: gains `checkpoint/simulation_overrides.ts` (moved from sequencer-client, with its unit tests); `buildGlobalVariables` removed from the `GlobalVariableBuilder` interface. - **sequencer-client**: imports the overrides builder from stdlib; `buildGlobalVariables` removed from the implementation. - **txe**: `buildGlobalVariables` removed; TXE node construction passes no rollup contract. Fixes A-1063
1 parent 15f769d commit 96d47a0

15 files changed

Lines changed: 1130 additions & 681 deletions

File tree

yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts

Lines changed: 457 additions & 0 deletions
Large diffs are not rendered by default.

yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.ts

Lines changed: 383 additions & 0 deletions
Large diffs are not rendered by default.

yarn-project/aztec-node/src/aztec-node/server.test.ts

Lines changed: 5 additions & 232 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ describe('aztec node', () => {
242242
12345,
243243
rollupVersion.toNumber(),
244244
globalVariablesBuilder,
245+
rollupContract,
245246
feeProvider,
246247
epochCache,
247248
getPackageVersion(),
@@ -709,220 +710,6 @@ describe('aztec node', () => {
709710
});
710711
});
711712

712-
describe('simulatePublicCalls', () => {
713-
const mockNextL1Slot = (slot: SlotNumber) => {
714-
jest.spyOn(epochCache, 'getEpochAndSlotInNextL1Slot').mockReturnValue({
715-
epoch: EpochNumber(0),
716-
slot,
717-
ts: 0n,
718-
nowSeconds: BigInt(NOW_S),
719-
});
720-
};
721-
722-
const makeSimulationBlockData = (
723-
blockNumber: BlockNumber,
724-
slotNumber: SlotNumber,
725-
checkpointNumber = CheckpointNumber(1),
726-
): BlockData => ({
727-
header: BlockHeader.empty({
728-
globalVariables: GlobalVariables.empty({ blockNumber, slotNumber }),
729-
}),
730-
archive: L2Block.empty().archive,
731-
blockHash: BlockHash.random(),
732-
checkpointNumber,
733-
indexWithinCheckpoint: IndexWithinCheckpoint(0),
734-
});
735-
736-
it('refuses to simulate public calls if the gas limit is too high', async () => {
737-
const tx = await mockTxForRollup(0x10000);
738-
unfreeze(tx.data.constants.txContext.gasSettings.gasLimits).l2Gas = 1e12;
739-
await expect(node.simulatePublicCalls(tx)).rejects.toThrow(/gas/i);
740-
});
741-
742-
it('uses the slot after the proposed checkpoint when it is later than the next L1 timestamp slot', async () => {
743-
const tx = await mockTxForRollup(0x10000);
744-
const checkpointNumber = CheckpointNumber(1);
745-
const proposedCheckpointBlockNumber = BlockNumber(9);
746-
const targetSlot = SlotNumber(10);
747-
l2BlockSource.getL2Tips.mockResolvedValue(makeTips({ proposed: proposedCheckpointBlockNumber }));
748-
l2BlockSource.getProposedCheckpointData.mockResolvedValue(
749-
makeProposedCheckpoint({
750-
checkpointNumber,
751-
blockNumber: proposedCheckpointBlockNumber,
752-
slotNumber: SlotNumber(9),
753-
}),
754-
);
755-
mockNextL1Slot(SlotNumber(5));
756-
globalVariablesBuilder.buildCheckpointGlobalVariables.mockResolvedValue({
757-
chainId,
758-
version: rollupVersion,
759-
slotNumber: targetSlot,
760-
timestamp: 0n,
761-
coinbase: EthAddress.ZERO,
762-
feeRecipient: AztecAddress.ZERO,
763-
gasFees: GasFees.empty(),
764-
});
765-
766-
await expect(node.simulatePublicCalls(tx)).rejects.toThrow();
767-
768-
// Slot is read from the proposed checkpoint payload header, so no block fetch is needed for it.
769-
expect(globalVariablesBuilder.buildGlobalVariables).not.toHaveBeenCalled();
770-
expect(globalVariablesBuilder.buildCheckpointGlobalVariables).toHaveBeenCalledWith(
771-
EthAddress.ZERO,
772-
AztecAddress.ZERO,
773-
targetSlot,
774-
);
775-
});
776-
777-
it('uses the next L1 timestamp slot when it is later than the slot after the proposed checkpoint', async () => {
778-
const tx = await mockTxForRollup(0x10000);
779-
const checkpointNumber = CheckpointNumber(1);
780-
const proposedCheckpointBlockNumber = BlockNumber(9);
781-
const targetSlot = SlotNumber(12);
782-
l2BlockSource.getL2Tips.mockResolvedValue(makeTips({ proposed: proposedCheckpointBlockNumber }));
783-
l2BlockSource.getProposedCheckpointData.mockResolvedValue(
784-
makeProposedCheckpoint({
785-
checkpointNumber,
786-
blockNumber: proposedCheckpointBlockNumber,
787-
slotNumber: SlotNumber(9),
788-
}),
789-
);
790-
mockNextL1Slot(targetSlot);
791-
globalVariablesBuilder.buildCheckpointGlobalVariables.mockResolvedValue({
792-
chainId,
793-
version: rollupVersion,
794-
slotNumber: targetSlot,
795-
timestamp: 0n,
796-
coinbase: EthAddress.ZERO,
797-
feeRecipient: AztecAddress.ZERO,
798-
gasFees: GasFees.empty(),
799-
});
800-
801-
await expect(node.simulatePublicCalls(tx)).rejects.toThrow();
802-
803-
expect(globalVariablesBuilder.buildGlobalVariables).not.toHaveBeenCalled();
804-
expect(globalVariablesBuilder.buildCheckpointGlobalVariables).toHaveBeenCalledWith(
805-
EthAddress.ZERO,
806-
AztecAddress.ZERO,
807-
targetSlot,
808-
);
809-
});
810-
811-
it('uses the latest proposed block slot when it is ahead of the proposed checkpoint', async () => {
812-
const tx = await mockTxForRollup(0x10000);
813-
const checkpointNumber = CheckpointNumber(1);
814-
const proposedCheckpointBlockNumber = BlockNumber(9);
815-
const latestProposedBlockNumber = BlockNumber(12);
816-
const targetSlot = SlotNumber(12);
817-
l2BlockSource.getL2Tips.mockResolvedValue(makeTips({ proposed: latestProposedBlockNumber }));
818-
l2BlockSource.getProposedCheckpointData.mockResolvedValue(
819-
makeProposedCheckpoint({
820-
checkpointNumber,
821-
blockNumber: proposedCheckpointBlockNumber,
822-
slotNumber: SlotNumber(9),
823-
}),
824-
);
825-
l2BlockSource.getBlockData.mockResolvedValue(
826-
makeSimulationBlockData(latestProposedBlockNumber, targetSlot, checkpointNumber),
827-
);
828-
mockNextL1Slot(SlotNumber(5));
829-
globalVariablesBuilder.buildCheckpointGlobalVariables.mockResolvedValue({
830-
chainId,
831-
version: rollupVersion,
832-
slotNumber: targetSlot,
833-
timestamp: 0n,
834-
coinbase: EthAddress.ZERO,
835-
feeRecipient: AztecAddress.ZERO,
836-
gasFees: GasFees.empty(),
837-
});
838-
839-
await expect(node.simulatePublicCalls(tx)).rejects.toThrow();
840-
841-
// The latest proposed block is ahead of the proposed checkpoint, so its slot is fetched.
842-
expect(l2BlockSource.getBlockData).toHaveBeenCalledWith({ number: latestProposedBlockNumber });
843-
expect(globalVariablesBuilder.buildGlobalVariables).not.toHaveBeenCalled();
844-
expect(globalVariablesBuilder.buildCheckpointGlobalVariables).toHaveBeenCalledWith(
845-
EthAddress.ZERO,
846-
AztecAddress.ZERO,
847-
targetSlot,
848-
);
849-
});
850-
851-
it('disregards missing proposed block slots and uses the next L1 timestamp slot', async () => {
852-
const tx = await mockTxForRollup(0x10000);
853-
const checkpointNumber = CheckpointNumber(1);
854-
const proposedCheckpointBlockNumber = BlockNumber(9);
855-
const latestProposedBlockNumber = BlockNumber(12);
856-
const targetSlot = SlotNumber(13);
857-
l2BlockSource.getL2Tips.mockResolvedValue(makeTips({ proposed: latestProposedBlockNumber }));
858-
l2BlockSource.getProposedCheckpointData.mockResolvedValue(
859-
makeProposedCheckpoint({
860-
checkpointNumber,
861-
blockNumber: proposedCheckpointBlockNumber,
862-
slotNumber: SlotNumber(9),
863-
}),
864-
);
865-
l2BlockSource.getBlockData.mockResolvedValue(undefined);
866-
mockNextL1Slot(targetSlot);
867-
globalVariablesBuilder.buildCheckpointGlobalVariables.mockResolvedValue({
868-
chainId,
869-
version: rollupVersion,
870-
slotNumber: targetSlot,
871-
timestamp: 0n,
872-
coinbase: EthAddress.ZERO,
873-
feeRecipient: AztecAddress.ZERO,
874-
gasFees: GasFees.empty(),
875-
});
876-
877-
await expect(node.simulatePublicCalls(tx)).rejects.toThrow();
878-
879-
// Latest proposed block slot is unavailable; falls back to the next L1 timestamp slot.
880-
expect(l2BlockSource.getBlockData).toHaveBeenCalledWith({ number: latestProposedBlockNumber });
881-
expect(globalVariablesBuilder.buildGlobalVariables).not.toHaveBeenCalled();
882-
expect(globalVariablesBuilder.buildCheckpointGlobalVariables).toHaveBeenCalledWith(
883-
EthAddress.ZERO,
884-
AztecAddress.ZERO,
885-
targetSlot,
886-
);
887-
});
888-
889-
it('treats slot zero as a valid proposed checkpoint slot', async () => {
890-
const tx = await mockTxForRollup(0x10000);
891-
const checkpointNumber = CheckpointNumber(0);
892-
const proposedCheckpointBlockNumber = BlockNumber(0);
893-
const targetSlot = SlotNumber(1);
894-
// No proposed checkpoint leads the frontier; the proposed-checkpoint frontier falls back to the
895-
// checkpointed tip (block 0, slot 0), whose slot is read via getBlockData.
896-
l2BlockSource.getL2Tips.mockResolvedValue(
897-
makeTips({ proposed: proposedCheckpointBlockNumber, checkpointed: checkpointNumber }),
898-
);
899-
l2BlockSource.getProposedCheckpointData.mockResolvedValue(undefined);
900-
l2BlockSource.getBlockData.mockResolvedValue(
901-
makeSimulationBlockData(proposedCheckpointBlockNumber, SlotNumber(0), checkpointNumber),
902-
);
903-
mockNextL1Slot(SlotNumber(0));
904-
globalVariablesBuilder.buildCheckpointGlobalVariables.mockResolvedValue({
905-
chainId,
906-
version: rollupVersion,
907-
slotNumber: targetSlot,
908-
timestamp: 0n,
909-
coinbase: EthAddress.ZERO,
910-
feeRecipient: AztecAddress.ZERO,
911-
gasFees: GasFees.empty(),
912-
});
913-
914-
await expect(node.simulatePublicCalls(tx)).rejects.toThrow();
915-
916-
expect(l2BlockSource.getBlockData).toHaveBeenCalledWith({ number: proposedCheckpointBlockNumber });
917-
expect(globalVariablesBuilder.buildGlobalVariables).not.toHaveBeenCalled();
918-
expect(globalVariablesBuilder.buildCheckpointGlobalVariables).toHaveBeenCalledWith(
919-
EthAddress.ZERO,
920-
AztecAddress.ZERO,
921-
targetSlot,
922-
);
923-
});
924-
});
925-
926713
describe('reloadKeystore', () => {
927714
it('throws BadRequestError if no file-based keystore directory is configured', async () => {
928715
// Default node has no keyStoreDirectory set
@@ -985,6 +772,7 @@ describe('aztec node', () => {
985772
12345,
986773
rollupVersion.toNumber(),
987774
globalVariablesBuilder,
775+
undefined,
988776
feeProvider,
989777
epochCache,
990778
getPackageVersion(),
@@ -1175,6 +963,7 @@ describe('aztec node', () => {
1175963
12345,
1176964
rollupVersion.toNumber(),
1177965
globalVariablesBuilder,
966+
undefined,
1178967
feeProvider,
1179968
epochCache,
1180969
getPackageVersion(),
@@ -1246,6 +1035,7 @@ describe('aztec node', () => {
12461035
12345,
12471036
rollupVersion.toNumber(),
12481037
globalVariablesBuilder,
1038+
undefined,
12491039
mock<FeeProvider>(),
12501040
epochCache,
12511041
getPackageVersion(),
@@ -1299,6 +1089,7 @@ describe('aztec node', () => {
12991089
12345,
13001090
rollupVersion.toNumber(),
13011091
globalVariablesBuilder,
1092+
undefined,
13021093
mock<FeeProvider>(),
13031094
epochCache,
13041095
getPackageVersion(),
@@ -1411,24 +1202,6 @@ describe('aztec node', () => {
14111202
};
14121203
}
14131204

1414-
/** Builds the payload of the atomic leading-proposed-checkpoint read (last block = startBlock). */
1415-
function makeProposedCheckpoint(args: {
1416-
checkpointNumber: CheckpointNumber;
1417-
blockNumber: BlockNumber;
1418-
slotNumber: SlotNumber;
1419-
}): ProposedCheckpointData {
1420-
return {
1421-
checkpointNumber: args.checkpointNumber,
1422-
header: CheckpointHeader.random({ slotNumber: args.slotNumber }),
1423-
archive: AppendOnlyTreeSnapshot.empty(),
1424-
checkpointOutHash: Fr.ZERO,
1425-
startBlock: args.blockNumber,
1426-
blockCount: 1,
1427-
totalManaUsed: 0n,
1428-
feeAssetPriceModifier: 0n,
1429-
};
1430-
}
1431-
14321205
describe('getCheckpoint', () => {
14331206
/** Builds a minimal ProposedCheckpointData stub. */
14341207
function makeProposedCheckpointData(

0 commit comments

Comments
 (0)