fix(platform-wallet): pool the same funding sources for a contact payment as for a plain send - #4378
Conversation
…ment as for a plain send #4329 made sends draw from BIP44 + BIP32 + every DashPay receiving account, because a wallet holding funds outside BIP44 saw them in its wallet-wide total and got "insufficient funds" trying to spend them. The DashPay pay-to-contact path never picked that up: its whole share of that PR was the mechanical `set_funding` → `add_funding` rename, leaving the funding set pinned to BIP44 account 0. So the bug #4329 fixed still reproduces, on the one path where contact-received funds are most likely to be sitting. Reported from mainnet: after 8 contact payments drained BIP44, the next one failed with `Insufficient funds: available 41505, required 100000` on a screen showing plenty — the money was in the contact-receiving accounts. `send_payment` now walks `SEND_FUNDING_SOURCES` through the same `resolve_source_accounts` the core path uses, rather than reimplementing the resolution. BIP44 is still offered first, so change keeps returning to it. CoinJoin stays excluded by construction, and so do the contact *external* accounts — those hold the counterparty's xpub and no key this wallet can sign with. A source the wallet does not have contributes nothing instead of failing the send. Two consequences of pooling, handled: - the signer closure resolved derivation paths from the single funding account, so it now consults a map built across every offered account; - a rejected broadcast released reservations on BIP44 only, which would strand inputs drawn from a BIP32 or receiving account until the TTL backstop and fail an immediate retry with a spurious insufficient-funds. It now releases across every account that offered inputs. `contact_payment_funds_from_a_dashpay_receiving_account` leaves BIP44 empty and funds only the receiving account. Verified it catches the defect rather than merely passing: pinned back to BIP44-only it fails on the insufficient-funds assertion.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
🕓 Ready for review — 3 ahead in queue (commit ebe696f) |
Build 29 carried dashpay/platform#4372 + #4373 and proved the legacy-contact fix on mainnet (29/29 external accounts built, 8 payments). It then hit a second, separate bug: contact payments fund only from BIP44, so the ninth payment failed with 'Insufficient funds: available 41505' while the balance showed plenty. 30 adds dashpay/platform#4378, which pools BIP44 + BIP32 + DashPay receiving accounts on the contact-payment path — the funding set a plain send has used since #4329.
Confirmed on testnetTwo runs, each decisive in one direction. They are different wallets — the failure was captured on one before this PR existed, the success on another after — so this is not a same-wallet A/B; each half stands on its own arithmetic. Without this PR — wallet holding 90,998,870 duffs in BIP44 and 17,498,551 elsewhere (contact-receiving + BIP32); the pay sheet offered
With this PR — wallet holding 6,899,172 duffs in BIP44 plus a 500,000 duff contact payment received minutes earlier: 7,000,000 exceeds that wallet's BIP44 by 100,828 duffs, so it is only satisfiable by reaching the contact-receiving account — which is exactly what An earlier run on the same stack also confirmed the parent PRs on mainnet: 29/29 legacy contacts built their external accounts and 8 contact payments went through — which is what exhausted BIP44 and surfaced this bug in the first place. |
…broadcast
This PR's description listed the pooled release as the one change without a
test, on the grounds that the harness had no rejecting broadcaster after a
pooled build. It has both pieces — `with_rejecting_broadcaster` and
`plant_receival_utxo` — so the gap was mine, not the harness's.
`rejected_broadcast_releases_every_pooled_funding_account` funds BIP44 and a
contact-receiving account with 60_000 each and pays 100_000: neither side
covers it alone, so selection must take from both and a retry can only succeed
if both reservations were released. The retry is the assertion.
Verified it catches the defect rather than merely passing — pinned back to the
BIP44-only release it fails exactly as a user would experience it:
an immediate retry must reselect every pooled input ...:
TransactionBuild("Coin selection error: Insufficient funds:
available 60000, required 100000")
The contact-receiving account's 60_000 stayed reserved until the TTL backstop.
fea6e95
into
fix/dashpay-drain-retry-cost
4 participants
Issue being fixed or feature implemented
#4329 made sends draw from BIP44 + BIP32 + every DashPay receiving account. Its own words:
The DashPay pay-to-contact path never picked that up. Its entire share of #4329 was a mechanical rename:
The funding set stayed pinned to BIP44 account 0. So the bug #4329 fixed still reproduces — on the one path where contact-received funds are most likely to be sitting.
Reported from mainnet, straight after the legacy-contact fix in #4372 made 29 contacts payable: eight contact payments drained BIP44, and the ninth failed with
on a home screen showing plenty. The money was in the contact-receiving accounts — spendable, signable, and simply not offered to selection.
What was done?
send_paymentnow walksSEND_FUNDING_SOURCESthrough the sameresolve_source_accountsthe core path uses, rather than reimplementing the resolution (it ispub(crate)now, and re-exported fromwallet::core).Two consequences of pooling, both handled:
managed_account.address_derivation_path). It now consults a map built across every offered account.How Has This Been Tested?
cargo test -p platform-wallet— 670 pass.cargo clippy --all-targetsclean,cargo fmtapplied.New test
contact_payment_funds_from_a_dashpay_receiving_account: leaves BIP44 empty, registers a contact-receiving account and gives it the wallet's only 1,000,000 duffs, then pays the contact 100,000. Reaching the signer at all proves the receiving account was offered to selection.Verified it catches the defect rather than merely passing — pinned back to BIP44-only funding it fails on the insufficient-funds assertion:
rejected_broadcast_releases_every_pooled_funding_accountcovers the release path: BIP44 and a contact-receiving account hold 60,000 duffs each and the payment is 100,000, so neither covers it alone — selection must take from both, and the retry after the rejection can only succeed if both reservations were released. The retry is the assertion.Also verified by falsification — pinned back to the BIP44-only release it fails exactly as a user would experience it:
The contact-receiving account's 60,000 stayed reserved until the TTL backstop.
Breaking Changes
None. A contact payment that succeeded before still succeeds and still takes its change to BIP44; the change only widens what the selector may draw from.
Checklist:
For repository code-owners and collaborators only