fix(CFv1): skip revocation of already repaid loans on credit lines

[EvgeniiZaitsevCW]

Main Changes

Fixed the _revokeLoan() internal function to skip the onAfterLoanRevocation() credit line hook for already repaid loans (where trackedBalance == 0). Previously, revoking an installment loan that contained fully repaid sub-loans would still call the credit line hook for those sub-loans, incorrectly decrementing the borrower's activeLoanCount and totalActiveLoanAmount in the credit line state. Now the hook is only called for sub-loans that have an outstanding balance at the time of revocation.

Migration

1. No special actions required, just upgrade the deployed CapybaraFinance smart-contracts.
2. To be sure there are no broken borrower states on existing credit lines, you can check all the cases of loan revocation through the block explorer database. Only installment loan revocation cases should be checked through event InstallmentLoanRevoked because fully repaid ordinary loans cannot be revoked. The installment loans with a single installment can be skipped as well.

[zeropath-ai]

✅ No security or compliance issues detected. Reviewed everything up to 81fcb18.

Security Overview

• 🔎 Scanned files: 8 changed file(s)
• 🔗 Scan Link: https://zeropath.com/app/repositories/eadeacfa-aab0-4d18-9661-fd3c609f4fda?scanId=2ceb0780-7295-4bdd-b7f5-e751f62d6901&codeScanTypes=PrScan&tab=issues

Detected Code Changes

Change Type	Relevant files
Configuration changes	► contracts/capybara-finance/.openzeppelin/unknown-2008.json     Add LendingMarketStorage contract details     Add CreditLineStorage contract details     Add LiquidityPoolStorage contract details ► contracts/capybara-finance/.openzeppelin/unknown-2009.json     Update LiquidityPoolStorage contract details     Add LendingMarketStorage contract details

[EvgeniiZaitsevCW]

@codex review please

[EvgeniiZaitsevCW]

@claude review please

[claude]

Claude finished @EvgeniiZaitsevCW's task in 1m 45s —— View job

---

Code Review

• Gather context and understand the request
• Read changed files
• Analyze the fix logic
• Review test coverage
• Post final review

---

Summary

The fix is correct and well-targeted. The bug was that onAfterLoanRevocation() was called for every sub-loan in an installment loan, even already-repaid ones, causing the credit line to incorrectly decrement activeLoanCount and totalActiveLoanAmount for balances that were no longer active.

---

Fix Analysis — LendingMarket.sol

The change (lines 777–784):

bool isLoanRepaid = _isRepaid(loan);  // captured BEFORE mutation

loan.trackedBalance = 0;
loan.trackedTimestamp = _blockTimestamp().toUint32();

if (!isLoanRepaid) {
    ICreditLine(creditLine).onAfterLoanRevocation(loanId);
}

Critical correctness detail: isLoanRepaid is captured before loan.trackedBalance = 0 is written. This is essential — after that line, _isRepaid() would always return true for every loan. The ordering is correct.

Interaction with revokeLoan() (ordinary loans, line 212):
revokeLoan() calls _checkIfLoanOngoing(loan) before _revokeLoan(), which already guarantees trackedBalance != 0. So the new guard is effectively always false (i.e., hook always called) for ordinary loans — semantically a no-op, but safe and consistent.

Interaction with revokeInstallmentLoan() (lines 221–248):
This is the only affected code path in practice. The guard ongoingSubLoanCount == 0 → revert LendingMarket_LoanAlreadyRepaid() (line 240) ensures at least one sub-loan has a balance, so the revocation requires at least one call to onAfterLoanRevocation. The fix correctly restricts hook invocations to only those sub-loans.

---

Test Coverage — LendingMarket.base.test.ts

The "All installments are repaid except the last one" test group (lines 4864–4945) is the primary coverage for the mixed repaid/unrepaid scenario. The updated revokeAndCheck helper correctly gates the positive assertion:

if (!loanRepaidStatesBeforeRevocation[i]) {
  await expect(tx).to.emit(creditLine, EVENT_NAME_ON_AFTER_LOAN_REVOCATION_CALLED).withArgs(loanId);
}

Minor observation: For the mixed scenario (some sub-loans repaid, some not), there is no explicit per-sub-loan negative assertion that the hook is not called for the repaid ones. Only the all-repaid case has a global not.to.emit check — which is a dead code path in practice since the contract would revert if all sub-loans were already repaid. This is not a blocking issue because:

1. The complex test (LendingMarket.complex.test.ts) validates borrowerStateAfter.activeLoanCount == 0 and borrowerStateAfter.totalActiveLoanAmount == 0, which would fail if the hook fired incorrectly for repaid sub-loans.
2. The loanId-specific withArgs assertion gives indirect confidence.

However, consider adding an explicit assertion in the mixed case for completeness:

if (loanRepaidStatesBeforeRevocation[i]) {
  await expect(tx).not.to.emit(creditLine, EVENT_NAME_ON_AFTER_LOAN_REVOCATION_CALLED).withArgs(loanId);
}

Test Coverage — LendingMarket.complex.test.ts

The addition of borrowerStateAfter.activeLoanCount == 0 and borrowerStateAfter.totalActiveLoanAmount == 0 checks (lines 544–545) is a good end-to-end validation that the credit line state is consistent after revocation. This directly exercises the actual impact of the original bug.

---

CHANGELOG

The version bump from v1.23.1 to v1.24.0 and the changelog entry are clear and well-written. The migration note appropriately flags that operators may want to audit historical revocation events via InstallmentLoanRevoked.

---

Verdict

Approved with a minor suggestion. The fix is minimal, correctly placed, and well-tested. The only optional improvement is adding explicit negative hook assertions for individual repaid sub-loans in the mixed-state test cases.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.89%. Comparing base (3531920) to head (81fcb18).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #62   +/-   ##
=======================================
  Coverage   99.89%   99.89%           
=======================================
  Files         162      162           
  Lines        6969     6971    +2     
  Branches     1164     1222   +58     
=======================================
+ Hits         6962     6964    +2     
  Misses          7        7           

Components	Coverage Δ
Base	100.00% <ø> (ø)
Asset Transit Desk	100.00% <ø> (ø)
Balance Freezer	100.00% <ø> (ø)
Balance Tracker	100.00% <ø> (ø)
Blueprint	100.00% <ø> (ø)
Capybara Finance	100.00% <100.00%> (ø)
Capybara Finance V2	100.00% <ø> (ø)
Card Payment Processor	100.00% <ø> (ø)
Card Payment Processor V2	100.00% <ø> (ø)
Cashier	99.74% <ø> (ø)
Credit Agent	100.00% <ø> (ø)
Multisig	100.00% <ø> (ø)
Net Yield Distributor	100.00% <ø> (ø)
Periphery	100.00% <ø> (ø)
Shared Wallet Controller	98.20% <ø> (ø)
BRLC Token	100.00% <ø> (ø)
Treasury	100.00% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".