Skip to content

test: stop test storage fixtures aging out against the wall clock#971

Open
topocount wants to merge 1 commit into
mainfrom
kjs/neyn-12633-defuse-remaining-storage-timebombs
Open

test: stop test storage fixtures aging out against the wall clock#971
topocount wants to merge 1 commit into
mainfrom
kjs/neyn-12633-defuse-remaining-storage-timebombs

Conversation

@topocount

@topocount topocount commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #970. That PR fixed the timebomb that fired on 2026-07-16; a time-travel sweep afterwards found three more still armed. This defuses the two nearest and shrinks the third.

Why these rot

StorageSlot::is_active compares invalidate_at against wall-clock SystemTime::now(), so any fixture pinned to a fixed grant date eventually ages out of its validity window and the engine silently drops that fid's messages.

StorageSlot::merge compounds it — it skips an expired slot outright (return false), so an aged-out grant zeroes the surrounding unit assertions too, not just is_active itself. That's why a storage expiry shows up as unrelated consensus timeouts rather than an obvious storage error.

Changes

Rolled the grant date for fixtures that only need storage to exist: consensus_test's register_fid / register_fid_with_eth_custody, and the three block_engine_tests rent events. Unit type is incidental in all five, and a now-dated grant still classifies as UnitType2025. This is the test_helper::default_storage_event pattern.

Dropped the is_active assertions in test_storage_slot_from_rent_event. That test must pin historical dates — a unit's cohort is decided by its grant timestamp, so a now-dated grant can only ever be the newest cohort — which makes wall-clock assertions unusable there. Each was already followed by a deterministic invalidate_at assertion covering the same ground, and the V18 blocks in that test never asserted is_active at all. The one remaining is_active checks for false on a grant backdated past every multiplier: stable forever.

Pinned distinct log indices for the paired fixtures in test_storage_slot_with_mix_of_units. Each pair shares fid, type and timestamp, so the only thing separating their primary keys (type + fid + block_number + log_index) was the factory's rand % 1000 block-number jitter — a 1-in-1000 DuplicateOnchainEvent flake, measured at 1 failure in 500 runs, which is what took out CI on #969.

Verification

Swept with a temporary harness (env-var offset wired through FarcasterTime::current, factory::time::current_timestamp and StorageSlot::is_active). Harness is not part of this PR. Failures by simulated date:

Simulated date Before After
today, +60d 0, 2 0, 0
+1y (2027-07-13) 4 lib + 14 of 19 consensus 1 lib, 0 consensus
+2y, +5y 5, 5 2, 2

At present, all targets green: 802 lib, 19 consensus, 4 conformance, 1 client parity.

Note the 14 consensus failures were invisible to the original sweep, which only ran --lib.

Residuals (not fixed here)

Two tests can't be fixed test-side, tracked in NEYN-12633:

  • test_storage_slot_with_mix_of_units (2027-07-13) — its legacy/2024 fixtures must stay historical to classify as those cohorts, and merge consults the wall clock, so its unit assertions rot regardless of what we assert.
  • test_storage_limits (2028-07-12) — holds a live legacy + 2024 unit by design; same bind.

Both dissolve if is_active took an explicit reference time instead of reading SystemTime::now() — arguably right anyway, since consensus code shouldn't depend on the wall clock. That's a production change, deliberately out of scope here.

🤖 Generated with Claude Code


Note

Low Risk
Changes are limited to test fixtures and assertions; no production storage or consensus logic is modified.

Overview
Test-only hardening so storage rent fixtures stop failing as real time passes. StorageSlot::is_active() uses wall-clock now, so fixed grant dates eventually expire and the engine drops messages—often showing up as flaky consensus timeouts rather than obvious storage errors.

Fixtures that only need active storage now use create_rent_event_with_timestamp(..., current_timestamp()) in consensus_test (register_fid, register_fid_with_eth_custody) and three block_engine_tests cases, matching the existing default_storage_event pattern.

In test_storage_slot_from_rent_event, wall-clock is_active() checks on pinned historical cohort dates were removed (kept deterministic invalidate_at assertions and the safe expired is_active() == false case). Comments explain why cohort tests must keep fixed dates.

In test_storage_slot_with_mix_of_units, paired rent events get pinned log_index values to avoid rare DuplicateOnchainEvent flakes from factory block-number jitter, and the 2025 fixture uses a rolling grant timestamp so merge assertions stay valid.

No production code changes.

Reviewed by Cursor Bugbot for commit 8c3a17d. Bugbot is set up for automated code reviews on this repo. Configure here.

A time-travel sweep (env-var offset wired through FarcasterTime::current,
factory::time::current_timestamp and StorageSlot::is_active) found three
dated cliffs still armed after #970. This defuses the two nearest and
shrinks the third.

StorageSlot::is_active compares invalidate_at against wall-clock
SystemTime::now(), so any fixture pinned to a fixed grant date eventually
ages out of its validity window. StorageSlot::merge makes it worse: it
skips an expired slot outright (returns false), so an aged-out grant zeroes
the surrounding unit assertions too, not just is_active itself.

Rolled the grant date for fixtures that only need storage to exist:
consensus_test's register_fid / register_fid_with_eth_custody, and the
three block_engine_tests rent events. The unit type is incidental in all
five; a now-dated grant still classifies as UnitType2025.

Dropped the is_active assertions in test_storage_slot_from_rent_event.
That test must pin historical dates -- a unit's cohort is decided by its
grant timestamp, so a now-dated grant can only ever be the newest cohort --
which makes wall-clock assertions unusable there. Each one was already
followed by a deterministic invalidate_at assertion covering the same
ground, and the V18 blocks in that test never asserted is_active at all.
The remaining is_active assertion checks for false on a grant backdated
past every multiplier, which is stable forever.

Also pinned distinct log indices for the paired fixtures in
test_storage_slot_with_mix_of_units. Each pair shares fid, type and
timestamp, so the only thing separating their primary keys (type + fid +
block_number + log_index) was the factory's rand % 1000 block-number
jitter -- a 1-in-1000 DuplicateOnchainEvent flake, measured at 1 failure
in 500 runs, which took out CI on #969.

Verified with the harness. Failures by simulated date, before -> after:
  today, +60d:  0, 2        -> 0, 0
  +1y:          4 lib + 14 consensus -> 1 lib, 0 consensus
  +2y, +5y:     5, 5        -> 2, 2
At present, all targets green: 802 lib, 19 consensus, 4 conformance,
1 client parity.

Two residuals remain, both needing a production change rather than a test
fix, tracked in NEYN-12633:
  - test_storage_slot_with_mix_of_units (2027-07-13): its legacy/2024
    fixtures must stay historical to classify as those cohorts, and merge
    consults the wall clock, so its unit assertions rot regardless.
  - test_storage_limits (2028-07-12): holds a live legacy + 2024 unit by
    design; same bind.
Both dissolve if is_active took an explicit reference time instead of
reading SystemTime::now() -- which is arguably right anyway, since
consensus code should not depend on the wall clock.
@vercel

vercel Bot commented Jul 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
snapchain-docs Ready Ready Preview, Comment Jul 16, 2026 7:17pm

Request Review

@github-actions

Copy link
Copy Markdown

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

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