Frozen IOUs should still be transferable to/from the issuer #6113

vlntb · 2025-12-05T10:35:12Z

High Level Overview of Change

The freeze logic was incorrectly blocking IOU issuers from transacting with their own frozen currency. This violated the XRP Ledger protocol, which specifies that "Counterparties of the frozen issuer can still send and receive payments directly to and from the issuing address."

Context of Change

Three issues were fixed:

isFrozen() function was returning true for issuers checking their own currency during global freeze, causing the issuer to be treated as frozen. The early return prevented reaching the later issuer exemption check.
LoanBrokerCoverDeposit::preclaim() was using accountHolds() with FreezeHandling::fhZERO_IF_FROZEN for all accounts, including issuers. Since issuers don't have a "balance" of their own tokens (infinite issuance ability), the balance check would return 0 and fail with tecINSUFFICIENT_FUNDS.
VaultWithdraw::preclaim() had checkFrozen() calls that were always executed, even when the destination or submitter was the asset issuer. Now skips freeze checks when either the destination or submitter is the IOU issuer.

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (non-breaking change that only restructures code)
Performance (increase or change in throughput and/or latency)
Tests (you added tests for code that already exists, or your new feature included in this PR)
Documentation update
Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
Release

API Impact

Public API: New feature (new methods and/or new fields)
Public API: Breaking change (in general, breaking changes should only impact the next api_version)
libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
Peer protocol change (must be backward compatible or bump the peer protocol version)

codecov · 2025-12-05T11:16:19Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.1%. Comparing base (0aa4eed) to head (8523f42).

Additional details and impacted files

@@                      Coverage Diff                       @@
##           ximinez/lending-XLS-66-ongoing   #6113   +/-   ##
==============================================================
  Coverage                            79.1%   79.1%           
==============================================================
  Files                                 839     839           
  Lines                               71385   71399   +14     
  Branches                             8317    8315    -2     
==============================================================
+ Hits                                56481   56497   +16     
+ Misses                              14904   14902    -2

Files with missing lines	Coverage Δ
src/libxrpl/ledger/View.cpp	`94.5% <100.0%> (+<0.1%)`	⬆️
src/xrpld/app/tx/detail/LoanBrokerCoverDeposit.cpp	`95.0% <100.0%> (+0.2%)`	⬆️
src/xrpld/app/tx/detail/VaultWithdraw.cpp	`99.2% <100.0%> (+0.1%)`	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…x-tecFROZEN

shawnxie999 · 2025-12-08T14:30:09Z

src/libxrpl/ledger/View.cpp

        return false;
    auto sle = view.read(keylet::account(issuer));
-    if (sle && sle->isFlag(lsfGlobalFreeze))
+    if (sle && sle->isFlag(lsfGlobalFreeze) && issuer != account)


it looks like this helper function might have existed for some time (and being used in existing protocols). then we'd probably need to amendment gate this change with lending protocol.

Agree. Added GlobalFreezeIssuer amendment.

Agree. Added GlobalFreezeIssuer amendment.

You don't need a new amendment - you can reuse LendingProtocol and/or SingleAssetVault. This bug only affects transactions related to those amendments.

include/xrpl/protocol/detail/features.macro

Tapanito · 2025-12-10T17:05:50Z

src/xrpld/app/tx/detail/VaultWithdraw.cpp

+    // When withdrawing IOU to the issuer, ignore freeze since spec allows
+    // returning frozen IOU assets to their issuer. MPTs don't have this
+    // exemption - MPT locks function like "deep freeze" with no issuer
+    // exception.


@vlntb, do I understand correctly that, if the depositor is the issuer of an MPT, and the asset is locked, the issue will still be able to withdraw from the vault?

You are correct. I was confused by one of the comments in the code, but after reviewing the XLS-33 spec, I found that locked MPTs CAN be sent back to the issuer
(section 2.1.2.2.5: 'cannot be used in any transactions other than sending value back to the issuer').
The implementation and comments were incorrect. I've fixed this in the latest commit - locked MPTs now have the same issuer exemption as frozen IOUs.

shawnxie999 · 2025-12-10T17:40:36Z

src/libxrpl/ledger/View.cpp

+    {
+        if (!view.rules().enabled(fixGlobalFreezeIssuer) || issuer != account)
+            return true;
+    }


should it really be a separate amendment or should it just be fixed as part of lending? cc @ximinez

We're thinking on the same line of thought: #6113 (comment)

ximinez · 2025-12-10T22:27:54Z

src/test/app/Freeze_test.cpp

+        // Verify issuer can still receive payments (uses isFrozen internally)
+        env(pay(holder, issuer, USD(10)));
+        env.close();
+


Should also verify that the issuer can still send payments.

// Verify issuer can still send payments (uses isFrozen internally) env(pay(issuer, holder, USD(10))); env.close();

Should also verify that the issuer can still send payments.

// Verify issuer can still send payments (uses isFrozen internally) env(pay(issuer, holder, USD(10))); env.close();

I see a lot of changes where the loan broker deposits and withdraws, but I would like to see a simple payment from the issuer here in testIsFrozenDirectly

src/test/app/Freeze_test.cpp

src/test/app/LoanBroker_test.cpp

…x-tecFROZEN

ximinez

There are a bunch of unit test failures

ximinez · 2025-12-22T22:45:04Z

src/test/app/Freeze_test.cpp

+                coverDeposit(holder, brokerKeylet.key, USD(10)),
+                ter(tecFROZEN));
+
+            env_post(coverDeposit(issuer, brokerKeylet.key, USD(20)));


This doesn't look right. Only the broker owner (owner) should be able to deposit cover. Not because of the freeze, but because of the LP rules. The CI results confirm that - there are many unit test failures.

ximinez · 2025-12-22T22:54:54Z

src/test/app/Freeze_test.cpp

+        // Verify issuer can still receive payments (uses isFrozen internally)
+        env(pay(holder, issuer, USD(10)));
+        env.close();
+


Should also verify that the issuer can still send payments.

// Verify issuer can still send payments (uses isFrozen internally) env(pay(issuer, holder, USD(10))); env.close();

I see a lot of changes where the loan broker deposits and withdraws, but I would like to see a simple payment from the issuer here in testIsFrozenDirectly

…x-tecFROZEN

vlntb added 2 commits December 4, 2025 19:38

fix

5fe00e3

test case description update

933447c

vlntb requested a review from ximinez December 5, 2025 10:53

vlntb marked this pull request as ready for review December 5, 2025 10:53

vlntb requested a review from Tapanito December 5, 2025 10:53

clarifying comment

a913a79

vlntb changed the title ~~Frozen assets should still be withdrawable~~ Frozen IOUs should still be transferrable to the issuer Dec 5, 2025

vlntb changed the title ~~Frozen IOUs should still be transferrable to the issuer~~ Frozen IOUs should still be transferable to the issuer Dec 5, 2025

vlntb added 3 commits December 8, 2025 05:48

Merge branch 'ximinez/lending-XLS-66-ongoing' into vlntb/RIPD-4257-fi…

efae404

…x-tecFROZEN

Global freeze + additional unit-test

6d7e851

fix LoanBrokerCoverDeposit

4e4e626

vlntb changed the title ~~Frozen IOUs should still be transferable to the issuer~~ Frozen IOUs should still be transferable to/from the issuer Dec 8, 2025

vlntb requested a review from shawnxie999 December 8, 2025 14:05

shawnxie999 reviewed Dec 8, 2025

View reviewed changes

GlobalFreezeIssuer amendment

cb744e7

vlntb requested a review from a team as a code owner December 10, 2025 16:58

Tapanito reviewed Dec 10, 2025

View reviewed changes

include/xrpl/protocol/detail/features.macro Show resolved Hide resolved

Tapanito reviewed Dec 10, 2025

View reviewed changes

shawnxie999 reviewed Dec 10, 2025

View reviewed changes

ximinez requested changes Dec 10, 2025

View reviewed changes

vlntb added 9 commits December 15, 2025 10:28

Merge branch 'ximinez/lending-XLS-66-ongoing' into vlntb/RIPD-4257-fi…

1b180ef

…x-tecFROZEN

amendment fix

ee33367

sync with target

18508d5

minor test fixes

8523f42

Issuer exception applies for both IOU and freeze

8e5b89e

Merge branch 'ximinez/lending-XLS-66-ongoing' into vlntb/RIPD-4257-fi…

e0b52e0

…x-tecFROZEN

added test issuer can still send payments

b41332e

Rewritten test to use LP and SAV

345ec23

added test case for cover withdraw

d6e44e7

vlntb added 2 commits December 19, 2025 19:57

Merge branch 'ximinez/lending-XLS-66-ongoing' into vlntb/RIPD-4257-fi…

93e1503

…x-tecFROZEN

sync with target

8011c43

ximinez requested changes Dec 22, 2025

View reviewed changes

vlntb added 5 commits December 24, 2025 09:13

Merge branch 'ximinez/lending-XLS-66-ongoing' into vlntb/RIPD-4257-fi…

298ef4a

…x-tecFROZEN

fixing renaming

9449ce9

extra checks after sync with target

1e21180

added test

57607cc

added amemndment gating

d58d49a

Frozen IOUs should still be transferable to/from the issuer #6113

Are you sure you want to change the base?

Frozen IOUs should still be transferable to/from the issuer #6113

Conversation

vlntb commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

Type of Change

API Impact

Uh oh!

codecov bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shawnxie999 Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ximinez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vlntb commented Dec 5, 2025 •

edited

Loading

codecov bot commented Dec 5, 2025 •

edited

Loading

shawnxie999 Dec 8, 2025 •

edited

Loading