fix: calculate_top_up_lamports

[ananas-block]

Summary by CodeRabbit

• Bug Fixes
  • Top-up calculation now uses epoch-based funding logic, improving accuracy across epoch boundaries and ensuring non-compressible accounts trigger the correct top-up amount.
• Tests
  • Updated unit tests to reflect new epoch-funded behavior, boundary conditions, and the distinction between compressible and non-compressible paths.

[coderabbitai]

Walkthrough

The pull request replaces the rent top-up calculation from a balance-driven approach to an epoch-based model: calculate_top_up_lamports now queries get_last_funded_epoch() (propagating errors), derives funding need from epoch arithmetic, and RentConfigTrait was removed from the prelude import. Tests updated to reflect epoch-based expectations.

Changes

Cohort / File(s)	Summary
Epoch-based rent calculation refactor program-libs/compressible/src/compression_info.rs	Removed RentConfigTrait from prelude imports. Replaced available-balance and rent-per-epoch logic with epoch-based logic: call get_last_funded_epoch()?, compute current_epoch from current_slot (using SLOTS_PER_EPOCH), calculate epochs_funded_ahead = (last_funded_epoch_number + 1) - current_epoch, short-circuit to 0 when epochs_funded_ahead >= max_funded_epochs, otherwise return lamports_per_write. Updated inline comments and propagated Result via ?.
Test expectations and labels program-libs/compressible/tests/compression_info.rs	Added and reorganized test paths; introduced a NOT COMPRESSIBLE path that expects a top-up of lamports_per_write. Renamed and adjusted boundary test descriptions and last_claimed_slot values (e.g., using SLOTS_PER_EPOCH at boundaries). Updated assertions and messages to reflect epoch-based semantics and the epochs_funded_ahead concept.

Sequence Diagram

sequenceDiagram
participant Caller
participant calculate_top_up
participant get_last_funded_epoch
Caller->>calculate_top_up: calculate_top_up_lamports(current_slot,...)
activate calculate_top_up
calculate_top_up->>get_last_funded_epoch: get_last_funded_epoch()
activate get_last_funded_epoch
alt success
get_last_funded_epoch-->>calculate_top_up: last_funded_epoch_number
else error
get_last_funded_epoch--xcalculate_top_up: Err(...)
Note right of calculate_top_up: error propagated via `?`
end
deactivate get_last_funded_epoch
rect rgb(230,240,255)
Note over calculate_top_up: derive epochs and funding horizon
calculate_top_up->>calculate_top_up: current_epoch = current_slot / SLOTS_PER_EPOCH
calculate_top_up->>calculate_top_up: epochs_funded_ahead = (last_funded_epoch_number + 1) - current_epoch
end
alt epochs_funded_ahead >= max_funded_epochs
rect rgb(200,255,200)
calculate_top_up-->>Caller: 0 (no top-up)
end
else epochs_funded_ahead < max_funded_epochs
rect rgb(255,245,200)
calculate_top_up-->>Caller: lamports_per_write (top-up)
end
end
deactivate calculate_top_up

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Verify correct propagation of Result from get_last_funded_epoch() and appropriate error handling.
• Check epoch arithmetic and boundary handling (off-by-one around (last_funded_epoch + 1) and SLOTS_PER_EPOCH).
• Review updated tests for coverage of compressible vs non-compressible paths and accurate boundary values.

Poem

🌙 Epochs whisper when the coffers sleep,
Slots count onward, steady, deep;
A top-up rings when eras lag,
Small coins drop to fix the flag,
Code marches on — in epochs we keep. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: calculate_top_up_lamports' directly addresses the main change: fixing the calculate_top_up_lamports function with a new epoch-based funding calculation approach.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 70.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jorrit/fix-calculate_top_up_lamports

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 00aed01 and 9524c49.

📒 Files selected for processing (2)

• program-libs/compressible/src/compression_info.rs (2 hunks)
• program-libs/compressible/tests/compression_info.rs (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

program-libs/compressible/src/compression_info.rs (1)

program-libs/compressible/src/rent/account_rent.rs (2)

• get_last_funded_epoch (180-211)
• slot_to_epoch (174-176)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-test -- ...
• GitHub Check: programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test -- func...
• GitHub Check: programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-system-prog...
• GitHub Check: Test batched-merkle-tree-simulate
• GitHub Check: Test program-libs-fast

🔇 Additional comments (3)

program-libs/compressible/src/compression_info.rs (1)

13-13: Import change aligns with the epoch-based refactor.

The removal of RentConfigTrait from the prelude and retention of SLOTS_PER_EPOCH is consistent with the new epoch-based calculation approach.

program-libs/compressible/tests/compression_info.rs (2)

408-419: PATH 2 grouping resolves the previous review concern.

The test case "exact boundary - lagging claim requires top-up" is now correctly placed under PATH 2 (NOT COMPRESSIBLE, NEEDS TOP-UP), addressing the past review comment about misplaced grouping. The test logic aligns with the epoch-based implementation:

• At epoch 1 with 2 epochs funded from epoch 0, last_funded_epoch = 1
• epochs_funded_ahead = (1+1)-1 = 1 < max=2, so top-up is needed

Based on learnings

---

475-475: Description updates enhance test clarity.

The updated descriptions at lines 475 and 490 now explicitly reference epoch-based funding parameters (last_claimed, funded through epoch X, epochs_funded_ahead), making the test intent clearer and more maintainable.

Also applies to: 490-490

---

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

program-libs/compressible/src/compression_info.rs (1)

92-131: Epoch-based top-up logic looks correct; small clarity nits only

The new flow—is_compressible check first, then get_last_funded_epoch + slot_to_epoch to derive epochs_funded_ahead—is consistent and avoids the duplicated rent math from before:

• last_funded_epoch_number matches the helper semantics (“paid through” epoch).
• epochs_funded_ahead = (last_funded_epoch + 1) - current_epoch with saturating_add/saturating_sub gives the expected counts:
  • “exactly max_funded_epochs (2)” at epoch 0 → epochs_funded_ahead = 2, top‑up 0.
  • “exact boundary - lagging claim” at epoch 1 with last_claimed at epoch 0 → epochs_funded_ahead = 1, top‑up lamports_per_write.
  • “2 epochs at epoch 1 boundary” with last_claimed_slot = SLOTS_PER_EPOCH → epochs_funded_ahead = 2, top‑up 0.

The saturating math also makes the behind‑on‑rent case safe, since negative deltas clamp to 0 and such states should already fall into the compressible branch via is_compressible.

Two optional cleanups you might consider (not blockers):

• Inline comment above epochs_funded_ahead could explicitly restate the formula as “epochs funded from current_epoch (inclusive) through last_funded_epoch (inclusive)” to make the off‑by‑one intent obvious.
• If you ever need to reuse epochs_funded or available_balance, a future refactor could let get_last_funded_epoch reuse the existing AccountRentState instead of recomputing internally, but that’s purely an optimization / API-tidy follow‑up.

Functionally, this change is solid and well-covered by the updated tests.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ce2203 and 00aed01.

📒 Files selected for processing (2)

• program-libs/compressible/src/compression_info.rs (2 hunks)
• program-libs/compressible/tests/compression_info.rs (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

program-libs/compressible/src/compression_info.rs (1)

program-libs/compressible/src/rent/account_rent.rs (2)

• get_last_funded_epoch (180-211)
• slot_to_epoch (174-176)

🔇 Additional comments (3)

program-libs/compressible/src/compression_info.rs (1)

11-14: Import of SLOTS_PER_EPOCH is appropriate and used

SLOTS_PER_EPOCH is needed for claim()’s completed_epochs * SLOTS_PER_EPOCH update later in the file and for coherent epoch math across rent utilities, so having it in the rent prelude here is correct.

program-libs/compressible/tests/compression_info.rs (2)

468-476: Updated max-funded description correctly captures epoch math

For the “exactly max_funded_epochs (2)” case, the new description:

Epoch 0: last_claimed=epoch 0, funded through epoch 1, epochs_funded_ahead=2 >= max=2

matches the implementation:

• last_claimed_slot = 0 → last_claimed_epoch = 0.
• Two epochs of rent funded → last_funded_epoch = 1.
• With current_epoch = 0, epochs_funded_ahead = (1 + 1) - 0 = 2, so the branch to return 0 is correct.

Comment and expectation are consistent with calculate_top_up_lamports.

---

487-494: Epoch-1 boundary test now cleanly distinguishes “up-to-date claim” from “lagging claim”

Changing last_claimed_slot to SLOTS_PER_EPOCH in “2 epochs at epoch 1 boundary” nicely complements the new lagging-claim test:

• Here last_claimed_epoch = 1, two epochs funded → last_funded_epoch = 2.
• At current_epoch = 1, epochs_funded_ahead = (2 + 1) - 1 = 2, which meets max_funded_epochs = 2, so expected_top_up: 0 is correct.

This gives you both scenarios:

• Epoch boundary with stale last_claimed_slot → needs top-up.
• Epoch boundary with up-to-date last_claimed_slot → considered well-funded.

Test coverage around the boundary behavior looks good.