fix(rs-sdk-ffi)!: sanitize properties in dash_sdk_document_set_properties - #4927
Conversation
…ties dash_sdk_document_set_properties stored the parsed JSON as is, so a base58 identifier or a hex byte string (top level or inside a typed array) reached the replace transition as text. The create path and the platform-wallet create/replace paths run DocumentType::sanitize_document_properties first; this one did not. The function now takes the SDK handle, the base58 contract id and the document type name, looks the contract up in the trusted context provider (the lookup dash_sdk_document_replace_on_platform makes with the same id) and sanitizes before storing. On any error the document is left unchanged. BREAKING CHANGE: dash_sdk_document_set_properties takes (sdk_handle, document_handle, data_contract_id, document_type_name, properties_json). The Swift SDK's documentReplace is updated and now surfaces the error instead of discarding it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 17 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: dashpay/platform/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
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 |
|
|
Issue being fixed or feature implemented
dash_sdk_document_set_propertiesstored the parsed JSON properties as they came in, without the schema sanitizer.dash_sdk_document_create, and the platform-walletcreate_document_with_signer/replace_document_with_signer, all runDocumentType::sanitize_document_propertiesfirst. That step turns base58 or hex strings into identifiers, hex or base64 strings into bytes, and narrows integers, including inside protocol version 14 typed array elements.The Swift SDK's
documentReplace(used by the SwiftExampleApp state-transition builder inTransitionDetailView.swift) goes throughset_properties. #4926 makes that builder send identifier and byte-array values as base58 and hex strings. On this path those strings stayed as text in the document and ended up in the replace transition as text, where the document type's schema expects byte arrays ("type": "array"). So the replace could not carry an identifier or byte-array property, whether top-level or inside a typed array.Example: a
notetype with an identifierauthor, a byte arraypayloadand a typed identifier arraymembers, updated with{"author":"US517G5965aydkZ46HS38QLi7UQiSojurfbQfKCELFx","payload":"deadbeef","members":["YMN9Qj5jPNp7j14VPcML1B6xGgcPWVZUGLFU3Mnyfaf","cGfHiC6Kgg3FpFZvgwGcswsCRtp4aBP2fzuXRQPizuN"]}Before, the stored document properties:
After:
What was done?
dash_sdk_document_set_properties(packages/rs-sdk-ffi/src/document/util.rs) now takes the SDK handle, the base58 contract id and the document type name. It gets the contract from the trusted context provider the same waydash_sdk_document_createdoes, which is also the lookupdash_sdk_document_replace_on_platformmakes with the same id. It then runssanitize_document_propertiesfor that document type before storing the map. If the contract or type cannot be resolved, it returns an error and leaves the document unchanged.documentReplace(StateTransitionExtensions.swift) passes the SDK handle, contract id and document type it already holds. It used to discard the returned error (_ =). It now frees the error and throwsSDKError.internalError.The C signature had to change: a
Documentrecords neither its contract id nor its type name, so the handle alone cannot identify the schema to sanitize against.git grepfinds no other caller (Swift, Kotlin, C or docs).How Has This Been Tested?
should_store_identifier_and_byte_properties_decoded_from_stringsinrs-sdk-ffi: builds a contract at the latest protocol version with an identifier, a byte array and a typed identifier array. It serves that contract from an offline trusted context provider (an address-literal quorum URL, so nothing resolves or connects), sets the JSON above throughdash_sdk_document_set_properties, and checks the stored values. Before the fix, this test failed withleft: Some(Text("US517G59...")) right: Some(Identifier([7, ...])).should_leave_the_document_unchanged_when_the_document_type_cannot_be_resolved: with an unknown contract id and with an unknown type name, the call returns anInternalErrorand the document stays unchanged.cargo test -p rs-sdk-ffi --lib document::: 75 passed.cargo clippy -p rs-sdk-ffiandcargo clippy -p rs-sdk-ffi --all-targets: no warnings.Breaking Changes
dash_sdk_document_set_propertieschanges from(document_handle, properties_json)to(sdk_handle, document_handle, data_contract_id, document_type_name, properties_json). FFI callers must pass the SDK handle, the base58 contract id and the document type name. The contract must be in the trusted context provider, as it already must be for thedash_sdk_document_replace_on_platform*call that follows. The only in-repo caller, the Swift SDK'sdocumentReplace, is updated here.Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code