feat(platform): get identities by non-unique public key hashes #2507
Conversation
commit 61ad58a Merge: 8d99708 6161f75 Author: QuantumExplorer <[email protected]> Date: Mon Mar 17 21:39:12 2025 +0700 Merge branch 'v2.0-dev' into feat/getIdentitiesByPublicKeyHashes1 commit 8d99708 Author: Quantum Explorer <[email protected]> Date: Thu Mar 6 21:04:45 2025 +0700 more work commit 0f26760 Author: Quantum Explorer <[email protected]> Date: Mon Mar 3 21:03:26 2025 +0700 temp work
chore: update to latest dash core 37 (#2483) feat(platform)!: token advanced distribution and updates (#2471) fix: token history contract (#2474) Co-authored-by: Ivan Shumkov <[email protected]> Co-authored-by: QuantumExplorer <[email protected]> fix(drive): using new rust dash core methods for reversed quorum hash to maintain backwards compatibility (#2489) feat: more granular integer document property types (#2455) Co-authored-by: Quantum Explorer <[email protected]> docs: update comment for data contract code range (#2476) feat: validate token name localizations (#2468) feat(sdk): get identity by non-unique keys build(deps): update grovedb to current develop test: test identity by non-unique pubkey hashes fix(sdk): dash core client fails to get quorum chore: minor fixes test(drive-abci): identity by non-unique pubkey start after chore: minor changes to verify feat(sdk): token and group queries (#2449) chore: revert limit 1 => limit none chore: add non-unique key to test identities test(sdk): test vectors for test_fetch_identity_by_non_unique_public_keys fix(platform)!: token distribution fixes and tests (#2494) chore(platform): bump to version 2.0.0-dev.1 (#2495) test: update assertion fix(sdk): make some things public (#2496) feat(platform): require token for document actions (#2498) fix: data contract proof doesn't work with new auto fields (#2501)
WalkthroughThis change set introduces comprehensive support for querying, fetching, proving, and verifying identities by non-unique public key hashes across multiple Rust packages. It adds new gRPC methods, protocol buffer definitions, Rust modules, and struct fields to distinguish between unique and non-unique public key hash queries. Numerous functions and fields are renamed for clarity, and new ones are added to handle the non-unique case. Tests, trait implementations, and versioning mechanisms are updated accordingly. The changes also include updates to versioning structures, conditional compilation, and test vectors, ensuring the new functionality is integrated and tested throughout the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant gRPC_Service
participant Platform
participant Drive
Client->>gRPC_Service: getIdentityByNonUniquePublicKeyHash(request)
gRPC_Service->>Platform: query_identity_by_non_unique_public_key_hash(request, state, version)
Platform->>Drive: fetch_full_identity_by_non_unique_public_key_hash(pubkey_hash, after, tx, version)
alt Prove requested
Drive->>Drive: prove_full_identity_by_non_unique_public_key_hash(pubkey_hash, after, tx, version)
Drive-->>Platform: IdentityAndNonUniquePublicKeyHashDoubleProof
Platform-->>gRPC_Service: Response with proof
else No prove
Drive-->>Platform: Option<Identity>
Platform-->>gRPC_Service: Response with identity bytes
end
gRPC_Service-->>Client: GetIdentityByNonUniquePublicKeyHashResponse
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (20)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
QuantumExplorer
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/rs-drive-verify-c-binding/src/lib.rs (1)
103-116: 💡 Verification agent🧩 Analysis chain
Rename function to clarify unique key hash usage
Renaming the function toverify_identity_id_by_unique_public_key_hashmakes the distinction clearer between unique and non-unique key hashes. Ensure that all references to the previous function name are fully updated across the codebase.
🏁 Script executed:
#!/bin/bash # Description: Verify that references to the old function name are fully replaced. rg "verify_identity_id_by_public_key_hash"Length of output: 391
Update Required: Inconsistent Function Name Usage Across Codebase
The Rust function in
packages/rs-drive-verify-c-binding/src/lib.rswas correctly renamed toverify_identity_id_by_unique_public_key_hashto clarify its unique key hash usage. However, our grep search shows that the C code inpackages/rs-drive-verify-c-binding/c/main.cstill references the old name (verify_identity_id_by_public_key_hash). Please update these references (including test function names) to reflect the new function name consistently across the codebase.
- File:
packages/rs-drive-verify-c-binding/c/main.c
- Update the call from
verify_identity_id_by_public_key_hashtoverify_identity_id_by_unique_public_key_hash- Rename test functions (e.g.,
test_verify_identity_id_by_public_key_hash) accordingly
🧹 Nitpick comments (14)
packages/rs-sdk/tests/fetch/identity.rs (1)
151-180: Consider making test assertions more environment-independentThe test correctly implements paging through identities using the
afterparameter, but the hard assertion for exactly 3 identities per non-unique key may be too specific to your test environment.In a production environment, this assumption could cause test failures if the number of identities sharing a public key changes.
Consider modifying the assertion to be more flexible:
- assert_eq!( - count, 3, - "expected exactly 3 identities with this non-unique public key" - ); + assert!( + count > 0, + "expected at least one identity with this non-unique public key" + );packages/rs-drive/src/drive/mod.rs (1)
281-287: Add a subtree path vector for non-unique key hashes
This function offers a handy vector-based path for non-unique public key hashes, mirroring the pattern used for unique key hashes. Consider adding dedicated unit tests to ensure coverage and guard against potential path construction errors.packages/rs-sdk/src/platform/types/identity.rs (2)
79-85: IntroduceNonUniquePublicKeyHashQuerystruct
This struct is well-defined for handling queries tied to non-unique public key hashes. Consider adding a clarifying comment or example about howafteris used in pagination or ordering logic.
86-107: ImplementQuery<IdentityRequest>forNonUniquePublicKeyHashQuery
The logic mirrors the approach for unique public key hashes, maintaining consistency. Optionally add test coverage for edge cases (e.g., invalid or absent hashes, usage ofafter) to further validate correctness.packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/mod.rs (1)
13-14: Doc referencesDriveVersionbut the code usesplatform_version
The doc comment in lines 13–14 references aDriveVersion, yet the function parameter is actuallyplatform_version. For clarity, the documentation should reflect the parameter name used in the function signature.-/// This function leverages the versioning system to direct the fetch operation -/// to the appropriate handler based on the `DriveVersion` provided. +/// This function leverages the versioning system to direct the fetch operation +/// to the appropriate handler based on the `PlatformVersion` provided.packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/mod.rs (1)
19-20: Clarify references toDriveVersionin doc comments
Similar to the fetch method, the doc references aDriveVersionwhereas the code relies onplatform_version.-/// verification method based on the provided platform version. +/// verification method based on the provided `PlatformVersion`.packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs (1)
30-31: Address the TODO regarding naming
The TODO questions whyidentity_by_unique_public_key_hashis used instead of a distinct field for non-unique public key hashes. This can be confusing for new contributors. Consider introducing a new configuration field or clarifying in the documentation.Would you like me to prepare a small refactor clarifying this name and removing the ambiguity?
packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/v0/mod.rs (2)
13-49: Consider providing a usage example in the doc comments.
The documentation is clear about the function’s purpose and process, but a concise code example illustrating how to call it could further help new developers or external callers.
50-81: Validate error messaging for missingidentity_proof.
Whenidentity_idis found butidentity_proofis missing, the error indicates an “incomplete proof.” Consider appending the relevantidentity_idfor clearer debugging, e.g., “missing identity proof for identity .”packages/dapi-grpc/protos/platform/v0/platform.proto (1)
36-37: New RPC method added for non-unique public key hash retrieval.This addition is consistent with the rest of the Platform service. Consider whether returning multiple identities in one call might be preferable or if an iterative approach via
start_afteris sufficient.Would you like me to open a follow-up issue proposing bulk retrieval if your use case demands multiple identities per request?
packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/v0/mod.rs (1)
56-219: Comprehensive testing of single and dual-identity scenarios.The tests are well-structured and demonstrate correct functionality with multiple identities sharing the same key. Consider adding a negative test scenario (e.g., unknown public key hash) to ensure the code handles no-match cases gracefully.
packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/v0/mod.rs (1)
159-192: Identity not found scenario.Returns an empty identity instead of an error, which is consistent with the design. Ensure documentation clarifies this behavior.
packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/v0/mod.rs (1)
31-52: Doc comment mismatch
The doc comment states "Given an identity, fetches the identity with its flags from storage," but the code actually fetches the identity by a non-unique public key hash. Consider updating the doc string to reflect the actual behavior for clarity.packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_identity_ids_by_non_unique_public_key_hash/mod.rs (1)
17-20: Documentation needs update for the new 'after' parameterWhile the documentation has been updated for the 'limit' parameter, it's missing documentation for the newly added 'after' parameter. Also, the method description still refers to 'DriveVersion' although the code now uses 'PlatformVersion'.
/// * `public_key_hash` - A non-unique public key hash corresponding to the identity ids to be fetched. /// * `limit` - An optional limit. + /// * `after` - An optional identity ID to fetch results after (for pagination). /// * `transaction` - Transaction arguments. /// * `platform_version` - A reference to the platform version.Also, update line 12 to reference PlatformVersion instead of DriveVersion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (52)
packages/dapi-grpc/build.rs(3 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto(2 hunks)packages/rs-dapi-client/src/transport/grpc.rs(1 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs(1 hunks)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_unique_public_key_hash/mod.rs(3 hunks)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_unique_public_key_hash/v0/mod.rs(4 hunks)packages/rs-drive-abci/src/query/identity_based_queries/mod.rs(1 hunks)packages/rs-drive-abci/src/query/service.rs(2 hunks)packages/rs-drive-abci/tests/strategy_tests/query.rs(1 hunks)packages/rs-drive-proof-verifier/src/proof.rs(4 hunks)packages/rs-drive-verify-c-binding/src/lib.rs(3 hunks)packages/rs-drive/Cargo.toml(3 hunks)packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_identity_ids_by_non_unique_public_key_hash/mod.rs(4 hunks)packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_identity_ids_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/mod.rs(2 hunks)packages/rs-drive/src/drive/identity/fetch/prove/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_identity_id_by_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_identity_ids_by_unique_public_key_hashes/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/queries/mod.rs(3 hunks)packages/rs-drive/src/drive/identity/identity_and_non_unique_public_key_hash_double_proof.rs(1 hunks)packages/rs-drive/src/drive/identity/mod.rs(1 hunks)packages/rs-drive/src/drive/mod.rs(2 hunks)packages/rs-drive/src/verify/identity/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_full_identities_by_public_key_hashes/v0/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/mod.rs(2 hunks)packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/v0/mod.rs(2 hunks)packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_identity_id_by_unique_public_key_hash/mod.rs(2 hunks)packages/rs-drive/src/verify/identity/verify_identity_id_by_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/mod.rs(2 hunks)packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/v0/mod.rs(1 hunks)packages/rs-platform-version/Cargo.toml(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/mod.rs(2 hunks)packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/v1.rs(2 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/mocks/v2_test.rs(1 hunks)packages/rs-sdk/src/platform/types/identity.rs(3 hunks)packages/rs-sdk/tests/fetch/identity.rs(2 hunks)packages/rs-sdk/tests/vectors/test_fetch_identity_by_non_unique_public_keys/quorum_pubkey-106-15957be6ce59202ce3df8dbd0604f02b4d71142b0d84600aa6fd73436efb869b.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
packages/rs-drive/Cargo.toml (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2431
File: packages/rs-drive/Cargo.toml:55-60
Timestamp: 2025-03-17T09:55:17.691Z
Learning: The grovedb dependencies in packages/rs-drive/Cargo.toml and related files are intentionally kept at specific revisions rather than using the latest stable version, with plans to update them at a later time.
packages/rs-sdk/tests/vectors/test_fetch_identity_by_non_unique_public_keys/quorum_pubkey-106-15957be6ce59202ce3df8dbd0604f02b4d71142b0d84600aa6fd73436efb869b.json (1)
Learnt from: lklimek
PR: dashpay/platform#2030
File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1
Timestamp: 2025-03-17T09:55:17.691Z
Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.
🧬 Code Definitions (13)
packages/rs-drive/src/drive/identity/fetch/prove/prove_identity_id_by_unique_public_key_hash/v0/mod.rs (1)
packages/rs-drive/src/verify/identity/verify_identity_id_by_unique_public_key_hash/mod.rs (1) (1)
verify_identity_id_by_unique_public_key_hash(36:61)
packages/rs-drive-abci/tests/strategy_tests/query.rs (1)
packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/mod.rs (1) (1)
verify_full_identity_by_unique_public_key_hash(39:62)
packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/mod.rs (1)
packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/mod.rs (1) (1)
fetch_full_identity_by_non_unique_public_key_hash(25:52)
packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_unique_public_key_hash/v0/mod.rs (1)
packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/mod.rs (1) (1)
verify_full_identity_by_unique_public_key_hash(39:62)
packages/rs-drive/src/verify/identity/verify_full_identities_by_public_key_hashes/v0/mod.rs (1)
packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/mod.rs (1) (1)
verify_identity_ids_by_unique_public_key_hashes(39:66)
packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/mod.rs (2)
packages/rs-drive/src/verify/identity/verify_full_identities_by_public_key_hashes/v0/mod.rs (1) (1)
Self(51:51)packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/v0/mod.rs (1) (1)
verify_identity_ids_by_unique_public_key_hashes_v0(40:96)
packages/rs-drive/src/drive/identity/fetch/prove/prove_identity_ids_by_unique_public_key_hashes/v0/mod.rs (1)
packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/mod.rs (1) (1)
verify_identity_ids_by_unique_public_key_hashes(39:66)
packages/rs-drive/src/verify/identity/mod.rs (5)
packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/mod.rs (1) (1)
verify_full_identity_by_non_unique_public_key_hash(53:78)packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/mod.rs (1) (1)
verify_full_identity_by_unique_public_key_hash(39:62)packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/mod.rs (1) (1)
verify_identity_id_by_non_unique_public_key_hash(37:64)packages/rs-drive/src/verify/identity/verify_identity_id_by_unique_public_key_hash/mod.rs (1) (1)
verify_identity_id_by_unique_public_key_hash(36:61)packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/mod.rs (1) (1)
verify_identity_ids_by_unique_public_key_hashes(39:66)
packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/v0/mod.rs (2)
packages/rs-sdk/src/platform/types/identity.rs (8) (8)
query(40:51)query(62:76)query(87:106)query(110:123)query(127:142)query(148:165)query(169:181)query(185:198)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_unique_public_key_hash/v0/mod.rs (3) (3)
test_invalid_public_key_hash(85:101)test_identity_not_found(104:121)test_identity_absence_proof(124:146)
packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/mod.rs (1)
packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/v0/mod.rs (1) (1)
verify_identity_id_by_non_unique_public_key_hash_v0(37:68)
packages/rs-drive/src/drive/identity/fetch/prove/mod.rs (1)
packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/mod.rs (1) (1)
prove_full_identity_by_non_unique_public_key_hash(48:74)
packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/mod.rs (1)
packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/v0/mod.rs (1) (1)
verify_full_identity_by_unique_public_key_hash_v0(41:65)
packages/rs-drive-abci/src/query/service.rs (3)
packages/dapi-grpc/clients/platform/v0/web/platform_pb_service.d.ts (2) (2)
Status(442:442)Platform(394:439)packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py (1) (1)
Platform(725:1457)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs (1) (1)
query_identity_by_non_unique_public_key_hash(17:69)
🪛 Biome (1.9.4)
packages/rs-sdk/tests/vectors/test_fetch_identity_by_non_unique_public_keys/quorum_pubkey-106-15957be6ce59202ce3df8dbd0604f02b4d71142b0d84600aa6fd73436efb869b.json
[error] 1-1: String values must be double quoted.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: Rust packages (rs-dapi-client) / Formatting
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (89)
packages/rs-sdk/tests/vectors/test_fetch_identity_by_non_unique_public_keys/quorum_pubkey-106-15957be6ce59202ce3df8dbd0604f02b4d71142b0d84600aa6fd73436efb869b.json (1)
1-1: Content matches expected test vector format.The file contains a non-JSON formatted hash value which is acceptable according to the project's testing framework. The retrieved learning confirms that
.jsonfiles in similar test vector directories intentionally do not follow standard JSON format.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-drive/src/drive/identity/fetch/prove/mod.rs (1)
4-4: Module addition follows established naming patterns.The new module
prove_full_identity_by_non_unique_public_key_hashaligns with the existing modules likeprove_full_identity_by_unique_public_key_hash, extending the identity proof system to support non-unique public key hashes.packages/rs-platform-version/Cargo.toml (1)
14-14: Improved TOML formatting consistency.The addition of a space before the equals sign in the
revfield improves readability and follows Rust ecosystem's TOML formatting conventions.packages/rs-drive/src/drive/identity/mod.rs (1)
43-45: Well-documented module addition for non-unique public key hash proofs.The new module
identity_and_non_unique_public_key_hash_double_proofis properly documented and follows the codebase's conditional compilation pattern. This addition complements the other changes being made to support identity operations with non-unique public key hashes.packages/rs-drive/src/drive/identity/fetch/prove/prove_identity_id_by_unique_public_key_hash/v0/mod.rs (1)
68-68: Method name updated to align with unique/non-unique public key hash distinctionThe method call has been correctly updated from
verify_identity_id_by_public_key_hashtoverify_identity_id_by_unique_public_key_hashto explicitly indicate that it works with unique public key hashes. This change maintains consistency with the broader codebase refactoring that distinguishes between unique and non-unique public key hash operations.packages/rs-drive/Cargo.toml (2)
55-60: Consistent formatting applied to grovedb dependenciesThe spacing around the
=sign has been standardized for consistency in the grovedb dependencies.
121-126: Improved readability for verify feature dependenciesThe
verifyfeature's dependencies have been reformatted into a multi-line structure for improved readability while maintaining the same logical grouping.packages/rs-drive/src/verify/identity/verify_identity_id_by_unique_public_key_hash/v0/mod.rs (1)
36-36: Method renamed to specify unique public key hash verificationThe method has been renamed from
verify_identity_id_by_public_key_hash_v0toverify_identity_id_by_unique_public_key_hash_v0to clearly indicate that it works with unique public key hashes. This change enhances code clarity and maintains consistency with other renamed methods across the codebase.packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/mod.rs (2)
2-2: Added new module for non-unique public key hash identity fetchingA new module
fetch_full_identity_by_non_unique_public_key_hashhas been added to support fetching full identities based on non-unique public key hashes, enhancing the platform's identity retrieval capabilities.
72-78: Updated method parameters for enhanced pagination supportThe
fetch_identity_ids_by_non_unique_public_key_hashmethod call has been updated with:
- Two new
Noneparameters for pagination support- Parameter name change from
drive_versiontoplatform_versionfor consistencyThese changes align with the overall enhancement of identity retrieval capabilities using public key hashes.
packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs (1)
70-70: Field naming updated for clarityThe field has been renamed from
identity_by_public_key_hashtoidentity_by_unique_public_key_hash, which adds specificity and clarifies that this handles unique public key hashes. This change aligns with the PR's focus on distinguishing between unique and non-unique public key hash operations.packages/rs-drive/src/drive/identity/fetch/prove/prove_identity_ids_by_unique_public_key_hashes/v0/mod.rs (1)
92-92: Method call updated for consistencyThe method call has been updated from
Drive::verify_identity_ids_by_public_key_hashestoDrive::verify_identity_ids_by_unique_public_key_hashesto maintain consistency with the renamed method implementation. This change correctly aligns with the codebase-wide refactoring to distinguish between unique and non-unique public key hash operations.packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/v0/mod.rs (1)
40-40: Function name updated for clarityThe function has been renamed from
verify_identity_ids_by_public_key_hashes_v0toverify_identity_ids_by_unique_public_key_hashes_v0, maintaining the same functionality while clarifying that it specifically handles unique public key hashes. This change is part of the systematic approach to differentiate between unique and non-unique public key hash operations.packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs (1)
55-55: Field name updated for consistencyThe field has been renamed from
identity_by_public_key_hashtoidentity_by_unique_public_key_hashin theDriveAbciQueryIdentityVersionsstruct, which enhances clarity by specifying that this handles unique public key hashes. This change aligns with similar renaming patterns across the codebase for improved consistency.packages/rs-drive-abci/src/query/identity_based_queries/mod.rs (1)
6-7: Module structure refactored to improve identity handling.The changes here split the single
identity_by_public_key_hashmodule into two specialized modules: one for non-unique and one for unique public key hashes. This provides better separation of concerns and allows for different handling strategies based on the uniqueness property of the public key hash.packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_unique_public_key_hash/v0/mod.rs (1)
90-90: Method call updated for consistency with renamed methods.The method call has been updated from
verify_full_identity_by_public_key_hashtoverify_full_identity_by_unique_public_key_hash, maintaining consistency with the module and method renaming pattern seen across the codebase. This change improves code clarity by being explicit about the uniqueness property of the public key hash.packages/rs-platform-version/src/version/mocks/v2_test.rs (1)
203-203: Field renamed for consistency with updated method names.The field name has been updated from
identity_by_public_key_hashtoidentity_by_unique_public_key_hashto align with the broader refactoring effort that disambiguates between unique and non-unique public key hash operations.packages/rs-drive/src/drive/identity/identity_and_non_unique_public_key_hash_double_proof.rs (1)
1-21: Well-structured new struct with comprehensive documentation.This new struct
IdentityAndNonUniquePublicKeyHashDoubleProofis well-designed for handling proofs related to identities with non-unique public key hashes. The documentation clearly explains the purpose of each field and when they might be used. The struct complements the other changes in this PR that split handling between unique and non-unique public key hashes.packages/rs-drive-abci/src/query/identity_based_queries/identity_by_unique_public_key_hash/mod.rs (3)
35-35: Path updated to reflect unique public key hash functionality.This change clarifies that this feature accesses the specific version bounds for unique public key hash queries instead of the more general public key hash queries.
44-45: Error message updated for consistency with the renamed feature.The error message now correctly references "identity_by_unique_public_key_hash" instead of the previous more generic name, ensuring consistent terminology throughout the codebase.
55-55: Method call renamed to reflect unique public key hash specificity.The method call has been updated to use the more specific
query_identity_by_unique_public_key_hash_v0name, which better describes its purpose of handling only unique public key hash queries.packages/rs-drive-abci/src/query/identity_based_queries/identity_by_unique_public_key_hash/v0/mod.rs (4)
17-17: Function renamed for clarity on key hash uniqueness.The function name has been updated from
query_identity_by_public_key_hash_v0toquery_identity_by_unique_public_key_hash_v0to clearly indicate it operates specifically on unique public key hashes, improving code clarity.
94-95: Test call updated to use renamed function.The test case has been correctly updated to call the renamed function, maintaining consistency throughout the codebase.
114-115: Test call updated to use renamed function.Consistent update to the test call to match the renamed function.
134-135: Test call updated to use renamed function.Another consistent update to the test call to match the renamed function.
packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/v1.rs (2)
30-30: Added support for non-unique public key hash identity fetching.This addition introduces functionality to fetch full identities using non-unique public key hashes, complementing the existing unique key hash functionality. The initial version is set to 0.
62-62: Added support for proving identities by non-unique public key hash.This addition introduces functionality to prove full identities using non-unique public key hashes, consistent with the fetching functionality added earlier. The initial version is set to 0.
packages/rs-drive-abci/tests/strategy_tests/query.rs (1)
269-269: Updated verification method call to use the renamed function.The method call has been updated from
verify_full_identity_by_public_key_hashtoverify_full_identity_by_unique_public_key_hashto match the renamed function, maintaining consistency with the other changes in this PR.packages/rs-drive/src/verify/identity/verify_identity_id_by_unique_public_key_hash/mod.rs (4)
36-41: Function name updated to clarify it works with unique public key hashesThe function has been renamed from
verify_identity_id_by_public_key_hashtoverify_identity_id_by_unique_public_key_hashto explicitly indicate that it works with unique public key hashes. This naming change improves code clarity by distinguishing it from operations that work with non-unique public key hashes.
47-48: Updated version check path to match renamed functionThe version check path has been correctly updated to match the renamed function, maintaining consistency throughout the codebase.
49-54: Updated internal method call to maintain naming consistencyThe internal method call has been properly renamed to
verify_identity_id_by_unique_public_key_hash_v0, maintaining consistency with the new naming convention.
55-59: Updated error message to reflect renamed methodThe error message now correctly references the updated method name, which is essential for proper error reporting and debugging.
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs (2)
113-116: Added fixed seed RNG for predictable non-unique key generationA deterministic random number generator with a fixed seed has been introduced to create a predictable non-unique public key that will be shared across all test identities. This enables testing functionality that relies on retrieving identities by non-unique public key hash.
124-127:✅ Verification successful
Modified keys vector to include a shared non-unique public key
The keys vector is now mutable and includes a shared voting key across all identities. This ensures that multiple identities will have the same public key hash, which is essential for testing the retrieval of identities by non-unique public key hash.
🏁 Script executed:
#!/bin/bash # Verify that the non-unique key is being used in tests consistently # Look for references to non_unique_key in test files rg "non_unique_key" --type rustLength of output: 15682
Non-Unique Key Usage Verified in Test Suite
The change to append a shared non-unique public key to the keys vector in the test is consistent with its usage across the codebase. The
rg "non_unique_key" --type rustoutput confirms that similar patterns exist in various test files, indicating that this approach is both intentional and consistent with the testing of identity retrieval by non-unique key hash.packages/rs-drive/src/verify/identity/verify_full_identities_by_public_key_hashes/v0/mod.rs (1)
50-56: Updated method call to use renamed function for unique public key hashesThe method call has been updated from
verify_identity_ids_by_public_key_hashestoverify_identity_ids_by_unique_public_key_hashesto maintain consistency with the new naming convention that explicitly distinguishes between unique and non-unique public key hash operations.packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/v0/mod.rs (3)
21-22: Updated documentation to reference renamed functionThe function documentation has been updated to reference the renamed function
verify_identity_id_by_unique_public_key_hash(), maintaining consistency in documentation and code.
40-42: Function renamed to clarify it works with unique public key hashesThe function has been renamed from
verify_full_identity_by_public_key_hash_v0toverify_full_identity_by_unique_public_key_hash_v0to explicitly indicate that it works with unique public key hashes, improving code clarity.
46-51: Updated internal method call to use renamed functionThe internal method call has been updated to use the renamed function
verify_identity_id_by_unique_public_key_hash, maintaining consistency with the new naming convention.packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/mod.rs (1)
39-62: Function renamed for clarity and consistencyThe method has been renamed from
verify_full_identity_by_public_key_hashtoverify_full_identity_by_unique_public_key_hashalong with corresponding internal calls and error messages. This renaming provides better clarity by explicitly distinguishing between unique and non-unique public key hash operations.packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/mod.rs (2)
98-98: New field added for non-unique public key hash supportAddition of the
fetch_full_identity_by_non_unique_public_key_hashfield to theDriveIdentityFetchPublicKeyHashesMethodVersionsstruct extends the platform's functionality to handle non-unique public key hash operations.
137-137: New proof method for non-unique public key hash addedAddition of the
prove_full_identity_by_non_unique_public_key_hashfield to theDriveIdentityProveMethodVersionsstruct complements the fetch capabilities, allowing for proof verification of identities using non-unique public key hashes.packages/rs-dapi-client/src/transport/grpc.rs (1)
493-500: New gRPC request implementation for non-unique public key hashImplementation of the transport request for
getIdentityByNonUniquePublicKeyHashfollows the established pattern and allows the gRPC client to retrieve identities using non-unique public key hashes. This addition aligns with the broader changes to support both unique and non-unique public key hash operations.packages/dapi-grpc/build.rs (2)
66-78: Updated VERSIONED_REQUESTS array with new request typeThe
VERSIONED_REQUESTSarray has been expanded from 40 to 41 elements to include the new"GetIdentityByNonUniquePublicKeyHashRequest". This ensures the build process correctly handles the new request type.
114-116: Added documentation for excluded response typeAdded clarification that
"GetIdentityByNonUniquePublicKeyHashResponse"is excluded fromVERSIONED_RESPONSESdue to needing custom proof handling. This documentation helps prevent confusion about why this response isn't included in the versioned responses array.packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/mod.rs (1)
39-66: Rename correctly implements distinction between unique and non-unique key hash operationsThe function has been renamed from
verify_identity_ids_by_public_key_hashestoverify_identity_ids_by_unique_public_key_hashes, with corresponding updates to method references and error messages. This change improves clarity by explicitly indicating that this function deals with unique public key hashes, distinguishing it from the newly added non-unique public key hash functionality.The renaming is consistent with the broader architectural changes in the PR, which separate unique from non-unique public key hash operations throughout the codebase.
packages/rs-sdk/tests/fetch/identity.rs (2)
1-1: Import updated to include new query typeThe import now includes
NonUniquePublicKeyHashQuery, which is correctly used in the new test.
120-150: Test correctly validates fetching by non-unique public key hashThe test setup is well structured, first fetching an existing identity and then collecting its non-unique public keys for testing. The assertion ensuring that at least one non-unique public key exists is a good precondition check.
packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs (2)
24-26: Method names updated for consistency with unique key hash distinctionThe method names have been correctly updated to specify "unique" public key hash operations, maintaining consistency with the broader architectural changes in this PR.
31-32: New methods for non-unique public key hash verification correctly addedTwo new methods have been added to support the verification of identities by non-unique public key hashes:
verify_full_identity_by_non_unique_public_key_hashverify_identity_id_by_non_unique_public_key_hashBoth methods are assigned version 0, consistent with other methods in this structure.
packages/rs-drive/src/verify/identity/mod.rs (2)
3-4: Module structure updated to separate unique and non-unique key hash functionalityThe previously merged
verify_full_identity_by_public_key_hashmodule has been correctly split into two dedicated modules:
verify_full_identity_by_non_unique_public_key_hashverify_full_identity_by_unique_public_key_hashThis separation provides clearer organization and follows the architectural pattern established in this PR.
10-12: Module organization completes the unique/non-unique separation patternThe modules for identity ID verification have been properly restructured:
- Added
verify_identity_id_by_non_unique_public_key_hash- Added
verify_identity_id_by_unique_public_key_hash- Renamed
verify_identity_ids_by_public_key_hashestoverify_identity_ids_by_unique_public_key_hashesThese changes complete the consistent pattern of separating functionality between unique and non-unique public key hash operations throughout the codebase.
packages/rs-drive/src/drive/mod.rs (1)
264-265: Expand feature gating for verification
By broadening the#[cfg(...)]condition to include the "verify" feature, you're allowing this function to be compiled when performing proofs and verifications. This aligns well with the need to handle identity proofs in both "server" and "verify" contexts.packages/rs-drive-verify-c-binding/src/lib.rs (1)
686-693: Update test to use the new function name
The test methodverify_identity_id_by_unique_public_key_hash()correctly exercises the renamed function, ensuring coverage for the unique key hash path. Consider adding a negative-case test for nonexistent key hashes if needed.packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/mod.rs (3)
1-2: Create a dedicated v0 module
Establishing a separate versioned module for non-unique public key hash proof logic provides a clear foundation for future version increments. Good approach for maintaining backward compatibility.
3-10: Introduce imports for identity proof structures
The imports reference the newly addedidentity_and_non_unique_public_key_hash_double_proofand other necessary modules. Make sure gating remains consistent with any feature flags used elsewhere in the code.
11-75: Implement proof generation for non-unique public key hashes
This new function is well-documented and follows a clear version-check pattern. Ensuring that the fallbackErris returned for unknown versions helps maintain strict versioning. Consider adding integration tests to verify proof construction and retrieval under various scenarios.packages/rs-sdk/src/platform/types/identity.rs (2)
12-12: Add import for non-unique public key hash request
ImportingGetIdentityByNonUniquePublicKeyHashRequestV0aligns with the expanded feature set for identity retrieval. No issues found.
35-36: Extend the delegate enum to include non-unique public key hash
Adding(GetIdentityByNonUniquePublicKeyHash, ...)to thedelegate_enum!macro centralizes identity requests effectively and keeps them properly enumerated.packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/mod.rs (1)
25-52: Implementation looks correct
The version dispatch and error handling for unknown versions appear sound. The function correctly returns anOption<Identity>and gracefully handles mismatched version scenarios.packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/mod.rs (1)
53-78: Version dispatch approach is consistent
Dispatching toverify_full_identity_by_non_unique_public_key_hash_v0and returning an error for any unknown version follows a clear version-control pattern. No logical errors are apparent.packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs (1)
42-52: Version bounds check is correct
The feature version check correctly guards against unsupported versions. Returning anUnsupportedQueryVersionerror is consistent with other version-based checks in this repository.packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/mod.rs (1)
37-64: Ensure consistent version mismatch error handling.
This method returnsUnknownVersionMismatchif a version other than 0 is received, which aligns with your versioning strategy. Just confirm that other versioned calls in this module use the same error style and messages for consistency.packages/rs-drive/src/drive/identity/fetch/queries/mod.rs (2)
3-3: Straightforward import addition.
Importingnon_unique_key_hashes_tree_path_vecis aligned with the usage in subsequent functions. No issues here.
235-251: Validate merged query results.
By merging thefull_identity_queryandidentity_id_by_non_unique_public_key_hash_query, you fetch data from multiple paths. Consider verifying that partial or missing data in either query is handled properly.Would you like me to generate a script that checks any test files referencing these queries to ensure all expected results are tested (e.g., presence/absence of identity)?
packages/rs-drive-abci/src/query/service.rs (1)
31-31: New import for the non-unique public key hash request/response.This import addition aligns well with the code changes below, which introduce an API method for identities by non-unique public key hash. No issues found.
packages/dapi-grpc/protos/platform/v0/platform.proto (1)
625-652:✅ Verification successful
New request/response message definitions for non-unique public key queries.
The structure aligns with existing message patterns, providing optional pagination and proofs. Verify that clients relying on paging logic (using
start_after) handle multiple results as expected.Consider searching existing tests to confirm the
start_afterfield is adequately exercised:
🏁 Script executed:
#!/bin/bash rg "start_after" -A 5Length of output: 88230
Paging logic validation for non-unique public key queries
The updated request/response definitions follow the established patterns—incorporating optional pagination (via the
start_afterfield) and proof support. The grep search confirms that tests across the codebase (in modules likers-drive-abciandrs-drive/tests) exercise thestart_afterbehavior for both valid and invalid cases. Please verify that any client logic relying on paging uses this field correctly, ensuring that handling of multiple results remains as expected.packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/v0/mod.rs (1)
9-53: Implementation reliably constructs and returns identity proofs.No concurrency or correctness issues are apparent. Storing operations in vectors and limiting results to one identity seems intentional for iterative retrieval. This is consistent with similar proof-generation methods.
packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/v0/mod.rs (8)
1-16: Imports and module setup look good.These imports provide essential modules, error types, and data structures. There's no indication of unused imports or missing dependencies.
28-33: Validate edge cases for public key hash length.The check for a 20-byte public key hash is consistent with expected usage. Consider explicitly handling or logging edge cases in the calling code if the length is incorrect.
35-45: Optional start_after handling.Properly validates the 32-byte ID length for
start_after. This helps avoid corruption or mismatched identity references. The existing error message is clear.
47-71: Prove branch logic looks correct.Building the proof response with identity and public key hash proofs is well-structured and aligns with the code’s error handling approach. Ensure relevant integration tests exist in other modules.
Do you want to verify usage in the codebase with a shell script scanning for references to
prove_full_identity_by_non_unique_public_key_hash?
72-99: Non-prove branch logic follows expected patterns.Fetching and serializing the identity if it exists, or returning
Noneif it doesn't, is appropriate. Ensure upstream code checks for theNonescenario correctly.
112-130: Test for invalid public key hash length.Good negative test coverage here, confirming that an 8-byte hash leads to an
InvalidArgument.
132-157: Test coverage for variousstart_afterlengths.Comprehensive checks ensure resilience against malformed inputs.
194-218: Proof of absence test.The logic is validated by checking for the
Proofobject when no identity is found underprove: true. This successfully guards against incomplete or corrupted missing proofs.packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_identity_ids_by_non_unique_public_key_hash/v0/mod.rs (3)
19-20: Additional limit parameter.Introducing the
limitparameter is a welcome addition. Confirm that paging logic or offsets are also handled when large sets of IDs may need to be fetched incrementally.Also applies to: 40-40
20-20:afterparameter usage.Filtering results starting “after” a given ID is well-handled. Consider verifying that the “after” logic aligns with other pagination or range-based queries throughout the codebase.
Also applies to: 47-50
35-44: Partitioning logic with version references.Switching from
drive_versiontoplatform_versionis consistent and clarifies the usage of broader version constraints. Be sure to test all existing references to see if further refactoring is needed.packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/v0/mod.rs (1)
12-28: Simple wrapper function looks correct
No major issues found. The function logically delegates to theoperations_v0version and initializes thedrive_operationsvector in a straightforward manner.packages/rs-drive-proof-verifier/src/proof.rs (3)
18-21: New imports for non-unique key functionality
These imports align well with the newly introduced code blocks handling the “non-unique” request/response structures.
333-338: Updated method call to 'verify_full_identity_by_unique_public_key_hash'
Renaming the method to emphasize uniqueness is consistent with the updated naming convention across the codebase.
346-436: Implementation of 'FromProof' for non-unique public key hash
This method correctly handles the extraction of the proof, parsing of request fields, and verification of the resulting identity. The error handling for invalid lengths and missing proofs is robust. Nicely done.packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs (2)
38-39: Renaming fields to 'unique'
Reflects the code’s shift to distinguishing unique public key hashes, ensuring clarity about which verification method is used.
45-46: Added fields for non-unique public key hash verification
These new fields expand the feature set to handle non-unique public key hashes, aligning with the PR’s objective.packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_identity_ids_by_non_unique_public_key_hash/mod.rs (5)
5-5: Updated imports to support new functionalityThe imports have been appropriately updated to support:
- The new operations tracking feature (LowLevelDriveOperation)
- Platform-wide versioning instead of drive-specific versioning (PlatformVersion)
These changes align with the PR's goal of enhancing identity retrieval functionality.
Also applies to: 7-7
24-31: Method signature updated with pagination supportThe method signature has been properly updated to:
- Add pagination support through 'limit' and 'after' parameters
- Transition from drive-specific versioning to platform-wide versioning
This change improves the API by supporting pagination for large result sets.
32-46: Implementation updated to use platform versioningThe implementation correctly uses the platform version to determine the appropriate handler, passing along the new pagination parameters to the v0 implementation.
55-86: Good addition of operations tracking methodThe new method for tracking operations follows the established pattern in the codebase:
- It has similar structure to the public method
- It's marked as
pub(crate)to indicate internal use- It properly delegates to a version-specific implementation
- It includes appropriate error handling for unknown versions
This addition supports the ability to track operations for fee calculation or auditing purposes.
1-87:❓ Verification inconclusive
Verify implementation of v0 methods with new parameters
The changes look good, but please ensure that the v0 implementations (
fetch_identity_ids_by_non_unique_public_key_hash_v0andfetch_identity_ids_by_non_unique_public_key_hash_operations_v0) properly support the new parameters (limit, after) and have been thoroughly tested.
🏁 Script executed:
#!/bin/bash # Check for the v0 implementation files to ensure they exist and handle the new parameters find packages/rs-drive/src -type f -name "v0.rs" | xargs grep -l "fetch_identity_ids_by_non_unique_public_key_hash" | xargs head -50Length of output: 130
I'll generate a new shell script to search the repository for the implementations of the v0 methods to verify that the new parameters (
limitandafter) are supported. Please run the following script and share the output:This will help us confirm how the new parameters are integrated into the v0 implementations.
🏁 Script executed:
#!/bin/bash echo "Searching for fetch_identity_ids_by_non_unique_public_key_hash_v0 implementation:" rg -n "fn fetch_identity_ids_by_non_unique_public_key_hash_v0" packages/rs-drive/src/ echo "------------------------------" echo "Searching for fetch_identity_ids_by_non_unique_public_key_hash_operations_v0 implementation:" rg -n "fn fetch_identity_ids_by_non_unique_public_key_hash_operations_v0" packages/rs-drive/src/Length of output: 1162
Ensure v0 implementations correctly handle the
limitandafterparametersThe changes look good overall; however, please verify that the v0 methods:
fetch_identity_ids_by_non_unique_public_key_hash_v0fetch_identity_ids_by_non_unique_public_key_hash_operations_v0in
packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_identity_ids_by_non_unique_public_key_hash/v0/mod.rsproperly accept and process the new
limitandafterparameters. Additionally, confirm that you have appropriate tests in place to validate their behavior.
...ages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/v0/mod.rs
Show resolved
Hide resolved
commit 61ad58a Merge: 8d99708 6161f75 Author: QuantumExplorer <[email protected]> Date: Mon Mar 17 21:39:12 2025 +0700 Merge branch 'v2.0-dev' into feat/getIdentitiesByPublicKeyHashes1 commit 8d99708 Author: Quantum Explorer <[email protected]> Date: Thu Mar 6 21:04:45 2025 +0700 more work commit 0f26760 Author: Quantum Explorer <[email protected]> Date: Mon Mar 3 21:03:26 2025 +0700 temp work
chore: update to latest dash core 37 (#2483) feat(platform)!: token advanced distribution and updates (#2471) fix: token history contract (#2474) Co-authored-by: Ivan Shumkov <[email protected]> Co-authored-by: QuantumExplorer <[email protected]> fix(drive): using new rust dash core methods for reversed quorum hash to maintain backwards compatibility (#2489) feat: more granular integer document property types (#2455) Co-authored-by: Quantum Explorer <[email protected]> docs: update comment for data contract code range (#2476) feat: validate token name localizations (#2468) feat(sdk): get identity by non-unique keys build(deps): update grovedb to current develop test: test identity by non-unique pubkey hashes fix(sdk): dash core client fails to get quorum chore: minor fixes test(drive-abci): identity by non-unique pubkey start after chore: minor changes to verify feat(sdk): token and group queries (#2449) chore: revert limit 1 => limit none chore: add non-unique key to test identities test(sdk): test vectors for test_fetch_identity_by_non_unique_public_keys fix(platform)!: token distribution fixes and tests (#2494) chore(platform): bump to version 2.0.0-dev.1 (#2495) test: update assertion fix(sdk): make some things public (#2496) feat(platform): require token for document actions (#2498) fix: data contract proof doesn't work with new auto fields (#2501)
lklimek
force-pushed
the
feat/getIdentitiesByPublicKeyHashes2
branch
from
e7938db to
09afcb6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/rs-drive/src/drive/identity/fetch/queries/mod.rs (1)
235-251: Consider adding a limit parameter or note the intended usage.
This method merges the full identity query with the non-unique hash query, potentially returning multiple identities if the hash is shared by more than one identity. Adding or clarifying a limit parameter (similar to pagination) could improve performance and predictability.packages/dapi-grpc/protos/platform/v0/platform.proto (1)
625-632: Consider adding explicit pagination parameters.
start_afteralone may not be sufficient for fully paginated retrieval if multiple identities share the same public key hash. Consider adding a limit parameter or a continued pagination mechanism to handle multiple results more flexibly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
packages/dapi-grpc/build.rs(3 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto(2 hunks)packages/rs-dapi-client/src/transport/grpc.rs(1 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs(1 hunks)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_unique_public_key_hash/mod.rs(3 hunks)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_unique_public_key_hash/v0/mod.rs(4 hunks)packages/rs-drive-abci/src/query/identity_based_queries/mod.rs(1 hunks)packages/rs-drive-abci/src/query/service.rs(2 hunks)packages/rs-drive-abci/tests/strategy_tests/query.rs(1 hunks)packages/rs-drive-proof-verifier/src/proof.rs(4 hunks)packages/rs-drive-verify-c-binding/src/lib.rs(3 hunks)packages/rs-drive/Cargo.toml(1 hunks)packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_identity_ids_by_non_unique_public_key_hash/mod.rs(4 hunks)packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_identity_ids_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/mod.rs(2 hunks)packages/rs-drive/src/drive/identity/fetch/prove/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_identity_id_by_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_identity_ids_by_unique_public_key_hashes/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/queries/mod.rs(3 hunks)packages/rs-drive/src/drive/identity/identity_and_non_unique_public_key_hash_double_proof.rs(1 hunks)packages/rs-drive/src/drive/identity/mod.rs(1 hunks)packages/rs-drive/src/drive/mod.rs(2 hunks)packages/rs-drive/src/verify/identity/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_full_identities_by_public_key_hashes/v0/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/mod.rs(2 hunks)packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/v0/mod.rs(2 hunks)packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_identity_id_by_unique_public_key_hash/mod.rs(2 hunks)packages/rs-drive/src/verify/identity/verify_identity_id_by_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/mod.rs(2 hunks)packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/v0/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/mod.rs(2 hunks)packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/v1.rs(2 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/mocks/v2_test.rs(1 hunks)packages/rs-sdk/src/platform/types/identity.rs(3 hunks)packages/rs-sdk/tests/fetch/identity.rs(2 hunks)packages/rs-sdk/tests/vectors/test_fetch_identity_by_non_unique_public_keys/quorum_pubkey-106-15957be6ce59202ce3df8dbd0604f02b4d71142b0d84600aa6fd73436efb869b.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (35)
- packages/rs-drive/src/drive/identity/fetch/prove/mod.rs
- packages/rs-drive/src/drive/identity/mod.rs
- packages/rs-drive/Cargo.toml
- packages/rs-drive-abci/src/query/identity_based_queries/mod.rs
- packages/rs-drive/src/drive/identity/fetch/prove/prove_identity_ids_by_unique_public_key_hashes/v0/mod.rs
- packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs
- packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/mod.rs
- packages/rs-platform-version/src/version/mocks/v2_test.rs
- packages/rs-drive/src/verify/identity/verify_full_identities_by_public_key_hashes/v0/mod.rs
- packages/rs-drive/src/verify/identity/verify_identity_id_by_unique_public_key_hash/v0/mod.rs
- packages/rs-drive/src/drive/identity/fetch/prove/prove_identity_id_by_unique_public_key_hash/v0/mod.rs
- packages/dapi-grpc/build.rs
- packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/mod.rs
- packages/rs-drive-abci/src/query/identity_based_queries/identity_by_unique_public_key_hash/mod.rs
- packages/rs-drive/src/drive/identity/identity_and_non_unique_public_key_hash_double_proof.rs
- packages/rs-drive-abci/tests/strategy_tests/query.rs
- packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/v0/mod.rs
- packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs
- packages/rs-drive/src/verify/identity/verify_identity_id_by_unique_public_key_hash/mod.rs
- packages/rs-drive/src/verify/identity/verify_full_identity_by_unique_public_key_hash/v0/mod.rs
- packages/rs-dapi-client/src/transport/grpc.rs
- packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/mod.rs
- packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_unique_public_key_hash/v0/mod.rs
- packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs
- packages/rs-drive-abci/src/query/identity_based_queries/identity_by_unique_public_key_hash/v0/mod.rs
- packages/rs-drive/src/verify/identity/verify_identity_ids_by_unique_public_key_hashes/mod.rs
- packages/rs-drive/src/verify/identity/mod.rs
- packages/rs-drive-verify-c-binding/src/lib.rs
- packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/v0/mod.rs
- packages/rs-drive/src/drive/mod.rs
- packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs
- packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/mod.rs
- packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_identity_ids_by_non_unique_public_key_hash/v0/mod.rs
- packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs
- packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_identity_ids_by_non_unique_public_key_hash/mod.rs
🧰 Additional context used
🧠 Learnings (1)
packages/rs-sdk/tests/vectors/test_fetch_identity_by_non_unique_public_keys/quorum_pubkey-106-15957be6ce59202ce3df8dbd0604f02b4d71142b0d84600aa6fd73436efb869b.json (1)
Learnt from: lklimek
PR: dashpay/platform#2030
File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1
Timestamp: 2025-03-17T09:55:17.691Z
Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.
🧬 Code Definitions (2)
packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/mod.rs (1)
packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/v0/mod.rs (1) (1)
verify_identity_id_by_non_unique_public_key_hash_v0(37:68)
packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/v0/mod.rs (2)
packages/rs-sdk/src/platform/types/identity.rs (8) (8)
query(40:51)query(62:76)query(87:106)query(110:123)query(127:142)query(148:165)query(169:181)query(185:198)packages/rs-drive-abci/src/query/identity_based_queries/identity_by_unique_public_key_hash/v0/mod.rs (3) (3)
test_invalid_public_key_hash(85:101)test_identity_not_found(104:121)test_identity_absence_proof(124:146)
🪛 Biome (1.9.4)
packages/rs-sdk/tests/vectors/test_fetch_identity_by_non_unique_public_keys/quorum_pubkey-106-15957be6ce59202ce3df8dbd0604f02b4d71142b0d84600aa6fd73436efb869b.json
[error] 1-1: String values must be double quoted.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (39)
packages/rs-sdk/tests/vectors/test_fetch_identity_by_non_unique_public_keys/quorum_pubkey-106-15957be6ce59202ce3df8dbd0604f02b4d71142b0d84600aa6fd73436efb869b.json (1)
1-1: Hash value looks valid for test vector.This test vector follows the established pattern for defining test data in the vector directories. The static analysis warning about double quotes can be ignored since, as noted in retrieved learnings, these JSON files are intentionally using a non-standard format within the testing framework.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/v1.rs (2)
30-30: New method version for fetch by non-unique public key hash.This addition properly integrates with the existing versioning system and supports the new functionality for fetching identities by non-unique public key hash.
62-62: New method version for proving identities by non-unique public key hash.This addition properly integrates with the existing versioning system and supports the new functionality for proving identities by non-unique public key hash.
packages/rs-sdk/tests/fetch/identity.rs (2)
1-1: Updated import to include new NonUniquePublicKeyHashQuery.The import statement has been updated to include the new query type needed for the non-unique public key hash functionality.
120-181:✅ Verification successful
Well-structured test for fetching identities by non-unique public keys.
The test implementation effectively validates the new functionality:
- It correctly sets up the test environment
- It filters for non-unique keys in the test identity
- It properly tests the pagination mechanism with the
afterparameter- It verifies the expected results
The assertion checking for exactly 3 identities might be environment-specific. Consider making this assertion more flexible if the test environment might have a variable number of identities.
🏁 Script executed:
#!/bin/bash # Check if the test relies on environment-specific data rg "expected exactly 3 identities" --context 5 --glob "*.rs"Length of output: 637
Test Design Reviewed: Identity Fetch by Non-Unique Public Keys
- The test sets up the environment correctly, retrieves the target identity, and filters non-unique public keys accurately.
- It properly implements the pagination mechanism using the
afterparameter.- The final assertion strictly requires exactly 3 identities with a non-unique public key. If the test environment might sometimes yield a different count, consider making this assertion more flexible.
packages/rs-drive-abci/src/query/service.rs (2)
31-31: Added new request and response types for identity by non-unique public key hash.These imports support the new functionality for retrieving identities by non-unique public key hashes.
415-425: Implementation of get_identity_by_non_unique_public_key_hash RPC method.The implementation properly follows the established pattern for query handlers in this service:
- It correctly uses the
handle_blocking_querymethod- It uses the appropriate query method from the Platform implementation
- It passes the correct endpoint name for logging and metrics
The integration test coverage concern raised in a previous review appears to be addressed by the test implementation in
packages/rs-sdk/tests/fetch/identity.rs.packages/rs-sdk/src/platform/types/identity.rs (4)
12-12: Import added for supporting non-unique public key hash requests.Good addition of the import for
GetIdentityByNonUniquePublicKeyHashRequestV0to support the new functionality for querying identities based on non-unique public key hashes.
35-36: LGTM - Expanded delegate_enum to include non-unique public key hash query.The delegate_enum macro has been properly extended to include the new entry for
GetIdentityByNonUniquePublicKeyHash, correctly linking it to its corresponding request and response types.
79-84: Well-structured new query type for non-unique public key hashes.The
NonUniquePublicKeyHashQuerystruct is well-defined with appropriate fields: a 20-bytekey_hashfor the public key hash and an optional 32-byteafterfield for pagination support. This structure properly accommodates the requirements for querying by non-unique public key hashes.
86-107: Implementation for NonUniquePublicKeyHashQuery follows established patterns.The
Query<IdentityRequest>trait implementation is consistent with other query implementations in the file:
- Properly checks if proofs are required
- Constructs the appropriate request object with the necessary fields
- Handles the conversion to the IdentityRequest type
The implementation correctly sets up the non-unique public key hash query with pagination support through the optional
afterparameter.packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/mod.rs (1)
1-79: Well-structured verification method with comprehensive documentation.The implementation of
verify_full_identity_by_non_unique_public_key_hashfollows the established pattern for verification methods:
- Clear module structure with versioned implementation
- Comprehensive documentation covering purpose, parameters, return values, and errors
- Proper version dispatching with appropriate error handling for unsupported versions
The method signature correctly handles all required parameters including the proof, public key hash, optional
afterparameter for pagination, and platform version reference.packages/rs-drive/src/drive/identity/fetch/fetch_by_public_key_hashes/fetch_full_identity_by_non_unique_public_key_hash/mod.rs (1)
1-53: Fetch method implementation follows established patterns.The
fetch_full_identity_by_non_unique_public_key_hashmethod is well-implemented:
- Clear module structure with versioned implementation
- Comprehensive documentation that explains purpose, parameters, and return values
- Proper version dispatching with appropriate error handling
- Correctly implemented as an instance method (
&self) since it needs to access drive stateThe method signature properly includes all necessary parameters including the public key hash, optional
afterparameter for pagination, transaction argument, and platform version reference.packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/mod.rs (1)
1-65: Comprehensive verification method for identity IDs by non-unique public key hash.The implementation of
verify_identity_id_by_non_unique_public_key_hashis well-structured:
- Clear module organization with versioned implementation
- Detailed documentation explaining parameters, return values, and error conditions
- Proper version dispatching with appropriate error handling
The method correctly handles verification of identity IDs using the provided proof, with support for pagination through the
afterparameter. The implementation maintains consistency with other verification methods in the codebase.packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs (1)
16-69: Clarify handling of potentially multiple identities.
While this function returns a singleGetIdentityByNonUniquePublicKeyHashResponse, the term "non-unique" implies there may be multiple identities. Confirm that returning only the first match is intentional or extend the handling for multiple identities.packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/v0/mod.rs (1)
51-81: Confirm behavior for multiple identity IDs.
This method verifies a single identity even though public key hashes can be non-unique. If multiple identity IDs share the same hash, ensure the logic is correct (e.g., picking the first discovered identity or requiring additional pagination).packages/rs-drive/src/drive/identity/fetch/queries/mod.rs (1)
105-105: Clarify the TODO regarding query limit.
You have a TODO comment stating, “why not limit 1?” If you only need a single identity ID, explicitly limiting the query might help reduce overhead. Otherwise, please explain the reasoning in a comment.packages/dapi-grpc/protos/platform/v0/platform.proto (2)
36-37: Method name consistency check.The newly introduced RPC method name aligns well with the existing naming patterns. It clearly distinguishes “non-unique” key retrieval from the existing “unique” key retrieval methods. No concerns here.
634-652: Single identity vs. multiple identities handling.The current response structure only contemplates returning one identity at a time (or proof), which might be enough for some use cases. If there is a need to fetch many identities sharing the same non-unique key, a repeated field could be introduced in future enhancements. Otherwise, this is consistent with existing patterns.
packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/v0/mod.rs (2)
9-55: Check for multiple identity IDs scenario.This method fetches identity IDs using a limit of 1, then proves only the first matching identity if multiple exist. This is likely by design, but ensure end-users are aware that only the first match is returned and proven. Otherwise, the code is logically consistent and follows the established pattern for generating double proofs.
56-219: Good test coverage with parametric scenarios.The tests verify single and multiple identity scenarios with a shared non-unique key, demonstrating how the “after” parameter navigates the set of matching identities. Consider adding tests for more than two identities to ensure robust pagination, but these already offer solid coverage.
packages/rs-drive/src/verify/identity/verify_identity_id_by_non_unique_public_key_hash/v0/mod.rs (1)
37-69: Doc mismatch with multiple results scenario.Although the doc comment at lines 33–35 states an error is thrown if more than one identity ID is found, the code returns
(root_hash, None)instead of an error. If this behavior is intentional, consider updating the doc comment accordingly.packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/v0/mod.rs (10)
18-27: Function signature looks consistent and well-structured.The destructuring of request fields is clear and keeps the function signature concise. No immediate concerns with argument patterns or visibility scope.
28-33: Validation forpublic_key_hashis correct.Using
Bytes20::from_vecand mapping an error toQueryError::InvalidArgumentensures an early, clear, and meaningful error response when the length is invalid.
35-45: Optionalstart_aftercheck appears proper.Similar validation pattern with
Bytes32::from_vecfor the optional identity ID is consistent and helps capture length mismatches early.
47-71: Proof flow aligns with similar queries.Generating and populating the proof response is handled cleanly. This logic mirrors other proof-based query flows, promoting consistency across the codebase.
72-99: Non-proof flow is straightforward and readable.The fallback path correctly fetches and serializes the identity. Returning
Nonefor a missing identity is consistent with the rest of the query architecture.
101-102: Return statement is concise.Wrapping the response inside a
QueryValidationResulthelps maintain consistency with other platform query patterns.
105-131:test_invalid_public_key_hashis thorough.The test ensures that a short public key hash triggers the intended validation error. This coverage is crucial to prevent silent invalid input acceptance.
132-157:test_invalid_start_aftereffectively checks various invalid lengths.Including multiple negative test cases for lengths beyond and below 32 bytes ensures robust stringency in input validation.
159-192:test_identity_not_foundproperly checks absence of identity.The test validates that the returned identity field is
None, demonstrating a consistent usage of non-existence behavior.
194-218:test_identity_absence_proofensures proof structure is correct.Verifying that the response returns a
Proofvariant when no identity is found upholds the requirement for reliable non-existence proof handling.packages/rs-drive-proof-verifier/src/proof.rs (7)
18-21: New imports for non-unique public key hash requests are appropriate.Including
get_identity_by_non_unique_public_key_hash_requestis consistent with the newly added features and does not introduce extraneous dependencies.
332-343: Use ofverify_full_identity_by_unique_public_key_hashis well-aligned.Calling
Drive::verify_full_identity_by_unique_public_key_hashhere reinforces consistency with unique key hash verification logic. No logical issues detected.
346-355: NewFromProofimplementation: method signature is consistent.Defines the request and response types for non-unique key hash queries in a manner parallel to existing
FromProofpatterns.
359-380: Parsing response proof fields is thoughtful.Extracting
proved_responsefrom theV0Result::Proofavoids runtime confusion between proof-based and non-proof results. Properly handles missing proof errors.
383-402: Graceful handling ofpublic_key_hashandstart_afterfields.Providing a separate error message for invalid lengths clarifies the root cause for any malformed request data.
405-430: Construction of double-proof object is neatly organized.Combining
identity_proof_bytesandproof.grovedb_proofintoIdentityAndNonUniquePublicKeyHashDoubleProofensures a clear separation of identity-level proof vs. public key hash proof elements.
432-436: Tenderdash proof verification is consistent with existing patterns.Returning
(maybe_identity, mtd.clone(), proof)demonstrates a uniform approach to handling possible absence while still providing all proof and metadata details.
...rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs
Outdated
Show resolved
Hide resolved
This reverts commit c8502a3.
…hpay/platform into feat/getIdentitiesByPublicKeyHashes2
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs (1)
2147-2158: Consider extending timeout logging.While the timeout mechanism is good, consider enhancing it to include the specific test that timed out to make debugging easier.
- .map_err(|_| format!("test timed out after {:?}", TIMEOUT))? + .map_err(|_| format!("test timed out after {:?} while executing function {}", TIMEOUT, std::any::type_name::<F>()))?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/mod.rs(1 hunks)packages/rs-drive-abci/Cargo.toml(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/mod.rs
🧰 Additional context used
🧠 Learnings (1)
packages/rs-drive-abci/Cargo.toml (1)
Learnt from: shumkov
PR: dashpay/platform#2375
File: packages/rs-drive-abci/Cargo.toml:61-63
Timestamp: 2025-04-10T07:39:26.156Z
Learning: In the `drive-abci` package, avoid adding unused dependencies like `hashbrown` to `Cargo.toml`. The team relies on CI to detect dependency version issues.
🧬 Code Graph Analysis (1)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs (2)
packages/platform-test-suite/test/e2e/withdrawals.spec.js (1)
INITIAL_BALANCE(41-41)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs (10)
None(57-57)None(241-241)None(331-331)None(424-424)None(2883-2883)None(2970-2970)None(3064-3064)None(3184-3184)None(3285-3285)None(3377-3377)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build 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) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (12)
packages/rs-drive-abci/Cargo.toml (1)
106-106: LGTM: New test-case dependency properly added to dev-dependencies.The added dependency is appropriately placed in the dev-dependencies section since it's only used for testing.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs (11)
12-13: Good addition: Documented constant for initial balance.Adding this constant improves code clarity and maintainability by avoiding magic numbers throughout the test file.
91-91: Optimization: Using slices instead of vectors.You've consistently changed from creating new vectors to using slice references when calling
process_raw_state_transitions. This is a good optimization that avoids unnecessary heap allocations.Also applies to: 159-159, 229-229, 340-340, 462-462
503-625: Complete test coverage for fixed amount distribution.The added tests comprehensively cover various scenarios for fixed amount distribution, including edge cases like zero amounts and potential overflow conditions.
626-716: Good test entropy validation for random distributions.The random distribution tests include a sophisticated entropy calculation to ensure proper randomness in the token distribution, which is an excellent approach for validating the random function behavior.
815-1082: Comprehensive test matrix for step-decreasing distributions.You've created an extensive test matrix covering various combinations of parameters for step-decreasing distributions, which will help ensure robustness against edge cases.
1083-1137: Clear test case structure for stepwise distribution.The stepwise distribution test uses a clear structure with descriptive comments explaining the expected behavior at each step of the distribution.
1140-1254: Well-parameterized linear distribution tests.The test_matrix approach for linear distribution tests allows testing multiple parameter combinations efficiently. Good use of the test-case crate.
1257-1600: Thorough polynomial distribution testing.The polynomial distribution tests cover critical edge cases including potential divide-by-zero errors and overflow conditions. The test functions also include helpful error messages.
1603-1862: Comprehensive logarithmic distribution tests.The logarithmic distribution tests explore numerous parameter combinations and error conditions, ensuring robust handling of this distribution type.
1864-2123: Good edge case coverage for inverted logarithmic distribution.These tests thoroughly validate the inverted logarithmic distribution function's behavior with various parameter combinations and edge cases.
2125-2685: Well-designed test infrastructure.The test_suite module provides excellent reusable infrastructure for token distribution testing. The TestSuite and TestStep structures allow for modular and maintainable tests. The timeout mechanism is a good addition to prevent tests from hanging.
...n/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs (1)
23-29: Minor wording issue in error message.The error message uses an unusual word ordering "identity by public key non unique hash query".
- "could not decode identity by public key non unique hash query".to_string(), + "could not decode identity by non-unique public key hash query".to_string(),packages/rs-drive/src/drive/identity/fetch/queries/mod.rs (1)
236-251: Enhance documentation for the new method.While the implementation of
full_identity_with_non_unique_public_key_hash_queryis correct, the method comment at line 236 appears to be copied from the unique key hash method and doesn't explain the non-unique specific behavior or the purpose of theafterparameter.- /// This query gets the full identity and the public key hash + /// This query gets the full identity and the non-unique public key hash + /// + /// The `after` parameter supports pagination by specifying the identity ID to start + /// retrieving results from, as multiple identities may share the same public key hash.packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/v0/mod.rs (2)
11-53: Consider clarifying the function’s scope and the comment about proving absence.Currently, this function retrieves at most one matching identity, ignoring the possibility of multiple matches. It also contains the comment “We only prove the absence of the public key hash,” yet the function may also prove that a single identity exists. You might want to revise the comment to reflect that the function secures both presence (for the first match) and absence (if none is found or if there are subsequent identities). Otherwise, the code looks consistent for single-identity proofs.
92-93: Correct the test error message.This line checks for a non-unique public key, but the message says “expected a unique key.” To avoid confusion, update the message to accurately reflect that a non-unique key is expected here.
-.expect("expected a unique key") +.expect("expected a non-unique key")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/identity/fetch/queries/mod.rs(3 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/mocks/v2_test.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs
- packages/rs-platform-version/src/version/mocks/v2_test.rs
- packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/v0/mod.rs (2)
packages/rs-drive/src/drive/identity/fetch/queries/mod.rs (2)
identity_id_by_non_unique_public_key_hash_query(96-111)full_identity_query(129-142)packages/rs-drive/src/verify/identity/verify_full_identity_by_non_unique_public_key_hash/mod.rs (1)
verify_full_identity_by_non_unique_public_key_hash(53-78)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (9)
packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs (5)
1-12: Imports look good, with clear separation of dependencies.The imports are well-organized, with error handling, platform types, and request/response version imports properly separated.
17-22: Function signature correctly implements non-unique query handling.The method signature properly includes all necessary parameters - the request object, platform state, and platform version - for querying identities by non-unique public key hashes.
31-36: Feature version bounds correctly configured.The feature version bounds are properly retrieved from the platform version configuration structure for the non-unique public key hash query.
37-51: Version checking logic is robust.The code properly extracts the feature version from the request and verifies it against supported version bounds, returning an appropriate error message if the version is unsupported.
53-67:❓ Verification inconclusive
Consider future version handling.
While the current implementation correctly handles version 0, there's no explicit handling for future versions that might be added. As the platform evolves, ensure version matching remains robust.
🏁 Script executed:
#!/bin/bash # Check if there are other version variants defined in the protocol rg -A 5 "enum Version" packages/dapi-grpc/protos/platform/v0/get_identity_by_non_unique_public_key_hash_request.protoLength of output: 254
Action: Review Future Version Handling in Identity By Non-Unique Public Key Hash Query
The current match block only handles version 0. As we expect the protocol to evolve, please verify manually whether additional version variants exist (or are planned) in the protocol definitions. If future versions are introduced, consider updating this code to either add new match arms or include a default case that explicitly handles unsupported versions.
- Location: packages/rs-drive-abci/src/query/identity_based_queries/identity_by_non_unique_public_key_hash/mod.rs (lines 53–67)
- Suggestion:
match version { RequestVersion::V0(request_v0) => { /* existing logic */ }, _ => Err("Unsupported version".into()), // Consider explicit handling for future versions }Please manually verify the protocol definitions for any planned additional versions and update this section accordingly.
packages/rs-drive/src/drive/identity/fetch/queries/mod.rs (3)
3-3: Import added for non-unique key hashes.The addition of
non_unique_key_hashes_tree_path_vecis necessary to support the new non-unique key hash querying functionality.
94-111: Good implementation with clear comments.The implementation for
identity_id_by_non_unique_public_key_hash_queryis well-designed with support for pagination through theafterparameter. The comments at lines 105-107 effectively explain why a range full query is used instead of limiting results.
245-245: Consistent naming with clear differentiation.The code correctly calls
identity_id_by_non_unique_public_key_hash_queryinstead of the unique variant, maintaining consistency with the new non-unique functionality throughout the codebase.packages/rs-drive/src/drive/identity/fetch/prove/prove_full_identity_by_non_unique_public_key_hash/v0/mod.rs (1)
117-219: Thorough multi-identity test scenario looks good.The second test fully demonstrates pagination when two identities share the same non-unique key, verifying correct ordering and final absence. Implementation and test coverage appear robust.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rs-drive-proof-verifier/src/proof.rs (1)
343-433: Consider handling multiple identities per request.
Currently, this implementation retrieves only a singleIdentityeven though non-unique public key hashes might map to multiple identities. If your design intends to return all matching identities in a paginated fashion, consider returning a collection instead of a singleIdentity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
packages/dapi-grpc/build.rs(3 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto(2 hunks)packages/rs-dapi-client/src/transport/grpc.rs(1 hunks)packages/rs-drive-abci/Cargo.toml(1 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs(1 hunks)packages/rs-drive-abci/src/query/service.rs(2 hunks)packages/rs-drive-proof-verifier/src/proof.rs(4 hunks)packages/rs-drive/Cargo.toml(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/mod.rs(2 hunks)packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/v1.rs(2 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/mocks/v2_test.rs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/rs-drive-abci/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs
- packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs
- packages/dapi-grpc/build.rs
- packages/rs-drive-abci/src/query/service.rs
- packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/mod.rs
- packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs
- packages/rs-drive/Cargo.toml
- packages/rs-platform-version/src/version/drive_versions/drive_identity_method_versions/v1.rs
- packages/rs-platform-version/src/version/mocks/v2_test.rs
- packages/rs-dapi-client/src/transport/grpc.rs
- packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs
- packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs
- packages/dapi-grpc/protos/platform/v0/platform.proto
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build 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) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (3)
packages/rs-drive-proof-verifier/src/proof.rs (3)
19-22: Imports look good.
43-43: Newly imported struct is correctly referenced.
330-330: Updated call to the unique public key verification method.
QuantumExplorer
changed the title
Issue being fixed or feature implemented
Our desire is to get some identities by non unique public key hash. The way this works however is that you can only get one at a time, and then if it's not the one you want you must try to get another.
What was done?
Implements methods to get identities by non unique public key hashes.
How Has This Been Tested?
Added unit tests.
Breaking Changes
Non breaking.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores