refactor(sdk)!: typed params for state transition methods #2932
Conversation
📝 WalkthroughWalkthroughAdds a macro-based options-extraction utility and applies it across wasm-dpp2; converts many wasm-sdk bindings to single options objects; implements full WASM flows for contracts, documents, identities, and tokens; updates JS facades and tests to options-based APIs; changes data-contract schema serialization to use platform values. Changes
Sequence Diagram(s)sequenceDiagram
participant JS as JavaScript caller
participant WASM as WasmSdk (wasm_bindgen)
participant Conv as wasm-dpp2 converters
participant SDK as Inner Rust SDK (builders/signing)
participant Network as Platform / Broadcast
JS->>WASM: call contractPublish(options)
WASM->>Conv: DataContractWasm::try_from_options(options.dataContract)
WASM->>Conv: IdentityPublicKeyWasm::try_from_options(options.identityKey)
WASM->>Conv: IdentitySignerWasm::try_from_options(options.signer)
WASM->>WASM: extract_settings_from_options(options.settings)
WASM->>SDK: build & sign DataContractPublishTransition
SDK->>Network: broadcast transition
Network-->>SDK: broadcast result
SDK-->>WASM: published contract / result
WASM-->>JS: return DataContractWasm
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ gRPC Query Coverage Report |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
packages/wasm-sdk/tests/functional/protocol.spec.mjs (1)
44-47: Same empty string verification applies here.This test uses the same fallback pattern as the previous test. The same verification for empty string handling in
getProtocolVersionUpgradeVoteStatusWithProofInfo()applies here as well.
🧹 Nitpick comments (16)
packages/rs-sdk/docs/IMPROVE_RETRY_ERROR_HANDLING.md (1)
8-8: Specify language for fenced code blocks.Markdown linting requires language identifiers on code blocks for consistency and syntax highlighting. Lines 8 and 13 contain code examples without language specifiers.
🔎 Proposed fix
-Example: `getDataContractHistory()` fails with proof verification error, but after all addresses are banned, the user sees: -``` +Example: `getDataContractHistory()` fails with proof verification error, but after all addresses are banned, the user sees: +```text Dapi client error: no available addresses to use -``` +```text Instead of the meaningful: -``` +```text Proof verification error: <actual error details> -``` +```textAlso applies to: 13-13
packages/wasm-sdk/tests/functional/epochs-blocks.spec.mjs (2)
20-24: Consider extracting the magic delay constant.The 100ms delay works, but it's a magic number. Consider extracting it as a named constant for clarity, or adding a brief comment explaining why this specific duration was chosen.
🔎 Optional: Extract named constant
+const CLEANUP_DELAY_MS = 100; // Allow pending microtasks to settle + after(async () => { - // Wait briefly to ensure any pending async operations complete - await new Promise((resolve) => { setTimeout(resolve, 100); }); + await new Promise((resolve) => { setTimeout(resolve, CLEANUP_DELAY_MS); }); if (client) { client.free(); } });
65-98: Test may be skipped entirely if no proTxHash is available.When both
evonodeProTxHashis null andbyRangereturns no results, the coregetEvonodesProposedEpochBlocksByIdsWithProofInfocall and its assertions (lines 92-97) are skipped entirely. Consider adding athis.skip()or a log message to make it explicit when this happens, so test runs are transparent about what was actually verified.🔎 Optional: Make test skip explicit
// Only test by IDs if we have a valid proTxHash if (testProTxHash) { const res = await client.getEvonodesProposedEpochBlocksByIdsWithProofInfo(epochIndex, [testProTxHash]); expect(res).to.be.ok(); expect(res.data).to.be.instanceOf(Map); expect(res.proof).to.be.ok(); expect(res.metadata).to.be.ok(); + } else { + this.skip(); // No proTxHash available to test byIds with proof }packages/wasm-sdk/src/wallet/key_generation.rs (1)
268-272: Consider moving imports to module level.These imports are used only within this function, but module-level imports are conventional in Rust. This is a minor style consideration that doesn't affect functionality.
🔎 Suggested refactor
Move these imports to the top of the file with other imports:
use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; use dash_sdk::dpp::identity::IdentityPublicKey; use dash_sdk::dpp::version::LATEST_PLATFORM_VERSION; use rand::rngs::StdRng; use rand::SeedableRng;packages/wasm-sdk/tests/functional/transitions/documents.spec.mjs (1)
192-195: Consider more meaningful assertions for replace/delete/transfer tests.
expect(true).to.be.trueis essentially a no-op. While the tests do verify success implicitly (no exception thrown), consider asserting on the operation result or fetching the document to verify the change. Based on learnings, use dirty-chai function-call style.🔎 Example improvement for documentReplace
// After replace, optionally fetch and verify the document was updated // Or at minimum, use a more descriptive assertion pattern // The current approach relies on "no throw = success" which is valid but weakpackages/wasm-sdk/src/state_transitions/document.rs (2)
29-39: Consider consolidating string extraction with other option utilities.
extract_string_from_optionsis only used once (line 373 fordocumentTypeName). Consider either:
- Moving to the shared utils module if it will be reused
- Inlining at the call site if truly single-use
105-106: Document cloning may be unnecessary.
document_wasm.clone()is called even thoughdocument_wasmis consumed immediately after. This pattern repeats in replace, transfer, purchase, and set_price methods. Consider using the value directly without cloning when possible.🔎 Example for document_create
- let document: Document = document_wasm.clone().into(); + let document: Document = document_wasm.into();Note: This depends on whether
document_wasmis used after the conversion. Check each method individually.packages/wasm-dpp2/src/private_key.rs (1)
129-151: Implementation looks correct.The
try_from_optionsmethod properly validates field presence and handles conversions with clear error messages. The pattern is consistent with similar manual implementations in this PR.Consider whether this manual implementation could use the
impl_try_from_options!macro for consistency with other types likeIdentityWasm,CoreScriptWasm, andDocumentWasm. The manual approach may be necessary due to theIntoWasmtrait usage, but verifying this would improve codebase uniformity.packages/wasm-dpp2/src/identifier.rs (1)
297-316: Implementation is correct.The
try_from_optionsmethod properly validates field presence and delegates to the existingtry_fromimplementation. Error handling is clear and consistent.Similar to
PrivateKeyWasm, consider whether theimpl_try_from_options!macro could be used here for consistency with types likeIdentityWasmandDocumentWasm, assuming the macro can accommodate the conversion logic.packages/wasm-sdk/tests/functional/transitions/contracts.spec.mjs (1)
87-156: Test dependency between suites.The
contractUpdatetest depends oncreatedContractIdbeing set by the priorcontractPublishtest. While Mocha runs tests sequentially by default (making this work), this creates an implicit ordering dependency. Consider documenting this dependency explicitly or using a shared setup if the tests ever need to run independently.The test logic itself is sound—proper security level selection (CRITICAL for updates), nonce cache refresh, schema augmentation, and version verification.
packages/wasm-sdk/tests/functional/transitions/identity.spec.mjs (1)
126-158: Test interdependency between add and disable key tests.The "disables a public key" test (line 126) depends on the previous "adds a new public key" test having successfully added a MEDIUM-level key. Line 141 asserts
keyToDisableexists, which will fail if the previous test didn't run or failed.This is acceptable for functional/integration tests where sequential operations are being validated, but consider adding a comment noting the dependency.
🔎 Suggested documentation improvement
it('disables a public key on identity', async function testDisablePublicKey() { + // Note: This test depends on the previous test having added a MEDIUM-level key // Identity update requires MASTER key (key index 0) const { signer } = createTestSignerAndKey(sdk, 1, 0);packages/wasm-dpp2/src/identity/signer.rs (1)
103-137: Multi-key-type support correctly implemented, but clippy warning should be addressed.The
get_key_hashmethod properly distinguishes betweenECDSA_HASH160(20-byte hash) andECDSA_SECP256K1(33-byte compressed pubkey that needs hashing). The logic is sound.However, clippy reports
result_large_errbecauseProtocolErroris at least 768 bytes. SinceProtocolErroris from thedppcrate, you can suppress this locally:🔎 Proposed fix to suppress clippy warning
+ #[allow(clippy::result_large_err)] fn get_key_hash(identity_public_key: &IdentityPublicKey) -> Result<[u8; 20], ProtocolError> {Alternatively, if this pattern is widespread, consider using
Box<ProtocolError>in theErrvariant at a higher level, but that would require broader changes to the dpp crate.packages/wasm-sdk/src/state_transitions/token.rs (3)
329-338: Redundant call toextract_settings_from_optionsfor user fee increase.Settings are already extracted on line 300 and stored in
settings. The code on lines 334-335 callsextract_settings_from_optionsagain, which duplicates the deserialization work.🔎 Proposed fix to reuse the already-extracted settings
// Add user fee increase from settings - let user_fee_increase = - get_user_fee_increase(extract_settings_from_options(&options_value)?.as_ref()); + let user_fee_increase = get_user_fee_increase(settings.as_ref()); if user_fee_increase > 0 { builder = builder.with_user_fee_increase(user_fee_increase); }
584-589: Same redundantextract_settings_from_optionspattern in token_burn.This pattern is repeated across multiple token operations. Consider fixing this in all places for consistency and efficiency.
🔎 Proposed fix
// Add user fee increase from settings - let user_fee_increase = - get_user_fee_increase(extract_settings_from_options(&options_value)?.as_ref()); + let user_fee_increase = get_user_fee_increase(settings.as_ref()); if user_fee_increase > 0 { builder = builder.with_user_fee_increase(user_fee_increase); }
831-836: Repeated redundantextract_settings_from_optionscalls across all token operations.This same pattern appears in token_transfer (831-836), token_freeze (1061-1066), token_unfreeze (1291-1296), token_destroy_frozen (1502-1507), token_emergency_action (1726-1731), token_claim (1935-1940), token_set_price (2177-2182), and token_direct_purchase (2399-2404).
Consider refactoring all of these to reuse the already-extracted
settingsvariable.Also applies to: 1061-1066, 1291-1296, 1502-1507, 1726-1731, 1935-1940, 2177-2182, 2399-2404
packages/wasm-sdk/src/state_transitions/addresses.rs (1)
924-931: Identity extraction inidentity_create_from_addressesuses different pattern than other methods.Lines 924-931 use manual
js_sys::Reflect::getandto_wasmfor identity extraction, while other methods in this file (lines 303-304, 511-512) useIdentityWasm::try_from_options. Consider using the consistent pattern for uniformity.🔎 Proposed fix for consistency
- // Extract identity from options - let identity_js = js_sys::Reflect::get(&options_value, &JsValue::from_str("identity")) - .map_err(|_| WasmSdkError::invalid_argument("identity is required"))?; - let identity: Identity = identity_js - .to_wasm::<wasm_dpp2::IdentityWasm>("Identity")? - .clone() - .into(); + // Extract identity from options + let identity: Identity = IdentityWasm::try_from_options(&options_value, "identity")?.into();
| impl From<TransferResult> for TokenTransferResultWasm { | ||
| fn from(result: TransferResult) -> Self { | ||
| match result { | ||
| TransferResult::IdentitiesBalances(balances) => { | ||
| // Get the first two balances (sender and recipient) | ||
| let mut iter = balances.into_values(); | ||
| let first_balance = iter.next(); | ||
| let second_balance = iter.next(); | ||
| TokenTransferResultWasm { | ||
| sender_balance: first_balance, | ||
| recipient_balance: second_balance, | ||
| group_power: None, | ||
| document: None, | ||
| } | ||
| } | ||
| TransferResult::HistoricalDocument(doc) => TokenTransferResultWasm { | ||
| sender_balance: None, | ||
| recipient_balance: None, | ||
| group_power: None, | ||
| document: Some(doc.into()), | ||
| }, | ||
| TransferResult::GroupActionWithDocument(power, doc) => TokenTransferResultWasm { | ||
| sender_balance: None, | ||
| recipient_balance: None, | ||
| group_power: Some(power as u32), | ||
| document: doc.map(|d| d.into()), | ||
| }, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
TransferResult::IdentitiesBalances conversion may incorrectly assign sender/recipient balances.
The conversion iterates over balances.into_values() and assigns the first value to sender_balance and the second to recipient_balance. However, BTreeMap::into_values() (or similar) iterates in key order, not insertion order. If the recipient's identifier sorts before the sender's, the balances will be swapped.
Consider returning both balances with their corresponding identifiers, or document that the order is based on identifier sorting rather than semantic sender/recipient roles.
🔎 Proposed fix - return identifiers with balances
#[wasm_bindgen(js_name = "TokenTransferResult")]
pub struct TokenTransferResultWasm {
- /// For IdentitiesBalances result - sender's new balance
- sender_balance: Option<u64>,
- /// For IdentitiesBalances result - recipient's new balance
- recipient_balance: Option<u64>,
+ /// For IdentitiesBalances result - first identity ID
+ first_identity_id: Option<IdentifierWasm>,
+ /// For IdentitiesBalances result - first identity's new balance
+ first_balance: Option<u64>,
+ /// For IdentitiesBalances result - second identity ID
+ second_identity_id: Option<IdentifierWasm>,
+ /// For IdentitiesBalances result - second identity's new balance
+ second_balance: Option<u64>,
// ... rest unchanged
}And update the From impl accordingly to preserve identity-balance associations.
Committable suggestion skipped: line range outside the PR's diff.
| export interface TokenFreezeOptions { | ||
| /** | ||
| * The ID of the data contract containing the token. | ||
| */ | ||
| dataContractId: Identifier; | ||
| /** | ||
| * The position of the token in the contract (0-indexed). | ||
| */ | ||
| tokenPosition: number; | ||
| /** | ||
| * The identity ID of the token authority performing the freeze. | ||
| */ | ||
| authorityId: Identifier; | ||
| /** | ||
| * The identity ID to freeze. | ||
| */ | ||
| frozenIdentityId: Identifier; | ||
| /** | ||
| * Optional public note for the freeze operation. | ||
| */ | ||
| publicNote?: string; | ||
| /** | ||
| * Signer containing the private key for the authority's authentication key. | ||
| * Use IdentitySigner to add the authentication key before calling. | ||
| */ | ||
| signer: IdentitySigner; | ||
| /** | ||
| * Optional group action info for group-managed token freezing. | ||
| * Use GroupStateTransitionInfoStatus.proposer() to propose a new group action, | ||
| * or GroupStateTransitionInfoStatus.otherSigner() to vote on an existing action. | ||
| */ | ||
| groupInfo?: GroupStateTransitionInfoStatus; | ||
| /** | ||
| * Optional settings for the broadcast operation. | ||
| * Includes retries, timeouts, userFeeIncrease, etc. | ||
| */ | ||
| settings?: PutSettings; | ||
| } | ||
| "#; |
There was a problem hiding this comment.
TypeScript interfaces for TokenFreezeOptions and TokenDestroyFrozenOptions are missing identityKey field.
Looking at lines 859-904 (TokenFreezeOptions) and 1313-1363 (TokenDestroyFrozenOptions), the identityKey field is not documented in the TypeScript interfaces, but the Rust code attempts to extract it (lines 1022-1024 and 1463-1465). This mismatch could cause runtime errors.
🔎 Proposed fix for TokenFreezeOptions
Add after line 889 in the TypeScript interface:
/**
* The identity public key to use for signing the transition.
*/
identityKey: IdentityPublicKey;Also applies to: 1313-1363
🤖 Prompt for AI Agents
In packages/wasm-sdk/src/state_transitions/token.rs around lines 859-904 and
1313-1363, the TypeScript interfaces TokenFreezeOptions and
TokenDestroyFrozenOptions are missing the identityKey field referenced by the
Rust extraction code; add a new documented property named identityKey of type
IdentityPublicKey to both interfaces (insert it near the signer field or where
identity-related fields are grouped) so the TypeScript definitions match the
Rust usage and prevent runtime mismatches.
| impl From<SetPriceResult> for TokenSetPriceResultWasm { | ||
| fn from(result: SetPriceResult) -> Self { | ||
| match result { | ||
| SetPriceResult::PricingSchedule(_owner_id, _schedule) => TokenSetPriceResultWasm { | ||
| group_power: None, | ||
| group_action_status: None, | ||
| document: None, | ||
| }, | ||
| SetPriceResult::HistoricalDocument(doc) => TokenSetPriceResultWasm { | ||
| group_power: None, | ||
| group_action_status: None, | ||
| document: Some(doc.into()), | ||
| }, | ||
| SetPriceResult::GroupActionWithDocument(power, doc) => TokenSetPriceResultWasm { | ||
| group_power: Some(power as u32), | ||
| group_action_status: None, | ||
| document: doc.map(|d| d.into()), | ||
| }, | ||
| SetPriceResult::GroupActionWithPricingSchedule(power, status, _schedule) => { | ||
| TokenSetPriceResultWasm { | ||
| group_power: Some(power as u32), | ||
| group_action_status: Some(format!("{:?}", status)), | ||
| document: None, | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
SetPriceResult::PricingSchedule variant discards useful information.
When the result is PricingSchedule(owner_id, schedule), both the owner ID and the pricing schedule are discarded. The WASM result returns None for all fields, which provides no useful information to the caller.
Consider exposing at least the owner_id or a boolean indicating success, so callers can distinguish this result variant from an error state.
🤖 Prompt for AI Agents
In packages/wasm-sdk/src/state_transitions/token.rs around lines 2079 to 2106,
the match arm for SetPriceResult::PricingSchedule currently returns all None and
discards owner_id and schedule; update that arm to preserve useful info by
populating at least one field: include the owner_id (e.g., group_action_status:
Some(owner_id.to_string())) or, if schedule is convertible to the WASM document
type, set document: Some(schedule.into()); ensure any necessary conversions are
available and adjust field choices to match TokenSetPriceResultWasm types so
callers can distinguish PricingSchedule from error cases.
| /** | ||
| * Convert hex string to Uint8Array | ||
| * @param {string} hex - Hex string | ||
| * @returns {Uint8Array} Byte array | ||
| */ | ||
| function hexToBytes(hex) { | ||
| const bytes = new Uint8Array(hex.length / 2); | ||
| for (let i = 0; i < hex.length; i += 2) { | ||
| bytes[i / 2] = parseInt(hex.substr(i, 2), 16); | ||
| } | ||
| return bytes; | ||
| } |
There was a problem hiding this comment.
hexToBytes function is defined but never used.
This helper function is not exported and not used within this file. Consider removing it or using it where applicable.
🔎 Suggested fix
Remove the unused function:
-/**
- * Convert hex string to Uint8Array
- * @param {string} hex - Hex string
- * @returns {Uint8Array} Byte array
- */
-function hexToBytes(hex) {
- const bytes = new Uint8Array(hex.length / 2);
- for (let i = 0; i < hex.length; i += 2) {
- bytes[i / 2] = parseInt(hex.substr(i, 2), 16);
- }
- return bytes;
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Convert hex string to Uint8Array | |
| * @param {string} hex - Hex string | |
| * @returns {Uint8Array} Byte array | |
| */ | |
| function hexToBytes(hex) { | |
| const bytes = new Uint8Array(hex.length / 2); | |
| for (let i = 0; i < hex.length; i += 2) { | |
| bytes[i / 2] = parseInt(hex.substr(i, 2), 16); | |
| } | |
| return bytes; | |
| } |
🤖 Prompt for AI Agents
In packages/wasm-sdk/tests/functional/fixtures/requiredTestData.mjs around lines
119 to 130, the helper function hexToBytes is defined but never used or
exported; remove the unused function definition to eliminate dead code, or if
it’s intended for reuse, export it (export function hexToBytes...) and update
callers to import and use it; choose removal if not required by tests to keep
the fixture file minimal.
| * Test identities: | ||
| * - Identity 1: 4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi | ||
| * - Identity 2: 8pkwN93v4wTWzzJAJkBCVPQJHMGwNa9TYfJUW24sAq11 | ||
| * - Identity 3: CktRuQ2mttgRGkXJtyksdKHjUdc2C4TgDzyB98oEzy8 | ||
| * |
There was a problem hiding this comment.
Identity 2 ID in comment doesn't match fixture.
The comment on line 12 shows 8pkwN93v4wTWzzJAJkBCVPQJHMGwNa9TYfJUW24sAq11 but the fixture in requiredTestData.mjs defines identityId2 as 8qbHbw2BbbTHBW1sbeqakYXVKRQM8Ne7pLK7m6CVfeR.
🔎 Suggested fix
* Test identities:
* - Identity 1: 4vJ9JU1bJJE96FWSJKvHsmmFADCg4gpZQff4P3bkLKi
-* - Identity 2: 8pkwN93v4wTWzzJAJkBCVPQJHMGwNa9TYfJUW24sAq11
+* - Identity 2: 8qbHbw2BbbTHBW1sbeqakYXVKRQM8Ne7pLK7m6CVfeR
* - Identity 3: CktRuQ2mttgRGkXJtyksdKHjUdc2C4TgDzyB98oEzy8
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/wasm-sdk/src/state_transitions/identity.rs (1)
861-882: Consider removing unused helper function.The
ecdsa_public_key_matches_identity_keyfunction is well-implemented but doesn't appear to be called anywhere in this file. Since it's not markedpub, it's not accessible from other modules either.If this function is intended for future use, consider adding a
#[allow(dead_code)]attribute with a comment explaining its purpose. Otherwise, remove it to reduce maintenance burden.🧹 Verification script to confirm it's unused
#!/bin/bash # Search for any usage of ecdsa_public_key_matches_identity_key in the codebase rg -n "ecdsa_public_key_matches_identity_key" --type rust
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/wasm-sdk/src/state_transitions/identity.rs
🧰 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/state_transitions/identity.rs
🧠 Learnings (7)
📚 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/state_transitions/identity.rs
📚 Learning: 2024-10-03T11:51:06.980Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-03T11:51:06.980Z
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/state_transitions/identity.rs
📚 Learning: 2024-10-06T16:11:34.946Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs:19-30
Timestamp: 2024-10-06T16:11:34.946Z
Learning: In the Rust file `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs`, within the `create_owner_identity_v1` function, the `add_public_keys` method of the `Identity` struct cannot fail and does not require explicit error handling.
Applied to files:
packages/wasm-sdk/src/state_transitions/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/wasm-sdk/src/state_transitions/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/wasm-sdk/src/state_transitions/identity.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/state_transitions/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/state_transitions/identity.rs
🧬 Code graph analysis (1)
packages/wasm-sdk/src/state_transitions/identity.rs (3)
packages/wasm-sdk/src/queries/utils.rs (1)
deserialize_required_query(11-28)packages/wasm-sdk/src/settings.rs (4)
extract_settings_from_options(259-272)from(66-73)from(203-226)get_user_fee_increase(278-284)packages/wasm-sdk/src/state_transitions/addresses.rs (2)
extract_settings_from_options(88-101)identity(838-840)
⏰ 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). (19)
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (wasm-dpp2) / Tests
- GitHub Check: Rust packages (dpp) / Detect immutable structure changes
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / Build Drive image - GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (9)
packages/wasm-sdk/src/state_transitions/identity.rs (9)
89-127: LGTM! Well-structured identity creation flow.The implementation properly extracts all required options, handles errors with descriptive messages, and follows a clear async flow. The TypeScript interface documentation is comprehensive and helps developers understand the required parameters.
182-217: LGTM! Clean top-up implementation.The method correctly handles the identity top-up flow and returns the new balance as a BigInt. The optional parameter (None at line 209) appears appropriate for the top-up context where the asset lock key is sufficient.
304-354: LGTM! Robust credit transfer implementation.The transfer flow is well-implemented with proper handling of both required and optional parameters. The result struct provides clear access to both balances post-transfer, which is useful for callers.
463-467: Good validation! Zero-amount check prevents invalid withdrawals.The validation ensures that withdrawal requests have a meaningful amount before processing. The error message is clear and actionable.
473-482: LGTM! Safe address parsing with proper error handling.The optional address parsing correctly uses
transpose()to handle the nested Result/Option, and provides a clear error message if the address is malformed.
605-609: LGTM! Revision increment is correctly handled.The in-place revision increment is intentional and well-documented. The comment clearly explains that the platform expects the new revision in the state transition, which aligns with the implementation.
614-642: LGTM! Robust master key validation and selection.The code properly:
- Filters for valid master keys (AUTHENTICATION + MASTER + supported key types)
- Validates at least one master key exists
- Ensures the signer has a matching private key
This prevents invalid update attempts with clear error messages.
645-673: LGTM! Safe key ID assignment logic.The calculation
max().copied().unwrap_or(0) + 1correctly handles:
- Empty key maps (starts at 1)
- Sequential IDs (max + 1)
- Gaps in IDs (avoids collisions)
The loop then assigns sequential IDs to each new key being added.
799-859: LGTM! Clean masternode voting implementation.The voting flow is well-structured with proper extraction of vote components and construction of the vote structures. The use of the PutVote trait keeps the implementation clean and consistent with SDK patterns.
Issue being fixed or feature implemented
Currently, state transition methods accept many arguments instead of typed objects, which is a better UX.
What was done?
How Has This Been Tested?
With new functional and unit tests.
Enabled skiped tests.
Breaking Changes
Public API of Evo SDK is changed
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.