Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions packages/rs-platform-wallet/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,18 @@ pub enum PlatformWalletError {
#[error("Shielded commitment tree update failed: {0}")]
ShieldedTreeUpdateFailed(String),

/// The local tree exceeds the maximum proven count from a sync containing
/// a response at least as fresh as the tree's recorded height bound.
/// Stale responses and unknown tree provenance cannot trigger this error.
/// No automatic repair is performed.
#[error("Shielded commitment tree has {local_leaf_count} leaves, but a sufficiently fresh sync reports only {chain_leaf_count}; explicitly rebuild the shielded tree and resync")]
ShieldedTreeInconsistent {
/// Number of leaves in the local tree after consuming the stream.
local_leaf_count: u64,
/// Largest commitment count reported by the stream's proofs.
chain_leaf_count: u64,
},

#[error("Shielded store error: {0}")]
ShieldedStoreError(String),

Expand Down
23 changes: 23 additions & 0 deletions packages/rs-platform-wallet/src/manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,29 @@ impl<P: PlatformWalletPersistence + 'static> PlatformWalletManager<P> {
}
}

/// Rebuild the network's shielded commitment tree on the next sync.
/// Preserves notes, nullifiers, spent status, and wallet registrations while
/// clearing the tree and rewinding scan cursors. The full rescan from zero
/// costs bandwidth and trial decryption proportional to the entire pool.
/// Stops background shielded sync like [`Self::clear_shielded`]; restart it
/// or request a sync after this explicit repair. Never invoked automatically.
#[cfg(feature = "shielded")]
pub async fn rebuild_shielded_tree(&self) -> Result<(), PlatformWalletError> {
let _clearing = self.registry.hold_clearing(WalletWorker::ShieldedSync);
let Some(_quiesced) = self.shielded_sync_manager.quiesce_held().await else {
return Err(PlatformWalletError::ShutdownIncomplete(
"shielded sync pass did not drain within the quiesce budget; rebuild aborted"
.to_string(),
));
};
let coordinator = self.shielded_coordinator().await.ok_or_else(|| {
PlatformWalletError::ShieldedStoreError(
"shielded rebuild requested but no coordinator is configured".to_string(),
)
})?;
coordinator.rebuild_shielded_tree().await
Comment on lines +1038 to +1051

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Expose the preserving rebuild and typed inconsistency result through FFI

The only non-destructive recovery operation, rebuild_shielded_tree, is reachable only through the Rust manager/coordinator API. No corresponding entry point exists in rs-platform-wallet-ffi or the Swift/Kotlin wrappers. The FFI sync path maps ShieldedTreeInconsistent to the generic ErrorWalletOperation with a rendered string, so mobile hosts cannot reliably distinguish this terminal tree condition from a transient sync failure or invoke the required repair. The existing shielded_clear operation is not equivalent because it removes registrations, notes, spent status, and related wallet state. Add a thin FFI marshal entry point for the Rust rebuild method and a stable result-code mapping for ShieldedTreeInconsistent.

source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, rust-quality, security-auditor)

}

/// Reset the platform-address (BLAST/DIP-17) incremental-sync
/// watermark and drop every cached balance across **all**
/// registered wallets, forcing a full rescan on the next sync.
Expand Down
10 changes: 9 additions & 1 deletion packages/rs-platform-wallet/src/manager/shielded_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,15 @@ impl ShieldedSyncManager {
return Ok(None);
}

let pass = coordinator.sync(force).await;
if !coordinator
.registered_subwallets()
.await
.iter()
.any(|id| &id.wallet_id == wallet_id)
{
return Ok(None);
}
let pass = coordinator.try_sync(force).await?;

// Extract this wallet's slice from the network-wide pass
// summary. If the wallet is registered, we'll get back an
Expand Down
63 changes: 50 additions & 13 deletions packages/rs-platform-wallet/src/wallet/shielded/coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1291,6 +1291,29 @@ impl NetworkShieldedCoordinator {
Ok(())
}

/// Empty only the commitment tree and rewind all subwallet scan cursors.
/// Notes, nullifiers, spent status, pending spends, and registrations survive.
/// The next sync performs a full rescan from zero, costing bandwidth and
/// trial decryption proportional to the entire pool. Never called automatically.
pub async fn rebuild_shielded_tree(&self) -> Result<(), PlatformWalletError> {
let _lifecycle = self.lifecycle.lock().await;
let mut store = self.store.write().await;
// Rewind even if the durable reset fails partway through. Rescanning
// an intact tree is harmless; skipping a partially reset tree is not.
store.rewind_scan_watermarks();
if let Ok(mut caught_up) = self.last_caught_up_at.lock() {
*caught_up = None;
}
store.reset_commitment_tree()?;
let remaining = store.tree_size()?;
if remaining != 0 {
return Err(PlatformWalletError::ShieldedTreeUpdateFailed(format!(
"commitment tree still has {remaining} leaves after rebuild reset"
)));
}
Ok(())
}

/// Run one shielded sync pass for every registered wallet on
/// this coordinator's network. Returns a per-wallet outcome
/// summary suitable for emission to UI / persistence layers
Expand All @@ -1315,6 +1338,27 @@ impl NetworkShieldedCoordinator {
/// [`PlatformEventManager::on_shielded_sync_completed`]:
/// crate::events::PlatformEventManager::on_shielded_sync_completed
pub async fn sync(&self, force: bool) -> ShieldedSyncPassSummary {
match self.try_sync(force).await {
Ok(summary) => summary,
Err(error) => {
let subwallets = self
.accounts
.read()
.await
.iter()
.map(|(id, views)| (*id, views.clone()))
.collect::<Vec<_>>();
self.fail_all_wallets(&subwallets, &error)
}
}
}
Comment on lines 1338 to +1354

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Test background sync handling of ShieldedTreeInconsistent

The typed try_sync path is tested, but the background sync() path is not tested for this new failure. sync() catches ShieldedTreeInconsistent, converts it through fail_all_wallets, and exposes per-wallet Err(String) outcomes to the event layer. Add a test that drives sync(false) with an excessive local tree and fresh count evidence, then asserts the per-wallet error outcome and error count before rebuilding.

source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality)


/// Run the same pass as [`Self::sync`], preserving typed failures for callers
/// that need to distinguish an inconsistent tree from a transient sync error.
pub async fn try_sync(
&self,
force: bool,
) -> Result<ShieldedSyncPassSummary, PlatformWalletError> {
// Network-wide cooldown gate. Snapshot the remaining
// window into a local before any await — `std::sync::Mutex`
// is `!Send` across await points (clippy's
Expand All @@ -1336,7 +1380,7 @@ impl NetworkShieldedCoordinator {
cooldown_total_secs = CAUGHT_UP_COOLDOWN.as_secs(),
"Coordinator sync skipped — within caught-up cooldown"
);
return Self::cooldown_skip_summary(self).await;
return Ok(Self::cooldown_skip_summary(self).await);
}

// Snapshot the flat subwallet registry. This Vec is both
Expand All @@ -1350,7 +1394,7 @@ impl NetworkShieldedCoordinator {
let mut summary = ShieldedSyncPassSummary::default();
if subwallets.is_empty() {
summary.sync_unix_seconds = Self::now_unix();
return summary;
return Ok(summary);
}

// Snapshot armed reservations and prefetch Platform's recorded
Expand Down Expand Up @@ -1378,18 +1422,14 @@ impl NetworkShieldedCoordinator {
// Second, distinct signal: commitments committed to the local
// tree as the interleaved consumer drains the SDK stream.
let on_tree_progress = self.tree_progress_handler();
let notes = match super::sync::sync_notes_across(
let notes = super::sync::sync_notes_across(
&self.sdk,
&self.store,
&subwallets,
on_progress.as_ref(),
on_tree_progress.as_ref(),
)
.await
{
Ok(r) => r,
Err(e) => return self.fail_all_wallets(&subwallets, &e),
};
.await?;
// Scan-based spend detection now happens INSIDE
// `sync_notes_across`: every scanned action's nullifier is
// replayed against each subwallet's store as part of the note
Expand Down Expand Up @@ -1441,10 +1481,7 @@ impl NetworkShieldedCoordinator {
self.release_stranded_spends(snapshot, &recorded).await;
}

let balances_per_sub = match super::sync::balances_across(&self.store, &subwallets).await {
Ok(r) => r,
Err(e) => return self.fail_all_wallets(&subwallets, &e),
};
let balances_per_sub = super::sync::balances_across(&self.store, &subwallets).await?;

// Restore-path activity derivation: reconstruct per-operation
// activity entries best-effort from the notes / outgoing notes
Expand Down Expand Up @@ -1524,7 +1561,7 @@ impl NetworkShieldedCoordinator {
summary =
build_per_wallet_summary(&subwallets, &notes, &newly_spent_per_sub, &balances_per_sub);
summary.sync_unix_seconds = Self::now_unix();
summary
Ok(summary)
}

/// Build a `ShieldedSyncPassSummary` where every registered
Expand Down
71 changes: 70 additions & 1 deletion packages/rs-platform-wallet/src/wallet/shielded/file_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ impl FileBackedShieldedStore {
}
}

/// Rewind all scan cursors without discarding receipts or spend guards.
pub(super) fn rewind_scan_watermarks(&mut self) {
for state in self.subwallets.values_mut() {
state.last_synced_index = 0;
}
Comment on lines +151 to +155

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Reset host-visible scan metadata during a tree rebuild

rewind_scan_watermarks resets only last_synced_index, but last_scanned_index remains at the pre-rebuild value and balance_source remains ScannedThisSession. Until the rescan completes, local_balance_snapshot can therefore report the old scan coverage and a successfully scanned session even though the commitment tree has been emptied. Reset the session metadata as part of the same rewind while retaining the note ledger as restored state.

Suggested change
/// Rewind all scan cursors without discarding receipts or spend guards.
pub(super) fn rewind_scan_watermarks(&mut self) {
for state in self.subwallets.values_mut() {
state.last_synced_index = 0;
}
for state in self.subwallets.values_mut() {
state.last_synced_index = 0;
state.last_scanned_index = None;
state.balance_source = if state.notes.is_empty() {
ShieldedBalanceSource::NoHistory
} else {
ShieldedBalanceSource::Restored
};
}

source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, rust-quality, security-auditor)

}

/// Open or create a shielded store at `path`.
///
/// SQLite is opened with **WAL journal + synchronous=NORMAL + temp_store=MEMORY**
Expand Down Expand Up @@ -194,6 +201,19 @@ impl FileBackedShieldedStore {
.map_err(|e| {
FileShieldedStoreError::Storage(format!("create pending_spends table: {e}"))
})?;
// The bound shares the tree's database/WAL: committing it before an
// append prevents a restart from pairing new leaves with an old bound.
pending_conn
.execute(
"CREATE TABLE IF NOT EXISTS shielded_tree_sync_height (
id INTEGER PRIMARY KEY CHECK (id = 0),
height INTEGER NOT NULL CHECK (height >= 0)
)",
[],
)
.map_err(|e| {
FileShieldedStoreError::Storage(format!("create tree sync height: {e}"))
})?;
// Additive, idempotent migration: old guards remain active.
let columns = {
let mut statement = pending_conn
Expand Down Expand Up @@ -542,6 +562,17 @@ impl ShieldedStore for FileBackedShieldedStore {
.unwrap_or_default())
}

fn note_spent_status(
&self,
id: SubwalletId,
nullifier: &[u8; 32],
) -> Result<Option<bool>, Self::Error> {
Ok(self
.subwallets
.get(&id)
.and_then(|state| state.note_spent_status(nullifier)))
}

fn mark_spent(&mut self, id: SubwalletId, nullifier: &[u8; 32]) -> Result<bool, Self::Error> {
let Some(sw) = self.subwallets.get_mut(&id) else {
return Ok(false);
Expand Down Expand Up @@ -857,6 +888,43 @@ impl ShieldedStore for FileBackedShieldedStore {
Ok(size)
}

fn tree_sync_height(&self) -> Result<Option<u64>, Self::Error> {
let conn = self
.pending_conn
.lock()
.map_err(|e| FileShieldedStoreError::Storage(format!("tree sync height mutex: {e}")))?;
let height: Option<i64> = conn
.query_row(
"SELECT MAX(height) FROM shielded_tree_sync_height",
[],
|row| row.get(0),
)
.map_err(|e| FileShieldedStoreError::Storage(format!("read tree sync height: {e}")))?;
height
.map(u64::try_from)
.transpose()
.map_err(|e| FileShieldedStoreError::Storage(format!("invalid tree sync height: {e}")))
}

fn set_tree_sync_height(&mut self, height: u64) -> Result<(), Self::Error> {
let height = i64::try_from(height).map_err(|e| {
FileShieldedStoreError::Storage(format!(
"tree sync height exceeds SQLite integer range: {e}"
))
})?;
let conn = self
.pending_conn
.lock()
.map_err(|e| FileShieldedStoreError::Storage(format!("tree sync height mutex: {e}")))?;
conn.execute(
"INSERT INTO shielded_tree_sync_height (id, height) VALUES (0, ?1)
ON CONFLICT(id) DO UPDATE SET height = MAX(height, excluded.height)",
[height],
)
.map_err(|e| FileShieldedStoreError::Storage(format!("write tree sync height: {e}")))?;
Ok(())
}

fn last_synced_note_index(&self, id: SubwalletId) -> Result<u64, Self::Error> {
Ok(self
.subwallets
Expand Down Expand Up @@ -974,7 +1042,8 @@ impl ShieldedStore for FileBackedShieldedStore {
"DELETE FROM commitment_tree_checkpoint_marks_removed;
DELETE FROM commitment_tree_checkpoints;
DELETE FROM commitment_tree_shards;
DELETE FROM commitment_tree_cap;",
DELETE FROM commitment_tree_cap;
DELETE FROM shielded_tree_sync_height;",
)
.map_err(|e| {
FileShieldedStoreError::Storage(format!("reset commitment tree tables: {e}"))
Expand Down
56 changes: 56 additions & 0 deletions packages/rs-platform-wallet/src/wallet/shielded/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,20 @@ pub trait ShieldedStore: Send + Sync {
/// Return all notes (spent and unspent) for `id`.
fn get_all_notes(&self, id: SubwalletId) -> Result<Vec<ShieldedNote>, Self::Error>;

/// Look up one receipt's spent status; `None` means the nullifier is unknown.
/// Stores with a nullifier index should override this compatibility fallback.
fn note_spent_status(
&self,
id: SubwalletId,
nullifier: &[u8; 32],
) -> Result<Option<bool>, Self::Error> {
Ok(self
.get_all_notes(id)?
.iter()
.find(|note| &note.nullifier == nullifier)
.map(|note| note.is_spent))
}

/// Mark `id`'s note with `nullifier` as spent. Returns `true`
/// if a matching unspent note was found.
fn mark_spent(&mut self, id: SubwalletId, nullifier: &[u8; 32]) -> Result<bool, Self::Error>;
Expand Down Expand Up @@ -460,6 +474,19 @@ pub trait ShieldedStore: Send + Sync {
/// found in the recorded anchors tree" at spend time).
fn tree_size(&self) -> Result<u64, Self::Error>;

/// An upper bound on the response heights needed to supply all local leaves.
/// `None` means provenance is unknown: a lower chain count cannot prove corruption.
fn tree_sync_height(&self) -> Result<Option<u64>, Self::Error> {
Ok(None)
}

/// Raise the tree's height bound before appending data from that response.
/// Persistent stores must durably order this write before the corresponding
/// appends. Stores without provenance retain the default unknown bound.
fn set_tree_sync_height(&mut self, _height: u64) -> Result<(), Self::Error> {
Ok(())
}

// ── Sync state (per-subwallet) ─────────────────────────────────────

/// Sync watermark for `id`: the count of note positions already
Expand Down Expand Up @@ -628,6 +655,13 @@ impl SubwalletState {
.try_fold(0u64, |total, note| total.checked_add(note.value))
}

pub(super) fn note_spent_status(&self, nullifier: &[u8; 32]) -> Option<bool> {
self.nullifier_index
.get(nullifier)
.and_then(|index| self.notes.get(*index))
.map(|note| note.is_spent)
}

pub(super) fn all_notes(&self) -> Vec<ShieldedNote> {
self.notes.clone()
}
Expand Down Expand Up @@ -853,6 +887,7 @@ pub struct InMemoryShieldedStore {
checkpoints: Vec<u32>,
/// Placeholder anchor; production stores compute the real Sinsemilla root.
anchor: [u8; 32],
tree_sync_height: Option<u64>,
}

impl InMemoryShieldedStore {
Expand Down Expand Up @@ -893,6 +928,17 @@ impl ShieldedStore for InMemoryShieldedStore {
.unwrap_or_default())
}

fn note_spent_status(
&self,
id: SubwalletId,
nullifier: &[u8; 32],
) -> Result<Option<bool>, Self::Error> {
Ok(self
.subwallets
.get(&id)
.and_then(|state| state.note_spent_status(nullifier)))
}

fn mark_spent(&mut self, id: SubwalletId, nullifier: &[u8; 32]) -> Result<bool, Self::Error> {
// In-memory store: the redrive map lives in the same
// `SubwalletState`, so `mark_spent` already dropped any resolved
Expand Down Expand Up @@ -1091,6 +1137,15 @@ impl ShieldedStore for InMemoryShieldedStore {
Ok(self.commitments.len() as u64)
}

fn tree_sync_height(&self) -> Result<Option<u64>, Self::Error> {
Ok(self.tree_sync_height)
}

fn set_tree_sync_height(&mut self, height: u64) -> Result<(), Self::Error> {
self.tree_sync_height = Some(self.tree_sync_height.unwrap_or(0).max(height));
Ok(())
}

fn last_synced_note_index(&self, id: SubwalletId) -> Result<u64, Self::Error> {
Ok(self
.subwallets
Expand Down Expand Up @@ -1142,6 +1197,7 @@ impl ShieldedStore for InMemoryShieldedStore {
// checkpoint vectors drops `tree_size()` to 0 and makes the
// next append start at position 0, matching the file-backed
// store's reset contract.
self.tree_sync_height = None;
self.commitments.clear();
self.marked_positions.clear();
self.checkpoints.clear();
Expand Down
Loading
Loading