Skip to content

feat(wallet): shield the whole Core balance with Max - #1106

Draft
Claudius-Maginificent wants to merge 4 commits into
fix/orphaned-txfrom
feat/shielded-max-drain
Draft

Claudius-Maginificent wants to merge 4 commits into
fix/orphaned-txfrom
feat/shielded-max-drain

Conversation

@Claudius-Maginificent

@Claudius-Maginificent Claudius-Maginificent commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

TL;DR: "Max" on a send from Core to the shielded pool moves the whole Core balance minus fees. An interrupted transfer can be finished from the Shielded tab. Stacked atop #1028 (fix/orphaned-tx). Blocked on dashpay/platform#5528, which #1028 pins.

User story

As a wallet user, I want Max on a Core-to-shielded send to shield everything I have, so that no dust is left behind and the send never fails for being a few duffs short of its own fees.

Scenario

Actual behavior

Max shields an estimated amount. The estimate can come up short of the real fees, or leave a small remainder in Core.

Expected behavior

Max shields the whole balance minus fees. The amount is fixed only when the funding transaction is built. If the app closes mid-transfer, the Shielded tab says the transfer is unfinished and offers "Finish now". That resumes the same transfer without paying twice.

Detailed discussion

Stacked atop #1028, which pins all Platform dependencies to 344e110a6, the head of dashpay/platform#5528 (stack #5150 → #5207 → #5210 → #5528). #5528 is the first point where SqlitePersister attests the wallet-drain persistence contract. This PR changes no dependency. It merges after #1028, once #5528 is on v5.1-dev.

  • Max drain: runs the upstream pooled wallet drain.
    • The drain is registered with the wallet first, and only then is its operation id stored in the DET k/v sidecar, scoped to the wallet and network. The registration is held until the drain returns, so a status read in between sees a running transfer, never "not sent". It is dropped before the outcome is read, so a drain that ended before building its lock reads as not sent and releases its id. The id is removed only once the transfer has an outcome.
    • A second Max transfer waits for the first to settle.
    • A read-only preview refuses a wallet that cannot cover the fees, or that holds too many coins for one transfer, before anything is stored. It also refuses, with a dedicated message, a spendable balance that differs from the one the user confirmed, so the drain moves what the confirmation showed. The lock floor is upstream's: it drops a drain too small to pay the pool fee before broadcast. "Finish now" resumes without a preview.
  • Shielded tab: looks up a stored transfer on arrival. While the transfer is unfinished, the tab offers "Finish now", which resumes the same drain and never builds a second funding lock. The status read moves no funds.
  • Send form: the caption says that Max moves the whole balance. Sending a Max transfer first fetches the drain's own read-only quote. The confirmation then shows the spendable Core balance the transfer spends, the fees and the expected shielded amount. That balance excludes CoinJoin and unconfirmed funds. The quote covers every account the drain pools (BIP44, BIP32 and DashPay receiving funds), and the confirmed balance travels with the transfer. In the Shield flow the destination is the wallet's own shielded balance, and both the Max and the exact-amount confirmations say so instead of naming an empty address. Editing the amount, switching wallet or network, or resetting the form returns to an exact-amount send.
  • User story SND-007 is updated in docs/user-stories.md.

What moved to #1028: two of this PR's former commits.

This PR now carries only the feature.

TODO(max-drain-1107): #1107 (feat/wallet-max-send-limits) makes the Core→Shielded Max a backend-quoted asset-lock ceiling, while this PR makes an untouched Max a whole-balance drain. Whichever lands second must merge them, so that one component owns the button: #1107's quote and limits, with the drain where Max is untouched.

  • Known gap left for that merge: the amount field still fills in the BIP44-only estimate. Only the confirmation shows the drain's own quote.
  • Also left for that merge: Max is hidden when BIP44 alone cannot pay the fee, even if BIP32 or DashPay funds could.

Testing

  • Clippy --all-features --lib --tests -- -D warnings is clean.
  • The library tests for the drain, the Shielded tab, the send screen, the shielded backend and the fee estimation pass.
  • Orchestration tests on an offline context with the in-memory store cover:
    • a status read between storing the id and starting the drain keeps the transfer pending;
    • an unrun transfer settles as not sent and releases its id;
    • a second transfer is refused;
    • Finish and status without a stored transfer;
    • an unreadable record counting as pending.
  • Orchestration tests also cover:
    • a drain that fails before building its lock settles as not sent and releases its id;
    • a refused second transfer leaves no registration behind.
  • Unit tests cover the drain-run state derivation, the quote, and the refusal of a changed balance.
  • Send form tests cover:
    • the quote-then-confirm flow, with the transfer bound to the confirmed balance;
    • the literal confirmation sentences for the own-pool marker and for a typed address;
    • the exact-amount own-pool sentence;
    • a late quote after an edit, a wallet change or a network switch.
  • Not live-tested on testnet, and no backend-e2e covers the drain or its resume.

🤖 Co-authored by Claudius the Magnificent AI Agent

🤖 Generated with Claude Code

https://claude.ai/code/session_012ujJpB9DBhvtbjhYB3aG9Q

@coderabbitai

coderabbitai Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lklimek lklimek added the blocked Blocked by something external to this issue label Oct 23, 2026
@thepastaclaw

thepastaclaw commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

🕓 Review not started yet because this PR is a draft.

  • Request normal review — click when the PR is ready for review.
  • Request priority review — click to move this review to the front of the queue.

Commit e473f30. Normal review starts when eligible; priority review starts as soon as a slot is available.

@Claudius-Maginificent

Copy link
Copy Markdown
Collaborator Author

Blocked on dashpay/platform#5528 (open draft), the head of the stack dashpay/platform#5150, dashpay/platform#5207, dashpay/platform#5210, then #5528 (all open). The temporary pin cc5d498837 is not on v5.1-dev, so the pin-ancestry guard fails. When #5528 merges, repin to the v5.1-dev merge commit and take the PR out of draft.

🤖 Co-authored by Claudius the Magnificent AI Agent

lklimek and others added 4 commits October 27, 2026 13:45
Max on a Core-to-shielded send no longer shields an estimated amount. It
runs the upstream pooled wallet drain, which moves everything minus fees
with the amount fixed only once the funding transaction is built, so Max
can no longer come up short of its own fees or leave dust behind.

- The drain's operation id is stored in the DET k/v sidecar, scoped to
  the wallet and network, before the transfer starts. A crash or restart
  at any point leaves the id to look the transfer up by; it is removed
  only once the transfer has an outcome, and a second Max transfer waits
  for the first to settle.
- A read-only drain preview refuses a wallet that cannot cover the fees
  or holds too many coins for one transfer before anything is stored,
  and sets the lock floor so a drain too small to pay the pool fee is
  abandoned before broadcast.
- The Shielded tab looks a stored transfer up on arrival and, while it
  is unfinished, says so and offers "Finish now", which resumes the same
  drain and never builds a second funding lock. The status read moves
  no funds and stays available when shielded sending is not.
- The send form and confirmation say Max moves the whole balance; any
  edit of the amount returns to an exact-amount send.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 967f947)
A status read that landed between storing the Max transfer's id and
registering its drain found nothing upstream, reported "not sent" and
dropped the id of a drain about to move the whole balance. The drain is
now registered first and the registration is held until its outcome is
read, so such a read sees a running transfer.

- Finish no longer previews the wallet: upstream sets the drain's lock
  floor from the pool fee and ignores the caller's, so DET passes none
  and drops the dead floor helper.
- A new read-only QuoteShieldMaxFromCore task returns the drain's own
  balance, fees and expected shielded amount for the confirmation.
- The drain-run state derivation is a pure function, and the
  orchestration (status before the run, an unrun transfer, a second
  transfer, nothing to finish, an unreadable record) is tested on an
  offline context.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Max confirmation showed the form's estimate, which covers BIP44
account 0 only, while the drain spends BIP32 and DashPay receiving funds
too, and it showed no fee. Sending a Max transfer now fetches the
drain's read-only quote first and opens the confirmation with the
balance it spends, the fees and the expected shielded amount. A quote
that arrives after the amount changed opens nothing.

The Max choice is also forgotten on a wallet or network switch and on a
form reset, so a value chosen for one wallet can no longer drain another.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In the Shield flow the destination is the own-pool marker, which has no
address, so the Max confirmation read "to the shielded address ." and
then asked the user to check that destination. The exact-amount
confirmation had the same gap. Both now say "into your own shielded
balance" for the marker, and keep the address wording for a typed one.

The Max quote could also go stale while the dialog stayed open: the
drain spends whatever is spendable when it is built. ShieldMaxFromCore
now carries the balance the user confirmed, and the pre-run preview
refuses a different one with ShieldedMaxBalanceChanged. The figure is
worded as the spendable Core balance, since CoinJoin and unconfirmed
funds are not moved.

- The registration docs say what the code does: the registration is
  held until the drain returns and is dropped before the outcome is
  read.
- New tests:
  - the literal confirmation sentences;
  - a late quote after a wallet or network change;
  - a drain that fails before its lock, run with a registered handle;
  - a refused second transfer leaving no registration behind.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lklimek
lklimek force-pushed the feat/shielded-max-drain branch from a9228c9 to e473f30 Compare October 27, 2026 14:56
@Claudius-Maginificent
Claudius-Maginificent changed the base branch from chore/bump-platform-v4.3 to fix/orphaned-tx October 27, 2026 14:56
@Claudius-Maginificent

Copy link
Copy Markdown
Collaborator Author

Rebuilt on top of #1028 (fix/orphaned-tx, which now pins dashpay/platform#5528). The base is changed to fix/orphaned-tx. The PR is now 4 commits: the feature plus the review fixes; the Platform pin comes from #1028.

Still blocked: it depends on #1028, which waits for the platform stack #5150 → #5207 → #5210 → #5528 to merge into v5.1-dev.

🤖 Co-authored by Claudius the Magnificent AI Agent

@Claudius-Maginificent Claudius-Maginificent added priority: low Low priority — nice to have priority: medium Medium priority and removed priority: low Low priority — nice to have labels Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked Blocked by something external to this issue priority: medium Medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants