Skip to content

feat(rs-sdk-ffi): expose optional platform_version in DashSDKConfig #3751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lklimek merged 2 commits into from
May 27, 2026
Merged

feat(rs-sdk-ffi): expose optional platform_version in DashSDKConfig #3751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e013336
feat(rs-sdk-ffi): expose optional platform_version in DashSDKConfig
lklimek May 27, 2026
4d3a565
Merge branch 'v3.1-dev' into feat/rs-sdk-ffi-config-version
lklimek May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions packages/rs-sdk-ffi/src/sdk.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@ pub struct DashSDKConfigExtended {
pub core_sdk_handle: *mut CoreSDKHandle,
}

fn apply_version(builder: SdkBuilder, platform_version: u32) -> Result<SdkBuilder, DashSDKError> {
if platform_version == 0 {
return Ok(builder);
}
match dash_sdk::dpp::version::PlatformVersion::get(platform_version) {
Ok(v) => Ok(builder.with_version(v)),
Err(e) => Err(DashSDKError::new(
DashSDKErrorCode::InvalidParameter,
format!("Invalid platform_version {}: {}", platform_version, e),
)),
Comment thread
lklimek marked this conversation as resolved.
}
Comment thread
lklimek marked this conversation as resolved.
Comment on lines +34 to +38
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: Redundant version number in error message

STILL VALID at 4d3a565 — sdk.rs:36 still reads format!("Invalid platform_version {}: {}", platform_version, e). PlatformVersionError::UnknownVersionError already formats as "no platform version {version}" (rs-platform-version/src/version/protocol_version.rs:92), so the FFI error reads "Invalid platform_version 99: no platform version 99". Either drop the inner error or drop the Invalid platform_version {n} prefix and propagate the inner string.

Suggested change
Err(e) => Err(DashSDKError::new(
DashSDKErrorCode::InvalidParameter,
format!("Invalid platform_version {}: {}", platform_version, e),
)),
}
Err(e) => Err(DashSDKError::new(
DashSDKErrorCode::InvalidParameter,
format!("Invalid platform_version: {}", e),
)),

source: ['claude', 'codex']

}
Comment thread
lklimek marked this conversation as resolved.
Comment on lines +28 to +39
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Add coverage for the new invalid-platform-version error path

STILL VALID at 4d3a565 (apply_version unchanged; no new tests in the latest delta). apply_version has three branches: 0 short-circuits to the SDK default, a known version is forwarded to SdkBuilder::with_version, and an unknown version returns DashSDKError { code: InvalidParameter, ... }. Every modified test fixture (token/{claim,emergency_action,freeze}.rs, tests/context_provider_test.rs) sets platform_version: 0, which short-circuits before the lookup — the InvalidParameter branch is unexercised.

A small #[cfg(test)] test in sdk.rs calling apply_version directly with u32::MAX (or dash_sdk_create with that value) and asserting DashSDKErrorCode::InvalidParameter would lock in the contract documented on the new field. The PR checklist already notes tests were not added, so this is a known gap.

source: ['claude', 'codex']


/// Internal SDK wrapper
pub(crate) struct SDKWrapper {
pub sdk: Sdk,
Expand Down Expand Up @@ -149,6 +162,11 @@ pub unsafe extern "C" fn dash_sdk_create(config: *const DashSDKConfig) -> DashSD
}
};

let builder = match apply_version(builder, config.platform_version) {
Ok(b) => b,
Err(e) => return DashSDKResult::error(e),
};

// Build SDK
let sdk_result = builder.build().map_err(FFIError::from);

Expand Down Expand Up @@ -257,6 +275,11 @@ pub unsafe extern "C" fn dash_sdk_create_extended(
}
}

let builder = match apply_version(builder, base_config.platform_version) {
Ok(b) => b,
Err(e) => return DashSDKResult::error(e),
};

// Build SDK
let sdk_result = builder.build().map_err(FFIError::from);

Expand Down Expand Up @@ -420,6 +443,11 @@ pub unsafe extern "C" fn dash_sdk_create_trusted(config: *const DashSDKConfig) -
info!("dash_sdk_create_trusted: adding trusted context provider to builder");
let builder = builder.with_context_provider(Arc::clone(&trusted_provider));

let builder = match apply_version(builder, config.platform_version) {
Ok(b) => b,
Err(e) => return DashSDKResult::error(e),
};

// Build SDK
let sdk_result = builder.build().map_err(FFIError::from);

Expand Down Expand Up @@ -572,6 +600,7 @@ pub unsafe extern "C" fn dash_sdk_create_with_callbacks(
skip_asset_lock_proof_verification: config_ref.skip_asset_lock_proof_verification,
request_retry_count: config_ref.request_retry_count,
request_timeout_ms: config_ref.request_timeout_ms,
platform_version: config_ref.platform_version,
},
context_provider: context_provider_handle,
core_sdk_handle: std::ptr::null_mut(),
Expand Down
1 change: 1 addition & 0 deletions packages/rs-sdk-ffi/src/token/claim.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ mod tests {
skip_asset_lock_proof_verification: false,
request_retry_count: 3,
request_timeout_ms: 5000,
platform_version: 0,
};

let result = unsafe { crate::sdk::dash_sdk_create(&config) };
Expand Down
1 change: 1 addition & 0 deletions packages/rs-sdk-ffi/src/token/emergency_action.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ mod tests {
skip_asset_lock_proof_verification: false,
request_retry_count: 3,
request_timeout_ms: 5000,
platform_version: 0,
};

let result = unsafe { crate::sdk::dash_sdk_create(&config) };
Expand Down
1 change: 1 addition & 0 deletions packages/rs-sdk-ffi/src/token/freeze.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ mod tests {
skip_asset_lock_proof_verification: false,
request_retry_count: 3,
request_timeout_ms: 5000,
platform_version: 0,
};

let result = unsafe { crate::sdk::dash_sdk_create(&config) };
Expand Down
4 changes: 4 additions & 0 deletions packages/rs-sdk-ffi/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ pub struct DashSDKConfig {
pub request_retry_count: u32,
/// Timeout for requests in milliseconds
pub request_timeout_ms: u64,
/// Pin to a specific Dash Platform protocol version.
/// `0` keeps the SDK default (auto-detect / latest); any non-zero value
/// is forwarded to `SdkBuilder::with_version` and rejected if unknown.
pub platform_version: u32,
Comment thread
lklimek marked this conversation as resolved.
}
Comment thread
lklimek marked this conversation as resolved.
Comment on lines 77 to 84
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Reword the "preserves ABI" claim — appending to a repr(C) struct is source-compatible only

STILL VALID at 4d3a565 (file unchanged in the latest delta). The current doc comment on DashSDKConfig (types.rs:64-84) does not itself make the ABI-preserving claim, but the PR description framing does. Appending platform_version to a #[repr(C)] struct passed by pointer is source-compatible (recompiling against the regenerated cbindgen header works), not binary-compatible: all four entry points unconditionally read config.platform_version (sdk.rs:165, sdk.rs:278, sdk.rs:446, sdk.rs:603), so a caller compiled against the previous header would allocate the shorter layout and the new library would read 4 bytes past the caller's allocation — UB resulting in a crash or unintended protocol-version pin.

Concrete risk is low: rs-sdk-ffi ships at 3.1.0-dev.6 with no committed binary-ABI promise, the cbindgen header is regenerated and shipped with the library, the Swift bridge is updated in lockstep, and the README examples already diverge from the struct layout (omitting skip_asset_lock_proof_verification). Fix is documentation-only — in the PR description (and optionally on the DashSDKConfig doc comment), replace the "preserves ABI" wording with "source-compatible — consumers must rebuild against the regenerated FFI header." Broader struct_size/struct_version discriminator design is tracked as out-of-scope.

source: ['claude', 'codex']


/// Result data type indicator for iOS
Expand Down
1 change: 1 addition & 0 deletions packages/rs-sdk-ffi/tests/context_provider_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ mod tests {
skip_asset_lock_proof_verification: false,
request_retry_count: 3,
request_timeout_ms: 30000,
platform_version: 0,
};

// Create extended config
Expand Down
3 changes: 2 additions & 1 deletion packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,14 @@ public final class SDK: @unchecked Sendable {
/// This uses a trusted context provider that fetches quorum keys and
/// data contracts from trusted HTTP endpoints instead of requiring proof verification.
/// This is suitable for mobile applications where proof verification would be resource-intensive.
public init(network: Network) throws {
public init(network: Network, platformVersion: UInt32 = 0) throws {
var config = DashSDKConfig()
config.network = network.ffiValue
config.dapi_addresses = nil
config.skip_asset_lock_proof_verification = false
config.request_retry_count = 1
config.request_timeout_ms = 8000 // 8 seconds
config.platform_version = platformVersion // 0 = SDK default (auto-detect)

// Create SDK with trusted setup — Rust side auto-detects local/regtest
// and uses the quorum sidecar at localhost:22444 instead of remote endpoints.
Expand Down
Loading