Skip to content

fix(sdk): bind voting proof queries to the default limit - #5356

Merged
QuantumExplorer merged 4 commits into
v5.0-devfrom
shumkov/fix-voting-proof-default-limit-20261009
Oct 11, 2026
Merged

QuantumExplorer merged 4 commits into
v5.0-devfrom
shumkov/fix-voting-proof-default-limit-20261009

Conversation

@shumkov

@shumkov shumkov commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Basic explanation

What this does: Proved vote-polls-by-end-date and contested-resource identity-votes queries with no caller-supplied limit send the SDK default of 100 and verify the proof against that same limit.

Value: An honest 100-row page can be verified when more votes or poll timestamps exist. This covers typed queries, raw requests and custom query producers.

Risks: This changes omitted-limit SDK requests. A node configured below 100 refuses them; callers can supply a supported explicit limit. Nodes configured above 100 return a 100-row page. Server validation, cryptographic verification and consensus behavior are unchanged. There is no node-cap discovery or clamping.

Issue being fixed or feature implemented

A proved voting response uses a finite server default, while the SDK previously verified an omitted limit as an unbounded query. Deterministic local reconstructions with 101 stored rows fail with Proof is missing data for query range before this change. The private audit inputs were unavailable; these are independently reconstructed regressions.

What was done?

  • Added an identity FetchMany::prepare_query hook before deriving the wire request and retaining the proof-verification query.
  • Public expect_fetch_many prepares its query before deriving both expectation keys; single-object Fetch and expectation removal retain their prior preparation.
  • Two overrides bind omitted limits to DEFAULT_QUERY_LIMIT only when the actual voting V0 request has prove: true. Explicit limits, unproved/unversioned requests and other query implementations retain their fields. Direct DAPI and FromProof callers are unchanged; offset/cursor fields and pagination traversal are preserved.
  • Other contested-resource queries (resource enumeration, vote state/contenders and voters for identity) retain their existing omitted-limit behavior.
  • SDK tests replay committed protobuf responses generated from real Drive state and deterministic Tenderdash signatures in the server-owned test target. A read-only corpus drift guard checks all 26 responses plus a PV14/public-key manifest; initialization and proof generation use the same pinned version, and regeneration is opt-in. The SDK remains verify-only, including native tests. Six explicit-limit recorded fixtures preserve the original fixture and response/proof bytes.

How Has This Been Tested?

  • Genuine RED/GREEN on the final fixture architecture: both public FetchMany regressions failed with the expected GroveDB query-range error on local tree 00935f6c, then passed on a78e4d62. Those trees differ only by the two voting preparation overrides; tests and all response bytes are identical, and both omitted/explicit mock keys are registered.
  • Mock-helper compatibility: identical public voting helper tests passed on upstream 1e69a4cb, failed on prior PR head 746f4d51 with MockExpectationNotFound, then passed after the companion correction. Final controls: 11 public mock tests and the existing expectation-removal test passed.
  • cargo test --locked -p dash-sdk --test main vote_poll_default_limit (7 passed on the final candidate): exact pages for short/full/truncated, descending and explicit-limit queries; request preservation; raw proof flags; altered proof/root/signature/query rejection; recorded proofs verified under legacy and explicit-limit requests.
  • Affected offline vector tests (vote_polls_by_ts, contested_resource_identity_votes, test_prefunded_specialized_balance_ok) first failed on new request keys, then passed using separately verified explicit-limit fixtures.
  • cargo test --locked -p drive-abci --test voting_default_limits: 4 passed (server caps 100/50, exact response equivalence, lower-cap refusal and read-only corpus drift guard). Two opt-in generations produced identical corpus bytes.
  • Full offline dash-sdk --test main: 145 passed, 15 pre-existing ignored, zero failed. This includes the affected recorded vectors and all public mock controls. Both targets passed cargo clippy -p <package> --test <target> --all-features --locked -- --no-deps -D warnings; owned-file formatting passed.
  • Standalone cargo tree -p dash-sdk -e normal,dev,build,features --target all contains no RocksDB or Drive full/server feature. Workspace all-targets coverage remains CI validation.
  • cargo check --locked -p dash-sdk --lib --no-default-features --features dapi-grpc/client and owned-file formatting checks passed.

All local tests use deterministic fixtures or committed offline responses on macOS with two Cargo jobs. No live queries, wasm32 or FFI runtime tests were run.

Breaking Changes

No wire-schema or consensus change. SDK omitted-limit proved voting requests now carry an explicit limit of 100; the configured-node-cap effects are described above. Existing implementations inherit an identity hook.

In-place changes to shipped generations

None. The production change is SDK request preparation; no versioned Drive/DPP generation or protocol table is edited.

Checklist:

  • I have performed a self-review of my own code
  • 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

PR Hygiene · 7f9a954

  • Bots — coderabbitai skipped after its own rate limit · 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.

Prepare the owned voting query before deriving its wire request and retaining
it for proof verification. Omitted limits on proved V0 requests use
DEFAULT_QUERY_LIMIT; other requests keep their fields through an identity hook.

Test would have caught this in CI: ✖ before fix, ✔ after. The two public
FetchMany regressions over 101 stored rows fail with missing query-range data
on the original SDK and pass with the same test bytes on the fixed SDK.

Cover typed, raw, custom and ProTxHash producers with genuine Drive proofs and
deterministic Tenderdash signatures. Preserve explicit limits, raw proof flags
and unrelated query bytes; refuse altered proofs, roots, signatures and limits.
Independently test public server defaults at 100/50 and lower-cap refusal.
Keep recorded fixtures and response bytes unchanged, adding verified explicit
limit request copies for offline compatibility.

This is client request preparation only: no server query semantics, consensus
generation, protocol table or offset behavior changes.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions github-actions Bot added this to the v5.0.0 milestone Oct 9, 2026
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: dashpay/platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 17e84f21-508e-40e4-b3e3-e6e10dc6fe1a

📥 Commits

Reviewing files that changed from the base of the PR and between ac12d70 and 746f4d5.


⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (14)
  • packages/rs-drive-abci/tests/voting_default_limits.rs
  • packages/rs-drive/tests/support/voting_default_limits.rs
  • packages/rs-sdk/Cargo.toml
  • packages/rs-sdk/src/platform/fetch_many.rs
  • packages/rs-sdk/tests/fetch/mod.rs
  • packages/rs-sdk/tests/fetch/vote_poll_default_limit.rs
  • packages/rs-sdk/tests/fetch/vote_poll_default_limit_controls.rs
  • packages/rs-sdk/tests/fetch/vote_poll_default_limit_vectors.rs
  • packages/rs-sdk/tests/vectors/contested_resource_identity_votes_not_found/msg_GetContestedResourceIdentityVotesRequest_258da6e5256df962780b73f9f3ad1ae91cbc444b7f1e59ae8f2ac12b6049081f.json
  • packages/rs-sdk/tests/vectors/contested_resource_identity_votes_ok/msg_GetContestedResourceIdentityVotesRequest_a2b96449c99ed7dc3457009aa5bb04449350c67e11fa686ce4ee4c913c0faa04.json
  • packages/rs-sdk/tests/vectors/test_prefunded_specialized_balance_ok/msg_GetVotePollsByEndDateRequest_be89641ab123d12c68d71850065166b8971da5fd5ffc42f8d9995243106fc4c6.json
  • packages/rs-sdk/tests/vectors/vote_polls_by_ts_ok/msg_GetVotePollsByEndDateRequest_be89641ab123d12c68d71850065166b8971da5fd5ffc42f8d9995243106fc4c6.json
  • packages/rs-sdk/tests/vectors/vote_polls_by_ts_order/msg_GetVotePollsByEndDateRequest_6c66b441bf6f9b37cb9b03fae2109c1fdf42718702c0d9b70d7c302d4ae5248b.json
  • packages/rs-sdk/tests/vectors/vote_polls_by_ts_order/msg_GetVotePollsByEndDateRequest_be89641ab123d12c68d71850065166b8971da5fd5ffc42f8d9995243106fc4c6.json

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



📝 Walkthrough

Walkthrough

The SDK now supplies DEFAULT_QUERY_LIMIT for proof-enabled V0 vote-poll and identity-vote queries when the caller omits a limit. New tests cover server limit behavior, SDK pagination, query serialization, proof verification, and recorded request vectors.

Changes

Voting Query Limits

Layer / File(s) Summary
Server limit fixtures and handler tests
packages/rs-drive/tests/support/voting_default_limits.rs, packages/rs-drive-abci/tests/voting_default_limits.rs
Shared fixtures create contested polls and abstaining votes. Handler tests compare omitted limits with configured limits and check errors for explicit limits above the configured default.
SDK query preparation
packages/rs-sdk/src/platform/fetch_many.rs, packages/rs-sdk/Cargo.toml
FetchMany adds a query-preparation hook. ResourceVote and VotePoll set the V0 limit to DEFAULT_QUERY_LIMIT when proofs are enabled and the limit is absent. Non-WASM test dependencies are added.
SDK pagination and proof tests
packages/rs-sdk/tests/fetch/mod.rs, packages/rs-sdk/tests/fetch/vote_poll_default_limit*.rs
Tests cover omitted and explicit limits, pagination, request serialization, proof rejection, unchanged caller requests, and recorded vote-poll and identity-vote vectors.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix


Merge Risk: ⚪ Minimal · up to 746f4

No actionable merge-blocking risk is established; the change is mergeable after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 46.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 7 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main SDK change: binding proved voting queries to the default limit.

Full details: Docstring Coverage

Explanation

Docstring coverage is 46.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 7 files. (1 skipped: 1 unsupported.)



  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 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

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

@shumkov
shumkov marked this pull request as ready for review October 9, 2026 12:21
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai ✓ · 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 9, 2026
@thepastaclaw

thepastaclaw commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

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

Apply the FetchMany preparation hook before deriving rich and wire mock keys,
so public voting expectations match the request that FetchMany executes.
Single-object Fetch and expectation removal retain identity preparation.

Test would have caught this in CI: the same public-helper regressions pass on
upstream 1e69a4c, fail on prior PR head 746f4d5 with MockExpectationNotFound,
and pass after the fix. Final controls: 11 public mock tests, 7 genuine voting
proof/signature tests, one existing removal test, and CI-flag SDK clippy pass.

Co-Authored-By: Codex <noreply@openai.com>

@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 request-preparation hook correctly binds the two affected voting queries before both transport encoding and proof verification. Verification against head 746f4d5 confirms one blocking violation of the stipulated client/server dependency rule and one mock-registration regression. This was a static review; no builds or tests were run, and Rust workspace tests and PR Hygiene were still pending in the supplied CI snapshot.

🔴 1 blocking | 🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gemini-3.8-flash-high (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: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: normal by gpt-6.1-sol (effort low) — The production change adds a contained SDK query-preparation hook and default-limit normalization, while the large remainder is tests and fixtures rather than intricate changes to cryptography, consensus or other critical surfaces.
  • Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer, gemini-3.8-flash-high — rust-quality (completed, effort high); agent phase1-reviewer
  • Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 35% left, 5h 66% left
  • 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 — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (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 `packages/rs-sdk/Cargo.toml`:
- [BLOCKING] packages/rs-sdk/Cargo.toml:56-57: Keep Drive's server feature outside the client SDK crate
  The new native dev-dependency enables Drive's `full` feature, which explicitly includes `server` in `packages/rs-drive/Cargo.toml`. Consequently, native SDK test builds enable GroveDB storage and server-side Drive code. This does not change the SDK's shipped production dependency tree or wasm32 builds, but it violates the supplied architectural rule forbidding client crates from enabling Drive's server feature; that rule provides no dev-dependency exception. Preserve the live-proof regressions in a separate integration-test harness outside the client crate, and keep dash-sdk's Drive dependency verify-only.

In `packages/rs-sdk/src/platform/fetch_many.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/fetch_many.rs:233-235: Apply query preparation when registering FetchMany mock expectations
  Execution now calls `Self::prepare_query`, but `MockDashPlatformSdk::expect_fetch_many` still calls `encode_rich_to_wire` without applying the hook. Registering an omitted-limit proved voting query therefore stores both expectation keys with `limit: None`, while the corresponding fetch executes a request with `limit: Some(100)`. MockDapiClient hashes the serialized request, so the fetch fails with `MockExpectationNotFound`; both vote-poll and identity-vote queries are affected. Apply the FetchMany implementation's preparation hook after constructing the rich query and before deriving either expectation key, and add regression coverage using the public `expect_fetch_many` helper. The added live-proof tests register raw response dumps instead, so they do not exercise this path.
Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

  • Contested voting queries ContestedResource, Contenders, and Voters in FetchMany omit prepare_query default limit — Outside this PR's scope. The three server handlers default omitted count values to the configured limit, while their proof-verifier decoders preserve None; however, those paths already behaved this way before the PR. The description explicitly scopes the change to two overrides and preserves other query implementations, and the identity preparation hook does not worsen these existing paths.
    • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

Comment thread packages/rs-sdk/Cargo.toml Outdated
Comment thread packages/rs-sdk/src/platform/fetch_many.rs
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them.
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 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet · thepastaclaw requested changes — dismiss the review or push a fix, 1 thread unresolved — resolve it. 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 waiting-bots Waiting for the review bots to report on this head and removed waiting-self-review Waiting for the author to post /self-reviewed labels Oct 9, 2026

@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.

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

At head dcf695c, request preparation binds both affected voting requests and their proof-verification queries to the same effective limit, and the public FetchMany mock-registration mismatch is fixed. One blocking architectural finding remains: this PR adds a native SDK dev-dependency that enables Drive's server feature. Verification was static; no builds or tests were run, and the supplied CI snapshot contains no completed Rust validation result while PR Hygiene remains pending.

🔴 1 blocking

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gemini-3.8-flash-high (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: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: normal by gpt-6.1-sol (effort low) — The production changes in fetch_many.rs and mock/sdk.rs normalize SDK request limits and expectation keys without changing cryptographic verification or consensus rules, while most of the large diff adds regression tests.
  • Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer, gemini-3.8-flash-high — rust-quality (completed, effort high); agent phase1-reviewer
  • Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 33% left, 5h 56% left
  • 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 — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (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 `packages/rs-sdk/Cargo.toml`:
- [BLOCKING] packages/rs-sdk/Cargo.toml:56-57: Keep Drive's server feature outside the client SDK crate
  (existing thread: https://github.com/dashpay/platform/pull/5356#discussion_r4230477534)
  This PR adds a native dev-dependency on Drive with `features = ["full", "verify"]`; `full` explicitly includes `server` in `packages/rs-drive/Cargo.toml`. Cargo resolver 2 keeps this activation out of ordinary downstream SDK builds, but native SDK builds requiring dev-dependencies unify it with the SDK's normal verification dependency. The new SDK regression tests consequently compile against server-enabled Drive and directly populate storage and generate proofs, crossing the stated client-crate feature boundary. Placing the shared fixture source under `rs-drive/tests` does not isolate that dependency: the SDK test includes it by path, so its server operations still compile inside the SDK target. Move live proof construction into a separate server-owned integration harness, or generate authenticated fixtures there and replay them through the SDK tests, leaving the SDK manifest verification-only. This is an architectural blocker, not evidence that server functionality ships to downstream SDK consumers or that an exploitable vulnerability exists.
Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

  • Extend default query count preparation to ContestedResource FetchMany queries — Outside this PR's scope: the ContestedResource FetchMany implementation remains unchanged, and the server's omitted-count default predates this change. The PR explicitly targets identity-vote and end-date poll queries; expanding preparation to another endpoint is an adjacent improvement, not a prerequisite for those fixes.
    • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

Keep dash-sdk verify-only even for native tests. Generate signed protocol-14
protobuf responses in the server-owned Drive ABCI target and verify them
through the public SDK FetchMany path. A default read-only drift guard checks
the committed corpus; regeneration requires an explicit environment flag.

Test would have caught this in CI: two public omitted-limit regressions
failed with GroveDB missing query-range data on tree 00935f6c, then passed
with identical tests and response bytes on a78e4d6. The RED tree removes
only the two voting preparation overrides and retains the default hook.

Verified: full offline SDK target 145 passed / 15 pre-existing ignored;
7 real-proof tests include exact root, signature, key and malformed-proof
refusals; server target 4 passed; two regenerations byte-identical; targeted
all-features clippy passed for both targets. The standalone SDK normal,
dev and build graph contains neither RocksDB nor Drive full/server.

No production query preparation, protocol tables or consensus behavior is
changed by this test-placement correction.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet · thepastaclaw ✓. 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 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.

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

Static verification of the complete diff at 08f8c7e confirms both prior findings are fixed: mock registration uses query preparation, and server-side fixture generation is outside the verify-only SDK crate. One non-blocking inconsistency remains in the frozen corpus's protocol-version pinning. No builds or tests were run; the supplied CI snapshot shows Kotlin validation queued and PR Hygiene pending, with no completed Rust validation results.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gemini-3.8-flash-high (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: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: normal by gpt-6.1-sol (effort low) — The production changes to FetchMany query preparation and mock expectation keys are contained SDK request-normalization logic, while most of the sizable diff adds fixtures and tests rather than changing cryptography, consensus, or other critical surfaces.
  • Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer, gemini-3.8-flash-high — rust-quality (completed, effort high); agent phase1-reviewer
  • Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 26% left, 5h 22% left
  • 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 — security-auditor (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, gpt-6.1-sol — security-auditor (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 `packages/rs-drive-abci/tests/voting_default_limits/corpus.rs`:
- [SUGGESTION] packages/rs-drive-abci/tests/voting_default_limits/corpus.rs:157: Initialize the frozen proof corpus with its pinned platform version
  The corpus pins population, proof generation, root hashing, and response metadata to PV14 through fixture::version(), but setup_drive_with_initial_state_structure(None) initializes storage with PlatformVersion::latest(). These snapshots currently coincide, so this is not a present test failure. When a later protocol changes initialization, the drift guard can report changes to the historical corpus solely because initialization selected a newer tree layout; opt-in regeneration would likewise emit PV14-labelled responses from that newer state. Pass Some(version) so initialization uses the same pinned snapshot as the rest of the corpus.
Out-of-scope follow-up suggestions (2)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

  • Extend query limit binding to contested vote state and contestant voters queries — Out of scope — the neighboring handlers default omitted counts to the configured limit while their request conversions preserve None, but those paths are pre-existing and unchanged. Their FetchMany implementations retain the identity preparation hook, and neither path is required by the two voting queries fixed in this PR; extending the fix is an adjacent improvement, not an in-scope defect.
    • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
  • Extend query preparation to other contested resource query types — Out of scope — omitted-count behavior in contested resource enumeration, contender state, and contestant voters predates this change. The PR neither worsens those paths nor relies on them to bind end-date poll and identity-vote requests, so the proposed broader audit is omitted from canonical review findings.
    • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

Comment thread packages/rs-drive-abci/tests/voting_default_limits/corpus.rs Outdated
Initialize the frozen voting proof corpus with its fixed platform version,
in addition to using it for population, proof generation, root hashing and
signed metadata. A later latest protocol must not alter historical fixtures.

There is no current behavior delta or honest red-to-green reproduction for
this setup refinement: the pinned version and latest are both PV14 today.
The existing four server tests and read-only drift guard passed from the
new source export, and all 27 committed corpus files remained byte-identical.
Targeted all-features clippy and formatting passed. Every SDK source and
asset remains identical to the tested parent, preserving its genuine
omitted-limit proof red-to-green and full offline suite evidence.

No production code, query semantics or protocol tables change.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — coderabbitai not yet · 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 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.

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

Independently reviewed the complete PR diff at 7f9a954 and found no remaining in-scope defects. Voting query preparation consistently binds transport requests, proof-verification queries, and public mock expectations to the same effective limit; server-owned fixture generation preserves the SDK's verify-only boundary, and all three prior findings are fixed. This was a static review without builds or test execution; the supplied CI snapshot shows Kotlin validation queued and PR Hygiene pending, and does not establish completed Rust validation.

🔴 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: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: normal by gpt-6.1-sol (effort low) — The SDK query-preparation hook, mock compatibility changes and extensive regression corpus warrant ordinary review, but the production diff does not implement intricate changes to cryptography, consensus or another critical surface.
  • 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 below 15% reserve: weekly 24% left, 5h 0% left), 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 — security-auditor (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, gpt-6.1-sol — security-auditor (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

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot added waiting-self-review Waiting for the author to post /self-reviewed bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. and removed waiting-bots Waiting for the review bots to report on this head labels Oct 9, 2026
@QuantumExplorer
QuantumExplorer merged commit 3898a8d into v5.0-dev Oct 11, 2026
29 of 30 checks passed
@QuantumExplorer
QuantumExplorer deleted the shumkov/fix-voting-proof-default-limit-20261009 branch October 11, 2026 05:32
@QuantumExplorer QuantumExplorer mentioned this pull request Oct 13, 2026
8 of 26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. waiting-self-review Waiting for the author to post /self-reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants