feat(server): report renewal targets dropped by an identity change - #423
feat(server): report renewal targets dropped by an identity change#423TarikGul wants to merge 1 commit into
Conversation
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.
| @@ -275,7 +288,13 @@ async fn owned_targets( | |||
| let _guard = ledger_lock.lock().await; | |||
There was a problem hiding this comment.
The lock is taken after the ledger was read, so a track_targets call landing in between is silently overwritten and that account is never renewed again. Moving this line above the read_entries call fixes it. It fails on main too, so happy for it to be a follow-up.
| ) | ||
| .await) | ||
| .await; | ||
| report.pruned = pruned; |
There was a problem hiding this comment.
By the time this line runs the prune is already on disk, but nothing guarantees the report reaches anyone: run_tick reads only slots_exhausted and outcomes.len(), and every step above can return Err and take the labels with it. Please make the warn! in owned_targets log dropped = ?pruned instead of a count, since that runs before any of those can fail. The durable version belongs with the reader you are already tracking.
| ``` | ||
|
|
||
| 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. |
There was a problem hiding this comment.
These two sentences contradict each other: the first says a pruned target is absent from the report, the second says it is listed in it. Please drop the first rather than keep both, and add a report.pruned line to the example below, which currently reads only outcomes and slotsExhausted. Both apply to ios/truapi-host/README.md at the matching paragraph, where the stale half also says renewal "quietly stops covering them".
| TargetRenewalStatus::SkippedExhausted => skipped += 1, | ||
| } | ||
| } | ||
| for label in &report.pruned { |
There was a problem hiding this comment.
A prune is not a renewal failure, but AllowanceRenewalFailed renders as "{name} renewal failed" in the same red state as a chain rejection, and nothing here increments a counter. In the all-foreign case, which is what this PR is for, that means a red failure line per label followed by the info notice "No tracked allowance targets". Please add an AllowanceRenewalPruned event and a pruned count on AllowanceRenewalReport; I tried it and clippy, fmt and the suite stay clean.
3 participants
A renewal 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
outcomeswith nothing saying why. RawAccounttargets 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.StatementRenewalReportcarries the labels it dropped. The information was already in hand at the pruning site; only the count reached the caller, so this is one additive field rather than new API surface.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 against entropy.The CLI reports each dropped target with the reason and what to do about it. Both host READMEs now point at
report.prunedinstead of telling hosts a pruned entry is undetectable.Scope
Raised twice in review on #417. A reader and an untrack are still missing and are tracked separately: those need decisions this does not, and the report may reduce how much a reader is worth.
Verification
Live through the CLI:
/renewagainst the People chain, period 20682, three targets renewed. The two pruning tests now assert the reported labels, not just that entries were dropped.