refactor(platform-wallet): generate txMetadata indices locally - #1
Closed
QuantumExplorer wants to merge 1 commit into
Closed
Conversation
Author
|
Superseded by dashpay#4277, which carries the complete change directly on the post-dashpay#4183 v4.2-dev base and includes the redesign review fixes. |
bfoss765
added a commit
that referenced
this pull request
Aug 4, 2026
Addresses reviewer thepastaclaw's blocking findings on PR dashpay#4204. Two of the four blockers are fixed here; the other two are structural and reported back for a decision rather than guessed (crypto/money path). Blocker dashpay#4 (FFI RNG abort) — shielded_send.rs / keys.rs: `generate_one_time_orchard_key` used `OsRng::fill_bytes`, which panics on an OS entropy-source failure. It is called from a `#[no_mangle] extern "C"` export, so that panic aborts the process across the C ABI before any JNI panic guard can convert it. Switch to `RngCore::try_fill_bytes`, return a typed `PlatformWalletError::ShieldedKeyDerivation`, and have the FFI export map it to `ErrorWalletOperation` instead of aborting. Test call sites and callers updated for the new `Result` return. Blocker dashpay#3 (bearer spend key hygiene) — funding.rs: `oneTimeSk` is bearer spend authority but was marshalled via the generic `read_id32`, leaving its intermediate JNI `Vec<u8>` and returned `[u8; 32]` unsanitized. Add a `read_key32_zeroizing` helper (mirroring `transactions::read_key32_zeroizing`): the returned key is `Zeroizing` and the intermediate JNI copy is scrubbed. `sk` derefs to `[u8; 32]`, so the downstream `sk.as_ptr()` FFI call is unchanged. NOT fixed here (reported for decision): Blocker #1 (affected-state wait): `wait_for_affected_state` does not exist in this head's SDK, and the pool-funded sibling still uses `wait_for_response` on this branch. The reviewer's fix is predicated on rebasing onto the v4.1-dev proof API (31c69cf); it must be done in lockstep for both Type-20 paths. Blocker dashpay#2 (persist claim recovery record): the redrive mechanism is keyed by SubwalletId + activity entry and driven by the per-subwallet sync loop. Claim notes belong to a foreign one-time key tracked in no subwallet, so a correct fix needs a new subwallet-less pending-claim record + reconciliation path, not a reuse of `arm_redrive_record`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
that referenced
this pull request
Aug 4, 2026
Addresses reviewer thepastaclaw's blocking findings on PR dashpay#4204. Two of the four blockers are fixed here; the other two are structural and reported back for a decision rather than guessed (crypto/money path). Blocker dashpay#4 (FFI RNG abort) — shielded_send.rs / keys.rs: `generate_one_time_orchard_key` used `OsRng::fill_bytes`, which panics on an OS entropy-source failure. It is called from a `#[no_mangle] extern "C"` export, so that panic aborts the process across the C ABI before any JNI panic guard can convert it. Switch to `RngCore::try_fill_bytes`, return a typed `PlatformWalletError::ShieldedKeyDerivation`, and have the FFI export map it to `ErrorWalletOperation` instead of aborting. Test call sites and callers updated for the new `Result` return. Blocker dashpay#3 (bearer spend key hygiene) — funding.rs: `oneTimeSk` is bearer spend authority but was marshalled via the generic `read_id32`, leaving its intermediate JNI `Vec<u8>` and returned `[u8; 32]` unsanitized. Add a `read_key32_zeroizing` helper (mirroring `transactions::read_key32_zeroizing`): the returned key is `Zeroizing` and the intermediate JNI copy is scrubbed. `sk` derefs to `[u8; 32]`, so the downstream `sk.as_ptr()` FFI call is unchanged. NOT fixed here (reported for decision): Blocker #1 (affected-state wait): `wait_for_affected_state` does not exist in this head's SDK, and the pool-funded sibling still uses `wait_for_response` on this branch. The reviewer's fix is predicated on rebasing onto the v4.1-dev proof API (31c69cf); it must be done in lockstep for both Type-20 paths. Blocker dashpay#2 (persist claim recovery record): the redrive mechanism is keyed by SubwalletId + activity entry and driven by the per-subwallet sync loop. Claim notes belong to a foreign one-time key tracked in no subwallet, so a correct fix needs a new subwallet-less pending-claim record + reconciliation path, not a reuse of `arm_redrive_record`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
encryptionKeyIndexhigh-water map and Platform document-count scan with a Rust-owned, non-zero 31-bit index generated by the operating-system CSPRNGArcout of the global FFI handle store before resolver callbacks and network waitsWhy
This is stacked on dashpay#4195 and redesigns where its index-selection policy lives.
The previous
1 + Platform document countplus an in-process mutex looked authoritative, but it could not provide an atomic cross-device allocation invariant. It also put mutable orchestration state on theIdentityWalletfacade and added a full paginated Platform scan to the first create. Since every document stores its ownkeyIndexandencryptionKeyIndex, and each ciphertext has a fresh IV, a repeated index is non-lossy: readers derive from each document's stored fields and both documents remain decryptable.Generating a valid BIP-32 child index at the Rust encryption boundary keeps the host thin without claiming a uniqueness guarantee the protocol does not enforce. It also removes the allocation network round trip and wallet-wide head-of-line blocking.
Compatibility and impact
{ keyIndex, encryptionKeyIndex, encryptedMetadata }ArcTests
cargo test -p platform-wallet --lib— 525 passedcargo test -p platform-wallet-ffi --lib— 217 passedcargo test -p platform-wallet-ffi --lib encrypted_document_rejects_oversized_payload_before_wallet_lookupcargo check -p rs-unified-sdk-jnicargo clippy -p platform-wallet -p platform-wallet-ffi -p rs-unified-sdk-jni --lib -- -D warnings./gradlew :sdk:testDebugUnitTest --tests org.dashfoundation.dashsdk.documents.DocumentTransactionsVersionValidationTest./gradlew :sdk:lintDebug