feat(rs-sdk-ffi): expose SDK protocol version pinning#3734
feat(rs-sdk-ffi): expose SDK protocol version pinning#3734thepastaclaw wants to merge 9 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds protocol-version-aware SDK creation across Rust FFI, Swift, and documentation. New C API constructors accept an explicit protocol version parameter; internal helpers validate and resolve protocol versions or default to auto-detect. Trusted SDK setup now enforces stricter DAPI defaults and error handling. Swift wrapper and docs are updated to demonstrate both pinned and auto-detect usage patterns. ChangesProtocol Version Pinning Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@packages/rs-sdk-ffi/src/sdk.rs`:
- Around line 341-349: The branch that checks config.core_sdk_handle only tests
for non-null but never uses it, so per-instance handles in DashSDKConfigExtended
are ignored; update the logic in dash_sdk_create_extended_with_protocol_version
to use the provided handle when non-null (e.g. obtain a context provider tied to
core_sdk_handle instead of unconditionally calling
CallbackContextProvider::from_global()), and pass that provider into
builder.with_context_provider; ensure you still fall back to from_global() only
when core_sdk_handle is null or when a handle-specific provider cannot be
created, producing the same error path via internal_error_result when
appropriate.
- Around line 139-163: The trusted setup logic (trusted_builder/trusted_setup)
currently re-parses dapi_addresses and rejects an empty string while
parse_dapi_addresses treats "" as None; change the trusted setup to call
parse_dapi_addresses(dapi_addresses) and branch on the resulting
Option<AddressList> (None = fallback to network defaults, Some(list) = use
provided addresses) instead of re-parsing the raw C string; apply the same
replacement at the other duplicate site that currently re-parses (the second
block that mirrors this logic) so both places treat an empty C string
consistently as “not provided.”
- Around line 165-176: The shared builder functions (e.g., base_builder and the
protocol-version constructors that call builder_with_protocol_version and
ultimately SdkBuilder::new or SdkBuilder::new_mock) only apply
network/dapi_addresses/protocol_version and ignore DashSDKConfig fields
request_retry_count, request_timeout_ms, and skip_asset_lock_proof_verification;
update these builders to propagate those config fields into the SdkBuilder
before returning (e.g., chain calls to the builder to set retry count, request
timeout, and enable/disable asset-lock-proof verification using the SdkBuilder
methods that control transport retry/timeout and verification flags) so the
values from DashSDKConfig are honored when build() is called.
🪄 Autofix (Beta)
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
Run ID: e421833c-e05a-4604-9e8c-e2755527c8a3
📒 Files selected for processing (3)
packages/rs-sdk-ffi/README.mdpackages/rs-sdk-ffi/src/sdk.rspackages/swift-sdk/Sources/SwiftDashSDK/SDK.swift
thepastaclaw
force-pushed
the
feature/ffi-sdk-protocol-version
branch
from
29648e9 to
65f0822
Compare
|
Addressed the CodeRabbit findings in the PR branch: trusted setup now reuses the shared DAPI address parser (so empty strings fall back to defaults), shared FFI builders propagate retry/timeout/proof settings, and extended SDK creation binds global callbacks to the provided per-instance Core SDK handle when present.\n\nValidation:\n- cargo test -p rs-sdk-ffi --no-default-features --features mocks\n- cargo check -p rs-sdk-ffi |
|
Addressed CodeRabbit finding 1 on the current PR branch.
Validation run:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
This PR adds an additive API across the Rust FFI layer and Swift wrapper to allow callers to explicitly pin the Dash Platform protocol version used by SdkBuilder, while preserving existing constructors’ default auto-detect behavior (protocol_version = 0).
Changes:
- Added new
*_with_protocol_versionFFI constructors and refactored SDK creation to share common builder/config logic. - Updated Swift
SDKinitializer to accept an optionalprotocolVersionparameter (defaulting to0) and call the new trusted FFI constructor. - Updated rs-sdk-ffi README examples for C/Swift/Python to show protocol-version pinning usage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift | Adds protocolVersion parameter (default 0) and forwards it into the trusted FFI constructor. |
| packages/rs-sdk-ffi/src/sdk.rs | Introduces protocol-version-aware creation entrypoints, shared builder helpers, and adds tests around pinning/validation. |
| packages/rs-sdk-ffi/src/context_callbacks.rs | Adds support for overriding the global callback handle with a per-instance Core SDK handle, plus unit tests. |
| packages/rs-sdk-ffi/README.md | Updates language binding examples and API reference to include the new protocol-version pinning constructor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| builder | ||
| .with_settings(request_settings_from_config(config)) | ||
| .with_proofs(!config.skip_asset_lock_proof_verification) |
There was a problem hiding this comment.
Addressed in 687508c. Added an explanatory comment at the with_proofs call site in apply_common_builder_config and expanded the field-level Rustdoc on DashSDKConfig::skip_asset_lock_proof_verification to state explicitly that it controls Platform state-proof verification for all SDK requests. The C example in the README has an equivalent note.
| } else if !config.core_sdk_handle.is_null() { | ||
| let core_sdk_handle = unsafe { &*config.core_sdk_handle }; | ||
| if let Some(callback_provider) = | ||
| crate::context_callbacks::CallbackContextProvider::from_core_sdk_handle(core_sdk_handle) | ||
| .or_else(crate::context_callbacks::CallbackContextProvider::from_global) | ||
| { | ||
| builder = builder.with_context_provider(callback_provider); |
| unsafe { set_global_callbacks(callbacks).expect("set callbacks") }; | ||
|
|
||
| let provider = CallbackContextProvider::from_core_sdk_handle(&CoreSDKHandle { | ||
| client: per_instance_handle, | ||
| }) | ||
| .expect("provider from callbacks"); |
| DashSDKConfig config = { | ||
| .network = DASH_SDK_NETWORK_TESTNET, | ||
| .dapi_addresses = "seed-1.testnet.networks.dash.org", | ||
| .skip_asset_lock_proof_verification = false, | ||
| .request_retry_count = 3, | ||
| .request_timeout_ms = 30000 | ||
| }; |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/rs-sdk-ffi/src/context_callbacks.rs`:
- Around line 245-269: This test mutates global callback state via
set_global_callbacks and doesn't restore it; wrap the mutation so
GLOBAL_CALLBACKS is restored after the test (e.g., capture the previous
callbacks before calling set_global_callbacks and restore them in a
drop/teardown or use a scoped helper) to avoid leaking state to other tests;
update the test around ContextProviderCallbacks / set_global_callbacks /
CallbackContextProvider::from_core_sdk_handle to save the prior
GLOBAL_CALLBACKS, set the test callbacks, run the assertions, and then restore
the saved callbacks in all paths (including on panic) so parallel tests remain
deterministic.
🪄 Autofix (Beta)
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
Run ID: 4053e0f6-0c16-453a-8c68-aae168b65dc7
📒 Files selected for processing (4)
packages/rs-sdk-ffi/README.mdpackages/rs-sdk-ffi/src/context_callbacks.rspackages/rs-sdk-ffi/src/sdk.rspackages/swift-sdk/Sources/SwiftDashSDK/SDK.swift
thepastaclaw
force-pushed
the
feature/ffi-sdk-protocol-version
branch
3 times, most recently
from
feca7c0 to
22365d9
Compare
|
Addressed the latest CodeRabbit/Copilot review items in feca7c0:
Validation:
|
thepastaclaw
force-pushed
the
feature/ffi-sdk-protocol-version
branch
2 times, most recently
from
feca7c0 to
22365d9
Compare
| /// - The returned handle inside `DashSDKResult` must be destroyed using the SDK destroy function to avoid leaks. | ||
| #[no_mangle] | ||
| pub unsafe extern "C" fn dash_sdk_create_extended_with_protocol_version( | ||
| config: *const DashSDKConfigExtended, |
There was a problem hiding this comment.
Why not just add protocol_version to DashSDKConfigExtended ? New constructors won't be needed.
There was a problem hiding this comment.
Good call — done in 687508c. protocol_version is now a field on DashSDKConfigExtended (0 = auto-detect), and the redundant dash_sdk_create_extended_with_protocol_version constructor has been removed. The two dash_sdk_create_with_callbacks variants are also consolidated behind a shared inner helper that sets the new field. We kept the plain-config _with_protocol_version constructors because adding a new field to DashSDKConfig itself touches a much larger surface (it is used by many tests, the Swift SDK, and the Python/C examples in the README), so that change felt out of scope for this PR.
| debug!("dash_sdk_create_trusted: testing basic HTTP connectivity"); | ||
| match reqwest::get("https://www.google.com").await { | ||
| Ok(_) => debug!("dash_sdk_create_trusted: basic HTTP test successful (Google)"), | ||
| Err(e) => warn!(error = %e, "dash_sdk_create_trusted: basic HTTP test failed"), | ||
| } |
| // Try the quorums endpoint directly | ||
| debug!("dash_sdk_create_trusted: testing quorums endpoint directly"); | ||
| match reqwest::get("https://quorums.testnet.networks.dash.org/quorums").await { | ||
| Ok(resp) => debug!(status = %resp.status(), "dash_sdk_create_trusted: direct quorums endpoint test successful"), | ||
| Err(e) => warn!(error = %e, "dash_sdk_create_trusted: direct quorums endpoint test failed"), | ||
| } | ||
|
|
||
| // Now try through the provider |
| /// - `config` must be a valid pointer to a DashSDKConfig structure | ||
| /// # Safety |
| /// Create a new SDK instance with an explicit protocol version override | ||
| /// | ||
| /// `protocol_version == 0` preserves the default auto-detect behavior. | ||
| /// | ||
| /// # Safety | ||
| /// - `config` must be a valid pointer to a DashSDKConfig structure for the duration of the call. | ||
| /// - The returned handle inside `DashSDKResult` must be destroyed using the SDK destroy function to avoid leaks. | ||
| #[no_mangle] | ||
| pub unsafe extern "C" fn dash_sdk_create_with_protocol_version( | ||
| config: *const DashSDKConfig, | ||
| protocol_version: u32, | ||
| ) -> DashSDKResult { | ||
| if config.is_null() { | ||
| return DashSDKResult::error(DashSDKError::new( | ||
| DashSDKErrorCode::InvalidParameter, | ||
| "Config is null".to_string(), | ||
| )); | ||
| } | ||
|
|
||
| let config = &*config; | ||
|
|
||
| // Parse configuration | ||
| let network: Network = config.network.into(); | ||
|
|
||
| let platform_version = match resolve_platform_version(protocol_version) { | ||
| Ok(platform_version) => platform_version, | ||
| Err(result) => return result, | ||
| }; | ||
|
|
| # Create SDK instance pinned to protocol version 11. | ||
| # Pass 0 to keep the default auto-detect behavior. | ||
| result = lib.dash_sdk_create_with_protocol_version(byref(config), 11) |
|
Addressed the valid review points on this branch:
I did not move Validation run:
@coderabbitai review |
|
✏️ Learnings added
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/rs-sdk-ffi/src/sdk.rs (2)
137-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThread the rest of
DashSDKConfigthrough both builder helpers.These paths still only apply network/DAPI/protocol version.
skip_asset_lock_proof_verification,request_retry_count, andrequest_timeout_msare never consumed beforebuild(), so callers can set them and still get the default SDK behavior.#!/bin/bash set -euo pipefail echo "== DashSDKConfig field usage in sdk.rs and types.rs ==" rg -n -C1 'skip_asset_lock_proof_verification|request_retry_count|request_timeout_ms' \ packages/rs-sdk-ffi/src/sdk.rs \ packages/rs-sdk-ffi/src/types.rs echo echo "== Candidate SdkBuilder setters/call sites in Rust sources ==" rg -n -C2 'with_.*(retry|timeout)|asset.*lock.*proof|proof.*verification|skip_asset_lock' . --glob '*.rs'Also applies to: 303-357
🤖 Prompt for 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. In `@packages/rs-sdk-ffi/src/sdk.rs` around lines 137 - 145, The builder helpers (both SdkBuilder::new(address_list) and SdkBuilder::new_mock()) fail to apply DashSDKConfig fields skip_asset_lock_proof_verification, request_retry_count, and request_timeout_ms; update build_sdk_builder (and the other helper path around lines 303-357) to call the corresponding SdkBuilder setters after creating the builder and before returning: read config.skip_asset_lock_proof_verification and call the builder method that disables/enables proof verification (e.g., with_skip_asset_lock_proof_verification or equivalent), set request retry count via the builder method for retries (e.g., with_request_retry_count), and convert config.request_timeout_ms into the builder timeout setter (e.g., with_request_timeout_ms or with_request_timeout using Duration::from_millis) so both branches (SdkBuilder::new and SdkBuilder::new_mock) consistently consume these config values.
166-175:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
core_sdk_handleinstead of falling back to globals here.Once this branch sees a non-null handle, it still calls
CallbackContextProvider::from_global()and never reads the handle value. That makes the per-instance handle inDashSDKConfigExtendeda no-op and can bind the SDK to the wrong Core SDK instance or fail with the global-registration error even when a valid handle was passed.#!/bin/bash set -euo pipefail echo "== core_sdk_handle branch in packages/rs-sdk-ffi/src/sdk.rs ==" rg -n -C2 'core_sdk_handle|from_global\(' packages/rs-sdk-ffi/src/sdk.rs echo echo "== handle-specific provider helpers in packages/rs-sdk-ffi/src/context_callbacks.rs ==" rg -n -C2 'from_core_sdk_handle|from_global' packages/rs-sdk-ffi/src/context_callbacks.rs🤖 Prompt for 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. In `@packages/rs-sdk-ffi/src/sdk.rs` around lines 166 - 175, The branch that checks config.core_sdk_handle.is_null() incorrectly always calls CallbackContextProvider::from_global(), ignoring the per-instance handle; change it to construct the provider from the passed handle when config.core_sdk_handle is non-null (e.g. call crate::context_callbacks::CallbackContextProvider::from_core_sdk_handle(core_sdk_handle) or equivalent) and pass that provider into builder.with_context_provider(callback_provider); if from_core_sdk_handle returns None or an error, return the same DashSDKResult::error as the current global-failure path. Ensure you reference config.core_sdk_handle (or the local core_sdk_handle variable) and use CallbackContextProvider::from_core_sdk_handle instead of CallbackContextProvider::from_global().
🤖 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.
Duplicate comments:
In `@packages/rs-sdk-ffi/src/sdk.rs`:
- Around line 137-145: The builder helpers (both SdkBuilder::new(address_list)
and SdkBuilder::new_mock()) fail to apply DashSDKConfig fields
skip_asset_lock_proof_verification, request_retry_count, and request_timeout_ms;
update build_sdk_builder (and the other helper path around lines 303-357) to
call the corresponding SdkBuilder setters after creating the builder and before
returning: read config.skip_asset_lock_proof_verification and call the builder
method that disables/enables proof verification (e.g.,
with_skip_asset_lock_proof_verification or equivalent), set request retry count
via the builder method for retries (e.g., with_request_retry_count), and convert
config.request_timeout_ms into the builder timeout setter (e.g.,
with_request_timeout_ms or with_request_timeout using Duration::from_millis) so
both branches (SdkBuilder::new and SdkBuilder::new_mock) consistently consume
these config values.
- Around line 166-175: The branch that checks config.core_sdk_handle.is_null()
incorrectly always calls CallbackContextProvider::from_global(), ignoring the
per-instance handle; change it to construct the provider from the passed handle
when config.core_sdk_handle is non-null (e.g. call
crate::context_callbacks::CallbackContextProvider::from_core_sdk_handle(core_sdk_handle)
or equivalent) and pass that provider into
builder.with_context_provider(callback_provider); if from_core_sdk_handle
returns None or an error, return the same DashSDKResult::error as the current
global-failure path. Ensure you reference config.core_sdk_handle (or the local
core_sdk_handle variable) and use CallbackContextProvider::from_core_sdk_handle
instead of CallbackContextProvider::from_global().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88e7bbd9-96a3-4710-aadd-8a459c530616
📒 Files selected for processing (3)
packages/rs-sdk-ffi/README.mdpackages/rs-sdk-ffi/src/sdk.rspackages/swift-sdk/Sources/SwiftDashSDK/SDK.swift
|
Addressed the latest CodeRabbit follow-ups in db8a8db:\n\n- shared FFI SDK builder setup now applies proof verification, retry, and timeout config consistently, including trusted builders\n- extended SDK creation now uses the supplied Core SDK handle via the registered callbacks instead of falling back to the global callback handle\n- trusted SDK creation reuses DAPI address parsing so empty addresses use network defaults\n\nValidation:\n- cargo fmt --all\n- cargo test -p rs-sdk-ffi --no-default-features --features mocks\n- cargo check -p rs-sdk-ffi |
- Move `protocol_version` into `DashSDKConfigExtended` per Lukasz's request: `dash_sdk_create_extended` now reads the field directly (0 = auto-detect) and the redundant `dash_sdk_create_extended_with_protocol_version` constructor is removed. Consolidates the two `dash_sdk_create_with_callbacks` variants behind a shared inner helper that sets the new field. - Make the `GLOBAL_CALLBACKS` unit tests deterministic by adding a `test_support::lock_global_callbacks_for_test` guard that serializes test access and restores any prior callbacks on drop. - Mark `CallbackContextProvider::from_core_sdk_handle` `unsafe` (it dereferences a raw pointer) and document the contract; update call sites accordingly. - Clarify in code and docs that `skip_asset_lock_proof_verification` toggles `SdkBuilder::with_proofs` for *all* requests rather than asset-lock-specific proofs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| /// with `InvalidParameter`. Callers that zero-initialize the struct | ||
| /// (e.g., Swift's `DashSDKConfigExtended()` or C `{0}` initializer) | ||
| /// automatically inherit the auto-detect default. | ||
| pub protocol_version: u32, |
There was a problem hiding this comment.
Addressed in 24eb5dfd86 + 6c448d8575 and pushed to the PR branch.
Fix summary:
- restored
DashSDKConfigExtendedto its original 3-field ABI/layout (no trailingprotocol_versionfield) - kept
dash_sdk_create_extendedABI-compatible and auto-detecting by default - added/restored separate protocol-version entry points:
dash_sdk_create_with_protocol_version,dash_sdk_create_extended_with_protocol_version,dash_sdk_create_trusted_with_protocol_version, anddash_sdk_create_with_callbacks_and_protocol_version - updated Swift and README to use the separate
_with_protocol_versionAPIs
Validation:
cargo fmt --all -- --checkcargo check -p rs-sdk-ffi --no-default-features --features mockscargo test -p rs-sdk-ffi --no-default-features --features mocks --lib sdk::tests
|
Refactored the protocol-version pinning change to use Summary:
Validation:
The broader |
Adding `protocol_version` to the exported `#[repr(C)]` `DashSDKConfigExtended` struct breaks ABI for existing callers. Drop the field and expose protocol-version pinning through dedicated exported constructors instead, leaving `dash_sdk_create_extended`'s ABI unchanged (auto-detect by default). New / restored entry points: - dash_sdk_create_with_protocol_version - dash_sdk_create_extended_with_protocol_version - dash_sdk_create_trusted_with_protocol_version - dash_sdk_create_with_callbacks_and_protocol_version Swift `SDK` initializer now calls `dash_sdk_create_trusted_with_protocol_version` directly; README C and Python examples document the separate functions; tests cover the new constructors and no longer rely on a config field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…creator `dash_sdk_create_with_callbacks_inner` was dispatching through the public `dash_sdk_create_extended` / `dash_sdk_create_extended_with_protocol_version` extern wrappers, paying an extra null-check on a stack-borrowed reference and a second dispatch hop. Call `create_extended_sdk_from_config` directly with a pre-resolved platform version. Resolving the version before the wrapper allocation also keeps an `InvalidParameter` early return from leaking the context-provider box. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| if protocolVersion == 0 { | ||
| result = dash_sdk_create_trusted(&config) | ||
| } else { | ||
| var extendedConfig = DashSDKConfigExtended() | ||
| extendedConfig.base_config = config | ||
| extendedConfig.context_provider = nil | ||
| extendedConfig.core_sdk_handle = nil | ||
| extendedConfig.protocol_version = protocolVersion | ||
| result = dash_sdk_create_extended(&extendedConfig) | ||
| } |
There was a problem hiding this comment.
Addressed in 6c448d8575: Swift now keeps the trusted creation path for both auto-detect and pinned protocol versions by calling dash_sdk_create_trusted_with_protocol_version(...). It no longer falls back to extended/mock creation when protocolVersion != 0.
| /// Optional Platform protocol version to pin the SDK to. | ||
| /// | ||
| /// `0` preserves the default auto-detect behavior; any non-zero value | ||
| /// must correspond to a known `PlatformVersion` or SDK creation fails | ||
| /// with `InvalidParameter`. Callers that zero-initialize the struct | ||
| /// (e.g., Swift's `DashSDKConfigExtended()` or C `{0}` initializer) | ||
| /// automatically inherit the auto-detect default. |
There was a problem hiding this comment.
Addressed in 24eb5dfd86 + 6c448d8575: DashSDKConfigExtended is back to its original three-field ABI/layout, and pinned versions use separate _with_protocol_version entry points instead of extending the exported struct.
|
|
||
| create_trusted_sdk_from_config(&*config, None) | ||
| } | ||
|
|
There was a problem hiding this comment.
Addressed in 24eb5dfd86 + 6c448d8575: added dash_sdk_create_trusted_with_protocol_version(config, protocol_version), while dash_sdk_create_trusted(config) remains the ABI-compatible auto-detect wrapper.
| /// SDK requests (wired to [`SdkBuilder::with_proofs(false)`]). The field | ||
| /// name reflects its original use case (skipping asset-lock proofs in | ||
| /// tests), but its effect is broader: callers should treat it as a | ||
| /// global "skip proofs" switch and only enable it for testing or for | ||
| /// trusted-context flows where proof verification is not desired. | ||
| /// | ||
| /// [`SdkBuilder::with_proofs(false)`]: https://github.com/dashpay/platform/blob/v3.1-dev/packages/rs-sdk/src/sdk.rs |
There was a problem hiding this comment.
Addressed in 5657829897: replaced the branch-specific GitHub URL with an intra-doc link to dash_sdk::SdkBuilder::with_proofs.
Validation: cargo fmt --all -- --check, cargo check -p rs-sdk-ffi --no-default-features --features mocks, and focused rs-sdk-ffi SDK tests passed. I also tried RUSTDOCFLAGS='-D warnings' cargo doc -p rs-sdk-ffi --no-deps --no-default-features --features mocks; it now gets past this link and fails on a pre-existing unrelated broken KeyType intra-doc link in signer.rs.
| DashSDKConfig config = { | ||
| .network = DASH_SDK_NETWORK_TESTNET, | ||
| .dapi_addresses = "seed-1.testnet.networks.dash.org", | ||
| .skip_asset_lock_proof_verification = false, | ||
| .request_retry_count = 3, | ||
| .request_timeout_ms = 30000 | ||
| }; |
There was a problem hiding this comment.
Addressed in 5657829897: the C example now uses a full DAPI URI with scheme and port: https://seed-1.testnet.networks.dash.org:1443.
| config = DashSDKConfig( | ||
| network=1, # Testnet | ||
| dapi_addresses=b"seed-1.testnet.networks.dash.org", | ||
| skip_asset_lock_proof_verification=False, | ||
| request_retry_count=3, | ||
| request_timeout_ms=30000 | ||
| ) |
There was a problem hiding this comment.
Addressed in 5657829897: the Python example now uses the same full DAPI URI form (https://seed-1.testnet.networks.dash.org:1443) so it satisfies AddressList::from_str parsing.
|
Replaced by #3751 |
Summary
Adds additive FFI and Swift SDK support for explicitly pinning the Dash
Platform protocol version used by
SdkBuilder.dash_sdk_*_with_protocol_versionconstructors for plain, extended,trusted, and callback-backed FFI SDK creation
protocol_version = 0auto-detect behavior
SDK(network:protocolVersion:), defaulting to0, so iOScallers can pass
11to pin Platform protocol version v11Validation
cargo fmt --all -- --checkcargo test -p rs-sdk-ffi sdk::tests --lib— 4 passedcargo checkcode-review dashpay/platform upstream/v3.1-dev feature/ffi-sdk-protocol-version— Recommendation: ship
functions
Summary by CodeRabbit
New Features
skip_asset_lock_proof_verificationconfiguration option.Documentation