fix: Conserve funds correctly when LoanPay fee payee is below reserve - #7843
fix: Conserve funds correctly when LoanPay fee payee is below reserve#7843Tapanito wants to merge 5 commits into
Conversation
LoanPay's fund-conservation check read XRP balances via accountHolds(SpendableHandling::FullBalance), which for XRP always defers to xrpLiquid (balance minus reserve, clamped at zero). When the broker fee landed on a payee sitting below its own reserve, the payee's clamped balance stayed zero and the fee vanished from the conservation sum, tripping the "funds are conserved" assert on an otherwise valid payment.
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| { | ||
| auto const sle = view.read(keylet::account(id)); | ||
| if (!sle) | ||
| return STAmount{}; |
There was a problem hiding this comment.
This can be easily tested. I think it's worth adding a test case to cover this line.
There was a problem hiding this comment.
I looked at whether this branch (missing account SLE) is actually reachable, and I don't think it is given the invariants around the three callers:
accountID_is the borrower making the payment — it signed the transaction, so it always exists.vaultPseudoAccountexists for as long as the vault/loan does.brokerPayeeis either the broker's pseudo-account, orbrokerOwner(sfOwneron the broker SLE) — and an account can't go throughAccountDeletewhile it still owns objects, so as long as the broker object exists, its owner account does too.
So hitting the !sle path here would require ledger state that violates one of those invariants (e.g. an owner account deleted while it still owns a broker) — not something reachable through normal transaction processing. I'd rather not fabricate a test that constructs invalid ledger state just to exercise this line; it's a defensive guard, not a real scenario.
If you see a path I'm missing where this is actually reachable, let me know and I'll add a test for it.
- Normalize includes in LoanPay.cpp to angle-bracket form and sort order - Use init-statement guard in Loan_test.cpp so bugprone-unchecked-optional-access can verify the check before dereferencing loanOpt
3 participants
LoanPay's fund-conservation check read XRP balances via accountHolds(SpendableHandling::FullBalance), which for XRP always defers to xrpLiquid (balance minus reserve, clamped at zero). When the broker fee landed on a payee sitting below its own reserve, the payee's clamped balance stayed zero and the fee vanished from the conservation sum, tripping the "funds are conserved" assert on an otherwise valid payment.
High Level Overview of Change
Context of Change
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)