Skip to content

Loan manager liquidate#30

Open
EbukaMoses wants to merge 3 commits into
LabsCrypt:mainfrom
EbukaMoses:LoanManager_liquidate
Open

Loan manager liquidate#30
EbukaMoses wants to merge 3 commits into
LabsCrypt:mainfrom
EbukaMoses:LoanManager_liquidate

Conversation

@EbukaMoses

Copy link
Copy Markdown

close #15

Acceptance criteria

[x] Adjust total_outstanding on liquidation to remove the liquidated loan's outstanding principal, consistent with repay/check_default
[x] Verify available-liquidity accounting in approve_loan after a liquidation
[x] Add a test that liquidates a loan and asserts subsequent approve_loan sees the freed liquidity
[x] Confirm no path leaves TotalOutstanding permanently inflated

…est for loan approval after liquidation

- Moved total outstanding adjustment before loan status update in the liquidation process.
- Added a new test to verify that a loan can be approved after the liquidation of a previous loan, ensuring proper handling of pool liquidity.
…test coverage

- Adjusted the sequence of operations in the loan liquidation process for better clarity.
- Expanded test suite to include scenarios for loan approval following liquidation, ensuring robust liquidity management.

@ogazboiz ogazboiz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the liquidation change and the new test look reasonable, but the pr needs cleanup before it can be reviewed properly:

  1. it commits the entire target/ build directory (around 96 files: fingerprints, .cargo-lock, build scripts) plus a 2,665-line test snapshot json. please drop the build artifacts: add target/ to .gitignore and run git rm -r --cached target && git commit, and reconsider whether the test_snapshots json belongs in the pr. right now the real change (loan_manager/src/lib.rs +8/-5 and test.rs) is buried under build output.

  2. ci fails on cargo fmt: loan_manager/src/test.rs:1537 has assert_eq! calls that fmt wants collapsed onto one line. run cargo fmt from the contracts root and commit.

  3. the branch is named LoanManager_liquidate, please use something like feat/loan-manager-liquidate to stay consistent with the other branches:

    git branch -m LoanManager_liquidate feat/loan-manager-liquidate
    git push origin -u feat/loan-manager-liquidate
    

once target/ is out of the diff, fmt is clean and ci is green, i'll review the liquidate logic in detail.

if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0

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.

[Contracts] LoanManager liquidate does not adjust total_outstanding, permanently understating available liquidity

2 participants