From 213aecbddef9c7fa669fb0f0dc40cfa3fea78b11 Mon Sep 17 00:00:00 2001 From: wodesiku Date: Sat, 22 Aug 2026 02:18:05 +0800 Subject: [PATCH] fix(store): invalidate storage map values on slot recreation --- .../store/src/db/models/queries/accounts.rs | 69 +++++++-- .../db/models/queries/accounts/delta/tests.rs | 146 ++++++++++++++++++ 2 files changed, 206 insertions(+), 9 deletions(-) diff --git a/crates/store/src/db/models/queries/accounts.rs b/crates/store/src/db/models/queries/accounts.rs index 90a040122..23c0e0140 100644 --- a/crates/store/src/db/models/queries/accounts.rs +++ b/crates/store/src/db/models/queries/accounts.rs @@ -38,6 +38,7 @@ use miden_protocol::account::{ StorageMap, StorageMapKey, StorageMapPatchEntries, + StoragePatchOperation, StorageSlot, StorageSlotContent, StorageSlotName, @@ -1059,13 +1060,35 @@ fn insert_account_storage_map_value_inner( Ok(update_count + insert_count) } +/// Closes all current rows for a storage-map slot. +fn invalidate_storage_map_slot( + conn: &mut SqliteConnection, + account_id: AccountId, + block_num: BlockNumber, + slot_name: StorageSlotName, +) -> Result { + Ok(diesel::update(schema::account_storage_map_values::table) + .filter( + schema::account_storage_map_values::account_id + .eq(account_id.to_bytes()) + .and(schema::account_storage_map_values::slot_name.eq(slot_name.to_raw_sql())) + .and(schema::account_storage_map_values::valid_until.eq(VALID_FOREVER)), + ) + .set(schema::account_storage_map_values::valid_until.eq(block_num.to_raw_sql())) + .execute(conn)?) +} + type PendingStorageInserts = Vec<(AccountId, StorageSlotName, StorageMapKey, Word)>; +type PendingStorageSlotClears = Vec<(AccountId, StorageSlotName)>; type PendingAssetInserts = Vec<(AccountId, AssetId, Option)>; fn prepare_full_account_update( update: &BlockAccountUpdate, account: Account, -) -> Result<(AccountStateForInsert, PendingStorageInserts, PendingAssetInserts), DatabaseError> { +) -> Result< + (AccountStateForInsert, PendingStorageInserts, PendingStorageSlotClears, PendingAssetInserts), + DatabaseError, +> { let account_id = account.id(); // sanity check the commitment of account matches the final state commitment @@ -1099,7 +1122,7 @@ fn prepare_full_account_update( } } - Ok((AccountStateForInsert::FullAccount(account), storage, assets)) + Ok((AccountStateForInsert::FullAccount(account), storage, vec![], assets)) } /// Prepares a full public-account insertion using roots computed by the account-state forest. @@ -1118,7 +1141,10 @@ fn prepare_precomputed_full_account_update( update: &BlockAccountUpdate, patch: &AccountPatch, precomputed: &PrecomputedPublicAccountState, -) -> Result<(AccountStateForInsert, PendingStorageInserts, PendingAssetInserts), DatabaseError> { +) -> Result< + (AccountStateForInsert, PendingStorageInserts, PendingStorageSlotClears, PendingAssetInserts), + DatabaseError, +> { let account_id = patch.id(); let code = patch.code().cloned().ok_or_else(|| { DatabaseError::DataCorrupted(format!( @@ -1184,7 +1210,7 @@ fn prepare_precomputed_full_account_update( is_network_account, }; - Ok((AccountStateForInsert::PrecomputedFullState(state), storage, assets)) + Ok((AccountStateForInsert::PrecomputedFullState(state), storage, vec![], assets)) } /// Prepares a partial public-account update using the latest row and precomputed forest roots. @@ -1204,7 +1230,10 @@ fn prepare_partial_account_update( patch: &AccountPatch, precomputed: &PrecomputedPublicAccountState, existing: &LatestAccountStateRow, -) -> Result<(AccountStateForInsert, PendingStorageInserts, PendingAssetInserts), DatabaseError> { +) -> Result< + (AccountStateForInsert, PendingStorageInserts, PendingStorageSlotClears, PendingAssetInserts), + DatabaseError, +> { // Build the minimal account state needed for partial patch application from the latest row that // was loaded with the account's creation metadata. let state_headers = existing.state_headers(account_id)?; @@ -1224,7 +1253,14 @@ fn prepare_partial_account_update( // --- Collect storage map updates. --------------------------- let mut storage = Vec::new(); + let mut storage_slot_clears = Vec::new(); for (slot_name, map_patch) in patch.storage().maps() { + if matches!( + map_patch.patch_op(), + StoragePatchOperation::Create | StoragePatchOperation::Remove + ) { + storage_slot_clears.push((account_id, slot_name.clone())); + } for (key, value) in map_patch.entries().into_iter().flat_map(StorageMapPatchEntries::as_map) { storage.push((account_id, slot_name.clone(), *key, *value)); @@ -1266,7 +1302,12 @@ fn prepare_partial_account_update( }); } - Ok((AccountStateForInsert::PartialState(account_state), storage, assets)) + Ok(( + AccountStateForInsert::PartialState(account_state), + storage, + storage_slot_clears, + assets, + )) } /// Returns the subset of `account_ids` whose latest committed state is a network account. @@ -1331,9 +1372,15 @@ pub(crate) fn upsert_accounts( // written. The storage and vault tables have FKs pointing to accounts `(account_id, // block_num)`, so inserting them earlier would violate those constraints when inserting a // brand-new account. - let (account_state, pending_storage_inserts, pending_asset_inserts) = match update.details() - { - AccountUpdateDetails::Private => (AccountStateForInsert::Private, vec![], vec![]), + let ( + account_state, + pending_storage_inserts, + pending_storage_slot_clears, + pending_asset_inserts, + ) = match update.details() { + AccountUpdateDetails::Private => { + (AccountStateForInsert::Private, vec![], vec![], vec![]) + }, // New account is always a full account, but also comes as an update AccountUpdateDetails::Public(patch) if patch.is_full_state() => { @@ -1461,6 +1508,10 @@ pub(crate) fn upsert_accounts( .set(&account_value) .execute(conn)?; + for (acc_id, slot_name) in pending_storage_slot_clears { + invalidate_storage_map_slot(conn, acc_id, block_num, slot_name)?; + } + // insert pending storage map entries TODO consider batching for (acc_id, slot_name, key, value) in pending_storage_inserts { if account_is_new { diff --git a/crates/store/src/db/models/queries/accounts/delta/tests.rs b/crates/store/src/db/models/queries/accounts/delta/tests.rs index 8eb735fa2..4b0e261c5 100644 --- a/crates/store/src/db/models/queries/accounts/delta/tests.rs +++ b/crates/store/src/db/models/queries/accounts/delta/tests.rs @@ -23,6 +23,7 @@ use miden_protocol::account::{ StorageMap, StorageMapKey, StorageMapPatch, + StorageMapPatchEntries, StorageSlot, StorageSlotName, StorageSlotPatch, @@ -791,6 +792,151 @@ fn optimized_delta_updates_storage_map_header() { ); } +#[test] +fn optimized_delta_removes_storage_map_values() { + const ACCOUNT_SEED: [u8; 32] = [31u8; 32]; + const SLOT_INDEX_MAP: usize = 3; + + let mut conn = setup_test_db(); + let block_1 = BlockNumber::from(1u32); + let block_2 = BlockNumber::from(2u32); + insert_block_header(&mut conn, block_1); + insert_block_header(&mut conn, block_2); + + let map_key = StorageMapKey::from_index(7); + let map_value = + Word::from([Felt::new_unchecked(10), Felt::ZERO, Felt::ZERO, Felt::ZERO]); + let second_map_key = StorageMapKey::from_index(8); + let second_map_value = + Word::from([Felt::new_unchecked(20), Felt::ZERO, Felt::ZERO, Felt::ZERO]); + let storage_map = + StorageMap::with_entries([(map_key, map_value), (second_map_key, second_map_value)]) + .unwrap(); + let component = AccountComponent::new( + CodeBuilder::default() + .compile_component_code( + "test::interface", + "@account_procedure pub proc map push.1 end", + ) + .unwrap(), + vec![StorageSlot::with_map( + StorageSlotName::mock(SLOT_INDEX_MAP), + storage_map, + )], + AccountComponentMetadata::new("test"), + ) + .unwrap(); + let account = AccountBuilder::new(ACCOUNT_SEED) + .account_type(AccountType::Public) + .with_component(component) + .with_component(AuthSingleSig::new(Approver::new( + PublicKeyCommitment::from(EMPTY_WORD), + AuthScheme::Falcon512Poseidon2, + ))) + .build_existing() + .unwrap(); + + let initial_patch = AccountPatch::try_from(account.clone()).unwrap(); + upsert_accounts( + &mut conn, + &[BlockAccountUpdate::new( + account.id(), + account.to_commitment(), + AccountUpdateDetails::Public(initial_patch), + )], + block_1, + &precomputed_states_from_account(&account), + ) + .unwrap(); + + let previous = select_full_account(&mut conn, account.id()).unwrap(); + let remove_patch = AccountPatch::new( + account.id(), + AccountStoragePatch::from_raw(BTreeMap::from_iter([( + StorageSlotName::mock(SLOT_INDEX_MAP), + StorageSlotPatch::Map(StorageMapPatch::Remove), + )])) + .unwrap(), + AccountVaultPatch::default(), + None, + Some(Felt::new_unchecked(previous.nonce().as_canonical_u64() + 1)), + ) + .unwrap(); + let mut expected = previous; + expected.apply_patch(&remove_patch).unwrap(); + + upsert_accounts( + &mut conn, + &[BlockAccountUpdate::new( + account.id(), + expected.to_commitment(), + AccountUpdateDetails::Public(remove_patch), + )], + block_2, + &precomputed_states_from_account(&expected), + ) + .unwrap(); + + use crate::db::schema::account_storage_map_values as map_values; + let latest_rows: i64 = map_values::table + .filter(map_values::account_id.eq(account.id().to_bytes())) + .filter(map_values::slot_name.eq(StorageSlotName::mock(SLOT_INDEX_MAP).to_raw_sql())) + .filter(map_values::valid_until.eq(VALID_FOREVER)) + .count() + .get_result(&mut conn) + .unwrap(); + assert_eq!( + latest_rows, 0, + "removed storage map values must not remain latest" + ); + + let block_3 = BlockNumber::from(3u32); + insert_block_header(&mut conn, block_3); + let recreated_key = StorageMapKey::from_index(9); + let recreated_value = + Word::from([Felt::new_unchecked(30), Felt::ZERO, Felt::ZERO, Felt::ZERO]); + let recreate_patch = AccountPatch::new( + account.id(), + AccountStoragePatch::from_raw(BTreeMap::from_iter([( + StorageSlotName::mock(SLOT_INDEX_MAP), + StorageSlotPatch::Map(StorageMapPatch::Create { + entries: StorageMapPatchEntries::from_iter([(recreated_key, recreated_value)]), + }), + )])) + .unwrap(), + AccountVaultPatch::default(), + None, + Some(Felt::new_unchecked(expected.nonce().as_canonical_u64() + 1)), + ) + .unwrap(); + let mut recreated = expected; + recreated.apply_patch(&recreate_patch).unwrap(); + + upsert_accounts( + &mut conn, + &[BlockAccountUpdate::new( + account.id(), + recreated.to_commitment(), + AccountUpdateDetails::Public(recreate_patch), + )], + block_3, + &precomputed_states_from_account(&recreated), + ) + .unwrap(); + + let latest_rows: i64 = map_values::table + .filter(map_values::account_id.eq(account.id().to_bytes())) + .filter(map_values::slot_name.eq(StorageSlotName::mock(SLOT_INDEX_MAP).to_raw_sql())) + .filter(map_values::valid_until.eq(VALID_FOREVER)) + .count() + .get_result(&mut conn) + .unwrap(); + assert_eq!( + latest_rows, 1, + "recreated storage map must contain only its new entries" + ); +} + #[test] fn apply_storage_patch_with_roots_uses_precomputed_map_root() { use miden_protocol::account::{AccountStorageHeader, StorageSlotHeader, StorageSlotType};