Repository navigation
Reduce reply-dependent traffic in private recovery - #999
Merged
Merged
Conversation
…crement 1, V1) Pin wallet-libraries main, which brings the wallet-pir transparent crates from crates.io at 0.1.0-rc.4, and adopt what changed there: - Transparent recovery makes one counted pass per account per sync. A service that asks the wallet to wait (a lagging publication or a capacity refusal) is not asked about that account again until its delay has passed; nothing waits inside a sync any more. Window growth and further pages wait for the next sync. - A directory entry whose replies failed three times holds the account, and the balance shows the stop reason a stalled recovery already shows. - Enhance lookups share one in-memory session per wallet for the life of the app, so the library can count rejected records per position and stop asking after three. The recovery wallet can now report unreadable records; the drain starts using that in the next change. - A receipt lookup the wallet gave up on reads as unavailable, as before, but the receipt no longer polls for it or asks for it to be looked up first. Tests: the recovery helpers now repeat runs as successive syncs would, the in-run publication wait tests are replaced by delay tests, and new tests cover the failed-entry hold, the format-6 companion upgraded in place, and the given-up receipt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t 1, V2) When the wallet asks a private service anything now depends only on what the service already sees, never on what a reply said: - Enhance: a record that arrives in a decoded row but does not decode itself is reported against its transaction's requests in that row and the rest of the batch is still asked, so the traffic matches an honest reply. After three such failures the wallet stops asking for it. A failed row query still stops the batch, as before. A record the wallet rejects no longer stops the batch either. - Enhance, status and history lookups run one pass per drain point: work a reply unlocks waits for the next one instead of starting another round. - Status: a private reply that does not decode leaves its transaction for a later sync and keeps private status on for the others. - Transparent recovery no longer holds an account after repeated stalls; holds come only from the library. Recovery runs once after the first foreground sync that follows a new block (or the first sync since the app started, a settings change, or forgetting the private ledger). - The recovery restart schedule doubles on its own timetable and starts over only on a new chain tip, a resume, a settings change or a lock, never because the outstanding count moved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eipts Two gaps from review of the first change: - Every handle the app opens on one wallet now has a test that it counts Enhance content failures in the same session: three unreadable replies recorded through one handle suspend the request for a second handle on that wallet, while a wallet at another path keeps its own allowance. The test reuses the private shielding fixture, now split so its pending memo query can be handed to Vizor's own handles. - Three content failures at one mined height now have a receipt test: the view becomes given up, distinct from unavailable, no later run asks again, and the receipt's detail state says given up. - The Dart check of which receipt states are waited on lists every state instead of defaulting, so the given-up arm cannot be dropped silently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lock Review found two ways a bad reply still changed what the wallet asked before the next block: - The recovery-only restart counted Enhance requests whose reply did not decode, or whose record the wallet rejected, and private status work whose reply was malformed. An honest reply completes that work and the count drops to zero; a bad one left it at one, so the app restarted the sync thirty seconds later at the same block and asked again. Such work is now recorded in memory for the wallet and left out of the restart count until the chain tip moves. Refusals, overloads and transport errors still count; the service already knows it refused. - After stale routing, the retry re-read the wallet's work, which still held the request an honest reply would have completed, so it asked one extra row and counted the failure twice. Enhance requests that failed on content are now left out of every query at that tip, including the retry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… replies A checkpoint makes one pass over status and history: the next address range its history reply opens waits for the next checkpoint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The pinned library reports a malformed display map as unavailable, since the service chooses it without knowing any txid; the receipt test now expects that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An honest "not found" for an unmined transaction leaves its status work in place, while an honest "mined" completes it, so leaving out only malformed replies still let the restart count, and a same-block sync after a resume, tell replies apart. Every private status txid that got any reply at this block, honest or not, is now left out of the recovery-only restart count and is not asked again until the next block. Request failures are not replies and still count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t began with The payload drain at the end of a checkpoint read the wallet's work after status and history had run, so a status reply that made a transaction mined unlocked Enhance queries in the same checkpoint. The checkpoint now records the private queries that exist when it begins, and its drain asks only those; anything unlocked during it waits for the next drain point. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each drain point (a checkpoint, or the payload drain at sync start) compares the private work before and after it runs. Enhance queries, rediscovery jobs and status lookups that appeared are recorded as created by replies at this block: they are not asked, and not counted towards a recovery-only restart, until the chain tip moves. A status reply that makes a transaction mined therefore no longer brings its Enhance query, or a restart for it, at the same block. The drain that ends a checkpoint now also limits rediscovery jobs to those that existed when the checkpoint began, so a job a reply created does not download its block in the same checkpoint. A private status row without the transaction's record counts as a reply about it: the transaction waits for the next block, and the sync fails without retrying at the same tip. The per-block record is reset whenever the sync sees the tip advance, not only when a sync starts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y tip Repins every zakura-core/wallet-libraries revision to c0573729 (the tip of wallet-libraries #197: private status polls on a fixed schedule and a random wait before an Enhance position's first query, both kept in the open wallet's session memory, and a fixed restore window for candidate transparent recovery). It takes the wallet-pir transparent crates from git at 0.1.0-rc.4. Only what the new API requires, with today's behaviour: the removed PreparedWork::requests_in_row is rebuilt from batches_by_tx_and_row, the status protocol's new Unreadable error still reads as Malformed, and a waiting Enhance position is, like a suspended one, never asked. The library now holds each Enhance position's wait in the shared Enhance session, and a wallet's first read in a session waits the full delay. The private-memo test fixture therefore makes its query due through the app's own handle, as the library's test database does, and so does the test that copies it to a second wallet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r requests Tests only. On the parent commit ten fail: Status (change B): - an_unreadable_status_reply_for_one_txid_does_not_skip_the_next: txid B is skipped after A's unreadable reply. - honest_and_forged_mined_answers_get_the_same_status_requests: the forged run stops asking at the first tip, the honest one at the second. - incomplete_private_status_coverage_leaves_the_sync_successful: the sync fails with PrivateStatusCoverageIncomplete. - a_settled_status_poll_is_asked_and_its_answer_discarded: the answer is applied. - an_unreadable_private_status_reply_defers_only_its_transaction and private_status_is_asked_once_per_block_whatever_the_reply now use the library's Unreadable, and fail as a request outcome. Enhance (change C): - a_truncated_row_reply_leaves_the_request_transcript_unchanged: the transcript ends at the truncated row. - row_request_failures_count_nothing_and_finish_the_batch: likewise. - an_unreadable_row_reply_reports_that_rows_requests: nothing is reported. - three_unreadable_rows_suspend_the_position_in_the_wallet: the real database never suspends the position. a_refused_status_request_still_defers_the_checkpoint passes on the parent: it guards the request outcome that must keep deferring. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the replies Change B of the remaining-leaks plan, on top of increment 1's per-block rules. The library's poll window decides which private status lookups are due at a tip; increment 1's record of txids answered or created at this block still bounds what is sent. In the status driver, every private reply records that tip's poll with record_private_status_poll and marks the txid answered, readable or not, and the next transaction is asked: - An unreadable row (the library's new Unreadable) and missing coverage are content outcomes: the poll is recorded, the work waits for its next scheduled tip, and private status stays on. - Only request outcomes (refusals, overload, a stale publication, transport errors, a reply envelope that does not match the session, which the library now reports as Malformed) defer the rest of the session's private status. They record nothing, so the work stays due. - Missing coverage no longer fails the sync, so SyncError's PrivateStatusCoverageIncomplete and Dart's polling stop for it are gone. It still counts as a reply about its transaction, which waits for the next block. - A poll the library marks settled is still sent, and its answer dropped: nothing is stored and nothing is resubmitted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Change C of the remaining-leaks plan. Increment 1 already counted a record that does not decode, and a record the wallet rejects, and kept going. What was left: - A row whose reply arrived for the session but does not decrypt (Pir) or decode (a short row) now counts against that row's requests through RecoveryWallet::record_unreadable, and the batch continues. Before, it ended the pass and turned private Enhance off for the sync, so the positions after it were never sent. - A request outcome inside a row (a transport, HTTP, JSON or generation error, a body over the size limit, or a reply whose framing, binding or length does not match the session) still defers private Enhance, but only after the batch has been asked to its end, so the transcript is the one an honest service sees. The Enhance client reports envelope and row failures with the same untyped Response error, so the envelope messages are listed by name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nding Tests only. On the parent commit a_waiting_enhance_position_is_pending_without_a_restart fails: the recovery status counts the waiting position nowhere, so it reads as complete. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uery as pending Vizor's part of change D. The library now holds each newly queued Enhance position for a random 0 to 2 minutes (EnhancePirWork::Waiting). The recovery status lists those positions with the work that cannot be retried now, so they read as pending rather than complete, and they bring no recovery-only restart: the checkpoint at each block asks them once due. The recovery log counts them too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r a bad reply Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…polls Tests only. The library now keeps private status polls in a session in memory (StatusPollSession), like the Enhance allowance. On the parent commit every_handle_on_a_wallet_shares_its_status_polls fails with "another handle on the wallet sees the poll": each handle the app opens starts its own session, so a poll recorded during a sync is due again through the next handle at the same tip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The library keeps a wallet's private status polls in memory, in a StatusPollSession, as it keeps the Enhance allowance and waits in an EnhancePirSession. Every handle the app opens on a wallet now carries the same session, keyed by the wallet's path like the Enhance session, so a poll recorded during a sync is not due again at the same tip through the next sync's handle or the recovery status read. An app restart starts a fresh session. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s rules wallet-libraries #197 dropped its private status poll window: no ten-tip window, no expiry cover query, no shared poll session, no settled polls and no separate error for an unreadable status row. The broadcast service already sees when a transaction is broadcast, and a fixed window plus a lone expiry query would still show timing, so status polling stays as it was. This undoes the Vizor side of that window: - Every wallet handle no longer carries a shared status poll session, and the test for it is gone. - The status driver no longer records a poll for each reply or sends polls whose answer it drops. It is back to increment 1: each private status txid is asked at most once per block, whatever the reply; a reply never restarts the sync at the same block; work a reply creates waits for the next block; and a request failure (an HTTP refusal, a transport error) stops private status for the rest of the session and may be retried later. - Missing coverage again fails the sync without a retry at the same block, and the app again pauses automatic sync for it while private queries stay on, as in increment 1. - The status tests that assumed the window (honest and forged answers getting the same requests across tips, a settled poll, an unreadable status row) are removed, and the per-block status tests are back to their increment 1 form. The test that a refused status request still defers the rest of the block's private status stays. - The Enhance README keeps what it says about Enhance after a bad reply and the wait before a first query; its status section is back to increment 1. The Enhance changes of this increment are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s poll window Repins every zakura-core/wallet-libraries revision to 2278565f (the tip of wallet-libraries #197 after it dropped the private status poll window). The library keeps its existing status polling: unresolved transactions are asked under the existing expiry rules until they are conclusively settled. It no longer has a status poll session, settled polls, a call to record a poll, or a separate error for a status row that does not decrypt or decode; such a row is again a malformed reply, as in increment 1, so Vizor's mapping for the removed error goes too. What #197 still brings is unchanged here: a random wait of up to two minutes before a new Enhance position's first query, and a fixed restore window of 50 external and 25 internal addresses for candidate transparent recovery. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…till grow the window The test that a forged receive does not change when transparent recovery runs forged it at the last derived address. Since wallet-libraries #197, an account that is still restoring watches a fixed window of addresses (external indexes below 50, internal below 25) whatever the replies show, so a receive there grows nothing, and the test's check that the forged receive grew the window failed. The privacy check itself still held: the honest and forged runs sent the same requests at the same blocks. The forged receive now sits at the restore window's last address, the only place a reply can still grow the window, so the test again compares an honest run with one whose window a forged reply grows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two history checks show a privately recovered receive and send as settled, then grow the account's address window without covering the new addresses and expect both to read as unsettled again. Since wallet-libraries #197, an account that is restoring checks a fixed window of addresses (every external index below 50) before it is activated. Growing the window into those addresses withdraws nothing, because the wallet has already looked there, so the transactions stayed settled and the checks failed. That is the intended result of the fixed window: once a restore has checked an address, later window growth onto it does not make the history provisional again. The fixture now repeats the growth, each run covering what the last one added, until the window reaches addresses the restore never checked. The expected history lines are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Repins every zakura-core/wallet-libraries revision to 0749f95b, main after #197 merged. Since the previous pin the library caps the random wait before a new Enhance position's first query at 75 seconds instead of two minutes, keeps a position's wait across read-only reads, and takes the wallet-pir transparent crates from crates.io at 0.1.0-rc.4 rather than from git. No Vizor code needed changing; the Enhance README now gives the 75-second bound. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ore window Three private recovery history tests show a recovered receive or send as settled, grow the account's address window once without covering the new addresses, and expect it to read as unsettled again. Since wallet-libraries #197 a restoring account checks a fixed window of addresses (every external index below 50) before activation, so growing the window into them withdraws nothing and the rows stayed settled. These failed on the branch before this commit as well (at the #197 head), not because of the repin to main. As in the history golden fixtures, the growth now repeats, each run covering what the last one added, until the window reaches addresses the restore never checked. What the tests check is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
p0mvn
marked this pull request as ready for review
October 14, 2026 17:36
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
czarcas7ic
reviewed
Oct 14, 2026
Contributor
Author
|
@codex review |
Contributor
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ed080783b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
p0mvn
marked this pull request as ready for review
October 14, 2026 23:11
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The wallet and voting dependencies now use published releases:
zcash_voting =5.2.0-rc.1,voting-crypto-deps =0.3.0, Common 3.0.0, and the coordinated wallet/PIR crates. Temporary Git patches for this dependency family are removed. The voting release is tagged onrelease/v5.2.x; 5.1.x retains its existing dependencies. LRZ clients must opt into 5.2 only after adapting and validating their wallet integration, including existing databases.This published wallet revision predates the publication-wait preservation and first-query-delay removal used by the previous dependency pin. Historical Orchard proving requests return an unsupported-circuit error; verification keeps its branch-specific circuit selection.
A private lookup hides what the wallet asks for, but a service can still learn from what the wallet does after receiving a reply: asking again immediately, fetching newly discovered work, stopping partway through a batch, or changing its retry timing. This PR reduces those reply-dependent reactions across transparent recovery, transaction payload recovery, status checks, and receipt display.
It consolidates and supersedes #991 and #993, including the accepted-reorg scheduling changes and the restriction that keeps reply-created lookups behind the next block.
These changes apply to the shared wallet core and app recovery behavior on desktop and mobile. They do not provide full traffic anonymity or cover traffic. Explicitly public work can still run in the same pass, a status observation can still lead to rebroadcast, and missing private status coverage still stops automatic sync pending a Settings decision. No public transaction lookup is added as a fallback for failed private recovery.
Randomized first-query waiting is deferred for separate analysis of timing privacy and display latency. This PR removes the timer and waiting-state plumbing, while retaining the next-block restriction on work created by private replies. The dependency removal is tracked in wallet-libraries #205; its tested head is pinned here, together with wallet-libraries #208, which preserves public publication waits when a selected private entry is malformed.
Breaking changes
Validation
The published dependency graph resolves successfully, with the wallet/voting/PIR family entirely from crates.io. The all-target Rust check and focused regressions are pending on this published dependency update. Broader privacy and hardware-wallet validation has not been repeated on this stack; earlier results below describe the previous dependency revisions.
The latest adversarial regressions reproduced two further timing defects before repair: a malformed selected entry removed the publication wait, and cancellation during settlement erased a newly reported delay. The complete transparent adapter suite passed with the library repair, which also received an independent Codex review with no findings. Vizor now preserves fresh waits during cancellation and pins that repaired library. Its complete focused transparent-recovery suite passed, including publication lag, capacity waits, cancellation, account isolation, and accepted reorgs. Independent Codex review of the updated app head reported no findings, and both repositories passed their configured CI gates.
The library removal passed focused SQLite payload-recovery and Enhance scheduling checks. Focused Vizor checks passed after repinning the dependency. Additional status regressions reproduced invalid-manifest and oversized-response failures before the repair and passed afterward. Focused status checks also passed for malformed-row suppression, retry accounting, coverage, and source policy. Results below describe the earlier validated recovery changes.
Earlier focused Rust checks passed on the consolidated branch, covering enhancement, status/checkpoints, transparent recovery and transaction details, wallet handles, history, private shielding, cache upgrades, sync errors, and accepted-reorg scheduling. The observed results confirm that bad contents do not stop later payload requests, three unreadable rows suspend the affected position, accepted rewinds reopen recovery while failed rewinds and lagging endpoints retain suppression, and growing beyond the restore window restores the unsettled-history warning. New regression checks reproduce both service-wait defects on the previous branch and verify the repair: failed private entries preserve the reported wait across new blocks and rewinds, catch-up can restore current transparent spend authority at an unchanged tip, and an expired wait permits only its account to retry. A stale commit also preserves the wait. The recovery-status read exposes the expired deadline to the app, whose existing tip poll can start the bounded catch-up pass without resetting the payload/status backoff.
Focused Flutter checks also passed for recovery restart timing and desktop receipt behavior, including stopping polling after a lookup is given up. Analysis of the changed app logic reported no issues.
A repository-wide Rust format check still reports existing differences in payload tests outside this repair.
Live-service tests, native app runs, and the separate Docker/regtest private-history profile have not been run in this session. Envelope-versus-content classification remains tied to the pinned client's error messages.