[shumkov]

Issue being fixed or feature implemented

credit_pool_backfill_tests::should_preserve_historical_token_history_flags_across_public_activation_candidates fails on clean v5.1-dev:

panicked at packages/rs-drive-abci/tests/strategy_tests/test_cases/credit_pool_backfill_tests.rs:196:17:
assertion `left == right` failed
  left: Some(8000)
 right: Some(108)

The assertion reads the bytes allocated at the activation epoch (epoch 2) in the TokenHistory contract's storage flags after the upgrade.

Cause, bisected: #5579 (60cc5f4508, "store contracts compiled from protocol version 15") is the first failing commit on v5.1-dev's first-parent history; its parent c9c98b56de passes, as do 54c7429591 (the 5.0.1 merge) and 0d91c133f0.

The tests activated PlatformVersion::latest(). Since #5579, protocol version 15 stores contracts compiled (CONTRACT_VERSIONS_V7, contract_storage_serialization_version 2; v15.rs note 8). An upgrade from 13 straight to 15 runs transition_to_version_14 at version 15 (perform_events_on_first_block_of_protocol_change/v0/mod.rs, the < 14 && >= 14 guard), so its apply_contract re-stores TokenHistory v2 compiled. The contract grows by 7995 bytes instead of 105, and the activation epoch is charged 8000 bytes instead of 108. The owner and base epoch are still preserved.

This change was on purpose. #5579 updated the TokenHistory flag-merge unit test, should_preserve_token_history_flag_owners_and_allocation_epochs, to run at both 14 (108) and the latest version (8000), and its description says "the version 14 activation tests now run at 14, which they test". It missed this test because the chain-simulation suite runs only on push and nightly (tests-rs-workspace.yml, github.event_name != 'pull_request'), not in PR CI.

What was done?

These strategy tests describe the version 14 upgrade: the credit pool nullifier backfill, and the TokenHistory v2 re-store that keeps its historical owner and base epoch and charges the growth to the activation epoch. They now activate protocol version 14 through an activation_target() helper instead of PlatformVersion::latest(). Protocol version 14 is shipped and active on testnet, so the 108-byte allocation they pin is frozen behaviour, and the coding conventions allow pinning a frozen version.

The code is the same pin #5531 makes in this file. Only the helper's doc comment differs: #5531's reason ("the latest version's upgrade installs a later token history contract") holds on its stack but not on v5.1-dev, where the latest upgrade installs the same v2 contract stored compiled. Expect a one-comment conflict when #5531 merges this.

The 13 → latest growth (8000) stays pinned in PR CI by should_preserve_token_history_flag_owners_and_allocation_epochs. The upgrade_fork_tests strategy tests, including should_preserve_masternode_ports_across_activation_and_restart, still upgrade to the latest version.

No production code changes.

How Has This Been Tested?

• Red, at v5.1-dev 5ec96313e9 with no changes: cargo test -p drive-abci --test strategy_tests -- credit_pool_backfill gives 1 passed, 1 failed: left: Some(8000), right: Some(108).
• Green, with this change: same command, 2 passed.
• Bisect: git bisect run with --first-parent from 118830417e (chore: merge v5.0-dev into v5.1-dev #5404, which brought the test into v5.1-dev) to 5ec96313e9. It reported 60cc5f4508 (feat(drive)!: store contracts compiled from protocol version 15 #5579) as the first bad commit.
• cargo test -p drive-abci --lib -- token_history_storage_flags credit_pool_backfill: 11 passed.
• As CI runs the chain-simulation suite (--all-features, RUST_MIN_STACK=4194304): cargo test -p drive-abci --all-features --test strategy_tests -- token_tests upgrade_fork_tests credit_pool_backfill gives 13 passed, 0 failed, and 4 ignored (long-running #[ignore] nightly upgrade tests).
• cargo clippy -p drive-abci --all-targets --all-features -- --no-deps -D warnings: clean.
• cargo fmt --all -- --check: clean.

Breaking Changes

None. Test-only change.

Checklist:

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

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · 6a1d075

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

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

Summary by CodeRabbit

• Tests
  • Updated credit-pool backfill and token-history test coverage to validate behavior against protocol version 14.
  • Expanded lifecycle checks across rejected rounds, alternate candidates, and restarts.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The credit-pool backfill tests now target PlatformVersion 14. Proposal versions, contract reads, root calculations, nullifier checks, and consensus-version assertions use that target through the activation lifecycle, including rejection, retry, and restart checks.

Changes: Credit-pool backfill test version pinning

Layer / File(s)	Summary
Pin activation lifecycle and assertions packages/rs-drive-abci/tests/strategy_tests/test_cases/credit_pool_backfill_tests.rs	The tests define PlatformVersion 14 as the activation target. Proposal setup, token-history reads, root calculations, nullifier checks, and consensus-version expectations use that target. The lifecycle still checks rejected rounds, alternate candidates, and reopened state.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Possibly related PRs

• dashpay/platform#5579: Changes protocol-15 compiled contract storage and tests TokenHistory storage-flag preservation, which connects to the version-sensitive backfill test.

Suggested reviewers: thepastaclaw

Merge Risk: Minimal · up to 6a1d0

The change pins historical backfill tests to protocol version 14 without changing production code. No actionable merge-blocking risk is identified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: the credit pool backfill strategy tests now activate protocol version 14 instead of PlatformVersion::latest(). The raw summary confirms the test file changes match thi…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: the credit pool backfill strategy tests now activate protocol version 14 instead of PlatformVersion::latest(). The raw summary confirms the test file changes match this. The title uses the conventional test(scope): format and is specific enough for a teammate scanning history to identify the primary change. The title is 89 characters, which is within a reasonable length for a single-line subject.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

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

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

Share

• X
• Mastodon
• Reddit
• LinkedIn

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: shumkov/v51-fix-backfill-test
• File: .coderabbit.yaml
• Commit: 6a1d075
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: d3780539-c183-4463-8aab-814a9d109e1e

Commits

Reviewing files that changed from the base of the PR and between b3b664d and 6a1d075.

Files selected for processing: 1

• packages/rs-drive-abci/tests/strategy_tests/test_cases/credit_pool_backfill_tests.rs

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

[github-actions]

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

[thepastaclaw]

♻️ No change to this PR's diff since commit 8808cae (an empty commit, a merge of the base or a rebase onto it): that review stands — approved; @thepastaclaw review asks for a review of this commit (commit 6a1d075)

[thepastaclaw]

Final validation — Phase 1 + Phase 2

The test-only change consistently targets protocol version 14 across upgrade voting, proposal construction, storage reads, and restart assertions, preserving the historical 108-byte allocation expectation without changing production behavior. Separate unit coverage retains the latest-version 8000-byte allocation assertion, and the existing activation lifecycle scenarios remain intact. Static verification found no actionable defects; current CI has no failed code checks, but Rust workspace runner selection remains pending, so Rust validation is not independently confirmed.

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (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: low by gpt-6.1-sol (effort low) — This is a small, contained test-only change that pins version-14 upgrade scenarios to their intended activation target without changing production behavior.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — rust-quality (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 74% left, weekly 83% left; passed over gemini-3.8-flash-high (antigravity quota lookup failed)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort medium); agent phase2-reviewer

[github-actions]

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

[github-actions]

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