feat(sdk): pre-fetch trusted address list #2874
Conversation
WalkthroughAdds masternode discovery to the trusted provider, a Wasm adapter that converts and returns discovered addresses, caching of discovered mainnet/testnet addresses during prefetch for WasmSdk builders, and consolidation of DPNS proof responses into a single ProofMetadataResponseWasm type. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/wasm-sdk/src/sdk.rs (3)
22-36: Consider propagating mutex errors instead of panicking.Line 34 uses
.unwrap()on mutex lock, which will panic if the mutex is poisoned. While mutex poisoning is rare, panics in WASM can be difficult to debug. Consider using.lock().map_err(...)to convert to aWasmSdkErrorfor consistency with other error handling in this module.- *cache.lock().unwrap() = Some(addresses); + *cache + .lock() + .map_err(|e| WasmSdkError::generic(format!("Failed to acquire address cache lock: {}", e)))? + = Some(addresses);
234-246: Consider returningResultfor consistency and better error handling.
new_mainnet()uses.expect()which will panic ifprefetchTrustedQuorumsMainnet()wasn't called first. Panics in WASM are difficult to debug and crash the entire application. The trusted variant (new_mainnet_trusted) properly returnsResult<Self, WasmSdkError>.Consider changing the signature to
pub fn new_mainnet() -> Result<Self, WasmSdkError>for consistency and to provide users with actionable error messages instead of panics.#[wasm_bindgen(js_name = "mainnet")] - pub fn new_mainnet() -> Self { - let mainnet_addresses = MAINNET_DISCOVERED_ADDRESSES.lock().unwrap().clone().expect( - "Mainnet addresses not prefetched. Call prefetchTrustedQuorumsMainnet() first.", - ); + pub fn new_mainnet() -> Result<Self, WasmSdkError> { + let mainnet_addresses = MAINNET_DISCOVERED_ADDRESSES + .lock() + .map_err(|e| WasmSdkError::generic(format!("Failed to acquire address cache lock: {}", e)))? + .clone() + .ok_or_else(|| { + WasmSdkError::generic( + "Mainnet addresses not prefetched. Call prefetchTrustedQuorumsMainnet() first.", + ) + })?; let address_list = dash_sdk::sdk::AddressList::from_iter(mainnet_addresses); let sdk_builder = SdkBuilder::new(address_list) .with_network(dash_sdk::dpp::dashcore::Network::Dash) .with_context_provider(WasmContext {}); - Self(sdk_builder) + Ok(Self(sdk_builder)) }
281-293: Same issue: consider returningResultfor consistency.Apply the same refactor as suggested for
new_mainnet()to avoid panics in WASM.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/wasm-sdk/src/context_provider.rs(1 hunks)packages/wasm-sdk/src/dpns.rs(3 hunks)packages/wasm-sdk/src/sdk.rs(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Rust code must passcargo clippy --workspacelinter checks
Rust code must be formatted usingcargo fmt --all
**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/wasm-sdk/src/context_provider.rspackages/wasm-sdk/src/sdk.rspackages/wasm-sdk/src/dpns.rs
🧠 Learnings (15)
📓 Common learnings
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2690
File: packages/wasm-sdk/src/queries/document.rs:383-384
Timestamp: 2025-07-10T00:37:09.586Z
Learning: In the Dash platform, DPNS (Dash Platform Name Service) contracts have the same ID across all networks (mainnet, testnet, etc.), so hardcoding the DPNS contract ID is appropriate and doesn't need to be configurable.
Learnt from: shumkov
Repo: dashpay/platform PR: 2206
File: packages/rs-platform-version/src/version/protocol_version.rs:155-157
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the dashmate default configurations, the default protocol version for local networks has been removed, and an update mechanism for the protocol version in the consensus parameters has been implemented.
Learnt from: shumkov
Repo: dashpay/platform PR: 2206
File: packages/rs-platform-version/src/version/protocol_version.rs:155-157
Timestamp: 2024-10-04T09:08:48.470Z
Learning: In the dashmate default configurations, the default protocol version for local networks has been removed, and an update mechanism for the protocol version in the consensus parameters has been implemented.
📚 Learning: 2024-11-28T13:49:17.301Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2317
File: packages/rs-dapi-client/src/address_list.rs:175-180
Timestamp: 2024-11-28T13:49:17.301Z
Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.
Applied to files:
packages/wasm-sdk/src/sdk.rspackages/wasm-sdk/src/dpns.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/wasm-sdk/src/sdk.rs
📚 Learning: 2024-10-18T15:39:51.172Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2254
File: packages/rs-sdk/src/sdk.rs:585-585
Timestamp: 2024-10-18T15:39:51.172Z
Learning: The 'platform' project uses Rust version 1.80, so code in 'packages/rs-sdk' can use features available in Rust 1.80, such as the `abs_diff()` method.
Applied to files:
packages/wasm-sdk/src/sdk.rs
📚 Learning: 2024-12-05T12:59:22.044Z
Learnt from: lklimek
Repo: dashpay/platform PR: 1924
File: packages/rs-dapi-client/src/request_settings.rs:74-74
Timestamp: 2024-12-05T12:59:22.044Z
Learning: The `AppliedRequestSettings` struct in `packages/rs-dapi-client/src/request_settings.rs` is intended for internal use within the monorepo and is not part of the public API.
Applied to files:
packages/wasm-sdk/src/sdk.rs
📚 Learning: 2024-10-30T11:19:59.163Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/tests/fetch/config.rs:233-233
Timestamp: 2024-10-30T11:19:59.163Z
Learning: In the Rust SDK's `rs-sdk/tests` integration tests (e.g., in `packages/rs-sdk/tests/fetch/config.rs`), we cannot create objects during tests because there is no support for object creation in this context. Therefore, hardcoded values for test identities must be used.
Applied to files:
packages/wasm-sdk/src/sdk.rs
📚 Learning: 2025-11-25T13:10:23.481Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use WASM bindings to connect Rust and JavaScript code
Applied to files:
packages/wasm-sdk/src/sdk.rspackages/wasm-sdk/src/dpns.rs
📚 Learning: 2024-10-28T10:38:49.538Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2259
File: packages/rs-dapi-client/src/executor.rs:49-56
Timestamp: 2024-10-28T10:38:49.538Z
Learning: In `packages/rs-dapi-client/src/executor.rs`, the `ExecutionResponse` struct always has a non-optional `address` field of type `Address` because an address is always present when there's a successful response.
Applied to files:
packages/wasm-sdk/src/sdk.rs
📚 Learning: 2025-10-23T13:04:39.160Z
Learnt from: thephez
Repo: dashpay/platform PR: 2816
File: packages/js-dapi-client/lib/networkConfigs.js:17-40
Timestamp: 2025-10-23T13:04:39.160Z
Learning: For the testnet DAPI address whitelist in packages/js-dapi-client/lib/networkConfigs.js, nodes may be intentionally excluded even if they are ENABLED if they are running pre-2.1 versions. Version compatibility is an important filtering criterion beyond just enabled status.
Applied to files:
packages/wasm-sdk/src/sdk.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/dpns.rs
📚 Learning: 2025-09-03T14:42:29.958Z
Learnt from: thephez
Repo: dashpay/platform PR: 2754
File: packages/wasm-sdk/docs.html:1970-1971
Timestamp: 2025-09-03T14:42:29.958Z
Learning: In packages/wasm-sdk/, the docs.html file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in docs.html, as manual changes to docs.html would be overwritten during regeneration.
Applied to files:
packages/wasm-sdk/src/dpns.rs
📚 Learning: 2025-09-03T14:41:16.196Z
Learnt from: thephez
Repo: dashpay/platform PR: 2754
File: packages/wasm-sdk/AI_REFERENCE.md:766-766
Timestamp: 2025-09-03T14:41:16.196Z
Learning: In packages/wasm-sdk/, the AI_REFERENCE.md file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in AI_REFERENCE.md, as manual changes to AI_REFERENCE.md would be overwritten during regeneration.
Applied to files:
packages/wasm-sdk/src/dpns.rs
📚 Learning: 2025-07-28T20:00:08.502Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/AI_REFERENCE.md:771-783
Timestamp: 2025-07-28T20:00:08.502Z
Learning: In packages/wasm-sdk/AI_REFERENCE.md, the documentation correctly shows the actual SDK method signatures (including identityCreate and identityTopUp with their full parameter lists), which may differ from the UI inputs shown in fixed_definitions.json. The UI may collect fewer parameters from users while handling additional requirements internally.
Applied to files:
packages/wasm-sdk/src/dpns.rs
📚 Learning: 2025-02-10T11:26:36.709Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2405
File: packages/wasm-sdk/src/verify.rs:26-68
Timestamp: 2025-02-10T11:26:36.709Z
Learning: In the wasm-sdk package, empty vectors and placeholder values are intentionally used in verification functions during the proof-of-concept stage to ensure proper compilation and type checking.
Applied to files:
packages/wasm-sdk/src/dpns.rs
📚 Learning: 2025-09-03T19:33:21.688Z
Learnt from: thephez
Repo: dashpay/platform PR: 2754
File: packages/wasm-sdk/api-definitions.json:1285-1285
Timestamp: 2025-09-03T19:33:21.688Z
Learning: In packages/wasm-sdk/api-definitions.json, thephez prefers to keep the existing "ripemd160hash20bytes1234" placeholder for ECDSA_HASH160 data field in documentation examples rather than using a valid base64-encoded format, maintaining consistency with the previous documentation approach.
Applied to files:
packages/wasm-sdk/src/dpns.rs
🧬 Code graph analysis (3)
packages/wasm-sdk/src/context_provider.rs (2)
packages/rs-sdk/src/sdk.rs (1)
new(867-872)packages/rs-dapi-client/src/address_list.rs (3)
uri(65-67)try_from(52-60)from_iter(274-281)
packages/wasm-sdk/src/sdk.rs (2)
packages/rs-dapi-client/src/dapi_client.rs (1)
address_list(121-123)packages/wasm-sdk/src/error.rs (1)
generic(69-71)
packages/wasm-sdk/src/dpns.rs (1)
packages/wasm-sdk/src/queries/mod.rs (5)
metadata(242-244)proof(252-254)from_parts(263-273)from(89-98)from(195-204)
⏰ 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). (7)
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (8)
packages/wasm-sdk/src/context_provider.rs (1)
137-158: LGTM! Clean implementation of masternode address fetching.The method correctly:
- Fetches URLs from the inner provider
- Parses each URL to a
Uriusingfrom_maybe_shared- Converts each
Urito anAddressviaTryFrom- Collects addresses into an
AddressListusingfrom_iterError messages are descriptive and include the problematic URL for debugging.
packages/wasm-sdk/src/sdk.rs (3)
17-20: LGTM! Static caches for discovered addresses.The lazy-initialized mutex-protected caches are appropriate for storing prefetched masternode addresses that can be shared across SDK builder invocations.
86-104: LGTM! Address fetching integrated into prefetch flow.The integration is well-placed: addresses are fetched after quorum prefetch succeeds, and before storing the context. If address fetching fails, the entire prefetch operation fails cleanly, allowing the user to retry.
263-271: LGTM! Proper error handling for missing prefetched addresses.Using
ok_or_elsewith a descriptive error message is the right approach. The error clearly instructs users to call the prefetch method first.packages/wasm-sdk/src/dpns.rs (4)
3-3: LGTM! Simplified imports for unified response type.The import cleanup removes the now-unused DPNS-specific response types in favor of the unified
ProofMetadataResponseWasm.
167-184: LGTM! Clean migration to unified response type.The method now returns
ProofMetadataResponseWasmusingfrom_parts(), correctly passing the usernames array as aJsValueand converting metadata/proof to their WASM wrapper types.
462-473: LGTM! Proper TypeScript typing for the unified response.The
unchecked_return_typeattribute correctly specifiesProofMetadataResponseTyped<Array<string>>which matches the data payload being returned.
475-501: LGTM! Correctly extracts single username from array response.The implementation cleverly reuses
fetch_dpns_usernames_with_proof, then modifies the response data to return just the first username (or null). Theunchecked_return_type = "ProofMetadataResponseTyped<string | null>"accurately reflects the return type. Theset_datamethod is available onProofMetadataResponseWasmand properly handles the type conversion.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rs-sdk-trusted-context-provider/src/provider.rs (1)
246-308: Discovery logic is solid; consider hardening host parsing and JSON decodingThe overall flow (call
/masternodes, enforce HTTP success +success == true, filter forENABLED+version_check == "success", and construct HTTPS DAPI URLs with network-specific ports) looks correct and appropriately strict.Two non-blocking improvements you might consider:
- More robust host extraction from
entry.addressUsing
rsplit_once(':')assumesaddressis always an IPv4-style"host:port". If this ever carries IPv6 or more complex forms, this could misbehave. You can instead parse a synthetic URL to extract the host robustly:- let body = response.text().await?; - let parsed: MasternodeDiscoveryResponse = serde_json::from_str(&body)?; + let parsed: MasternodeDiscoveryResponse = response.json().await?; @@ - let mut addresses = Vec::new(); - for entry in parsed - .data - .into_iter() - .filter(|m| m.status == "ENABLED" && m.version_check.as_deref() == Some("success")) - { - let host_port = entry.address; - let host = host_port - .rsplit_once(':') - .map(|(h, _)| h) - .unwrap_or(host_port.as_str()); - let https_url = format!("https://{}:{}", host, dapi_port); + let mut addresses = Vec::new(); + for entry in parsed + .data + .into_iter() + .filter(|m| m.status == "ENABLED" && m.version_check.as_deref() == Some("success")) + { + let host_port = entry.address; + let synthetic = format!("scheme://{}", host_port); + let parsed_host = Url::parse(&synthetic).map_err(|e| { + TrustedContextProviderError::NetworkError(format!( + "Invalid masternode address '{}': {}", + host_port, e + )) + })?; + let host = parsed_host.host_str().ok_or_else(|| { + TrustedContextProviderError::NetworkError(format!( + "Masternode address '{}' has no host", + host_port + )) + })?; + let https_url = format!("https://{}:{}", host, dapi_port); let url = url::Url::parse(&https_url).map_err(|e| { TrustedContextProviderError::NetworkError(format!( "Invalid masternode URL '{}': {}", https_url, e )) })?; addresses.push(url); }
- Use
response.json()directlyAs shown in the diff,
response.json::<MasternodeDiscoveryResponse>().await?is a bit simpler thantext()+serde_json::from_str, and keeps deserialization errors clearly associated with the HTTP response.Both are incremental improvements; the current implementation is functionally fine as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rs-sdk-trusted-context-provider/src/provider.rs(3 hunks)packages/wasm-sdk/src/context_provider.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Rust code must passcargo clippy --workspacelinter checks
Rust code must be formatted usingcargo fmt --all
**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/wasm-sdk/src/context_provider.rspackages/rs-sdk-trusted-context-provider/src/provider.rs
🧬 Code graph analysis (2)
packages/wasm-sdk/src/context_provider.rs (1)
packages/rs-dapi-client/src/address_list.rs (3)
uri(65-67)try_from(52-60)from_iter(274-281)
packages/rs-sdk-trusted-context-provider/src/provider.rs (1)
packages/wasm-sdk/src/context_provider.rs (1)
fetch_masternode_addresses(137-159)
⏰ 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). (7)
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (2)
packages/wasm-sdk/src/context_provider.rs (1)
137-159: WASM masternode address conversion is consistent and well-guardedThe flow from discovered URLs →
Uri→rs_dapi_client::Address→AddressListis clean, and the error messages keep the original URL in context, which will be useful for debugging malformed entries. No issues from a correctness or ergonomics standpoint on this path.packages/rs-sdk-trusted-context-provider/src/provider.rs (1)
27-80: Masternode discovery models correctly capture response shape
MasternodeEntry/MasternodeDiscoveryResponselook appropriate for a{ success, data: [...] }response, and theversionCheck→version_checkrename plusOption<String>typing is a good fit for potentially missing or failing checks. DerivingDebug/Deserializehere is reasonable and should keep serde integration straightforward.
|
✅ 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: "0e94a349ea1e66f46750967469acc010a55cdae5778fcaa8218b97d40ec70249"
)Xcode manual integration:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/wasm-sdk/src/sdk.rs (4)
18-72: Global address caches and helpers look correct; a couple of optional robustness tweaksThe new
MAINNET_DISCOVERED_ADDRESSES/TESTNET_DISCOVERED_ADDRESSESstatics plusparse_addresses,default_*_addresses, andfetch_and_cache_addressesform a coherent flow:
parse_addressescorrectly reusesUriparsing andAddress::try_fromso validation stays centralized.- Default mainnet/testnet lists are clearly separated and only used as fallback.
fetch_and_cache_addressescleanly converts the(addr, _status)tuples into a cachedVec<Address>behind aMutex<Option<_>>, consistent with existing*_TRUSTED_CONTEXTstatics. As per coding guidelines, naming and 4‑space indent look good.Two optional improvements to consider:
- Status filtering: if
fetch_masternode_addressescan return entries that should not be used (e.g., disabled/banned, wrong version), you might want to filter by_statushere instead of ignoring it, to avoid polluting the cache with unusable nodes.- Empty fallback visibility: if parsing ever yielded an empty
Vec<Address>(e.g., due to a typo in the hardcoded seeds), this would silently result in an emptyAddressList. A debug log or assertion whendefault_*_addresses()returns empty could make such misconfigurations easier to catch during development.These are non-blocking; the current implementation is functionally sound.
122-160:prefetchTrustedQuorums*now also fail on masternode discovery errors—confirm intended behaviorThe new calls to:
fetch_and_cache_addresses(&trusted_context, &MAINNET_DISCOVERED_ADDRESSES)inprefetch_trusted_quorums_mainnet, andfetch_and_cache_addresses(&trusted_context, &TESTNET_DISCOVERED_ADDRESSES)inprefetch_trusted_quorums_testnetmean that these methods now return an error not only when quorum prefetching fails, but also when masternode address discovery fails.
This tightens the contract: a successful
prefetchTrustedQuorums*now implies both quorums and address caches are populated, but it also introduces new failure modes (e.g., iffetch_masternode_addressesis unsupported on some nodes or is more fragile than quorum prefetch).If backward compatibility of “quorums prefetch rarely fails” is important, you might consider treating address-discovery failure as a soft failure (log + keep existing behavior) rather than bubbling it up, or at least documenting this stronger error condition for callers.
270-313: Mainnet builders correctly prefer discovered addresses with sane fallback; small DRY opportunityFor both:
new_mainnet, andnew_mainnet_trustedthe pattern:
let mainnet_addresses = MAINNET_DISCOVERED_ADDRESSES .lock() .unwrap() .clone() .unwrap_or_else(default_mainnet_addresses);is correct and nicely ensures:
- Use of the discovered address cache when
prefetchTrustedQuorumsMainnethas run.- Fallback to the trimmed hardcoded list when the cache is empty or never populated.
This keeps behavior deterministic and avoids blocking network calls inside the builders. As per coding guidelines, the static name and indentation are also in line with expectations.
Optional refactor: the
MAINNET_DISCOVERED_ADDRESSESaccess + fallback appears twice; extracting a small helper likefn mainnet_addresses() -> Vec<Address>would reduce duplication and keep mainnet behavior in one place. Not required for correctness.
315-358: Testnet builders mirror mainnet logic correctly; consider shared helper for symmetrySimilarly, in:
new_testnet, andnew_testnet_trustedthe use of
TESTNET_DISCOVERED_ADDRESSESwithunwrap_or_else(default_testnet_addresses)mirrors mainnet behavior and looks correct:
- Prefetched addresses are used when available.
- The hardcoded testnet seeds are used only as a fallback.
This symmetry with mainnet is good for maintainability and caller expectations.
As with mainnet, you could optionally DRY this into a
fn testnet_addresses() -> Vec<Address>helper (and/or a genericfn addresses_for(cache, default_fn)) to centralize address-selection logic, but the current duplication is small and acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/wasm-sdk/src/sdk.rs(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Rust code must passcargo clippy --workspacelinter checks
Rust code must be formatted usingcargo fmt --all
**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/wasm-sdk/src/sdk.rs
🧠 Learnings (5)
📚 Learning: 2024-11-28T13:49:17.301Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2317
File: packages/rs-dapi-client/src/address_list.rs:175-180
Timestamp: 2024-11-28T13:49:17.301Z
Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.
Applied to files:
packages/wasm-sdk/src/sdk.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/wasm-sdk/src/sdk.rs
📚 Learning: 2024-10-30T11:19:59.163Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/tests/fetch/config.rs:233-233
Timestamp: 2024-10-30T11:19:59.163Z
Learning: In the Rust SDK's `rs-sdk/tests` integration tests (e.g., in `packages/rs-sdk/tests/fetch/config.rs`), we cannot create objects during tests because there is no support for object creation in this context. Therefore, hardcoded values for test identities must be used.
Applied to files:
packages/wasm-sdk/src/sdk.rs
📚 Learning: 2024-10-28T10:38:49.538Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2259
File: packages/rs-dapi-client/src/executor.rs:49-56
Timestamp: 2024-10-28T10:38:49.538Z
Learning: In `packages/rs-dapi-client/src/executor.rs`, the `ExecutionResponse` struct always has a non-optional `address` field of type `Address` because an address is always present when there's a successful response.
Applied to files:
packages/wasm-sdk/src/sdk.rs
📚 Learning: 2025-10-23T13:04:39.160Z
Learnt from: thephez
Repo: dashpay/platform PR: 2816
File: packages/js-dapi-client/lib/networkConfigs.js:17-40
Timestamp: 2025-10-23T13:04:39.160Z
Learning: For the testnet DAPI address whitelist in packages/js-dapi-client/lib/networkConfigs.js, nodes may be intentionally excluded even if they are ENABLED if they are running pre-2.1 versions. Version compatibility is an important filtering criterion beyond just enabled status.
Applied to files:
packages/wasm-sdk/src/sdk.rs
🧬 Code graph analysis (1)
packages/wasm-sdk/src/sdk.rs (2)
packages/rs-dapi-client/src/address_list.rs (2)
uri(65-67)try_from(52-60)packages/rs-dapi-client/src/dapi_client.rs (1)
address_list(121-123)
⏰ 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). (8)
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (1)
packages/wasm-sdk/src/sdk.rs (1)
4-8: Address/Uri imports align with new usageTop-level
UriandAddressimports match their usage inparse_addressesandnew_with_addresses, and naming/formatting is consistent with the Rust guidelines for this repo.
Issue being fixed or feature implemented
Hardcoded list of evonodes is outdated. Since we already pre-fetch trusted list of quorums we can do same with evo nodes.
What was done?
How Has This Been Tested?
With existing tests
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
API Changes
✏️ Tip: You can customize this high-level summary in your review settings.