[shumkov]

Issue being fixed or feature implemented

This fixes thepastaclaw's blocking finding on #5608 (reply).

In rs-dapi-client, the getIdentityByNonUniquePublicKeyHash transport entry declared no needs_protocol_version. NonUniquePublicKeyHashQuery sends V1 only once the network basis (the lower of the stored version and the verified network version) is 14. But rs-sdk has a blanket impl<T> Query<T> for T, so a caller can pass a raw IdentityRequest::GetIdentityByNonUniquePublicKeyHash carrying V1 straight to Identity::fetch and skip that selector.

On a network proven only at protocol 13, that request still went out. A node below 14 refuses V1 with gRPC UNKNOWN: a 4.x node fails to decode the version, and a 5.0 node running protocol 13 rejects it as an unsupported query version. The SDK treats that status as a node fault, so it retried the request and banned honest nodes.

The bug shipped in 5.0.1. This PR targets v5.0-dev for the 5.0.x line, and the next v5.0-dev → v5.1-dev forward merge carries it into v5.1. It was first opened against v5.1-dev with no v5.0 backport planned; that no longer applies.

What was done?

• packages/rs-dapi-client/src/transport/grpc.rs: the entry now declares needs_protocol_version, the same way the neighbouring entries that match on request.version do. V1 needs PROTOCOL_VERSION_OF_5_0 (14). V0 and None need nothing.
  • On a network not yet proven at 14, the fetch path refuses a V1 request with Error::ProtocolVersionNotActive, or asks the network for its version first, before contacting any node.
  • The match lists every arm, so adding a V2 forces a decision here.
• The selector is unchanged. It sends V1 only at a basis of 14 or more, which the gate already lets through, so the gate never blocks the SDK's own V1. A test pins this.
• packages/rs-dapi-client/tests/request_inventory.rs:
  • The verdict for GetIdentityByNonUniquePublicKeyHashRequest.v1 changes from NoGate(...) to Gate(14).
  • The test that lists the request shapes needing 14 one by one now includes the V0 → None / V1 → Some(14) pair, and is renamed to match.
• packages/rs-sdk/src/sdk/protocol_gate_tests.rs: new tests that replay the signed identity-discovery vectors through the gate harness. These vectors are signed with the same test quorum key as the protocol-gate vectors.
  • should_refuse_a_v1_non_unique_key_lookup_until_protocol_14_is_verified: the SDK is verified at 13 and the node would answer V1 with UNKNOWN. A raw V1 passed to Identity::fetch is refused with ProtocolVersionNotActive after one probe; it is never sent and no node is banned. A raw V0 still goes out and finds the identity.
  • should_send_a_v1_non_unique_key_lookup_once_protocol_14_is_verified:
    • At 14, the SDK's own lookup sends the V1 request and gets through the gate with no probe.
    • On a fresh SDK, a caller's raw V1 goes out once, after the probe shows 14.
• packages/rs-sdk/src/sdk/identity_discovery_tests.rs: hash, request and response are now pub(super) so the gate tests can reuse them. CheckedAt is shared between test modules the same way.
• book/src/sdk/protocol-version-gate.md: the protocol 14 table now lists getIdentityByNonUniquePublicKeyHash request version 1. The page no longer groups it with the additions that ban no node.

How Has This Been Tested?

Red → green. On the v5.0-dev base, I swapped grpc.rs back to origin/v5.0-dev and kept the new tests. Every new or changed assertion fails, and all of them pass with the fix:

Test	Before the fix	With the fix
should_refuse_a_v1_non_unique_key_lookup_until_protocol_14_is_verified	✖ expected a refusal of get_identity_by_non_unique_public_key_hash, got Err(DapiClientError(Transport(Grpc(Status { code: Unknown, message: "unsupported query version", .. })))): the V1 request reached the node	✔
should_send_a_v1_non_unique_key_lookup_once_protocol_14_is_verified	✖ probes 0 != 1: a fresh SDK sent the caller's V1 without asking the network	✔
request_inventory::should_need_the_declared_protocol_version_for_each_addition	✖ left: None, right: Some(14)	✔
request_inventory::should_need_protocol_14_only_for_composite_chained_token_pool_and_non_unique_v1_requests	✖ left: None, right: Some(14)	✔

Local runs on the final tree:

• cargo test -p dash-sdk -p rs-dapi-client --all-features --locked --lib --bins --tests --no-fail-fast: 814 passed, 0 failed. The 27 ignored are the existing live-network tests and the inventory export.
• cargo clippy -p rs-dapi-client -p dash-sdk --all-targets --all-features --locked -- --no-deps -D warnings: clean.
• cargo fmt --all -- --check: clean.

Breaking Changes

None for callers of NonUniquePublicKeyHashQuery. Only a hand-built V1 request sent through the fetch path changes behaviour: before protocol 14 is proven, it is now refused locally instead of getting nodes banned.

Checklist:

• I have performed a self-review of my own code
• My Rust, FFI, WASM, Swift or Kotlin changes follow the coding conventions
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request
• If I added or changed a system contract's document schemas, I checked the contract viewer link posted on this pull request

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · f585aa6

• Bots — thepastaclaw ✓
• Self-review — post /self-reviewed
• Build green
• Approvals — you own every area touched; none needed

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

Summary by CodeRabbit

• Bug Fixes
  • V1 non-unique identity lookups now require a verified protocol version of 14 or higher. Requests using V0 remain available without a protocol-version requirement.
  • The SDK now blocks V1 lookups from being sent to nodes on protocol 13, while allowing them once protocol 14 is verified. When needed, the SDK checks the node’s protocol version before sending a V1 lookup.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: shumkov/v51-nonunique-v1-gate
• File: .coderabbit.yaml
• Commit: f585aa6
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: ab3196d6-8d72-442d-a9e2-c559212c9fe4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

✅ Final review complete — no blockers (commit f585aa6) · triage: normal

[thepastaclaw]

Final validation — Phase 1 + Phase 2

The PR correctly declares protocol 14 as required for V1 non-unique public-key-hash requests while preserving ungated V0 and absent-version behavior. The SDK gate, request delegation, request inventory, and regression tests consistently cover both raw requests and the SDK-selected request path. CI reports the Rust workspace tests and relevant build checks successful, with no in-scope issues found.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a contained Rust SDK transport-gating fix with meaningful protocol-version behavior and tests, but it does not meet the critical-tier requirement of a large diff changing consensus, cryptography, funds, storage migration, or peer-facing deserialization.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The runtime change correctly gates only V1 non-unique identity lookups at protocol 14 while preserving V0 behavior, and the relevant Rust workspace check passed. The protocol-gate book page still says that V1 is selected only by the SDK and that it does not need a transport declaration, so it should be updated to document the new raw-request gate.

🟡 1 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Update protocol-gate documentation for non-unique-key V1 requests
book/src/sdk/protocol-version-gate.md:22-25

The implementation now declares getIdentityByNonUniquePublicKeyHash V1 as requiring protocol 14, so hand-built V1 requests participate in the generic SDK gate and are refused or probed before reaching pre-14 nodes. This section still says that V1 is only selected by the SDK and groups it with additions that do not need a declaration, which no longer describes the complete behavior and could mislead callers about the safety of raw V1 requests. Add the request to the protocol-14 table and clarify that both the selector and the transport gate prevent V1 from reaching pre-14 nodes.

_{source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality)}

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a contained SDK/DAPI request-gating behavior change with meaningful protocol-version compatibility implications, but it does not itself alter consensus, cryptography, funds movement, or peer-facing deserialization.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `book/src/sdk/protocol-version-gate.md`:
- [SUGGESTION] book/src/sdk/protocol-version-gate.md:22-25: Update protocol-gate documentation for non-unique-key V1 requests
  The implementation now declares `getIdentityByNonUniquePublicKeyHash` V1 as requiring protocol 14, so hand-built V1 requests participate in the generic SDK gate and are refused or probed before reaching pre-14 nodes. This section still says that V1 is only selected by the SDK and groups it with additions that do not need a declaration, which no longer describes the complete behavior and could mislead callers about the safety of raw V1 requests. Add the request to the protocol-14 table and clarify that both the selector and the transport gate prevent V1 from reaching pre-14 nodes.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[github-actions]

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-10-28T14:52:46.869Z

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The change correctly gates hand-built V1 non-unique-key identity requests at protocol 14 while preserving unrestricted V0 behavior. The transport metadata is propagated through the existing SDK fetch and delegated request paths, and the inventory, integration tests, and documentation are consistent with the fix. Relevant CI checks passed; the remaining queued Kotlin SDK check is unrelated to this Rust/DAPI change.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a contained but cross-crate Rust logic change to protocol-version gating and request dispatch, without modifying consensus, cryptography, funds movement, storage, or peer-facing deserialization.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.