Skip to content

test(drive-abci): gate PR runs to one comprehensive chain simulation - #4297

Merged
QuantumExplorer merged 4 commits into
v4.2-devfrom
claude/pr-strategy-test-gate
Aug 5, 2026
Merged

test(drive-abci): gate PR runs to one comprehensive chain simulation#4297
QuantumExplorer merged 4 commits into
v4.2-devfrom
claude/pr-strategy-test-gate

Conversation

@QuantumExplorer

@QuantumExplorerQuantumExplorer commented Aug 5, 2026

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

The drive-abci strategy suite runs 92 whole-chain simulations on every Rust PR — 261s of test CPU, 18% of the entire workspace suite — and its 7–13s simulations set the wall-clock floor of the test phase. Most PRs get no signal from running all of them.

What was done?

New comprehensive simulation (comprehensive_tests.rs): one 30-block, ~6s run that packs every subsystem that composes deterministically:

  • per-block identity creation, top-ups, key additions, credit withdrawals, credit transfers
  • document create/replace/delete on dashpay, random contract creation
  • token minting against a precomputed token contract
  • address funding from core asset locks, address-to-address transfers, identity top-ups from address balances
  • random core height increases with validator quorum rotation
  • two epoch changes with masternode payouts

with verify_state_transition_results (per-transition proof verification), verify_sum_trees, and a final grovedb integrity check. Verified via the run's state-transition stats that all ten transition types actually execute (98 IdentityCreate, 207 Batch, 41 TopUp, 15 CreditTransfer, 11 Withdrawal, 12 IdentityUpdate, 9 ContractCreate, plus address ops).

Determinism trick: a single hard-coded start identity means contract deployment (block 2, before identity inserts begin at block 3) has exactly one owner candidate, so the token contract's id — and the token id the mint op signs against — is precomputable.

CI gating (tests-rs-workspace.yml): on pull_request events the nextest filter excludes binary_id(drive-abci::strategy_tests) except this test. Push, nightly, and dispatch runs keep the full suite — the same post-merge safety-net pattern as the shielded phase, so a regression in an excluded simulation surfaces minutes after merge. Filter verified with cargo nextest list: exactly 1 of 97 strategy-binary tests selected, lib/bin tests unaffected.

Latent panic fix (strategy-tests): IdentityTransfer(None) drew a recipient from an empty range when exactly one identity existed (gen_range(0..0)); the guard now requires two identities.

Not covered by the PR-path simulation (nightly/push still cover them): shielded operations (separate CI phase with its own cache), contested-resource voting, protocol upgrade forks, failure injection, and masternode list mutations.

Expected saving: the strategy tail currently bounds the test phase's 117s wall time; with it gone the tail is the 19s wallet-storage rekey test, so roughly ~60s off every Rust PR job. PR coverage on codecov will dip slightly relative to base since the excluded simulations' coverage only appears on push runs.

How Has This Been Tested?

  • New simulation passes locally in 5.8s, deterministic across runs (fixed seeds).
  • State-transition stats confirm every operation type executes rather than silently no-oping.
  • cargo nextest list with the exact CI filter expression selects 1 strategy test + all non-strategy tests.
  • cargo fmt and cargo clippy --all-features clean on both changed crates.

Breaking Changes

None.

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

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Prevented identity-to-identity transfers from failing when only one identity is available.
    • Improved reliability of immutable-structure checks in pull request validation.
  • Tests

    • Added comprehensive coverage for mixed platform operations across 30 blocks.
    • Expanded verification of state transitions, payouts, asset operations, and database consistency.
    • Improved strategy test execution and coverage reporting, including carryforward coverage for skipped areas.

The drive-abci strategy suite (92 whole-chain simulations, 261s of test
CPU — 18% of the entire workspace suite) runs on every Rust PR while
its multi-second simulations set the wall-clock floor of the test
phase. Replace it on the PR path with a single comprehensive
simulation and keep the full suite on push and nightly runs, the same
safety-net pattern as the shielded phase.
The new simulation packs the subsystems that compose deterministically
into one 30-block run (~6s): per-block identity creation, top-ups, key
additions, credit withdrawals, credit transfers, document
create/replace/delete on dashpay, random contract creation, token
minting, address funding from core asset locks, address transfers,
identity top-ups from address balances, random core height increases
with validator quorum rotation, and epoch changes with masternode
payouts — with per-transition proof verification and sum-tree
verification enabled. A fixed seed keeps it deterministic; a single
hard-coded start identity forces contract ownership so the token
contract id (and the token id the mint op signs against) is
precomputable.
Also fixes a latent panic in strategy-tests: IdentityTransfer(None)
drew a recipient from an empty range when exactly one identity
existed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actionsgithub-actionsBot added this to the v4.2.0 milestone Aug 5, 2026
@coderabbitai

coderabbitaiBot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The workflows now resolve immutable-structure baselines from the merge commit’s first parent. Workspace CI runs a comprehensive strategy test, separates coverage phases, and uploads coverage by category. Identity transfers now require at least two identities.

Changes

Strategy validation

Layer / File(s)Summary
Merge-base immutable checks
.github/workflows/tests-rs-wallet.yml, .github/workflows/tests-rs-workspace.yml
The workflows derive changed Rust files and baseline contents from the merge commit’s first parent.
Comprehensive mixed-operations strategy test
packages/rs-drive-abci/tests/strategy_tests/test_cases/comprehensive_tests.rs, packages/rs-drive-abci/tests/strategy_tests/test_cases/mod.rs
A deterministic 30-block strategy test configures mixed platform operations, mock RPC responses, verification, and final state assertions.
Strategy test selection and transfer guard
.github/workflows/tests-rs-workspace.yml, packages/strategy-tests/src/lib.rs
Workspace CI retains the comprehensive strategy test, runs remaining strategy tests separately on push and nightly events, and requires at least two identities for identity transfers.
Coverage reuse and uploads
.github/workflows/tests-rs-workspace.yml, .codecov.yml
Coverage reuse and completion tracking now distinguish strategy and shielded phases. Codecov uploads use separate rust, rust-strategy, and rust-shielded files with carryforward enabled.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
participant comprehensive_mixed_operations_test
participant NetworkStrategy
participant Platform
participant MockCoreRpc
participant Grovedb
comprehensive_mixed_operations_test->>NetworkStrategy: configure mixed platform operations
NetworkStrategy->>Platform: execute 30 blocks
Platform->>MockCoreRpc: broadcast asset-unlock transactions
MockCoreRpc-->>Platform: return unknown unlock statuses
Platform->>Grovedb: verify state transitions and sum trees
Grovedb-->>comprehensive_mixed_operations_test: return zero reported issues
Loading

Possibly related PRs

Suggested reviewers:vivekgsharma, shumkov

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly describes the main change: limiting pull-request drive-abci runs to one comprehensive chain simulation.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/pr-strategy-test-gate

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.

@codecov

codecovBot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.04%. Comparing base (5bbd7c9) to head (c1e9a00).

Additional details and impacted files
@@ Coverage Diff @@## v4.2-dev #4297 +/- ##
============================================
- Coverage 87.61% 87.04% -0.57% 
============================================
Files 2704 2704 Lines 345206 345206 ============================================
- Hits 302443 300483 -1960 - Misses 42763 44723 +1960 
ComponentsCoverage Δ
dpp88.61% <ø> (-0.23%)⬇️
drive85.86% <ø> (-0.40%)⬇️
drive-abci88.33% <ø> (-1.33%)⬇️
sdk∅ <ø> (∅)
dapi-client∅ <ø> (∅)
platform-version∅ <ø> (∅)
platform-value92.88% <ø> (ø)
platform-wallet∅ <ø> (∅)
drive-proof-verifier48.02% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

QuantumExplorerand others added 3 commits August 5, 2026 17:16
The check listed changed files via `gh pr view`, and a runner with
broken connectivity to api.github.com failed the whole Rust job three
times in a row before any test ran — while git fetches to github.com
kept working from the same box. The pull_request checkout is the PR
merge commit, whose first parent is the base-branch tip the merge was
built on, so the changed-file list and base file contents now come
from `git diff`/`git show` against that parent, entirely over git
transport. Verified against a live merge ref that the first parent
equals the base tip and the diff yields exactly the PR's files.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Excluding the chain-simulation suite on PRs made every PR report a
spurious ~0.57% project-coverage drop (drive-abci -1.33%): base
coverage comes from full-suite push runs while PRs upload gated
coverage. Split coverage into three codecov flags — rust (always
uploaded), rust-strategy (chain simulations, push/nightly only), and
rust-shielded (shielded phase, when it runs) — with carryforward
enabled, so codecov reuses the base commit's coverage for any flag a
PR run doesn't upload and PR reports stay comparable.
The chain simulations now run as their own phase (push/nightly only)
with their own lcov, and the nextest phase uses the same gated filter
on every event so the rust flag measures the same population
everywhere. Carryforward also replaces the shielded coverage cache
from #4293 entirely — instead of restoring a content-verified cached
lcov on skip runs, the flag is simply not uploaded and codecov carries
the base's forward, which deletes the content-hash, cache-restore, and
cache-save steps. The complete-suite tree-hash marker now additionally
requires the chain-simulation phase to have succeeded, so a PR
fast-path run cannot mark its tree as fully tested for a same-tree
post-merge push run.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@QuantumExplorer

Copy link
Copy Markdown
MemberAuthor

Pushed two additions since the last review pass:

  • c1e9a00e40 — codecov carryforward flags. Coverage now uploads as three flags: rust (always), rust-strategy (chain simulations, push/nightly only), rust-shielded (when the shielded phase runs). With carryforward: true in .codecov.yml, codecov reuses the base commit's coverage for any flag a PR doesn't upload, so the ~0.57% spurious project-coverage drop this PR previously showed disappears from future PR reports. This also replaces the shielded coverage cache from ci: skip shielded Rust tests on PRs without shielded changes #4293 entirely (content-hash/restore/save steps deleted) — not uploading the flag is strictly simpler than restoring a verified cached lcov, and the skip decision itself still comes from the shielded-change detector. The complete-suite tree-hash marker now requires the chain-simulation phase to have succeeded too, closing the same marker hole for the strategy suite that was previously closed for shielded.
  • 839d6c99ff — immutable-structure check over git transport. The step's gh pr view call failed three consecutive runs on a runner with broken api.github.com connectivity; the changed-file list and base contents now come from git diff/git show against the PR merge commit's first parent (verified against the live merge ref that the first parent equals the base tip).

🤖 Generated with Claude Code

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.codecov.yml:
- Around line 9-15: Bootstrap the new Codecov flags before enabling
carryforward: update .codecov.yml for rust-strategy and rust-shielded and adjust
.github/workflows/tests-rs-workspace.yml at lines 326-328 so the migration PR
uploads both reports, or configure carryforward to use the last complete commit
containing each flag rather than the combined-only baseline.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: deb98897-adb3-4e92-a278-69cddc917456

📥 Commits

Reviewing files that changed from the base of the PR and between 839d6c9 and c1e9a00.

📒 Files selected for processing (3)
  • .codecov.yml
  • .github/workflows/tests-rs-wallet.yml
  • .github/workflows/tests-rs-workspace.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/tests-rs-wallet.yml

Comment thread.codecov.yml
@QuantumExplorer
QuantumExplorer merged commit 91f24ee into v4.2-devAug 5, 2026
20 checks passed
@QuantumExplorer
QuantumExplorer deleted the claude/pr-strategy-test-gate branch August 5, 2026 13:01
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@QuantumExplorer