feat(sdk): async get quorum public key #2865
Conversation
WalkthroughThe PR adds asynchronous quorum public key retrieval to the ContextProvider trait and standardizes the return type to a fixed-size 48-byte array. A new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (6)📓 Common learnings📚 Learning: 2024-11-20T16:05:40.200ZApplied to files:
📚 Learning: 2024-10-10T10:30:19.883ZApplied to files:
📚 Learning: 2025-01-23T09:23:32.740ZApplied to files:
📚 Learning: 2025-08-05T13:55:39.147ZApplied to files:
📚 Learning: 2024-10-22T10:53:12.111ZApplied to files:
🧬 Code graph analysis (1)packages/rs-sdk-trusted-context-provider/examples/with_fallback.rs (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
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 |
✅ gRPC Query Coverage Report |
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "290188adf46e5f8a8e35eface53138246f786ea4a64327a960cf22d278f1cee5"
)Xcode manual integration:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/rs-sdk-ffi/src/context_callbacks.rs (1)
112-128: Async wrapper is correct and object‑safe
get_quorum_public_key_asynccorrectly reuses the sync implementation and returns aPin<Box<dyn Future<...> + Send + 'static>>without capturing&self, so the future is'staticand safe to use behind trait objects. The only caveat is that the call remains fully blocking; if you ever introduce a truly async FFI callback, this wrapper will be the right place to integrate it.packages/rs-sdk-trusted-context-provider/src/provider.rs (1)
435-451: Trusted async method wiring looks good (still sync under the hood)The new
get_quorum_public_key_asynccorrectly delegates to the existing sync implementation and wraps the result in a'staticfuture, aligning with the updatedContextProvidertrait. As your comment notes, this remains blocking; when you’re ready to refactor, this will be the natural place to route through the underlying async HTTP path instead of the sync/block_onflow.packages/rs-sdk/src/mock/provider.rs (1)
102-125: Good extraction of shared quorum‑key logic into a sync helperCentralizing the cache lookup, core fetch, cache write, and optional dump in
get_quorum_public_key_sync_implkeeps the trait impls thin and avoids duplication. The cache key and value types line up withquorum_public_keys_cache, and returning a[u8; 48]by value is efficient since the array isCopy.packages/wasm-sdk/src/context_provider.rs (2)
22-36: Async method forWasmContextcorrectly mirrors the sync behavior
get_quorum_public_key_asyncsimply wraps the syncget_quorum_public_keyerror in a ready future, which keeps the “non‑trusted mode is not supported in WASM” semantics consistent across both sync and async calls while satisfying the trait.
86-101:WasmTrustedContextasync delegation is straightforward and safeDelegating
get_quorum_public_key_asynctoself.inner.get_quorum_public_key_asyncreuses the trusted HTTP provider’s implementation and preserves its behavior without introducing new lifetime or threading concerns. This matches the pattern used in other wrappers in the PR.packages/rs-context-provider/src/provider.rs (3)
6-6: Async quorum‑key API onContextProvideris well‑shaped for trait objectsAdding
get_quorum_public_key_asyncwith aPin<Box<dyn Future<...> + Send + 'static>>return keeps the trait object‑safe and lets all providers expose a uniform async interface while the sync method remains available. The docs clearly mark the async method as the primary API and clarify the 48‑byte key shape, which matches the rest of the PR.Also applies to: 65-87, 88-103
136-148: BlanketContextProviderimpl forAsRef<dyn ContextProvider>forwards async correctlyThe new
get_quorum_public_key_asyncimplementation simply forwards toself.as_ref(), preserving any custom behavior of the underlying provider and reusing its'staticfuture. This keepsArc<T>,Box<T>, etc., ergonomically usable asContextProviders without extra boilerplate.
282-296: Mock async implementation correctly wraps the existing sync behavior
MockContextProvider::get_quorum_public_key_asyncreuses the sync file‑based implementation and returns a ready future. This keeps the mock’s behavior unchanged while satisfying the async trait and avoids any reference‑lifetime issues, since only theResult<[u8; 48], ContextProviderError>is moved into the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/rs-context-provider/src/provider.rs(6 hunks)packages/rs-sdk-ffi/src/context_callbacks.rs(1 hunks)packages/rs-sdk-trusted-context-provider/src/provider.rs(1 hunks)packages/rs-sdk/src/mock/provider.rs(2 hunks)packages/wasm-sdk/src/context_provider.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-sdk-ffi/src/context_callbacks.rspackages/rs-sdk-trusted-context-provider/src/provider.rspackages/rs-sdk/src/mock/provider.rspackages/wasm-sdk/src/context_provider.rspackages/rs-context-provider/src/provider.rs
packages/rs-sdk-ffi/**/*.{h,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Follow unified SDK function prefixes: dash_core_sdk_* for Core, dash_sdk_* for Platform, dash_unified_sdk_* for unified APIs
Files:
packages/rs-sdk-ffi/src/context_callbacks.rs
packages/wasm-sdk/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep WASM SDK docs in sync (run generate_docs.py) when updating the WASM SDK
Files:
packages/wasm-sdk/src/context_provider.rs
🧠 Learnings (15)
📓 Common learnings
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs:148-151
Timestamp: 2024-11-20T16:05:40.200Z
Learning: In `packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs`, when converting public keys from `QuorumForSavingV0` to `VerificationQuorum`, it's acceptable to use `.expect()` for public key conversion, as the conversion has been verified and panics are acceptable in this context.
Learnt from: shumkov
Repo: dashpay/platform PR: 2259
File: packages/rs-dapi-client/src/dapi_client.rs:137-139
Timestamp: 2024-10-22T10:53:12.111Z
Learning: In `packages/rs-dapi-client/src/dapi_client.rs`, when passing data into asynchronous code, ensure that data structures are `Send + Sync`. Using `Arc<AtomicUsize>` is necessary for the retry counter.
📚 Learning: 2024-11-20T16:05:40.200Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs:148-151
Timestamp: 2024-11-20T16:05:40.200Z
Learning: In `packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs`, when converting public keys from `QuorumForSavingV0` to `VerificationQuorum`, it's acceptable to use `.expect()` for public key conversion, as the conversion has been verified and panics are acceptable in this context.
Applied to files:
packages/rs-sdk-ffi/src/context_callbacks.rspackages/rs-sdk-trusted-context-provider/src/provider.rspackages/rs-sdk/src/mock/provider.rspackages/wasm-sdk/src/context_provider.rspackages/rs-context-provider/src/provider.rs
📚 Learning: 2025-01-23T09:23:32.740Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2405
File: packages/rs-sdk/src/sync.rs:88-95
Timestamp: 2025-01-23T09:23:32.740Z
Learning: The `block_on` function in `packages/rs-sdk/src/sync.rs` is currently only used in tests, and its WebAssembly implementation is deferred until there's a user request for it.
Applied to files:
packages/rs-sdk-ffi/src/context_callbacks.rspackages/rs-sdk-trusted-context-provider/src/provider.rspackages/wasm-sdk/src/context_provider.rspackages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-10T10:30:19.883Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2232
File: packages/rs-sdk/src/mock/sdk.rs:90-95
Timestamp: 2024-10-10T10:30:19.883Z
Learning: In `packages/rs-sdk/src/mock/sdk.rs`, the `load_expectations` method in `MockDashPlatformSdk` remains asynchronous (`async`) for backward compatibility, even though it now delegates to the synchronous `load_expectations_sync` method.
Applied to files:
packages/rs-sdk/src/mock/provider.rspackages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-10T10:30:25.283Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2232
File: packages/rs-sdk/src/mock/sdk.rs:129-130
Timestamp: 2024-10-10T10:30:25.283Z
Learning: In the codebase `packages/rs-sdk/src/mock/sdk.rs`, `tokio::sync::Mutex` is used instead of `std::sync::Mutex` because `std::sync::MutexGuard` is not `Send` and cannot be used in async contexts.
Applied to files:
packages/rs-sdk/src/mock/provider.rspackages/rs-context-provider/src/provider.rs
📚 Learning: 2024-11-20T20:43:41.185Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/tests/strategy_tests/masternodes.rs:212-220
Timestamp: 2024-11-20T20:43:41.185Z
Learning: In `packages/rs-drive-abci/tests/strategy_tests/masternodes.rs`, the pattern of generating a `PrivateKey`, converting it to bytes, and reconstructing a `BlsPrivateKey` from those bytes is intentional. Avoid suggesting to simplify this code in future reviews.
Applied to files:
packages/rs-sdk/src/mock/provider.rs
📚 Learning: 2025-07-28T20:04:48.458Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/index.html:4360-4416
Timestamp: 2025-07-28T20:04:48.458Z
Learning: In packages/wasm-sdk, the wallet helper `derive_key_from_seed_with_path` (Rust function in src/wallet/key_derivation.rs) is synchronous; its JS wrapper returns a value immediately, so `await` is unnecessary.
Applied to files:
packages/wasm-sdk/src/context_provider.rs
📚 Learning: 2025-01-23T09:23:32.740Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2405
File: packages/rs-sdk/src/sync.rs:88-95
Timestamp: 2025-01-23T09:23:32.740Z
Learning: The `block_on` function in `packages/rs-sdk/src/sync.rs` requires true blocking behavior, which is why a non-blocking WebAssembly implementation using `wasm_bindgen_futures::spawn_local` is not suitable.
Applied to files:
packages/wasm-sdk/src/context_provider.rs
📚 Learning: 2025-08-05T13:55:39.147Z
Learnt from: thephez
Repo: dashpay/platform PR: 2718
File: packages/wasm-sdk/index.html:0-0
Timestamp: 2025-08-05T13:55:39.147Z
Learning: The get_identity_keys_with_proof_info function in the Rust WASM bindings does not support the "search" key request type and lacks the searchPurposeMap parameter. When proof mode is enabled with keyRequestType === 'search', the implementation falls back to the non-proof version (get_identity_keys) to maintain functionality.
Applied to files:
packages/wasm-sdk/src/context_provider.rs
📚 Learning: 2024-11-22T08:19:14.448Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2345
File: packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs:93-99
Timestamp: 2024-11-22T08:19:14.448Z
Learning: In `packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs`, the `insert_contract` method requires an owned `BlockInfo`, so cloning `block_info` is necessary when calling it.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-06T16:17:34.571Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:105-105
Timestamp: 2024-10-06T16:17:34.571Z
Learning: In `run_block_proposal` in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, when retrieving `last_block_time_ms`, it's acceptable to use `platform_state` instead of `block_platform_state`, even after updating the protocol version.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-04T14:16:05.798Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2207
File: packages/rs-drive-proof-verifier/src/proof.rs:1646-1664
Timestamp: 2024-10-04T14:16:05.798Z
Learning: In the implementation of `FromProof<platform::GetContestedResourceIdentityVotesRequest>` in `packages/rs-drive-proof-verifier/src/proof.rs`, when matching `maybe_votes`, using `.expect()` on `v.into_iter().next()` is acceptable because the prior match arm `Some(v) if v.is_empty()` ensures that the map is not empty, preventing a panic.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-29T14:44:01.184Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-dapi-client/src/executor.rs:38-38
Timestamp: 2024-10-29T14:44:01.184Z
Learning: In `packages/rs-dapi-client/src/executor.rs`, for the structs `ExecutionError<E>` and `ExecutionResponse<R>`, only the serde derives (`Serialize`, `Deserialize`) should be conditional based on the "mocks" feature; the other derives (`Debug`, `Clone`, `Eq`, `PartialEq`) should always be present.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-06T16:18:07.994Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:119-120
Timestamp: 2024-10-06T16:18:07.994Z
Learning: In the `run_block_proposal` function in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, it's acceptable to pass `platform_state` to `perform_events_on_first_block_of_protocol_change`, even if `block_platform_state` has been updated.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-11-20T10:01:50.837Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs:94-94
Timestamp: 2024-11-20T10:01:50.837Z
Learning: In `packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs`, when converting with `PublicKey::try_from`, it's acceptable to use `.expect()` to handle potential conversion errors.
Applied to files:
packages/rs-context-provider/src/provider.rs
🧬 Code graph analysis (5)
packages/rs-sdk-ffi/src/context_callbacks.rs (4)
packages/rs-context-provider/src/provider.rs (4)
get_quorum_public_key_async(81-86)get_quorum_public_key_async(136-148)get_quorum_public_key_async(186-195)get_quorum_public_key_async(285-295)packages/rs-sdk-trusted-context-provider/src/provider.rs (1)
get_quorum_public_key_async(435-451)packages/rs-sdk/src/mock/provider.rs (1)
get_quorum_public_key_async(190-202)packages/wasm-sdk/src/context_provider.rs (2)
get_quorum_public_key_async(22-36)get_quorum_public_key_async(86-101)
packages/rs-sdk-trusted-context-provider/src/provider.rs (2)
packages/rs-context-provider/src/provider.rs (4)
get_quorum_public_key_async(81-86)get_quorum_public_key_async(136-148)get_quorum_public_key_async(186-195)get_quorum_public_key_async(285-295)packages/rs-sdk-ffi/src/context_callbacks.rs (1)
get_quorum_public_key_async(113-128)
packages/rs-sdk/src/mock/provider.rs (2)
packages/rs-context-provider/src/provider.rs (8)
get_quorum_public_key_async(81-86)get_quorum_public_key_async(136-148)get_quorum_public_key_async(186-195)get_quorum_public_key_async(285-295)get_quorum_public_key(104-109)get_quorum_public_key(150-158)get_quorum_public_key(197-205)get_quorum_public_key(300-332)packages/rs-sdk/src/core/dash_core_client.rs (1)
get_quorum_public_key(127-146)
packages/wasm-sdk/src/context_provider.rs (3)
packages/rs-context-provider/src/provider.rs (4)
get_quorum_public_key_async(81-86)get_quorum_public_key_async(136-148)get_quorum_public_key_async(186-195)get_quorum_public_key_async(285-295)packages/rs-sdk-ffi/src/context_callbacks.rs (1)
get_quorum_public_key_async(113-128)packages/rs-sdk-trusted-context-provider/src/provider.rs (1)
get_quorum_public_key_async(435-451)
packages/rs-context-provider/src/provider.rs (4)
packages/rs-sdk-ffi/src/context_callbacks.rs (1)
get_quorum_public_key_async(113-128)packages/rs-sdk-trusted-context-provider/src/provider.rs (1)
get_quorum_public_key_async(435-451)packages/rs-sdk/src/mock/provider.rs (1)
get_quorum_public_key_async(190-202)packages/wasm-sdk/src/context_provider.rs (2)
get_quorum_public_key_async(22-36)get_quorum_public_key_async(86-101)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Rust packages (rs-sdk-ffi) / Linting
- GitHub Check: Rust packages (rs-sdk-ffi) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (1)
packages/rs-sdk/src/mock/provider.rs (1)
189-211: Async wrapper and sync delegation for gRPC context are consistent
get_quorum_public_key_asyncdelegates toget_quorum_public_key_sync_impland wraps the result in a ready future, and the syncget_quorum_public_keyalso goes through the same helper. This ensures both sync and async paths share identical caching and dump behavior without lifetimes orSendissues.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rs-context-provider/src/provider.rs (1)
65-92: Clarify documentation regarding sync/async implementation pattern.The documentation states that
get_quorum_public_key_asyncis the "primary method" and that the sync version "should typically wrap this method usingdash_sdk::sync::block_on()" (lines 67-69, 90-92). However, all current implementations in this file and across the codebase (see relevant snippets) do the opposite: they wrap the sync version in an async block.Based on comments in the codebase like "For now, we just wrap the synchronous version" and "could be made truly async in the future", this appears to be forward-looking guidance for when true async implementations are available.
Consider clarifying the documentation to acknowledge the current implementation pattern while indicating the intended future direction:
/// Fetches the public key for a specified quorum asynchronously. /// -/// This is the primary method that should be implemented. The synchronous version -/// [get_quorum_public_key](ContextProvider::get_quorum_public_key) should typically -/// wrap this method using `dash_sdk::sync::block_on()`. +/// This is the primary method that should be implemented when true async support is available. +/// Currently, most implementations wrap the synchronous +/// [get_quorum_public_key](ContextProvider::get_quorum_public_key) method in an async block. +/// In the future, when async support is available in the underlying systems, the synchronous +/// version should wrap this method using `dash_sdk::sync::block_on()`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rs-context-provider/src/provider.rs(6 hunks)packages/rs-sdk/src/mock/provider.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-sdk/src/mock/provider.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/rs-context-provider/src/provider.rs
🧠 Learnings (15)
📚 Learning: 2024-11-20T16:05:40.200Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs:148-151
Timestamp: 2024-11-20T16:05:40.200Z
Learning: In `packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs`, when converting public keys from `QuorumForSavingV0` to `VerificationQuorum`, it's acceptable to use `.expect()` for public key conversion, as the conversion has been verified and panics are acceptable in this context.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-10T10:30:25.283Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2232
File: packages/rs-sdk/src/mock/sdk.rs:129-130
Timestamp: 2024-10-10T10:30:25.283Z
Learning: In the codebase `packages/rs-sdk/src/mock/sdk.rs`, `tokio::sync::Mutex` is used instead of `std::sync::Mutex` because `std::sync::MutexGuard` is not `Send` and cannot be used in async contexts.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-22T10:53:12.111Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2259
File: packages/rs-dapi-client/src/dapi_client.rs:137-139
Timestamp: 2024-10-22T10:53:12.111Z
Learning: In `packages/rs-dapi-client/src/dapi_client.rs`, when passing data into asynchronous code, ensure that data structures are `Send + Sync`. Using `Arc<AtomicUsize>` is necessary for the retry counter.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-06T16:11:34.946Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs:19-30
Timestamp: 2024-10-06T16:11:34.946Z
Learning: In the Rust file `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs`, within the `create_owner_identity_v1` function, the `add_public_keys` method of the `Identity` struct cannot fail and does not require explicit error handling.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2025-07-28T20:04:48.458Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/index.html:4360-4416
Timestamp: 2025-07-28T20:04:48.458Z
Learning: In packages/wasm-sdk, the wallet helper `derive_key_from_seed_with_path` (Rust function in src/wallet/key_derivation.rs) is synchronous; its JS wrapper returns a value immediately, so `await` is unnecessary.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2025-01-23T09:23:32.740Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2405
File: packages/rs-sdk/src/sync.rs:88-95
Timestamp: 2025-01-23T09:23:32.740Z
Learning: The `block_on` function in `packages/rs-sdk/src/sync.rs` is currently only used in tests, and its WebAssembly implementation is deferred until there's a user request for it.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2025-08-05T13:55:39.147Z
Learnt from: thephez
Repo: dashpay/platform PR: 2718
File: packages/wasm-sdk/index.html:0-0
Timestamp: 2025-08-05T13:55:39.147Z
Learning: The get_identity_keys_with_proof_info function in the Rust WASM bindings does not support the "search" key request type and lacks the searchPurposeMap parameter. When proof mode is enabled with keyRequestType === 'search', the implementation falls back to the non-proof version (get_identity_keys) to maintain functionality.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2025-01-23T09:23:32.740Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2405
File: packages/rs-sdk/src/sync.rs:88-95
Timestamp: 2025-01-23T09:23:32.740Z
Learning: The `block_on` function in `packages/rs-sdk/src/sync.rs` requires true blocking behavior, which is why a non-blocking WebAssembly implementation using `wasm_bindgen_futures::spawn_local` is not suitable.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-11-22T08:19:14.448Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2345
File: packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs:93-99
Timestamp: 2024-11-22T08:19:14.448Z
Learning: In `packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs`, the `insert_contract` method requires an owned `BlockInfo`, so cloning `block_info` is necessary when calling it.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-06T16:17:34.571Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:105-105
Timestamp: 2024-10-06T16:17:34.571Z
Learning: In `run_block_proposal` in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, when retrieving `last_block_time_ms`, it's acceptable to use `platform_state` instead of `block_platform_state`, even after updating the protocol version.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-04T14:16:05.798Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2207
File: packages/rs-drive-proof-verifier/src/proof.rs:1646-1664
Timestamp: 2024-10-04T14:16:05.798Z
Learning: In the implementation of `FromProof<platform::GetContestedResourceIdentityVotesRequest>` in `packages/rs-drive-proof-verifier/src/proof.rs`, when matching `maybe_votes`, using `.expect()` on `v.into_iter().next()` is acceptable because the prior match arm `Some(v) if v.is_empty()` ensures that the map is not empty, preventing a panic.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-29T14:44:01.184Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-dapi-client/src/executor.rs:38-38
Timestamp: 2024-10-29T14:44:01.184Z
Learning: In `packages/rs-dapi-client/src/executor.rs`, for the structs `ExecutionError<E>` and `ExecutionResponse<R>`, only the serde derives (`Serialize`, `Deserialize`) should be conditional based on the "mocks" feature; the other derives (`Debug`, `Clone`, `Eq`, `PartialEq`) should always be present.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-06T16:18:07.994Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:119-120
Timestamp: 2024-10-06T16:18:07.994Z
Learning: In the `run_block_proposal` function in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, it's acceptable to pass `platform_state` to `perform_events_on_first_block_of_protocol_change`, even if `block_platform_state` has been updated.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-10-10T10:30:19.883Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2232
File: packages/rs-sdk/src/mock/sdk.rs:90-95
Timestamp: 2024-10-10T10:30:19.883Z
Learning: In `packages/rs-sdk/src/mock/sdk.rs`, the `load_expectations` method in `MockDashPlatformSdk` remains asynchronous (`async`) for backward compatibility, even though it now delegates to the synchronous `load_expectations_sync` method.
Applied to files:
packages/rs-context-provider/src/provider.rs
📚 Learning: 2024-11-20T10:01:50.837Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs:94-94
Timestamp: 2024-11-20T10:01:50.837Z
Learning: In `packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs`, when converting with `PublicKey::try_from`, it's acceptable to use `.expect()` to handle potential conversion errors.
Applied to files:
packages/rs-context-provider/src/provider.rs
🧬 Code graph analysis (1)
packages/rs-context-provider/src/provider.rs (4)
packages/rs-sdk/src/mock/provider.rs (1)
get_quorum_public_key_async(190-210)packages/wasm-sdk/src/context_provider.rs (2)
get_quorum_public_key_async(22-36)get_quorum_public_key_async(86-101)packages/rs-sdk-ffi/src/context_callbacks.rs (1)
get_quorum_public_key_async(113-128)packages/rs-sdk-trusted-context-provider/src/provider.rs (1)
get_quorum_public_key_async(435-451)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (4)
packages/rs-context-provider/src/provider.rs (4)
6-6: LGTM! Necessary imports for async support.The addition of
FutureandPinimports is appropriate for the new async trait method.
136-148: LGTM! Proper delegation pattern.The implementation correctly forwards the async call to the underlying provider, consistent with other methods in this impl block.
186-197: LGTM! Sound implementation that addresses previous concerns.The implementation correctly computes the result synchronously under the lock and then wraps it in a future. This avoids the previous soundness issue where a
MutexGuardcould have been held while returning a future with borrowed data. The pattern is consistent with implementations across the codebase.
284-297: LGTM! Consistent mock implementation.The implementation correctly wraps the synchronous version in an async block, consistent with the pattern used across all ContextProvider implementations. The documentation appropriately describes this as a mock implementation.
Have you tried? @lklimek maybe it's not a bad idea to switch to async. From wasm perspective it would be better. rs-sdk I guess too |
|
FFI can handle async btw. |
|
@QuantumExplorer @pauldelucia @lklimek excellent! If you agree, I'll make the context provider async, and it will solve my and @pauldelucia issues. |
| /// | ||
| /// A ContextProvider should be thread-safe and manage timeouts and other concurrency-related issues internally, | ||
| /// as the [FromProof](crate::FromProof) implementations can block on ContextProvider calls. | ||
| pub trait ContextProvider: Send + Sync { |
There was a problem hiding this comment.
I agree with @shumkov to switch to async ContextProvider; however, we quite often use it in sync context, so it's not a trivial change.
I'd suggest the following approach:
- create
#[async_trait] ContextProviderAsync, and move the async code there - create a wrapper ContextProviderSync(ContextProviderAsync) that will implement
ContextProvider + From<ContextProviderAsync> + IntoContextProvider>, and will delegate sync requests to async methods - sdk should require and store ContextProviderAsync, we can add Sdk::context_provider_sync() can still return ContextProvider (using wrapper), so that we don't need to change too much code
I can implement the wrapper if we agree on this approach, just please isolate these two traits.
There was a problem hiding this comment.
Sounds good to me. No worries, I will deal with the wrapper and implementation.
Issue being fixed or feature implemented
In dash-spv, this function is needed for fetching quorum public keys from MasternodeListEngine so we can verify queries and state transitions. This is an asynchronous operation, so the getter function should be async. However, FFI may not be able to use async functions, so we create an additional function
get_quorum_public_keys_asyncforContextProviderand also keep the synchronousget_quorum_public_keys.What was done?
Added
get_quorum_public_keys_asynctoContextProvider.How Has This Been Tested?
DET
Breaking Changes
Is this a breaking change? I guess apps using ContextProvider will need to add the new function.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Release Notes
New Features
API Changes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.