chore(platform)!: update rust-dashcore (incl secp256k1 0.33) by lklimek · Pull Request #5307 · dashpay/platform

lklimek · 2026-10-06T12:13:00Z

Basic explanation

What this does: Pins rust-dashcore to 40e7b24c46100496a279f34952b851d40aa93a5f from rust-dashcore #1145, including backward-compatible reading of legacy BLS public keys. This is a port of #4932's migration to v5.1-dev.

Value: Gives #5150 a separately reviewable dependency base, so its diff focuses on wallet history and persistence repairs. #4932 and its existing dependent PRs remain unchanged.

Risks: This changes public Rust cryptography APIs and introduces a WebCrypto runtime requirement for WASM; the Node.js >=22 package requirements are being split into separate PR #5310. The merged base supplies the versioned Core v24 behavior. The SPV lifecycle and Node runtime requirements are addressed in stacked #5311 and #5310, which must land with this migration.

Issue being fixed or feature implemented

Port of #4932 from v5.0-dev to v5.1-dev, retaining its commit ancestry. Dependency base for #5150; no SQLite accounting migration, wallet history replay, or SwiftData accounting repair is included here.

What was done?

Port the secp256k1 0.33 API adaptations and WASM random-source configuration from chore(platform)!: bump rust-dashcore to 719de34b (secp256k1 0.33) #4932. Node.js package requirements and related README changes are excluded from this PR and will be raised directly to >=22 in a separate PR.
Pin all eight rust-dashcore workspace dependencies and all 13 upstream lockfile packages to 40e7b24c46100496a279f34952b851d40aa93a5f from rust-dashcore fix(wallet-lib): spending immature coinbase transactions #1145. Its newer dev base also updates transitive dependencies; Cargo.lock is regenerated.
Adapt Core crypto, SPV and spent-height query APIs. Use the shared BLS backend through dpp::bls, convert Ed25519 byte keys through the public operational-key API, preserve canonical node-ID bytes, and read owned masternode snapshots through the public SPV queries. The branch includes the SPV lifecycle fixes from fix(platform-wallet): retain SPV client and track shutdown through cancellation #5311 and fix(platform-wallet)!: tell a retryable SPV stop from one that needs a restart #5322.
Remove DPP's core_key_wallet_bip_38 and SDK's core_key_wallet_bip38 forwards because upstream removed BIP38 support. Adapt raw hash extraction and coinbase test fixtures to the newer upstream API.
Merge v5.1-dev at c301cb60a5 after chore: merge v5.0-dev into v5.1-dev #5308. Keep the Core v24 identity, payout, port-resolution and persistence implementations from feat(platform)!: support Core v24 masternode identities at protocol version 14 #5227/feat(drive-abci)!: resolve Core v24 platform ports at protocol version 14 #5228 unchanged relative to the base. Remove this PR's superseded strict masternode conversions and legacy projection adapters.
Retain zeroize 1.9.0 and the global blst patch at 71a00877. DPP's blsful -> blstrs_plus dependency requires exactly blst = 0.3.12; the crates.io release omits aggregate_verify and fast_aggregate_verify under WASM/no_std, which the new dash-pkc calls. The patch is one upstream commit after tag v0.3.12, changes only the Rust bindings, and keeps the package version at 0.3.12. It affects native builds too.

In-place changes to shipped generations

The existing generation methods receive mechanical secp256k1 API adaptations with unchanged digest construction, signature rules and canonical byte representations. Masternode processing, persistence and protocol-version tables now match the base branch; the historical/PV14 boundary comes from #5227/#5228. No additional consensus behavior change is intended by the local API adaptations.

choose_quorum/v0 (both selection helpers) and signature_verification_quorum_set/v0/quorums.rs keep their score bytes unchanged for all selecting protocol versions (1–14): the old From<Hash> for [u8; 32] and Hash::to_byte_array() both return the same inner array without reversal. Existing quorum-selection tests cover the adapted code.

Merge order

This branch pins the unmerged rust-dashcore #1145 head and depends on that compatibility fix. PlatformNodeId storage migration remains deferred.

Land this migration together with the Node runtime requirements in #5310 and SPV lifecycle adaptation in #5311. #5150 remains stacked above this PR, but must retain or restore dashpay/rust-dashcore#1112 separately: current dev is the parent of that unmerged spent-claim restoration commit, and #5150 uses its additional API. #5307 itself does not require #1112.

How Has This Been Tested?

Validation on 40e7b24c:

151 selected tests passed across platform-wallet, platform-wallet-storage and drive-abci: provider-key vectors, masternode lookup, SPV lifecycle, quorum selection, coinbase credit-pool decoding, SQLite persistence and buffer semantics.
Clippy passed for platform-wallet and platform-wallet-storage with --all-targets --locked --offline -- --no-deps -D warnings.
Clippy passed for dash-sdk and drive-abci with --all-targets --all-features --locked --offline -- --no-deps -D warnings.
Locked Cargo metadata, formatting, whitespace checks, and consistent pins across the eight direct dependencies and 13 lockfile packages passed.

The full workspace test suite, WASM/mobile builds and live-network tests were not rerun for this pin.

Outstanding review blockers

WASM Node runtime requirement: Node version declarations have been removed from this PR at the author's request. PR #5310 supplies the Node >=22 package requirements, documentation and matching Docker runtime; this dependency must be landed together with the migration.
The historical-storage test suggestion refers to this PR's superseded projection test, removed when adopting the base implementation. No frozen pre-migration byte fixture was added in this merge.

Breaking Changes

Public Core/secp256k1 Rust types and methods change; Platform consumers are adapted here. The migrated WASM entropy backend requires global WebCrypto; the Node.js >=22 minimum and related package/documentation changes are handled in a separate PR. The FFI ABI is unchanged. Legacy BLS public-key hex encodings can be read automatically through bincode Serde. Compatibility for old PlatformNodeId encodings and other historical record-layout changes remains unresolved; no database migration is included. DPP's core_key_wallet_bip_38 and SDK's core_key_wallet_bip38 features are removed because upstream removed BIP38.

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

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

PR Hygiene · `9af2033`

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.

Bump every rust-dashcore git dependency from e4208c90 to 719de34b (dashpay/rust-dashcore dev, the merge of #1049). The breaking change in range is rust-dashcore #1042: secp256k1 0.30 -> 0.33, rand 0.9, getrandom 0.4, and the secp256k1 context argument dropped API-wide. The range also carries key-wallet fixes #1035 (DIP-15 contact pools extend as payments arrive), #1009 (provider transactions consult every fund-bearing account), #1048, and test-only #920/#1038. Mechanical call-site migration: drop Secp256k1 contexts (derive_priv/from_priv/public_key/from_secret_key/Keypair::new take no context); secp.sign_ecdsa/recover_ecdsa/verify_ecdsa become methods on the key/signature; SecretKey::from_slice/from_byte_array become from_secret_bytes (slices go through <[u8; 32]>::try_from mapped to Error::InvalidSecretKey, the error from_slice returned); secret_bytes -> to_secret_bytes; SecretKey/SharedSecret lost AsRef<[u8]>, so use as_secret_bytes; thread_rng -> rng; StdRng::from_entropy -> from_os_rng. The secp256k1::hashes re-export is gone, so hex rendering uses the hex crate (same output). Seeded RNG bridging (rand 0.8 caller -> secp's rand 0.9 StdRng) now fills a 32-byte seed and calls from_seed, which is what rand_core 0.6 from_rng did, so seeded keys are unchanged; drive-abci tests that feed seed_from_u64 into Keypair::new use the secp re-exported StdRng (seed_from_u64 and ChaCha12 are identical across rand_core 0.6/0.9). rs-platform-encryption moves its own secp256k1 to 0.33.1 so only one secp256k1 is in the graph. simple-signer takes a direct rand 0.8 dep for the RngCore its callers pass. rs-dpp enables getrandom 0.3/0.4 wasm_js on wasm32-unknown-unknown, which the new transitive getrandom versions require there. Rust toolchain already at 1.98.1, matching rust-dashcore. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

infraclaw: install the reviewed publisher prerequisite only; no application or runner changes.

Conflicts resolved: - Cargo.toml: v5.0-dev's grovedb dce8252f, this branch's rust-dashcore 719de34b. - Cargo.lock: regenerated from v5.0-dev's lock; the only resolved change is secp256k1 0.30.0 -> 0.33.1 (sys 0.10.1 -> 0.14.1, bitcoin-io dropped), the same delta the bump produced before. - rs-platform-encryption tests: v5.0-dev's seeded StdRng, with secp 0.33's context-free generate_keypair. - rs-sdk put_document: v5.0-dev's restructured transition, with rand 0.9's from_os_rng / random. - runner-image-candidate.yml: v5.0-dev's version (controller 1be03edb); the feature-branch trigger is no longer needed now that this branch carries v5.0-dev's runner-image CI. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

secp256k1 0.33 draws randomness through rand 0.9 / getrandom 0.3, and key-wallet through getrandom 0.4. Their `wasm_js` backend reads only `globalThis.crypto.getRandomValues`; getrandom 0.2's `js` feature also fell back to Node's `require("crypto")`. Node.js exposes the WebCrypto global by default from v19, so on Node 18 even a deterministic call such as deriving a public key (secp256k1 rerandomizes its context afterwards) traps. Node 18 has been end-of-life since April 2025. Raise `engines.node` of wasm-sdk, wasm-dpp2 and js-evo-sdk to >=20, matching dashmate, and say why in the evo-sdk README and next to the getrandom features in rs-dpp. BREAKING CHANGE: @dashevo/wasm-sdk, @dashevo/wasm-dpp2 and @dashevo/evo-sdk require Node.js >= 20. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…o secp256k1 0.33 Code that landed on v5.0-dev after this bump was first written (encrypted_for in rs-sdk and wasm-sdk, moderation charter requests) still used the secp256k1 0.30 / rand 0.8 API. Migrate it the same way as the rest of the bump: drop the `Secp256k1` context from `PublicKey::from_secret_key`, `SecretKey::from_slice` -> `from_secret_bytes`, `StdRng::from_entropy` -> `from_os_rng`, and `Bytes32::random_with_rng` -> `Bytes32::new(rng.random())` since platform-value's helper takes a rand 0.8 `StdRng`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Merge PR #4932 into v5.1-dev and carry the compatibility adaptations for rust-dashcore PR #1112 at 870af146. Preserve the existing cryptography implementations, legacy RPC projection, and FFI ABI. Keep SQLite replay and accounting repairs, SwiftData accounting changes, and wallet accounting changes in the dependent PR #5150. 🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent

coderabbitai · 2026-10-06T12:13:12Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: dashpay/platform/.coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: 1f94efba-8c0e-4a15-84c8-4ea98eba3ebd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

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

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

thepastaclaw · 2026-10-06T12:19:46Z

⛔ Final review complete — 1 blocking finding(s) (commit 9af2033) · triage: critical

thepastaclaw

Final validation — Phase 1 + Phase 2

Verified the supplied findings against head ad2b8c2, including the exact pinned upstream SPV implementation. Two migration regressions remain: successful SPV startup discards the runtime's client, and legacy WASM-DPP inherits an undeclared WebCrypto runtime requirement; the overlapping historical-storage test suggestions are consolidated below. Verification was static only: the supplied CI snapshot shows Kotlin build/tests passing, principal Rust and JS suites skipped, and PR Hygiene pending.

🔴 2 blocking | 🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

Triage: critical by gpt-6.1-sol (effort low) — The large cross-cutting diff directly adapts signature and key-handling implementations in packages/rs-dpp/src/identity/identity_public_key/key_type.rs and packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs, alongside historical masternode processing and serialization, rather than merely bumping dependencies.
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 below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); 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`:
- [BLOCKING] Cargo.toml:70: Adapt SpvRuntime to the newly non-blocking SPV run method
  The pinned revision changes DashSpvClient::run() from waiting for shutdown to spawning an internal sync task and returning after startup succeeds. packages/rs-platform-wallet/src/spv/runtime.rs:374–381 still treats that return as shutdown: it removes self.client and clears the peer tracker. Successful startup therefore makes broadcasts fail with "client not started", progress queries return None, and quorum lookups lose access to the running client. SpvRuntime::stop() subsequently finds no client and joins only the completed startup wrapper, so it reports success without stopping the upstream task. That task retains a client clone, storage and event handlers, allowing sync and callbacks to continue after the runtime reports shutdown. Retain the client after successful startup and adapt shutdown tracking to the new upstream task ownership, including the bounded-stop behavior. Add regression coverage for successful startup followed by client queries and explicit stop.

In `packages/rs-dpp/Cargo.toml`:
- [BLOCKING] packages/rs-dpp/Cargo.toml:101-103: Propagate the new Node runtime requirement to legacy WASM-DPP
  The new entropy requirement also reaches @dashevo/wasm-dpp through its DPP dependency. Its ECDSA signByPrivateKey path calls the pinned Core signer, whose secp256k1 0.33 implementation rerandomizes the signing context using rand 0.9 and getrandom 0.3. That backend calls globalThis.crypto.getRandomValues without getrandom 0.2's CommonJS crypto fallback, so ordinary Node 18 scripts without a WebCrypto shim now fail when signing. packages/wasm-dpp/package.json declares no Node minimum, its production loader in lib/index.ts installs no shim, and the legacy dash SDK still documents support for any Node version. The test bootstrap explicitly installs crypto.webcrypto, masking this production failure. Apply the declared Node >=20 migration to the legacy package and update its consumers' advertised requirement, or initialize WebCrypto in the production Node loader to preserve previous support. Cover the production loader without relying on the test bootstrap's shim.

In `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs:546-550: Pin pre-migration bytes in the historical storage compatibility test
  Both expected_bytes here and the actual bytes at lines 570–574 are generated with the migrated MasternodeV0 codec and current dependencies. This verifies that extended RPC records project to the same representation as legacy-shaped inputs, but it does not establish compatibility with previously persisted bytes: a shared encoding change would affect both sides and still pass. Preserving the shipped persisted layout is an explicit goal of this migration. Add a fixed fixture produced at base revision 723b09e63610057377ef4efc808476fae43e88ef, deserialize it through deserialize_masternode_entry, and reproduce it byte-for-byte through serialize_masternode_entry for a frozen historical protocol and PlatformVersion::latest(). Those helpers also exercise the versioned Masternode enum used by persistence, which this direct MasternodeV0 encoding test bypasses. No actual persisted-encoding incompatibility was established by this review.

Keep the Core v24 masternode handling and persistence from #5227/#5228, replacing the migration's superseded strict legacy conversions. Retain rust-dashcore #1112, the WASM blst patch, and existing rand versions. Validated formatting, scoped all-target/all-feature Clippy, 90 Core-update tests, 3 masternode persistence tests and the DPP BLS protocol-boundary test. Co-Authored-By: Codex <noreply@openai.com> 🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent

Declare the minimum Node version in wasm-dpp and dash and align their README requirements with the existing WASM SDK packages. Validated the production WASM-DPP loader and ECDSA signing on Node 20.20.2 without the test bootstrap's WebCrypto shim. Co-Authored-By: Codex <noreply@openai.com> 🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent

Restore package metadata and documentation from v5.1-dev so the Node minimum can be raised directly to 22 in a separately reviewable PR. The WASM entropy backend remains part of the rust-dashcore migration. Co-Authored-By: Codex <noreply@openai.com> 🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent

Use 314f1060 across all workspace dependencies and lockfile sources. This merged dev revision excludes the separate unmerged spent-claim restoration API needed by PR #5150. Co-Authored-By: Codex <noreply@openai.com> 🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent

github-actions · 2026-10-06T15:08:31Z

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 2 threads unresolved — resolve them. 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.

lklimek

/self-reviewed

thepastaclaw

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

At d9149f1, two integration blockers remain: the dependency update changes SPV lifecycle ownership and introduces a WebCrypto requirement without the corresponding runtime adaptations. The historical-storage suggestion is outdated because its targeted projection test was removed and the retained persistence code matches the updated base. Validation was static only; the supplied CI snapshot still had Rust workspace, JS, Docker, and Swift schema checks queued, with dedicated wallet tests skipped.

🔴 2 blocking

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🔴 Blocking: Adapt SpvRuntime to the newly non-blocking SPV run method
packages/rs-platform-wallet/src/spv/runtime.rs:379-381

At the pinned 314f1060 revision, DashSpvClient::run_locked starts an internally owned task, stores its handle and cancellation token in sync_loop, and returns Ok after startup. This unconditional cleanup therefore removes a successfully running client and clears its peer tracker. Broadcasts and quorum queries subsequently reject the missing client, progress queries return None, and stop() can report success after joining only the completed startup wrapper while upstream synchronization and event monitors remain active.

Shutdown also needs an ownership adaptation, not just client retention. The pinned upstream stop() takes SyncLoop out of its shared Option before awaiting the task, coordinator, network, and storage teardown. The direct timeout around c.stop() at runtime.rs:417 drops that future on expiry, abandoning the remaining teardown; retrying cannot recover it through the runtime's startup-wrapper handle. Retain the client after successful startup and own teardown in a tracked task that survives timeout and caller cancellation, with retries joining the same work and restart blocked until completion. The separately described #5311 fix is absent from this exact head and must land before or together with the migration.

_{source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general)}

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

Triage: critical by gpt-6.1-sol (effort low) — This large cross-cutting migration directly changes cryptographic key generation in packages/rs-dpp/src/identity/identity_public_key/key_type.rs and BLS key validation, private-key serialization, and canonical node-ID derivation in packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs, beyond merely bumping dependencies.
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 below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); 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 `packages/rs-platform-wallet/src/spv/runtime.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/spv/runtime.rs:379-381: Adapt SpvRuntime to the newly non-blocking SPV run method
  At the pinned 314f1060 revision, DashSpvClient::run_locked starts an internally owned task, stores its handle and cancellation token in sync_loop, and returns Ok after startup. This unconditional cleanup therefore removes a successfully running client and clears its peer tracker. Broadcasts and quorum queries subsequently reject the missing client, progress queries return None, and stop() can report success after joining only the completed startup wrapper while upstream synchronization and event monitors remain active.

  Shutdown also needs an ownership adaptation, not just client retention. The pinned upstream stop() takes SyncLoop out of its shared Option before awaiting the task, coordinator, network, and storage teardown. The direct timeout around c.stop() at runtime.rs:417 drops that future on expiry, abandoning the remaining teardown; retrying cannot recover it through the runtime's startup-wrapper handle. Retain the client after successful startup and own teardown in a tracked task that survives timeout and caller cancellation, with retries joining the same work and restart blocked until completion. The separately described #5311 fix is absent from this exact head and must land before or together with the migration.

In `packages/rs-dpp/Cargo.toml`:
- [BLOCKING] packages/rs-dpp/Cargo.toml:102-103: Propagate the new Node runtime requirement to legacy WASM-DPP
  (existing thread: https://github.com/dashpay/platform/pull/5307#discussion_r4195848422)
  Legacy WASM-DPP's ECDSA signByPrivateKey path reaches DPP's signer::sign and the pinned Core recoverable signer. DPP enables secp256k1's rand feature, and secp256k1 0.33.1 rerandomizes its signing context through rand::random(). The getrandom 0.3 wasm_js backend enabled here calls globalThis.crypto.getRandomValues without the previous CommonJS crypto fallback. Node 18 scripts without global WebCrypto therefore fail during signing.

  This head still declares no Node minimum in packages/wasm-dpp/package.json, installs no WebCrypto shim in its production loader, and advertises unrestricted Node support for the legacy SDK. Its test bootstrap explicitly supplies crypto.webcrypto, masking that production condition. The reported Node 22 smoke test validates the proposed supported environment but does not update the compatibility contract shipped here. Land #5310's package requirements, consumer documentation, and runtime alignment before or together with this migration, or preserve older environments through a production-loader shim. Keep production-loader coverage independent of the test bootstrap.

lklimek · 2026-10-06T15:45:24Z

Both @thepastaclaw blockers addressed in separate PRs #5310 and #5311

…5310)

github-actions · 2026-10-06T17:28:42Z

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-10-08T13:25:28.102Z

github-actions · 2026-10-06T17:37:36Z

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 3 threads unresolved — resolve them. 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

At c0b4b97, two blockers are confirmed: the SPV lifecycle wrapper still treats successful startup as termination, and the root-only BLS patch does not protect external Cargo/WASM consumers. The Node runtime finding is fixed, and the historical-storage test finding no longer applies because the criticized test was removed. Verification was static; the supplied CI snapshot has Rust workspace and several JS/E2E checks pending, with the dedicated Rust wallet job skipped.

🔴 2 blocking

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

Triage: critical by gpt-6.1-sol (effort low) — The cross-cutting migration directly changes cryptographic and key-handling code, including RNG seeding in rs-dpp/src/identity/identity_public_key/key_type.rs and BLS key validation and node-ID construction in rs-platform-wallet/src/wallet/provider_key_at_index.rs, rather than merely bumping dependencies.
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 below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); 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`:
- [BLOCKING] Cargo.toml:78-80: Make the WASM BLS compatibility fix reach external Cargo consumers
  Cargo ignores a dependency workspace's [patch] table and does not inherit its lockfile. Consequently, an external workspace consuming dash-sdk or drive-proof-verifier from this Git revision does not receive this compatibility fix. Both reach DPP's bls-signatures feature, which enables blsful → blstrs_plus and dashcore/bls → dashcore-crypto → dash-pkc. blstrs_plus requires blst exactly 0.3.12, and blst's links constraint prevents selecting a second version for dash-pkc. In registry blst 0.3.12, aggregate_verify and fast_aggregate_verify are gated behind std, which its build script excludes on wasm32-unknown-unknown; the pinned dash-pkc calls both unconditionally in pkgs/pkc/src/bls/scheme_ietf.rs:157 and :176. Thus the workspace patch allows in-tree WASM builds while external consumers still encounter missing-method compilation errors. Make aggregate verification compatible in the owning dash-pkc implementation, or encode a compatible dependency in the actual dependency chain, then update the pins while preserving DPP's historical behavior. Moving the patch into another dependency's workspace would not fix external consumption.
- [BLOCKING] Cargo.toml:70: Adapt SpvRuntime to the newly non-blocking SPV run method
  (existing thread: https://github.com/dashpay/platform/pull/5307#discussion_r4195848409)
  At the pinned 314f1060 revision, DashSpvClient::run_locked() spawns the continuous sync task, stores its SyncLoop internally, and returns Ok after startup. SpvRuntime::run() still unconditionally removes self.client and clears the peer tracker at runtime.rs:379–381 when that call returns. Successful startup therefore disables broadcasts, progress queries, and quorum lookups; stop() then finds no client and reports success after joining only the completed startup wrapper, while upstream synchronization and callbacks can remain active. Shutdown also needs cancellation-safe ownership: upstream stop() takes the internal SyncLoop before awaiting its task, coordinator, network, and storage cleanup, but the current runtime drops that future on timeout. Retaining the client alone would leave teardown completion untracked after timeout or caller cancellation. Incorporate #5311's lifecycle adaptation with this pin, preserving a tracked teardown task and restart barrier until cleanup completes, and cover post-start queries, explicit shutdown, and timeout/cancellation retries.

github-actions · 2026-10-07T08:06:47Z

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 2 threads unresolved — resolve them. 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

Verified the findings against head 938b60f and the exact pinned upstream sources; two blockers remain after consolidating the duplicate SPV reports. The Node runtime requirement is satisfied, and the historical projection-test suggestion no longer applies. This was static verification only: the supplied CI snapshot shows successful JS/Docker builds and Kotlin checks, with Rust workspace tests running and JS/runtime suites queued.

🔴 2 blocking

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

Triage: critical by gpt-6.1-sol (effort low) — This cross-cutting migration directly changes cryptographic key generation and handling, including RNG seeding in packages/rs-dpp/src/identity/identity_public_key/key_type.rs and provider key derivation in packages/rs-platform-wallet/src/wallet/provider_key_at_index.rs, rather than merely updating dependency pins.
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 below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); 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`:
- [BLOCKING] Cargo.toml:78-80: Make the WASM BLS compatibility fix reach external Cargo consumers
  (existing thread: https://github.com/dashpay/platform/pull/5307#discussion_r4198966217)
  This patch makes repository builds work, but Cargo does not inherit a dependency workspace's root [patch.crates-io] table. An external wasm32-unknown-unknown consumer enabling DPP's bls-signatures feature brings in blsful → blstrs_plus, whose manifest requires exactly blst =0.3.12. The migrated dashcore BLS feature also enables dash-pkc at e6402ced, whose scheme_ietf.rs calls fast_aggregate_verify and aggregate_verify unconditionally. Registry blst 0.3.12 gates both methods behind std, and its build script deliberately does not enable that configuration for this target. Consequently, the external dependency graph lacks the methods required by the migrated implementation, even though the workspace's patched builds pass.

  The reported registry-versus-patched reproduction demonstrates why the workaround is necessary, not that it reaches downstream Cargo roots. Make compatibility effective through the normal dependency chain, or explicitly document the required consumer-root patch and its exact revision in the supported integration instructions. Validate that configuration from a separate Cargo root. Existing prebuilt npm artifacts are not affected by this dependency-resolution issue.
- [BLOCKING] Cargo.toml:70: Adapt SpvRuntime to the newly non-blocking SPV run method
  (existing thread: https://github.com/dashpay/platform/pull/5307#discussion_r4195848409)
  At this pin, DashSpvClient::run_locked spawns the synchronization task, stores its handle internally, and returns Ok(()) after startup. SpvRuntime::run still treats that return as termination: runtime.rs:379–381 removes self.client and clears the peer tracker. Successful startup therefore removes access to broadcasts, progress and quorum queries while upstream synchronization continues. A subsequent stop finds no client and joins only the completed startup wrapper; PlatformWalletManager::shutdown then records WorkerStatus::Ok without stopping the upstream task. That task retains the client and event handlers, so synchronization and callbacks can remain active after reported shutdown.

  Client retention alone is insufficient: runtime.rs:417 directly times out c.stop(), while the pinned upstream stop takes its internal SyncLoop before awaiting teardown. Cancelling that future can discard shutdown ownership, and Platform's outer startup handle cannot establish that the internal task and remaining cleanup completed. Incorporate the #5311 lifecycle adaptation with this migration: retain the client after successful startup, preserve a tracked teardown task across timeout and caller cancellation, let retries join the same work, and prevent restart until teardown completes. The separately reported #5311 tests do not validate this head, whose runtime has no corresponding lifecycle change.

…ncellation (#5311)

github-actions · 2026-10-07T09:36:52Z

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 2 threads unresolved — resolve them. 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 inspected cryptographic adaptations preserve signing preimages and canonical byte handling, and the incorporated Node requirements and SPV teardown changes resolve the earlier lifecycle blockers. Two blocking defects remain: external Cargo WASM consumers do not inherit the BLS compatibility patch, and clearing a constructed-but-not-running SPV client can leave its persistence worker writing to the cleared directory. This was a static review at the exact head; the supplied CI snapshot shows Kotlin checks passing, with Rust workspace validation, JS builds, Swift schema validation, and Docker builds still pending.

🔴 2 blocking | 🟡 2 suggestion(s)

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

Triage: critical by gpt-6.1-sol (effort low) — The large, cross-cutting diff directly adapts cryptographic and key-handling implementations in packages/rs-dpp/src/identity/identity_public_key/key_type.rs, packages/rs-platform-encryption/src/ecdh.rs, and wallet signing and key-derivation code, beyond merely bumping dependencies.
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 below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
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 xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); 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 `packages/rs-platform-wallet/src/spv/runtime.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/spv/runtime.rs:620-623: Stop the constructor's persistence worker before clearing storage
  A supported start(config) followed by clear_storage() without spawning the run loop leaves the original storage worker alive. At the pinned rust-dashcore revision 314f1060, DiskStorageManager::new() immediately starts a five-second persistence worker, but DashSpvClient::stop() stops storage only when sync_loop contains a running loop. This closure consequently returns without stopping that worker, and stop_locked drops the client before clear_storage_with reopens the same directory. DiskStorageManager has no Drop cleanup for its worker handle; the detached task retains the old storage Arcs while the directory lock is released. Its persistence loop continues creating storage subdirectories and can write still-dirty header segments into the cleared or restarted store. The previous live-client clear path cleared through the existing storage manager, which explicitly cancelled its worker. Stop the client's storage explicitly within the owned cleanup before dropping it, including cleanup after failed startup, and add a constructor-only clear regression that verifies the old worker cannot write after clearing.
- [SUGGESTION] packages/rs-platform-wallet/src/spv/runtime.rs:299-304: Separate retained client ownership from background running state
  Retaining the client is necessary for safe teardown, but its presence no longer proves that synchronization is running. The pinned upstream run() returns after startup; a later sync-loop failure invokes on_error and schedules stop_failed(), without returning another error to this wrapper. finish_startup therefore leaves self.client populated after the upstream client has stopped. SpvRuntime::is_started() tests only that Option, and platform_wallet_manager_spv_is_running exposes the result as whether SPV is currently running. PlatformEventManager forwards the error without updating lifecycle state, so that query can remain true indefinitely after background synchronization terminates. Keep resource ownership separate from running status, using the upstream is_running() query or an explicit lifecycle signal, and cover successful startup followed by a background failure.
- [SUGGESTION] packages/rs-platform-wallet/src/spv/runtime.rs:999-1000: Add a regression that runs the real SPV startup wrapper
  This is useful unit coverage for finish_startup, but it passes Ok(()) directly and installs an unrelated empty Tokio task. It never executes SpvRuntime::run() or creates the upstream SyncLoop, so it cannot detect a regression in the production startup-task wiring, and its stop assertion does not exercise running-client teardown. Add a separate regression that calls spawn_run_loop(), waits for the actual startup task to finish, then checks retained query access and explicit stop. The existing offline configuration restricts connections to an empty configured peer list; the pinned network manager supports that configuration without DNS discovery or peer connections, so this can exercise real startup without network-dependent testing.

In `Cargo.toml`:
- [BLOCKING] Cargo.toml:78-80: Make the WASM BLS compatibility fix reach external Cargo consumers
  (existing thread: https://github.com/dashpay/platform/pull/5307#discussion_r4198966217)
  Cargo applies this patch only when Platform is the consuming workspace root; an independent application depending on DPP or dash-sdk does not inherit it or Platform's lockfile. The ordinary dependency graph still combines blsful → blstrs_plus 0.8.18, which requires exactly registry blst 0.3.12, with dashcore → dashcore-crypto → dash-pkc. At the pinned base-sdk revision e6402ced, dash-pkc's scheme_ietf.rs unconditionally calls fast_aggregate_verify and aggregate_verify. Registry blst 0.3.12 gates both methods behind std, and its build script excludes that configuration on wasm32-unknown-unknown. External WASM consumers enabling BLS therefore encounter missing-method compilation errors, even though repository builds use the compatible patch. The reported patched reproducer establishes the implementation fix, but not its delivery to downstream consumers. Make the repair available through ordinary transitive dependency declarations, or explicitly document and support the required consumer-root override, and validate a standalone WASM consumer outside this workspace. This affects Rust-to-WASM consumers, not users of the already-built npm artifacts.

Co-authored-by: Codex <noreply@openai.com>

github-actions · 2026-10-07T15:37:29Z

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 1 thread unresolved — resolve it. 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 current head incorporates the Node-runtime, retained-client shutdown, and external-consumer BLS fixes, but one blocking SPV storage-worker defect and two lifecycle/testing suggestions remain. The storage-clear path can reopen the directory while its original persistence worker is still alive. Validation was static only; the supplied CI snapshot shows successful JS builds and Kotlin checks, with Rust workspace tests, Docker builds, and most JS checks pending and Rust wallet tests skipped.

🔴 1 blocking | 🟡 2 suggestion(s)

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

Triage: critical by gpt-6.1-sol (effort low) — The large cross-cutting diff directly changes cryptographic key parsing, signature handling and canonical encodings in packages/rs-dpp/src/bls/bls_signatures.rs and packages/rs-dpp/src/bls/serde.rs, rather than merely updating dependency pins.
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 below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); 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 `packages/rs-platform-wallet/src/spv/runtime.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/spv/runtime.rs:620-623: Stop the constructor's persistence worker before clearing storage
  (existing thread: https://github.com/dashpay/platform/pull/5307#discussion_r4205797515)
  This cleanup stops only the upstream client before dropping it and reopening its storage directory. At the exact rust-dashcore pin, DiskStorageManager::new() immediately starts a five-second persistence task, but DashSpvClient::stop() reaches storage.stop() only when a SyncLoop exists. Therefore start(config) followed by clear_storage() without spawn_run_loop() releases the original directory lock without stopping its writer. The detached task retains the old storage components and path; persistence recreates directories and writes dirty segments, so clearing or restarting can race with stale writes. This PR replaces the base's live-client clear path, which cleared through the existing storage manager and cancelled that worker. Explicitly stop the original storage within owned teardown before releasing or reopening the directory, and ensure construction/startup failure cleanup cannot detach the same worker. Add a constructor-only clear regression that checks for writes after multiple persistence periods. The cleanup described in your #5322 reply is not present here: runtime.rs is unchanged since 7c73e983.
- [SUGGESTION] packages/rs-platform-wallet/src/spv/runtime.rs:186-188: Separate retained client ownership from background running state
  (existing thread: https://github.com/dashpay/platform/pull/5307#discussion_r4205797527)
  is_started() measures retained client ownership, but platform_wallet_manager_spv_is_running exposes it as synchronization liveness. The pinned upstream run() returns after startup; a later background failure cancels its running token, reports on_error, and schedules stop_failed() without returning through this runtime's completed startup wrapper. PlatformEventManager only forwards the error, so the retained client remains populated and the public running query can stay true after synchronization stops. Keep the client for teardown, but expose a separate running query backed by upstream is_running() and route the FFI export through it. Your real channel-failure regression tests the appropriate boundary, but neither that regression nor the separate query from #5322 appears in this head.
- [SUGGESTION] packages/rs-platform-wallet/src/spv/runtime.rs:999-1000: Add a regression that runs the real SPV startup wrapper
  (existing thread: https://github.com/dashpay/platform/pull/5307#discussion_r4205797537)
  This regression calls finish_startup(Ok(())) directly and installs an unrelated empty Tokio task. offline_runtime() constructs the client but never starts its upstream SyncLoop, so the assertions test constructor-level query access and helper behavior rather than production startup wiring or running-client teardown. The other spawn_run_loop() calls in this module exercise teardown refusal or an unstarted runtime, not successful startup. Add a separate offline regression that calls spawn_run_loop(), waits for its actual startup task, checks retained query access, explicitly stops the running client, and verifies restart. The existing empty restricted-peer configuration supports this without DNS discovery or peer connections. The production-wrapper regression linked in your #5322 reply has not been incorporated into the reviewed tree.

…a restart (#5322) Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Codex GPT-6 <noreply@openai.com>

github-actions · 2026-10-08T11:28:05Z

Waiting for bot review — coderabbitai not yet · thepastaclaw ✓, 1 thread unresolved — resolve it. 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

At the exact reviewed head, seven prior findings are fixed and the historical masternode-test finding is outdated because its target was removed. One in-scope SQLite format-versioning suggestion remains; the proposed clear-error worker leak is refuted by the pinned upstream implementation, and no blocking defect was verified. This was static verification only: the supplied CI snapshot still had Rust workspace and several integration suites pending, with the dedicated Rust wallet job skipped.

🟡 1 suggestion(s)

Review provenance

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

Triage: critical by gpt-6.1-sol (effort low) — The large cross-cutting diff directly changes cryptographic and signature handling in packages/rs-dpp/src/bls/bls_signatures.rs, packages/rs-dpp/src/bls/serde.rs and packages/rs-dpp/src/signing.rs, exceeding a dependency-only migration.
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 below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
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 xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); 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:68-71: Represent the Core transaction codec change in the SQLite schema
  This pin changes the binary Serde representation of Core BLS public keys: the base dependency serializes BLSPublicKey as a 96-character hex string even in binary formats, while the new BlsPkBytes alias serializes 48 raw bytes. That reaches SQLite because core_state.rs writes the complete key_wallet::TransactionRecord, including provider transaction payloads, through bincode::serde into core_transactions.record_blob. The SQLite migrations remain unchanged through V018, and load_state directly decodes those blobs using the new upstream type. An affected database therefore passes the storage compatibility gate but fails when its transaction records are loaded. The documented requirement to recreate development databases makes the incompatibility intentional, but does not distinguish the two storage formats at open or restore time. Preserve the old codec through a migration, or introduce an explicit incompatible storage epoch with an actionable rejection explaining the recreation requirement. Add a frozen base-produced provider-transaction blob to verify the selected policy. This is a SQLite format boundary, not a PlatformVersion or accounting migration.

thepastaclaw · 2026-10-08T12:22:36Z

+dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "314f10600e35311f3d875a6e8740476ab6c79c0c" }
+dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "314f10600e35311f3d875a6e8740476ab6c79c0c" }
+dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "314f10600e35311f3d875a6e8740476ab6c79c0c" }
+key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "314f10600e35311f3d875a6e8740476ab6c79c0c" }


🟡 Suggestion: Represent the Core transaction codec change in the SQLite schema

This pin changes the binary Serde representation of Core BLS public keys: the base dependency serializes BLSPublicKey as a 96-character hex string even in binary formats, while the new BlsPkBytes alias serializes 48 raw bytes. That reaches SQLite because core_state.rs writes the complete key_wallet::TransactionRecord, including provider transaction payloads, through bincode::serde into core_transactions.record_blob. The SQLite migrations remain unchanged through V018, and load_state directly decodes those blobs using the new upstream type. An affected database therefore passes the storage compatibility gate but fails when its transaction records are loaded. The documented requirement to recreate development databases makes the incompatibility intentional, but does not distinguish the two storage formats at open or restore time. Preserve the old codec through a migration, or introduce an explicit incompatible storage epoch with an actionable rejection explaining the recreation requirement. Add a frozen base-produced provider-transaction blob to verify the selected policy. This is a SQLite format boundary, not a PlatformVersion or accounting migration.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)}

Pin all eight workspace dependencies and Cargo.lock to 40e7b24c from dashpay/rust-dashcore#1145. Adapt public Ed25519 and masternode snapshot APIs, raw hash extraction, and coinbase test fixtures to its newer base. Quorum scores retain identical raw digest bytes at every protocol version; the removed Into conversion and to_byte_array both return the inner array. PlatformNodeId storage migration remains outside this change. Validation: 151 targeted tests; scoped Clippy for wallet, storage, SDK and Drive ABCI; formatting, locked metadata and consistent dependency pins. BREAKING CHANGE: Remove core_key_wallet_bip_38 and core_key_wallet_bip38 feature forwards because upstream key-wallet removed BIP38 support. 🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent Co-Authored-By: Codex GPT-6 <noreply@openai.com>

github-actions · 2026-10-08T13:25:41Z

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 1 thread unresolved — resolve it. 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

Static verification at 9af2033 confirms one blocking native FFI compilation defect and one remaining SQLite codec-compatibility suggestion. The prior SPV lifecycle, external BLS dependency, and Node runtime findings are addressed; the removed historical projection test makes its specific finding outdated. No local builds or tests were run: exact-head Android CI confirms the compilation failure, the supplied snapshot shows the principal Rust/JS/Swift checks skipped after package detection failed, and PR Hygiene remained pending.

🔴 1 blocking | 🟡 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: 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: ffi-engineer); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

Triage: critical by gpt-6.1-sol (effort low) — The large, cross-cutting diff directly changes cryptographic implementations and key serialization in packages/rs-dpp/src/bls/bls_signatures.rs and packages/rs-dpp/src/bls/serde.rs, alongside signature and key-handling adaptations across the SDK and consensus execution paths.
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 below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); 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 `packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs`:
- [BLOCKING] packages/rs-platform-wallet-ffi/src/provider_key_at_index.rs:389: Convert the FFI node ID through EddsaPkHash
  This helper does not compile against the pinned dependency. EddsaPkBytes supplies hash() through dash-types' Hashable trait, which is not imported here; the exact-head Android job 113334723053 in run 37784185743 reports E0599 at this statement. Importing that trait alone would still return the backend hash type rather than dashcore's wrapper that supplies to_canonical_bytes(). The public EddsaPkHash::from(EddsaPkBytes) conversion performs the required wrapping, as the adapted wallet accessors and masternode locator already demonstrate. This module and platform-wallet-ffi are unconditional dependencies of both unified native bridge crates, so the failure prevents their libraries from being produced. Use the wrapper conversion to restore compilation and preserve the canonical 20-byte Tenderdash node ID.

In `Cargo.toml`:
- [SUGGESTION] Cargo.toml:68-71: Represent the Core transaction codec change in the SQLite schema
  (existing thread: https://github.com/dashpay/platform/pull/5307#discussion_r4218839067)
  The dependency update changes persisted transaction representations while SQLite retains the same schema identity. At base pin 40268cc0, CoinbasePayload has seven derived-Serde fields; pin 40e7b24c appends merkle_root_asset_unlocks. The new version-conditional native bincode implementation does not protect sqlite/schema/blob.rs, which uses bincode::serde. In an old coinbase TransactionRecord, the new deserializer consequently consumes the following txid's byte-length prefix as the added Option discriminator and fails; core_state.rs:944 propagates that failure out of wallet-state loading. The node-ID adaptation also changes binary Serde from canonical PlatformNodeId bytes to EddsaPkHash's reversed internal bytes, allowing old provider-transaction records to decode with reversed canonical IDs. Neither the blob codec nor migrations through V018 distinguish these generations. The documented accounting/history deferral explains why no migration was added, but these failures are introduced by the dependency pin and can be addressed without that broader redesign. Preserve the legacy persistence representation or establish an explicit schema-selected compatibility boundary, and cover it with frozen pre-upgrade transaction-record blobs.

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

Centralize the constructor-started persistence worker lifetime in dash-spv — Outside the required Platform adaptation: SpvRuntime explicitly stops the constructor worker before fallible client construction, and the pinned upstream starts persistence only after successful synchronization and network startup. Changing the ownership contract for other direct upstream consumers would expand this review beyond the Platform paths being migrated.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

thepastaclaw · 2026-10-08T14:17:36Z

    let mut pk32 = [0u8; 32];
    pk32.copy_from_slice(unsafe { std::slice::from_raw_parts(pubkey_ptr, 32) });
-    let node_id = dashcore::PlatformNodeId::from_ed25519_public_key(&pk32).to_byte_array();
+    let node_id = EddsaPkBytes::from_bytes(pk32).hash().to_canonical_bytes();


🔴 Blocking: Convert the FFI node ID through EddsaPkHash

This helper does not compile against the pinned dependency. EddsaPkBytes supplies hash() through dash-types' Hashable trait, which is not imported here; the exact-head Android job 113334723053 in run 37784185743 reports E0599 at this statement. Importing that trait alone would still return the backend hash type rather than dashcore's wrapper that supplies to_canonical_bytes(). The public EddsaPkHash::from(EddsaPkBytes) conversion performs the required wrapping, as the adapted wallet accessors and masternode locator already demonstrate. This module and platform-wallet-ffi are unconditional dependencies of both unified native bridge crates, so the failure prevents their libraries from being produced. Use the wrapper conversion to restore compilation and preserve the canonical 20-byte Tenderdash node ID.

Suggested change

let node_id = EddsaPkBytes::from_bytes(pk32).hash().to_canonical_bytes();

let node_id = dashcore::eddsa::EddsaPkHash::from(EddsaPkBytes::from_bytes(pk32))

.to_canonical_bytes();

_{source: gpt-6.1-sol (phase2-reviewer: general)}

PastaPastaPasta and others added 7 commits September 23, 2026 11:37

$@infraclaw-dash$

ci: bootstrap trusted publisher before feature runner adoption

7b63fc6

Merge pull request #5175: bootstrap trusted CI image publisher

e2fd47d

infraclaw: install the reviewed publisher prerequisite only; no application or runner changes.

github-actions Bot added this to the v5.1.0 milestone Oct 6, 2026

lklimek mentioned this pull request Oct 6, 2026

fix(platform-wallet)!: restore Core spending state and repair persisted accounting #5150

Open

5 of 7 tasks

thepastaclaw suggested changes Oct 6, 2026

View reviewed changes

Comment thread Cargo.toml Outdated

Comment thread packages/rs-dpp/Cargo.toml

Comment thread ...form_events/core_based_updates/update_masternode_list/update_state_masternode_list/v0/mod.rs Outdated

lklimek added 2 commits October 6, 2026 13:49

lklimek changed the title ~~chore(platform)!: port rust-dashcore migration from #4932 to v5.1-dev~~ chore(platform)!: update rust-dashcore Oct 6, 2026

lklimek changed the title ~~chore(platform)!: update rust-dashcore~~ chore(platform)!: update rust-dashcore and require node >= 20 in wasm Oct 6, 2026

lklimek changed the title ~~chore(platform)!: update rust-dashcore and require node >= 20 in wasm~~ chore(platform)!: update rust-dashcore Oct 6, 2026

This was referenced Oct 6, 2026

chore!: require Node.js >=22 for JS packages and >=24 for dashmate #5310

Merged

fix(platform-wallet): retain SPV client and track shutdown through cancellation #5311

Merged

This was referenced Oct 6, 2026

feat(sdk)!: key limits, DIP-14 sub-feature derivation and decode-any-kind for DashPay Connect #4844

Draft

chore(platform)!: bump rust-dashcore to 719de34b (secp256k1 0.33) #4932

Closed

lklimek changed the title ~~chore(platform)!: update rust-dashcore~~ chore(platform)!: update rust-dashcore (incl secp256k1 0.33) Oct 6, 2026

lklimek marked this pull request as ready for review October 6, 2026 15:08

github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Oct 6, 2026

lklimek commented Oct 6, 2026

View reviewed changes

thepastaclaw suggested changes Oct 6, 2026

View reviewed changes

chore!: require Node.js >=22 for JS packages and >=24 for dashmate (#…

c0b4b97

…5310)

thepastaclaw suggested changes Oct 6, 2026

View reviewed changes

Comment thread Cargo.toml Outdated

Merge branch 'v5.1-dev' into chore/rust-dashcore-1112-v5.1

938b60f

lklimek mentioned this pull request Oct 7, 2026

refactor: share rust-dashcore BLS backend through dpp::bls #5320

Merged

4 of 7 tasks

thepastaclaw suggested changes Oct 7, 2026

View reviewed changes

fix(platform-wallet): retain SPV client and track shutdown through ca…

7c73e98

…ncellation (#5311)

Claudius-Maginificent mentioned this pull request Oct 7, 2026

fix(platform-wallet)!: tell a retryable SPV stop from one that needs a restart #5322

Merged

thepastaclaw suggested changes Oct 7, 2026

View reviewed changes

Comment thread packages/rs-platform-wallet/src/spv/runtime.rs

Comment thread packages/rs-platform-wallet/src/spv/runtime.rs

Comment thread packages/rs-platform-wallet/src/spv/runtime.rs

refactor: share rust-dashcore BLS backend through dpp::bls (#5320)

4045c13

Co-authored-by: Codex <noreply@openai.com>

thepastaclaw suggested changes Oct 7, 2026

View reviewed changes

fix(platform-wallet)!: tell a retryable SPV stop from one that needs …

8377b64

…a restart (#5322) Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Codex GPT-6 <noreply@openai.com>

Claudius-Maginificent mentioned this pull request Oct 8, 2026

fix(spv): let a manual disconnect stop a connection that is still starting dashpay/dash-evo-tool#1056

Open

4 of 8 tasks

thepastaclaw reviewed Oct 8, 2026

View reviewed changes

lklimek mentioned this pull request Oct 8, 2026

fix(crypto): deserialize legacy BLS public keys dashpay/rust-dashcore#1145

Open

2 of 4 tasks

thepastaclaw suggested changes Oct 8, 2026

View reviewed changes

	let node_id = EddsaPkBytes::from_bytes(pk32).hash().to_canonical_bytes();
	let node_id = dashcore::eddsa::EddsaPkHash::from(EddsaPkBytes::from_bytes(pk32))
	.to_canonical_bytes();

Conversation

lklimek commented Oct 6, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Basic explanation

Issue being fixed or feature implemented

What was done?

In-place changes to shipped generations

Merge order

How Has This Been Tested?

Outstanding review blockers

Breaking Changes

Checklist:

PR Hygiene · 9af2033

Uh oh!

coderabbitai Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

thepastaclaw commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Final validation — Phase 1 + Phase 2

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lklimek left a comment

Choose a reason for hiding this comment

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

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

1 finding(s) not shown inline (the lines are not part of this PR's diff)

Uh oh!

lklimek commented Oct 6, 2026

Uh oh!

github-actions Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

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

Uh oh!

Uh oh!

github-actions Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

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

Uh oh!

github-actions Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

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

Uh oh!

github-actions Bot commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thepastaclaw left a comment

lklimek commented Oct 6, 2026 •

edited by github-actions Bot

Loading

PR Hygiene · `9af2033`

coderabbitai Bot commented Oct 6, 2026 •

edited

Loading

thepastaclaw commented Oct 6, 2026 •

edited

Loading

github-actions Bot commented Oct 6, 2026 •

edited

Loading

github-actions Bot commented Oct 6, 2026 •

edited

Loading

github-actions Bot commented Oct 6, 2026 •

edited

Loading

github-actions Bot commented Oct 7, 2026 •

edited

Loading

github-actions Bot commented Oct 7, 2026 •

edited

Loading

github-actions Bot commented Oct 7, 2026 •

edited

Loading

github-actions Bot commented Oct 8, 2026 •

edited

Loading

github-actions Bot commented Oct 8, 2026 •

edited

Loading