From 893791b912ef8d8ab8fb21f4e5c4530dd51af824 Mon Sep 17 00:00:00 2001 From: sergerad Date: Mon, 10 Aug 2026 13:23:42 +1200 Subject: [PATCH] Incremental account codes pruning --- .../005_incremental_code_pruning.sql | 19 +++ crates/store/src/db/migrations/tests/mod.rs | 3 +- .../store/src/db/models/queries/accounts.rs | 88 ++++++++-- .../src/db/models/queries/accounts/tests.rs | 160 +++++++++++++++++- crates/store/src/db/schema.rs | 8 + 5 files changed, 261 insertions(+), 17 deletions(-) create mode 100644 crates/store/src/db/migrations/005_incremental_code_pruning.sql diff --git a/crates/store/src/db/migrations/005_incremental_code_pruning.sql b/crates/store/src/db/migrations/005_incremental_code_pruning.sql new file mode 100644 index 000000000..a346723a1 --- /dev/null +++ b/crates/store/src/db/migrations/005_incremental_code_pruning.sql @@ -0,0 +1,19 @@ +-- Make account-code pruning churn-driven. Previously each prune re-scanned every account row still +-- valid past the cutoff. A code pinned at one prune can only become collectable at a later prune if +-- its maximal referencing interval ends between the two cutoffs, so it suffices to scan rows whose +-- `valid_until` crossed the cutoff since the previous prune and probe each candidate code for a +-- surviving reference. + +-- Existence probe for candidate codes: for a given `code_commitment`, is there any row whose +-- validity interval reaches past the cutoff (`valid_until > cutoff`)? +CREATE INDEX idx_accounts_code_probe + ON accounts(code_commitment, valid_until) + WHERE code_commitment IS NOT NULL; + +-- Single-row (id = 0) record of the cutoff through which account-code pruning has completed. +-- Updated in the same transaction as the prune itself, so it is exact and crash-consistent. The +-- row is absent until the first prune under this schema, which runs a full (non-windowed) pass. +CREATE TABLE prune_progress ( + id INTEGER NOT NULL PRIMARY KEY CHECK (id = 0), + codes_cutoff BIGINT NOT NULL +); diff --git a/crates/store/src/db/migrations/tests/mod.rs b/crates/store/src/db/migrations/tests/mod.rs index ddb9c034e..9584a32f7 100644 --- a/crates/store/src/db/migrations/tests/mod.rs +++ b/crates/store/src/db/migrations/tests/mod.rs @@ -10,11 +10,12 @@ use super::*; use crate::db::models::queries::VALID_FOREVER; use crate::db::schema; -const EXPECTED_SCHEMA_HASHES: [SchemaHash; 4] = [ +const EXPECTED_SCHEMA_HASHES: [SchemaHash; 5] = [ SchemaHash::from_hex("cc92cb332410e6f63036b52cf953acb446c142d5c0fbbdbd6d3b4f466510b210"), SchemaHash::from_hex("7c783947d0bb2c9745d28f4bdcf329f84ad970c36aa07ea85441e62718d8bbbb"), SchemaHash::from_hex("e026a70464e897ae9a217f45c80d72341b1bfb757200e57e41145348473a9961"), SchemaHash::from_hex("a581a13b00e4aa1d4539459e2b351c0585fad33c5a876f830c9b943adac92dea"), + SchemaHash::from_hex("34bd293251a2647715dd91fa245bcd98d635e8070871b4f8335b3a3db364fc1e"), ]; #[test] diff --git a/crates/store/src/db/models/queries/accounts.rs b/crates/store/src/db/models/queries/accounts.rs index 24d644224..e6ac5afe0 100644 --- a/crates/store/src/db/models/queries/accounts.rs +++ b/crates/store/src/db/models/queries/accounts.rs @@ -1736,8 +1736,21 @@ fn prune_account_storage_map_values( /// inside the window, all open-ended (current) rows, and each account's baseline row — the row /// still valid at the cutoff even though it was written before it. /// -/// The forced `idx_accounts_code_validity` covering index keeps the subquery an index-only range -/// scan, sized by rows valid at or after the cutoff rather than total history. +/// The prune is churn-driven: a code pinned at the previous prune (some row with +/// `valid_until > prev_cutoff` referenced it) can only become collectable now if the row holding +/// its maximal `valid_until` expired inside `(prev_cutoff, cutoff_block]`. Candidate codes are +/// therefore collected from that window — an `idx_accounts_code_validity` range scan sized by +/// churn since the previous prune — and each is deleted only if the `idx_accounts_code_probe` +/// existence probe finds no row still referencing it past the cutoff. The previous cutoff is +/// persisted in `prune_progress` within the same transaction; when absent (first prune after +/// migration, or a fresh database) a full pass over all rows valid past the cutoff runs instead. +/// +/// Correctness of the windowed candidate set rests on two invariants: +/// - Rows are only ever closed to the `block_num` of the block currently being applied, which is +/// always above the cutoff, so every expiry crosses the window of some later prune. A write path +/// that back-dated `valid_until` below the current cutoff would leak the code forever. +/// - Every `account_codes` row is inserted alongside an `accounts` row referencing it (see +/// [`upsert_accounts`]); an orphan code with no referencing row would never become a candidate. #[miden_instrument( target = COMPONENT, err, @@ -1751,16 +1764,63 @@ fn prune_account_codes( ) -> Result { use diesel::sql_types::BigInt; - diesel::sql_query( - "DELETE FROM account_codes \ - WHERE code_commitment NOT IN ( \ - SELECT DISTINCT code_commitment \ - FROM accounts INDEXED BY idx_accounts_code_validity \ - WHERE code_commitment IS NOT NULL \ - AND valid_until > ?1 \ - )", - ) - .bind::(cutoff_block) - .execute(conn) - .map_err(DatabaseError::Diesel) + let prev_cutoff: Option = + SelectDsl::select(schema::prune_progress::table, schema::prune_progress::codes_cutoff) + .first(conn) + .optional() + .map_err(DatabaseError::Diesel)?; + + let deleted = match prev_cutoff { + // Codes are already pruned through this cutoff and nothing can become collectable while the + // cutoff stands still, so skip without moving the marker backwards. + Some(prev_cutoff) if prev_cutoff >= cutoff_block => return Ok(0), + Some(prev_cutoff) => diesel::sql_query( + "DELETE FROM account_codes \ + WHERE code_commitment IN ( \ + SELECT DISTINCT code_commitment \ + FROM accounts INDEXED BY idx_accounts_code_validity \ + WHERE code_commitment IS NOT NULL \ + AND valid_until > ?1 \ + AND valid_until <= ?2 \ + ) \ + AND NOT EXISTS ( \ + SELECT 1 \ + FROM accounts INDEXED BY idx_accounts_code_probe \ + WHERE accounts.code_commitment = account_codes.code_commitment \ + AND accounts.valid_until > ?2 \ + )", + ) + .bind::(prev_cutoff) + .bind::(cutoff_block) + .execute(conn) + .map_err(DatabaseError::Diesel)?, + // No recorded cutoff: full pass. The forced `idx_accounts_code_validity` covering index + // keeps the subquery an index-only range scan, sized by rows valid at or after the cutoff + // rather than total history. + None => diesel::sql_query( + "DELETE FROM account_codes \ + WHERE code_commitment NOT IN ( \ + SELECT DISTINCT code_commitment \ + FROM accounts INDEXED BY idx_accounts_code_validity \ + WHERE code_commitment IS NOT NULL \ + AND valid_until > ?1 \ + )", + ) + .bind::(cutoff_block) + .execute(conn) + .map_err(DatabaseError::Diesel)?, + }; + + diesel::insert_into(schema::prune_progress::table) + .values(( + schema::prune_progress::id.eq(0), + schema::prune_progress::codes_cutoff.eq(cutoff_block), + )) + .on_conflict(schema::prune_progress::id) + .do_update() + .set(schema::prune_progress::codes_cutoff.eq(cutoff_block)) + .execute(conn) + .map_err(DatabaseError::Diesel)?; + + Ok(deleted) } diff --git a/crates/store/src/db/models/queries/accounts/tests.rs b/crates/store/src/db/models/queries/accounts/tests.rs index 282c94663..60f0a898b 100644 --- a/crates/store/src/db/models/queries/accounts/tests.rs +++ b/crates/store/src/db/models/queries/accounts/tests.rs @@ -1287,6 +1287,13 @@ fn make_full_state_update(account: &Account) -> BlockAccountUpdate { /// The `push_value` must be different for each variant to produce a distinct MAST root and thus a /// distinct [`AccountCode::commitment`]. fn build_account_with_code(push_value: u32) -> Account { + // Seed [2u8; 32] keeps the account ID distinct from the other test helpers. + build_account_with_code_seeded(push_value, [2u8; 32]) +} + +/// Same as [`build_account_with_code`] but with a caller-chosen ID seed, for tests that need +/// multiple distinct accounts sharing the same code. +fn build_account_with_code_seeded(push_value: u32, seed: [u8; 32]) -> Account { let code_src = format!("@account_procedure pub proc variant push.{push_value} end"); let component_code = CodeBuilder::default() .compile_component_code("test::code_prune", &code_src) @@ -1301,8 +1308,7 @@ fn build_account_with_code(push_value: u32) -> Account { ) .unwrap(); - // Seed [2u8; 32] keeps the account ID distinct from the other test helpers. - AccountBuilder::new([2u8; 32]) + AccountBuilder::new(seed) .account_type(AccountType::Public) .with_component(component) .with_component(AuthSingleSig::new(Approver::new( @@ -1558,6 +1564,156 @@ fn test_prune_account_code_retains_baseline_code() { ); } +/// Returns the cutoff recorded in `prune_progress`, if any. +fn codes_prune_cutoff(conn: &mut SqliteConnection) -> Option { + SelectDsl::select(schema::prune_progress::table, schema::prune_progress::codes_cutoff) + .first(conn) + .optional() + .expect("Failed to query prune_progress") +} + +/// Prune test 5: the incremental (windowed) codes prune must not delete a code whose expiring +/// reference crosses the cutoff window while another account still references it, and must delete +/// it once the last reference expires in a later window. +#[test] +fn test_prune_account_code_incremental_cross_account_reference() { + let mut conn = setup_test_db(); + + // The "switcher" account changes code first; the "holdout" account keeps code A pinned. + // Both accounts are created with code A at block 0. + // Prune 1 (tip 2R+1 → cutoff R+1): no marker yet, full pass; nothing is collectable. + // Block 2R+2: the switcher moves to code B — its code-A row expires at 2R+2. + // Prune 2 (tip 3R+2 → cutoff 2R+2): the expired row crosses the window `(R+1, 2R+2]`, making + // code A a candidate, but the holdout's open row still references it → retained. + // Block 3R+3: the holdout moves to code B — its code-A row expires at 3R+3. + // Prune 3 (tip 4R+3 → cutoff 3R+3): code A is a candidate again and no row references it past + // the cutoff → pruned. Code B is retained. + let block_0 = BlockNumber::from(0u32); + let block_first_prune = BlockNumber::from(2 * HISTORICAL_BLOCK_RETENTION + 1); + let block_switcher_to_b = BlockNumber::from(2 * HISTORICAL_BLOCK_RETENTION + 2); + let block_second_prune = BlockNumber::from(3 * HISTORICAL_BLOCK_RETENTION + 2); + let block_holdout_to_b = BlockNumber::from(3 * HISTORICAL_BLOCK_RETENTION + 3); + let block_third_prune = BlockNumber::from(4 * HISTORICAL_BLOCK_RETENTION + 3); + + for block in [block_0, block_switcher_to_b, block_holdout_to_b] { + insert_block_header(&mut conn, block); + } + + let switcher_on_a = build_account_with_code(1); + let switcher_on_b = build_account_with_code(2); + let holdout_on_a = build_account_with_code_seeded(1, [4u8; 32]); + let holdout_on_b = build_account_with_code_seeded(2, [4u8; 32]); + + assert_ne!(switcher_on_a.id(), holdout_on_a.id(), "accounts must be distinct"); + let code_commitment_a = switcher_on_a.code().commitment(); + let code_commitment_b = switcher_on_b.code().commitment(); + assert_eq!( + code_commitment_a, + holdout_on_a.code().commitment(), + "both accounts must share code A" + ); + + for account in [&switcher_on_a, &holdout_on_a] { + upsert_accounts( + &mut conn, + &[make_full_state_update(account)], + block_0, + &precomputed_states_from_account(account), + ) + .expect("block 0 upsert failed"); + } + + let (_, _, codes_deleted) = + prune_history(&mut conn, block_first_prune).expect("prune_history failed"); + assert_eq!(codes_deleted, 0, "no code is collectable while both accounts run code A"); + + upsert_accounts( + &mut conn, + &[make_full_state_update(&switcher_on_b)], + block_switcher_to_b, + &precomputed_states_from_account(&switcher_on_b), + ) + .expect("switcher code-change upsert failed"); + + let (_, _, codes_deleted) = + prune_history(&mut conn, block_second_prune).expect("prune_history failed"); + assert_eq!(codes_deleted, 0, "code A must survive while the holdout still references it"); + assert!( + account_code_exists(&mut conn, code_commitment_a), + "code A must be retained while the holdout references it" + ); + + upsert_accounts( + &mut conn, + &[make_full_state_update(&holdout_on_b)], + block_holdout_to_b, + &precomputed_states_from_account(&holdout_on_b), + ) + .expect("holdout code-change upsert failed"); + + let (_, _, codes_deleted) = + prune_history(&mut conn, block_third_prune).expect("prune_history failed"); + assert_eq!(codes_deleted, 1, "exactly one code (A) must be pruned"); + assert!(!account_code_exists(&mut conn, code_commitment_a), "code A must be pruned"); + assert!( + account_code_exists(&mut conn, code_commitment_b), + "current code B must be retained" + ); +} + +/// Prune test 6: `prune_progress` records the cutoff of the last codes prune; re-pruning at the +/// same or a lower cutoff deletes nothing and never moves the marker backwards. +#[test] +fn test_prune_account_codes_marker_never_regresses() { + let mut conn = setup_test_db(); + + // Same shape as prune test 2: code A at block 0 is superseded by code B at block R+1, so a + // prune at tip 2R+1 (cutoff R+1) collects code A. + let block_0 = BlockNumber::from(0u32); + let block_code_b = BlockNumber::from(HISTORICAL_BLOCK_RETENTION + 1); + let block_prune = BlockNumber::from(2 * HISTORICAL_BLOCK_RETENTION + 1); + + insert_block_header(&mut conn, block_0); + insert_block_header(&mut conn, block_code_b); + + let account_a = build_account_with_code(1); + let account_b = build_account_with_code(2); + + upsert_accounts( + &mut conn, + &[make_full_state_update(&account_a)], + block_0, + &precomputed_states_from_account(&account_a), + ) + .expect("block 0 upsert failed"); + upsert_accounts( + &mut conn, + &[make_full_state_update(&account_b)], + block_code_b, + &precomputed_states_from_account(&account_b), + ) + .expect("code-change upsert failed"); + + assert_eq!(codes_prune_cutoff(&mut conn), None, "no marker before the first prune"); + + let cutoff = i64::from(HISTORICAL_BLOCK_RETENTION + 1); + let (_, _, codes_deleted) = prune_history(&mut conn, block_prune).expect("first prune failed"); + assert_eq!(codes_deleted, 1, "exactly one code (A) must be pruned"); + assert_eq!(codes_prune_cutoff(&mut conn), Some(cutoff), "marker must record the cutoff"); + + // Re-pruning at the same tip is a no-op. + let (_, _, codes_deleted) = prune_history(&mut conn, block_prune).expect("second prune failed"); + assert_eq!(codes_deleted, 0, "re-pruning at the same cutoff must delete nothing"); + assert_eq!(codes_prune_cutoff(&mut conn), Some(cutoff), "marker must be unchanged"); + + // Pruning at a lower tip (cutoff 0) must not move the marker backwards. + let (_, _, codes_deleted) = + prune_history(&mut conn, BlockNumber::from(HISTORICAL_BLOCK_RETENTION)) + .expect("stale prune failed"); + assert_eq!(codes_deleted, 0, "pruning below the marker must delete nothing"); + assert_eq!(codes_prune_cutoff(&mut conn), Some(cutoff), "marker must never regress"); +} + #[test] #[miden_node_test_macro::enable_logging] fn network_accounts_subset_classifies_correctly() { diff --git a/crates/store/src/db/schema.rs b/crates/store/src/db/schema.rs index e571333d1..ebeeffaed 100644 --- a/crates/store/src/db/schema.rs +++ b/crates/store/src/db/schema.rs @@ -89,6 +89,13 @@ diesel::table! { } } +diesel::table! { + prune_progress (id) { + id -> Integer, + codes_cutoff -> BigInt, + } +} + diesel::table! { transactions (transaction_id) { transaction_id -> Binary, @@ -111,5 +118,6 @@ diesel::allow_tables_to_appear_in_same_query!( note_scripts, notes, nullifiers, + prune_progress, transactions, );