[romchornyi]

Issue being fixed or feature implemented

A wallet that broadcasts a transaction and gets no answer from the network ("status unknown") keeps the transaction in its mempool view, so its change looks spendable. If that transaction never reached the network, every later payment built on the change is an orphan no node accepts — the wallet keeps sending coins the chain never had. dashpay/platform's wallet now tracks such unresolved sends and needs to keep their outputs out of coin selection until the network confirms them, without writing anything into wallet state (a lock that a cancelled build forgets to undo would freeze the coin).

TransactionBuilder had no way for a caller to say "do not offer these coins" for one build.

What was done?

• TransactionBuilder::exclude_outpoints(iter) — a per-build set of outpoints coin selection never offers. It accumulates and works in any call order relative to add_funding / add_inputs. It is applied in assemble_unsigned, the one place selection runs, so it covers add_funding, add_funding_reservation_only and seeded inputs alike. Nothing is written to the wallet.
  • An excluded funding candidate is dropped silently, like a reserved or locked coin.
  • An excluded seeded input (add_inputs) is a hard error, BuilderError::ExcludedInput { outpoint }, returned before anything is reserved — the caller asked for that coin explicitly. It names the first excluded input in seeding order, so the error is deterministic.
• Read-only getters a caller needs to explain a shortfall at the build's own terms: outputs() (explicit outputs only — not special-payload credit outputs, and only a placeholder under SelectionStrategy::All), fee_rate(), selection_strategy(), has_special_payload().
• Seeded inputs are tracked as an ordered list of outpoints, read only when an exclusion is set.

Wallet-level helpers (build_and_sign_transaction and its funded builder) take no exclusions; the API doc says so.

Related: #1078 adds a persistent locked-outpoint set on ManagedWalletInfo for masternode collateral. That is wallet state and outlives a build; this is a per-build exclusion the caller recomputes each time. They don't overlap.

How Has This Been Tested?

• cargo test -p key-wallet --lib: 689 passed, 18 ignored. New tests: an excluded candidate is never selected; an excluded seeded input fails with ExcludedInput and reserves nothing; with two excluded seeded inputs the error names the first, stable over repeated builds; an excluded candidate plus a signing failure after selection leaves no reservation (asserts SigningFailed, so the release path is exercised).
• cargo clippy and cargo fmt --check clean on key-wallet; key-wallet-ffi, key-wallet-manager and dash-spv type-check.
• Used by the dashpay/platform change through a local [patch]: platform-wallet 1321/1321 tests pass against this branch.

Breaking Changes

BuilderError gains the variant ExcludedInput. The enum is not #[non_exhaustive], so a downstream exhaustive match on it no longer compiles. In this workspace the only users go through to_string().

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · 2e0202f

• Bots — coderabbitai ✓
• Self-review — address ZocoLini requested changes, then post /self-reviewed
• Within your 5 open PRs
• Build green
• Approvals
  • key-wallet (key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs) — QuantumExplorer or ZocoLini or xdustinface

When every box is checked the PR Hygiene check passes and this can merge.

Summary by CodeRabbit

• New Features
  • Added the ability to exclude selected funding inputs when building a transaction. If an explicitly selected input is excluded, transaction building reports that input; if exclusions leave too few funds, the existing insufficient-funds error is returned.
  • Added read-only access to transaction outputs, fee rate, selection strategy, and special-payload status.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: dashpay/rust-dashcore/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 78070a14-0400-4203-aad5-9ed09c0fe234

📥 Commits

Reviewing files that changed from the base of the PR and between 7c1f6be and 2e0202f.

📒 Files selected for processing (1)

• key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

TransactionBuilder now tracks excluded outpoints and caller-seeded inputs. Assembly rejects an excluded seeded input, filters excluded funding candidates, and returns the existing coin-selection error if the remaining candidates are insufficient.

Changes

Transaction input exclusions

Layer / File(s)	Summary
Track exclusions and seeded inputs key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs	TransactionBuilder accumulates outpoint exclusions and exposes accessors for outputs, fee rate, selection strategy, and special-payload presence. add_inputs records added outpoints in seeding order.
Apply exclusions during assembly key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs	Assembly returns ExcludedInput for the first excluded seeded outpoint in seeding order. It filters other excluded candidates before coin selection. Tests cover exclusion order, insufficient candidates, and reservation cleanup after signing failure.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Suggested reviewers: zocolini

Merge Risk: ⚪ Minimal · up to 2e020

This adds optional per-build outpoint exclusions to the transaction builder, with tests covering selection, ordering, and cleanup. No merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 2e020

The checked paths enforce exclusions before selection and reservation without adding spend authority or a persistent wallet lock. The restriction is opt-in for each build, however, and the external payment integration was not available to verify complete enforcement.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The direct effect is bounded to the candidate inputs of a configured builder and the selected-input reservations of its attached funding accounts. Supplying exclusions removes spending choices or causes a build error; it does not grant additional signing authority.

Trust Boundaries and Controls

• observed — Explicit seeding cannot override an exclusion: conflicting seeded outpoints fail before reservation. Other excluded candidates are filtered before selection, while existing reservation and optional finality checks continue to constrain the remaining inputs.

Resilience and Maintainability Implications

• observed — The existing concurrency precondition remains important: funding and reservation must occur under an uninterrupted wallet lock. Owner-guarded release prevents a returned failure from freeing an input subsequently reserved by another build; the exclusion API does not replace either requirement.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 1 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: allowing transaction builds to exclude outpoints from coin selection.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

❌ Patch coverage is 88.42975% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.45%. Comparing base (e4208c9) to head (2e0202f).
⚠️ Report is 80 commits behind head on dev.

Files with missing lines	Patch %	Lines
.../wallet/managed_wallet_info/transaction_builder.rs	88.42%	14 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1100      +/-   ##
==========================================
+ Coverage   77.34%   77.45%   +0.11%     
==========================================
  Files         329      320       -9     
  Lines       83857    81465    -2392     
==========================================
- Hits        64855    63100    -1755     
+ Misses      19002    18365     -637     

Flag	Coverage Δ
core	78.90% <ø> (+0.64%)	⬆️
ffi	50.85% <ø> (-1.03%)	⬇️
rpc	20.00% <ø> (ø)
spv	91.78% <ø> (-0.36%)	⬇️
wallet	80.26% <88.42%> (+0.48%)	⬆️

Files with missing lines	Coverage Δ
.../wallet/managed_wallet_info/transaction_builder.rs	92.46% <88.42%> (-0.23%)	⬇️

... and 142 files with indirect coverage changes

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[romchornyi]

/self-reviewed

[github-actions]

Ready for review — key-wallet: QuantumExplorer or ZocoLini or xdustinface.
Full checklist in the description.

[ZocoLini]

I don't think key-wallet needs this. require_final_inputs() already drops every candidate that is neither confirmed nor IS-locked, from both add_funding and add_inputs, and writes nothing to the wallet. Change from a send that never reached the network never gets an IS lock, so it is never selected, and nothing gets built on top of it. The only difference is the "accepted, not locked yet" window, which by your own e2e is under 1 s. e2e is under 1 s.

A per-build outpoint list also leaves the policy outside the wallet, so any path that doesn't pass it still spends the phantom change. Suggest calling .require_final_inputs() in platform's payment builds and closing this.

A different option is to drop require final inputs and always filter out those inputs that cannot be consider final. This change only makes sense if you are open to have users waiting a little bit for their tx to be ISlocked, so it is up to the mobile and platform team, I dont have a strong opinion on this

[github-actions]

Bots are done — your move: address ZocoLini requested changes, then post /self-reviewed.
Full checklist in the description.

[romchornyi]

Thanks @ZocoLini — you're right, require_final_inputs() covers it. I moved dashpay/platform#5258 to it (platform's payment builds and its spendable / max figures now use only confirmed or InstantSend-locked coins) and dropped the dependency on this PR.

Checked on testnet on the same scenarios as the outpoint-exclusion version: a send that ended "status unknown" with P2P blocked, then an immediate second payment — refused, nothing built on the unconfirmed change, spendable again right after the first send's IS lock; the same with the app killed and restarted in between; and a fresh incoming payment spent ~20 s after it arrived, already IS-locked, so the extra wait isn't noticeable in practice. Same results, much less code, and no dependency on probing being enabled.

Closing.