fix(platform-wallet): local-ledger ownership guard (V27-007)#3648
fix(platform-wallet): local-ledger ownership guard (V27-007)#3648lklimek wants to merge 1 commit into
Conversation
The SDK returns post-transition state for every address touched by a
transfer transition — both inputs and outputs. When the source wallet
sends credits to a foreign address (e.g. bank's primary receive address),
the response includes the bank's post-credit balance. Without an ownership
check, `transfer` wrote that foreign balance into the source wallet's local
ledger via `set_address_credit_balance`, corrupting `total_credits()`.
Symptom chain (PA-004b / PA-009c teardown, QA-010):
1. Test wallet trims residual credits to bank's primary address.
2. `transfer` writes bank's balance into the source wallet's ledger.
3. `total_credits()` is inflated; dust-gate passes; sweep begins.
4. Sweep selects bank's address (now in the source ledger), tries to
sign, fails ("No private key for address P2pkh(...)").
5. Failure cascade in teardown.
Fix: guard `set_address_credit_balance` with `contains_platform_address`
in `transfer`, mirroring the identical guard already present in
`fund_from_asset_lock`. The recipient wallet's syncer observes inbound
credits on its own addresses; the source wallet must not.
Also applied the same guard defensively to `withdrawal` (no foreign
platform output addresses appear there today, but consistency with the
local-ledger ownership invariant is cleaner than a latent risk).
Carved out of #3549 (source commit 16636f0) as a
standalone production fix on v3.1-dev. No e2e tests, no dependency
bumps included.
References: V27-007, QA-010
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
lklimek
changed the title
lklimek
added
the
ready for final review
Review GateCommit:
|
QuantumExplorer
left a comment
QuantumExplorer
left a comment
There was a problem hiding this comment.
We haven't been using the platform_payment_managed_account, and have plans to get rid of it entirely. Instead we use the thing in platform wallet.
3 participants
Issue being fixed or feature implemented
The SDK returns post-transition state for every address touched by a transfer transition — both inputs and outputs. When the source wallet sends credits to a foreign address (e.g. a bank's primary receive address), the response includes that foreign address's post-credit balance. Without an ownership check,
transferwrote the foreign balance into the source wallet's local ledger viaset_address_credit_balance, corruptingtotal_credits().Symptom chain (PA-004b / PA-009c teardown, QA-010):
transferwrites the bank's balance into the source wallet's ledger.total_credits()is inflated; the dust-gate passes; a sweep begins.No private key for address P2pkh(...)).This is a standalone correctness fix carved out of #3549 (source commit
16636f01c0). It is not present in the #3554 stack base — it is introduced by #3549 only and is self-contained.What was done?
transfer: guardset_address_credit_balancewithaccount.contains_platform_address(&p2pkh), mirroring the identical guard already present infund_from_asset_lock. Foreign output addresses are silently skipped; the recipient wallet's own syncer is responsible for observing its inbound credits.withdrawal: applied the same guard defensively. Withdrawals carry no foreign platform output addresses today, so the guard is never expected to fire, but it keeps the local-ledger ownership invariant consistent withtransfer.# Local-ledger ownership invariant (V27-007 / QA-010)doc section ontransferdocumenting the invariant.Net change: +27 LOC across 2 files, no e2e tests, no
rust-dashcorebump, noCargo.lockchurn.How Has This Been Tested?
cargo build -p platform-walletonorigin/v3.1-dev— clean (rust-dashcore stays at base rev53130869;contains_platform_addressis available onkey-wallet@53130869, no dependency on the excluded cluster-A bump).cargo clippy -p platform-wallet— clean, zero warnings on the changed crate.Breaking Changes
None. Internal post-broadcast ledger-update behavior only; public API is unchanged.
Checklist:
For repository code-owners and collaborators only
Source: extracted from #3549 (cluster F — local-ledger ownership guard, source commit
16636f01c0).🤖 Generated with Claude Code