feat: native state-transition deserialization, plus document/put/ContractBounds fixes - #40
Conversation
IdentityPublicKey.toObject() stored the raw ContractBounds object in the map, so Cbor.encode() threw "No converter for SingleContractDocumentType" when serializing any identity whose key carried SingleContractDocumentType bounds. This crashed sending/accepting a DashPay contact request via PlatformStateRepository.storeIdentity -> identity.toBuffer().toHex(). - toObject() now emits contractBounds.toObject() (a nested Map); the Map constructor reconstructs typed bounds via ContractBounds.from(Map). - ContractBounds gains from(Map) and toNative(), and toNative() now carries the bounds through the Rust SDK instead of dropping them. - storeIdentity's toBuffer() log is wrapped in try/catch so serialization can never break fetchIdentity again. - ContactRequests prefers a recipient encryption key whose contract bounds are scoped to the DashPay contract (new Identity.getFirstPublicKey overload). - Regression + selection tests added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d put input Two fixes surfaced by the DashPay wallet's DashConnect feature (first consumer of the mobile document put/replace path): - Document(RustDocument, dataContractId) left `type` null because the native document carries no type name. Add a `type` parameter and pass it from the DapiClient query (which already knows the type). A null type breaks any subsequent replace/put, whose native calls require a non-null type string. - put_document_sdk / replace_document_sdk used .expect() on document_type_for_name and CallbackSigner::new, aborting the whole process on a missing type or bad signer. Return an Err string instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add a ferment-exported deserialize_state_transition(bytes) -> StateTransitionInfo to platform-mobile. Modern state transitions (produced by the Rust SDK / wasm-sdk) are bincode-serialized, which the legacy Kotlin StateTransitionFactory (CBOR-only, and lacking IdentityUpdate support) cannot decode. This gives callers a native path to inspect a serialized transition's type, name, and owner id. StateTransitionInfo is intentionally minimal; extend with added-public-keys / revision / nonce as callers need them. fermented.rs regenerated (ferment full mode) with the new binding; the two broken platform_mobile_provider_Cache FFI conversion impls that ferment v0.2.3 mis-generates were removed, as before. Builds clean in cbindgen_only mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NativeStateTransition.deserialize(bytes) calls the new platformMobileStateTransitionDeserializeStateTransition FFI binding and maps the result to a StateTransitionInfo (type, name, ownerId), freeing the native struct afterward. Gives Kotlin callers a working path to decode modern (bincode) transitions, which the CBOR-only StateTransitionFactory cannot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tion Extend the native deserializer to expose all the information carried by a dash-st (IdentityUpdate) transition: - StateTransitionInfo gains revision and identity_nonce - identity_update_public_keys_to_add(bytes) returns the added keys as the standard IdentityPublicKey type (converted from IdentityPublicKeyInCreation), reusing the existing IdentityPublicKey ferment/SWIG/clone plumbing rather than a bespoke struct SWIG: DEFINE_LIST_RESULT maps Result<Vec<IdentityPublicKey>> to Result<List<IdentityPublicKey>> (state_transition.i, wired into root.i). Kotlin: NativeStateTransition.deserialize now returns revision, identityNonce and addPublicKeys (List<IdentityPublicKey>). fermented.rs regenerated; the mis-generated Cache FFI impls removed as before. Builds end-to-end (:dpp:compileKotlin incl. native link). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ignore.i/clone.h Follow the canonical SWIG pattern: deserialize_state_transition now maps through DEFINE_RESULT to base.Result<StateTransitionInfo> (rather than the raw ferment Result wrapper with a manual destroy), so memory is managed by the typemap like every other SDK call. This requires StateTransitionInfo: Clone plus a StateTransitionInfo_clone export in clone.rs. Regenerate ignore.i and clone.h via ignore.py (which must be run whenever the header changes) so the new types' _ctor/_destroy ignores and the StateTransitionInfo clone overload are present. The Kotlin wrapper drops the manual destroy and just calls Result.unwrap(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…orts dash-sdk-android/src/main/rust is a parallel dash_sdk_bindings crate over the same platform-mobile; its committed fermented.rs was stale, so the Android arm64 libdash_sdk_bindings.a lacked the new state-transition symbols and the cmake link failed (undefined platform_mobile_state_transition_* / StateTransitionInfo_clone). Regenerate it to match dash-sdk-bindings (identical ferment config + shared platform-mobile). Host cargo check passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds native state-transition deserialization and Java FFI bindings. It preserves contract-bound identity keys, adds contract-aware key selection, retains document types, improves error handling, updates metadata tests, and limits Android builds to 64-bit ABIs. ChangesSDK integration
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to The current changes are merge-ready after normal checks and review; no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant JavaSDK
participant SWIG
participant PlatformMobile
participant BincodeStateTransition
JavaSDK->>SWIG: NativeStateTransition.deserialize(bytes)
SWIG->>PlatformMobile: deserialize_state_transition(bytes)
PlatformMobile->>BincodeStateTransition: Deserialize bincode bytes
BincodeStateTransition-->>PlatformMobile: Transition data or error
PlatformMobile-->>SWIG: StateTransitionInfo or String error
SWIG-->>JavaSDK: Mapped Java result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dpp/src/main/java/org/dashj/platform/dashpay/ContactRequests.kt`:
- Around line 41-50: Update the fallback after the DashPay-scoped lookup in
getFirstPublicKey so it selects only enabled MEDIUM ECDSA_SECP256K1 encryption
keys with contractBounds == null, then retain the existing high-security
authentication-key fallback. Add a test covering an other-contract-bound key
preceding an unbound encryption key and verify the unbound key is selected.
In `@dpp/src/main/java/org/dashj/platform/dpp/document/Document.kt`:
- Around line 65-70: Require the type parameter in the RustDocument constructor
by removing its default value, then update DapiClient.deserializeDocument and
ContactRequests.create to pass the known document type when constructing native
Documents. Ensure every native-document construction site supplies a non-null
type for subsequent put or replace operations.
In `@platform-mobile/src/state_transition.rs`:
- Around line 36-38: Update deserialize_state_transition to use a shared decoder
that first supports tagged StateTransition payloads and falls back to bare
IdentityUpdateTransition payloads, preserving the existing StateTransitionInfo
result. Make identity_update_public_keys_to_add reuse this decoder, and add
regression tests covering both tagged and bare identity-update payloads.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fca11536-e251-48b0-80bb-8a345e1c474d
📒 Files selected for processing (22)
dash-sdk-android/build.gradledash-sdk-android/src/main/rust/src/fermented.rsdash-sdk-bindings/src/fermented.rsdash-sdk-java/src/main/cpp/clone.hdash-sdk-java/src/main/swig/ignore.idash-sdk-java/src/main/swig/root.idash-sdk-java/src/main/swig/state_transition.idpp/src/main/java/org/dashj/platform/dapiclient/DapiClient.ktdpp/src/main/java/org/dashj/platform/dashpay/ContactRequests.ktdpp/src/main/java/org/dashj/platform/dpp/document/Document.ktdpp/src/main/java/org/dashj/platform/dpp/identity/ContractBounds.ktdpp/src/main/java/org/dashj/platform/dpp/identity/Identity.ktdpp/src/main/java/org/dashj/platform/dpp/identity/IdentityPublicKey.ktdpp/src/main/java/org/dashj/platform/dpp/statetransition/NativeStateTransition.ktdpp/src/main/java/org/dashj/platform/sdk/platform/PlatformStateRepository.ktdpp/src/test/kotlin/org/dashj/platform/contracts/wallet/TxMetaDataTests.ktdpp/src/test/kotlin/org/dashj/platform/dpp/identity/IdentityPublicKeyTest.ktdpp/src/test/kotlin/org/dashj/platform/dpp/identity/IdentitySelectKeyTest.ktplatform-mobile/src/clone.rsplatform-mobile/src/lib.rsplatform-mobile/src/put.rsplatform-mobile/src/state_transition.rs
abaranouski
left a comment
There was a problem hiding this comment.
approved, though the build failed on platform monorepo's Rust dependency
…back getFirstPublicKey's bounds-agnostic fallback ignored contractBounds entirely, so it could hand back a key scoped to an unrelated contract instead of one with no contract bounds. Extend the contract-aware overload to accept a nullable contractId (null = unbound only) and use it in the fallback.
The RustDocument-based Document constructor defaulted type to null, so several put/replace call sites silently dropped the document type after publishing, breaking any subsequent replace/put on that document. Make type required and pass it through at every call site.
Nine commits from implementing the first wallet-side DashConnect / key-exchange login
(dash-wallet MO-945), which needed SDK capabilities that did not exist yet.
Native state-transition deserialization (the main addition)
The Kotlin
StateTransitionFactoryis CBOR-only, so it cannot decode transitions producedby the Rust/wasm SDKs (bincode with platform versioning). A wallet receiving a serialized
IdentityUpdateTransitionin a QR code had no way to inspect it before signing.platform-mobile/src/state_transition.rs:deserialize_state_transition(bytes)using DPP'snative
deserialize_from_bytes, returning aStateTransitionInfowith the transition type,name, owner id, revision, identity nonce, and — for identity updates — the added public keys
as existing
IdentityPublicKeyvalues (no new FFI type).NativeStateTransition.kt: Kotlin wrapper, result mapped throughbase.ResultviaDEFINE_RESULT/DEFINE_LIST_RESULTso memory is managed by the typemaps like every otherSDK call, rather than a raw ferment wrapper with a manual destroy.
ignore.i/clone.hregenerated (viaignore.py), anddash-sdk-android'sfermented.rssynced with the new exports — without that the Android cmake link fails on undefined symbols.
Fixes found while using the SDK from the wallet
Document.typewas null on queried documents. Only the create path set it, and a nulltype segfaults
platformMobilePutReplaceDocumentSdkthrough JNI. The type is now carried ondocuments built from native results.
put.rspanicked on bad input..expect()calls replaced withErrreturns, so invalidarguments surface as errors instead of aborting the process.
ContractBoundswas dropped inIdentityPublicKeyCBOR serialization, so contract-boundkeys did not round-trip. Covered by new
IdentityPublicKeyTestandIdentitySelectKeyTest.Verification
Published as
4.0.1-SNAPSHOTand consumed by the dash-wallet DashConnect branch, where thefull login flow completes end to end on testnet: a scanned
IdentityUpdateTransitionisdeserialized, verified to target our identity and to add exactly our derived login keys, and
the two keys are then registered on the identity.
Note for reviewers: yappr serializes that transition without the outer
StateTransitionenum variant tag (wasm-dpp2's
IdentityUpdateTransition::to_bytes()serializes the bare innertransition), so callers may need to handle both framings. Exposing
IdentityUpdateTransition::deserialize_from_bytesdirectly would be a natural follow-up.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Compatibility