Skip to content

Commit d259113

Browse files
committed
fix(archiver): resolve L2-to-L1 witness from a single store snapshot
getL2ToL1MembershipWitness assembled its witness from several non-atomic archiver reads (getTxEffect, then the epoch blocks, target block and checkpoint metadata read inside computeL2ToL1MembershipWitness). A store commit landing between those reads could splice together two different chain states and surface a spurious "message does not exist" error even when the tx-effect index was healthy. Wrap the witness assembly in a single store.transactionAsync so every archiver read binds to one write transaction and observes a consistent snapshot (the store serializes writers, so no commit can interleave). The L1 Outbox roots fetch stays outside the transaction to avoid holding the archiver's writer lock across a network round-trip, and the tx effect is re-read inside the snapshot so the receipt's block number and tx index stay consistent with the block data. Add a kv-store test asserting reads inside a transaction see a consistent snapshot while a concurrent write is queued behind it.
1 parent 632095c commit d259113

4 files changed

Lines changed: 87 additions & 15 deletions

File tree

yarn-project/archiver/src/archiver.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ export class Archiver extends ArchiverDataSourceBase implements L2BlockSink, Tra
203203
this.outboxTreesResolver = new OutboxTreesResolver(
204204
this.outbox,
205205
this,
206+
this.dataStores.db,
206207
() => Promise.resolve(this.getL1BlockNumber()),
207208
l1Constants.epochDuration,
208209
);

yarn-project/archiver/src/modules/outbox_trees_resolver.test.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
} from '@aztec/foundation/branded-types';
1010
import { sha256Trunc } from '@aztec/foundation/crypto/sha256';
1111
import { Fr } from '@aztec/foundation/curves/bn254';
12+
import type { AztecAsyncKVStore } from '@aztec/kv-store';
1213
import { type L2ToL1MembershipWitness, computeEpochOutHash } from '@aztec/stdlib/messaging';
1314
import { TxHash } from '@aztec/stdlib/tx';
1415

@@ -41,7 +42,13 @@ describe('OutboxTreesResolver lazy roots cache', () => {
4142
return Promise.resolve(makeZeroRoots());
4243
});
4344

44-
resolver = new OutboxTreesResolver(outbox, archiver, () => Promise.resolve(syncedL1Block), EPOCH_DURATION);
45+
resolver = new OutboxTreesResolver(
46+
outbox,
47+
archiver,
48+
makeFakeStore(),
49+
() => Promise.resolve(syncedL1Block),
50+
EPOCH_DURATION,
51+
);
4552
});
4653

4754
// Exercises the private #getRoots path through a witness request whose epoch has no blocks,
@@ -83,6 +90,7 @@ describe('OutboxTreesResolver lazy roots cache', () => {
8390
const notSyncedResolver = new OutboxTreesResolver(
8491
outbox,
8592
archiver,
93+
makeFakeStore(),
8694
() => Promise.resolve(undefined),
8795
EPOCH_DURATION,
8896
);
@@ -114,7 +122,7 @@ describe('OutboxTreesResolver witness building', () => {
114122
outbox = mock<OutboxContract>();
115123
archiver = mock<OutboxTreesArchiverView>();
116124
// Witness tests fetch the covering roots once at synced block 0 via the outbox mock.
117-
resolver = new OutboxTreesResolver(outbox, archiver, () => Promise.resolve(0n), EPOCH_DURATION);
125+
resolver = new OutboxTreesResolver(outbox, archiver, makeFakeStore(), () => Promise.resolve(0n), EPOCH_DURATION);
118126
});
119127

120128
it('returns undefined when the tx is not yet in a block', async () => {
@@ -216,6 +224,7 @@ describe('OutboxTreesResolver witness building', () => {
216224
const drifting = new OutboxTreesResolver(
217225
outbox,
218226
archiver,
227+
makeFakeStore(),
219228
() => Promise.resolve(BigInt(syncedCalls++)),
220229
EPOCH_DURATION,
221230
);
@@ -226,6 +235,17 @@ describe('OutboxTreesResolver witness building', () => {
226235

227236
// --- Helpers ----------------------------------------------------------------
228237

238+
/**
239+
* Store double whose `transactionAsync` just runs the callback. The archiver view is mocked here, so
240+
* there is no real snapshot to isolate; the resolver only needs the callback invoked. Snapshot
241+
* consistency of the real store is covered by the kv-store transaction tests.
242+
*/
243+
function makeFakeStore(): AztecAsyncKVStore {
244+
const store = mock<AztecAsyncKVStore>();
245+
store.transactionAsync.mockImplementation(callback => callback());
246+
return store;
247+
}
248+
229249
function makeZeroRoots(): Fr[] {
230250
return Array.from({ length: MAX_CHECKPOINTS_PER_EPOCH }, () => Fr.ZERO);
231251
}

yarn-project/archiver/src/modules/outbox_trees_resolver.ts

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { BlockNumber, EpochNumber } from '@aztec/foundation/branded-types';
33
import { chunkBy } from '@aztec/foundation/collection';
44
import type { Fr } from '@aztec/foundation/curves/bn254';
55
import { type Logger, createLogger } from '@aztec/foundation/log';
6+
import type { AztecAsyncKVStore } from '@aztec/kv-store';
67
import type { L2Block } from '@aztec/stdlib/block';
78
import type { CheckpointData } from '@aztec/stdlib/checkpoint';
89
import { getEpochAtSlot } from '@aztec/stdlib/epoch-helpers';
@@ -56,6 +57,7 @@ export class OutboxTreesResolver {
5657
constructor(
5758
private readonly outbox: OutboxContract,
5859
private readonly archiver: OutboxTreesArchiverView,
60+
private readonly store: AztecAsyncKVStore,
5961
private readonly getSyncedL1BlockNumber: () => Promise<bigint | undefined>,
6062
private readonly epochDuration: number,
6163
private readonly log: Logger = createLogger('archiver:outbox'),
@@ -73,6 +75,9 @@ export class OutboxTreesResolver {
7375
message: Fr,
7476
messageIndexInTx?: number,
7577
): Promise<L2ToL1MembershipWitness | undefined> {
78+
// Read the tx effect once up front only to learn which epoch's Outbox roots to fetch. The roots
79+
// read hits L1, so it stays outside the store transaction below: holding the archiver's writer
80+
// lock across a network round-trip would stall block sync.
7681
const indexed = await this.archiver.getTxEffect(txHash);
7782
if (!indexed) {
7883
this.log.trace(`No tx effect for tx, no witness available`, { txHash });
@@ -87,23 +92,39 @@ export class OutboxTreesResolver {
8792
return undefined;
8893
}
8994

90-
const receipt = {
91-
txHash,
92-
epochNumber,
93-
blockNumber: indexed.l2BlockNumber,
94-
txIndexInBlock: indexed.txIndexInBlock,
95-
};
96-
9795
const syncedBefore = await this.getSyncedL1BlockNumber();
9896
try {
99-
return await computeL2ToL1MembershipWitness(this.#node, roots, message, receipt, messageIndexInTx);
97+
// Assemble the witness inside a single store transaction so every archiver read it depends on
98+
// (the tx effect, the epoch's blocks, the target block and the checkpoint metadata) observes one
99+
// consistent snapshot. `transactionAsync` serializes against writers, and the substore reads
100+
// below bind to it automatically, so no store commit can land mid-assembly and splice together
101+
// two different chain states — which would otherwise surface as a spurious "message does not
102+
// exist" error even when the tx-effect index is healthy.
103+
return await this.store.transactionAsync(async () => {
104+
// Re-read the tx effect within the snapshot so the receipt's block number and tx index stay
105+
// consistent with the block and checkpoint data read while building the witness. The epoch is
106+
// reused from the pre-transaction read (it selected the roots above); if a reorg moved the tx
107+
// to a different epoch in between, the block lookups simply miss and the helper returns
108+
// undefined for the caller to retry.
109+
const indexedInSnapshot = await this.archiver.getTxEffect(txHash);
110+
if (!indexedInSnapshot) {
111+
return undefined;
112+
}
113+
const receipt = {
114+
txHash,
115+
epochNumber,
116+
blockNumber: indexedInSnapshot.l2BlockNumber,
117+
txIndexInBlock: indexedInSnapshot.txIndexInBlock,
118+
};
119+
return await computeL2ToL1MembershipWitness(this.#node, roots, message, receipt, messageIndexInTx);
120+
});
100121
} catch (err) {
101122
// The helper throws if the locally-assembled epoch root disagrees with the cached Outbox root.
102-
// The cached roots are pinned to the node's synced L1 block, but the helper reads block /
103-
// checkpoint data live, so if the archiver advanced mid-assembly the two can momentarily reflect
104-
// different heights and produce a transient mismatch. When the synced block moved during
105-
// assembly we treat the mismatch as transient and return undefined so the caller can retry; a
106-
// mismatch at a stable synced block is a genuine node/L1 disagreement and must surface.
123+
// The cached roots are pinned to the node's synced L1 block, but the block / checkpoint data is
124+
// read from a store snapshot taken after that fetch, so if the archiver advanced in between the
125+
// two can reflect different heights and produce a transient mismatch. When the synced block moved
126+
// during assembly we treat the mismatch as transient and return undefined so the caller can
127+
// retry; a mismatch at a stable synced block is a genuine node/L1 disagreement and must surface.
107128
const syncedAfter = await this.getSyncedL1BlockNumber();
108129
if (syncedBefore !== syncedAfter && err instanceof Error && err.message.includes('does not match Outbox')) {
109130
this.log.debug(`Transient outbox root mismatch during sync, returning no witness`, {

yarn-project/kv-store/src/lmdb-v2/store.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,36 @@ describe('AztecLMDBStoreV2', () => {
115115
});
116116
});
117117

118+
it('gives reads inside a transaction a consistent snapshot while a concurrent write is queued', async () => {
119+
const map = store.openMap<string, string>('snapshot');
120+
await map.set('k', 'v1');
121+
122+
const started = promiseWithResolvers<void>();
123+
const release = promiseWithResolvers<void>();
124+
125+
const snapshotReads = store.transactionAsync(async () => {
126+
const first = await map.getAsync('k');
127+
started.resolve();
128+
await release.promise;
129+
const second = await map.getAsync('k');
130+
return [first, second];
131+
});
132+
133+
await started.promise;
134+
// Queue a competing write for the same key. The single writer serializes it behind the in-flight
135+
// transaction, so it cannot commit until that transaction finishes.
136+
const concurrentWrite = map.set('k', 'v2');
137+
release.resolve();
138+
139+
const [first, second] = await snapshotReads;
140+
await concurrentWrite;
141+
142+
// Both reads inside the transaction observe the pre-write value; the queued write only lands after.
143+
expect(first).toBe('v1');
144+
expect(second).toBe('v1');
145+
expect(await map.getAsync('k')).toBe('v2');
146+
});
147+
118148
it('should serialize writes correctly', async () => {
119149
const key = Buffer.from('foo');
120150
const inc = () =>

0 commit comments

Comments
 (0)