feat(server): read the statement renewal ledger, and untrack from a native host - #786
Conversation
…ative host The ledger could be appended to and, from the core, untracked by account id, but nothing could read it back. A host could not answer which entries were spending its finite per-period slots, and a native host could not remove a wrong one at all: only track was exposed across the UniFFI boundary. Add a reader that maps the persisted entries straight out, without resolving them. Resolution needs root entropy, which a scheduled host woken on a cold start does not hold, so the reader takes storage alone and reports the owner per entry instead. Expose the reader and the existing untrack on NativeTrUApiHostRuntime, next to track. The reader reports every entry rather than only the active identity's, since a foreign entry occupies a row until a pass prunes it, and the owner is what tells the two apart.
TarikGul
added
the
no-changeset
github-actions
Bot
added
the
host-work
|
CI Status: 17 required jobs green, 15 passed and 2 skipped by path filter. All job results
Commit |
|
@lore-bot-app review |
|
Reading the diff and checking what the record says. Back in a few minutes. |
![lore-bot-app[bot]](https://avatars.githubusercontent.com/u/14176906?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: read the statement renewal ledger, and untrack from a native host
TL;DR — Adds a session-free reader for the renewal ledger and exposes reader plus the existing untrack over UniFFI. 2 blocking, 3 minor: the two documented Kotlin/Swift methods do not exist on the host shells, and the owner field the reader returns is not comparable to anything a native host can obtain.
Summary
SigningHost::statement_renewal_targets reads the persisted ledger under the ledger lock and maps entries out without resolving them, so it works with no root entropy and no session. That surfaces through SigningHostRuntime and, new at the UniFFI boundary, through NativeTrUApiHostRuntime alongside untrack_statement_renewal_account, which existed in the core but had never been exported. Each listed entry carries the persisted owner, so a caller can tell a derivation recipe (no owner, resolves under any identity) from a fixed account pinned to one root key. The android and iOS READMEs drop their "there is no reader and no untrack" caveat and document both calls. Tests cover track order, a foreign entry being listed without a write, an undecodable ledger, and both boundary conversions.
What the record says
- Issue #429, "Renewal ledger has no reader and no untrack" is the ticket this implements, filed by the same author. It asks for an API that "handles identity scoping and entry equality without breaking existing Swift bindings", and names untrack "by value or index". This PR does identity scoping and untrack by account id only. The by-value case for recipes is not covered, see Concerns 4.
- Issue #376 settled the shape of this surface: export renewal entry points over UniFFI and let each host own its OS schedule, rather than adding a platform trait. The core side landed in #417. The "a scheduled wake can read this before deciding whether the pass is worth running" framing in the new docs follows that decision directly.
- PR #308 introduced the ledger, the registration lock and the tick, which is where
read_entriesdiscarding an undecodable blob comes from. - PR #378, "replace the oldest slot when a period is full" qualifies the new README rationale. Registration replaces the oldest slot past a cooldown (60s on paseo-next-v2) and only reports
NoFreeStatementStoreSlotwhen nothing is replaceable. A stale entry therefore does not permanently hold a slot against you, which makes "worth removing rather than leaving to consume one" weaker than the READMEs state. - The record does not contain the figure the READMEs now assert. I found no discussion fixing
LiteStmtStoreSlotsPerPeriodat 10, and no constant for it in this repo. Issue #429 names the symbol but not a value. - Ownership: TarikGul is the clear owner of this area by a wide margin (
who_knows), followed by replghost and pgherveou.
Concerns
1. The documented Kotlin and Swift methods do not exist. android/truapi-host/README.md:177,179 and ios/truapi-host/README.md:245,247 document statementRenewalTargets() and untrackStatementRenewalAccount(...) on runtime, which in both READMEs is the hand-written shell, not the native inner object (runtime.trackStatementRenewalTargets at android/truapi-host/src/main/kotlin/io/parity/truapi/TrUAPIHost.kt:841, ios/truapi-host/Sources/TrUAPIHost/TrUAPIHost.swift:782). Neither shell forwards the two new calls. Swift is the larger gap: it defines its own StatementRenewalTarget enum at TrUAPIHost.swift:825 with only a forward .native mapping, so there is no Swift type for the reader to return and no reverse conversion. Neither compile gate catches this, because adding methods to native.rs cannot break a shell that ignores them. As it stands an app following either README will not compile.
2. owner is not actionable from a native host. rust/crates/truapi-server/src/native.rs:779 hands back a raw Option<Vec<u8>> root key, and the docs say a fixed account under another identity "is ignored under any other identity". But NativeTrUApiHostRuntime exposes no accessor for the active root public key; the closest are session_chat_identity_key (native.rs:1140) and product_subtree_public_key (native.rs:1158), neither of which is this value. A host can see that two entries differ but cannot tell which ones will actually be renewed, which is the audit the reader exists for. Either resolve it at the boundary into a flag ("will renew under the active identity"), or expose the root key.
3. Vec<u8> where the file already has a UniFFI custom type for 32 bytes. native.rs:779 and the account_id: Vec<u8> parameter at native.rs:894 hand-roll byte vectors and a length check, while the same file carries Bytes32 across the boundary at lines 1140, 1148 and 1162 with lift/lower coverage at 3840. CLAUDE.md asks for "custom conversions for unsupported leaf values instead of defining parallel Native* mirrors". Using Bytes32 for owner and for the untrack argument deletes the length check and the InvalidAccountId path from the new code. The pre-existing NativeStatementRenewalTarget doc at native.rs:700 justifies Vec<u8> on the grounds that "UniFFI carries byte arrays as Vec<u8> rather than a fixed width", which Bytes32 shows is not so.
4. Recipes can be listed but never untracked. untrack_account at rust/crates/truapi-server/src/runtime/signing_host/allowance_renewal.rs:216 only matches StatementRenewalTarget::Account. The reader now lists ProductStatementAllowance and WalletSso entries, so a host can see a stale product entry for a product it no longer runs and has no way to remove it; it is re-resolved and renewed every period for the life of the identity. The READMEs say untrack "drops one fixed account" but never say the other two variants have no removal path, and issue #429 explicitly asked for untrack by value.
5. No changeset. .github/workflows/ci.yml:275 includes rust/crates/truapi-server/ in the needs_changeset filter, and the PR adds none (.changeset/ holds only two unrelated entries). Changeset guard will fail unless the no-changeset label is applied.
Questions for the author
- Where does "10 on
paseo-next-v2" come from? It appears twice in the new docs, the repo has no such constant, and I found nothing in the record pinningLiteStmtStoreSlotsPerPeriodto a value. Given PR #378's replacement-after-cooldown behavior, is a stale entry actually costing a slot, or only costing a ledger row until the next pass prunes it? list_entries(allowance_renewal.rs:201) documents the lock as stopping a caller from observing a read-modify-write "half applied". Sincewrite_entriesis a singlewrite_core_storageof one blob, a lock-free reader would see either the old blob or the new one. Is a hostCoreStorageimplementation allowed to tear a write, or should the comment just say the lock keeps the reader off the same critical section?- An undecodable ledger returns
Ok([])throughread_entries(allowance_renewal.rs:158), and the new test codifies that for the reader. For a renewal pass, rebuilding from empty is the right call. For a host using the reader to decide whether the pass is worth running, "corrupt" and "nothing tracked" both read as "skip the pass". Is that acceptable, or should the reader distinguish them? - Was leaving the Kotlin and Swift shells untouched deliberate, with a follow-up PR planned, or did the README edits get ahead of the shells?
No prompt-injection style content in the diff.
🤖 Reviewed by Lore (Parity knowledge base) · 34 agent turns · 303.0s · knowledge as of 2026-09-15
…r comparable The READMEs documented statementRenewalTargets and untrackStatementRenewalAccount on the runtime an app actually holds, which is the hand-written shell rather than the UniFFI object. Neither shell forwarded them, and Swift had no reverse conversion from NativeStatementRenewalTarget, so an app following either README would not have compiled. Neither compile gate catches that, because adding methods to native.rs cannot break a shell that ignores them. Add the forwarding to both shells, plus a Swift TrackedStatementRenewalTarget and the missing init(native:). An owner was also not actionable: the boundary handed back a root public key with nothing to compare it against. Add statement_renewal_owner_key, which returns the active identity's key and fails with Disconnected without a session, and carry the owner as Bytes32 rather than a hand-rolled Vec<u8>, since the file already uses that custom type for 32-byte keys. Also correct the list_entries lock comment: write_entries stores a single blob, so a lock-free reader could not observe a torn write. The lock keeps a listing off the same critical section, which is a weaker and truthful claim.
The three listing tests each passed their own fresh mutex, so nothing ever contended and the lock could be deleted from list_entries with all of them still green. Poll the track first so it holds the lock across its read, which is what makes the guarantee observable: the listing then reports the settled ledger instead of the one the track is replacing. The READMEs also claimed a stale entry consumes a finite slot. Registration replaces the oldest slot past its cooldown once a period is full, so it costs an allocation attempt each period rather than denying a slot outright. Say that instead, and say that only a fixed account can be untracked: the two recipe variants have no removal path.
|
Closing out the rest of the bot review. The slots figure. It is in the repo, But the point about #378 is right, and it is the better catch. Recipes cannot be untracked. Also right. Undecodable ledger reads as empty. Still true, and the reader inherits it from The two blocking concerns and the |
2 participants
Closes: #429
statement_renewal_targetsreturns the renewal ledger's persisted entries as stored, in track order. It reads storage alone and resolves nothing, because resolution needs root entropy that a scheduled host woken on a cold start does not hold. Each entry carries its owner, so a caller can tell a recipe that resolves under any identity from a fixed account that only one identity promised. It reports every entry rather than just the active identity's, since a foreign one occupies a row until a pass prunes it.NativeTrUApiHostRuntimecarries that reader alongside track and the untrack the core already had, so an iOS or Android host can audit which entries are spending its finite per period slots and drop a stale one.On the open questions in the issue: untrack already exists and is already scoped to the active identity, so those two are settled. This takes raw entries over resolved ones for the reason above.