fix: Check if the MPT first loss cover can be sent to the broker before deleting the broker - #7125
Conversation
There was a problem hiding this comment.
Went over the changes
One likely copy-paste bug flagged inline: the deep-freeze check targets brokerOwner (already checked earlier) instead of brokerPseudo, making the new check a no-op.
Review by ReviewBot 🤖
Review by Claude Opus 4.6 · Prompt: V15
|
Making it a draft for a couple of hours to investigate a potential bug. |
…brokerdelete-preclaim
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7125 +/- ##
=========================================
- Coverage 82.1% 82.1% -0.0%
=========================================
Files 1010 1010
Lines 76151 76170 +19
Branches 7424 7410 -14
=========================================
+ Hits 62497 62512 +15
- Misses 13654 13658 +4
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR tightens LoanBrokerDelete validation to prevent bypassing MPToken lock/freeze restrictions when returning broker cover during deletion. It ensures the broker pseudo-account is eligible to send the cover back to the broker owner before deletion proceeds (amendment-gated), and adds regression tests covering both MPToken lock and IOU freeze scenarios.
Changes:
- Add an amendment-gated
preclaim()check to validate the broker pseudo-account is not frozen/locked for the vault asset when cover would be transferred. - Add a regression test ensuring locked MPToken cover cannot be returned to the owner via broker deletion when the fix is enabled.
- Add a regression test ensuring a frozen IOU trustline on the broker pseudo-account blocks broker deletion early (with the fix) vs invariant failure (without).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/test/app/LoanBroker_test.cpp | Adds regression tests for LoanBrokerDelete behavior when the broker pseudo-account’s holding is locked (MPT) or frozen (IOU), with/without fixCleanup3_2_0. |
| src/libxrpl/tx/transactors/lending/LoanBrokerDelete.cpp | Adds fixCleanup3_2_0-gated preclaim() call to checkFrozen() on the broker pseudo-account when cover would be returned. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…brokerdelete-preclaim
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
…re deleting the broker (XRPLF#7125) Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
High Level Overview of Change
This PR adds some checks in LoanBrokerDelete::preclaim to ensure we can send the MPT to the broker account when we delete it.
Context of Change
It's currently possible to bypass MPT locks. preclaim() only validates the destination owner's freeze state, never the source pseudo-account. doApply() then transfers sfCoverAvailable via accountSend(), and the low-level MPT send path does not revalidate locks.
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)