refactor(PERC-8463): extract charge_fee_to_insurance helper

[dcccrypto]

Summary (SYNC-06 / PERC-8463)

Pure refactor: extract shared charge_fee_to_insurance helper from identical duplicated logic in settle_maintenance_fee and settle_maintenance_fee_best_effort_for_crank (both in percolator-core/src/percolator.rs).

Previously, both functions contained byte-for-byte identical blocks that:

1. Check if fee_credits is negative
2. Compute owed = -fee_credits, pay = min(owed, current_cap)
3. Drain capital via set_capital helper (preserves c_tot aggregate per spec §4.1)
4. Credit insurance_fund.balance and insurance_fund.fee_revenue
5. Credit back fee_credits by amount paid
6. Track paid_from_capital for return

These are now consolidated into fn charge_fee_to_insurance(&mut self, idx: usize) -> u128.

Semantics

IDENTICAL to prior behavior:

• Same set_capital/saturating_sub/saturating_add arithmetic
• Same insurance fund routing (balance + fee_revenue)
• Same return value (u128 paid into insurance)
• No on-chain layout change
• No bug fixes, no behavioral changes

Verification

• cargo check --all-features passes clean

Test plan

• CI runs cargo test and fee/settle suites green
• Reviewer confirms the two removed blocks were identical (diff should show -24/+22 lines in one file)

Summary by CodeRabbit

• Refactor
  • Improved internal code organization in the fee settlement system to reduce duplication and enhance maintainability. The observable behavior and functionality remain unchanged.

[coderabbitai]

📝 Walkthrough

Walkthrough

Extracted maintenance-fee settlement logic for handling negative fee_credits into a new shared helper method charge_fee_to_insurance(), removing code duplication in settle_maintenance_fee and settle_maintenance_fee_best_effort_for_crank while preserving observable state mutations and return values.

Changes

Cohort / File(s)

Summary

Refactored Maintenance Fee Settlement src/percolator.rs

Added internal helper method charge_fee_to_insurance(&mut self, idx: usize) -> u128 that encapsulates fee-to-insurance transfer logic (checking negative fee_credits, computing owed amount, calculating payable portion from capital, updating insurance fund balance/revenue, and crediting back fee_credits). Replaced inline duplicated conditional blocks in both settle_maintenance_fee and settle_maintenance_fee_best_effort_for_crank with single helper call.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• feat(PERC-8270): two-phase keeper — LiquidationPolicy, ADL fields, settle helpers, accrue_market_to, new keeper_crank signature #68: Directly related extraction and refactoring of maintenance-fee settlement helpers in src/percolator.rs.
• feat: PERC-8461 — enable recurring maintenance fees per spec §8.2 #79: Both PRs modify the maintenance-fee settlement flow involving fee_credits deduction and capital payment into the insurance fund.

Poem

🐰 A helper hops forth, no duplication in sight,
Two paths now as one—the refactor's delight!
Fee credits and capital dance in harmony,
Insurance fund cheers this sweet victory! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main refactoring change: extracting a new helper function. It is concise, specific, includes the task ID, and accurately reflects the primary modification.
Description check	✅ Passed	The description covers the Summary and Semantics sections thoroughly but lacks a 'How to test' section with reproduction steps and an incomplete Checklist with only two items partially addressed.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/PERC-8463-charge-fee-helper

---

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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/percolator.rs (1)

2593-2620: Use the new helper as the single debt-payment path.

Line 2601 introduces the shared helper, but pay_fee_debt_from_capital (around Line 2634 onward) still duplicates the same mutation sequence. Routing that function through this helper would reduce drift risk.

♻️ Suggested follow-up refactor

 fn pay_fee_debt_from_capital(&mut self, idx: u16) {
-    if self.accounts[idx as usize].fee_credits.is_negative()
-        && !self.accounts[idx as usize].capital.is_zero()
-    {
-        let owed = neg_i128_to_u128(self.accounts[idx as usize].fee_credits.get());
-        let current_cap = self.accounts[idx as usize].capital.get();
-        let pay = core::cmp::min(owed, current_cap);
-        if pay > 0 {
-            // Use set_capital helper to maintain c_tot aggregate (spec §4.1)
-            self.set_capital(idx as usize, current_cap.saturating_sub(pay));
-            self.insurance_fund.balance += pay;
-            self.insurance_fund.fee_revenue += pay;
-            self.accounts[idx as usize].fee_credits = self.accounts[idx as usize]
-                .fee_credits
-                .saturating_add(u128_to_i128_clamped(pay));
-        }
-    }
+    let _ = self.charge_fee_to_insurance(idx as usize);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/percolator.rs` around lines 2593 - 2620, The new helper
charge_fee_to_insurance encapsulates the debt-pay sequence; update
pay_fee_debt_from_capital to call charge_fee_to_insurance(idx) instead of
duplicating the capital/insurance_fund/account mutations so there’s a single
debt-payment path. Replace the duplicated logic that computes
owed/current_cap/pay, calls set_capital, updates insurance_fund.balance and
insurance_fund.fee_revenue, and increments accounts[idx].fee_credits with a call
to charge_fee_to_insurance(idx), returning or aggregating its paid value as the
original function did, ensuring any return value or side-effects (c_tot via
set_capital) remain unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/percolator.rs`:
- Around line 2593-2620: The new helper charge_fee_to_insurance encapsulates the
debt-pay sequence; update pay_fee_debt_from_capital to call
charge_fee_to_insurance(idx) instead of duplicating the
capital/insurance_fund/account mutations so there’s a single debt-payment path.
Replace the duplicated logic that computes owed/current_cap/pay, calls
set_capital, updates insurance_fund.balance and insurance_fund.fee_revenue, and
increments accounts[idx].fee_credits with a call to
charge_fee_to_insurance(idx), returning or aggregating its paid value as the
original function did, ensuring any return value or side-effects (c_tot via
set_capital) remain unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cbd914a1-9bc0-4bbf-b37a-a019af608a6c

📥 Commits

Reviewing files that changed from the base of the PR and between c864aaf and 2008ec4.

📒 Files selected for processing (1)

• src/percolator.rs