-
Notifications
You must be signed in to change notification settings - Fork 44
refactor(sdk)!: cleanup JS SDK params and return types #2858
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.
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
base: v2.2-dev
Are you sure you want to change the base?
Conversation
WalkthroughPublic API and wasm-bindings refactor: identifier parameters replaced with Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant JSFacade as JS Facade
participant WasmSdk as Wasm SDK
rect `#F0F8FF`
note right of JSFacade: Before — generic inputs/outputs
User->>JSFacade: fetch(contractId: "abc")
JSFacade->>WasmSdk: get_data_contract("abc")
WasmSdk-->>JSFacade: JsValue (generic)
JSFacade-->>User: Promise<any>
end
rect `#F6FFF0`
note right of JSFacade: After — IdentifierLike + typed proof
User->>JSFacade: fetch(contractId: IdentifierLike)
JSFacade->>WasmSdk: get_data_contract(contractId)
WasmSdk-->>JSFacade: wasm.DataContract / ProofMetadataResponseTyped<...>
JSFacade-->>User: Promise<wasm.DataContract | ProofMetadataResponseTyped<...>>
end
sequenceDiagram
actor User
participant DocsFacade as Documents Facade
participant Parser as Query Parser
participant WasmSdk as Wasm SDK
User->>DocsFacade: query(query: wasm.DocumentsQuery)
DocsFacade->>Parser: parse_documents_query(query)
Parser-->>DocsFacade: DocumentsQueryParsed
DocsFacade->>WasmSdk: get_documents(parsed)
WasmSdk-->>DocsFacade: Map<wasm.Identifier, wasm.Document | undefined>
DocsFacade-->>User: Promise<Map<wasm.Identifier, wasm.Document | undefined>>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to inspect closely:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 (2)**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
packages/wasm-sdk/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (10)📓 Common learnings📚 Learning: 2025-07-28T20:04:48.458ZApplied to files:
📚 Learning: 2025-09-24T05:16:54.422ZApplied to files:
📚 Learning: 2025-07-28T20:00:08.502ZApplied to files:
📚 Learning: 2025-09-03T19:33:21.688ZApplied to files:
📚 Learning: 2024-11-20T20:43:41.185ZApplied to files:
📚 Learning: 2025-09-03T14:41:16.196ZApplied to files:
📚 Learning: 2025-09-03T15:44:33.889ZApplied to files:
📚 Learning: 2025-09-03T14:42:29.958ZApplied to files:
📚 Learning: 2025-07-28T20:00:24.323ZApplied to files:
🧬 Code graph analysis (1)packages/wasm-sdk/src/wallet/key_derivation.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). (7)
🔇 Additional comments (7)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/wasm-sdk/src/queries/voting/voters.rs (1)
24-74: Align non-proof voters return type with actual JS values
get_contested_resource_voters_for_identitydeclaresunchecked_return_type = "Array<string>", but the implementation pushesIdentifierWasminstances into theArray. The proof variant correctly returnsArray<string>(base58) in its data payload.Either the TS type or the implementation should be adjusted; otherwise, consumers will see a typed
string[]but getIdentifierobjects at runtime.If you want both methods to return base58 strings (which matches the new
unchecked_return_type), consider something like:- let array = Array::new(); - for voter in voters.0.into_iter() { - let identifier_js = IdentifierWasm::from(voter.0); - array.push(&JsValue::from(identifier_js)); - } + let array = Array::new(); + for voter in voters.0.into_iter() { + let base58 = voter.0.to_string(Encoding::Base58); + array.push(&JsValue::from_str(&base58)); + }This keeps the runtime result consistent with
Array<string>and with the proof version’s payload shape.Also applies to: 162-185
packages/wasm-sdk/src/dpns.rs (1)
252-305: Avoid clobbering concurrent preorder callbacksStoring the JS callback in the
PREORDER_CALLBACKthread-local means every invocation shares the same slot. If twodpns_register_namecalls overlap (common with async/await), the second call overwrites the first callback, so when the first preorder finishes it either fires the wrong handler or none at all. That’s a correctness bug for concurrent usage. Please keep the callback scoped to the individual request—e.g., move the clonedjs_sys::Functioninto theFnOnceyou pass toRegisterDpnsNameInput(wrapping it in aSendhelper if needed) so each invocation retains its own handler instead of relying on shared global state.
🧹 Nitpick comments (19)
packages/wasm-sdk/src/state_transitions/contracts/mod.rs (2)
24-35: Update doc comments to reflect JS-facing parameter names.The doc comments reference Rust-style snake_case parameter names (
owner_id,contract_definition,private_key_wif,key_id), but JavaScript users will see camelCase names (ownerId,contractDefinition,privateKeyWif,keyId). Since this function is exposed via wasm_bindgen, the documentation should reflect the actual JS API surface that users will interact with.Consider updating the parameter names in the doc comments:
/// # Arguments /// -/// * `owner_id` - The identity ID that will own the contract -/// * `contract_definition` - JSON string containing the contract definition -/// * `private_key_wif` - The private key in WIF format for signing -/// * `key_id` - Optional key ID to use for signing (if None, will auto-select) +/// * `ownerId` - The identity ID that will own the contract +/// * `contractDefinition` - JSON string containing the contract definition +/// * `privateKeyWif` - The private key in WIF format for signing +/// * `keyId` - Optional key ID to use for signing (if None, will auto-select)
193-205: Update doc comments to reflect JS-facing parameter names.Similar to
contract_create, these doc comments use snake_case parameter names that don't match the camelCase names exposed to JavaScript users.Consider updating the parameter names in the doc comments:
/// # Arguments /// -/// * `contract_id` - The ID of the contract to update -/// * `owner_id` - The identity ID that owns the contract -/// * `contract_updates` - JSON string containing the updated contract definition -/// * `private_key_wif` - The private key in WIF format for signing -/// * `key_id` - Optional key ID to use for signing (if None, will auto-select) +/// * `contractId` - The ID of the contract to update +/// * `ownerId` - The identity ID that owns the contract +/// * `contractUpdates` - JSON string containing the updated contract definition +/// * `privateKeyWif` - The private key in WIF format for signing +/// * `keyId` - Optional key ID to use for signing (if None, will auto-select)packages/wasm-sdk/src/state_transitions/documents/mod.rs (3)
111-112: Minor: Inconsistent log formatting.The target and field assignments are split across lines in an unusual way. While functional, this differs from standard Rust formatting conventions.
Consider formatting consistently:
- target = "wasm_sdk", pubkey = % hex::encode(& public_key_bytes), hash160 = % - hex::encode(& public_key_hash160), "Looking for authentication key" + target = "wasm_sdk", + pubkey = %hex::encode(&public_key_bytes), + hash160 = %hex::encode(&public_key_hash160), + "Looking for authentication key"
346-347: Minor: Inconsistent log formatting.Similar to Line 111-112, the field assignments are split in an unusual way.
Consider:
- target = "wasm_sdk", id = % doc_id.to_string(Encoding::Base58), - present = maybe_doc.is_some(), "Document entry" + target = "wasm_sdk", + id = %doc_id.to_string(Encoding::Base58), + present = maybe_doc.is_some(), + "Document entry"
1291-1298: Minor: Inconsistent formatting in fallback branch.The match arm formatting is more verbose than necessary.
Consider:
- _ => Self::build_js_result_object( - "DocumentPurchased", - &document_id_base58, - vec![ - ("status", JsValue::from_str("success")), - ("message", JsValue::from_str("Document purchase processed")), - ], - ), // Purchase was processed but document not returned + _ => { + // Purchase was processed but document not returned + Self::build_js_result_object( + "DocumentPurchased", + &document_id_base58, + vec![ + ("status", JsValue::from_str("success")), + ("message", JsValue::from_str("Document purchase processed")), + ], + ) + }packages/wasm-sdk/src/queries/voting/resources.rs (1)
207-226: Return-type hints for getContestedResources accurately describe the JS payloads*The unchecked return types
Array<any>andProofMetadataResponseTyped<Array<any>>match the fact that you serialize eachContestedResourceentry into a plain JS value and optionally wrap the result inProofMetadataResponseWasm. If you later introduce a dedicated TS shape for contested resources, these strings will be a good place to tighten the typings, but they’re fine as-is for this refactor.packages/wasm-sdk/src/queries/voting/votes.rs (1)
195-225: unchecked_return_type for identity votes matches serialized JS structuresDeclaring
Array<any>forgetContestedResourceIdentityVotesandProofMetadataResponseTyped<{ votes: Array<any> }>for the proof variant correctly mirrors theresource_votes_to_jsonoutput and the{ votes }wrapper passed intoProofMetadataResponseWasm::from_sdk_parts. As with contested resources, you could later replaceanywith a dedicated vote DTO type, but it’s not required for this cleanup.packages/wasm-sdk/src/wallet/key_generation.rs (1)
107-174: WIF/hex helpers match new KeyPairWasm API and network handlingThe updates to
generateKeyPairs,keyPairFromWif, andkeyPairFromHexcorrectly reusebuild_key_pair, enforce sensible bounds oncount, validate input length/format for hex, and surface network as"mainnet"/"testnet"consistently in the JS-facingnetworkfield.If you ever want to reduce duplication, you could factor the
"mainnet"/"testnet"string-to-Networkand label logic into a small helper shared across these methods.packages/wasm-sdk/src/wallet/extended_derivation.rs (1)
261-309: deriveDashpayContactKey builds the DIP15 path from identifiers and reuses common derivationThe DashPay contact key derivation:
- Parses the options object (including
IdentifierLikeidentity IDs) via serde/IdentifierWasm.- Converts identity IDs to hex (for embedding in the path) and computes
coin_typefrom network.- Builds the DIP15 path
m/9'/coin_type'/15'/account'/0x<sender>/0x<receiver>/<index>and logs it.- Reuses
derive_common_from_mnemonicand wraps the result inDashpayContactKeyInfoWasm.This aligns with DIP14/15 conventions and avoids duplicating low-level derivation logic.
If you anticipate more DIP-style paths later, consider a small helper to build the
"m/..."string segments to centralize formatting rules.packages/wasm-sdk/src/queries/token.rs (2)
102-138: calculateTokenIdFromContract binding matches IdentifierLike input and string outputThe parameter annotations (
contractIdasIdentifier | Uint8Array | stringandtokenPositionas a JS camelCase number) align with usingidentifier_from_jsand au16token position. Returning base58 viaIdentifier::to_string(Encoding::Base58)is consistent with other ID-returning helpers.Minor note: the example in the doc comment still shows
calculateTokenId(...)instead ofcalculateTokenIdFromContract(...); consider updating it when convenient to avoid confusion.
450-649: Last-claim decoding (plain and with proof) is thorough and defensiveFor
getTokenPerpetualDistributionLastClaim:
- Direct gRPC usage with
prove: falseis explicit to avoid proof-verification issues.- The match on the response handles all documented
PaidAtvariants:
TimestampMs→(timestamp_ms, 0)BlockHeight→(0, height)Epoch→(0, epoch as u64)RawBytes→ decodes 8-byte big-endian[timestamp_secs, height]with:
- sanity checks on timestamp (
0or ≥ 1609459200) and height (>= 1),- warnings and clamping when data looks invalid, and
- a 4-byte fallback for block-height-only encodings with logging for short lengths.
- It explicitly errors if a Proof result is returned despite
prove: falseand when the response version is missing.The
WithProofInfovariant usesRewardDistributionMomentand converts its enum variants into the same(timestamp_ms, height)shape before wrapping inTokenLastClaimWasm, which is then placed intoProofMetadataResponseWasm.Overall, this is a solid, defensive interpretation of server-side encodings while keeping the JS-visible type stable.
If server-side encoding rules for
RawBytesevolve, consider centralizing the decode/validate logic into a small helper to keep both this function and any future callers in sync.Also applies to: 978-1039
packages/wasm-sdk/src/queries/mod.rs (1)
215-218: Typed TS alias for proof responses matches the new wrapper pattern
ProofMetadataResponseTyped<T> = ProofMetadataResponse & { data: T }is a clean way to express typed payloads on top of the existingProofMetadataResponseJS class and aligns with how the facades now type proof-wrapped results. Please just ensure this file has been run throughcargo fmt/Clippy with the rest of the crate.packages/js-evo-sdk/src/epoch/facade.ts (1)
39-57: Consider tightening proof payload generics for evonode proposed blocksThe non-proof calls return
Map<wasm.Identifier, bigint>, but the proof variants are typed aswasm.ProofMetadataResponseTyped<unknown>. If the underlying wasm methods also return aMap<Identifier, bigint>inside the proof wrapper, consider updating these to:
Promise<wasm.ProofMetadataResponseTyped<Map<wasm.Identifier, bigint>>>for both
evonodesProposedBlocksByIdsWithProofandevonodesProposedBlocksByRangeWithProofto preserve type information end‑to‑end.packages/js-evo-sdk/src/dpns/facade.ts (1)
31-40: DPNS resolve/register shapes look good; narrow the callback typeUpdating
resolveNameto returnPromise<string | undefined>andregisterNameto acceptidentityId: wasm.IdentifierLikeand returnwasm.RegisterDpnsNameResultmatches the wasm layer and improves type safety. The only refinement I’d suggest is tighteningonPreorder?: Functionto something likeonPreorder?: () => void(or a more specific signature if it receives args) to avoid the very looseFunctiontype.packages/js-evo-sdk/src/voting/facade.ts (2)
9-39: Voting query methods align with wasm types; consider refining identity vote payloads
contestedResourceVoteState*andvotePollsByEndDate*now use the wasm query/response types directly, which is a big win for type safety and matches the pattern used elsewhere in the SDK. ForcontestedResourceIdentityVotesand its proof variant, if the wasm bindings expose a concrete vote entry type, consider replacingany[]/{ votes: Array<any> }with that specific type to avoid leakinganyinto consumers.
41-46: Masternode vote typing and JSON serialization could be tightenedThe
masternodeVotearg object and call wiring look correct, but there are a couple of potential improvements:
- Narrow
indexValues: string | any[]to something likestring | unknown[]or a dedicated union type that reflects whatasJsonStringcan handle.- If
w.masternodeVotereturns a known wasm result type, returning that instead ofPromise<any>would strengthen the facade’s contract.- The non-null assertion on
asJsonString(indexValues)!assumes serialization can never fail; if failure is possible, consider explicit error handling (e.g., throwing an argument error) instead of relying on!.These are refinements only; the current behavior is consistent with the underlying call.
packages/wasm-sdk/src/queries/document.rs (2)
289-302: Identifier autodetection in JSON where clauses works; update the TODO for clarityThe new
JsonValue::Stringbranch that treats 44‑character alphanumeric strings as potential base58Identifiers and falls back toValue::Texton parse failure is a reasonable heuristic for document IDs in queries. The existing TODO comment (“Should use Identifier::try_from and return text if failed”) is now slightly misleading, since you already attempt a parse and gracefully fall back. Consider rewording or removing the TODO to reflect the current behavior and the rationale for the length/character heuristic.
321-354: getDocuments proof wrapper wiring is correct; consider reusing map construction and using undefined*Both
get_documentsandget_documents_with_proof_infoconstruct the sameMap<IdentifierWasm, DocumentWasm | null>from the fetched results, and the proof variant then wraps it inProofMetadataResponseWasm::from_parts. This wiring matches the newunchecked_return_typestrings and theProofMetadataResponseTyped<Map<Identifier, Document | undefined>>TS alias.Two potential cleanups:
- Factor out the shared “build
documents_mapfromDocuments” loop into a small helper to keep the two methods in sync.- Today
Nonedocuments are stored asJsValue::NULL, while the TS type advertisesDocument | undefined; consider switching toJsValue::UNDEFINED(or equivalent) to better match the declaredundefinedshape.These are non‑blocking and can be done incrementally.
Also applies to: 356-394
packages/js-evo-sdk/src/tokens/facade.ts (1)
114-172: Consider more specific return types for transition methods.All transition methods currently return
Promise<any>, which bypasses TypeScript's type checking. If the underlying wasm SDK methods have known return types, consider using them for better type safety and developer experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/wasm-sdk/src/dpns.rs (1)
122-197: Remove duplicate code.Lines 122-197 are exact duplicates of lines 45-120. This will cause compilation errors due to duplicate definitions of:
DEFAULT_DPNS_USERNAMES_LIMIT(line 45 and 122)resolve_dpns_usernames_limit(line 47 and 124)usernames_from_documents(line 54 and 131)DPNS_USERNAMES_QUERY_TS(line 73 and 150)DpnsUsernamesQueryJs(line 94 and 171)DpnsUsernamesQueryInput(line 99 and 176)DpnsUsernamesQueryParsed(line 105 and 182)parse_dpns_usernames_query(line 110 and 187)Apply this diff to remove the duplicate code:
-const DEFAULT_DPNS_USERNAMES_LIMIT: u32 = 10; - -fn resolve_dpns_usernames_limit(limit: Option<u32>) -> u32 { - match limit { - Some(0) | None => DEFAULT_DPNS_USERNAMES_LIMIT, - Some(value) => value, - } -} - -fn usernames_from_documents(documents_result: Documents) -> Array { - let usernames_array = Array::new(); - for (_, doc_opt) in documents_result { - if let Some(doc) = doc_opt { - let properties = doc.properties(); - if let (Some(Value::Text(label)), Some(Value::Text(parent_domain))) = ( - properties.get("label"), - properties.get("normalizedParentDomainName"), - ) { - let username = format!("{}.{}", label, parent_domain); - usernames_array.push(&JsValue::from(username)); - } - } - } - usernames_array -} - -// TS definition for DpnsUsernamesQuery used by getDpnsUsernames* -#[wasm_bindgen(typescript_custom_section)] -const DPNS_USERNAMES_QUERY_TS: &'static str = r#" -/** - * Query parameters for retrieving DPNS usernames. - */ -export interface DpnsUsernamesQuery { - /** - * Identity to fetch usernames for. - */ - identityId: IdentifierLike; - - /** - * Maximum number of usernames to return. Use 0 for default. - * @default 10 - */ - limit?: number; -} -"#; - -#[wasm_bindgen] -extern "C" { - #[wasm_bindgen(typescript_type = "DpnsUsernamesQuery")] - pub type DpnsUsernamesQueryJs; -} - -#[derive(Deserialize)] -#[serde(rename_all = "camelCase")] -struct DpnsUsernamesQueryInput { - identity_id: IdentifierWasm, - #[serde(default)] - limit: Option<u32>, -} - -struct DpnsUsernamesQueryParsed { - identity_id: Identifier, - limit: Option<u32>, -} - -fn parse_dpns_usernames_query( - query: DpnsUsernamesQueryJs, -) -> Result<DpnsUsernamesQueryParsed, WasmSdkError> { - let input: DpnsUsernamesQueryInput = - deserialize_required_query(query, "Query object is required", "DPNS usernames query")?; - - Ok(DpnsUsernamesQueryParsed { - identity_id: input.identity_id.into(), - limit: input.limit, - }) -} - #[wasm_bindgen(js_name = "DpnsUsernamesProofResponse")]packages/wasm-sdk/src/queries/identity.rs (3)
198-283: Remove duplicate TypeScript custom section and struct definitions.Lines 198-220 and 241-263 contain identical
typescript_custom_sectiondefinitions forIdentitiesContractKeysQuery. Lines 221-240 and 264-283 contain identical struct definitions for the query types. This duplication should be removed.Apply this diff to remove the duplicate definitions:
-#[wasm_bindgen(typescript_custom_section)] -const IDENTITIES_CONTRACT_KEYS_QUERY_TS: &'static str = r#" -/** - * Query parameters for fetching identities' public keys for a contract. - */ -export interface IdentitiesContractKeysQuery { - /** - * Identity identifiers to fetch keys for. - */ - identityIds: Array<IdentifierLike>; - - /** - * Data contract identifier (reserved for future filtering). - */ - contractId: IdentifierLike; - - /** - * Optional list of purposes to include. - * @default undefined - */ - purposes?: number[]; -} -"#; -#[wasm_bindgen] -extern "C" { - #[wasm_bindgen(typescript_type = "IdentitiesContractKeysQuery")] - pub type IdentitiesContractKeysQueryJs; -} -#[derive(Deserialize)] -#[serde(rename_all = "camelCase")] -struct IdentitiesContractKeysQueryInput { - #[serde(rename = "identityIds")] - identity_ids: Vec<IdentifierWasm>, - #[serde(rename = "contractId")] - contract_id: IdentifierWasm, - #[serde(default)] - purposes: Option<Vec<u32>>, -} -struct IdentitiesContractKeysQueryParsed { - identity_ids: Vec<Identifier>, - contract_id: Identifier, - purposes: Option<Vec<u32>>, -} #[wasm_bindgen(typescript_custom_section)] const IDENTITIES_CONTRACT_KEYS_QUERY_TS: &'static str = r#"
1410-1496: Document or fix the proof limitation for multi-identity queries.The TODO on line 1410 and the implementation (lines 1457-1460) reveal that only the first identity's proof and metadata are returned when querying multiple identities. This is misleading—callers may assume the proof covers all returned data. Additionally, if no identities have keys,
unwrap_or_default()returns empty proof/metadata rather than an error.Consider either:
- Returning an error when multiple identities are requested with proof
- Properly combining/aggregating proofs for all identities
- Documenting this limitation prominently in the TypeScript interface
932-1000: Document thecontractIdparameter's reserved status or add validation.The
contractIdparameter is parsed but intentionally not used in the WASM SDK implementation (prefixed with underscore on line 943). While the TypeScript JSDoc notes it's "reserved for future filtering," users passing a contract ID may expect it to filter results, which it currently does not. The comment indicates batch query functionality isn't fully exposed yet.Recommended action: Add inline documentation clarifying that
contractIdis reserved for future use and currently ignored, or add validation to warn users when a non-default contract ID is provided without filtering being applied.
♻️ Duplicate comments (1)
packages/wasm-sdk/src/queries/identity.rs (1)
1159-1163: Restore the search fallback in proof mode.The removal of the search fallback is a behavioral regression. The previous implementation fell back to the non-proof search when
keyRequestType === 'search'in proof mode, allowing callers to still receive results (without proof data). Returning an error here breaks existing consumers.Based on learnings
🧹 Nitpick comments (1)
packages/wasm-sdk/src/queries/identity.rs (1)
470-496: Consider standardizing onfrom_sdk_partsfor consistency.Several proof-enabled methods use
from_partswith manual.into()conversions (lines 470-496, 1175-1179, 1298-1339), while others usefrom_sdk_partswhich handles the conversion internally (lines 736-740, 809-813, etc.). Usingfrom_sdk_partsconsistently would reduce boilerplate and improve maintainability.Example for
get_identity_with_proof_info:- identity - .map(|identity| { - ProofMetadataResponseWasm::from_parts( - JsValue::from(IdentityWasm::from(identity)), - metadata.into(), - proof.into(), - ) - }) - .ok_or_else(|| WasmSdkError::not_found("Identity not found")) + identity + .map(|identity| { + ProofMetadataResponseWasm::from_sdk_parts( + JsValue::from(IdentityWasm::from(identity)), + metadata, + proof, + ) + }) + .ok_or_else(|| WasmSdkError::not_found("Identity not found"))Also applies to: 1119-1180, 1298-1339
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/js-evo-sdk/src/system/facade.ts(1 hunks)packages/wasm-sdk/src/dpns.rs(9 hunks)packages/wasm-sdk/src/queries/identity.rs(18 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/wasm-sdk/src/dpns.rspackages/wasm-sdk/src/queries/identity.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/dpns.rspackages/wasm-sdk/src/queries/identity.rs
🧠 Learnings (12)
📓 Common learnings
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.
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.
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.
📚 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/js-evo-sdk/src/system/facade.tspackages/wasm-sdk/src/dpns.rspackages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-09-02T13:30:17.703Z
Learnt from: thephez
Repo: dashpay/platform PR: 2739
File: packages/wasm-sdk/test/ui-automation/tests/state-transitions.spec.js:1-171
Timestamp: 2025-09-02T13:30:17.703Z
Learning: In packages/wasm-sdk/index.html, state transition definitions are loaded dynamically from api-definitions.json via the loadApiDefinitions() function that fetches './api-definitions.json'. The UI doesn't have hardcoded transition definitions - instead it populates categories, types, inputs, and labels from this JSON configuration file at runtime.
Applied to files:
packages/js-evo-sdk/src/system/facade.ts
📚 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/js-evo-sdk/src/system/facade.tspackages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-09-02T13:30:17.703Z
Learnt from: thephez
Repo: dashpay/platform PR: 2739
File: packages/wasm-sdk/test/ui-automation/tests/state-transitions.spec.js:1-171
Timestamp: 2025-09-02T13:30:17.703Z
Learning: In packages/wasm-sdk/index.html, state transition definitions are loaded dynamically from api-definitions.json rather than being hardcoded in the HTML file. The UI loads transition categories, types, inputs, and labels from this JSON configuration file.
Applied to files:
packages/js-evo-sdk/src/system/facade.ts
📚 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.rspackages/wasm-sdk/src/queries/identity.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.rspackages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-07-28T20:00:24.323Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/docs.html:2359-2383
Timestamp: 2025-07-28T20:00:24.323Z
Learning: In packages/wasm-sdk/docs.html, QuantumExplorer confirmed that placeholder private keys in documentation examples are acceptable as they are not real keys, though field name accuracy for the SDK API should still be maintained.
Applied to files:
packages/wasm-sdk/src/queries/identity.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/queries/identity.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/wasm-sdk/src/queries/identity.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/queries/identity.rs
📚 Learning: 2024-10-09T00:22:57.778Z
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-09T00:22:57.778Z
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/wasm-sdk/src/queries/identity.rs
🧬 Code graph analysis (2)
packages/wasm-sdk/src/dpns.rs (3)
packages/wasm-dpp2/src/identifier.rs (6)
value(72-72)value(80-80)from(23-25)from(29-31)from(35-37)from(41-43)packages/js-evo-sdk/src/dpns/facade.ts (1)
username(47-50)packages/wasm-sdk/src/queries/utils.rs (2)
deserialize_required_query(10-27)identifier_from_js(86-90)
packages/wasm-sdk/src/queries/identity.rs (3)
packages/wasm-dpp2/src/identifier.rs (8)
try_from(48-60)try_from(65-92)try_from(97-99)try_from(105-115)from(23-25)from(29-31)from(35-37)from(41-43)packages/wasm-sdk/src/error.rs (2)
invalid_argument(73-75)not_found(81-83)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 (11)
packages/wasm-sdk/src/dpns.rs (6)
19-43: LGTM! Strongly-typed result structs are well-defined.The
RegisterDpnsNameResultandDpnsUsernameInfostructs properly expose DPNS data to JavaScript with appropriatewasm_bindgenannotations and camelCase field names.
302-383: LGTM! Improved type safety with strongly-typed return.The method now returns
RegisterDpnsNameResultinstead ofJsValue, providing better type safety and eliminating manual serialization.
393-398: LGTM! Cleaner return type.Returning
Option<String>instead ofJsValueimproves type clarity and makes the API more intuitive.
400-522: LGTM! Type-safe DPNS username queries.Both
get_dpns_username_by_nameandget_dpns_username_by_name_with_proof_infonow return strongly-typed results (DpnsUsernameInfoandProofMetadataResponseWasmrespectively), improving type safety and API clarity.
524-559: LGTM! Consistent query interface and clear return types.The methods now use the typed
DpnsUsernamesQueryJsinput and return strongly-typed results. The logic to extract a single username (lines 550-558) is clean and handles the empty case appropriately.
1-599: Reminder: Run formatting, linting, and regenerate docs.As per coding guidelines:
- Format Rust code with
cargo fmt- Run Clippy linter for Rust code
- Keep WASM SDK docs in sync by running
generate_docs.pywhen updating the WASM SDKAs per coding guidelines.
packages/wasm-sdk/src/queries/identity.rs (2)
470-496: LGTM! Consistent error handling for proof-enabled query.The change to return
ProofMetadataResponseWasmand emit anot_founderror when the identity is absent aligns with the refactor's goal of standardizing proof-enabled response types.
1064-1115: LGTM! Clean implementation of token balance fetching.The implementation properly converts identifiers, constructs the query, fetches balances, and returns a well-structured Map. Error handling is appropriate throughout.
packages/js-evo-sdk/src/system/facade.ts (3)
8-16: Excellent type safety improvements!The refactor from
Promise<any>to specific wasm types (StatusResponse,CurrentQuorumsInfo) significantly improves type safety for SDK consumers. The implementation is clean and follows a consistent pattern.
18-36: Well-structured type refinements.The methods properly distinguish between proof and non-proof variants using
ProofMetadataResponseTyped<T>wrapper. The use ofbigint | undefinedappropriately handles optional values, andwasm.IdentifierLikeprovides flexibility for identity identifier inputs.
43-51: Path element methods properly typed.The
pathElementsmethods correctly implement the proof/non-proof pattern with appropriate return types (PathElement[]andProofMetadataResponseTyped<PathElement[]>).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/wasm-sdk/src/queries/identity.rs (1)
1120-1124: Restore the search fallback in proof mode.This change removes the fallback behavior where
get_identity_keys_with_proof_infowould use the non-proof implementation for search requests. Returning an error here breaks existing consumers who rely on that fallback to still get results (without proof data). Please reinstate the previous behavior by calling the non-proofget_identity_keysand wrapping the result with default/placeholder metadata and proof.Based on learnings
🧹 Nitpick comments (2)
packages/wasm-sdk/src/dpns.rs (1)
22-31: Consider adding Serialize derive for consistency.
DpnsUsernameInfoincludes#[derive(Clone, Serialize)](line 34), butRegisterDpnsNameResultonly derivesClone. If there's a need to serializeDpnsUsernameInfoto JSON/JsValue, the same might apply toRegisterDpnsNameResultfor consistency. If not needed, this is fine.packages/wasm-sdk/src/queries/document.rs (1)
288-302: Identifier detection heuristic may yield false positives.The heuristic
s.len() == 44 && s.chars().all(|c| c.is_alphanumeric())(line 291) attempts to recognize Base58-encoded identifiers but could match other 44-character alphanumeric strings. The fallback toValue::Text(line 298) prevents errors, but this might lead to unexpected behavior if legitimate text values match the pattern. The TODO comment (line 289) correctly notes thatIdentifier::try_fromshould be used, which would be more precise.Consider implementing the TODO to use
Identifier::try_fromdirectly and only fall back to text on failure, eliminating the need for the heuristic check:- JsonValue::String(s) => { - // TODO: Should use Identifier::try_from and return text if failed - // Check if it's an identifier (base58 encoded) - if s.len() == 44 && s.chars().all(|c| c.is_alphanumeric()) { - // Try to parse as identifier - match Identifier::from_string( - s, - dash_sdk::dpp::platform_value::string_encoding::Encoding::Base58, - ) { - Ok(id) => Ok(platform_value!(id)), - Err(_) => Ok(Value::Text(s.clone())), - } - } else { - Ok(Value::Text(s.clone())) - } - } + JsonValue::String(s) => { + // Try to parse as Base58 identifier first + match Identifier::from_string( + s, + dash_sdk::dpp::platform_value::string_encoding::Encoding::Base58, + ) { + Ok(id) => Ok(platform_value!(id)), + Err(_) => Ok(Value::Text(s.clone())), + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/wasm-sdk/src/dpns.rs(9 hunks)packages/wasm-sdk/src/queries/document.rs(7 hunks)packages/wasm-sdk/src/queries/identity.rs(20 hunks)packages/wasm-sdk/src/queries/voting/resources.rs(2 hunks)packages/wasm-sdk/src/wallet/key_derivation.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/wasm-sdk/src/wallet/key_derivation.rspackages/wasm-sdk/src/queries/voting/resources.rspackages/wasm-sdk/src/queries/identity.rspackages/wasm-sdk/src/dpns.rspackages/wasm-sdk/src/queries/document.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/wallet/key_derivation.rspackages/wasm-sdk/src/queries/voting/resources.rspackages/wasm-sdk/src/queries/identity.rspackages/wasm-sdk/src/dpns.rspackages/wasm-sdk/src/queries/document.rs
🧠 Learnings (19)
📓 Common learnings
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.
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.
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.
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.
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.
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.
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/docs.html:2359-2383
Timestamp: 2025-07-28T20:00:24.323Z
Learning: In packages/wasm-sdk/docs.html, QuantumExplorer confirmed that placeholder private keys in documentation examples are acceptable as they are not real keys, though field name accuracy for the SDK API should still be maintained.
📚 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/wallet/key_derivation.rs
📚 Learning: 2025-09-24T05:16:54.422Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2784
File: packages/js-evo-sdk/src/wallet/functions.ts:24-26
Timestamp: 2025-09-24T05:16:54.422Z
Learning: The WASM SDK methods like `deriveKeyFromSeedWithExtendedPath` and `deriveKeyFromSeedWithPath` already handle mnemonic-to-seed conversion internally, so the JS wrapper functions can safely pass mnemonic strings directly without needing to convert them to seeds first.
Applied to files:
packages/wasm-sdk/src/wallet/key_derivation.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/wallet/key_derivation.rspackages/wasm-sdk/src/queries/identity.rspackages/wasm-sdk/src/dpns.rspackages/wasm-sdk/src/queries/document.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/wallet/key_derivation.rspackages/wasm-sdk/src/dpns.rspackages/wasm-sdk/src/queries/document.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/wasm-sdk/src/wallet/key_derivation.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/queries/voting/resources.rspackages/wasm-sdk/src/queries/identity.rspackages/wasm-sdk/src/dpns.rspackages/wasm-sdk/src/queries/document.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/wasm-sdk/src/queries/voting/resources.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/queries/identity.rspackages/wasm-sdk/src/queries/document.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-07-28T20:00:24.323Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/docs.html:2359-2383
Timestamp: 2025-07-28T20:00:24.323Z
Learning: In packages/wasm-sdk/docs.html, QuantumExplorer confirmed that placeholder private keys in documentation examples are acceptable as they are not real keys, though field name accuracy for the SDK API should still be maintained.
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-06-18T03:44:14.385Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2673
File: packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs:1164-1197
Timestamp: 2025-06-18T03:44:14.385Z
Learning: QuantumExplorer determined that a CodeRabbit suggestion about fixing signable_bytes calculation in identity update tests with contract-bound keys was incorrect - the code flow is working as intended without the suggested changes.
Applied to files:
packages/wasm-sdk/src/queries/identity.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/queries/identity.rspackages/wasm-sdk/src/queries/document.rs
📚 Learning: 2024-10-09T00:22:57.778Z
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-09T00:22:57.778Z
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/wasm-sdk/src/queries/identity.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.rspackages/wasm-sdk/src/queries/document.rs
📚 Learning: 2025-08-14T15:03:56.681Z
Learnt from: thephez
Repo: dashpay/platform PR: 2726
File: packages/wasm-sdk/check_documentation.py:69-76
Timestamp: 2025-08-14T15:03:56.681Z
Learning: In packages/wasm-sdk/api-definitions.json, the structure is nested by categories: { "queries": { "categoryName": { "label": "...", "queries": { "actualQueryName": {...} } } } }. The check_documentation.py script correctly iterates over categories and then accesses the nested 'queries'/'transitions' objects within each category to collect the actual query/transition names.
Applied to files:
packages/wasm-sdk/src/queries/document.rs
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/wasm-sdk/** : Keep WASM SDK docs in sync (run generate_docs.py) when updating the WASM SDK
Applied to files:
packages/wasm-sdk/src/queries/document.rs
📚 Learning: 2024-10-29T14:40:54.727Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/core/transaction.rs:0-0
Timestamp: 2024-10-29T14:40:54.727Z
Learning: In `packages/rs-sdk/src/platform/document_query.rs` and `packages/rs-sdk/src/core/transaction.rs`, certain places don't implement `IntoInner`, so direct error mappings cannot be simplified using `IntoInner`. A TODO comment has been added to address this in a future PR.
Applied to files:
packages/wasm-sdk/src/queries/document.rs
📚 Learning: 2024-10-30T11:04:33.634Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/platform/fetch_unproved.rs:0-0
Timestamp: 2024-10-30T11:04:33.634Z
Learning: In `packages/rs-sdk/src/platform/fetch_unproved.rs`, the `execute()` method consumes the request object, so cloning the request is necessary before passing it to `execute()` and `maybe_from_unproved_with_metadata`.
Applied to files:
packages/wasm-sdk/src/queries/document.rs
🧬 Code graph analysis (4)
packages/wasm-sdk/src/wallet/key_derivation.rs (2)
packages/wasm-sdk/src/wallet/key_generation.rs (1)
from(44-52)packages/wasm-sdk/src/wallet/extended_derivation.rs (1)
from(156-167)
packages/wasm-sdk/src/queries/identity.rs (2)
packages/wasm-sdk/src/error.rs (2)
invalid_argument(73-75)not_found(81-83)packages/wasm-sdk/src/queries/mod.rs (5)
metadata(242-244)proof(252-254)from_parts(263-273)from(89-98)from(195-204)
packages/wasm-sdk/src/dpns.rs (2)
packages/js-evo-sdk/src/dpns/facade.ts (1)
username(47-50)packages/wasm-sdk/src/queries/utils.rs (1)
identifier_from_js(86-90)
packages/wasm-sdk/src/queries/document.rs (2)
packages/js-evo-sdk/src/documents/facade.ts (1)
query(13-16)packages/wasm-sdk/src/queries/mod.rs (5)
from_parts(263-273)from(89-98)from(195-204)metadata(242-244)proof(252-254)
⏰ 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). (6)
- 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 (27)
packages/wasm-sdk/src/queries/voting/resources.rs (1)
207-210: LGTM! TypeScript return type metadata added.The addition of
unchecked_return_type = "Array<any>"improves the TypeScript developer experience by making the return type explicit in the generated bindings.packages/wasm-sdk/src/wallet/key_derivation.rs (8)
20-90: LGTM! Well-structured param-object pattern.The extern TS types and corresponding input structs follow a clean pattern for type-safe WASM bindings. The use of
Optionfor optional fields and#[serde(default)]is correct.
200-228: LGTM! Proper WASM binding pattern.The
DerivationPathWasmstruct and itsFromimplementation follow best practices for wasm_bindgen exports. The use ofgetterfor primitives andgetter_with_cloneforString, along with camelCase JS naming, is correct.
230-243: LGTM! Consistent structure for DIP13 paths.The
Dip13DerivationPathWasmstruct appropriately includes thedescriptionfield and follows the same binding patterns asDerivationPathWasm.
245-275: LGTM! Type-safe key info structures.Both
SeedPhraseKeyInfoWasmandPathDerivedKeyInfoWasmproperly expose key information with appropriate camelCase naming for JS consumers. These replace looseJsValuetypes with strongly-typed exports.
399-454: Verify the simplified seed-to-key derivation approach.While the type safety improvement with
SeedPhraseKeyInfoWasmis excellent, the comment at lines 415-417 flags that this function uses a simplified approach (taking the first 32 bytes of the seed) rather than proper BIP44 HD derivation. This deviates from the BIP44 standard which specifies paths likem/44'/5'/0'/0/0.If this function is intended for production use, consider implementing proper BIP44 derivation similar to
derive_key_from_seed_with_path. If this simplified approach is intentional for specific use cases, the function name and documentation should clarify this distinction to prevent misuse.Is this simplified derivation intentional, or should this function implement proper BIP44 HD derivation?
457-517: LGTM! Proper HD derivation with improved types.The function correctly implements BIP32 HD derivation and now returns the strongly-typed
PathDerivedKeyInfoWasm. The implementation properly parses the derivation path, creates a master key, and derives the key at the specified path.
519-565: LGTM! Consistent typed return values for derivation paths.All four functions (
derivation_path_bip44_mainnet,derivation_path_bip44_testnet,derivation_path_dip9_mainnet,derivation_path_dip9_testnet) now returnDerivationPathWasmusing theFromtrait for clean conversion. This provides better type safety than previous implementations.
567-595: LGTM! DIP13 paths with helpful descriptions.Both DIP13 derivation path functions now return
Dip13DerivationPathWasmwith descriptive text. The direct construction approach is appropriate here.Documentation update required: Since this PR changes public API return types and parameter types across the WASM SDK, please regenerate the documentation by running
generate_docs.pyto ensure the docs reflect the new strongly-typed interfaces.Based on coding guidelines.
packages/wasm-sdk/src/dpns.rs (9)
45-69: LGTM!The DPNS username utility functions are well-implemented. The default limit handling and document-to-username conversion logic is correct.
71-120: LGTM!The DPNS usernames query TS bindings and parsing logic follow the established patterns in the codebase and are properly typed.
122-142: LGTM!The proof response types are well-structured and properly expose fields with wasm_bindgen getters.
143-205: LGTM!The internal helper methods properly encapsulate DPNS username query preparation and fetching logic, with clean separation between proof and non-proof paths.
225-306: LGTM!The
dpns_register_namemethod has been properly updated to return a strongly-typedRegisterDpnsNameResultand useidentifier_from_jsfor parsing the identity ID parameter.
316-378: LGTM!The
dpns_resolve_nameandget_dpns_username_by_namemethods are correctly updated to return strongly-typed results with proper error handling.
380-448: LGTM!The proof-enabled username query properly returns
ProofMetadataResponseWasmand correctly serializes theDpnsUsernameInforesult toJsValuebefore wrapping.
450-482: LGTM!The
get_dpns_usernamesandget_dpns_usernamemethods properly use the new query-based API and handle the single-username case correctly.
483-521: LGTM!The proof-enabled username query methods properly handle both batch and single-username cases with appropriate return types.
packages/wasm-sdk/src/queries/document.rs (4)
56-96: LGTM!The TypeScript interface updates to use
IdentifierLikefor identifier fields are consistent with the broader API standardization in this PR.
321-394: LGTM!The document query methods are correctly updated to use
ProofMetadataResponseWasmfor proof-enabled queries, with proper type annotations and consistent document conversion logic.
396-441: LGTM!The
get_documentmethod properly validates the data contract and document type before fetching, with correct identifier parsing and error handling.
443-497: LGTM!The proof-enabled document fetch properly returns
ProofMetadataResponseWasmand correctly handles the optional document result throughJsValue::from.packages/wasm-sdk/src/queries/identity.rs (5)
197-262: LGTM!The new
IdentitiesContractKeysQueryTS bindings and parsing logic follow the established patterns in the codebase and are well-documented.
431-457: LGTM!The
get_identity_with_proof_infomethod correctly returnsProofMetadataResponseWasmand handles the case where the identity is not found.
511-647: LGTM!The
get_identity_keysmethod properly handles all request types including the search request with direct DAPI communication.
893-961: Verify the batch query limitation.The TODO comment at line 1371 notes that
get_identities_contract_keys_with_proof_inforeturns proof only for the first identity. Both methods iterate and fetch keys individually rather than using a batch query. Ensure this approach meets performance requirements and that callers are aware of this limitation (especially the proof limitation).Also applies to: 1372-1457
672-702: LGTM!The proof-enabled identity query methods consistently use
ProofMetadataResponseWasmwrappers and properly handle error cases with appropriate type conversions.Also applies to: 734-775, 798-838, 1143-1173, 1175-1225, 1227-1257, 1259-1300, 1302-1369, 1459-1517
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/js-evo-sdk/src/system/facade.ts (1)
38-41: Consider usingwasm.IdentifierLikeforstateTransitionHash(same as prior review)This still takes
stateTransitionHash: string, while other identifier/hash parameters across the SDK usewasm.IdentifierLike. To keep the API consistent and allow callers to passUint8Arrayor other identifier forms, consider:- async waitForStateTransitionResult(stateTransitionHash: string): Promise<wasm.StateTransitionResult> { + async waitForStateTransitionResult(stateTransitionHash: wasm.IdentifierLike): Promise<wasm.StateTransitionResult> {and adjust call sites if needed.
packages/js-evo-sdk/src/tokens/facade.ts (1)
150-154: Avoid unsafe non-null assertions onasJsonString(same concern as earlier review)
asJsonStringreturnsstring | undefined, but bothsetPriceForDirectPurchaseandconfigUpdatestill use the non-null assertion:asJsonString(priceData)!; asJsonString(configValue)!;If a caller omits
priceData/configValue(e.g., from plain JS or a mis-typed TS call), this will passundefinedto the wasm binding, likely causing a confusing runtime error rather than a clear validation failure.Consider following the safer pattern used elsewhere (string or
null), e.g.:- return w.tokenSetPriceForDirectPurchase(contractId, tokenPosition, identityId, priceType, asJsonString(priceData)!, privateKeyWif, publicNote ?? null); + return w.tokenSetPriceForDirectPurchase( + contractId, + tokenPosition, + identityId, + priceType, + asJsonString(priceData) ?? null, + privateKeyWif, + publicNote ?? null, + ); ... - return w.tokenConfigUpdate(contractId, tokenPosition, configItemType, asJsonString(configValue)!, identityId, privateKeyWif, publicNote ?? null); + return w.tokenConfigUpdate( + contractId, + tokenPosition, + configItemType, + asJsonString(configValue) ?? null, + identityId, + privateKeyWif, + publicNote ?? null, + );or adding explicit runtime checks that throw a clear error if these arguments are missing.
Also applies to: 168-171
🧹 Nitpick comments (1)
packages/js-evo-sdk/src/identities/facade.ts (1)
102-105: Use nullish coalescing instead of||forstartAfter
startAfter || undefinedwill also convert other falsy values (e.g., empty string) toundefined. Using nullish coalescing avoids that and documents the intent more clearly:- return w.getIdentityByNonUniquePublicKeyHashWithProofInfo(publicKeyHash, startAfter || undefined); + return w.getIdentityByNonUniquePublicKeyHashWithProofInfo(publicKeyHash, startAfter ?? undefined);or simply pass
startAfterif the wasm binding already treatsundefinedcorrectly for omission.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/js-evo-sdk/src/identities/facade.ts(3 hunks)packages/js-evo-sdk/src/system/facade.ts(1 hunks)packages/js-evo-sdk/src/tokens/facade.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
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.
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.
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.
📚 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/js-evo-sdk/src/system/facade.tspackages/js-evo-sdk/src/identities/facade.tspackages/js-evo-sdk/src/tokens/facade.ts
📚 Learning: 2025-09-02T13:30:17.703Z
Learnt from: thephez
Repo: dashpay/platform PR: 2739
File: packages/wasm-sdk/test/ui-automation/tests/state-transitions.spec.js:1-171
Timestamp: 2025-09-02T13:30:17.703Z
Learning: In packages/wasm-sdk/index.html, state transition definitions are loaded dynamically from api-definitions.json via the loadApiDefinitions() function that fetches './api-definitions.json'. The UI doesn't have hardcoded transition definitions - instead it populates categories, types, inputs, and labels from this JSON configuration file at runtime.
Applied to files:
packages/js-evo-sdk/src/system/facade.ts
📚 Learning: 2025-09-02T13:30:17.703Z
Learnt from: thephez
Repo: dashpay/platform PR: 2739
File: packages/wasm-sdk/test/ui-automation/tests/state-transitions.spec.js:1-171
Timestamp: 2025-09-02T13:30:17.703Z
Learning: In packages/wasm-sdk/index.html, state transition definitions are loaded dynamically from api-definitions.json rather than being hardcoded in the HTML file. The UI loads transition categories, types, inputs, and labels from this JSON configuration file.
Applied to files:
packages/js-evo-sdk/src/system/facade.ts
📚 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/js-evo-sdk/src/system/facade.ts
📚 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/js-evo-sdk/src/system/facade.ts
📚 Learning: 2024-10-24T04:59:20.436Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:75-77
Timestamp: 2024-10-24T04:59:20.436Z
Learning: Tenderdash's `tx` RPC method accepts transaction hashes in base64 encoding, or in hex encoding if prefixed with `0x`. Therefore, in `packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js`, it's acceptable to use `stHash.toString('base64')` when calling `requestTenderRpc('tx', { hash: stHash.toString('base64') })`.
Applied to files:
packages/js-evo-sdk/src/system/facade.ts
📚 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/js-evo-sdk/src/system/facade.tspackages/js-evo-sdk/src/identities/facade.tspackages/js-evo-sdk/src/tokens/facade.ts
📚 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/js-evo-sdk/src/system/facade.ts
📚 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/js-evo-sdk/src/identities/facade.ts
📚 Learning: 2025-07-28T20:00:24.323Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/docs.html:2359-2383
Timestamp: 2025-07-28T20:00:24.323Z
Learning: In packages/wasm-sdk/docs.html, QuantumExplorer confirmed that placeholder private keys in documentation examples are acceptable as they are not real keys, though field name accuracy for the SDK API should still be maintained.
Applied to files:
packages/js-evo-sdk/src/identities/facade.ts
📚 Learning: 2025-01-23T02:10:08.979Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2439
File: packages/rs-dpp/src/errors/consensus/state/token/mod.rs:4-22
Timestamp: 2025-01-23T02:10:08.979Z
Learning: The user QuantumExplorer prefers to handle documentation of breaking changes separately from the code changes, particularly for token-related error types and validation rules.
Applied to files:
packages/js-evo-sdk/src/tokens/facade.ts
🧬 Code graph analysis (3)
packages/js-evo-sdk/src/system/facade.ts (1)
packages/js-evo-sdk/src/sdk.ts (1)
wasm(68-71)
packages/js-evo-sdk/src/identities/facade.ts (2)
packages/js-evo-sdk/src/sdk.ts (1)
wasm(68-71)packages/js-evo-sdk/src/documents/facade.ts (1)
query(13-16)
packages/js-evo-sdk/src/tokens/facade.ts (1)
packages/js-evo-sdk/src/sdk.ts (1)
wasm(68-71)
⏰ 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). (6)
- 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 (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (4)
packages/js-evo-sdk/src/system/facade.ts (2)
8-36: Typed system query wrappers look goodEach method correctly awaits
sdk.getWasmSdkConnected()and returns strongly-typed wasm responses (StatusResponse,CurrentQuorumsInfo,bigint | undefined, andProofMetadataResponseTyped<...>). This aligns with the broader SDK typing refactor and keeps runtime behavior simple pass-through.
43-51: Path element query wrappers are consistent
pathElementsandpathElementsWithProofmirror the pattern used elsewhere (simple delegation plus typedPathElement[]/ProofMetadataResponseTyped<...>results). No issues spotted.packages/js-evo-sdk/src/identities/facade.ts (1)
17-85: Identity query typings align well with wasm SDKThe updated signatures for
fetchWithProof, key/nonce/balance/balances/balanceAndRevision queries, public-key-hash lookups, contract keys, and token balance helpers now return concrete wasm types andProofMetadataResponseTyped<...>wrappers. The implementations are thin delegates togetWasmSdkConnected()and the corresponding wasm methods, matching the pattern used in other facades and keeping behavior unchanged while strengthening type safety.Also applies to: 92-115, 117-125
packages/js-evo-sdk/src/tokens/facade.ts (1)
18-41: Token query typings look consistent and well-scopedThe token-related query methods (price, total supply, statuses, balances, identity token infos, direct purchase prices, contract info, perpetual distribution last claim) now return explicit wasm types and
Map<wasm.Identifier, ...>structures, plusProofMetadataResponseTyped<...>for proof variants. Implementations are simple delegates to the connected wasm SDK instance and match the broader typing pattern in the PR.Also applies to: 23-41, 33-41, 43-61, 63-81, 83-91, 93-101, 103-111
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/wasm-sdk/src/wallet/key_derivation.rs (1)
225-238: Consider consistent getter attribute patterns across WASM structs.This struct uses field-level
getterandgetter_with_cloneattributes, whileDerivationPathWasm(line 200) uses struct-levelgetter_with_clone, andSeedPhraseKeyInfoWasm/PathDerivedKeyInfoWasmuse field-levelgetter_with_clonefor all fields.While all three approaches are valid, consider standardizing on the most explicit pattern used here:
getter_with_clonefor String fields andgetterfor primitives.Apply this diff to make
DerivationPathWasmconsistent:-#[wasm_bindgen(js_name = "DerivationPathInfo", getter_with_clone)] +#[wasm_bindgen(js_name = "DerivationPathInfo")] #[derive(Clone)] pub struct DerivationPathWasm { + #[wasm_bindgen(getter_with_clone)] pub path: String, + #[wasm_bindgen(getter)] pub purpose: u32, - #[wasm_bindgen(js_name = "coinType")] + #[wasm_bindgen(getter, js_name = "coinType")] pub coin_type: u32, + #[wasm_bindgen(getter)] pub account: u32, + #[wasm_bindgen(getter)] pub change: u32, + #[wasm_bindgen(getter)] pub index: u32, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/wasm-sdk/src/wallet/key_derivation.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/wasm-sdk/src/wallet/key_derivation.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/wallet/key_derivation.rs
🧠 Learnings (6)
📓 Common learnings
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.
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.
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.
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.
📚 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/wallet/key_derivation.rs
📚 Learning: 2025-09-24T05:16:54.422Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2784
File: packages/js-evo-sdk/src/wallet/functions.ts:24-26
Timestamp: 2025-09-24T05:16:54.422Z
Learning: The WASM SDK methods like `deriveKeyFromSeedWithExtendedPath` and `deriveKeyFromSeedWithPath` already handle mnemonic-to-seed conversion internally, so the JS wrapper functions can safely pass mnemonic strings directly without needing to convert them to seeds first.
Applied to files:
packages/wasm-sdk/src/wallet/key_derivation.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/wallet/key_derivation.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/wasm-sdk/src/wallet/key_derivation.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/wallet/key_derivation.rs
🧬 Code graph analysis (1)
packages/wasm-sdk/src/wallet/key_derivation.rs (2)
packages/wasm-sdk/src/wallet/key_generation.rs (1)
from(44-52)packages/wasm-sdk/src/wallet/extended_derivation.rs (1)
from(156-167)
⏰ 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) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- 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/wallet/key_derivation.rs (8)
200-223: LGTM! Clean conversion from internal type to WASM binding.The
DerivationPathWasmstruct and itsFrom<DerivationPath>implementation correctly expose all derivation path components to JavaScript with appropriate field renaming.
240-253: LGTM! Properly structured WASM binding for seed phrase key information.All String fields appropriately use
getter_with_clonefor JavaScript interop.
255-270: LGTM! Properly structured WASM binding for path-derived key information.Consistent field annotations with appropriate
getter_with_clonefor all String fields.
395-448: LGTM! Type-safe return value improves API clarity.The function now returns a strongly-typed
SeedPhraseKeyInfoWasmstruct instead of a generic JsValue, making the API more predictable and type-safe for TypeScript consumers.
453-512: LGTM! Enhanced type safety for path-based key derivation.The strongly-typed
PathDerivedKeyInfoWasmreturn value improves API ergonomics and enables better TypeScript integration.
516-560: LGTM! Consistent type-safe returns for BIP44 and DIP9 derivation paths.All four functions now return strongly-typed
DerivationPathWasmstructs via theFromtrait conversion, providing better type safety and IDE support for JavaScript/TypeScript consumers.
564-590: LGTM! Appropriate specialized type for DIP13 identity key paths.The
Dip13DerivationPathWasmtype correctly represents the simplified DIP13 path structure (m/9'/coin'/account') and includes a helpful description field.
200-590: Reminder: Update WASM SDK documentation.Multiple new WASM-exported structs and updated function signatures were added. Please ensure you run
generate_docs.pyto keep the WASM SDK documentation in sync with these API changes.As per coding guidelines.
Issue being fixed or feature implemented
This is continuation of JS SDK refactoring. The goal of this PR is to cleanup params and return types.
What was done?
How Has This Been Tested?
Updated existing tests.
Breaking Changes
JS API params and return types are changed
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Improvements
Chores