Skip to content

chore: merge v5.0-dev into v5.1-dev (5.0.1) - #5608

Merged
shumkov merged 12 commits into
v5.1-devfrom
shumkov/merge-v5.0.1-into-v5.1
Oct 28, 2026
Merged

shumkov merged 12 commits into
v5.1-devfrom
shumkov/merge-v5.0.1-into-v5.1

Conversation

@shumkov

@shumkov shumkov commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Basic explanation

What this does: Brings v5.1-dev up to date with v5.0-dev at 093268c09b: the 5.0.0 and 5.0.1 releases, ten PRs in all. The last forward merge was #5551. It merged 78d494f525, which is the base of this merge.

Value: v5.1-dev gets the 5.0.0 and 5.0.1 fixes:

Risks: Low.

Issue being fixed or feature implemented

v5.1-dev was ten PRs behind v5.0-dev.

What was done?

Merged origin/v5.0-dev (093268c09b) into origin/v5.1-dev (0d91c133f0) as a merge commit (ea5edddac8). One test-only commit follows (c5872a9b93, see "Test coverage of protocols 13 and 14").

Incoming PRs:

Pins and Cargo.lock

Conflicts resolved by hand

Semantic conflicts resolved (in the merge commit)

dpp::bls port of rs-drive-abci/tests/older_node_answers.rs (new in #5561). This test signs its proofs through blsful (dpp::bls_signatures), which v5.1-dev no longer ships (#5320). It is ported the same way v5.1-dev ported tests/nonunique_merged_proof.rs's mod signed:

v5.0-dev (blsful) v5.1-dev (dpp::bls)
SecretKey::<Bls12381G2Impl>::from_hash(b"older-node-answers-quorum-fixture") SecretKey::from_be_bytes(&scalar), scalar 13b8f447…7bfe
(&key().public_key()).into() key().public_key().to_bytes()
sign(SignatureSchemes::Basic, &digest)…as_raw_value().to_compressed() sign(&digest)…to_bytes()

dpp::bls has no from_hash, so the test uses the scalar that blsful derives from the same label. blsful computes it with HKDF-SHA256: salt BLS-SIG-KEYGEN-SALT-, input the label followed by a zero byte, info [0, 48], 48 bytes read big-endian modulo r. The same computation gives 6f295ecb…eea0 for nonunique-merged-proof-quorum-fixture, which is the scalar v5.1-dev already hardcodes. So the quorum key and signatures are the same ones v5.0-dev's test produces. No recorded vector depends on this key: the test signs answers it generates itself.

rs-sdk protocol_gate_tests::should_ask_a_fresh_sdk_network_for_its_version_once_then_send (#5561). The test ended with assert_eq!(sdk.protocol_version_number(), V14) and failed on the merge (left: 15, right: 14). The cause is the mock SDK, which answers every expectation with metadata at LATEST_VERSION (mock/sdk.rs). So the two mocked fee-pots answers raise the unpinned SDK on to the latest version, which is 14 on v5.0-dev and 15 here. #5560's description already notes this mock behaviour as a follow-up.

The test now expects max(14, latest). It still checks that the probe raised the SDK to 14, because ensure_network_serves sends only once min(SDK version, verified version) ≥ 14, so without that the first call would have been refused. The verified == 14 and single-probe assertions are unchanged. The test fails on the merge without this change and passes with it.

Protocol versions (LATEST = 15)

  • fix(sdk)!: refuse requests the network cannot serve yet instead of banning nodes #5561's gate.

    • Every needs_protocol_version declaration is fix(sdk)!: refuse requests the network cannot serve yet instead of banning nodes #5561's, all at 14 (PROTOCOL_VERSION_OF_5_0): the 13 methods 5.0 added, GetDocumentsRequestV1.chained and sub_queries, and the shielded requests' token_id.
    • packages/dapi-grpc/protos is byte-identical between v5.0-dev and v5.1-dev. So v5.1 adds no method, field, oneof arm or enum value that would need its own version (15), and rs-dapi-client --test request_inventory passes against v5.1's proto.
    • Both branches have the same set of TransportRequest impls, and the wrappers forward the declaration (delegate.rs and IdentitiesContractKeysQuery). EvoNode sends getStatus, which needs no version.
    • No v5.1-only code sends a request directly, outside the gated Fetch, FetchMany and FetchUnproved paths.
  • fix(sdk): retry document queries answered by nodes on an older protocol version #5560's exact-match document guard. An answer is verified only at the version that signed it, or at the latest known version when a newer version signed it. At LATEST = 15:

    • an unpinned SDK still starts at the network's minimum version (13 on mainnet and testnet) and follows verified answers;
    • so it verifies PV14 answers at 14 on testnet and PV13 answers at 13 on mainnet, and would verify a future PV16 answer at 15;
    • only an SDK pinned or seeded at 15 on a PV14 network sees its document queries refused as stale and then reported as a configuration error, which is the behaviour fix(sdk): retry document queries answered by nodes on an older protocol version #5560 designed.

    v5.1 adds no document-proof verifier. document_query_signed_version's source scan, which checks that every verifier reads proof bytes only through the guard, passes.

  • test(sdk): run operation-proof tests in CI and select the verifier generation from the verifier #5559's verifier-generation dispatch. PV15 selects DRIVE_VERSION_V9, as PV14 does: verifier generation 1 and prover generation 1. Protocols 1–13 select 0 and 0. The dispatch knows generations 0 and 1, so it serves every version from 1 to 15.

  • fix(drive-abci): answer queries for features not active yet with INVALID_ARGUMENT #5557. PV15 shares DRIVE_ABCI_QUERY_VERSIONS_V2 with PV14. v5.1's changes under rs-drive-abci/src/query add no query feature with an activation version. features_not_active runs at 14 and at the latest version, so now at 14 and 15.

  • Tests that follow "latest". I went through every test the ten PRs added or changed whose versions depend on PlatformVersion::latest():

    • Ranges and exhaustive loops mean the same at 15: protocol_gate_tests's decision matrix over every stored and verified version, nonunique_merged_proof, identity_by_non_unique_public_key_hash and identity_discovery::signed_versions. So do tests that pin 13 or 14: older_node_answers, drive-abci protocol_gate and the recorded vectors.
    • The refusal tests in document_query_signed_version and document_query_signature compare the previous version with the latest: 13 against 14 on v5.0-dev, 14 against 15 here. The guard they test only compares numbers, so they mean the same.
    • The gate test above was corrected in the merge commit.
    • Three tests stopped covering a version mainnet or testnet runs. The next section covers them.

Test coverage of protocols 13 and 14 (c5872a9b93)

#5560's tests chose their versions relative to the latest one. On v5.0-dev, where latest is 14, that covered 13 (mainnet) and 14 (testnet). At 15 the document query tests no longer run at 13, and the outcome layout test lost its case of an SDK at the producer's version. The follow-up commit makes them run over every version from 13 through the latest:

  • rs-drive-abci/tests/document_query_signature.rs should_verify_a_signed_document_answer_at_its_signed_version: [previous(), latest()] → 13..=latest.
  • rs-drive-abci/tests/document_query_signed_version.rs should_verify_document_answers_at_the_version_that_signed_them: previous and latest → 13..=latest, for every document verifier.
  • rs-sdk/src/sdk/signed_version_tests.rs should_verify_signed_document_outcome_layouts_without_lowering_security_or_pins: [(latest, 13), (latest, 14), (13, 14)] → stored 13..=latest × the recorded producers 13 and 14. That restores the "above, at or below the producer" its doc comment describes.

It also updates the comment on #5559's dispatch: it said protocol version 14 selects generation 1, and protocol 15 does too, so it now says "from 14".

With latest = 14 the first two ranges produce the lists those tests had. The third adds one case, an SDK at 13 reading a protocol 13 outcome. So the commit is equally correct on v5.0-dev. Mutation check: with the document guard temporarily refusing answers signed at protocol 13, both document tests pass on the merge commit and fail on c5872a9b93.

Files changed on both branches and merged automatically (worth a look in review)

v5.0-dev changed 137 paths. At the merge commit, 99 of them are as v5.0-dev has them: 98 byte-identical, plus #5559's deleted v1 verifier, which is absent on both branches. The other 38 are the five hand edits above and these 33 files. Each of the 33 is byte-for-byte what a clean three-way merge of the base, v5.1-dev and v5.0-dev produces:

Please merge with a merge commit (not squash) so the v5.0-dev history stays shared with v5.1-dev.

How Has This Been Tested?

Run locally on macOS (arm64).

On the merge commit ea5edddac8:

  • cargo check --workspace --all-targets --locked: passes.
  • cargo clippy --workspace --all-targets --all-features --locked -- --no-deps -D warnings (CI's invocation): passes. The amend that corrected the gate test touched only dash-sdk, so clippy was then run again with the same flags on dash-sdk alone, which passes.
  • cargo fmt --all -- --check: passes.
  • Codegen: the proto and the generated dapi-grpc clients are byte-identical across v5.0-dev, v5.1-dev and the merge (only the version fields of dapi-grpc's Cargo.toml and package.json changed), so there is nothing to regenerate.
  • yarn install --immutable --immutable-cache and yarn constraints: both pass, and the tree stays clean.
  • Targeted suites, all with --all-features --locked, all with 0 failed:
    • drive-abci integration tests:
      • protocol_gate: 3 passed (1 ignored, the vector exporter);
      • document_query_signed_version: 6 passed;
      • document_query_signature: 3 passed (1 ignored, the exporter);
      • document_outcome: 6 passed (1 ignored);
      • identity_discovery: 13 passed (1 ignored);
      • older_node_answers: 3 passed;
      • nonunique_merged_proof: 12 passed;
      • query_request_errors, including features_not_active: 20 passed.
    • drive-abci --lib -- identity_by_non_unique document_query: 175 passed (1 ignored).
    • dash-sdk --lib: all 508 passed. Of these, 69 are in protocol_gate_tests, signed_version_tests, identity_discovery_tests, platform::types::identity::tests and document_query_shape_tests.
    • drive-proof-verifier: lib and vector tests, 377 passed.
    • dash-platform-queries, including check_request_tests: 77 passed.
    • rs-dapi-client --test request_inventory: 5 passed (1 ignored).
    • drive --lib -- verify_state_transition_was_executed_with_proof: 83 passed.
    • platform-wallet-storage --lib -- parent_permissions (fix(platform-wallet-storage): open the wallet store on Android #5352): 7 passed.
  • CI's non-shielded phase, mirrored with cargo test because cargo-nextest is not installed here:
    • Invocation: CI's 26-package list with --all-features --locked --lib --bins --tests --no-fail-fast -- --skip shield and CI's RUST_MIN_STACK=4194304 CARGO_PROFILE_DEV_DEBUG=0 CARGO_PROFILE_DEV_CODEGEN_UNITS=256. It runs the full strategy_tests binary, not only the comprehensive simulation a PR run takes.
    • Result: 20,796 passed, 0 failed, 70 ignored. Of these:
      • dpp: 5,517 passed.
      • drive-abci lib: 4,169 passed (14 ignored); strategy_tests: 115 passed (4 ignored).
      • drive: 4,065 passed (6 ignored).
      • platform-wallet: 1,389; platform-wallet-storage: 438; platform-wallet-ffi: 427; rs-sdk-ffi: 392.
      • dash-sdk lib: 505; drive-proof-verifier lib: 349.
  • cargo check -p wasm-sdk -p wasm-dpp2 -p wasm-drive-verify --target wasm32-unknown-unknown --locked: passes.

On c5872a9b93:

  • The changed suites again: document_query_signature 3 passed, document_query_signed_version 6, dash-sdk signed_version_tests 26, and the drive verifier tests 83.
  • CI-flag clippy on drive-abci, dash-sdk, drive and drive-proof-verifier, and fmt.
  • The mutation check described above.

Two independent reviews of the merge, by separate agents with different lenses. Neither found a must-fix issue.

  • Merge integrity. Every v5.0-dev hunk is present and no v5.1-dev change was reverted. The 33 auto-merged files are exactly what git merge-file produces. Function counts in the merged .rs files equal v5.1 + v5.0 − base, so no hunk was duplicated. Nothing references the deleted v1 verifier or the old test paths. The lockfile delta equals v5.0-dev's. The reviewer also recomputed the three BLS fixture scalars on its own.
  • Versioning and PV15. This covered the gate's declarations and wrappers, bypass paths, the exact-match guard on testnet, mainnet and newer networks, the dispatch at every version, fix(drive-abci): answer queries for features not active yet with INVALID_ARGUMENT #5557, and the tests that follow "latest". Its findings are folded in: the gate test fix, the follow-up commit, and the notes below.

Known limitations and notes

Breaking Changes

The merge adds none of its own. #5560's and #5561's API changes carry over as they are:

  • drive_proof_verifier::Error::ProofProtocolVersionMismatch, dash_sdk::Error::ProtocolVersionNotActive, and StaleNodeError::ProtocolVersion and BehindProtocolUpgrade are new variants of exhaustive enums.
  • TransportRequest gains the provided method needs_protocol_version.
  • Requests that protocol 14 added are refused locally until the network has proven protocol 14.

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

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · c5872a9

  • Bots — thepastaclaw requested changes — dismiss the review or push a fix
  • Self-review — post /self-reviewed
  • Build running
  • Approvals
    • github (.github/workflows/tests-rs-workspace.yml) — ktechmidas
    • files with no dedicated owner — you own it
    • dashmate — you own it
    • system-contracts — you own it
    • js-wasm-sdk — you own it
    • rs-drive-abci — you own it
    • rs-drive — you own it
    • wallet-storage (packages/rs-platform-wallet-storage/SECRETS.md, packages/rs-platform-wallet-storage/src/parent_permissions.rs) — lklimek
    • rust-sdk-ffi — you own it
    • rust-sdk — you own it

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.

HashEngineering and others added 12 commits October 26, 2026 14:24
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…/0383 (#5582)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…68/0369 (#5583)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…LID_ARGUMENT (#5557)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ol version (#5560)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…nning nodes (#5561)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…neration from the verifier (#5559)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Brings 5.0.1 (v5.0-dev 093268c) into v5.1-dev: the 5.0.0 release
(#5594, with #5352, #5582, #5583), #5557, #5558, #5560, #5561, #5559 and
the 5.0.1 release bump (#5606).

Conflict resolutions:
- CHANGELOG.md: [Unreleased] on top, then the 5.0.1 and 5.0.0 entries.
- rs-sdk error.rs: both sides added methods to Error; kept v5.1's
  consensus_error and #5560's spares_responder and into_exhausted_error.
- rs-drive-abci tests/signed_response/mod.rs, moved from
  drive-proof-verifier by #5559: #5559's location, doc and
  drive_proof_verifier ContextProvider import, with v5.1's dpp::bls port.

Semantic conflicts resolved:
- rs-drive-abci tests/older_node_answers.rs (#5561): ported to dpp::bls,
  since v5.1-dev no longer ships blsful (#5320). The key is the scalar
  blsful's SecretKey::from_hash derived from the same label, so the
  public key and Basic-scheme signature bytes are unchanged.
- rs-sdk protocol_gate_tests.rs (#5561):
  should_ask_a_fresh_sdk_network_for_its_version_once_then_send expected
  the SDK at 14 after its calls. The mock answers every expectation at
  the latest version, which was 14 on v5.0-dev and is 15 here, so the
  mocked fee-pots answers raise an unpinned SDK on to 15 (left: 15,
  right: 14). The test now expects the higher of 14 and the latest
  version; that the probe raised the SDK to 14 is shown by the first
  call being sent at all. Fails on the merge without this change, passes
  with it.

Workspace and package versions follow v5.0-dev to 5.0.1, as the previous
forward merges did. rust-dashcore stays on dev aca02026.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#5560's tests chose their versions relative to the latest one: the
previous and the latest version, or the latest version paired with
recorded protocol 13 and 14 producers. On v5.0-dev, where latest is 14,
that covered 13 (mainnet) and 14 (testnet). With protocol 15 as the
latest version, the document query tests no longer ran at 13, and the
outcome layout test lost its SDK at the producer's version. They now
run over every version from 13 through the latest:

- drive-abci document_query_signature: a signed document answer is
  verified at each version from 13 through the latest.
- drive-abci document_query_signed_version: every document verifier
  reaches proof verification at each version from 13 through the latest.
- dash-sdk signed_version_tests: the outcome layouts of the protocol 13
  and 14 producers are verified by an SDK stored at each version from 13
  through the latest, so above, at and below each producer.

With latest = 14 the first two give the lists they had, and the third
adds an SDK at 13 reading a protocol 13 outcome, so the change is
equally correct on v5.0-dev. The refusal tests between the previous and
the latest version are unchanged; the guard they test compares numbers.

The comment on #5559's verifier dispatch said protocol version 14
selects generation 1; protocol 15 does too, so it now says "from 14".

Mutation check: with the document query guard refusing answers signed
at protocol 13, should_verify_a_signed_document_answer_at_its_signed_version
and should_verify_document_answers_at_the_version_that_signed_them pass
before this change and fail after it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@shumkov shumkov added this to the v5.1.0 milestone Oct 27, 2026
@coderabbitai

coderabbitai Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Too many files!

This PR contains 126 files, which is 26 over the limit of 100.

To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch.

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 0d91c13 and c5872a9.

Files ignored due to path filters: 9
  • Cargo.lock is excluded by !**/*.lock
  • packages/rs-drive-proof-verifier/tests/vectors/document-outcome/newer-contract.bin is excluded by !**/*.bin
  • packages/rs-drive-proof-verifier/tests/vectors/document-outcome/pv14-documents-response.bin is excluded by !**/*.bin
  • packages/rs-drive-proof-verifier/tests/vectors/protocol-gate/contracts-by-range-pv13.bin is excluded by !**/*.bin
  • packages/rs-drive-proof-verifier/tests/vectors/protocol-gate/epoch-probe-newer.bin is excluded by !**/*.bin
  • packages/rs-drive-proof-verifier/tests/vectors/protocol-gate/epoch-probe-pv13.bin is excluded by !**/*.bin
  • packages/rs-drive-proof-verifier/tests/vectors/protocol-gate/epoch-probe-pv14.bin is excluded by !**/*.bin
  • packages/rs-drive-proof-verifier/tests/vectors/protocol-gate/latest-versions-pv13.bin is excluded by !**/*.bin
  • packages/rs-drive-proof-verifier/tests/vectors/protocol-gate/quorum-key.bin is excluded by !**/*.bin
Files selected for processing: 126
  • .github/workflows/tests-rs-workspace.yml
  • CHANGELOG.md
  • Cargo.toml
  • book/src/SUMMARY.md
  • book/src/error-handling/drive-errors.md
  • book/src/evo-sdk/getting-started.md
  • book/src/sdk/fetch-traits.md
  • book/src/sdk/protocol-version-gate.md
  • book/src/sdk/put-operations.md
  • book/src/versioning/versioned-dispatch.md
  • package.json
  • packages/app-connect-contract/package.json
  • packages/bench-suite/package.json
  • packages/dapi-grpc/Cargo.toml
  • packages/dapi-grpc/package.json
  • packages/dapi/package.json
  • packages/dash-platform-queries/src/documents/array_element_clauses.rs
  • packages/dash-platform-queries/src/documents/average_proof_helpers.rs
  • packages/dash-platform-queries/src/documents/check_request_tests.rs
  • packages/dash-platform-queries/src/documents/composite_document_query.rs
  • packages/dash-platform-queries/src/documents/count_proof_helpers.rs
  • packages/dash-platform-queries/src/documents/document_average.rs
  • packages/dash-platform-queries/src/documents/document_count.rs
  • packages/dash-platform-queries/src/documents/document_having_entries.rs
  • packages/dash-platform-queries/src/documents/document_query.rs
  • packages/dash-platform-queries/src/documents/document_ranked_entries.rs
  • packages/dash-platform-queries/src/documents/document_split_averages.rs
  • packages/dash-platform-queries/src/documents/document_split_counts.rs
  • packages/dash-platform-queries/src/documents/document_split_sums.rs
  • packages/dash-platform-queries/src/documents/document_sum.rs
  • packages/dash-platform-queries/src/documents/mod.rs
  • packages/dash-platform-queries/src/documents/sum_proof_helpers.rs
  • packages/dash-spv/package.json
  • packages/dashmate/package.json
  • packages/dashpay-contract/package.json
  • packages/document-history-contract/package.json
  • packages/dpns-contract/package.json
  • packages/js-dapi-client/package.json
  • packages/js-dash-sdk/package.json
  • packages/js-evo-sdk/README.md
  • packages/js-evo-sdk/package.json
  • packages/js-grpc-common/package.json
  • packages/keyword-search-contract/package.json
  • packages/masternode-reward-shares-contract/package.json
  • packages/moderation-charters-contract/package.json
  • packages/platform-test-suite/package.json
  • packages/rs-dapi-client/src/transport.rs
  • packages/rs-dapi-client/src/transport/grpc.rs
  • packages/rs-dapi-client/tests/request_inventory.rs
  • packages/rs-dapi-client/tests/request_inventory/v4.1.1.txt
  • packages/rs-dapi-client/tests/request_inventory/v5.0.txt
  • packages/rs-drive-abci/Cargo.toml
  • packages/rs-drive-abci/src/query/document_query/v1/tests.rs
  • packages/rs-drive-abci/src/query/identity_based_queries/keys_remaining_budgets/v0/mod.rs
  • packages/rs-drive-abci/src/query/service.rs
  • packages/rs-drive-abci/tests/document_outcome.rs
  • packages/rs-drive-abci/tests/document_query_signature.rs
  • packages/rs-drive-abci/tests/document_query_signed_version.rs
  • packages/rs-drive-abci/tests/identity_discovery.rs
  • packages/rs-drive-abci/tests/identity_discovery/mainnet.rs
  • packages/rs-drive-abci/tests/older_node_answers.rs
  • packages/rs-drive-abci/tests/protocol_gate.rs
  • packages/rs-drive-abci/tests/query_request_errors.rs
  • packages/rs-drive-abci/tests/query_request_errors/features_not_active.rs
  • packages/rs-drive-abci/tests/query_request_errors/offsets.rs
  • packages/rs-drive-abci/tests/signed_response/mod.rs
  • packages/rs-drive-proof-verifier/Cargo.toml
  • packages/rs-drive-proof-verifier/src/error.rs
  • packages/rs-drive-proof-verifier/src/proof.rs
  • packages/rs-drive-proof-verifier/src/proof/chained_document.rs
  • packages/rs-drive-proof-verifier/src/proof/composite_document.rs
  • packages/rs-drive-proof-verifier/src/proof/document_average.rs
  • packages/rs-drive-proof-verifier/src/proof/document_count.rs
  • packages/rs-drive-proof-verifier/src/proof/document_having.rs
  • packages/rs-drive-proof-verifier/src/proof/document_ranked.rs
  • packages/rs-drive-proof-verifier/src/proof/document_sum.rs
  • packages/rs-drive-proof-verifier/src/verify.rs
  • packages/rs-drive-proof-verifier/tests/vectors/README.md
  • packages/rs-drive-proof-verifier/tests/vectors/document-outcome/README.md
  • packages/rs-drive-proof-verifier/tests/vectors/identity-discovery/README.md
  • packages/rs-drive-proof-verifier/tests/vectors/protocol-gate/README.md
  • packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/mod.rs
  • packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs
  • packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v1/mod.rs
  • packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v3.rs
  • packages/rs-platform-version/src/version/drive_versions/mod.rs
  • packages/rs-platform-wallet-storage/SECRETS.md
  • packages/rs-platform-wallet-storage/src/parent_permissions.rs
  • packages/rs-sdk-ffi/src/error.rs
  • packages/rs-sdk-ffi/src/sdk.rs
  • packages/rs-sdk-ffi/tests/integration_tests/document.rs
  • packages/rs-sdk-ffi/tests/integration_tests/ffi_utils.rs
  • packages/rs-sdk/README.md
  • packages/rs-sdk/src/error.rs
  • packages/rs-sdk/src/platform/data_contracts_latest_versions.rs
  • packages/rs-sdk/src/platform/delegate.rs
  • packages/rs-sdk/src/platform/fetch.rs
  • packages/rs-sdk/src/platform/fetch_many.rs
  • packages/rs-sdk/src/platform/fetch_unproved.rs
  • packages/rs-sdk/src/platform/identities_contract_keys_query.rs
  • packages/rs-sdk/src/platform/query_settings.rs
  • packages/rs-sdk/src/platform/types/epoch.rs
  • packages/rs-sdk/src/platform/types/identity.rs
  • packages/rs-sdk/src/sdk.rs
  • packages/rs-sdk/src/sdk/document_query_shape_tests.rs
  • packages/rs-sdk/src/sdk/identity_discovery_tests.rs
  • packages/rs-sdk/src/sdk/protocol_gate.rs
  • packages/rs-sdk/src/sdk/protocol_gate_tests.rs
  • packages/rs-sdk/src/sdk/quorum_key.rs
  • packages/rs-sdk/src/sdk/signed_version_tests.rs
  • packages/rs-sdk/src/sync.rs
  • packages/rs-sdk/tests/fetch/config.rs
  • packages/rs-sdk/tests/fetch/document.rs
  • packages/token-history-contract/package.json
  • packages/wallet-lib/package.json
  • packages/wallet-utils-contract/package.json
  • packages/wasm-dpp/package.json
  • packages/wasm-dpp2/package.json
  • packages/wasm-drive-verify/package.json
  • packages/wasm-sdk/package.json
  • packages/wasm-sdk/src/context_provider.rs
  • packages/wasm-sdk/src/error.rs
  • packages/wasm-sdk/src/queries/data_contract.rs
  • packages/wasm-sdk/src/sdk.rs
  • packages/wasm-sdk/tests/functional/contracts.spec.ts
  • packages/withdrawals-contract/package.json

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

  • 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

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

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

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.

@github-actions github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Oct 27, 2026
@thepastaclaw

thepastaclaw commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

⛔ Final review complete — 1 blocking finding(s) (commit c5872a9) · triage: critical

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final validation — Phase 1 + Phase 2

The merge preserves the intended protocol-version and proof-verification changes, but the public Fetch path can still send a V1 non-unique identity request without protocol gating. The new document-query preflight also omits grouped/HAVING document shapes and pending-window conflicts for aggregate queries, allowing malformed requests to reach the network. CI has queued checks but no reported code failures in the supplied snapshot.

🔴 1 blocking | 🟡 3 suggestion(s)

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

🔴 Blocking: Gate V1 non-unique identity requests before sending
packages/rs-dapi-client/src/transport/grpc.rs:952-959

The generated transport implementation leaves needs_protocol_version as None for GetIdentityByNonUniquePublicKeyHashRequest. The IdentityRequest delegate forwards that value, and the blanket Query<T> for T implementation makes a raw IdentityRequest::GetIdentityByNonUniquePublicKeyHash(...) usable through the public Identity::fetch path. A caller can therefore supply a V1 request while the SDK has only established protocol 13; the SDK sends it to an older node, whose unsupported-query response is retryable and can cause the node to be banned. This bypasses the central purpose of the protocol gate. Declare protocol 14 for V1 requests, while leaving V0 ungated, and add a regression test covering a raw V1 request through Identity::fetch.

source: gpt-6.1-sol (phase2-reviewer: general)

Review provenance

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

  • Triage: critical by gpt-6.1-sol (effort low) — This is a large, intricate merge whose diff changes cryptographic proof verification and signed-response handling across peer-facing SDK, Drive, and verifier code, including protocol-gated compatibility behavior.
  • 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)
  • Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); 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 `packages/rs-dapi-client/src/transport/grpc.rs`:
- [BLOCKING] packages/rs-dapi-client/src/transport/grpc.rs:952-959: Gate V1 non-unique identity requests before sending
  The generated transport implementation leaves `needs_protocol_version` as `None` for `GetIdentityByNonUniquePublicKeyHashRequest`. The `IdentityRequest` delegate forwards that value, and the blanket `Query<T> for T` implementation makes a raw `IdentityRequest::GetIdentityByNonUniquePublicKeyHash(...)` usable through the public `Identity::fetch` path. A caller can therefore supply a V1 request while the SDK has only established protocol 13; the SDK sends it to an older node, whose unsupported-query response is retryable and can cause the node to be banned. This bypasses the central purpose of the protocol gate. Declare protocol 14 for V1 requests, while leaving V0 ungated, and add a regression test covering a raw V1 request through `Identity::fetch`.

In `packages/dash-platform-queries/src/documents/document_query.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/documents/document_query.rs:632-647: Reject grouped and HAVING document queries during preflight
  `Documents::check_request` validates only the absence of subqueries and that the projection is `SelectFunction::Documents`. It does not reject `group_by` or `having`. The server's `validate_and_route` rejects `GROUP BY` with `SELECT DOCUMENTS`, and rejects HAVING for non-aggregate projections, before producing a proof. Since this preflight exists to prevent sending shapes that the verifier cannot reproduce, these invalid requests should be rejected locally; the same check is inherited by the single `Document` view.

In `packages/dash-platform-queries/src/documents/count_proof_helpers.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/documents/count_proof_helpers.rs:79-88: Validate pending window selections in aggregate request preflight
  `check_count_request` constructs its shape only from `request.where_clauses`; it ignores `time_range_clauses` and `integer_range_clauses`. The verifier later resolves those pending selections and calls `validate_resolved_time_range_clause_shapes`, which rejects an ordinary range or `IN` clause on the same field as a resolved window selection. Consequently, aggregate count requests with such conflicting clauses pass the local check and are sent before failing during proof verification. Apply the same pending-selection conflict rule in `check_count_request`; the equivalent omission exists in `check_sum_request` and `check_average_request`.

In `packages/rs-sdk/src/sdk.rs`:
- [SUGGESTION] packages/rs-sdk/src/sdk.rs:463-479: Preserve protocol-version ratcheting in the public metadata verifier
  `verify_response_metadata` remains public and documents that callers provide metadata from a proof and quorum signature they already verified, but it now performs only the freshness check. Before this change it also recorded the authenticated protocol version, updating the SDK's effective version and verified-version information. Consumers that manually verify a response and then call this public method no longer bootstrap protocol-version state, so later protocol-gated requests may probe unnecessarily or remain refused, and protocol-version observers do not receive the authenticated update. Restore the ratcheting behavior in this public method, while keeping the crate-private `_as_of` helper's separate semantics for internally handled mock or arrival-time paths.

Comment thread packages/dash-platform-queries/src/documents/document_query.rs
Comment thread packages/dash-platform-queries/src/documents/count_proof_helpers.rs
Comment thread packages/rs-sdk/src/sdk.rs
@github-actions

Copy link
Copy Markdown
Contributor

Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix.
Full checklist in the description.

@github-actions github-actions Bot added waiting-self-review Waiting for the author to post /self-reviewed and removed waiting-bots Waiting for the review bots to report on this head labels Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📖 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-27T18:06:48.463Z

@shumkov

shumkov commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Re the blocking finding on packages/rs-dapi-client/src/transport/grpc.rs (non-unique identity V1 not gated):

Agreed, this is a real gap: a hand-built V1 non-unique identity request reaches the network through Query<T> for T and bypasses the SDK's version selector. This code came in unchanged from v5.0-dev (it shipped in 5.0.1); this merge does not touch it. No 5.0.2 is planned, so we'll fix it on v5.1-dev in a follow-up PR right after this merge: gate V1 at protocol 14, update the request inventory, and add a regression test.

@shumkov
shumkov merged commit 54c7429 into v5.1-dev Oct 28, 2026
78 of 82 checks passed
@shumkov
shumkov deleted the shumkov/merge-v5.0.1-into-v5.1 branch October 28, 2026 01:42
shumkov added a commit that referenced this pull request Oct 28, 2026
…is proven

The getIdentityByNonUniquePublicKeyHash transport entry declared no
needs_protocol_version. NonUniquePublicKeyHashQuery picks V1 only once
the network basis is 14, but the blanket `Query<T> for T` lets a caller
pass a raw IdentityRequest carrying V1 to Identity::fetch, skipping that
selector. On a network proven only at 13 the request went out, every
node below 14 refused it with UNKNOWN, and the SDK retried and banned
honest nodes.

V1 now declares protocol 14 (V0 and an empty version declare nothing),
so the fetch path refuses it with ProtocolVersionNotActive, or asks the
network first, without contacting a node. The selector is unchanged: it
picks V1 only at a basis the gate already lets through. The request
inventory verdict for the V1 arm is now Gate(14).

Answers thepastaclaw's blocking finding on #5608.

Tests would have caught this in CI: ✖ before fix, ✔ after, with
grpc.rs swapped back to origin/v5.0-dev and the new tests kept:
- should_refuse_a_v1_non_unique_key_lookup_until_protocol_14_is_verified:
  ✖ "expected a refusal ..., 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)
- request_inventory should_need_the_declared_protocol_version_for_each_addition
  and should_need_protocol_14_only_for_composite_chained_token_pool_and_non_unique_v1_requests:
  ✖ left: None, right: Some(14)
All ✔ with the fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-self-review Waiting for the author to post /self-reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants