feat(platform-wallet): sweep + recover CoinJoin mixed coins for the DashSync→SDK migration

[llbartekll]

What

Adds platform-wallet support for recovering and offloading CoinJoin ("mixed") coins for wallets migrating from DashSync (legacy SPV) to the SwiftDashSDK Core wallet. Post-migration CoinJoin is being dropped, so this lets users find their scattered mixed coins and sweep them back into spendable BIP44 balance.

Four commits:

1. feat(platform-wallet): CoinJoin sweep + one-time recovery gap-limit — sweep_coinjoin_to_address (empties the CoinJoin account to a destination address) + set_coinjoin_gap_limit for a one-time wide recovery scan that finds deep mixed coins scattered across large address holes (DashSync used a gap of 400 vs the SDK default of 30).
2. feat(coinjoin): chunk the CoinJoin sweep across multiple transactions — partitions the spendable UTXO set into balanced chunks of ≤500 inputs and returns Vec<Transaction>, so a heavy mixer whose UTXOs exceed a single relayable tx still sweeps (broadcast with partial-failure tolerance).
3. chore(deps): re-pin rust-dashcore to dev (refactor(rs-dpp): bring error type to one format #804 merged) — see the dependency note below.
4. fix(coinjoin): sign internal-chain ("change") mixed coins in the sweep — DashSync CoinJoin uses both external (…/0/i) and internal (…/1/i, mixing change) chains; the SDK's CoinJoin account is external-only. The sweep now builds a dual-chain address → DerivationPath resolver from the CoinJoin account xpub so internal-chain UTXOs sign correctly (otherwise a single internal input blocks the whole all-or-nothing chunk).

Also includes the FFI (core_wallet/broadcast.rs) and swift-sdk wrapper (ManagedCoreWallet.swift) surface for the above.

⚠️ Dependency bump (intentional)

Commit 3 bumps the workspace rust-dashcore pin from eb889af1 → 7ff6b246 (8 crates). 7ff6b246 is eb889af1 + 5 commits and includes the merged dashpay/rust-dashcore#804, which the internal-chain signing (commit 4) depends on. This is a forward bump, not a revert.

Verification

• cargo build -p platform-wallet — clean (branch rebased onto current v3.1-dev).
• cargo test -p platform-wallet — dual-chain resolver unit tests pass (resolves_external_and_internal_chain_addresses, errors_when_an_input_address_is_unresolvable).
• DashSDKFFI.xcframework rebuilt; the SwiftExampleApp and the downstream dashwallet-ios app both build against it (iOS simulator).
• Runtime: confirmed on a ~2k-address testnet CoinJoin wallet — a 278-input sweep spanning both chains confirmed on-chain.

Merge order

The dashwallet-ios app branch links the SwiftDashSDK built from this branch, so this PR should merge into v3.1-dev before the app-side PR.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added CoinJoin sweep functionality to consolidate CoinJoin UTXOs to a destination address, automatically handling large UTXO sets across multiple transactions.
  • Added ability to configure CoinJoin address pool gap limit for recovery scenarios.

[thepastaclaw]

✅ Review complete (commit 41d5e8a)

[thepastaclaw]

Code Review

PR adds a CoinJoin recovery + chunked sweep flow that is well-scoped and faithfully mirrors key-wallet's fee/sizing conventions. One blocking issue: the new set_coinjoin_gap_limit public API does not reject gap_limit == 0, which underflows self.gap_limit - 1 inside maintain_gap_limit (debug panic / release wrap to u32::MAX → effectively unbounded address generation under the wallet write lock). A few non-blocking defensive-coding improvements in the sweep path as well.

🔴 1 blocking | 🟡 2 suggestion(s) | 💬 1 nitpick(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/wallet/core/coinjoin_recovery.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/core/coinjoin_recovery.rs:96-98: Reject zero CoinJoin gap limit before maintaining the pool
  `set_coinjoin_gap_limit` takes `gap_limit: u32` from the Swift FFI and writes it directly into `pool.gap_limit` (only clamped on the upper end via `MAX_GAP_LIMIT`). If `gap_limit == 0`, `AddressPool::maintain_gap_limit` evaluates `self.gap_limit - 1` when `highest_used` is `None` (verified in the pinned key-wallet at `address_pool.rs:888-891`). That panics in debug builds and wraps to `u32::MAX` in release, causing the recovery helper to try generating ~4 billion addresses while holding the wallet write lock. Since this is a brand-new public API (FFI does not validate either), reject the zero case at the boundary before touching the pool.

In `packages/rs-platform-wallet/src/wallet/core/broadcast.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/core/broadcast.rs:451-457: Remove address from `needed` only after `path_map.insert` actually succeeds
  `coinjoin_sweep_path_map` does `if needed.remove(addr) { if let Some(info) = pool.address_info(addr) { path_map.insert(...) } }`. The `remove` happens unconditionally on a match, but the `path_map.insert` is gated on `address_info` returning `Some`. The resolver's documented contract is "errors if any input can't be resolved, rather than failing mid-sign" — but if `address_info` ever returns `None` for an address `generate_addresses` just produced (an invariant violation in `AddressPool`), the address is silently dropped from `needed`, `needed.is_empty()` becomes true so the defensive check at line 462 does not fire, and `signer.sign_tx` later fails with the very "no derivation path for input address" error the resolver was added to prevent. Today `generate_address_at_index` inserts into both `addresses` and `address_index` atomically, so this can't trigger — but inverting the gating preserves the "removed from needed ⇒ inserted into path_map" invariant by construction, so a future address-pool refactor cannot silently break this code path.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/core/broadcast.rs:370-388: Partial-broadcast failures are swallowed silently when at least one chunk succeeds
  When some chunks broadcast and others fail, the function returns `Ok(broadcast)` with only the successful subset; `last_err` and the count of failures are dropped. The docstring (lines 194–198) calls this out as intentional — the caller is expected to refresh balance and re-run — but the result type currently carries no signal that the sweep was partial. The Swift caller's `sweepCoinJoinAccount` therefore can't surface "swept K of N chunks" telemetry or warn the user before claiming success. At minimum, log dropped errors via `tracing::warn!` inside the loop so the failures are observable in logs; longer term, consider returning a richer outcome type so the FFI consumer can distinguish full vs partial success.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 72f52ad5-f0d0-4d1f-a64e-0b4f4fb0933f

📥 Commits

Reviewing files that changed from the base of the PR and between a0ead20 and 41d5e8a.

📒 Files selected for processing (1)

• packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs

💤 Files with no reviewable changes (1)

• packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs

---

📝 Walkthrough

Walkthrough

This PR implements CoinJoin sweep-to-address and address-pool gap-limit recovery across the Rust wallet core, FFI bindings, and Swift SDK, enabling users to consolidate scattered CoinJoin UTXOs into a single destination address and temporarily widen address detection windows for recovery scenarios.

Changes

CoinJoin Sweep and Gap-Limit Recovery

Layer / File(s)	Summary
Core: sweep_coinjoin_to_address, chunking, path map, tests packages/rs-platform-wallet/src/wallet/core/broadcast.rs	Implements UTXO chunking via ceil-division, sweep transaction construction and signing across both CoinJoin chains (external /0/ and internal /1/), dual-chain address→derivation-path mapping, and unit tests for partitioning and path resolution.
Core: set_coinjoin_gap_limit (recovery) packages/rs-platform-wallet/src/wallet/core/coinjoin_recovery.rs	Temporarily widens CoinJoin pool gap_limit, materializes addresses via maintain_gap_limit, bumps SPV monitor revision to regenerate compact filters, and returns highest generated address index. Includes module documentation and error handling for missing wallets/accounts/pools.
Core module wiring packages/rs-platform-wallet/src/wallet/core/mod.rs	Declares internal coinjoin_recovery submodule in wallet core.
FFI entry points packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs	Adds core_wallet_sweep_coinjoin and core_wallet_set_coinjoin_gap_limit FFI functions with pointer validation, address parsing, resolver-backed signer creation, heap-allocated txid buffer in wire order, and highest-index output. Documentation for core_wallet_free_tx_bytes updated to cover txid buffer cleanup.
Swift SDK wrappers packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swift	Adds sweepCoinJoinAccount and setCoinJoinGapLimit public methods that call the new FFI functions, parse returned txid buffers into wire-order [Data] arrays, and return highest generated address index for gap-limit changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

ready for final review

Suggested reviewers

• shumkov
• thepastaclaw
• lklimek

Poem

🐰 A sweep through the CoinJoin mist,
Both chains untangled, no UTXO missed,
Gap limits widened for coins that ran deep,
Now from FFI to Swift, transactions leap!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat(platform-wallet): sweep + recover CoinJoin mixed coins for the DashSync→SDK migration' directly and clearly summarizes the main changes: adding sweep and recovery functionality for CoinJoin coins during wallet migration.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/coinjoin-sweep-and-recovery

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

packages/rs-platform-wallet/src/wallet/core/broadcast.rs (1)

214-367: 🏗️ Heavy lift

Release the wallet write lock before awaiting chunk signing.

Line 215 acquires wallet_manager.write(), and the loop at Lines 349-352 keeps that writer held across every sign_tx(...).await. By Line 287 you already own the UTXO snapshot and path_map, so the remaining async signing work no longer needs mutable wallet state; keeping the lock here serializes unrelated wallet operations behind resolver/keychain latency.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rs-platform-wallet/src/wallet/core/broadcast.rs` around lines 214 -
367, The wallet write lock taken by let mut wm =
self.wallet_manager.write().await is held across async signer.sign_tx(...).await
calls; release it before the chunk signing loop by ending the wm borrow scope
(or calling drop(wm)) immediately after you've extracted
account_xpub/account_type/network, managed_account, utxos and computed path_map
(i.e., right after the path_map variable is created) so the for chunk in
utxos.chunks(...) loop and signer.sign_tx awaits run without the wallet_manager
write lock held.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/rs-platform-wallet/src/wallet/core/broadcast.rs`:
- Around line 309-310: This file fails rustfmt; reformat the Rust source by
running `cargo fmt --all` (or `rustfmt`) so the expression using
fee_rate.calculate_fee(BASE_SIZE_1_OUTPUT_NO_CHANGE + input_count * INPUT_SIZE)
and other nearby hunks (e.g., around the calculate_fee call and constants
BASE_SIZE_1_OUTPUT_NO_CHANGE / INPUT_SIZE) conform to project style; simply run
the formatter and commit the updated layout so CI rustfmt checks pass.
- Around line 464-482: The loop in broadcast.rs always calls
pool.generate_addresses(BATCH, ...) which can overshoot max_index; change it to
compute remaining = max_index.saturating_sub(generated) and call
pool.generate_addresses(remaining.min(BATCH), key_source, true) (or the
equivalent usize/min) so the final batch is clamped to the remaining search
window, and then increment generated by the actual batch size returned/used
rather than unconditionally by BATCH; update references to BATCH in that loop
accordingly (e.g., the variables generated, max_index, and the call to
generate_addresses).

In `@packages/rs-platform-wallet/src/wallet/core/coinjoin_recovery.rs`:
- Around line 46-51: Run rustfmt across the repo (cargo fmt --all) to fix
formatting in coinjoin_recovery.rs; specifically reformat the block that
acquires wm via self.wallet_manager.write().await and the call to
wm.get_wallet_and_info_mut(&self.wallet_id) which constructs
PlatformWalletError::WalletNotFound, so that the wm, wallet_manager,
get_wallet_and_info_mut, self.wallet_id and PlatformWalletError::WalletNotFound
symbols adhere to rustfmt style and CI passes.

---

Nitpick comments:
In `@packages/rs-platform-wallet/src/wallet/core/broadcast.rs`:
- Around line 214-367: The wallet write lock taken by let mut wm =
self.wallet_manager.write().await is held across async signer.sign_tx(...).await
calls; release it before the chunk signing loop by ending the wm borrow scope
(or calling drop(wm)) immediately after you've extracted
account_xpub/account_type/network, managed_account, utxos and computed path_map
(i.e., right after the path_map variable is created) so the for chunk in
utxos.chunks(...) loop and signer.sign_tx awaits run without the wallet_manager
write lock held.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f7447e1d-e8ba-4c9e-a0c5-eb5fdc12c991

📥 Commits

Reviewing files that changed from the base of the PR and between 1235dd3 and c65905f.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

• Cargo.toml
• packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs
• packages/rs-platform-wallet/src/wallet/core/broadcast.rs
• packages/rs-platform-wallet/src/wallet/core/coinjoin_recovery.rs
• packages/rs-platform-wallet/src/wallet/core/mod.rs
• packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/CoreWallet/ManagedCoreWallet.swift

[thepastaclaw]

Code Review

All four prior findings are fixed at head c65905f. The latest commit cleanly applies the suggested fixes (zero gap-limit guard, insert-then-remove ordering in path_map, tracing::warn on per-chunk broadcast failures, runtime input-count invariant). One new in-scope blocking issue surfaces in the cumulative review: the new sweep FFI entry point core_wallet_sweep_coinjoin parses the caller-supplied destination with assume_checked() and never matches it against the wallet's network, so a wrong-network address would silently drain the entire CoinJoin account to a script on the wrong chain.

🔴 1 blocking

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs:168-188: Validate the sweep destination against the wallet's network
  `core_wallet_sweep_coinjoin` parses `dest_address` with `dashcore::Address::from_str(...).assume_checked()` and then passes the resulting `Address` straight into `sweep_coinjoin_to_address`, which builds and broadcasts a transaction that drains every spendable CoinJoin UTXO to that script. The wallet's network is read a few lines below (line 175) but is never used to verify the destination. A mainnet `CoreWallet` will therefore accept a testnet/devnet/regtest address (or vice versa), and the resulting all-input → single-output sweep will be broadcast on the wallet's network with the wrong-network script — the entire CoinJoin account becomes unrecoverable. This is especially severe because this entry point is the all-funds sweep used by the DashSync migration, and the address comes from arbitrary Swift caller input. Parse as `Address<NetworkUnchecked>` and `require_network(wallet.network())` inside the storage closure so the failure surfaces as a `PlatformWalletFFIResult` error before any tx is built.

[llbartekll]

Re the CodeRabbit nitpick about releasing the wallet write lock across the sign .await loop in the CoinJoin sweep (no inline thread for it): declining for now.

• It is a tokio::sync::RwLock — designed to be held across .await, so this is not a deadlock/correctness issue, only a throughput observation (already documented in the code comment there).
• The broadcast loop already runs outside the lock, and the locked section performs no wallet mutation (it snapshots UTXOs into an owned Vec and builds an owned path_map), so dropping the lock early buys no snapshot→broadcast atomicity.
• The only thing it would save is local, fast signing time during a rare one-time single-user migration sweep, at the cost of re-touching the deliberate wallet/info borrow ordering.

Tracking it as a possible future optimization if this path is ever reused in a concurrent/server context.

[thepastaclaw]

Code Review

Latest delta (f52644b) is the focused FFI network-validation fix plus rustfmt. The prior blocking finding is FIXED at packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs:168-185 — dest_unchecked.require_network(network) runs inside the storage closure before any tx is built. Cumulative review surfaces no remaining in-scope blockers. Two suggestion-level findings about the sweep path are worth surfacing: (1) the sweep fee constants are not an upper bound for ≥253-input chunks, and (2) the public Rust sweep_coinjoin_to_address still trusts its dest argument's network. One nitpick on broadcast loop error preservation. The codex-general 'materialize internal CoinJoin chain' concern is downgraded to a suggestion because the SDK's CoinJoin account explicitly models a single (external) pool and the recovery scan is documented as a one-time post-migration helper.

🟡 3 suggestion(s) | 💬 1 nitpick(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/wallet/core/broadcast.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/core/broadcast.rs:289-310: Sweep fee constants are not a true upper bound for large chunks
  `BASE_SIZE_1_OUTPUT_NO_CHANGE = 8 + 1 + 1 + 34` hard-codes the input-count CompactSize as 1 byte and `INPUT_SIZE = 148` is the typical, not maximum, P2PKH input size. Chunks are explicitly allowed up to MAX_INPUTS_PER_SWEEP, and when `input_count >= 253` the CompactSize input-count grows to 3 bytes; compressed P2PKH ECDSA inputs can serialize to 149 bytes at the larger DER signature size. Because the sweep subtracts the fee *before* signing and never recomputes from the signed transaction, the computed fee can land below 1 duff/byte on large chunks — exactly the heavy-CoinJoin case this PR targets. Use upper-bound constants (or compute the varint length from `input_count`) so the fee is always ≥ policy minimum and the all-funds invariant holds.
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/core/broadcast.rs:198-210: Enforce destination network on the Rust sweep API itself
  The wrong-network destination check now lives in the FFI wrapper, but `CoreWallet::sweep_coinjoin_to_address` accepts any `NetworkChecked` `dashcore::Address` and proceeds to drain every spendable CoinJoin UTXO to it. Any non-FFI Rust caller (other consumers of `rs-platform-wallet`, future tests, future wrappers) can pass a mainnet-checked address to a testnet wallet and the sweep will succeed with funds locked to an unspendable script on this chain. Move the `dest.network() == self.network()` check into the sweep method itself so the fund-safety invariant is owned by the API that performs the sweep, not by a single caller.

In `packages/rs-platform-wallet/src/wallet/core/coinjoin_recovery.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/core/coinjoin_recovery.rs:79-108: Recovery widens only the external CoinJoin pool, not the internal change branch
  `set_coinjoin_gap_limit` widens the first pool returned by `address_pools_mut()`. The same PR adds dual-chain signing in `coinjoin_sweep_path_map` because DashSync puts mixed change on the internal `/1/i` branch, yet that branch is not materialized by this recovery API — SPV will not watch scripts that have never been pushed into a pool. If migrated wallets are expected to discover new mixed coins on the internal chain via this scan, the recovery is incomplete; if the assumption is that internal-chain coins are only ever imported via an out-of-band UTXO import, that contract is worth documenting at the top of this module so future callers do not silently miss funds.

[thepastaclaw]

Code Review

Latest delta a0ead20 is a clippy-only cleanup in broadcast.rs (drop a redundant clone on a Copy field, range-contains in a test) — no new latest-delta findings. Re-validation of the four prior findings against current head shows all four are STILL_VALID and carried forward: sweep fee constants under-bound large chunks, the Rust sweep API still defers the destination-network invariant to the FFI wrapper, recovery widens only the external CoinJoin pool while internal-chain mixed change is unmaterialized for SPV, and the all-fail broadcast path returns only the final chunk's error.

🟡 1 suggestion(s) | 💬 1 nitpick(s)

2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/wallet/core/coinjoin_recovery.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/core/coinjoin_recovery.rs:79-108: Recovery widens only the external CoinJoin pool while sweep expects both chains
  `set_coinjoin_gap_limit` calls `address_pools_mut().into_iter().next()` and widens that single pool. Yet `coinjoin_sweep_path_map` (broadcast.rs:271-287) explicitly resolves signing paths across BOTH external `/0/` and internal `/1/` chains, with a comment that DashSync CoinJoin puts mixing change on the internal chain. SPV only watches scripts that have been materialized into a pool's script index, so a recovery scan that widens only the external pool will not discover deep internal-chain mixed coins; the dual-chain signing fix only helps with internal-chain UTXOs that arrive through some other import path. Migrated users running `setCoinJoinGapLimit` (Swift wrapper presents this as the migration recovery scan) can silently miss change-chain funds. Either materialize the internal CoinJoin branch alongside the external pool, or rename/document the API to make the external-only scope explicit and require an alternate import path for internal-chain coins.

[github-actions]

✅ DashSDKFFI.xcframework built for this PR.

• Workflow run: https://github.com/dashpay/platform/actions/runs/27409663458
• Artifacts: DashSDKFFI.xcframework (folder), DashSDKFFI.xcframework.zip, xc_checksum.txt

SwiftPM (host the zip at a stable URL, then use):

.binaryTarget(
  name: "DashSDKFFI",
  url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
  checksum: "df456b74b6d921a30f3ad26200cb7ff3cec275b5910883aa6fc77dda40ae4751"
)

Xcode manual integration:

• Download 'DashSDKFFI.xcframework' artifact from the run link above.
• Drag it into your app target (Frameworks, Libraries & Embedded Content) and set Embed & Sign.
• If using the Swift wrapper package, point its binaryTarget to the xcframework location or add the package and place the xcframework at the expected path.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.82%. Comparing base (f4ed60f) to head (41d5e8a).
⚠️ Report is 16 commits behind head on v3.1-dev.

Additional details and impacted files

@@             Coverage Diff              @@
##           v3.1-dev    #3817      +/-   ##
============================================
+ Coverage     70.73%   70.82%   +0.09%     
============================================
  Files            20       20              
  Lines          2788     2797       +9     
============================================
+ Hits           1972     1981       +9     
  Misses          816      816              

Components	Coverage Δ
dpp	∅ <ø> (∅)
drive	∅ <ø> (∅)
drive-abci	∅ <ø> (∅)
sdk	∅ <ø> (∅)
dapi-client	∅ <ø> (∅)
platform-version	∅ <ø> (∅)
platform-value	∅ <ø> (∅)
platform-wallet	∅ <ø> (∅)
drive-proof-verifier	∅ <ø> (∅)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[thepastaclaw]

Code Review

Carried-forward prior findings: all four prior findings are resolved at 43ee9af — fee bound is now a true upper bound (149 B/input + dynamic varint), the wrong-network sweep check is enforced inside the Rust API (not just FFI), the broadcast loop preserves the FIRST chunk error via get_or_insert, and the external-only recovery widening is documented as correct given that internal-chain mixed coins are imported as UTXOs (the sweep path_map still re-derives both chains for signing). New findings in the latest delta: codex-flagged null/network issues in core_wallet_send_to_addresses are about pre-existing code that this PR does not modify and are dropped as out of scope. Only a single in-scope doc nitpick remains on core_wallet_free_tx_bytes whose contract now covers both serialized-tx and txid buffers.

💬 1 nitpick(s)

[thepastaclaw]

Code Review

Carried-forward prior findings: none — prior-1 (core_wallet_free_tx_bytes doc) is FIXED at packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs:250-254, where the doc now explicitly enumerates both the send_to_addresses tx-bytes buffer and the sweep_coinjoin count*32 txid buffer. New findings in the latest delta (43ee9af..41d5e8a is docs-only): none introduced by the delta itself. Cumulative review against the current head surfaced one concrete edge case in the hand-built sweep tx assembly (dust-output bound) and a few low-value suggestions; no blocking issues, so APPROVE with a single suggestion comment.

🟡 1 suggestion(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/wallet/core/broadcast.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/core/broadcast.rs:340-346: Sweep chunk can build a sub-dust output and silently fall back to partial success
  The hand-built sweep tx bypasses `TransactionBuilder` and only guards `total_input <= fee`. If a chunk's `total_input - fee` lands above zero but below the destination script's dust threshold (≈546 duffs for P2PKH), the chunk produces a TxOut that relay policy will reject. Because broadcast loops over chunks and tolerates partial failure (line 408-432), the API can still return `Ok(broadcast)` while a dust chunk is silently dropped and those UTXOs remain unswept — exactly the failure mode the all-input chunking is designed to prevent. CoinJoin denominations are normally well above the bound, but DashSync-imported sub-denomination change can ride in on the internal chain and land alone in a one-input final chunk, which is precisely the path the new dual-chain resolver enables. Validate `output_amount >= dest.script_pubkey().dust_value()` before building the TxOut and return a typed build error, or fold the dust remainder into a sibling chunk so the sweep is all-or-nothing per UTXO.

[thepastaclaw]

Code Review

Carried-forward prior findings: none — prior-1 (core_wallet_free_tx_bytes doc) is FIXED at packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs:250-254, where the doc now explicitly enumerates both the send_to_addresses tx-bytes buffer and the sweep_coinjoin count*32 txid buffer. New findings in the latest delta (43ee9af..41d5e8a is docs-only): none introduced by the delta itself. Cumulative review against the current head surfaced one concrete edge case in the hand-built sweep tx assembly (dust-output bound) and a few low-value suggestions; no blocking issues, so APPROVE with a single suggestion comment.

Reviewed commit: 41d5e8a

Fresh response to a new review request. The PR head is unchanged since the exact-SHA automated review, so this records the same verified result without duplicating inline threads.

Code Review

🟡 1 suggestion(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/wallet/core/broadcast.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/core/broadcast.rs:340-346: Sweep chunk can build a sub-dust output and silently fall back to partial success
  The hand-built sweep tx bypasses `TransactionBuilder` and only guards `total_input <= fee`. If a chunk's `total_input - fee` lands above zero but below the destination script's dust threshold (≈546 duffs for P2PKH), the chunk produces a TxOut that relay policy will reject. Because broadcast loops over chunks and tolerates partial failure (line 408-432), the API can still return `Ok(broadcast)` while a dust chunk is silently dropped and those UTXOs remain unswept — exactly the failure mode the all-input chunking is designed to prevent. CoinJoin denominations are normally well above the bound, but DashSync-imported sub-denomination change can ride in on the internal chain and land alone in a one-input final chunk, which is precisely the path the new dual-chain resolver enables. Validate `output_amount >= dest.script_pubkey().dust_value()` before building the TxOut and return a typed build error, or fold the dust remainder into a sibling chunk so the sweep is all-or-nothing per UTXO.