[PastaPastaPasta]

Issue being fixed or feature implemented

Dash Core builds Platform offline from a vendored workspace (dashpay/dash#7623). Its bundle script runs cargo vendor --locked, and that fails on v5.1-dev:

found duplicate version of package `base58ck v0.5.0` vendored from two sources:
	source 1: registry `crates-io`
	source 2: https://github.com/rust-bitcoin/rust-bitcoin?rev=7ba35c7c1dd63892bfb9c9395bbd80e2ab5df516#7ba35c7c

Cargo can't put one name and version from two sources into a single directory source. v5.1-dev has two such splits, which #5307 and #5514 introduced:

• rust-bitcoin crates. rust-dashcore takes bitcoin-crypto from rust-bitcoin rev 7ba35c7c, because the crates.io release needs secp256k1 0.32, not 0.33. That rev also carries base58ck 0.5.0, bitcoin_hashes 1.2.0, bitcoin-internals 0.7.0 and bitcoin-units 0.5.0. base-sdk and rust-dashcore's dashcore-crypto take the same versions from crates.io.
• base-sdk revisions. refactor!: migrate Platform cryptography to rust-dashcore dev (aca02026) #5514 moved rust-dashcore to aca02026, which pins base-sdk d9e0d241. dpp kept its own dash-pkc at 33fcc59e, so dash-num, dash-pkc and dash-types 0.1.0-beta appear from two git revisions. [patch] can't merge those.

What was done?

• dpp now takes dash-pkc from dashcore-crypto's re-export (dashcore_crypto::dash_pkc) instead of pinning base-sdk itself. dashcore-crypto is a new workspace dependency at the same rust-dashcore rev as dashcore. This makes dpp use d9e0d241, the base-sdk revision rust-dashcore already uses, and every rust-dashcore bump now moves it too.
• Added a root [patch.crates-io] that points the four rust-bitcoin crates at the same rev as bitcoin-crypto. Each of the four is needed: the vendor probe was rerun one line at a time, and each line uncovered the next duplicate.
• Regenerated Cargo.lock. Nine packages are removed and nothing is added or changes version; dpp's own entry lists dashcore-crypto instead of dash-pkc. The removed packages are:
  • the crates.io copies of the four crates;
  • the 33fcc59e copies of dash-num, dash-pkc and dash-types;
  • bitcoin-internals 0.5.0 and hex-conservative 0.3.2, which only the crates.io bitcoin-units used.

What changes in compiled code:

• dpp's BLS code now builds against dash-pkc d9e0d241 instead of 33fcc59e. Between those revisions the pkc sources change only in these ways:

  • error enums derive thiserror with the same messages;
  • len() % 16 != 0 becomes !len().is_multiple_of(16);
  • serde no longer implies codec;
  • a compile_error! fires for bls on x86_64 without ADX unless portable is on.

  BLS signing and verification are untouched. dpp's bls-signatures always enables dashcore/bls, which turns on dash-pkc/portable, so that check passes. blst was already a single package built with portable, still at 0.3.17.

  The revision also raises base-sdk's MSRV to 1.89; the toolchain is 1.98. Because dpp's copy is now the same package as rust-dashcore's, it also gets std, ecdsa, eddsa and serde. None of those features gate BLS code.

• dash-pkc and dashcore-crypto now build against the git base58ck and bitcoin_hashes. The functions they use behave the same:

  • encode_unbounded, decode_check, sha256, sha256d and ripemd160 are unchanged;
  • the git rev fixes a crates.io encode_unbounded panic on some 90-byte payloads.

• dashcore::base58 re-exports base58ck, so it now has the git API. That API drops decode, the variant without the checksum. Platform's only call was a test in rs-sdk's dashpay/invitation.rs. The test checks a WIF's network byte, and it now uses decode_check, which returns the same payload with the checksum verified and stripped.

[patch] only applies at the workspace root, so crates that depend on Platform by git are unaffected. dashpay/rust-dashcore#1224 removed the base58ck split upstream. Once Platform bumps past it, the patch shrinks to bitcoin-internals alone; #5648 tracks that.

How Has This Been Tested?

Cargo 1.98.1 (the pinned toolchain), on v5.1-dev a317e4d:

• cargo vendor --locked "$(mktemp -d)": fails on v5.1-dev, succeeds on this branch (822 crates).
• cargo check --locked --workspace --all-targets and cargo clippy --locked --workspace --all-targets -- -D warnings: pass.
• cargo test --locked -p dpp --features bls-signatures --lib bls: 22 passed.
• cargo test --locked -p dpp --features bls-signatures --test bls_compatibility: 9 passed.
• cargo check --locked -p dpp --no-default-features: passes.
• cargo test --locked -p dash-sdk --lib should_round_trip_a_wif: passes.

Breaking Changes

None.

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
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · c7b0e20

• Bots — thepastaclaw ✓
• Self-review — post /self-reviewed
• Build green
• Approvals
  • files with no dedicated owner (Cargo.lock, Cargo.toml) — QuantumExplorer or shumkov
  • dpp (packages/rs-dpp/Cargo.toml, packages/rs-dpp/src/bls/bls_signatures.rs, packages/rs-dpp/tests/bls_compatibility.rs) — QuantumExplorer or shumkov
  • rust-sdk (packages/rs-sdk/src/platform/dashpay/invitation.rs) — lklimek or shumkov

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 25 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: build/vendorable-workspace-lockfile
• File: .coderabbit.yaml
• Commit: c7b0e20
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 231f4486-b0b9-4826-913d-ce82a430a3d8

Commits

Reviewing files that changed from the base of the PR and between 44b4c42 and c7b0e20.

Files ignored due to path filters: 1

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

Files selected for processing: 5

• Cargo.toml
• packages/rs-dpp/Cargo.toml
• packages/rs-dpp/src/bls/bls_signatures.rs
• packages/rs-dpp/tests/bls_compatibility.rs
• packages/rs-sdk/src/platform/dashpay/invitation.rs

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: build/vendorable-workspace-lockfile
• File: .coderabbit.yaml
• Commit: 44b4c42
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 832c2884-23ef-47bf-acd1-fd401ec4a07d

Commits

Reviewing files that changed from the base of the PR and between 01d2c8d and 44b4c42.

Files ignored due to path filters: 1

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

Files selected for processing: 4

• Cargo.toml
• packages/rs-dpp/Cargo.toml
• packages/rs-dpp/src/bls/bls_signatures.rs
• packages/rs-dpp/tests/bls_compatibility.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The workspace adds dashcore-crypto and pins four Bitcoin crate patches. rs-dpp uses the workspace dependency for BLS support and updates BLS imports. The invitation WIF round-trip test uses checksum-validating Base58 decoding.

Changes: BLS dependency alignment

Layer / File(s)	Summary
Configure BLS dependency sources Cargo.toml, packages/rs-dpp/Cargo.toml	The workspace adds dashcore-crypto and patches four Bitcoin crates to a pinned rust-bitcoin revision. rs-dpp replaces the optional dash-pkc dependency and enables BLS through dashcore-crypto.
Use re-exported BLS types packages/rs-dpp/src/bls/bls_signatures.rs, packages/rs-dpp/tests/bls_compatibility.rs	The BLS implementation and compatibility test use types from dashcore_crypto::dash_pkc::bls.

Invitation WIF test

Layer / File(s)	Summary
Validate WIF Base58 checksum packages/rs-sdk/src/platform/dashpay/invitation.rs	The WIF round-trip test uses decode_check before checking the network byte.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

• build: shrink the vendoring [patch.crates-io] on the next rust-dashcore bump #5648: This follow-up concerns the root Cargo patch entries added in this change and their reduction during a later rust-dashcore revision update.

Possibly related PRs

• dashpay/platform#5517: Updates workspace rust-dashcore dependencies, which can change the dependency sources aligned here.
• dashpay/platform#5639: Updates workspace rust-dashcore dependencies and lockfile source resolution.
• dashpay/platform#5471: Moves the invitation codec into rs-sdk and includes related WIF handling.

Suggested reviewers: lklimek, shumkov

Merge Risk: Minimal · previous review 44b4c

The dependency alignment and WIF test update are consistent with their intended contracts, with no established merge-blocking risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the primary change: restoring vendorable workspace lockfile support.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 3 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 2

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[github-actions]

Waiting for bot review — coderabbitai not yet · thepastaclaw ✓. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

✅ Final review complete — no blockers (commit c7b0e20) · triage: normal

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Verified the complete two-file diff at head 42a2204 against the supplied reviewer evidence and cached upstream sources; no actionable in-scope defects were found. The patches use the existing rust-bitcoin revision, and the lockfile removes six registry packages without adding packages or retaining duplicate name/version pairs. This was static verification only: the supplied CI snapshot shows Kotlin checks passing, Rust workspace and wallet tests skipped, and PR Hygiene pending; vendoring and Rust build/test results were not independently reproduced.

Review provenance

Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff is a small, contained build fix that unifies four dependency sources and removes duplicate lockfile entries without changing application logic.
• Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 71% left, 5h 100% left
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort medium); agent phase2-reviewer

[github-actions]

@coderabbitai review

No review for 42a22044 yet, so PR Hygiene is asking once. If nothing arrives, the requirement is dropped for this commit and the pull request is labelled bot-review-skipped.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The dependency-source unification and lockfile changes match the stated vendoring goal, and the updated Base58 test uses the available checked decoder correctly. However, patching the pinned Dashcore dependency removes dpp::dashcore::base58::decode through DPP’s public re-export, so the PR’s “Breaking Changes: None” declaration is inaccurate. Rust workspace tests and JS builds were still in progress, with no failed code checks reported.

🟡 2 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a cross-cutting dependency and build-resolution change with a small API call adjustment, but dependency and lockfile changes do not by themselves meet the critical-surface bar.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

🤖 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:85-88: Declare or preserve the removed public Base58 API
  These patches select the rust-bitcoin revision whose Base58 API no longer provides `decode`. Because `packages/rs-dpp/src/lib.rs:16` publicly re-exports `dashcore`, downstream users can observe this through `dpp::dashcore::base58::decode`, which will no longer compile. Updating the internal invitation test covers Platform’s only current caller but does not preserve downstream compatibility. Either retain a compatibility API in the owning rust-dashcore layer, or mark this release as breaking and document migration to `decode_check`; the current `Breaking Changes: None` declaration is inaccurate.
- [SUGGESTION] Cargo.toml:79-88: Add a CI guard for duplicate Cargo sources
  The PR’s primary goal is to keep `cargo vendor --locked` working, but the repository has no persistent check that detects a future same-name, same-version package resolving from multiple sources. A later dependency bump could silently reintroduce the split and break Dash Core’s offline bundle. Add a lightweight CI guard, such as running `cargo vendor --locked` into a temporary directory or checking the lockfile for duplicate `(name, version)` pairs with different sources.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The dependency consolidation at this exact head correctly removes same-name, same-version source splits and routes DPP's BLS code through the pinned rust-dashcore re-export. One in-scope regression-prevention suggestion remains: the repository has no lightweight guard against future duplicate Cargo sources; the Rust workspace test check is still in progress in the supplied CI snapshot, while no related failure is reported.

🟡 1 suggestion(s)

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

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a cross-cutting Rust dependency and lockfile change that alters the compiled cryptographic dependency revisions and a base58 test call, but it does not itself modify cryptographic algorithms, consensus rules, signatures, key handling, or other critical behavior.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

🤖 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:86-90: Add a CI guard for duplicate Cargo sources
  (existing thread: https://github.com/dashpay/platform/pull/5473#discussion_r4406228931)
  This PR fixes a vendoring failure that can be reintroduced by a later dependency update: two packages with the same name and version resolved from different sources cause `cargo vendor --locked` to fail. The existing Dash Core bundle check runs only when Dash Core moves its Platform pin, so it is a late and conditional signal rather than a repository-level regression check. Add a lightweight lockfile check for duplicate `(name, version)` pairs with distinct `source` values, or an appropriate focused vendoring validation, so dependency changes detect this invariant before the next downstream pin bump.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[PastaPastaPasta]

Approved