Skip to content

Commit 52c4d30

Browse files
authored
feat: merge-train/spartan-v5 (#24851)
BEGIN_COMMIT_OVERRIDE fix(foundation): always include a result key in json-rpc responses (#24840) feat(prover-node): make epoch proving robust to prune-induced fork faults (#24678) END_COMMIT_OVERRIDE
2 parents def7152 + 6795246 commit 52c4d30

15 files changed

Lines changed: 1018 additions & 347 deletions

yarn-project/end-to-end/src/single-node/proving/upload_failed_proof.test.ts

Lines changed: 71 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { Logger } from '@aztec/aztec.js/log';
44
import { tryRmDir } from '@aztec/foundation/fs';
55
import { promiseWithResolvers } from '@aztec/foundation/promise';
66
import { sleep } from '@aztec/foundation/sleep';
7-
import { downloadEpochProvingJob, rerunEpochProvingJob } from '@aztec/prover-node';
7+
import { downloadEpochProvingJob, rerunCheckpointProvingJob, rerunEpochProvingJob } from '@aztec/prover-node';
88
import type { TestProverNode } from '@aztec/prover-node/test';
99

1010
import { jest } from '@jest/globals';
@@ -61,11 +61,11 @@ describe('single-node/proving/upload_failed_proof', () => {
6161
await tryRmDir(rerunDownloadDir, logger);
6262
});
6363

64-
// Makes the prover's top-tree prove always throw (v5 uses the session's topTreeProveOverride hook;
65-
// pre-v5 it patched finalizeEpoch), intercepts tryUploadSessionFailure (pre-v5 tryUploadEpochFailure)
66-
// to capture the upload URL, then waits for epoch 1 to start and for the upload to complete. Tears
67-
// down the live context, downloads the proving job data, and re-runs it via rerunEpochProvingJob with
68-
// fake proofs on a fresh config.
64+
// Makes the prover's top-tree prove always throw. Because every checkpoint prover still succeeds, the
65+
// session fails on its own account (state 'failed'), which the session manager treats as a genuine,
66+
// race-free failure and uploads a post-mortem eagerly via tryUploadEpochFailure(sessionId, checkpoints).
67+
// Intercepts that to capture the upload URL, then tears down the live context, downloads the proving
68+
// job data, and re-runs it via rerunEpochProvingJob with fake proofs on a fresh config.
6969
it('uploads failed proving job state and re-runs it on a fresh instance', async () => {
7070
// Make initial prover node fail to prove, via the session's top-tree-prove hook.
7171
const proverNode = test.proverNodes[0].getProverNode() as TestProverNode;
@@ -77,19 +77,20 @@ describe('single-node/proving/upload_failed_proof', () => {
7777
},
7878
});
7979

80-
// And track when the epoch failure upload is complete
80+
// Track when the epoch failure upload is complete. It fires eagerly when a full session fails with
81+
// healthy provers (a top-tree failure here).
8182
const { promise: epochUploaded, resolve: onEpochUploaded } = promiseWithResolvers<string>();
82-
const origTryUploadEpochFailure = proverNode.tryUploadSessionFailure.bind(proverNode);
83-
proverNode.tryUploadSessionFailure = async (session: any) => {
84-
const url = await origTryUploadEpochFailure(session);
83+
const origTryUploadEpochFailure = proverNode.tryUploadEpochFailure.bind(proverNode);
84+
proverNode.tryUploadEpochFailure = async (id: any, checkpoints: any) => {
85+
const url = await origTryUploadEpochFailure(id, checkpoints);
8586
if (url !== undefined) {
8687
onEpochUploaded(url);
8788
}
8889
return url;
8990
};
9091

91-
// Warp to the start of epoch one so prover node starts proving epoch 0,
92-
// and wait for the data to be uploaded to the remote file store
92+
// Warp to the start of epoch one so the prover node starts proving, fails at the top tree, and
93+
// uploads the failed proving-job data to the remote file store.
9394
await test.warpToEpochStart(1);
9495
const epochUploadUrl = await epochUploaded;
9596

@@ -122,4 +123,62 @@ describe('single-node/proving/upload_failed_proof', () => {
122123

123124
logger.info(`Test succeeded`);
124125
});
126+
127+
// Same shape as above, one level down: forces a single checkpoint's sub-tree to fail (every checkpoint
128+
// prover fails, via the test hook), which uploads that checkpoint's proving data on its own. Intercepts
129+
// tryUploadCheckpointFailure for the URL, then downloads and re-proves just that checkpoint via
130+
// rerunCheckpointProvingJob (no epoch top-tree / L1 submit).
131+
it('uploads failed checkpoint proving state and re-proves it on a fresh instance', async () => {
132+
const proverNode = test.proverNodes[0].getProverNode() as TestProverNode;
133+
proverNode.setCheckpointHooks({
134+
checkpointProveOverride: async () => {
135+
await sleep(1000);
136+
logger.warn(`Triggering error on checkpoint sub-tree prove`);
137+
throw new Error(`Fake error while proving checkpoint`);
138+
},
139+
});
140+
141+
// The checkpoint post-mortem upload fires eagerly when a CheckpointProver's block proofs reject.
142+
const { promise: checkpointUploaded, resolve: onCheckpointUploaded } = promiseWithResolvers<string>();
143+
const origTryUploadCheckpointFailure = proverNode.tryUploadCheckpointFailure.bind(proverNode);
144+
proverNode.tryUploadCheckpointFailure = async (prover: any) => {
145+
const url = await origTryUploadCheckpointFailure(prover);
146+
if (url !== undefined) {
147+
onCheckpointUploaded(url);
148+
}
149+
return url;
150+
};
151+
152+
// Warp so the prover node starts registering checkpoints; the first one's sub-tree fails and uploads.
153+
await test.warpToEpochStart(1);
154+
const checkpointUploadUrl = await checkpointUploaded;
155+
156+
await test.teardown();
157+
158+
const rerunDownloadPath = join(rerunDownloadDir, 'data.bin');
159+
logger.warn(`Downloading checkpoint proving job data and state`, { rerunDataDir, rerunDownloadPath });
160+
await downloadEpochProvingJob(checkpointUploadUrl, logger, {
161+
dataDirectory: rerunDataDir,
162+
jobDataDownloadPath: rerunDownloadPath,
163+
});
164+
165+
logger.warn(`Rerunning checkpoint proving job from ${rerunDownloadPath}`);
166+
await rerunCheckpointProvingJob(
167+
rerunDownloadPath,
168+
logger,
169+
{
170+
...config,
171+
realProofs: false,
172+
dataStoreMapSizeKb: 1024 * 1024,
173+
dataDirectory: rerunDataDir,
174+
proverAgentCount: 2,
175+
proverId: EthAddress.random(),
176+
...(await getACVMConfig(logger)),
177+
...(await getBBConfig(logger)),
178+
},
179+
context.genesis,
180+
);
181+
182+
logger.info(`Test succeeded`);
183+
});
125184
});

yarn-project/foundation/src/json-rpc/server/safe_json_rpc_server.test.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,17 @@ describe('SafeJsonRpcServer', () => {
7676
it('calls an RPC function with no inputs nor outputs', async () => {
7777
const response = await send({ method: 'clear', params: [] });
7878
expect(response.status).toBe(200);
79-
expect(response.text).toEqual(JSON.stringify({ jsonrpc }));
79+
expect(response.text).toEqual(JSON.stringify({ jsonrpc, result: null }));
8080
expect(testState.notes).toEqual([]);
8181
});
8282

83+
it('returns an explicit null result (not an omitted field) when a handler returns undefined', async () => {
84+
const response = await send({ method: 'getNote', params: [99] });
85+
expect(response.status).toBe(200);
86+
expect(response.text).toEqual(JSON.stringify({ jsonrpc, result: null }));
87+
expect(JSON.parse(response.text)).toHaveProperty('result', null);
88+
});
89+
8390
it('calls an RPC function that returns a primitive object and a bigint', async () => {
8491
const response = await send({ method: 'getStatus', params: [] });
8592
expect(response.status).toBe(200);
@@ -185,7 +192,7 @@ describe('SafeJsonRpcServer', () => {
185192
expect(resp.text).toEqual(
186193
JSON.stringify([
187194
{ jsonrpc: '2.0', id: 42, result: { status: 'ok', count: '2' } },
188-
{ jsonrpc: '2.0', id: 43 },
195+
{ jsonrpc: '2.0', id: 43, result: null },
189196
]),
190197
);
191198
});
@@ -210,7 +217,7 @@ describe('SafeJsonRpcServer', () => {
210217
expect(resp.text).toEqual(
211218
JSON.stringify([
212219
{ jsonrpc: '2.0', id: 42, error: { code: -32601, message: 'Method not found: toString' } },
213-
{ jsonrpc: '2.0', id: 43 },
220+
{ jsonrpc: '2.0', id: 43, result: null },
214221
]),
215222
);
216223
});
@@ -221,7 +228,7 @@ describe('SafeJsonRpcServer', () => {
221228
expect(resp.status).toEqual(200);
222229
expect(resp.text).toEqual(
223230
JSON.stringify([
224-
{ jsonrpc: '2.0', id: 43 },
231+
{ jsonrpc: '2.0', id: 43, result: null },
225232
{ jsonrpc: '2.0', error: { code: -32600, message: 'Invalid Request' }, id: null },
226233
]),
227234
);

yarn-project/foundation/src/json-rpc/server/safe_json_rpc_server.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,11 @@ export class SafeJsonRpcServer {
224224
result = await this.proxy.call(method, params);
225225
}
226226

227-
return { jsonrpc, id, result };
227+
// Coerce an undefined return value to null so the response always carries a `result` key.
228+
// JSON.stringify drops undefined-valued keys, which would otherwise produce a JSON-RPC
229+
// response with neither `result` nor `error` — a spec violation that leaves callers unable
230+
// to distinguish "not found" from a malformed response.
231+
return { jsonrpc, id, result: result ?? null };
228232
} catch (err: any) {
229233
if (err && err instanceof ZodError) {
230234
const message = err.issues.map(e => `${e.message} (${e.path.join('.')})`).join('. ') || 'Validation error';

yarn-project/foundation/src/json-rpc/test/integration.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { JsonRpcFetch } from '../client/fetch.js';
12
import { createSafeJsonRpcClient } from '../client/safe_json_rpc_client.js';
23
import { TestNote, TestState, type TestStateApi, TestStateSchema } from '../fixtures/test_state.js';
34
import { startHttpRpcServer } from '../server/safe_json_rpc_server.js';
@@ -125,6 +126,37 @@ describe('JsonRpc integration', () => {
125126
});
126127
});
127128

129+
describe('null results against optional schemas', () => {
130+
let client: TestStateApi;
131+
let url: string;
132+
133+
beforeEach(async () => {
134+
({ server, httpServer, client, url } = await createJsonRpcTestSetup(testState, TestStateSchema));
135+
});
136+
137+
it('client accepts the explicit null result the server sends for an undefined return', async () => {
138+
const response = await fetch(url, {
139+
method: 'POST',
140+
headers: { 'content-type': 'application/json' },
141+
body: JSON.stringify({ jsonrpc: '2.0', id: 0, method: 'getNote', params: [10] }),
142+
});
143+
expect(await response.json()).toEqual({ jsonrpc: '2.0', id: 0, result: null });
144+
145+
await expect(client.getNote(10)).resolves.toBeUndefined();
146+
});
147+
148+
it('client resolves undefined when any server returns null for an optional result schema', async () => {
149+
const nullResultFetch: JsonRpcFetch = (_host, body) =>
150+
Promise.resolve({
151+
response: body.map((req: { id: number }) => ({ jsonrpc: '2.0', id: req.id, result: null })),
152+
headers: { get: () => undefined },
153+
});
154+
const nullClient = createSafeJsonRpcClient<TestStateApi>(url, TestStateSchema, { fetch: nullResultFetch });
155+
156+
await expect(nullClient.getNote(0)).resolves.toBeUndefined();
157+
});
158+
});
159+
128160
describe('namespaced', () => {
129161
let lettersState: TestState;
130162
let numbersState: TestState;

0 commit comments

Comments
 (0)