From 8450d44c976302713384c3b223a35ae80cf7a9eb Mon Sep 17 00:00:00 2001 From: tarikgul Date: Sun, 16 Aug 2026 23:30:02 -0400 Subject: [PATCH 1/2] feat(server): report renewal targets dropped by an identity change A pass silently discarded targets promised by a previous identity. It counted them in a log line and returned only the survivors, so a host saw a target stop appearing in `outcomes` with nothing saying why. Raw account targets are exactly the ones that do not survive a rotation, and the surface has no way to list the ledger, so there was no way to notice or recover. `StatementRenewalReport` now carries the labels it dropped. The information was already in hand at the pruning site; only the count reached the caller. Labels come from one function shared with `resolve_target`, so a pruned entry reads the same as a renewed one, and it derives without an active session because pruning is decided before the ledger is resolved. Reviewed twice on #417 as a gap this leaves open. A reader and an untrack are still missing, tracked separately: those need design decisions this does not. --- android/truapi-host/README.md | 2 +- ios/truapi-host/README.md | 2 +- .../Sources/TrUAPIHost/truapi_server.swift | 49 +++++++++++++++++++ rust/crates/truapi-host-cli/src/main.rs | 7 +++ .../runtime/signing_host/allowance_renewal.rs | 49 ++++++++++++++----- .../runtime/statement_allowance/renewal.rs | 14 ++++++ 6 files changed, 109 insertions(+), 14 deletions(-) diff --git a/android/truapi-host/README.md b/android/truapi-host/README.md index 39d23e8b..a8607bea 100644 --- a/android/truapi-host/README.md +++ b/android/truapi-host/README.md @@ -89,7 +89,7 @@ core.trackStatementRenewalTargets( ) ``` -The ledger persists across launches, and it is append-only: there is no untrack, and an entry is dropped only when the identity that promised it changes. `WalletSso` and `ProductStatementAllowance` are derivation recipes and survive that; `Account` carries a fixed account id and does not, so re-track raw accounts whenever the active identity changes. A pruned target is absent from the report rather than reported as failed. There is no reader and no untrack on this surface: a host cannot list what is tracked, cannot remove a wrong entry, and cannot detect a pruned one except by noticing it missing from a report. Re-tracking is idempotent, so the safe habit is to re-track the full set after every identity change rather than trying to reason about what survived. +The ledger persists across launches, and it is append-only: there is no untrack, and an entry is dropped only when the identity that promised it changes. `WalletSso` and `ProductStatementAllowance` are derivation recipes and survive that; `Account` carries a fixed account id and does not, so re-track raw accounts whenever the active identity changes. A pruned target is absent from the report rather than reported as failed. Pruned targets are listed in `report.pruned`, which is how a host learns to re-track one. There is still no reader and no untrack on this surface, so a host cannot list what is tracked or remove a wrong entry. Re-tracking is idempotent, so the safe habit is to re-track the full set after every identity change rather than trying to reason about what survived. Then run a pass from a `WorkManager` worker. It submits extrinsics and blocks until they are included, so keep it off the main thread. It needs an active session too, which is the whole difficulty here: a worker on a cold start has none until you restore one, and the pass then fails with the bare reason `Disconnected`. Restore the session first, and read that reason as "not ready" rather than as a renewal failure. `startStatementAllowanceRenewal()` does not need this care, since its loop skips a tick with no session and retries. diff --git a/ios/truapi-host/README.md b/ios/truapi-host/README.md index 8067bb51..8133f8a3 100644 --- a/ios/truapi-host/README.md +++ b/ios/truapi-host/README.md @@ -109,7 +109,7 @@ try runtime.trackStatementRenewalTargets([ ]) ``` -The ledger persists across launches, and it is append-only: there is no untrack, and an entry is dropped only when the identity that promised it changes. `.walletSso` and `.productStatementAllowance` are derivation recipes, so they survive that; `.account` carries a fixed account id and does not. Re-track raw accounts whenever the active identity changes, or renewal quietly stops covering them — a pruned target is absent from the report rather than reported as failed. There is no reader and no untrack on this surface: a host cannot list what is tracked, cannot remove a wrong entry, and cannot detect a pruned one except by noticing it missing from a report. Re-tracking is idempotent, so the safe habit is to re-track the full set after every identity change rather than trying to reason about what survived. +The ledger persists across launches, and it is append-only: there is no untrack, and an entry is dropped only when the identity that promised it changes. `.walletSso` and `.productStatementAllowance` are derivation recipes, so they survive that; `.account` carries a fixed account id and does not. Re-track raw accounts whenever the active identity changes, or renewal quietly stops covering them — a pruned target is absent from the report rather than reported as failed. Pruned targets are listed in `report.pruned`, which is how a host learns to re-track one. There is still no reader and no untrack on this surface, so a host cannot list what is tracked or remove a wrong entry. Re-tracking is idempotent, so the safe habit is to re-track the full set after every identity change rather than trying to reason about what survived. Then run a pass from a background task, off the main thread. It needs an active session too, which is the whole difficulty here: a `BGTaskScheduler` wake on a cold start has none until you restore one, and the pass then fails with the bare reason `Disconnected`. Restore the session first, and read that reason as "not ready" rather than as a renewal failure. `startStatementAllowanceRenewal()` does not need this care, since its loop skips a tick with no session and retries. diff --git a/ios/truapi-host/Sources/TrUAPIHost/truapi_server.swift b/ios/truapi-host/Sources/TrUAPIHost/truapi_server.swift index a48d3721..35b81f0d 100644 --- a/ios/truapi-host/Sources/TrUAPIHost/truapi_server.swift +++ b/ios/truapi-host/Sources/TrUAPIHost/truapi_server.swift @@ -4133,6 +4133,17 @@ public struct StatementRenewalReport: Equatable, Hashable { * Per-target outcomes in ledger order. */ public var outcomes: [StatementRenewalOutcome] + /** + * Labels of targets this pass dropped because a different identity + * promised them. + * + * Dropping is silent otherwise: a pruned target simply stops appearing in + * `outcomes`, and the surface has no way to list the ledger, so a host + * could only infer it from an absence. A raw account target does not + * survive a change of root entropy, so this is how a host learns to + * re-track one. + */ + public var pruned: [String] /** * Whether the pass hit slot exhaustion for this period. */ @@ -4147,11 +4158,22 @@ public struct StatementRenewalReport: Equatable, Hashable { /** * Per-target outcomes in ledger order. */outcomes: [StatementRenewalOutcome], + /** + * Labels of targets this pass dropped because a different identity + * promised them. + * + * Dropping is silent otherwise: a pruned target simply stops appearing in + * `outcomes`, and the surface has no way to list the ledger, so a host + * could only infer it from an absence. A raw account target does not + * survive a change of root entropy, so this is how a host learns to + * re-track one. + */pruned: [String], /** * Whether the pass hit slot exhaustion for this period. */slotsExhausted: Bool) { self.period = period self.outcomes = outcomes + self.pruned = pruned self.slotsExhausted = slotsExhausted } @@ -4173,6 +4195,7 @@ public struct FfiConverterTypeStatementRenewalReport: FfiConverterRustBuffer { try StatementRenewalReport( period: FfiConverterUInt32.read(from: &buf), outcomes: FfiConverterSequenceTypeStatementRenewalOutcome.read(from: &buf), + pruned: FfiConverterSequenceString.read(from: &buf), slotsExhausted: FfiConverterBool.read(from: &buf) ) } @@ -4180,6 +4203,7 @@ public struct FfiConverterTypeStatementRenewalReport: FfiConverterRustBuffer { public static func write(_ value: StatementRenewalReport, into buf: inout [UInt8]) { FfiConverterUInt32.write(value.period, into: &buf) FfiConverterSequenceTypeStatementRenewalOutcome.write(value.outcomes, into: &buf) + FfiConverterSequenceString.write(value.pruned, into: &buf) FfiConverterBool.write(value.slotsExhausted, into: &buf) } } @@ -5801,6 +5825,31 @@ fileprivate struct FfiConverterOptionTypeProductAccountId: FfiConverterRustBuffe } } +#if swift(>=5.8) +@_documentation(visibility: private) +#endif +fileprivate struct FfiConverterSequenceString: FfiConverterRustBuffer { + typealias SwiftType = [String] + + public static func write(_ value: [String], into buf: inout [UInt8]) { + let len = Int32(value.count) + writeInt(&buf, len) + for item in value { + FfiConverterString.write(item, into: &buf) + } + } + + public static func read(from buf: inout (data: Data, offset: Data.Index)) throws -> [String] { + let len: Int32 = try readInt(&buf) + var seq = [String]() + seq.reserveCapacity(Int(len)) + for _ in 0 ..< len { + seq.append(try FfiConverterString.read(from: &buf)) + } + return seq + } +} + #if swift(>=5.8) @_documentation(visibility: private) #endif diff --git a/rust/crates/truapi-host-cli/src/main.rs b/rust/crates/truapi-host-cli/src/main.rs index e99e94c5..a114cdc8 100644 --- a/rust/crates/truapi-host-cli/src/main.rs +++ b/rust/crates/truapi-host-cli/src/main.rs @@ -1414,6 +1414,13 @@ async fn run_renew(session: &mut SigningHostSession) -> Result<()> { TargetRenewalStatus::SkippedExhausted => skipped += 1, } } + for label in &report.pruned { + terminal_ui::output_event(SystemEvent::AllowanceRenewalFailed { + target: label.clone(), + reason: "dropped from the ledger: promised by a previous identity, so re-track it" + .to_string(), + }); + } terminal_ui::output_event(SystemEvent::AllowanceRenewalReport { period: report.period, renewed, diff --git a/rust/crates/truapi-server/src/runtime/signing_host/allowance_renewal.rs b/rust/crates/truapi-server/src/runtime/signing_host/allowance_renewal.rs index 8b9346ef..eb8bb6e8 100644 --- a/rust/crates/truapi-server/src/runtime/signing_host/allowance_renewal.rs +++ b/rust/crates/truapi-server/src/runtime/signing_host/allowance_renewal.rs @@ -185,10 +185,23 @@ fn decode_entries(blob: &[u8]) -> Result, String> { } /// Resolve a ledger entry into a concrete account for this session's entropy. +/// The label a target reports under, derivable without an active session so a +/// pruned entry reads the same as a renewed one. +fn target_label(target: &StatementRenewalTarget) -> String { + match target { + StatementRenewalTarget::ProductStatementAllowance { product_id } => { + format!("product:{product_id}") + } + StatementRenewalTarget::WalletSso => "wallet-sso".to_string(), + StatementRenewalTarget::Account { label, .. } => label.clone(), + } +} + fn resolve_target( entropy: &[u8], target: &StatementRenewalTarget, ) -> Result { + let label = target_label(target); match target { StatementRenewalTarget::ProductStatementAllowance { product_id } => { let pair = derive_sr25519_hard_path( @@ -197,7 +210,7 @@ fn resolve_target( ) .map_err(|err| err.to_string())?; Ok(ResolvedRenewalTarget { - label: format!("product:{product_id}"), + label, account_id: pair.public.to_bytes(), }) } @@ -205,12 +218,12 @@ fn resolve_target( let pair = derive_sr25519_hard_path(entropy, &["wallet", "sso"]) .map_err(|err| err.to_string())?; Ok(ResolvedRenewalTarget { - label: "wallet-sso".to_string(), + label, account_id: pair.public.to_bytes(), }) } - StatementRenewalTarget::Account { account_id, label } => Ok(ResolvedRenewalTarget { - label: label.clone(), + StatementRenewalTarget::Account { account_id, .. } => Ok(ResolvedRenewalTarget { + label, account_id: *account_id, }), } @@ -262,7 +275,7 @@ async fn owned_targets( storage: &(impl CoreStorage + ?Sized), ledger_lock: &Mutex<()>, owner: [u8; 32], -) -> Result, String> { +) -> Result<(Vec, Vec), String> { let (owned, foreign): (Vec<_>, Vec<_>) = read_entries(storage) .await? .into_iter() @@ -275,7 +288,13 @@ async fn owned_targets( let _guard = ledger_lock.lock().await; write_entries(storage, &owned).await?; } - Ok(owned.into_iter().map(|entry| entry.target).collect()) + Ok(( + owned.into_iter().map(|entry| entry.target).collect(), + foreign + .iter() + .map(|entry| target_label(&entry.target)) + .collect(), + )) } /// One renewal pass: resolve the ledger against the active session and renew @@ -288,7 +307,7 @@ pub(super) async fn renew_now( let period = statement_allowance::slot::current_period( current_unix_secs().map_err(|err| err.to_string())?, ); - let targets = owned_targets( + let (targets, pruned) = owned_targets( signing_host.platform.as_ref(), signing_host.renewal.ledger_lock(), owner_key(&entropy)?, @@ -299,6 +318,7 @@ pub(super) async fn renew_now( return Ok(StatementRenewalReport { period, outcomes: Vec::new(), + pruned, slots_exhausted: false, }); } @@ -338,14 +358,16 @@ pub(super) async fn renew_now( chain_state: &chain_state, ring: &ring, }; - Ok(renew_targets( + let mut report = renew_targets( &context, bandersnatch, period, &resolved, signing_host.renewal.registration_lock(), ) - .await) + .await; + report.pruned = pruned; + Ok(report) } /// Spawn the periodic renewal loop; repeated calls are no-ops. The loop holds @@ -547,9 +569,12 @@ mod tests { .await .unwrap(); - let targets = owned_targets(&storage, &lock(), OWNER).await.unwrap(); + let (targets, pruned) = owned_targets(&storage, &lock(), OWNER).await.unwrap(); assert_eq!(targets, vec![product("a.dot")]); + // Reported, not just dropped: the pass is a host's only view of the + // ledger, so a silent prune is one it cannot notice or re-track. + assert_eq!(pruned, vec!["device".to_string()]); // Dropped, not merely skipped, so the cost is paid once. assert_eq!( read_entries(&storage).await.unwrap(), @@ -573,7 +598,7 @@ mod tests { assert_eq!( owned_targets(&storage, &lock(), OWNER).await.unwrap(), - Vec::new() + (Vec::new(), vec!["device".to_string()]) ); assert_eq!(read_entries(&storage).await.unwrap(), Vec::new()); }); @@ -589,7 +614,7 @@ mod tests { .unwrap(); let after_seeding = storage.writes(); - let targets = owned_targets(&storage, &lock(), OWNER).await.unwrap(); + let (targets, _pruned) = owned_targets(&storage, &lock(), OWNER).await.unwrap(); assert_eq!(targets, vec![product("a.dot")]); // Every tick calls this; rewriting the ledger each time would be waste. diff --git a/rust/crates/truapi-server/src/runtime/statement_allowance/renewal.rs b/rust/crates/truapi-server/src/runtime/statement_allowance/renewal.rs index a3314a3f..b64fc741 100644 --- a/rust/crates/truapi-server/src/runtime/statement_allowance/renewal.rs +++ b/rust/crates/truapi-server/src/runtime/statement_allowance/renewal.rs @@ -110,6 +110,15 @@ pub struct StatementRenewalReport { pub period: u32, /// Per-target outcomes in ledger order. pub outcomes: Vec, + /// Labels of targets this pass dropped because a different identity + /// promised them. + /// + /// Dropping is silent otherwise: a pruned target simply stops appearing in + /// `outcomes`, and the surface has no way to list the ledger, so a host + /// could only infer it from an absence. A raw account target does not + /// survive a change of root entropy, so this is how a host learns to + /// re-track one. + pub pruned: Vec, /// Whether the pass hit slot exhaustion for this period. pub slots_exhausted: bool, } @@ -260,6 +269,9 @@ fn fold_outcomes( StatementRenewalReport { period, outcomes, + // fold_outcomes only sees targets that survived to be renewed; the + // caller that read the ledger attaches what it dropped. + pruned: Vec::new(), slots_exhausted, } } @@ -476,6 +488,7 @@ mod tests { } ), ], + pruned: Vec::new(), slots_exhausted: false, } ); @@ -506,6 +519,7 @@ mod tests { ), outcome("c", TargetRenewalStatus::SkippedExhausted), ], + pruned: Vec::new(), slots_exhausted: true, } ); From f04e6d0d74930f477e8c229609b484cfe00e48f3 Mon Sep 17 00:00:00 2001 From: tarikgul Date: Mon, 17 Aug 2026 11:23:35 -0400 Subject: [PATCH 2/2] fix(server): hold the ledger lock across the prune's read Pruning rewrites the whole ledger but read it outside the lock, so a `track_targets` landing in the gap was overwritten by a view that predated it and that account was tracked nowhere afterwards. `track_targets` already holds the lock across its own read-modify-write; this now matches. The new test fails without the change, losing the concurrently tracked target outright. The dropped labels are logged where the prune happens rather than only returned. Every step between there and the report can fail, and `run_tick` reads only the exhaustion flag and the outcome count, so the log is the one place a prune is recorded unconditionally. The durable version belongs with the reader. A prune is not a failed renewal, so it no longer borrows the failure event, which rendered it red beside chain rejections and counted toward nothing. There is an `AllowanceRenewalPruned` event and a `pruned` count in the summary, so the all-foreign case reads as three dropped targets rather than three failures followed by "no tracked allowance targets". Both host READMEs said a pruned target was absent from the report one sentence before saying it was listed in it. The stale half is gone and the examples read `report.pruned`. --- android/truapi-host/README.md | 6 +- ios/truapi-host/README.md | 6 +- rust/crates/truapi-host-cli/src/main.rs | 5 +- .../crates/truapi-host-cli/src/terminal_ui.rs | 20 ++++++- .../runtime/signing_host/allowance_renewal.rs | 56 +++++++++++++++++-- 5 files changed, 79 insertions(+), 14 deletions(-) diff --git a/android/truapi-host/README.md b/android/truapi-host/README.md index a8607bea..52a38234 100644 --- a/android/truapi-host/README.md +++ b/android/truapi-host/README.md @@ -89,13 +89,17 @@ core.trackStatementRenewalTargets( ) ``` -The ledger persists across launches, and it is append-only: there is no untrack, and an entry is dropped only when the identity that promised it changes. `WalletSso` and `ProductStatementAllowance` are derivation recipes and survive that; `Account` carries a fixed account id and does not, so re-track raw accounts whenever the active identity changes. A pruned target is absent from the report rather than reported as failed. Pruned targets are listed in `report.pruned`, which is how a host learns to re-track one. There is still no reader and no untrack on this surface, so a host cannot list what is tracked or remove a wrong entry. Re-tracking is idempotent, so the safe habit is to re-track the full set after every identity change rather than trying to reason about what survived. +The ledger persists across launches, and it is append-only: there is no untrack, and an entry is dropped only when the identity that promised it changes. `WalletSso` and `ProductStatementAllowance` are derivation recipes and survive that; `Account` carries a fixed account id and does not. A dropped target is listed in `report.pruned`, which is how a host learns to re-track one and keep renewal covering it. There is still no reader and no untrack on this surface, so a host cannot list what is tracked or remove a wrong entry. Re-tracking is idempotent, so the safe habit is to re-track the full set after every identity change rather than trying to reason about what survived. Then run a pass from a `WorkManager` worker. It submits extrinsics and blocks until they are included, so keep it off the main thread. It needs an active session too, which is the whole difficulty here: a worker on a cold start has none until you restore one, and the pass then fails with the bare reason `Disconnected`. Restore the session first, and read that reason as "not ready" rather than as a renewal failure. `startStatementAllowanceRenewal()` does not need this care, since its loop skips a tick with no session and retries. ```kotlin val report = core.renewStatementAllowances() report.outcomes.forEach { Log.i(TAG, "${it.label}: ${it.status}") } +report.pruned.forEach { + // Promised by a previous identity and discarded; re-track to keep it renewed. + Log.w(TAG, "dropped: $it") +} if (report.slotsExhausted) { // Every slot for this period is taken and none was replaceable. } diff --git a/ios/truapi-host/README.md b/ios/truapi-host/README.md index 8133f8a3..55bc8c22 100644 --- a/ios/truapi-host/README.md +++ b/ios/truapi-host/README.md @@ -109,7 +109,7 @@ try runtime.trackStatementRenewalTargets([ ]) ``` -The ledger persists across launches, and it is append-only: there is no untrack, and an entry is dropped only when the identity that promised it changes. `.walletSso` and `.productStatementAllowance` are derivation recipes, so they survive that; `.account` carries a fixed account id and does not. Re-track raw accounts whenever the active identity changes, or renewal quietly stops covering them — a pruned target is absent from the report rather than reported as failed. Pruned targets are listed in `report.pruned`, which is how a host learns to re-track one. There is still no reader and no untrack on this surface, so a host cannot list what is tracked or remove a wrong entry. Re-tracking is idempotent, so the safe habit is to re-track the full set after every identity change rather than trying to reason about what survived. +The ledger persists across launches, and it is append-only: there is no untrack, and an entry is dropped only when the identity that promised it changes. `.walletSso` and `.productStatementAllowance` are derivation recipes, so they survive that; `.account` carries a fixed account id and does not. A dropped target is listed in `report.pruned`, which is how a host learns to re-track one and keep renewal covering it. There is still no reader and no untrack on this surface, so a host cannot list what is tracked or remove a wrong entry. Re-tracking is idempotent, so the safe habit is to re-track the full set after every identity change rather than trying to reason about what survived. Then run a pass from a background task, off the main thread. It needs an active session too, which is the whole difficulty here: a `BGTaskScheduler` wake on a cold start has none until you restore one, and the pass then fails with the bare reason `Disconnected`. Restore the session first, and read that reason as "not ready" rather than as a renewal failure. `startStatementAllowanceRenewal()` does not need this care, since its loop skips a tick with no session and retries. @@ -118,6 +118,10 @@ let report = try runtime.renewStatementAllowances() for outcome in report.outcomes { log("\(outcome.label): \(outcome.status)") } +for label in report.pruned { + // Promised by a previous identity and discarded; re-track to keep it renewed. + log("dropped: \(label)") +} if report.slotsExhausted { // Every slot for this period is taken and none was replaceable. } diff --git a/rust/crates/truapi-host-cli/src/main.rs b/rust/crates/truapi-host-cli/src/main.rs index a114cdc8..d9c200de 100644 --- a/rust/crates/truapi-host-cli/src/main.rs +++ b/rust/crates/truapi-host-cli/src/main.rs @@ -1415,10 +1415,8 @@ async fn run_renew(session: &mut SigningHostSession) -> Result<()> { } } for label in &report.pruned { - terminal_ui::output_event(SystemEvent::AllowanceRenewalFailed { + terminal_ui::output_event(SystemEvent::AllowanceRenewalPruned { target: label.clone(), - reason: "dropped from the ledger: promised by a previous identity, so re-track it" - .to_string(), }); } terminal_ui::output_event(SystemEvent::AllowanceRenewalReport { @@ -1427,6 +1425,7 @@ async fn run_renew(session: &mut SigningHostSession) -> Result<()> { fresh, failed, skipped, + pruned: report.pruned.len(), }); if report.slots_exhausted { diff --git a/rust/crates/truapi-host-cli/src/terminal_ui.rs b/rust/crates/truapi-host-cli/src/terminal_ui.rs index a3626ed4..0962d295 100644 --- a/rust/crates/truapi-host-cli/src/terminal_ui.rs +++ b/rust/crates/truapi-host-cli/src/terminal_ui.rs @@ -140,12 +140,16 @@ pub enum SystemEvent { target: String, reason: String, }, + AllowanceRenewalPruned { + target: String, + }, AllowanceRenewalReport { period: u32, renewed: usize, fresh: usize, failed: usize, skipped: usize, + pruned: usize, }, NotificationDelivered { id: u32, @@ -1348,21 +1352,31 @@ impl App { Some(reason), ActivityState::Failed, ), + // A prune is not a failed renewal: nothing was rejected, an entry + // this identity never promised was discarded. Rendering it red beside + // chain rejections reads as an error the host should chase. + SystemEvent::AllowanceRenewalPruned { target } => self.activity( + format!("allowance:{target}"), + format!("{} dropped from the ledger", allowance_name(&target)), + Some("promised by a previous identity; re-track it to keep it renewed".to_string()), + ActivityState::Warning, + ), SystemEvent::AllowanceRenewalReport { period, renewed, fresh, failed, skipped, + pruned, } => { - if renewed + fresh + failed + skipped == 0 { + if renewed + fresh + failed + skipped + pruned == 0 { self.notice( NoticeTone::Info, "No tracked allowance targets".to_string(), Some(format!("Statement period {period}")), ); } else { - let tone = if failed + skipped > 0 { + let tone = if failed + skipped + pruned > 0 { NoticeTone::Warning } else { NoticeTone::Success @@ -1371,7 +1385,7 @@ impl App { tone, "Allowance renewal finished".to_string(), Some(format!( - "Period {period} · {renewed} renewed · {fresh} fresh · {failed} failed · {skipped} skipped" + "Period {period} · {renewed} renewed · {fresh} fresh · {failed} failed · {skipped} skipped · {pruned} pruned" )), ); } diff --git a/rust/crates/truapi-server/src/runtime/signing_host/allowance_renewal.rs b/rust/crates/truapi-server/src/runtime/signing_host/allowance_renewal.rs index eb8bb6e8..df382621 100644 --- a/rust/crates/truapi-server/src/runtime/signing_host/allowance_renewal.rs +++ b/rust/crates/truapi-server/src/runtime/signing_host/allowance_renewal.rs @@ -271,29 +271,39 @@ fn resolve_targets( /// an account it never promised, so it is removed rather than skipped — the cost /// is paid once per identity change instead of on every tick. The ledger is only /// rewritten when something was actually dropped. +/// +/// The lock covers the read as well as the write, because this is a +/// read-modify-write of the whole ledger: reading outside it lets a +/// [`track_targets`] call land in the gap and be overwritten by a view that +/// predates it, leaving that account tracked nowhere and never renewed again. +/// +/// The labels are logged here rather than only returned. Every step between this +/// and the report can fail, and `run_tick` does not read the report at all, so +/// the log is the one place a prune is recorded unconditionally. async fn owned_targets( storage: &(impl CoreStorage + ?Sized), ledger_lock: &Mutex<()>, owner: [u8; 32], ) -> Result<(Vec, Vec), String> { + let _guard = ledger_lock.lock().await; let (owned, foreign): (Vec<_>, Vec<_>) = read_entries(storage) .await? .into_iter() .partition(|entry| entry.is_owned_by(owner)); + let pruned: Vec = foreign + .iter() + .map(|entry| target_label(&entry.target)) + .collect(); if !foreign.is_empty() { warn!( - dropped = foreign.len(), + dropped = ?pruned, "pruning renewal targets promised by a previous identity" ); - let _guard = ledger_lock.lock().await; write_entries(storage, &owned).await?; } Ok(( owned.into_iter().map(|entry| entry.target).collect(), - foreign - .iter() - .map(|entry| target_label(&entry.target)) - .collect(), + pruned, )) } @@ -550,6 +560,40 @@ mod tests { }); } + /// Pruning rewrites the whole ledger, so it has to hold the lock across its + /// read too. Reading outside it lets a `track_targets` land in the gap and be + /// overwritten by a view that predates it, leaving that account tracked + /// nowhere and never renewed again. + #[test] + fn a_prune_does_not_overwrite_a_concurrent_track() { + let storage = YieldingStorage::default(); + let ledger_lock = lock(); + let device = StatementRenewalTarget::Account { + account_id: [9; 32], + label: "device".to_string(), + }; + + futures::executor::block_on(async { + // A foreign entry, so the pass prunes and therefore writes. + track_targets(&storage, &ledger_lock, OTHER_OWNER, vec![device]) + .await + .unwrap(); + + let (pruned, tracked) = futures::join!( + owned_targets(&storage, &ledger_lock, OWNER), + track_targets(&storage, &ledger_lock, OWNER, vec![product("a.dot")]), + ); + pruned.unwrap(); + tracked.unwrap(); + + assert_eq!( + read_targets(&storage, OWNER).await.unwrap(), + vec![product("a.dot")], + "the concurrently tracked target was overwritten by the prune" + ); + }); + } + /// A raw account promised by a previous identity must not be renewed under a /// later one: it would spend that identity's slots on an account it never /// promised.