[PastaPastaPasta]

Issue being fixed or feature implemented

The vendored BLS library's native threshold sharing/recovery APIs read the first 32 identifier bytes without checking the declared length, and safe Rust recovery accepts arbitrary identifier vectors before calling a C API without per-identifier lengths. Check these contracts before the read. Dash Core uses fixed 32-byte identifiers, whose behavior is unchanged; a daemon attack path has not been established.

What was done?

• Reject identifiers shorter than 32 bytes in native sharing and recovery and all three safe Rust recovery methods.
• Preserve longer identifiers' existing first-32-byte interpretation, scalar reduction, and zero/duplicate recovery checks.
• Document the native/Rust identifier contract and the raw C caller's 32-readable-byte precondition without changing public C signatures.
• Add native and Rust tests for every sharing/recovery type and identifier position, legitimate 32/33-byte controls, and relationships between recovered keys and signatures.

How Has This Been Tested?

On Apple Silicon macOS, configured a fresh worktree from develop f35a24cfca126e3f910f2ffe85d2609b06bc9602 with the prebuilt depends prefix:

• make -C src/dashbls -j1; full native runtest: 1,455 assertions across 17 cases passed.
• Real Rust/C/native build and cargo test --manifest-path rust-bindings/bls-signatures/Cargo.toml -j1 -- --test-threads=1: all 24 Rust tests passed. Used a disposable copy of the current source, restoring only the matching vendored revision's ignored Relic CMake configuration template for build setup and capping the CMake build to one job.
• Removed the guards in separate checks: the native and Rust contract tests failed on expected rejection assertions. Restoring the guards passed. Short declared extents retained initialized backing of at least 32 bytes throughout these checks.
• Normal Dash Core make -j2 and test_dash --run_test=bls_tests passed.
• Targeted whitespace lint, git diff --check, and an independent four-pass source review passed.

Breaking Changes

Unsupported identifiers shorter than 32 bytes now return ordinary errors. Valid identifiers, including longer inputs interpreted through their first 32 bytes, retain existing behavior. Raw C callers must still supply readable buffers because that API has no per-identifier length parameter.

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone

This pull request was created by Codex.

[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 41 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 39f18538-9863-431c-9f62-004a7791e9f3

📥 Commits

Reviewing files that changed from the base of the PR and between f35a24c and 8886953.

📒 Files selected for processing (8)

• src/dashbls/include/dashbls/threshold.hpp
• src/dashbls/rust-bindings/bls-dash-sys/c-bindings/threshold.h
• src/dashbls/rust-bindings/bls-signatures/src/elements.rs
• src/dashbls/rust-bindings/bls-signatures/src/lib.rs
• src/dashbls/rust-bindings/bls-signatures/src/private_key.rs
• src/dashbls/rust-bindings/bls-signatures/src/utils.rs
• src/dashbls/src/test.cpp
• src/dashbls/src/threshold.cpp

• Autopilot · 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.}

[thepastaclaw]

✅ Final review complete — no blockers (commit 8886953) · triage: low

[thepastaclaw]

Final validation — Phase 1 + Phase 2

The native guards reject short identifiers before the fixed-size reads, and all three safe Rust recovery methods validate lengths before entering FFI. The changes preserve longer identifiers’ first-32-byte interpretation and existing Dash callers, with regression tests covering all affected types and recovery positions. This was a static review only: the supplied CI snapshot shows successful lint and multiple platform builds, while Linux tests and Windows, sanitizer, and multiprocess builds remain pending.

Review provenance

Source: reviewer 1: glm-5.3-flash (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: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff adds straightforward minimum-length guards to native and safe Rust BLS threshold APIs with focused contract tests, without changing cryptographic algorithms or valid-identifier behavior.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 98% left, weekly 96% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% 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 — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

[PastaPastaPasta]

NACK; dashbls is a vendor project; changes must go to dashpay/bls-signatures, not be made in repo here

[PastaPastaPasta]

Moved this library-only change to dashpay/bls-signatures#134, based on the exact develop revision currently vendored by Dash. All eight changed files match the reviewed patch byte-for-byte; the upstream native suite (1,455 assertions) and all 24 safe Rust tests pass, and the independent transfer review approved.

Closing this direct vendor-edit PR as superseded. Once the library change is accepted upstream, Dash should receive it through the normal vendor import. The upstream PR is now linked and watched here.

---

🤖 Posted autonomously by Codex on behalf of pasta.