Skip to content

Commit f4dd802

Browse files
authored
Merge pull request #3250 from ProvableHQ/mohammadfawaz/query_doc_followup
docs/test(query): qualify Stack snapshot claim + add name-collision tests
2 parents 6440df1 + 8ce234b commit f4dd802

3 files changed

Lines changed: 74 additions & 27 deletions

File tree

synthesizer/process/src/query.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,17 @@ impl<N: Network> Process<N> {
5555
}
5656

5757
/// Evaluates a query function against historic finalize-store state at the given block
58-
/// `height`. Mapping reads route through `FinalizeStore::get_historical_mapping_value`, which
59-
/// reconstructs the value applicable at `height` from the per-key update log (a confirmed-only
60-
/// table — entries are only written during finalize, never modified). Pinning every read to
61-
/// `height` therefore gives true snapshot semantics: block production advancing past `height`
62-
/// during evaluation cannot disturb the result, and we don't need to take an atomic batch on
63-
/// the store (so this path doesn't contend with block finalization either).
58+
/// `height`. Mapping reads route through `FinalizeStore::get_historical_mapping_value` and
59+
/// are snapshot-consistent at `height` without contending with block finalization.
6460
///
65-
/// `state` is supplied by the caller — typically `VM::evaluate_query_at_height` constructs it
66-
/// from the historic block at `height` so query operands reading block metadata
67-
/// (`block.height`, `block.timestamp`, the random seed) reflect that block.
61+
/// `state` is supplied by the caller — typically built from the historic block at `height`
62+
/// so block-metadata operands reflect that block.
6863
///
69-
/// Available only when snarkVM is built with `--features history`. snarkOS calls this with
70-
/// `current_block_height()` for "latest", or any earlier height for historic queries.
64+
/// Caveat: the live `Stack` has interior mutability, so a concurrent redeploy of the same
65+
/// program could perturb its structural caches mid-query. Mapping values are pinned at
66+
/// `height`; program structure is not. Known gap — see `VM::evaluate_query_at_height`.
67+
///
68+
/// Available only with `--features history`.
7169
pub fn evaluate_query_at_height<N: Network, P: FinalizeStorage<N>>(
7270
state: FinalizeGlobalState,
7371
store: &FinalizeStore<N, P>,

synthesizer/program/src/parse.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,61 @@ query uses_call:
333333
assert!(result.is_err(), "expected program parse to fail when query contains 'call'");
334334
}
335335

336+
#[test]
337+
fn test_program_rejects_duplicate_query_names() {
338+
let result = Program::<CurrentNetwork>::from_str(
339+
r"
340+
program dup_query.aleo;
341+
342+
query foo:
343+
add 0u64 1u64 into r0;
344+
output r0 as u64.public;
345+
346+
query foo:
347+
add 0u64 2u64 into r0;
348+
output r0 as u64.public;",
349+
);
350+
assert!(result.is_err(), "expected program parse to fail with two queries named 'foo'");
351+
}
352+
353+
#[test]
354+
fn test_program_rejects_query_name_colliding_with_function() {
355+
let result = Program::<CurrentNetwork>::from_str(
356+
r"
357+
program qf_collision.aleo;
358+
359+
function foo:
360+
input r0 as u64.private;
361+
output r0 as u64.private;
362+
363+
query foo:
364+
add 0u64 1u64 into r0;
365+
output r0 as u64.public;",
366+
);
367+
assert!(result.is_err(), "expected program parse to fail when a query reuses a function name");
368+
}
369+
370+
#[test]
371+
fn test_program_rejects_query_name_colliding_with_closure() {
372+
let result = Program::<CurrentNetwork>::from_str(
373+
r"
374+
program qc_collision.aleo;
375+
376+
closure foo:
377+
input r0 as field;
378+
output r0 as field;
379+
380+
function noop:
381+
input r0 as u64.private;
382+
output r0 as u64.private;
383+
384+
query foo:
385+
add 0u64 1u64 into r0;
386+
output r0 as u64.public;",
387+
);
388+
assert!(result.is_err(), "expected program parse to fail when a query reuses a closure name");
389+
}
390+
336391
#[test]
337392
fn test_program_parse_function_zero_inputs() -> Result<()> {
338393
// Initialize a new program.

synthesizer/src/vm/mod.rs

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -331,25 +331,19 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
331331
}
332332

333333
/// Evaluates a query function against finalize-store state at the given block `height`.
334-
/// Returns the typed outputs (no `(height, outputs)` tuple — the caller already supplied
335-
/// the height).
334+
/// Returns the typed outputs.
336335
///
337-
/// All mapping reads are pinned to `height` via the finalize store's per-key historical
338-
/// update map (entries at any given height are immutable once written), so block production
339-
/// advancing past `height` mid-evaluation cannot disturb the result. The
340-
/// `FinalizeGlobalState` is also reconstructed from the block at `height`, so query operands
341-
/// reading block metadata see that block's values.
336+
/// Mapping reads are pinned to `height` via the per-key historical update map, and the
337+
/// `FinalizeGlobalState` is reconstructed from the block at `height`. Available only with
338+
/// `--features history`.
342339
///
343-
/// snarkOS calls this with `current_block_height()` for "latest" semantics, or any earlier
344-
/// height for historic queries. Available only when snarkVM is built with `--features history`
345-
/// — the per-height update map that pins reads is only populated under that feature.
340+
/// snarkOS calls this with `current_block_height()` for "latest", or any earlier height
341+
/// for historic queries. `height` must satisfy `height <= current_block_height()`.
346342
///
347-
/// `height` must satisfy `height <= current_block_height()`. Reading a future height
348-
/// returns "no block exists" rather than a misleading None.
349-
///
350-
/// Concurrency: this call does NOT take `self.process.lock()` (which would serialize
351-
/// queries against block production); it relies on `Arc<Stack<N>>` immutability and on the
352-
/// fact that historic-table entries are immutable.
343+
/// Caveat: the `Stack` itself uses interior mutability, so a concurrent redeploy of the
344+
/// same program could perturb its structural caches mid-query. Mapping values are
345+
/// snapshot-consistent at `height`; program structure is not. Known gap; a future
346+
/// `StackSnapshot`-style fix would close it.
353347
#[cfg(feature = "history")]
354348
#[inline]
355349
pub fn evaluate_query_at_height(

0 commit comments

Comments
 (0)