feat(dpp): add Identity new_with_input_addresses_and_keys() #2971
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughAdded a feature-gated Identity constructor that derives identity IDs from input addresses, extracted identity ID derivation into a reusable helper, flipped two verification flags to true, changed SDK APIs to require explicit address nonces, and added nonce-fetching logic in the WASM SDK. Changes
Sequence Diagram(s)sequenceDiagram
actor Client as WASM_SDK
participant RPC as Platform_RPC
participant SDK as Platform_SDK
participant DPP as rs-dpp
Client->>RPC: fetch_many(addresses)
RPC-->>Client: AddressInfo (with nonces)
Client->>Client: merge nonces into inputs_map
Client->>SDK: put_with_address_funding(inputs_with_nonce, keys, ...)
SDK->>DPP: identity_id_from_input_addresses(inputs_with_nonce)
DPP-->>SDK: Identity (constructed via new_with_input_addresses_and_keys)
SDK-->>Client: result / state transition broadcast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rs-sdk/src/platform/transition/put_identity.rs (1)
49-54: Clarify the documentation phrasing.The phrase "This is a preferred method" is ambiguous—it's unclear whether "this" refers to
put_with_address_funding_with_nonceor toIdentity::new_with_input_addresses_and_keys. Consider rephrasing for clarity.📝 Suggested documentation improvement
/// Creates an identity funded by Platform addresses using explicit nonces. /// /// Use [Identity::new_with_input_addresses_and_keys](dpp::identity::Identity::new_with_input_addresses_and_keys) -/// to create an identity. Then use this method to put it to the platform. +/// to create an identity with the same nonces provided to this method. /// -/// This is a preferred method, as you need to use the same nonces when creating the identity. +/// # Important +/// +/// The `inputs_with_nonce` parameter must contain the same nonces used when +/// constructing the identity via `new_with_input_addresses_and_keys`, as the +/// identity ID is derived from these addresses and nonces.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rs-sdk/src/platform/transition/put_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/rs-sdk/src/platform/transition/put_identity.rs
🧠 Learnings (6)
📚 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/rs-sdk/src/platform/transition/put_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/rs-sdk/src/platform/transition/put_identity.rs
📚 Learning: 2024-09-30T12:11:35.148Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2186
File: packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/mod.rs:48-54
Timestamp: 2024-09-30T12:11:35.148Z
Learning: In the identity credit withdrawal transition code, the field `platform_version.drive_abci.validation_and_processing.state_transitions.identity_credit_withdrawal_state_transition.transform_into_action` is not an `Option` type, so handling `None` cases is unnecessary.
Applied to files:
packages/rs-sdk/src/platform/transition/put_identity.rs
📚 Learning: 2024-11-28T13:49:17.301Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2317
File: packages/rs-dapi-client/src/address_list.rs:175-180
Timestamp: 2024-11-28T13:49:17.301Z
Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.
Applied to files:
packages/rs-sdk/src/platform/transition/put_identity.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/rs-sdk/src/platform/transition/put_identity.rs
📚 Learning: 2025-10-09T15:59:28.329Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187
Timestamp: 2025-10-09T15:59:28.329Z
Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.
Applied to files:
packages/rs-sdk/src/platform/transition/put_identity.rs
🧬 Code graph analysis (1)
packages/rs-sdk/src/platform/transition/put_identity.rs (2)
packages/rs-sdk/src/platform/transition/address_inputs.rs (1)
collect_address_infos_from_proof(44-90)packages/rs-sdk/src/platform/transition/broadcast.rs (2)
broadcast(23-23)broadcast(38-93)
⏰ 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). (20)
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / Build Drive image - GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
- GitHub Check: Swift SDK and Example build (warnings as errors)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
🔇 Additional comments (4)
packages/rs-sdk/src/platform/transition/put_identity.rs (4)
3-3: LGTM: Import changes align with API simplification.The import changes correctly reflect the removal of automatic nonce fetching functionality, keeping only the items needed for explicit nonce-based identity creation.
55-63: LGTM: Method signature is well-designed.The explicit nonce-based API is clear and type-safe. The return of both the
IdentityandAddressInfosprovides useful information to callers.
108-127: LGTM: Clean delegation pattern.The implementation correctly delegates to the helper function with all parameters forwarded appropriately.
149-211: LGTM: Robust implementation with proper verification.The helper function correctly:
- Creates the state transition with explicit nonces
- Validates the structure before broadcasting
- Verifies the proved identity ID matches the expected ID
- Collects and validates address information from the proof
The error handling is appropriate for all edge cases.
lklimek
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/wasm-sdk/src/state_transitions/addresses.rs (1)
1047-1059: Suggest more robust merge logic using key lookup instead of zip.The current implementation uses
zipto mergeinputs_mapandfetched_addresses, relying on BTreeMap's sorted iteration order. While both maps should have identical keys (validated at line 1041), usingzipis fragile and requires the defensiveaddress_left != address_rightcheck.Consider a more explicit approach that directly looks up each address:
♻️ Proposed refactor using key lookup
- // merge nonces into inputs map - let inputs = inputs_map - .into_iter() - .zip(fetched_addresses) - .map(|((address_left, amount), (address_right, info))| { - if address_left != address_right { - Err(WasmSdkError::generic( - "Address mismatch when merging nonces for identity creation; platform bug?", - ))? - } - let nonce = info.nonce; - Ok((address_left, (nonce, amount))) - }) - .collect::<Result<BTreeMap<_, _>, WasmSdkError>>()?; + // merge nonces into inputs map + let inputs = inputs_map + .into_iter() + .map(|(address, amount)| { + let info = fetched_addresses.get(&address).ok_or_else(|| { + WasmSdkError::generic(format!( + "Address {} not found in fetched addresses; platform bug?", + address + )) + })?; + let nonce = info.nonce; + Ok((address, (nonce, amount))) + }) + .collect::<Result<BTreeMap<_, _>, WasmSdkError>>()?;packages/rs-sdk/src/platform/transition/put_identity.rs (1)
49-63: Consider clarifying nonce consistency requirement in documentation.The documentation helpfully explains that
new_with_input_addresses_and_keysshould be used to create the identity and that "you need to use the same nonces when creating the identity." Consider adding a note about what happens if the nonces don't match (e.g., transaction failure, identity ID mismatch) to make the requirement more explicit.Example addition:
/// Use [Identity::new_with_input_addresses_and_keys](dpp::identity::Identity::new_with_input_addresses_and_keys) /// to create an identity. Then use this method to put it to the platform. /// /// This is a preferred method, as you need to use the same nonces when creating the identity. /// The identity ID is derived from the input addresses and their nonces, so using different /// nonces will result in an identity ID mismatch and transaction failure.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rs-sdk/src/platform/transition/put_identity.rspackages/wasm-sdk/src/state_transitions/addresses.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/addresses.rspackages/rs-sdk/src/platform/transition/put_identity.rs
🧠 Learnings (10)
📓 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: CR
Repo: dashpay/platform PR: 0
File: packages/swift-sdk/SwiftExampleApp/CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:38.019Z
Learning: Applies to packages/swift-sdk/SwiftExampleApp/**/{KeychainManager,*Key,*Identity}.swift : Store private keys separately from identities using iOS Keychain for secure storage; private keys belong to public keys, not identities
📚 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/addresses.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/state_transitions/addresses.rspackages/rs-sdk/src/platform/transition/put_identity.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/state_transitions/addresses.rspackages/rs-sdk/src/platform/transition/put_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/addresses.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: Applies to **/swift-sdk/**/*.{swift,h,m,c} : iOS SDK must combine both Core (SPV wallet) and Platform (identity/documents) functionality in the unified SDK with proper function naming: `dash_core_sdk_*` prefix for Core functions, `dash_sdk_*` prefix for Platform functions, and `dash_unified_sdk_*` prefix for unified functions
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2186
File: packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/mod.rs:48-54
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the identity credit withdrawal transition code, the field `platform_version.drive_abci.validation_and_processing.state_transitions.identity_credit_withdrawal_state_transition.transform_into_action` is not an `Option` type, so handling `None` cases is unnecessary.
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.rspackages/rs-sdk/src/platform/transition/put_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/rs-sdk/src/platform/transition/put_identity.rs
📚 Learning: 2024-11-28T13:49:17.301Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2317
File: packages/rs-dapi-client/src/address_list.rs:175-180
Timestamp: 2024-11-28T13:49:17.301Z
Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.
Applied to files:
packages/rs-sdk/src/platform/transition/put_identity.rs
📚 Learning: 2025-10-09T15:59:28.329Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187
Timestamp: 2025-10-09T15:59:28.329Z
Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.
Applied to files:
packages/rs-sdk/src/platform/transition/put_identity.rs
🧬 Code graph analysis (2)
packages/wasm-sdk/src/state_transitions/addresses.rs (3)
packages/rs-sdk/src/platform/transition/put_identity.rs (1)
inputs(162-162)packages/wasm-sdk/src/error.rs (1)
generic(71-73)packages/wasm-sdk/src/queries/address.rs (1)
nonce(42-44)
packages/rs-sdk/src/platform/transition/put_identity.rs (2)
packages/rs-sdk/src/platform/transition/address_inputs.rs (1)
collect_address_infos_from_proof(44-90)packages/rs-sdk/src/platform/transition/broadcast.rs (2)
broadcast(23-23)broadcast(38-93)
⏰ 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). (21)
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Build Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / Build Drive image - GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
- 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 (7)
packages/wasm-sdk/src/state_transitions/addresses.rs (4)
5-5: LGTM: Import additions support new nonce-fetching functionality.The new imports (BTreeMap, BTreeSet, FetchMany, Identity, Sdk, Credits, AddressNonce) are necessary for the fetch_nonces_into_address_map helper and updated type signatures.
Also applies to: 18-21
976-977: LGTM: Correct nonce fetching and passing.The function correctly fetches nonces using the new
fetch_nonces_into_address_maphelper (line 977) and passes the nonce-enriched inputs toput_with_address_funding(line 996), aligning with the new API requirement for explicit nonces.Also applies to: 996-996
171-201: No changes needed—the implementation is correct.The
transfer_address_fundsmethod in the SDK already handles nonce fetching internally. At line 55 ofpackages/rs-sdk/src/platform/transition/transfer_address_funds.rs, the implementation callsfetch_inputs_with_nonce(self, &inputs).await?before delegating totransfer_address_funds_with_nonce. The comment at line 193 in the reviewed code correctly documents this behavior ("which handles nonces, building, and broadcasting").The pattern differs from
identity_create_from_addresses(which explicitly callsfetch_nonces_into_address_mapbefore proceeding), but both approaches are valid. Nonces are properly fetched and incremented in either case.
303-341: Nonces are fetched internally by the underlying RS SDK method.The
top_up_from_addressescall delegates nonce fetching to the RS SDK'sIdentity::top_up_from_addressesimplementation, which automatically fetches nonces viafetch_inputs_with_noncebefore creating the transition. This is documented in the RS SDK method's comment: "nonces looked up automatically."However, this differs from the pattern in
address_funds_transferandidentity_create_from_addressesin the same file, which both explicitly callfetch_nonces_into_address_mapat the WASM layer. Consider aligning the pattern here for consistency.packages/rs-sdk/src/platform/transition/put_identity.rs (3)
3-3: LGTM: Removed unused imports.The removal of
fetch_inputs_with_nonceandnonce_incis correct since the method now requires nonces to be provided by the caller rather than fetching them internally.
108-127: LGTM: Clean delegation to helper function.The implementation correctly delegates to the helper function with all parameters passed through.
149-210: LGTM: Correct implementation with proper validation.The function correctly:
- Uses caller-provided nonces (no internal fetching)
- Creates the state transition with
try_from_inputs_with_signer- Validates that the proved identity ID matches the expected identity ID (lines 192-199)
- Collects and returns address infos with proper validation
The identity ID validation is critical for preventing mismatches when the identity is derived from input addresses and nonces.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/rs-dpp/src/identity/identity.rs:
- Around line 122-146: The doc comment for new_with_input_addresses_and_keys is
inaccurate: the inputs map value is (AddressNonce, Credits), not just
AddressNonce, and Credits are ignored when deriving the identity id; update the
function doc to state that inputs is a BTreeMap<PlatformAddress, (AddressNonce,
Credits)> and explicitly note that only the address and AddressNonce are used by
identity_id_from_input_addresses (Credits are not part of ID derivation). Keep
the existing call to crate::state_transition::identity_id_from_input_addresses
and ensure the docs mention that function as the derivation used.
In @packages/wasm-sdk/src/state_transitions/addresses.rs:
- Around line 1014-1062: The function fetch_nonces_into_address_map should first
validate that inputs_map is non-empty and return
WasmSdkError::invalid_argument(...) when empty; keep using
dash_sdk::query_types::AddressInfo::fetch_many to fetch infos but replace the
zip-based merge with a key-based join: iterate over inputs_map.keys(), look up
each key in fetched_addresses (the BTreeMap from fetch_many), and for any
missing key return WasmSdkError::invalid_argument(...) indicating which input
addresses were not found/are empty; when present, extract info.nonce and
preserve the Credits value to build the resulting BTreeMap<(PlatformAddress,
(AddressNonce, Credits))>; update all validation errors to use
WasmSdkError::invalid_argument instead of WasmSdkError::generic and remove the
address-left/address-right mismatch logic.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rs-dpp/src/identity/identity.rspackages/wasm-sdk/src/state_transitions/addresses.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/addresses.rspackages/rs-dpp/src/identity/identity.rs
🧠 Learnings (9)
📓 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: 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.
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.
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/tests/fetch/config.rs:233-233
Timestamp: 2024-10-30T11:19:59.163Z
Learning: In the Rust SDK's `rs-sdk/tests` integration tests (e.g., in `packages/rs-sdk/tests/fetch/config.rs`), we cannot create objects during tests because there is no support for object creation in this context. Therefore, hardcoded values for test identities must be used.
📚 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/addresses.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/addresses.rspackages/rs-dpp/src/identity/identity.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/state_transitions/addresses.rs
📚 Learning: 2024-11-28T13:49:17.301Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2317
File: packages/rs-dapi-client/src/address_list.rs:175-180
Timestamp: 2024-11-28T13:49:17.301Z
Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.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/addresses.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 WebAssembly as a bridge between Rust and JavaScript implementations
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.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/rs-dpp/src/identity/identity.rs
📚 Learning: 2024-11-20T20:43:41.185Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/tests/strategy_tests/masternodes.rs:212-220
Timestamp: 2024-11-20T20:43:41.185Z
Learning: In `packages/rs-drive-abci/tests/strategy_tests/masternodes.rs`, the pattern of generating a `PrivateKey`, converting it to bytes, and reconstructing a `BlsPrivateKey` from those bytes is intentional. Avoid suggesting to simplify this code in future reviews.
Applied to files:
packages/rs-dpp/src/identity/identity.rs
🧬 Code graph analysis (2)
packages/wasm-sdk/src/state_transitions/addresses.rs (2)
packages/rs-sdk/src/platform/transition/put_identity.rs (1)
inputs(162-162)packages/wasm-sdk/src/error.rs (1)
generic(71-73)
packages/rs-dpp/src/identity/identity.rs (1)
packages/rs-dpp/src/state_transition/traits/state_transition_identity_id_from_inputs.rs (1)
identity_id_from_input_addresses(25-44)
⏰ 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). (21)
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (drive) / 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-sim)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Swift SDK and Example build (warnings as errors)
🔇 Additional comments (2)
packages/rs-dpp/src/identity/identity.rs (1)
1-4: Import additions look fine; ensure they’re feature-justified.
No issues here, assumingAddressNonceis only needed for the new constructor path.packages/wasm-sdk/src/state_transitions/addresses.rs (1)
974-1012: Good flow change: nonce-enrich inputs beforeput_with_address_funding.
Fetching nonces here keeps the public JS API simpler and matches the “nonces fetched automatically” docs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/wasm-sdk/src/state_transitions/addresses.rs:
- Line 1040: Fix the typo in the inline comment near the filter_map check in
addresses.rs: change "shouuld" to "should" in the comment that currently reads
"// sanity check - filter_map above shouuld have removed any non-existing
addresses" so the comment reads "... should have removed any non-existing
addresses".
🧹 Nitpick comments (2)
packages/wasm-sdk/src/state_transitions/addresses.rs (1)
1047-1069: Consider simplifying the merge logic to avoid relying on BTreeMap ordering.The current
zipapproach works because bothBTreeMaps have identical keys in the same order, and the address mismatch check (line 1052) validates this. However, this relies on a subtle ordering assumption that could be fragile during future refactoring.A clearer approach would directly look up each input address in the fetched map:
♻️ Alternative implementation
- // merge nonces into inputs map - let inputs = inputs_map - .into_iter() - .zip(fetched_addresses) - .map( - |((address_requested, amount), (address_received, info_received))| { - if address_requested != address_received { - Err(WasmSdkError::invalid_argument( - format!("Address mismatch when merging nonces for identity creation ({} vs {}); platform bug?", - address_requested, address_received) - ))? - } - if amount > info_received.balance { - Err(WasmSdkError::invalid_argument(format!( - "Input address {} has insufficient balance: requested {}, available {}", - address_requested, amount, info_received.balance - )))? - } - let nonce = info_received.nonce; - Ok((address_requested, (nonce, amount))) - }, - ) - .collect::<Result<BTreeMap<_, _>, WasmSdkError>>()?; + // merge nonces into inputs map + let inputs = inputs_map + .into_iter() + .map(|(address, amount)| { + let info = fetched_addresses.get(&address).ok_or_else(|| { + WasmSdkError::invalid_argument(format!( + "Address {} not found when merging nonces; platform bug?", + address + )) + })?; + if amount > info.balance { + return Err(WasmSdkError::invalid_argument(format!( + "Input address {} has insufficient balance: requested {}, available {}", + address, amount, info.balance + ))); + } + Ok((address, (info.nonce, amount))) + }) + .collect::<Result<BTreeMap<_, _>, WasmSdkError>>()?;This approach is more explicit about the lookup relationship and doesn't depend on iterator ordering alignment.
packages/rs-dpp/src/identity/identity.rs (1)
122-147: Nonce semantics are well-documented; prioritize unit tests over implementation verificationThe implementation correctly delegates to the validated
identity_id_from_input_addresseshelper and existing constructor. The nonce semantics concern is valid but already documented consistently across the codebase—the helper function (line 22 instate_transition_identity_id_from_inputs.rs) and SDK documentation both state that "nonces should represent state after creation of the identity (e.g., be incremented by 1)." This is an intentional design requiring callers to use post-transaction nonce values, and the wasm-sdk shows the proper usage pattern (fetching current nonces and incrementing them before passing to the constructor).Focus on test coverage instead: The constructor lacks direct unit tests. Add tests demonstrating:
- Correct identity ID generation from various input combinations
- Proper error handling for empty inputs
- The nonce increment requirement and its effect on derived identity ID
- Consistency with existing identity creation flow
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rs-dpp/src/identity/identity.rspackages/wasm-sdk/src/state_transitions/addresses.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/rs-dpp/src/identity/identity.rspackages/wasm-sdk/src/state_transitions/addresses.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: 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.
📚 Learning: 2024-10-06T16:11:34.946Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs:19-30
Timestamp: 2024-10-06T16:11:34.946Z
Learning: In the Rust file `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs`, within the `create_owner_identity_v1` function, the `add_public_keys` method of the `Identity` struct cannot fail and does not require explicit error handling.
Applied to files:
packages/rs-dpp/src/identity/identity.rspackages/wasm-sdk/src/state_transitions/addresses.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/rs-dpp/src/identity/identity.rspackages/wasm-sdk/src/state_transitions/addresses.rs
📚 Learning: 2024-11-20T20:43:41.185Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/tests/strategy_tests/masternodes.rs:212-220
Timestamp: 2024-11-20T20:43:41.185Z
Learning: In `packages/rs-drive-abci/tests/strategy_tests/masternodes.rs`, the pattern of generating a `PrivateKey`, converting it to bytes, and reconstructing a `BlsPrivateKey` from those bytes is intentional. Avoid suggesting to simplify this code in future reviews.
Applied to files:
packages/rs-dpp/src/identity/identity.rspackages/wasm-sdk/src/state_transitions/addresses.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2186
File: packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_credit_withdrawal/mod.rs:48-54
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the identity credit withdrawal transition code, the field `platform_version.drive_abci.validation_and_processing.state_transitions.identity_credit_withdrawal_state_transition.transform_into_action` is not an `Option` type, so handling `None` cases is unnecessary.
Applied to files:
packages/rs-dpp/src/identity/identity.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/rs-dpp/src/identity/identity.rspackages/wasm-sdk/src/state_transitions/addresses.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/rs-dpp/src/identity/identity.rspackages/wasm-sdk/src/state_transitions/addresses.rs
📚 Learning: 2024-11-28T13:49:17.301Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2317
File: packages/rs-dapi-client/src/address_list.rs:175-180
Timestamp: 2024-11-28T13:49:17.301Z
Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.rs
📚 Learning: 2024-11-20T16:05:40.200Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs:148-151
Timestamp: 2024-11-20T16:05:40.200Z
Learning: In `packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs`, when converting public keys from `QuorumForSavingV0` to `VerificationQuorum`, it's acceptable to use `.expect()` for public key conversion, as the conversion has been verified and panics are acceptable in this context.
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.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/state_transitions/addresses.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/addresses.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 WebAssembly as a bridge between Rust and JavaScript implementations
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.rs
🧬 Code graph analysis (1)
packages/rs-dpp/src/identity/identity.rs (1)
packages/rs-dpp/src/state_transition/traits/state_transition_identity_id_from_inputs.rs (1)
identity_id_from_input_addresses(25-44)
⏰ 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). (21)
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (wasm-dpp2) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Tests
- 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 Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / Build Drive 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 (8)
packages/wasm-sdk/src/state_transitions/addresses.rs (7)
5-5: LGTM! Import additions support the new nonce-fetching functionality.The new imports are all necessary for the
fetch_nonces_into_address_maphelper function.Also applies to: 13-13, 15-15, 20-21
171-201: Inconsistency between AI summary and actual changes.The AI summary states that
address_funds_transferwas updated to usefetch_nonces_into_address_map, but the provided code shows no such changes to this method. It continues to calltransfer_address_fundsdirectly without the new helper.This might indicate:
- The changes were made elsewhere (in the SDK layer)
- The summary is incorrect
- Changes are missing from this file
Please verify whether
address_funds_transfershould also be updated to fetch nonces explicitly, or if the SDK'stransfer_address_fundsmethod handles this internally.
976-977: LGTM! Correct integration of nonce-fetching logic.The changes properly fetch nonces before creating the identity and pass the enriched inputs map (with nonces) to
put_with_address_funding. The type transition fromBTreeMap<PlatformAddress, Credits>toBTreeMap<PlatformAddress, (AddressNonce, Credits)>is handled correctly.Also applies to: 996-996
1014-1022: LGTM! Clear documentation and signature.The helper function is properly scoped as private, has clear documentation, and uses appropriate types.
1023-1038: LGTM! Clean address fetching logic.The code efficiently fetches all input addresses in a single call and properly handles the conversion from
IndexMaptoBTreeMap.
1041-1045: LGTM! Proper validation of fetched addresses.The length check ensures all requested addresses were successfully fetched from the platform.
1070-1071: LGTM! Clean return statement.The function correctly returns the merged map with the expected type.
packages/rs-dpp/src/identity/identity.rs (1)
1-4: LGTM: Clean import additionsThe new imports for
PlatformAddressandAddressNonceare appropriately added to support the new constructor, and movingRevisionto the prelude is a reasonable refactoring.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/wasm-sdk/src/state_transitions/addresses.rs:
- Around line 1047-1068: The current use of zip() to pair inputs_map and
fetched_addresses is unsafe because it relies on positional alignment; instead
build an indexed lookup for fetched_addresses (e.g., convert fetched_addresses
into a map keyed by address_received) and then iterate inputs_map.into_iter(),
for each (address_requested, amount) perform an explicit lookup in that fetched
map, return an error if the address is missing, check amount <=
info_received.balance and extract nonce, and collect the results into the same
Result<BTreeMap<_, _>, WasmSdkError>() so order/pairing is guaranteed; update
the closure that previously matched |((address_requested, amount),
(address_received, info_received))| accordingly to use a lookup by
address_requested and preserve the same error messages (including address
mismatch/insufficient balance) and returned tuple shape (address_requested,
(nonce, amount)).
🧹 Nitpick comments (1)
packages/wasm-sdk/src/state_transitions/addresses.rs (1)
1024-1025: Consider using.copied()ifPlatformAddressimplementsCopy.If
PlatformAddressimplementsCopy, using.copied().collect()is slightly more idiomatic than.cloned().collect().♻️ Potential simplification
- let input_addresses: BTreeSet<PlatformAddress> = - inputs_map.keys().cloned().collect::<BTreeSet<_>>(); + let input_addresses: BTreeSet<PlatformAddress> = + inputs_map.keys().copied().collect::<BTreeSet<_>>();
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/wasm-sdk/src/state_transitions/addresses.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/addresses.rs
🧠 Learnings (13)
📓 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.
📚 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/addresses.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/addresses.rs
📚 Learning: 2024-11-28T13:49:17.301Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2317
File: packages/rs-dapi-client/src/address_list.rs:175-180
Timestamp: 2024-11-28T13:49:17.301Z
Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.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/state_transitions/addresses.rs
📚 Learning: 2024-11-20T16:05:40.200Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs:148-151
Timestamp: 2024-11-20T16:05:40.200Z
Learning: In `packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs`, when converting public keys from `QuorumForSavingV0` to `VerificationQuorum`, it's acceptable to use `.expect()` for public key conversion, as the conversion has been verified and panics are acceptable in this context.
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.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/addresses.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/state_transitions/addresses.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/state_transitions/addresses.rs
📚 Learning: 2024-11-03T10:39:11.242Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2305
File: packages/rs-drive-abci/src/abci/handler/finalize_block.rs:81-0
Timestamp: 2024-11-03T10:39:11.242Z
Learning: In `packages/rs-drive-abci/src/abci/handler/finalize_block.rs`, the use of `.expect("commit transaction")` after `app.commit_transaction(platform_version)` is intentional due to the provided comment explaining its necessity. Do not flag this usage in future reviews.
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.rs
📚 Learning: 2024-12-03T15:00:50.385Z
Learnt from: lklimek
Repo: dashpay/platform PR: 1924
File: packages/rs-sdk/src/sdk.rs:855-870
Timestamp: 2024-12-03T15:00:50.385Z
Learning: In `packages/rs-sdk/src/sdk.rs`, remember that `rustls_pemfile::certs` returns an iterator over `Result`, not a `Result` directly. Handle it accordingly in future code reviews.
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.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/addresses.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 WebAssembly as a bridge between Rust and JavaScript implementations
Applied to files:
packages/wasm-sdk/src/state_transitions/addresses.rs
🧬 Code graph analysis (1)
packages/wasm-sdk/src/state_transitions/addresses.rs (2)
packages/rs-sdk/src/platform/transition/put_identity.rs (1)
inputs(162-162)packages/wasm-sdk/src/error.rs (2)
generic(71-73)invalid_argument(75-77)
⏰ 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). (14)
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios)
- GitHub Check: Build rs-sdk-ffi for iOS targets (aarch64-apple-ios-sim)
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / Build Drive image - GitHub Check: Build JS packages / Build JS
🔇 Additional comments (2)
packages/wasm-sdk/src/state_transitions/addresses.rs (2)
5-21: LGTM! Imports are appropriate for the new functionality.The added imports support the nonce fetching logic and are all utilized in the new helper function.
976-977: Integration with helper function looks correct.The call to
fetch_nonces_into_address_mapproperly enhances the inputs before passing them to the SDK method.
Comment on lines
+1047
to
+1068
| let inputs = inputs_map | ||
| .into_iter() | ||
| .zip(fetched_addresses) | ||
| .map( | ||
| |((address_requested, amount), (address_received, info_received))| { | ||
| if address_requested != address_received { | ||
| Err(WasmSdkError::invalid_argument( | ||
| format!("Address mismatch when merging nonces for identity creation ({} vs {}); platform bug?", | ||
| address_requested, address_received) | ||
| ))? | ||
| } | ||
| if amount > info_received.balance { | ||
| Err(WasmSdkError::invalid_argument(format!( | ||
| "Input address {} has insufficient balance: requested {}, available {}", | ||
| address_requested, amount, info_received.balance | ||
| )))? | ||
| } | ||
| let nonce = info_received.nonce; | ||
| Ok((address_requested, (nonce, amount))) | ||
| }, | ||
| ) | ||
| .collect::<Result<BTreeMap<_, _>, WasmSdkError>>()?; |
Contributor
There was a problem hiding this comment.
Critical: Unsafe zip operation can cause address/nonce mismatches.
Using zip() to combine two BTreeMap iterators by position is unsafe. Even though BTreeMap maintains sorted order, after the filter_map operation on line 1037 that removes None values, there's no guarantee that inputs_map and fetched_addresses will have the same iteration order or even contain exactly the same keys.
If the ordering differs for any reason, address A could be paired with the nonce for address B, causing incorrect signature creation and transaction failures.
🔒 Proposed fix: Explicitly look up each address
// merge nonces into inputs map
let inputs = inputs_map
.into_iter()
- .zip(fetched_addresses)
- .map(
- |((address_requested, amount), (address_received, info_received))| {
- if address_requested != address_received {
- Err(WasmSdkError::invalid_argument(
- format!("Address mismatch when merging nonces for identity creation ({} vs {}); platform bug?",
- address_requested, address_received)
- ))?
- }
+ .map(|(address, amount)| {
+ let info = fetched_addresses.get(&address).ok_or_else(|| {
+ WasmSdkError::invalid_argument(format!(
+ "Address {} not found in fetched results",
+ address
+ ))
+ })?;
+
- if amount > info_received.balance {
+ if amount > info.balance {
Err(WasmSdkError::invalid_argument(format!(
- "Input address {} has insufficient balance: requested {}, available {}",
- address_requested, amount, info_received.balance
+ "Input address {} has insufficient balance: requested {}, available {}",
+ address, amount, info.balance
)))?
}
- let nonce = info_received.nonce;
- Ok((address_requested, (nonce, amount)))
+ let nonce = info.nonce;
+ Ok((address, (nonce, amount)))
},
)
.collect::<Result<BTreeMap<_, _>, WasmSdkError>>()?;🤖 Prompt for AI Agents
In @packages/wasm-sdk/src/state_transitions/addresses.rs around lines 1047 -
1068, The current use of zip() to pair inputs_map and fetched_addresses is
unsafe because it relies on positional alignment; instead build an indexed
lookup for fetched_addresses (e.g., convert fetched_addresses into a map keyed
by address_received) and then iterate inputs_map.into_iter(), for each
(address_requested, amount) perform an explicit lookup in that fetched map,
return an error if the address is missing, check amount <= info_received.balance
and extract nonce, and collect the results into the same Result<BTreeMap<_, _>,
WasmSdkError>() so order/pairing is guaranteed; update the closure that
previously matched |((address_requested, amount), (address_received,
info_received))| accordingly to use a lookup by address_requested and preserve
the same error messages (including address mismatch/insufficient balance) and
returned tuple shape (address_requested, (nonce, amount)).
Contributor
Author
There was a problem hiding this comment.
We have safety checks there (comparing length + comparing addresses), there is no reason for ordering to change, and even if there is we will detect it.
Lookup will be more expensive.
Contributor
There was a problem hiding this comment.
@lklimek, you're absolutely right. Since both inputs_map and fetched_addresses are BTreeMaps with the same keys (verified by the length check), they will iterate in the same sorted order, and the explicit address comparison check will catch any unexpected issues. The zip approach is both correct and more efficient here. Thank you for the clarification!
🧠 Learnings used
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.
Learnt from: lklimek
Repo: dashpay/platform PR: 2254
File: packages/rs-sdk/src/sdk.rs:654-658
Timestamp: 2024-10-18T15:43:32.447Z
Learning: In `packages/rs-sdk/src/sdk.rs`, within the `verify_metadata_height` function, when using `compare_exchange` on `last_seen_height`, it's acceptable to use `Ordering::Relaxed` for the failure ordering, as any inconsistency would only trigger an extra loop iteration without affecting correctness.
Learnt from: shumkov
Repo: dashpay/platform PR: 2317
File: packages/rs-dapi-client/src/address_list.rs:175-180
Timestamp: 2024-11-28T13:49:17.301Z
Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.
Learnt from: shumkov
Repo: dashpay/platform PR: 2351
File: packages/rs-dapi-client/src/address_list.rs:241-250
Timestamp: 2024-11-28T14:08:53.428Z
Learning: In the `IntoIterator` implementation for `AddressList` in `packages/rs-dapi-client/src/address_list.rs`, since `self` is consumed, holding a write lock while converting to an iterator does not block other operations.
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.
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.
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.
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs:148-151
Timestamp: 2024-11-20T16:05:40.200Z
Learning: In `packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs`, when converting public keys from `QuorumForSavingV0` to `VerificationQuorum`, it's acceptable to use `.expect()` for public key conversion, as the conversion has been verified and panics are acceptable in this context.
Learnt from: lklimek
Repo: dashpay/platform PR: 1924
File: packages/rs-sdk/src/sdk.rs:855-870
Timestamp: 2024-12-03T15:00:50.385Z
Learning: In `packages/rs-sdk/src/sdk.rs`, remember that `rustls_pemfile::certs` returns an iterator over `Result`, not a `Result` directly. Handle it accordingly in future code reviews.
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Identity creation with funds from platform addresses is confusing:
it's not clear how to create identity ID.
What was done?
Added Identity::new_with_input_addresses_and_keys() constructor that generated identity ID internally.
How Has This Been Tested?
With dash evo tool
Breaking Changes
None, new feature
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Bug Fixes
New Features
Chores / Breaking Change
✏️ Tip: You can customize this high-level summary in your review settings.