Skip to content

build: update rust-dashcore to v0.44.0#3973

Open
thepastaclaw wants to merge 2 commits into
dashpay:v4.0-devfrom
thepastaclaw:build/rust-dashcore-0.43.0
Open

build: update rust-dashcore to v0.44.0#3973
thepastaclaw wants to merge 2 commits into
dashpay:v4.0-devfrom
thepastaclaw:build/rust-dashcore-0.43.0

Conversation

@thepastaclaw

@thepastaclaw thepastaclaw commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Issue being fixed or feature implemented

Updates Platform's rust-dashcore workspace dependencies from pinned rev 5c0113e7901551450f6063023eec4be95beeb6b9 to the published v0.44.0 release tag.

What was done?

  • Updated all rust-dashcore workspace dependencies in root Cargo.toml to tag = "v0.44.0":
    • dashcore
    • dash-network-seeds
    • dash-spv
    • key-wallet
    • key-wallet-ffi
    • key-wallet-manager
    • dash-network
    • dashcore-rpc
  • Refreshed Cargo.lock so rust-dashcore crates resolve to v0.44.0, peeled to commit 991c6ebe24d7ea8ba7d900a052b25be8c5498409.
  • Removed the previous local wallet FFI compatibility workarounds from this PR; this branch is now only the dependency bump plus the required lockfile refresh.

How Has This Been Tested?

Local validation on macOS arm64:

cargo metadata --locked --no-deps --format-version 1
git diff --check
cargo check -p platform-wallet-ffi --locked
cargo test -p platform-wallet-ffi mnemonic_words --locked

Pre-push code review gate:

code-review dashpay/platform upstream/v4.0-dev e40e084ac42b7e66e38c73cd27801b3b7275142b "Update rust-dashcore workspace dependencies from pinned rev 5c0113e7901551450f6063023eec4be95beeb6b9 to the v0.44.0 release tag and refresh Cargo.lock only as needed"

Result: Recommendation: ship.

Breaking Changes

None intended in Platform code. This PR only moves the rust-dashcore dependency source to the v0.44.0 release tag.

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

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • Chores
    • Updated several Dash-related workspace dependencies to point to newer pinned revisions.
    • This refreshes the underlying source used by the project while keeping the overall configuration unchanged.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1baa43d3-70e9-4921-940e-89a1878b00dd

📥 Commits

Reviewing files that changed from the base of the PR and between e40e084 and c624074.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • Cargo.toml

📝 Walkthrough

Walkthrough

Cargo.toml updates the git revision used for eight Dash-related workspace dependencies to a newer pinned commit.

Changes

Dash workspace dependency bump

Layer / File(s) Summary
Update Dash crate rev pins
Cargo.toml
The workspace git rev for dashcore, dash-network-seeds, dash-spv, key-wallet, key-wallet-ffi, key-wallet-manager, dash-network, and dashcore-rpc is updated.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Suggested reviewers

  • shumkov
  • lklimek
  • llbartekll
  • QuantumExplorer

Poem

🐇 I hopped by Cargo with a careful glance,
A new pinned commit got its little dance.
Dash crates now point to a fresher stream,
Tight and tidy, like a rabbit’s dream.
Hop hop! ✨

🚥 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 clearly and concisely describes the dependency bump to rust-dashcore v0.44.0.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@github-actions github-actions Bot added this to the v4.0.0 milestone Jun 30, 2026
@thepastaclaw

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor
✅ 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

thepastaclaw commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

✅ Review complete (commit c624074)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@Cargo.toml`:
- Around line 53-60: The git dependencies in Cargo.toml are pinned by tag, which
can drift if the tag is moved; update each of the listed dependency entries
(dashcore, dash-network-seeds, dash-spv, key-wallet, key-wallet-ffi,
key-wallet-manager, dash-network, and dashcore-rpc) to use the fixed commit SHA
with rev = "3d0d5dcd4ad64e2199a726651bca7f8ffac123e6" instead of tag =
"v0.43.0".

In `@packages/rs-platform-wallet-ffi/src/mnemonic_words.rs`:
- Around line 157-189: The CJK wrapping logic in the mnemonic normalization path
should avoid using a global string replacement for each matched candidate,
because `s.replace` in this loop can rewrite earlier or unrelated occurrences
and prevent longer matches from being inserted correctly. Update the candidate
insertion logic in the normalization routine that iterates over
`normalized.split(' ')` and scans `wchars` so replacements are applied at the
matched position only, preserving longer matches and the intended ordering of
inserted `IDEO_SP` separators.
🪄 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: 149c6b9f-6a68-410b-a6e1-e18c94e73866

📥 Commits

Reviewing files that changed from the base of the PR and between a3a4d43 and 6b3a351.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • Cargo.toml
  • packages/rs-platform-wallet-ffi/Cargo.toml
  • packages/rs-platform-wallet-ffi/src/mnemonic_words.rs

Comment thread Cargo.toml Outdated
Comment thread packages/rs-platform-wallet-ffi/src/mnemonic_words.rs Outdated

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

PR cleanly moves the workspace to rust-dashcore v0.43.0 and reintroduces three removed mnemonic recover-flow helpers (word_list, normalize_phrase, cleanup_phrase) inside platform-wallet-ffi. No consensus, FFI ABI, or memory-safety regressions. Real concerns center on (a) tag vs. rev pinning across all rust-dashcore git deps — also requested by a human reviewer — and (b) the new cleanup_phrase auto-split using global String::replace plus an asymmetric trim, in code paths the new tests do not actually validate against key_wallet's checker.

🟡 6 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`:
- [SUGGESTION] Cargo.toml:53-60: Pin rust-dashcore workspace deps to a commit SHA, not a moving tag
  All eight rust-dashcore workspace deps use `tag = "v0.43.0"`. Git tags are mutable — if upstream ever re-tags v0.43.0 (force-push, retag during a backport), every future `cargo update` silently pulls a different tree, with no Cargo.lock churn that names the new commit obviously. The prior pin in this workspace was a `rev = "…"`, and a human reviewer (Lukasz) and CodeRabbit independently requested keeping that convention by pinning to `rev = "3d0d5dcd4ad64e2199a726651bca7f8ffac123e6"` (what v0.43.0 currently resolves to). Switch all eight entries (`dashcore`, `dash-network-seeds`, `dash-spv`, `key-wallet`, `key-wallet-ffi`, `key-wallet-manager`, `dash-network`, `dashcore-rpc`) to that rev. The tag can stay as documentation in a comment.
- [SUGGESTION] Cargo.toml:53-60: v0.43.0 tag is older than the previously pinned post-release rev — audit dropped commits
  The previous pin was rev `5c0113e7901551450f6063023eec4be95beeb6b9`, described as a post-release commit on top of v0.43.0; this PR moves back to the tag (commit `3d0d5dcd…`). Any rust-dashcore changes that landed between v0.43.0 and `5c0113e7` — including potential fixes in `dashcore`, `dash-spv`, `dashcore-rpc`, or `key-wallet` — are silently reverted. Before merging, enumerate the dropped commits (`git log 3d0d5dcd..5c0113e7` in the rust-dashcore tree) and confirm none are security-relevant, and consider noting the audit in the PR description so reviewers do not have to re-derive it.

In `packages/rs-platform-wallet-ffi/src/mnemonic_words.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/mnemonic_words.rs:160-189: cleanup_phrase CJK auto-split uses global String::replace, corrupting unrelated occurrences
  Line 177's `s = s.replace(&candidate, &wrapped)` is an unbounded global replacement called from inside a per-word, per-window scanner. Any other position in `s` that happens to contain `candidate` — a different word, a different window within the same word, or a substring of a longer valid match later in the scan — is wrapped in ideographic spaces as well, fragmenting previously-valid words. The `i += j - 1` / `i += 1` cursor advancement assumes only the current window mutated, which the global replace violates; the trailing `while s.contains(&dbl_ideo) { … IDEO_SP }` collapse only fixes adjacent ideographic spaces, it cannot rejoin a word that was split mid-way. Replace with a position-aware splitter: walk the original word left-to-right with a longest-match table-lookup against `word_in_any_list`, and write the result into a fresh buffer. The current Japanese test (line 347) only asserts that *some* ideographic space appears, so this bug is not exercised — strengthening that test (see separate finding) will surface it.
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/mnemonic_words.rs:84-113: word_list (bip39 crate) and phrase validator (key_wallet) are independent sources of truth
  `word_list` / `word_in_any_list` source words from the new `bip39 = 2.2` dependency, while `phrase_is_valid_any` still routes through `key_wallet::Mnemonic::validate`, which uses key-wallet's embedded wordlists. BIP-39 lists are standardized today so both tables agree, but there is no compile-time or test-time guarantee they stay byte-identical across upgrades of either crate. If they ever drift (typo fix, locale variant, NFC/NFKD form), the recover UI will accept a word that the real validator rejects (or vice versa), and `cleanup_phrase`'s auto-split will build phrases that pass membership but fail seed derivation — a confusing UX failure that only appears on real recover input. Add a test that asserts `word_list(lang)` equals the wordlist key-wallet uses for each `Language` (or that every word in `word_list(lang)` makes `key_wallet::Mnemonic::validate` succeed in a phrase composed only of that wordlist). Long-term, request a `word_list()` accessor on `key_wallet::mnemonic::Language` upstream and drop the `bip39` direct dep.
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/mnemonic_words.rs:150-155: cleanup_phrase fast path leaks trailing whitespace
  Line 150 does `s = s.trim_start().to_string()`. If the phrase already validates after normalization (lines 152–155), `s` is returned without ever trimming the end, so input like `"abandon … about \n"` is returned as `"abandon … about "`. The autosplit branch explicitly uses `s.trim()` at line 181, and the doc on `normalize_phrase` (line 117) promises "ends trimmed" — the asymmetry is almost certainly unintentional. Change line 150 to `s.trim().to_string()`.
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/mnemonic_words.rs:327-350: Japanese no-space autosplit test does not check that the result is a valid phrase
  `mnemonic_words_japanese_spaced_phrase_validates_and_no_space_phrase_autosplits` only asserts `cleanup_phrase(&nospace).contains(IDEO_SP)` — a splitter that wraps arbitrary CJK substrings still passes. The contract of `cleanup_phrase` for a no-space CJK paste is that it reconstructs a *valid* phrase, so the test should assert `phrase_is_valid_any(&normalize_phrase(&cleanup_phrase(&nospace)))`, or assert it round-trips to the original normalized phrase. This will also exercise the global-replace concern above against a realistic 12-word input.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Latest delta (642b9dd) is a clean cargo-only change: converts all eight rust-dashcore workspace deps from tag = "v0.43.0" to rev = "3d0d5dcd..." plus matching Cargo.lock churn. Prior-1 is fixed. No new findings from the latest delta. Four prior findings against packages/rs-platform-wallet-ffi/src/mnemonic_words.rs (file unchanged) and the dropped-commits audit on Cargo.toml are carried forward. Prior-3 (wordlist source divergence) is dropped as outdated — key_wallet v0.43.0 itself sources wordlists from the bip39 crate, so there is no real divergence.

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

Findings not posted inline (2)

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

  • [NITPICK] (no file): Untitled finding
  • [NITPICK] (no file): Untitled finding
🤖 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/mnemonic_words.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/mnemonic_words.rs:160-189: cleanup_phrase CJK auto-split uses global String::replace, corrupting unrelated occurrences
  Carried forward (prior-2; STILL VALID — file unchanged in this delta). The auto-split loop scans one CJK word/window at a time, but line 177's `s = s.replace(&candidate, &wrapped)` is an unbounded global replacement over the entire phrase. Any other position in `s` that contains `candidate` — a substring of a different word, an earlier window in the same word, or a substring of a later valid match — also gets wrapped in ideographic spaces, fragmenting previously-valid words. The cursor advancement (`i += j - 1` / `i += 1`) assumes only the current window mutated, which the global replace violates; the trailing `while s.contains(&dbl_ideo) { … IDEO_SP }` collapse only fixes adjacent ideographic spaces and cannot rejoin a word split mid-way. Reachable across the FFI: Swift `Mnemonic.cleanupPhrase` returns the corrupted string into the recover flow. Concrete failure mode: a valid Japanese phrase containing both `かいせん` and `いせん` would have `かいせん` split as `か` / `いせん`, producing 13 tokens after Swift sees the cleaned string. Replace with a position-aware splitter that walks the original word left-to-right with a longest-match table lookup against `word_in_any_list` and writes into a fresh buffer.
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/mnemonic_words.rs:150-155: cleanup_phrase fast path leaks trailing whitespace across the FFI boundary
  Carried forward (prior-4; STILL VALID). Line 150 uses `s.trim_start().to_string()`. If the normalized phrase validates at lines 152–155, `s` is returned without trailing whitespace ever being stripped, so input like `"abandon … about \n"` is returned as `"abandon … about "` and crosses the C boundary unchanged. The autosplit branch uses `s.trim()` at line 181, and `normalize_phrase`'s doc (line 117) promises "ends trimmed". The asymmetry is almost certainly unintentional and means Swift callers see inconsistent cleanup results depending on which branch ran. Trim both ends.
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/mnemonic_words.rs:327-350: Japanese no-space autosplit test does not verify the cleaned phrase is valid
  Carried forward (prior-5; STILL VALID). `mnemonic_words_japanese_spaced_phrase_validates_and_no_space_phrase_autosplits` only asserts `cleanup_phrase(&nospace).contains(IDEO_SP)`. A splitter that wraps arbitrary CJK substrings — the precise failure mode in the global-replace finding above — still passes this test. The contract `cleanupPhrase` advertises to Swift for a no-space CJK paste is reconstruction of a *valid* phrase, so the test should also assert validity. Strengthening this assertion exercises the global-replace bug on a realistic 12-word input.

In `Cargo.toml`:
- [SUGGESTION] Cargo.toml:53-60: Audit rust-dashcore commits dropped by moving back to the v0.43.0 release commit
  Carried forward (prior-6; STILL VALID). The latest delta correctly pins by `rev` (fixing prior-1) but the new pin `3d0d5dcd4ad64e2199a726651bca7f8ffac123e6` is the v0.43.0 *release* commit, while the previous pin `5c0113e7901551450f6063023eec4be95beeb6b9` was a post-v0.43.0 commit on `v4.0-dev`. Commits in the range `3d0d5dcd..5c0113e7` are silently reverted across `dashcore`, `dash-spv`, `dashcore-rpc`, `key-wallet`, `key-wallet-ffi`, and `key-wallet-manager`. A reviewer flagged at least one concrete concern: post-release commit `7ff6b246 fix(key-wallet): correct CoinJoin discovery` is reportedly in the dropped range — if accurate, it changes CoinJoin derivation paths and discovery logic, which affects what a restored wallet sees. Before merging, please enumerate the dropped commits (`git log 3d0d5dcd..5c0113e7` in rust-dashcore), confirm none touch the FFI/header surface or fix consensus-/wallet-relevant bugs that Platform relies on, and summarize the audit in the PR description so reviewers don't have to re-derive it.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Latest delta fixes three of the four prior findings (CJK global replace, fast-path trim, Japanese no-space test assertion). One real-but-narrow new edge case remains: the 8-char NFKD candidate window in cleanup_phrase cannot match the single 9-NFKD-char Japanese wordlist entry だいじょうぶ, so any no-space recover input containing that word will fail to round-trip. The Cargo.toml downgrade-audit suggestion is carried forward only as a low-confidence nit since the PR description now enumerates the dropped commits.

🟡 1 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-ffi/src/mnemonic_words.rs`:
- [SUGGESTION] packages/rs-platform-wallet-ffi/src/mnemonic_words.rs:180-197: 8-char NFKD window misses the only Japanese wordlist entry longer than 8 NFKD chars (だいじょうぶ)
  `let mut j = core::cmp::min(8, wlen - i);` caps the longest candidate the splitter will try at 8 NFKD scalars. The bip39 Japanese wordlist (which key-wallet uses verbatim and which this PR matches against via `word_in_any_list`) is stored in NFKD form; every entry except one fits in 8 NFKD scalars, but だいじょうぶ decomposes to た+◌゙ い し+◌゙ ょ う ふ+◌゙ — 9 scalars — because each voiced kana expands to base + U+3099. Trace: at the position of だ the loop's largest candidate is the 8-scalar prefix `だいじょうふ` (the trailing U+3099 dropped), which is not in any wordlist. j then decrements; no shorter prefix is in any wordlist either, so the matcher either skips this position or later wraps shorter substrings such as `いじょう`, leaving a cleaned phrase that no longer round-trips through `phrase_is_valid_any(normalize_phrase(...))`. The current `mnemonic_words_japanese_spaced_phrase_validates_and_no_space_phrase_autosplits` fixture happens to use indices [145, 1301, 1265, 427, 1647, 960, 483, 1581, 482, 726, 1234, 1924], none of which contain voiced marks, so the regression is invisible today. Replace the hardcoded `8` with the maximum NFKD scalar length across the supported wordlists (or iterate from `wlen - i` downward), and add a fixture whose 12 words include だいじょうぶ so the regression is locked in.

Comment thread packages/rs-platform-wallet-ffi/src/mnemonic_words.rs Outdated
Comment thread Cargo.toml Outdated

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Verified the latest delta (3aa07c5..1568e5d) against source. Prior-1 is FIXED: packages/rs-platform-wallet-ffi/src/mnemonic_words.rs:182 now uses let mut j = wlen - i; with an explanatory comment at lines 180-181, and the new test at lines 371-389 covers the だいじょうぶ NFKD-long-word case. Prior-2 is resolved at the documentation layer the agents converged on: the PR body's Release Commit Audit / Impact note now spells out that the dropped post-release rust-dashcore commits do not change the preserved Platform wallet FFI mnemonic surface; the missing inline Cargo.toml comment is below the noise threshold for a request. No new findings in the latest delta.

Note: GitHub does not allow me to approve my own PR, so this clean self-review is posted as a COMMENT while preserving the verifier result.

@thepastaclaw thepastaclaw force-pushed the build/rust-dashcore-0.43.0 branch from 1568e5d to e40e084 Compare June 30, 2026 16:29
@thepastaclaw thepastaclaw changed the title build: update rust-dashcore to v0.43.0 build: update rust-dashcore to v0.44.0 Jun 30, 2026
@thepastaclaw

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor
✅ 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
Cargo.toml (1)

53-60: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Pin the rust-dashcore workspace deps to the release commit, not the tag.

Lines 53-60 reintroduce mutable git refs. Cargo.lock only freezes the current resolution; the next lockfile refresh or any consumer building without that lockfile can resolve different code if v0.44.0 is ever retargeted. Since the PR already identifies the peeled release commit, keep the release intent but switch these entries back to rev = "991c6ebe24d7ea8ba7d900a052b25be8c5498409".

Suggested patch
-dashcore = { git = "https://github.com/dashpay/rust-dashcore", tag = "v0.44.0" }
-dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", tag = "v0.44.0" }
-dash-spv = { git = "https://github.com/dashpay/rust-dashcore", tag = "v0.44.0" }
-key-wallet = { git = "https://github.com/dashpay/rust-dashcore", tag = "v0.44.0" }
-key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", tag = "v0.44.0" }
-key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", tag = "v0.44.0" }
-dash-network = { git = "https://github.com/dashpay/rust-dashcore", tag = "v0.44.0" }
-dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", tag = "v0.44.0" }
+dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "991c6ebe24d7ea8ba7d900a052b25be8c5498409" }
+dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "991c6ebe24d7ea8ba7d900a052b25be8c5498409" }
+dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "991c6ebe24d7ea8ba7d900a052b25be8c5498409" }
+key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "991c6ebe24d7ea8ba7d900a052b25be8c5498409" }
+key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "991c6ebe24d7ea8ba7d900a052b25be8c5498409" }
+key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "991c6ebe24d7ea8ba7d900a052b25be8c5498409" }
+dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "991c6ebe24d7ea8ba7d900a052b25be8c5498409" }
+dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "991c6ebe24d7ea8ba7d900a052b25be8c5498409" }
#!/bin/bash
set -euo pipefail

echo "Resolve the published tag to its peeled commit:"
git ls-remote https://github.com/dashpay/rust-dashcore 'refs/tags/v0.44.0^{}'

echo
echo "Current workspace dependency declarations:"
sed -n '53,60p' Cargo.toml

echo
echo "Current lockfile source entries for the affected crates:"
rg -n -A1 -B1 '^(name = "(dashcore|dash-network-seeds|dash-spv|key-wallet|key-wallet-ffi|key-wallet-manager|dash-network|dashcore-rpc)"|source = "git\+https://github.com/dashpay/rust-dashcore)' Cargo.lock || true
🤖 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 `@Cargo.toml` around lines 53 - 60, The workspace dependency entries for
rust-dashcore are still pinned to the mutable v0.44.0 tag, which should be
replaced with the fixed release commit. Update the dependency declarations for
dashcore, dash-network-seeds, dash-spv, key-wallet, key-wallet-ffi,
key-wallet-manager, dash-network, and dashcore-rpc in Cargo.toml to use the
peeled release rev instead of tag, preserving the same git source. Use the
existing release commit identifier referenced in the PR so future lockfile
refreshes and builds without Cargo.lock stay on the intended code.
🤖 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.

Duplicate comments:
In `@Cargo.toml`:
- Around line 53-60: The workspace dependency entries for rust-dashcore are
still pinned to the mutable v0.44.0 tag, which should be replaced with the fixed
release commit. Update the dependency declarations for dashcore,
dash-network-seeds, dash-spv, key-wallet, key-wallet-ffi, key-wallet-manager,
dash-network, and dashcore-rpc in Cargo.toml to use the peeled release rev
instead of tag, preserving the same git source. Use the existing release commit
identifier referenced in the PR so future lockfile refreshes and builds without
Cargo.lock stay on the intended code.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6e1a07af-0ce0-4b5b-9e98-37fa27ec1598

📥 Commits

Reviewing files that changed from the base of the PR and between 6b3a351 and e40e084.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • Cargo.toml

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Carried-forward prior findings (prior-1 NFKD windowing, prior-2 Cargo.toml audit comment) are both OUTDATED at HEAD e40e084: the latest delta dropped the local packages/rs-platform-wallet-ffi/src/mnemonic_words.rs workaround entirely, leaving the cumulative PR as a clean retarget of all eight rust-dashcore workspace deps from pinned rev 5c0113e7 to tag v0.44.0 (Cargo.lock pins resolved commit 991c6ebe). No new findings emerged from the latest delta — five independent agents (claude/codex general, security, two FFI) all returned empty in-scope finding sets, and CodeRabbit has no actionable inline comments.

Note: GitHub does not allow me to submit this review event on my own PR, so this is posted as a COMMENT while preserving the verified findings.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Latest delta (e40e084..c624074) only swaps tag = "v0.44.0" for rev = "991c6ebe24d7ea8ba7d900a052b25be8c5498409" across the eight rust-dashcore workspace deps in Cargo.toml and refreshes Cargo.lock to record the same resolved source — a strictly stronger supply-chain pin since git tags are mutable but commit hashes are not. No source code changes, no FFI/ABI surface changes, and the prior review carried zero active findings. All historical prior items (mnemonic NFKD windowing, FFI-shim rationale comment) remain OUTDATED because the local Platform-side wallet FFI workaround they targeted is no longer part of this PR.

Note: GitHub does not allow me to submit this review event on my own PR, so this is posted as a COMMENT while preserving the verified findings.

@lklimek lklimek marked this pull request as ready for review June 30, 2026 17:42
@lklimek lklimek requested a review from QuantumExplorer as a code owner June 30, 2026 17:42
@lklimek lklimek added the ready for final review Ready for the final review. If AI was involved in producing this PR, it has already had a reviewer. label Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for final review Ready for the final review. If AI was involved in producing this PR, it has already had a reviewer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants