Repository navigation
test(sdk): re-record GroveDB V0 offline vectors as V1 and re-enable their tests - #5299
Merged
Merged
Conversation
…ir tests again #5294 made every client refuse GroveDB V0 proof envelopes, so 24 rs-sdk vector directories recorded on old devnets (protocol versions 4 and 9) could no longer be replayed, and their 16 rs-sdk tests, 8 test_case variants and 7 rs-sdk-ffi tests were ignored under offline-testing. Re-recorded them with scripts/generate_test_vectors.sh against a local devnet built from v5.0-dev (65e1969, protocol version 14, drive-abci and rs-dapi from source, SDK_TEST_DATA=true). Every recorded grovedb_proof now starts with byte 1 (V1). The contested-name tests ran against three identities contending dash/testname, two single-contender names (a11ce, b0bby) and three masternode votes on dash/testname. Removed the V0 ignores, and pointed the rs-sdk-ffi voters test at a contestant of the new recording. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 164 files, which is 64 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
📒 Files selected for processing (164)
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Waiting for bot review — coderabbitai not yet · thepastaclaw ✓. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
the
waiting-bots
Collaborator
|
✅ Final review complete — no blockers (commit c177826) · triage: normal |
thepastaclaw
approved these changes
Oct 6, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Verified the combined reviewer evidence against head c177826. The changes are limited to test re-enablement and refreshed recordings in the 24 directories containing V0 proofs; all 151 current proof envelopes are V1, corresponding quorum-key files exist, and the updated FFI contestant ID matches a recorded request. No actionable in-scope findings remain; validation was static, and the supplied CI snapshot reports successful Rust workspace, Swift, Kotlin, and JS SDK checks, with PR Hygiene pending.
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:
normalbygpt-6.1-sol(effort low) — This is a broad test-fixture refresh across 24 vector directories and SDK integration tests that requires checking replay consistency and preserved coverage, but changes no production critical surface. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort high); agentphase2-reviewer
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.
- Re-enabled voters test passes vacuously on None/empty — Out of scope — the permissive None/empty assertions already exist unchanged in the base version of test_contested_resource_voters_for_identity. This PR restores proof replay and updates the contestant ID to c5b2761e93571bc94ed94f0c7a2336c5ba60fe98c5f59eaf5142b0482594eaa6, which matches a recorded GetContestedResourceVotersForIdentityRequest with the required query fields. A missing or mismatched recording does not silently return None: MockDapiClient::execute returns MockExpectationNotFound, the FFI propagates that error, and the test panics. Stronger positive-result assertions would improve pre-existing test coverage but are not required for this PR's stated goal.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
Contributor
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
3 of 23 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Basic explanation
What this does: The Rust SDK has offline tests that replay recorded network answers ("test vectors") instead of talking to a real node. Some of those recordings were made long ago, in an old proof format (GroveDB proof envelope V0) that clients now refuse, so their tests were switched off. This re-records them against a fresh local network built from this branch, so they use the current format (V1), and switches the tests back on.
Value: 23 rs-sdk tests (16 tests plus 7
test_casevariants, with one more variant sharing a recording) and 7 rs-sdk-ffi tests run in CI again. That covers identity, balance, token, group-action, prefunded-balance and contested-name (masternode voting) queries. The Swift SDK'stestSimpleIdentityFetchreplays thetest_identity_readrecording, so it should pass again too.Risks: Low. Only test code and recorded JSON change; no runtime or consensus code is touched. The contested-name recordings depend on chain state that was set up by hand (see below), so recording them again needs the same setup.
Issue being fixed or feature implemented
#5294 makes every client refuse GroveDB V0 proof envelopes at every protocol version. 24 vector directories in
packages/rs-sdk/tests/vectors/, recorded on devnets at protocol versions 4 and 9, carried V0 envelopes (first byte of eachgrovedb_proofis0). Their tests were ignored underoffline-testingsince #4701.Before (on
v5.0-dev):After (this branch):
The 15 tests still ignored are unrelated: vectors were never recorded for them, or they are disabled for other reasons (PLAN-653, offline mode).
What was done?
packages/rs-sdk/scripts/generate_test_vectors.sh <test>(one test at a time, never the wipe-everything form). Everygrovedb_proofinpackages/rs-sdk/tests/vectors/now starts with byte1; a scan finds no V0 envelope left. No other vector directory changed.#[cfg_attr(feature = "offline-testing", ignore = "recorded vectors carry GroveDB V0 proofs; ...")]attributes (16) and the matching=> ignore[...]ontest_casevariants ofcontested_resources_fieldsandcontested_rss_vote_state_fields(8). Thenot(offline-testing)ignores that guard network runs needing the contested-name setup are kept.#[ignore]s intests/integration_tests/{identity,token,contested_resource}.rs, and pointedtest_contested_resource_voters_for_identityat a contestant of the new recording (c5b2761e..., the one holding two votes).How the vectors were recorded
dashmate setup local(3 masternodes) from65e1969c6f, drive-abci and rs-dapi built from source withbuildArgs.SDK_TEST_DATA="true", protocol version 14, regtest.create_sdk_test_data, so they need no extra setup.check_mn_voting_prerequisites, Seed DPNS contested-name prerequisites via SDK_TEST_DATA (extend create_sdk_test_data) #3720) need state that is still set up by hand. For this recording:rs-scripts'register_identity;testname, plusalice(a11ce) andbobby(b0bby) for two of them, throughSdk::register_dpns_name, giving 3 contested resources and 3 contenders ondash/testname;dash/testname(two for one contender, one for another) throughPutVote.How Has This Been Tested?
cargo test -p dash-sdk --test main(offline): 136 passed, 0 failed.cargo test -p rs-sdk-ffi --test integration: 34 passed, 0 failed, 0 ignored.--features generate-test-vectors).GetContestedResourceVotersForIdentityRequestfor the FFI contestant id exists and carries the voters, so that test cannot pass on a missing vector.rustfmt --checkon the edited test files.Breaking Changes
None.
Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
c177826/self-reviewedrust-sdk-ffi(packages/rs-sdk-ffi/tests/integration_tests/contested_resource.rs,packages/rs-sdk-ffi/tests/integration_tests/identity.rs,packages/rs-sdk-ffi/tests/integration_tests/token.rs) — lklimek or shumkovrust-sdk(packages/rs-sdk/tests/fetch/contested_resource.rs,packages/rs-sdk/tests/fetch/contested_resource_vote_state.rs,packages/rs-sdk/tests/fetch/contested_resource_voters.rsand 158 more) — lklimek or shumkovWhen every box is checked the
PR Hygienecheck passes and this can merge.