fix(stake): promote cooldown-timelock fields out of colliding _reserved bytes

[Morenikeoa]

Problem

#242's two-phase cooldown-increase timelock packed pending_cooldown_slots and cooldown_proposed_at_slot into StakePool::_reserved[10..26], believing that range was free ("the previously-free _reserved[10..26] region"). It wasn't: PERC-313's high-water-mark feature already owns the entire _reserved[10..32] range (hwm_enabled at [10], hwm_floor_bps at [11..13], epoch_high_water_tvl at [16..24], hwm_last_epoch at [24..32]).

Both features are reachable on the same pool with no guard preventing co-use:

• Proposing a cooldown increase overwrites hwm_enabled/hwm_floor_bps/part of epoch_high_water_tvl, silently disabling HWM or wedging the withdrawal floor so it can never be satisfied.
• refresh_hwm() — called on every deposit/withdraw/junior-deposit once HWM is enabled — overwrites cooldown_proposed_at_slot and part of pending_cooldown_slots, corrupting or fabricating a pending cooldown proposal and voiding the timelock's 48h LP exit-window guarantee.

The existing collision regression test (pending_cooldown_bytes_do_not_collide_with_neighbors) checked against the discriminator, version, market_resolved, the tranche flag, and realized_junior_loss — every neighbor except the HWM region it actually overlaps, which is why this shipped unnoticed.

Fix

Promoted both fields to real StakePool struct fields, mirroring exactly how pending_admin was added for the v1→v2 bump (same rationale: _reserved didn't have enough contiguous free bytes, so this is a real field, not more _reserved packing): STAKE_POOL_SIZE 384 → 400, CURRENT_VERSION 2 → 3. Fresh-start cutover per existing project convention — no v2 pools exist, so no migration is needed.

• Updated the two stale hardcoded-384 size assertions (tests/struct_layout.rs, src/state.rs) and the field-offset test's stale _reserved offset (320 → 336, accounting for the two new 8-byte fields at 320/328).
• Strengthened the existing collision test to also assert against every HWM accessor — the exact check that was missing.
• Added tests/poc_cooldown_timelock_hwm_overlap.rs, a dedicated regression covering all three corruption directions from the bug reports (cooldown-proposal-corrupts-HWM, HWM-refresh-corrupts-cooldown-proposal, HWM-activity-fabricates-a-phantom-cooldown-proposal).

Proof of Fix

• Verified the negative: temporarily swapped back the old _reserved-packed state.rs and confirmed all three new regression tests fail with exactly the corruption described in the issues (e.g. hwm_enabled flips after a cooldown proposal; a phantom cooldown_proposed_at_slot of 562949953497605 appears from HWM activity alone with no cooldown instruction ever issued). Restoring the fix makes all three pass.
• Full cargo test --lib (142 tests) and every non-SBF-dependent integration/proptest file (197 more tests, 339 total) pass with no regressions.
• The two v16/v17 LiteSVM e2e suites require a pre-built target/deploy/percolator_stake.so via cargo build-sbf, which isn't available in this sandbox (cargo-build-sbf not found) — this is a pre-existing environment limitation, not something this change causes; the same suites fail identically against unpatched main here for the same reason.

Related

Closes #250
Closes #258

Summary by CodeRabbit

• New Features

  • Stake pool state now supports the v3 cooldown timelock layout.
  • State version and size expectations were updated to reflect the new on-chain format.

• Bug Fixes

  • Fixed cooldown timing data from overlapping with high-water-mark values.
  • Writing cooldown settings no longer corrupts nearby stake pool fields.
  • High-water-mark updates no longer alter cooldown values unexpectedly.

[coderabbitai]

Warning

Review limit reached

@Morenikeoa, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 34 minutes and 45 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f527e779-2ddb-48e4-9d7e-72f8042d0254

📥 Commits

Reviewing files that changed from the base of the PR and between 9952896 and 0540112.

📒 Files selected for processing (1)

• src/state.rs

📝 Walkthrough

Walkthrough

StakePool cooldown timelock storage was moved to explicit v3 struct fields, the state version and size expectations were updated, and new regression tests cover cooldown/HWM state isolation.

Changes

StakePool cooldown layout fix

Layer / File(s)	Summary
Explicit cooldown fields src/state.rs	StakePool now stores pending_cooldown_slots and cooldown_proposed_at_slot as real fields, with accessors writing those fields directly and CURRENT_VERSION set to 3.
Layout size and offsets src/state.rs, tests/struct_layout.rs	The v3 struct size is asserted as 400, and offset checks are updated for the new field placement and _reserved boundary.
Cooldown/HWM overlap regressions src/processor.rs, tests/poc_cooldown_timelock_hwm_overlap.rs	The cooldown timelock comment is clarified, and regression tests verify cooldown and HWM writes do not overwrite each other’s state.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• dcccrypto/percolator-stake#246: Introduced the cooldown-increase flow and pending-cooldown accessors that this PR moves into explicit StakePool fields.

Poem

I hopped past bytes with careful feet,
And kept the HWM fields neat.
The cooldown sighed, the pool stayed bright,
No overlap gnawed in the night.
🐇🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately describes the main storage-layout fix.
Description check	✅ Passed	The description is detailed and covers problem, fix, proof, and related issues, though it doesn't follow the template headings exactly.
Linked Issues check	✅ Passed	The changes implement the requested non-overlapping storage layout, version bump, and regression coverage for both linked issues.
Out of Scope Changes check	✅ Passed	The modified files and tests all support the cooldown/HWM collision fix, with no clear unrelated code changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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 `@src/state.rs`:
- Around line 132-134: The ownership comment on the `_reserved` layout in
`state.rs` is misleading because it says bytes [10..26] are “genuinely free
again” even though the HWM accessors still own `_reserved[10..32]`. Update the
comment near the `_reserved` field to match the actual ownership contract used
by the HWM accessor methods and avoid implying that [10..26] is free for reuse.

🪄 Autofix (Beta)

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: e960983e-f4b1-4596-b40a-a4304ab762e7

📥 Commits

Reviewing files that changed from the base of the PR and between c3edb7f and 9952896.

📒 Files selected for processing (4)

• src/processor.rs
• src/state.rs
• tests/poc_cooldown_timelock_hwm_overlap.rs
• tests/struct_layout.rs