Skip to content

fix(sync): report only genuinely locked accounts in sync summary #2340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
igamigo wants to merge 1 commit into from
Closed

fix(sync): report only genuinely locked accounts in sync summary #2340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@

* [FEATURE][cli] Added a `--payback-note-type` option to `swap` so the payback note can be created as public or private (defaults to private). Public payback works without any off-band advice now that SWAP derives the payback recipient deterministically ([#2190](https://github.com/0xMiden/rust-sdk/pull/2190)).

### Changes

* [rust] `AccountUpdates` records the local account states superseded by a same-nonce network transaction alongside the public and mismatched-private account updates, exposed via `AccountUpdates::superseded_local_states`. State sync discards the affected transactions as part of account sync rather than relying on its caller to forward a separate return value. `miden_client::store::AccountUpdates` and `miden_client::sync::AccountUpdates` now name the same type.

### Fixes

* [FIX][rust] `SyncSummary::locked_accounts` now reports only the private accounts a sync actually locked. A mismatched commitment that is already present in local history is stale network data and leaves the account usable, but every mismatch was previously reported as a lock — so `miden-client-cli sync` could print a non-zero "Locked accounts" count for accounts that remained usable. The count also no longer re-reports accounts that were already locked before the sync.

## 0.16.0-alpha.1 (2026-07-17)

### Breaking Changes
Expand Down
39 changes: 1 addition & 38 deletions crates/rust-client/src/store/account.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
// ACCOUNT RECORD
// ================================================================================================
use alloc::vec::Vec;
use core::fmt::Display;

use miden_protocol::account::{Account, AccountId, PartialAccount};
use miden_protocol::account::{Account, PartialAccount};
use miden_protocol::{Felt, Word};

use crate::ClientError;
use crate::sync::PublicAccountUpdate;

// ACCOUNT RECORD DATA
// ================================================================================================
Expand Down Expand Up @@ -175,38 +173,3 @@ impl Display for AccountStatus {
}
}
}

// ACCOUNT UPDATES
// ================================================================================================

/// Contains account changes to apply to the store.
pub struct AccountUpdates {
/// Updated public accounts, either as full state replacements or incremental deltas.
updated_public_accounts: Vec<PublicAccountUpdate>,
/// Network account commitments that don't match the current tracked state for private
/// accounts.
mismatched_private_accounts: Vec<(AccountId, Word)>,
}

impl AccountUpdates {
/// Creates a new instance of `AccountUpdates`.
pub fn new(
updated_public_accounts: Vec<PublicAccountUpdate>,
mismatched_private_accounts: Vec<(AccountId, Word)>,
) -> Self {
Self {
updated_public_accounts,
mismatched_private_accounts,
}
}

/// Returns the updated public accounts.
pub fn updated_public_accounts(&self) -> &[PublicAccountUpdate] {
&self.updated_public_accounts
}

/// Returns the mismatched private accounts.
pub fn mismatched_private_accounts(&self) -> &[(AccountId, Word)] {
&self.mismatched_private_accounts
}
}
10 changes: 2 additions & 8 deletions crates/rust-client/src/store/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,9 @@ mod smt_forest;
pub use smt_forest::AccountSmtForest;

mod account;
pub use account::{
AccountRecord,
AccountRecordData,
AccountStatus,
AccountUpdates,
ClientAccountType,
};
pub use account::{AccountRecord, AccountRecordData, AccountStatus, ClientAccountType};

pub use crate::sync::PublicAccountUpdate;
pub use crate::sync::{AccountUpdates, PublicAccountUpdate};
mod note_record;
pub use note_record::{
InputNoteRecord,
Expand Down
48 changes: 45 additions & 3 deletions crates/rust-client/src/sync/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,18 @@ where
// Get the sync update from the network
let state_sync_update = state_sync.sync_state(&mut partial_mmr, input).await?;

let sync_summary: SyncSummary = (&state_sync_update).into();
debug!(sync_summary = ?sync_summary, "Sync summary computed");
let mut sync_summary: SyncSummary = (&state_sync_update).into();

// Accounts whose network commitment diverges from the local state. The store locks such an
// account only when the diverging commitment is absent from local history, so which of
// these actually get locked is only observable by reading their status back afterwards.
let lock_candidates: Vec<AccountId> = state_sync_update
.account_updates
.mismatched_private_accounts()
.iter()
.map(|(id, _)| *id)
.collect();
let already_locked = self.locked_accounts_among(&lock_candidates).await?;

// Post-sync observer hooks; run before persisting. Per-observer errors are logged, not
// propagated.
Expand All @@ -146,6 +156,15 @@ where
.await
.map_err(ClientError::StoreError)?;

sync_summary.locked_accounts = self
.locked_accounts_among(&lock_candidates)
.await?
.into_iter()
.filter(|account_id| !already_locked.contains(account_id))
.collect();

debug!(sync_summary = ?sync_summary, "Sync summary computed");

// Cache MMR so pruning can reuse in-memory MMR.
self.cache_partial_mmr(partial_mmr).await?;

Expand Down Expand Up @@ -202,6 +221,24 @@ where
Ok(summary)
}

/// Returns which of the given accounts are currently locked.
///
/// Accounts missing from the store are treated as not locked.
async fn locked_accounts_among(
&self,
account_ids: &[AccountId],
) -> Result<BTreeSet<AccountId>, ClientError> {
let mut locked = BTreeSet::new();
for account_id in account_ids {
let header = self.store.get_account_header(*account_id).await?;
if header.is_some_and(|(_, status)| status.is_locked()) {
locked.insert(*account_id);
}
}

Ok(locked)
}

/// Builds a default [`StateSyncInput`] from the current client state.
///
/// This includes all tracked account headers, all unique note tags, all unspent input and
Expand Down Expand Up @@ -357,7 +394,12 @@ pub struct SyncSummary {
pub consumed_notes: Vec<NoteId>,
/// IDs of on-chain accounts that have been updated.
pub updated_accounts: Vec<AccountId>,
/// IDs of private accounts that have been locked.
/// IDs of private accounts that this sync locked because their on-chain commitment diverged
/// from the local state.
///
/// A divergence that resolves to a commitment already present in local history is stale
/// network data rather than a real conflict; it leaves the account usable and is not reported
/// here. Accounts already locked before this sync are also excluded.
pub locked_accounts: Vec<AccountId>,
/// IDs of committed transactions.
pub committed_transactions: Vec<TransactionId>,
Expand Down
88 changes: 40 additions & 48 deletions crates/rust-client/src/sync/state_sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,22 +283,14 @@ impl StateSync {
state_sync_update.block_num = sync_data.chain_tip_header.block_num();

let new_commitments = derive_account_commitments(&sync_data.transactions);
let superseded_states = self
.account_state_sync(
&mut state_sync_update.account_updates,
&accounts,
&new_commitments,
block_num,
&sync_data.chain_tip_header,
)
.await?;

// Discard the local transactions whose result lost a same-nonce race against the network.
for superseded_state in superseded_states {
state_sync_update
.transaction_updates
.apply_superseded_account_state(superseded_state);
}
self.account_state_sync(
&mut state_sync_update,
&accounts,
&new_commitments,
block_num,
&sync_data.chain_tip_header,
)
.await?;

// Apply local changes: update the MMR, screen notes, and apply state transitions.
self.apply_sync_result(sync_data, &mut state_sync_update, current_partial_mmr)
Expand Down Expand Up @@ -672,35 +664,32 @@ impl StateSync {
/// * Private accounts that have been marked as mismatched because the current commitment
/// doesn't match the one received from the node. The client will need to handle these cases
/// as they could be a stale account state or a reason to lock the account.
///
/// Returns the local states that were superseded by a same-nonce network transaction; the
/// caller must discard the transactions that produced them.
/// * Local account states superseded by a same-nonce network transaction, whose producing
/// transactions are discarded before this method returns.
async fn account_state_sync(
&self,
account_updates: &mut AccountUpdates,
state_sync_update: &mut StateSyncUpdate,
accounts: &[AccountHeader],
account_commitment_updates: &[(AccountId, Word)],
block_from: BlockNumber,
chain_tip_header: &BlockHeader,
) -> Result<Vec<Word>, ClientError> {
) -> Result<(), ClientError> {
// "Public" here includes both Public and Network accounts, since both have
// their state stored on-chain and follow the same sync path.
let (public_accounts, private_accounts): (Vec<_>, Vec<_>) =
accounts.iter().partition(|header| !header.id().is_private());

let superseded_states = self
.sync_public_accounts(
account_updates,
account_commitment_updates,
&public_accounts,
block_from,
chain_tip_header,
)
.await?;
self.sync_public_accounts(
&mut state_sync_update.account_updates,
account_commitment_updates,
&public_accounts,
block_from,
chain_tip_header,
)
.await?;

// If a private account commitment differs between the node and local then we verify the
// commitment from the node before flagging the account as mismatched.
let mut mismatched_private_accounts = Vec::new();
for header in &private_accounts {
let account_id = header.id();
let local_commitment = header.to_commitment();
Expand All @@ -715,13 +704,15 @@ impl StateSync {
.verify_private_account_mismatch(account_id, local_commitment, chain_tip_header)
.await?
{
mismatched_private_accounts.push((account_id, proven_commitment));
state_sync_update
.account_updates
.push_mismatched_private_account(account_id, proven_commitment);
}
}

account_updates.extend(AccountUpdates::new(Vec::new(), mismatched_private_accounts));
state_sync_update.discard_superseded_transactions();

Ok(superseded_states)
Ok(())
}

/// Verifies a private account commitment against an account witness from the node.
Expand Down Expand Up @@ -781,7 +772,7 @@ impl StateSync {
/// single `get_account` call that requests every storage map and the vault.
///
/// Accounts whose vault or maps are too large to fit in a single response fall back to the
/// incremental [`PublicAccountUpdate::Delta`] path, which fetches vault and storage map
/// incremental [`PublicAccountUpdate::Patch`] path, which fetches vault and storage map
/// updates over the synced block range.
async fn sync_public_accounts(
&self,
Expand All @@ -790,11 +781,9 @@ impl StateSync {
current_public_accounts: &[&AccountHeader],
block_from: BlockNumber,
chain_tip_header: &BlockHeader,
) -> Result<Vec<Word>, ClientError> {
) -> Result<(), ClientError> {
let local_headers: BTreeMap<AccountId, &AccountHeader> =
current_public_accounts.iter().map(|header| (header.id(), *header)).collect();
// Local states that lost a same-nonce race; their transactions must be discarded.
let mut superseded_states = Vec::new();
for (id, commitment) in commitment_updates {
let Some(local_header) = local_headers.get(id).copied() else {
continue;
Expand All @@ -809,16 +798,16 @@ impl StateSync {
.await?
{
PublicAccountSync::Apply(public_update) => {
account_updates.extend(AccountUpdates::new(vec![*public_update], Vec::new()));
account_updates.push_public_update(*public_update);
},
PublicAccountSync::Superseded => {
superseded_states.push(local_header.to_commitment());
account_updates.push_superseded_local_state(local_header.to_commitment());
},
PublicAccountSync::Ignore => {},
}
}

Ok(superseded_states)
Ok(())
}

// SYNC PUBLIC ACCOUNTS HELPERS
Expand Down Expand Up @@ -1373,7 +1362,7 @@ mod tests {
let commitment_updates = vec![(account.id(), account.to_commitment())];
let mut account_updates = AccountUpdates::default();

let superseded = state_sync
state_sync
.sync_public_accounts(
&mut account_updates,
&commitment_updates,
Expand All @@ -1389,7 +1378,7 @@ mod tests {
"public account sync should ignore node snapshots that are older than local"
);
assert!(
superseded.is_empty(),
account_updates.superseded_local_states().is_empty(),
"an older node snapshot must not supersede the local state"
);
}
Expand All @@ -1410,7 +1399,7 @@ mod tests {
let commitment_updates = vec![(account.id(), account.to_commitment())];
let mut account_updates = AccountUpdates::default();

let superseded = state_sync
state_sync
.sync_public_accounts(
&mut account_updates,
&commitment_updates,
Expand All @@ -1426,8 +1415,8 @@ mod tests {
"a same-nonce fork must not overwrite the account while its tx is still pending"
);
assert_eq!(
superseded,
vec![local_header.to_commitment()],
account_updates.superseded_local_states(),
[local_header.to_commitment()],
"the superseded local state should be reported so its transaction is discarded"
);
}
Expand Down Expand Up @@ -1594,7 +1583,7 @@ mod tests {
let commitment_updates = vec![(account.id(), account.to_commitment())];
let mut account_updates = AccountUpdates::default();

let superseded = state_sync
state_sync
.sync_public_accounts(
&mut account_updates,
&commitment_updates,
Expand All @@ -1605,7 +1594,10 @@ mod tests {
.await
.unwrap();

assert!(superseded.is_empty(), "the transaction must not be superseded");
assert!(
account_updates.superseded_local_states().is_empty(),
"the transaction must not be superseded"
);
assert!(
account_updates.updated_public_accounts().is_empty(),
"the target state must not overwrite the local account"
Expand Down
Loading
Loading