Revert "feat(platform-wallet): add encrypted txMetadata document support" - #4279
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
💤 Files with no reviewable changes (14)
📝 WalkthroughWalkthroughThe change removes encrypted ChangesEncrypted transaction metadata removal
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
✅ Final review complete — no blockers (commit e4c31ad) |
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex + Sonnet
The PR is an exact file-for-file revert of #4277 and consistently removes the encrypted txMetadata implementation across Rust, FFI/JNI, Kotlin APIs, dependencies, exports, and tests. No functional or blocking issue remains, but the revert reintroduces two unused Kotlin imports that should be removed as a small cleanup.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed),gpt-5.6-sol— rust-quality (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— final-verifier (fallback) - Sonnet reviewers:
claude-sonnet-5— general (completed),claude-sonnet-5— security-auditor (completed),claude-sonnet-5— rust-quality (completed),claude-sonnet-5— ffi-engineer (completed)
💬 1 nitpick(s)
| import kotlinx.coroutines.Dispatchers | ||
| import kotlinx.coroutines.withContext |
There was a problem hiding this comment.
💬 Nitpick: Unused coroutine imports reintroduced by the revert
This revert re-adds Dispatchers and withContext, but neither symbol is referenced anywhere in the file. They were also unused before #4277—the removed encrypted-document methods did not reference them—so restoring them only creates dead-import IDE and linter noise. Delete both imports.
source: ['claude']
Two conflicts, both "each side added an arm", resolved by keeping both: - `packages/rs-platform-wallet-ffi/src/error.rs`: the txMetadata caller-input arms (payload size / wire version / encryptionKeyIndex -> ErrorInvalidParameter) and v4.2-dev's structural `Sdk(Protocol(Generic))` signer-prefix arm (#4183) both sit before the `_` catch-all. The signer arm stays last so it keeps its catch-all-only role. - `DashSdkError.kt`: this branch's typed `2 -> PlatformWallet.InvalidParameter` arm plus v4.2-dev's code-6 signer-marker fallback. Note v4.2-dev merged the encrypted-txMetadata foundation as #4277 and then reverted it in #4279, so the net base diff for those files is empty and the full stack is carried here.
…k so the merge keeps it The log dependency was first added by the encrypted-txMetadata change (dashpay#4277), then reverted on v4.2-dev (dashpay#4279). This branch carries the log line only passively (unchanged from the merge-base), so GitHub's 3-way PR merge applies the base-side deletion and the merged Cargo.toml loses the declaration — while the log:: breadcrumb calls this branch adds in changeset/core_bridge.rs remain, producing error[E0433]: unresolved crate log in the Kotlin SDK CI build. Relocate log = "0.4" out of the reverted Logging hunk into the untouched Security region so it is a branch-owned insertion that survives the merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…b into v41-keystore-qa5 Reconstruction of PR dashpay#4301 (feat/shielded-two-note-invites @d6610262b0) on top of qa5's tip. dashpay#4301 is based on a current v4.2-dev point; merging it directly would have dragged in 9 unrelated commits (dashpay#4287, dashpay#4266, dashpay#4279, dashpay#4276, dashpay#4278, dashpay#4277, and duplicate dashpay#4183/dashpay#4191/dashpay#4251), including dashpay#4277's competing encrypted-txMetadata implementation that collides with qa5's dashpay#4186. The reconstruction cherry-picks ONLY dashpay#4301's own commit; the resulting delta is byte-identical to the original (1256 insertions, 8 deletions across the same 11 files) — only hunk offsets differ. Verified: platform-wallet 774/774, platform-wallet-ffi 284/284, dpp shielded 216/216 (incl. both multi/single-output fee-vs-action-count tests), the four dashpay#4204 security tests, and dashpay#4301's three note-selection tests all pass. rustfmt clean; no new clippy warnings.
…k so the merge keeps it The log dependency was first added by the encrypted-txMetadata change (#4277), then reverted on v4.2-dev (#4279). This branch carries the log line only passively (unchanged from the merge-base), so GitHub's 3-way PR merge applies the base-side deletion and the merged Cargo.toml loses the declaration — while the log:: breadcrumb calls this branch adds in changeset/core_bridge.rs remain, producing error[E0433]: unresolved crate log in the Kotlin SDK CI build. Relocate log = "0.4" out of the reverted Logging hunk into the untouched Security region so it is a branch-owned insertion that survives the merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r FFI, JNI, Kotlin, and Swift Squash of PR dashpay#4259 (feat/kotlin-sdk-sign-message, head 09a27ac), rebased onto v4.2-dev. CoreWallet::sign_message — classic Dash signed messages, exposed through the full binding chain (Rust -> C FFI -> Kotlin/JNI and Swift). Given a P2PKH address the wallet holds keys for and an arbitrary UTF-8 message, returns the 65-byte recoverable signature base64-encoded — the same wire format as dashj's ECKey.signMessage and Dash Core's signmessage RPC. Includes digest-capability guard, typed errors (FFI code 31 ErrorSigningKeyUnavailable), null-pointer empty-message ABI contract, unpaired-surrogate rejection in Kotlin, and dashj byte-for-byte parity goldens. Rebase resolution: the TxMetadataPayloadTooLarge FFI mapping arm was dropped — the variant it matched was removed from v4.2-dev by the encrypted-txMetadata revert (dashpay#4279), so the arm no longer compiles and the error cannot be produced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reverts #4277
Summary by CodeRabbit