build: bump rust-dashcore to dev head 78d10022

[Claudius-Maginificent]

Issue being fixed or feature implemented

Platform (and the dash-evo-tool integration line that consumes it) was pinned to an older rust-dashcore rev (5c0113e7, 0.43.0), missing 15 commits of upstream key-wallet / dash-spv fixes and drifting from the dev line. This bumps rust-dashcore to its current dev head so those fixes flow into platform and, downstream, into dash-evo-tool.

• Blocking relationship: targets v4.1-dev (the new base) so the bump lands where all wallet work now branches from.
• Depends on: dashpay/rust-dashcore#833 (unmerged) — adds Zeroize for ExtendedPrivKey, letting this PR replace manual non_secure_erase() calls with Zeroizing RAII. Currently pinned to chore(release): update changelog and bump version to 0.24.0-dev.14 #833's branch head; must re-pin to rust-dashcore dev HEAD (or a release tag) once chore(release): update changelog and bump version to 0.24.0-dev.14 #833 merges, so this doesn't ship depending on a feature branch that could be rebased/force-pushed.

What was done?

• Bumped all 8 rust-dashcore workspace dependencies (dashcore, dash-spv, key-wallet, key-wallet-ffi, key-wallet-manager, dash-network, dash-network-seeds, dashcore-rpc) from 5c0113e7 → 78d10022 (rust-dashcore dev head; 0.43.0 → 0.45.0), then twice more as rust-dashcore PR #833 evolved: → f42498e0 (adds Zeroize for ExtendedPrivKey) → a8c57fe8 (drops Copy from ExtendedPrivKey, adds Drop that zeroizes automatically). Refreshed Cargo.lock each time.
• rust-dashcore's dev added a required Signer::extended_public_key trait method (breaking the build). Adapted all impl Signer blocks:
  • MnemonicResolverCoreSigner (rs-sdk-ffi): a real BIP-32 extended-pubkey derivation.
  • Three rs-dpp test-signer stubs (FixedKeySigner in mod.rs, the address_funds and the feature-gated shielded signing tests): return Err — a single-raw-key stub has no chain code.
• DRY: extracted the shared resolver-FFI + parse + seed + derive boilerplate into one resolve_and_derive helper used by both derive_priv and extended_public_key.
• Zeroization: master and derived extended keys now self-wipe via ExtendedPrivKey's own Drop impl (rust-dashcore rev a8c57fe8) on every exit path — success, derive-error early return, and panic-unwind alike. seed/mnemonic/the final scalar remain Zeroizing-wrapped (plain buffers, no self-Drop of their own).
• Zero-crossing refinement: resolve_and_derive takes an extract: impl FnOnce(&ExtendedPrivKey) -> T closure instead of returning the key itself, so no ExtendedPrivKey ever crosses a function boundary — derive_priv/extended_public_key only ever see the extracted T (a scalar or ExtendedPubKey). Combined with Drop-based self-wiping and the type no longer being Copy (so moves are real moves, no bitwise-duplicate residue), this closes the transient-copy gap entirely — not just narrows it.
• Test: added extended_public_key_matches_independent_derivation, asserting full-struct equality plus explicit chain_code/depth/network checks against an independently-derived xpub on the same BIP-39 vector.

How Has This Been Tested?

• cargo check --workspace --all-targets: PASS
• cargo check -p dpp --tests --features state-transition-signing,core_key_wallet,shielded-client: PASS (the feature set that gates the shielded signing tests)
• cargo test -p rs-sdk-ffi mnemonic_resolver: 6/6 PASS
• cargo test -p dpp --features state-transition-signing,core_key_wallet,bls-signatures,shielded-client: 29/29 PASS (incl. the FixedKeySigner::extended_public_key stubs)
• Wallet-crate unit tests: green (platform-wallet / -ffi / -storage)
• cargo fmt --all + cargo clippy -p rs-sdk-ffi --all-targets -- -D warnings: 0 errors, 0 warnings

Breaking Changes

None to platform's public API — this is an internal adaptation to key-wallet's new required trait method.

Notes for reviewers

rust-dashcore is now pinned to PR #833's head (f42498e0) rather than dev, because #833 (open, unmerged) adds impl Zeroize for ExtendedPrivKey — the fix this PR originally deferred as a TODO(upstream). With that impl available, mnemonic_resolver_core_signer.rs wraps master/derived in Zeroizing<ExtendedPrivKey> instead of calling .private_key.non_secure_erase() by hand at 4 call sites; RAII now covers every exit path instead of three hand-placed wipes. The two secp256k1::SecretKey::non_secure_erase() calls at the sign sites (sign_ecdsa, public_key) are unchanged — SecretKey still has no Zeroize impl, so neither Zeroizing nor .zeroize() applies there.

Dependency-graph caveat: this pins platform to an unmerged rust-dashcore branch. Before merging to v4.1-dev, #833 should land on rust-dashcore dev and this PR should re-pin to the resulting dev head (or a release tag) rather than ship depending on a feature branch.

packages/rs-platform-wallet-ffi/src/sign_with_mnemonic_resolver.rs carries an identical mirrored manual-erase pattern (same TODO(upstream) shape) — left untouched here as a follow-up once this lands.

Update (commit b175d508): addressed thepastaclaw's two carried-forward findings on mnemonic_resolver_core_signer.rs — (1) the Copy ExtendedPrivKey stack-residue concern, by having resolve_and_derive take an extraction closure so the key never crosses a function boundary at all (only the extracted T does); (2) the missing extended_public_key test coverage, which was also failing codecov/patch at 0% — added a test asserting full-struct equality plus explicit chain-code/depth/network checks against an independent derivation.

Update (commits 56612fd5, 374df987): rust-dashcore PR #833 gained a further commit (a8c57fe8) dropping Copy from ExtendedPrivKey and adding a Drop impl that zeroizes automatically — the actual upstream fix for the "irreducible transient copy" caveat noted above. Re-pinned to that head; a workspace-wide cargo check confirmed zero call sites elsewhere in platform relied on ExtendedPrivKey's implicit-copy semantics, so no fallout fixes were needed outside this file. Simplified mnemonic_resolver_core_signer.rs accordingly: dropped the now-redundant Zeroizing<ExtendedPrivKey> wrapping around master/derived (the type wipes itself on drop now), keeping Zeroizing only where it still does real work (mnemonic/seed/scalar buffers).

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 (test-signer stubs updated for the new trait method)
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[coderabbitai]

📝 Walkthrough

Walkthrough

Bumps Dashcore-related crate git revisions in Cargo.toml, and adds an extended_public_key method to Signer implementations. MnemonicResolverCoreSigner gains a refactored derivation helper (resolve_and_derive) with updated zeroization handling, while test FixedKeySigner stubs implement stubbed extended_public_key returning errors.

Changes

Dependency Revision Bump

Layer / File(s)	Summary
Workspace dependency rev bump Cargo.toml	Git rev pins for dashcore, dash-network-seeds, dash-spv, key-wallet, key-wallet-ffi, key-wallet-manager, dash-network, and dashcore-rpc updated to a new commit.

Signer extended_public_key Implementation

Layer / File(s)	Summary
MnemonicResolverCoreSigner derivation refactor and extended_public_key packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs	Adds resolve_and_derive helper returning Zeroizing<ExtendedPrivKey>, refactors derive_priv to use it, updates zeroization docs, and implements extended_public_key(path) returning ExtendedPubKey.
Test FixedKeySigner stub updates packages/rs-dpp/src/state_transition/mod.rs, .../address_funding_from_asset_lock_transition/signing_tests.rs, .../shield_from_asset_lock_transition/signing_tests.rs	Adds ExtendedPubKey imports and stub extended_public_key implementations returning an error (no chain code) in each test's FixedKeySigner.

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

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant MnemonicResolverCoreSigner
  participant resolve_and_derive
  Caller->>MnemonicResolverCoreSigner: extended_public_key(path)
  MnemonicResolverCoreSigner->>resolve_and_derive: resolve_and_derive(path)
  resolve_and_derive-->>MnemonicResolverCoreSigner: Zeroizing<ExtendedPrivKey>
  MnemonicResolverCoreSigner->>MnemonicResolverCoreSigner: convert to ExtendedPubKey
  MnemonicResolverCoreSigner-->>Caller: ExtendedPubKey

Loading

Possibly related PRs

• dashpay/platform#3634: Both PRs modify MnemonicResolverCoreSigner's Signer implementation in the same file.
• dashpay/platform#3671: The main PR's extended_public_key addition to FixedKeySigner/Signer is required by this PR's new signer-based constructors.
• dashpay/platform#3757: Both PRs update the same Cargo.toml workspace dependency git rev pins for Dashcore-related crates.

Suggested labels: ready for final review

Suggested reviewers: QuantumExplorer, shumkov, lklimek

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is relevant to the dependency bump, though it omits the signer and zeroization changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ 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 chore/bump-rust-dashcore-dev

---

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.}

[thepastaclaw]

✅ Review complete (commit 5a2b2b5)

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.18%. Comparing base (a3a4d43) to head (5a2b2b5).

Files with missing lines	Patch %	Lines
packages/rs-dpp/src/state_transition/mod.rs	0.00%	2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##           v4.1-dev    #3976      +/-   ##
============================================
- Coverage     87.18%   87.18%   -0.01%     
============================================
  Files          2632     2632              
  Lines        327563   327565       +2     
============================================
  Hits         285592   285592              
- Misses        41971    41973       +2     

Components	Coverage Δ
dpp	87.70% <0.00%> (-0.01%)	⬇️
drive	86.14% <ø> (ø)
drive-abci	89.45% <ø> (ø)
sdk	∅ <ø> (∅)
dapi-client	∅ <ø> (∅)
platform-version	∅ <ø> (∅)
platform-value	92.20% <ø> (ø)
platform-wallet	∅ <ø> (∅)
drive-proof-verifier	49.55% <ø> (ø)

🚀 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

Dependency bump to rust-dashcore rev 78d10022 with adaptations for the new required Signer::extended_public_key trait method. One blocking gap: a third impl KwSigner for FixedKeySigner in the shielded signing tests was missed and will fail to compile whenever the shielded-client feature is enabled. Zeroization refactor is careful but slightly regresses stack residue by returning a Copy ExtendedPrivKey from the helper.

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

Findings not posted inline (1)

These findings could not be anchored to the current diff, but they are still part of this review.

• [BLOCKING] packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs:60-80: Missing extended_public_key on shielded test FixedKeySigner — breaks build under shielded-client — The two sibling impl KwSigner for FixedKeySigner blocks in packages/rs-dpp/src/state_transition/mod.rs (line 3374) and state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs (line 241) were updated with the new required extended_public_key stub, but th...

🤖 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-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs`:
- [BLOCKING] packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs:60-80: Missing `extended_public_key` on shielded test `FixedKeySigner` — breaks build under `shielded-client`
  The two sibling `impl KwSigner for FixedKeySigner` blocks in `packages/rs-dpp/src/state_transition/mod.rs` (line 3374) and `state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs` (line 241) were updated with the new required `extended_public_key` stub, but this file's impl at line 60 was missed. `key_wallet::signer::Signer` in rev `78d10022` requires `extended_public_key` as a non-defaulted method, so this impl fails with `E0046: not all trait items implemented, missing: extended_public_key` under `--features state-transition-signing,core_key_wallet,shielded-client`.

  The gap slipped past `cargo check --workspace --all-targets` because the file is gated on `#![cfg(all(test, feature = "state-transition-signing", feature = "core_key_wallet", feature = "shielded-client"))]` and `shielded-client` is not part of the default feature bundle. Any downstream consumer or CI job enabling shielded features will fail to compile. Also add `ExtendedPubKey` to the `use key_wallet::bip32::…` import at line 36.
- [BLOCKING] packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs:60-80: Missing `extended_public_key` on shielded test `FixedKeySigner` — breaks build under `shielded-client`
  The two sibling `impl KwSigner for FixedKeySigner` blocks in `packages/rs-dpp/src/state_transition/mod.rs` (line 3374) and `state_transitions/address_funds/address_funding_from_asset_lock_transition/signing_tests.rs` (line 241) were updated with the new required `extended_public_key` stub, but this file's impl at line 60 was missed. `key_wallet::signer::Signer` in rev `78d10022` requires `extended_public_key` as a non-defaulted method, so this impl fails with `E0046: not all trait items implemented, missing: extended_public_key` under `--features state-transition-signing,core_key_wallet,shielded-client`.

  The gap slipped past `cargo check --workspace --all-targets` because the file is gated on `#![cfg(all(test, feature = "state-transition-signing", feature = "core_key_wallet", feature = "shielded-client"))]` and `shielded-client` is not part of the default feature bundle. Any downstream consumer or CI job enabling shielded features will fail to compile. Also add `ExtendedPubKey` to the `use key_wallet::bip32::…` import at line 36.

In `packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs`:
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:247-338: `resolve_and_derive` returns `Copy` `ExtendedPrivKey`, leaving an extra unwiped private-scalar stack copy
  `key_wallet::bip32::ExtendedPrivKey` derives `Copy` (confirmed in `key-wallet/src/bip32.rs` at rev `78d10022`), so `Ok(derived)` performs a bitwise copy across the function boundary. Callers (`derive_priv` at line 349, `extended_public_key` at line 418) then wipe their local `derived.private_key`, but the origin scalar in `resolve_and_derive`'s stack slot remains un-wiped.

  This is the same class of leak the existing `TODO(upstream)` at lines 310–319 acknowledges (upstream fix is `ZeroizeOnDrop` on `ExtendedPrivKey`/`SecretKey`), but the refactor genuinely adds one more transient copy relative to the pre-refactor `derive_priv` flow, which extracted `secret_bytes()` and wiped the derived scalar in the same frame that created it. This can be fixed here without waiting for upstream: instead of returning `ExtendedPrivKey`, have `resolve_and_derive` take a closure `FnOnce(&ExtendedPrivKey) -> T` (or one variant per extraction — `Zeroizing<[u8; 32]>` for the scalar, `ExtendedPubKey` for the xpub) so the raw xpriv never crosses a function boundary.

  Zeroization is defense-in-depth, so this is a suggestion rather than blocking — but it directly contradicts the PR description's claim that the refactor preserves the wipe-on-every-return-path invariant.
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:414-425: No test coverage for `extended_public_key` derivation
  The new required `Signer::extended_public_key` method derives and returns a full BIP-32 `ExtendedPubKey` (public point + chain code), but the test module only exercises `sign_ecdsa`, `public_key`, resolver errors, and `supported_methods`. That leaves the new production path unpinned: a refactor could return the wrong path's xpub, lose network/depth/chain-code metadata, or accidentally derive only a raw public key while all existing tests still pass. Add a deterministic test that calls `extended_public_key(&test_path())` against a known BIP-39 test vector and asserts equality with an independently constructed `ExtendedPubKey::from_priv(&secp, &ExtendedPrivKey::new_master(...).derive_priv(...))` on the same mnemonic/network/path.

[thepastaclaw]

Code Review

Incremental review: 1 prior finding(s) fixed and 3 carried forward; latest delta only updates the shielded signer test. Latest push (27d235f) resolves the sole prior blocking issue by adding the required extended_public_key stub to the shielded-client FixedKeySigner test signer. No new defects were introduced by the delta. Three prior non-blocking findings on mnemonic_resolver_core_signer.rs are unchanged in the current head and are carried forward: resolve_and_derive still returns Copy ExtendedPrivKey by value (unwiped helper-frame residue), the new extended_public_key path still lacks direct test coverage, and a zeroization comment still contains a stale line-number reference.

🟡 4 suggestion(s) | 💬 2 nitpick(s)

Carried-forward findings already raised (6)

These findings were not re-posted as new inline comments because an existing review thread already covers them.

• [SUGGESTION] (deduped existing open thread) packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:247-338: Carried forward: resolve_and_derive returns Copy ExtendedPrivKey, leaving an unwiped private-scalar stack copy in the helper frame — Carried forward from the prior review at ef7cdf2 — this file is untouched in the latest delta. key_wallet::bip32::ExtendedPrivKey derives Copy at rust-dashcore rev 78d10022, so Ok(derived) at line 337 performs a bitwise copy across the function boundary. Callers derive_priv (line 356)...
• [SUGGESTION] (deduped existing open thread) packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:414-425: Carried forward: no test coverage for extended_public_key derivation — Carried forward from the prior review at ef7cdf2 — still valid at head. The #[cfg(test)] mod tests block (starting line 428) exercises sign_ecdsa, public_key, NotFound, NullHandle, and supported_methods, but never calls extended_public_key, even though this PR introduces it as a ne...
• [NITPICK] (deduped existing open thread) packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:322-326: Carried forward: stale line-number reference in zeroization comment — Carried forward from the prior review at ef7cdf2 — still valid at head. The comment at line 324 says line 323's erase would be skipped without this explicit call, but line 323 is the start of this very comment; the actual success-path master.private_key.non_secure_erase() is on line 335 (the...
• [SUGGESTION] (deduped existing open thread) packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:247-338: resolve_and_derive returns Copy ExtendedPrivKey, leaving an extra unwiped private-scalar stack copy — Prior finding still valid at current head. Current head still returns Result from resolve_and_derive; latest delta did not touch this file. key_wallet::bip32::ExtendedPrivKey derives Copy (confirmed in key-wallet/src/bip32.rs at rev 78d10022), so Ok(derived) performs a...
• [SUGGESTION] (deduped existing open thread) packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:414-425: No test coverage for extended_public_key derivation — Prior finding still valid at current head. Test module still covers sign_ecdsa/public_key/errors/supported_methods but has no extended_public_key call. The new required Signer::extended_public_key method derives and returns a full BIP-32 ExtendedPubKey (public point + chain code), but the tes...
• [NITPICK] (deduped existing open thread) packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:322-326: Stale line-number reference in zeroization comment — Prior finding still valid at current head. Comment still says line 323's erase would be skipped. The comment says line 323's erase would be skipped without this explicit call, but line 323 is itself the start of this comment; the actual success-path master.private_key.non_secure_erase() is no...

🤖 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-sdk-ffi/src/mnemonic_resolver_core_signer.rs`:
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:247-338: Carried forward: `resolve_and_derive` returns `Copy` `ExtendedPrivKey`, leaving an unwiped private-scalar stack copy in the helper frame
  Carried forward from the prior review at ef7cdf2d — this file is untouched in the latest delta. `key_wallet::bip32::ExtendedPrivKey` derives `Copy` at rust-dashcore rev `78d10022`, so `Ok(derived)` at line 337 performs a bitwise copy across the function boundary. Callers `derive_priv` (line 356) and `extended_public_key` (line 423) wipe their local `derived.private_key`, but the origin scalar in `resolve_and_derive`'s stack slot is never wiped.

  This is exactly the class of residue the `TODO(upstream)` at lines 310-319 acknowledges, but the refactor materially adds one more transient copy vs the pre-refactor `derive_priv` path, which extracted `secret_bytes()` and wiped the derived scalar in the same frame that created it. That contradicts the PR description's stated wipe-on-every-return-path invariant.

  Fixable locally without waiting for upstream: have `resolve_and_derive` accept a `FnOnce(&ExtendedPrivKey) -> T` closure (or split into two extraction helpers — one returning `Zeroizing<[u8; 32]>` for the scalar, one returning `ExtendedPubKey`) so the raw xpriv never crosses a function boundary. Zeroization is defense-in-depth, hence suggestion rather than blocking.
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:414-425: Carried forward: no test coverage for `extended_public_key` derivation
  Carried forward from the prior review at ef7cdf2d — still valid at head. The `#[cfg(test)] mod tests` block (starting line 428) exercises `sign_ecdsa`, `public_key`, `NotFound`, `NullHandle`, and `supported_methods`, but never calls `extended_public_key`, even though this PR introduces it as a new required-trait production path that returns a full BIP-32 `ExtendedPubKey` (public point + chain code + depth + network).

  Without a test, a future refactor could return the wrong path's xpub, drop chain-code/depth/network metadata, or accidentally return only a leaf public key while every existing test still passes. This is the FFI-visible signer boundary that Swift wallet code will use for local non-hardened child derivation — silent metadata loss is exactly what regressions look like here.

  Add a deterministic test that calls `signer.extended_public_key(&test_path()).await` against the existing `ENGLISH_PHRASE` BIP-39 vector and asserts equality with an independently constructed `ExtendedPubKey::from_priv(&secp, &ExtendedPrivKey::new_master(Network::Testnet, &Mnemonic::parse(ENGLISH_PHRASE)?.to_seed(""))?.derive_priv(&secp, &test_path())?)` — plus a spot-check on `depth`, `network`, and `chain_code` so future refactors can't silently strip that metadata.
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:247-338: `resolve_and_derive` returns `Copy` `ExtendedPrivKey`, leaving an extra unwiped private-scalar stack copy
  Prior finding still valid at current head. Current head still returns Result<ExtendedPrivKey> from resolve_and_derive; latest delta did not touch this file.

  `key_wallet::bip32::ExtendedPrivKey` derives `Copy` (confirmed in `key-wallet/src/bip32.rs` at rev `78d10022`), so `Ok(derived)` performs a bitwise copy across the function boundary. Callers (`derive_priv` at line 349, `extended_public_key` at line 418) then wipe their local `derived.private_key`, but the origin scalar in `resolve_and_derive`'s stack slot remains un-wiped.

  This is the same class of leak the existing `TODO(upstream)` at lines 310–319 acknowledges (upstream fix is `ZeroizeOnDrop` on `ExtendedPrivKey`/`SecretKey`), but the refactor genuinely adds one more transient copy relative to the pre-refactor `derive_priv` flow, which extracted `secret_bytes()` and wiped the derived scalar in the same frame that created it. This can be fixed here without waiting for upstream: instead of returning `ExtendedPrivKey`, have `resolve_and_derive` take a closure `FnOnce(&ExtendedPrivKey) -> T` (or one variant per extraction — `Zeroizing<[u8; 32]>` for the scalar, `ExtendedPubKey` for the xpub) so the raw xpriv never crosses a function boundary.

  Zeroization is defense-in-depth, so this is a suggestion rather than blocking — but it directly contradicts the PR description's claim that the refactor preserves the wipe-on-every-return-path invariant.
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:414-425: No test coverage for `extended_public_key` derivation
  Prior finding still valid at current head. Test module still covers sign_ecdsa/public_key/errors/supported_methods but has no extended_public_key call.

  The new required `Signer::extended_public_key` method derives and returns a full BIP-32 `ExtendedPubKey` (public point + chain code), but the test module only exercises `sign_ecdsa`, `public_key`, resolver errors, and `supported_methods`. That leaves the new production path unpinned: a refactor could return the wrong path's xpub, lose network/depth/chain-code metadata, or accidentally derive only a raw public key while all existing tests still pass. Add a deterministic test that calls `extended_public_key(&test_path())` against a known BIP-39 test vector and asserts equality with an independently constructed `ExtendedPubKey::from_priv(&secp, &ExtendedPrivKey::new_master(...).derive_priv(...))` on the same mnemonic/network/path.

[thepastaclaw]

Code Review

Incremental review of 27d235fe..3be6885 plus revalidation of the prior open findings. The latest push fixes the prior zeroization-residue concern by moving the rust-dashcore pin to f42498e0 from rust-dashcore PR #833, where ExtendedPrivKey now implements Zeroize, then wrapping both master and derived xprivs in Zeroizing<ExtendedPrivKey>. The stale zeroization line-number comment is also fixed. I did not find a new code blocker in the Platform delta.

Merge caveat: current Cargo.toml pins all rust-dashcore workspace deps to f42498e0d04257e28b4e457c16629904a872ab61, which is the head of open dashpay/rust-dashcore#833, not yet a merged/tagged rust-dashcore ref. Please do not merge this Platform PR until #833 lands on a durable rust-dashcore ref, or update the pin to the merged/tagged commit if it changes.

One nonblocking suggestion remains: MnemonicResolverCoreSigner::extended_public_key still has no direct unit test. The existing mnemonic_resolver_core_signer tests cover sign_ecdsa, public_key, resolver errors, null handle, and advertised methods, but never call extended_public_key, so they would not catch losing BIP-32 chain-code/depth/network metadata on this new trait path.

Validation run locally at 3be6885:

• cargo check -p dpp --tests --features state-transition-signing,core_key_wallet,shielded-client --locked
• cargo check -p rs-sdk-ffi --tests --locked
• cargo test -p rs-sdk-ffi mnemonic_resolver_core_signer --locked (5 passed)

GitHub CI still had Build JS packages / Build JS and Rust workspace tests / Tests (macOS) in progress when I reviewed.

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[thepastaclaw]

Code Review

Incremental review at 3be6885 vs prior 27d235f. The delta is a clean refactor that replaces manual non_secure_erase() on ExtendedPrivKey fields with Zeroizing<ExtendedPrivKey>, leaning on rust-dashcore PR #833's new Zeroize impl, and repins all 8 rust-dashcore workspace deps to PR #833's branch head f42498e0. Prior findings: prior-1 (Copy residue on the derived local) and prior-2 (no test for extended_public_key) remain valid; prior-3 (stale line-number comment) is fixed by the doc rewrite. One new blocking issue in the delta: the workspace is pinned to an unmerged upstream PR's branch head.

🔴 1 blocking | 🟡 2 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 `Cargo.toml`:
- [BLOCKING] Cargo.toml:53-60: Workspace pinned to unmerged rust-dashcore PR #833 branch head — repin before merging
  All 8 rust-dashcore workspace dependencies are pinned to `f42498e0d04257e28b4e457c16629904a872ab61`, which is the head of unmerged rust-dashcore PR #833 (the `Zeroize for ExtendedPrivKey` change this platform PR depends on). Merging this branch as-is would make platform's build depend on an integration PR commit rather than a protected upstream line — the branch could be rebased or force-pushed, invalidating the pin. The PR description already flags this as a pre-merge gate. Once rust-dashcore#833 lands on `dev`, repin all 8 `rev` values to the resulting `dev` head (or a release tag) and refresh `Cargo.lock` before this PR is merged.

In `packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs`:
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:313-317: Carried forward (prior-1, STILL VALID): unwrapped `derived: ExtendedPrivKey` leaves an unwiped `Copy` stack slot before `Zeroizing::new`
  The refactor materially improved the surrounding code by wrapping `master` in `Zeroizing` and removing the manual `non_secure_erase()` calls, but the `derived` intermediate at lines 313-315 is still bound as a bare `ExtendedPrivKey` before being wrapped at line 317. Upstream `key-wallet::bip32::ExtendedPrivKey` remains `Copy` in PR #833 (Zeroize is added but Copy is not removed), so `Zeroizing::new(derived)` performs a bitwise copy of the private scalar into the returned wrapper. The original `derived` stack slot retains a byte-for-byte copy of the scalar until that stack region is reused; the `Zeroize` impl only fires through the outer wrapper. This narrowly contradicts the module-level docstring's advertised invariant that no `ExtendedPrivKey` scalar survives past the trait-method boundary. Inline the derivation into the return expression, or bind directly as `let derived: Zeroizing<ExtendedPrivKey> = Zeroizing::new(master.derive_priv(&secp, path).map_err(...)?);` so the raw `ExtendedPrivKey` never occupies a named stack slot. This is defense-in-depth hardening, not a correctness bug.
- [SUGGESTION] packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs:391-401: Carried forward (prior-2, STILL VALID): `extended_public_key` has no direct test coverage
  This PR adds a real production implementation of `Signer::extended_public_key` that will be called across the Swift/Rust FFI boundary by wallet code doing non-hardened local child derivation, but the `#[cfg(test)] mod tests` block only exercises `sign_ecdsa`, `public_key`, resolver `NOT_FOUND`, null-handle, and `supported_methods`. The xpub path has different output semantics than `public_key`: it must preserve BIP-32 chain code, depth, parent fingerprint, child number, network, and public point. A regression that returns the wrong path's xpub or silently drops metadata would leave all existing tests passing. Add a deterministic test using the existing `ENGLISH_PHRASE` fixture that calls `signer.extended_public_key(&test_path()).await`, then independently derives the reference `ExtendedPubKey` via `ExtendedPrivKey::new_master(...).derive_priv(...); ExtendedPubKey::from_priv(...)` and asserts full equality (including `network`, `depth`, `chain_code`, and encoded xpub string).

[thepastaclaw]

🔴 Blocking: Workspace pinned to unmerged rust-dashcore PR #833 branch head — repin before merging

All 8 rust-dashcore workspace dependencies are pinned to f42498e0d04257e28b4e457c16629904a872ab61, which is the head of unmerged rust-dashcore PR #833 (the Zeroize for ExtendedPrivKey change this platform PR depends on). Merging this branch as-is would make platform's build depend on an integration PR commit rather than a protected upstream line — the branch could be rebased or force-pushed, invalidating the pin. The PR description already flags this as a pre-merge gate. Once rust-dashcore#833 lands on dev, repin all 8 rev values to the resulting dev head (or a release tag) and refresh Cargo.lock before this PR is merged.

_{source: ['codex']}

[thepastaclaw]

Still present at b175d50.

[Claudius-Maginificent]

Correct and already tracked — this is a deliberate interim state, not an oversight. The PR description's "Blocking relationship" and "Notes for reviewers" sections (updated with this push) call out the same gate explicitly: all 8 rev pins point at rust-dashcore PR #833's branch head because that's what supplies Zeroize for ExtendedPrivKey, and this PR must not merge to v4.1-dev until #833 lands on rust-dashcore dev (or a tag) and the pins are refreshed accordingly. Leaving this thread open as the pre-merge gate it is — will repin and refresh Cargo.lock once #833 merges upstream.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

[thepastaclaw]

Correction: this finding is still present at 5a2b2b50, not resolved. The current top-level review carries it forward as the single blocking issue: Cargo.toml still pins all 8 rust-dashcore workspace deps to unmerged PR #833 head f42498e0d04257e28b4e457c16629904a872ab61.

_{The earlier auto-reconciliation text on this comment was wrong; the top-level review at this SHA is the canonical result.}

[thepastaclaw]

Code Review

Prior findings on mnemonic_resolver_core_signer.rs are resolved: resolve_and_derive now takes an extraction closure so ExtendedPrivKey never crosses a helper boundary, and extended_public_key has direct test coverage against an independent BIP-39/BIP-32 derivation. One blocking issue remains carried forward: all 8 rust-dashcore workspace deps are still pinned to the unmerged PR #833 head f42498e0, which the PR description itself flags as a pre-merge gate.

🔴 1 blocking

Carried-forward findings already raised (1)

These findings were not re-posted as new inline comments because an existing review thread already covers them.

• [BLOCKING] (deduped existing open thread) Cargo.toml:53-60: Repin rust-dashcore workspace deps once PR #833 merges — do not merge on unmerged branch head — All 8 rust-dashcore workspace entries (dashcore, dash-network-seeds, dash-spv, key-wallet, key-wallet-ffi, key-wallet-manager, dash-network, dashcore-rpc) are pinned to f42498e0d04257e28b4e457c16629904a872ab61, which is the head of the still-open rust-dashcore PR #833. That comm...

🤖 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 `Cargo.toml`:
- [BLOCKING] Cargo.toml:53-60: Repin rust-dashcore workspace deps once PR #833 merges — do not merge on unmerged branch head
  All 8 rust-dashcore workspace entries (`dashcore`, `dash-network-seeds`, `dash-spv`, `key-wallet`, `key-wallet-ffi`, `key-wallet-manager`, `dash-network`, `dashcore-rpc`) are pinned to `f42498e0d04257e28b4e457c16629904a872ab61`, which is the head of the still-open rust-dashcore PR #833. That commit is what supplies the `Zeroize` impl for `ExtendedPrivKey` that the refactor in `mnemonic_resolver_core_signer.rs` now depends on. Until #833 lands on `dev`, platform depends on a feature-branch commit that can be rebased, force-pushed, or garbage-collected — any of which would silently break future `cargo` fetches or allow the pinned code to diverge from what upstream ends up shipping. The PR description itself flags this as a pre-merge gate. Once rust-dashcore #833 merges to `dev`, re-pin all 8 entries to the resulting `dev` HEAD (or a release tag) and refresh Cargo.lock before merging.

[thepastaclaw]

Code Review

One blocking finding carried forward: all 8 rust-dashcore workspace deps at Cargo.toml:53-60 remain pinned to unmerged rust-dashcore PR #833 head f42498e0. Latest push (5a2b2b5) is doc/test-only refinement — no new defects. Prior findings on the ExtendedPrivKey Copy transient and extended_public_key test coverage remain resolved.

🔴 1 blocking

Carried-forward findings already raised (1)

These findings were not re-posted as new inline comments because an existing review thread already covers them.

• [BLOCKING] (deduped existing open thread) Cargo.toml:53-60: Repin rust-dashcore workspace deps before merge — currently on unmerged PR #833 head — All 8 rust-dashcore workspace entries (dashcore, dash-network-seeds, dash-spv, key-wallet, key-wallet-ffi, key-wallet-manager, dash-network, dashcore-rpc) at Cargo.toml:53-60 are pinned to f42498e0d04257e28b4e457c16629904a872ab61, the head of the still-open rust-dashcore PR #833...

🤖 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 `Cargo.toml`:
- [BLOCKING] Cargo.toml:53-60: Repin rust-dashcore workspace deps before merge — currently on unmerged PR #833 head
  All 8 rust-dashcore workspace entries (`dashcore`, `dash-network-seeds`, `dash-spv`, `key-wallet`, `key-wallet-ffi`, `key-wallet-manager`, `dash-network`, `dashcore-rpc`) at Cargo.toml:53-60 are pinned to `f42498e0d04257e28b4e457c16629904a872ab61`, the head of the still-open rust-dashcore PR #833. That commit is the sole source of the `Zeroize` impl for `ExtendedPrivKey` that this PR's `Zeroizing<ExtendedPrivKey>` refactor in `packages/rs-sdk-ffi/src/mnemonic_resolver_core_signer.rs` depends on. Because the pin references a feature-branch commit (not a merged commit on `dev` or a release tag), it can be rebased, force-pushed, or garbage-collected upstream — any of which would silently break future `cargo` fetches or let the pinned code diverge from what upstream eventually ships. Codex confirmed PR #833's head has already moved to `a8c57fe863c9…`, so this pin is also stale relative to the current feature-branch tip. The PR description explicitly flags this as a pre-merge gate. Once rust-dashcore #833 merges to `dev`, re-pin all 8 entries to the resulting `dev` HEAD (or a release tag) and refresh `Cargo.lock` before merging.