Skip to content

fix(cardano): align proptest epochs with strict assertions - #1050

Open
scarmuega wants to merge 3 commits into
mainfrom
fix/strict-proptest-epoch-alignment
Open

fix(cardano): align proptest epochs with strict assertions#1050
scarmuega wants to merge 3 commits into
mainfrom
fix/strict-proptest-epoch-alignment

Conversation

@scarmuega

@scarmuega scarmuega commented Jul 6, 2026

Copy link
Copy Markdown
Member

Problem

Running cargo test --workspace --all-features produced 14 test failures in dolos-cardano lib tests (115 passed; 14 failed).

All failures panicked at #[cfg(feature = "strict")] assertions in crates/cardano/src/model/epoch_value.rs (schedule, live_mut, replace, transition), which enforce that a delta's epoch equals the entity's current EpochValue epoch.

Root cause

The strict feature (enabled by --all-features) turns on epoch-alignment assertions that guard a production invariant: ESTART transitions every entity in lockstep with the ledger epoch, so a delta constructed at epoch E is always applied to an entity whose EpochValue is also at epoch E.

The proptest strategies violated this invariant by generating the entity and the delta with independently randomized epochs — both drawn from any_epoch() (3u64..1_000_000u64). When the two epochs disagreed (almost always), apply tripped the strict assertion.

Failing tests (14)

Module Tests
accounts stake_delegation_roundtrip, stake_delegation_serde_roundtrip, vote_delegation_roundtrip, vote_delegation_serde_roundtrip, stake_deregistration_roundtrip, stake_deregistration_serde_roundtrip, pool_delegator_retire_roundtrip, drep_delegator_drop_roundtrip, account_transition_roundtrip
epochs epoch_stats_update_roundtrip, epoch_transition_roundtrip, epoch_transition_v2_roundtrip
pools pool_registration_roundtrip, pool_transition_roundtrip

Fix

Add epoch-parameterized strategy variants that pin the EpochValue to a caller-chosen epoch, and rewire the 14 failing tests to share a single epoch between entity and delta via prop_flat_map.

For transition deltas (AccountTransition, PoolTransition, EpochTransition, EpochTransitionV2), the entity is generated at epoch and the delta at epoch + 1, matching the assert_eq!(self.epoch + 1, next_epoch) invariant.

New strategies

  • any_epoch_value_at(epoch, inner) / any_epoch_value_no_next_at(epoch, inner)epoch_value.rs
  • any_account_state_at(epoch)accounts.rs
  • any_pool_state_at(epoch)pools.rs
  • any_epoch_state_at(epoch) / any_epoch_state_no_rolling_next_at(epoch)epochs.rs
  • *_at(epoch) delta strategies for each affected delta

This models the production invariant the strict feature is meant to guard, rather than disabling the assertions.

Verification

cargo test -p dolos-cardano --all-features
test result: ok. 129 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
cargo clippy --workspace --all-targets --all-features
Finished `dev` profile [unoptimized + debuginfo] target(s)

The dolos-minibf workspace-level failure is pre-existing and unrelated (passes when run independently: 212 passed; 0 failed).

Summary by CodeRabbit

  • Bug Fixes
    • Improved consistency in epoch-related account, epoch, and pool state handling.
    • Strengthened roundtrip checks for state changes across epoch boundaries, reducing the chance of hidden edge-case regressions.
    • Expanded test coverage for transitions, registrations, and serialized data paths to better validate real-world epoch-matched scenarios.

The `strict` feature (enabled by `--all-features`) turns on
epoch-alignment assertions in `EpochValue` methods (`schedule`,
`live_mut`, `replace`, `transition`). These assertions enforce a
production invariant: a delta's epoch must equal the entity's current
`EpochValue` epoch, which holds because ESTART transitions every
entity in lockstep with the ledger epoch.

The proptest strategies generated entity and delta with independently
random epochs (both drawn from `any_epoch()`), violating the
invariant and tripping the strict assertions under `--all-features`.

Add epoch-parameterized strategy variants (`any_epoch_value_at`,
`any_account_state_at`, `any_pool_state_at`,
`any_epoch_state_at`, etc.) and rewire the 14 failing tests to
share a single epoch between entity and delta via `prop_flat_map`.
For transition deltas, the entity is generated at `epoch` and the
delta at `epoch + 1`, matching the `assert_eq!(self.epoch + 1,
next_epoch)` invariant.

All 14 previously-failing tests now pass; dolos-cardano reports
129 passed, 0 failed.
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/strict-proptest-epoch-alignment

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.

scarmuega added 2 commits July 6, 2026 17:41
The CI already runs clippy with --all-features but tests only with
default features. This let 14 strict-feature proptest failures slip
through undetected. Add a second test step with --all-features to
catch feature-gated code paths.
The --all-features test step exposed a pre-existing bug in the work
buffer (crates/cardano/src/work.rs): when blocks jump multiple epochs
(e.g. from epoch 0 to epoch 2 during bulk import), the EstartBoundary
pop_work arm puts the block into OpenBatch without re-checking if the
block is still in a later epoch. This causes EpochStatsUpdate::apply
to call live_mut with the wrong epoch, tripping the strict assertion.

Fixing this requires passing ChainSummary to pop_work so it can detect
multi-epoch jumps — a separate refactor. Revert the CI change until
that bug is fixed.
Sign up for free to 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