Merge branch 'ximinez/lending-XLS-66' into ximinez/lending-enabled

.github/workflows/on-pr.yml

@@ -122,7 +122,7 @@ jobs:
     needs:
       - should-run
       - build-test
-    if: ${{ needs.should-run.outputs.go == 'true' && contains(fromJSON('["release", "master"]'), github.ref_name) }}
+    if: ${{ needs.should-run.outputs.go == 'true' && (startsWith(github.base_ref, 'release') || github.base_ref == 'master') }}
     uses: ./.github/workflows/reusable-notify-clio.yml
     secrets:
       clio_notify_token: ${{ secrets.CLIO_NOTIFY_TOKEN }}

include/xrpl/protocol/detail/features.macro

@@ -67,8 +67,6 @@ XRPL_FEATURE(Clawback,                   Supported::yes, VoteBehavior::DefaultNo
 XRPL_FIX    (UniversalNumber,            Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(XRPFees,                    Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes)
-XRPL_FEATURE(FlowSortStrands,            Supported::yes, VoteBehavior::DefaultYes)
-XRPL_FEATURE(Flow,                       Supported::yes, VoteBehavior::DefaultYes)
 
 // The following amendments are obsolete, but must remain supported
 // because they could potentially get enabled.
@@ -127,7 +125,9 @@ XRPL_RETIRE_FEATURE(Escrow)
 XRPL_RETIRE_FEATURE(EnforceInvariants)
 XRPL_RETIRE_FEATURE(ExpandedSignerList)
 XRPL_RETIRE_FEATURE(FeeEscalation)
+XRPL_RETIRE_FEATURE(Flow)
 XRPL_RETIRE_FEATURE(FlowCross)
+XRPL_RETIRE_FEATURE(FlowSortStrands)
 XRPL_RETIRE_FEATURE(HardenedValidations)
 XRPL_RETIRE_FEATURE(ImmediateOfferKilled)
 XRPL_RETIRE_FEATURE(MultiSign)

src/test/app/CrossingLimits_test.cpp

@@ -267,9 +267,8 @@ public:
         // strand dry until the liquidity is actually used)
 
         // The implementation allows any single step to consume at most 1000
-        // offers. With the `FlowSortStrands` feature enabled, if the total
+        // offers.If the total number of offers consumed by all the steps
-        // number of offers consumed by all the steps combined exceeds 1500, the
+        // combined exceeds 1500, the payment stops.
-        // payment stops.
         {
             Env env(*this, features);
 
@@ -457,16 +456,12 @@ public:
         // below the limit. However, if all the offers are consumed it would
         // create a tecOVERSIZE error.
 
-        // The featureFlowSortStrands introduces a way of tracking the total
-        // number of consumed offers; with this feature the transaction no
-        // longer fails with a tecOVERSIZE error.
         // The implementation allows any single step to consume at most 1000
-        // offers. With the `FlowSortStrands` feature enabled, if the total
+        // offers. If the total number of offers consumed by all the steps
-        // number of offers consumed by all the steps combined exceeds 1500, the
+        // combined exceeds 1500, the payment stops. Since the first set of
-        // payment stops. Since the first set of offers consumes 998 offers, the
+        // offers consumes 998 offers, the second set will consume 998, which is
-        // second set will consume 998, which is not over the limit and the
+        // not over the limit and the payment stops. So 2*998, or 1996 is the
-        // payment stops. So 2*998, or 1996 is the expected value when
+        // expected value.
-        // `FlowSortStrands` is enabled.
         n_offers(env, 998, alice, XRP(1.00), USD(1));
         n_offers(env, 998, alice, XRP(0.99), USD(1));
         n_offers(env, 998, alice, XRP(0.98), USD(1));
@@ -474,24 +469,10 @@ public:
         n_offers(env, 998, alice, XRP(0.96), USD(1));
         n_offers(env, 998, alice, XRP(0.95), USD(1));
 
-        bool const withSortStrands = features[featureFlowSortStrands];
+        env(offer(bob, USD(8000), XRP(8000)), ter(tesSUCCESS));
-
-        auto const expectedTER = [&]() -> TER {
-            if (!withSortStrands)
-                return TER{tecOVERSIZE};
-            return tesSUCCESS;
-        }();
-
-        env(offer(bob, USD(8000), XRP(8000)), ter(expectedTER));
         env.close();
 
-        auto const expectedUSD = [&] {
+        env.require(balance(bob, USD(1996)));
-            if (!withSortStrands)
-                return USD(0);
-            return USD(1996);
-        }();
-
-        env.require(balance(bob, expectedUSD));
     }
 
     void
@@ -507,9 +488,7 @@ public:
         using namespace jtx;
         auto const sa = testable_amendments();
         testAll(sa);
-        testAll(sa - featureFlowSortStrands);
         testAll(sa - featurePermissionedDEX);
-        testAll(sa - featureFlowSortStrands - featurePermissionedDEX);
     }
 };
 

src/test/app/LoanBroker_test.cpp

@@ -877,16 +877,16 @@ class LoanBroker_test : public beast::unit_test::suite
         PrettyAsset const asset = [&]() {
             if (getAsset)
                 return getAsset(env, issuer, alice);
-            env(trust(alice, issuer["IOU"](1'000'000)));
+            env(trust(alice, issuer["IOU"](1'000'000)), THISLINE);
             env.close();
             return PrettyAsset(issuer["IOU"]);
         }();
 
-        env(pay(issuer, alice, asset(100'000)));
+        env(pay(issuer, alice, asset(100'000)), THISLINE);
         env.close();
 
         auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset});
-        env(tx);
+        env(tx, THISLINE);
         env.close();
         auto const le = env.le(vaultKeylet);
         VaultInfo vaultInfo = [&]() {
@@ -898,12 +898,15 @@ class LoanBroker_test : public beast::unit_test::suite
             return;
 
         env(vault.deposit(
-            {.depositor = alice, .id = vaultKeylet.key, .amount = asset(50)}));
+                {.depositor = alice,
+                 .id = vaultKeylet.key,
+                 .amount = asset(50)}),
+            THISLINE);
         env.close();
 
         auto const brokerKeylet =
             keylet::loanbroker(alice.id(), env.seq(alice));
-        env(set(alice, vaultInfo.vaultID));
+        env(set(alice, vaultInfo.vaultID), THISLINE);
         env.close();
 
         auto broker = env.le(brokerKeylet);
@@ -914,23 +917,23 @@ class LoanBroker_test : public beast::unit_test::suite
             auto jv = getTxJv();
             // empty broker ID
             jv[sfLoanBrokerID] = "";
-            env(jv, ter(temINVALID));
+            env(jv, ter(temINVALID), THISLINE);
             // zero broker ID
             jv[sfLoanBrokerID] = to_string(uint256{});
             // needs a flag to distinguish the parsed STTx from the prior
             // test
-            env(jv, txflags(tfFullyCanonicalSig), ter(temINVALID));
+            env(jv, txflags(tfFullyCanonicalSig), ter(temINVALID), THISLINE);
         };
         auto testZeroVaultID = [&](auto&& getTxJv) {
             auto jv = getTxJv();
             // empty broker ID
             jv[sfVaultID] = "";
-            env(jv, ter(temINVALID));
+            env(jv, ter(temINVALID), THISLINE);
             // zero broker ID
             jv[sfVaultID] = to_string(uint256{});
             // needs a flag to distinguish the parsed STTx from the prior
             // test
-            env(jv, txflags(tfFullyCanonicalSig), ter(temINVALID));
+            env(jv, txflags(tfFullyCanonicalSig), ter(temINVALID), THISLINE);
         };
 
         if (brokerTest == CoverDeposit)
@@ -942,23 +945,26 @@ class LoanBroker_test : public beast::unit_test::suite
 
             // preclaim: tecWRONG_ASSET
             env(coverDeposit(alice, brokerKeylet.key, issuer["BAD"](10)),
-                ter(tecWRONG_ASSET));
+                ter(tecWRONG_ASSET),
+                THISLINE);
 
             // preclaim: tecINSUFFICIENT_FUNDS
-            env(pay(alice, issuer, asset(100'000 - 50)));
+            env(pay(alice, issuer, asset(100'000 - 50)), THISLINE);
             env.close();
             env(coverDeposit(alice, brokerKeylet.key, vaultInfo.asset(10)),
                 ter(tecINSUFFICIENT_FUNDS));
 
             // preclaim: tecFROZEN
-            env(fset(issuer, asfGlobalFreeze));
+            env(fset(issuer, asfGlobalFreeze), THISLINE);
             env.close();
             env(coverDeposit(alice, brokerKeylet.key, vaultInfo.asset(10)),
-                ter(tecFROZEN));
+                ter(tecFROZEN),
+                THISLINE);
         }
         else
             // Fund the cover deposit
-            env(coverDeposit(alice, brokerKeylet.key, vaultInfo.asset(10)));
+            env(coverDeposit(alice, brokerKeylet.key, vaultInfo.asset(10)),
+                THISLINE);
         env.close();
 
         if (brokerTest == CoverWithdraw)
@@ -970,47 +976,54 @@ class LoanBroker_test : public beast::unit_test::suite
 
             // preclaim: tecWRONG_ASSSET
             env(coverWithdraw(alice, brokerKeylet.key, issuer["BAD"](10)),
-                ter(tecWRONG_ASSET));
+                ter(tecWRONG_ASSET),
+                THISLINE);
 
             // preclaim: tecNO_DST
             Account const bogus{"bogus"};
             env(coverWithdraw(alice, brokerKeylet.key, asset(10)),
                 destination(bogus),
-                ter(tecNO_DST));
+                ter(tecNO_DST),
+                THISLINE);
 
             // preclaim: tecDST_TAG_NEEDED
             Account const dest{"dest"};
             env.fund(XRP(1'000), dest);
-            env(fset(dest, asfRequireDest));
+            env(fset(dest, asfRequireDest), THISLINE);
             env.close();
             env(coverWithdraw(alice, brokerKeylet.key, asset(10)),
                 destination(dest),
-                ter(tecDST_TAG_NEEDED));
+                ter(tecDST_TAG_NEEDED),
+                THISLINE);
 
             // preclaim: tecNO_PERMISSION
-            env(fclear(dest, asfRequireDest));
+            env(fclear(dest, asfRequireDest), THISLINE);
-            env(fset(dest, asfDepositAuth));
+            env(fset(dest, asfDepositAuth), THISLINE);
             env.close();
             env(coverWithdraw(alice, brokerKeylet.key, asset(10)),
                 destination(dest),
-                ter(tecNO_PERMISSION));
+                ter(tecNO_PERMISSION),
+                THISLINE);
 
             // preclaim: tecFROZEN
-            env(trust(dest, asset(1'000)));
+            env(trust(dest, asset(1'000)), THISLINE);
-            env(fclear(dest, asfDepositAuth));
+            env(fclear(dest, asfDepositAuth), THISLINE);
-            env(fset(issuer, asfGlobalFreeze));
+            env(fset(issuer, asfGlobalFreeze), THISLINE);
             env.close();
             env(coverWithdraw(alice, brokerKeylet.key, asset(10)),
                 destination(dest),
-                ter(tecFROZEN));
+                ter(tecFROZEN),
+                THISLINE);
 
             // preclaim:: tecFROZEN (deep frozen)
-            env(fclear(issuer, asfGlobalFreeze));
+            env(fclear(issuer, asfGlobalFreeze), THISLINE);
             env(trust(
-                issuer, asset(1'000), dest, tfSetFreeze | tfSetDeepFreeze));
+                    issuer, asset(1'000), dest, tfSetFreeze | tfSetDeepFreeze),
+                THISLINE);
             env(coverWithdraw(alice, brokerKeylet.key, asset(10)),
                 destination(dest),
-                ter(tecFROZEN));
+                ter(tecFROZEN),
+                THISLINE);
         }
 
         if (brokerTest == CoverClawback)
@@ -1029,23 +1042,27 @@ class LoanBroker_test : public beast::unit_test::suite
                 env(coverClawback(issuer),
                     loanBrokerID(brokerKeylet.key),
                     amount(vaultInfo.asset(2)),
-                    ter(tecNO_PERMISSION));
+                    ter(tecNO_PERMISSION),
+                    THISLINE);
 
                 // preclaim: NoFreeze is set
-                env(fset(issuer, asfAllowTrustLineClawback | asfNoFreeze));
+                env(fset(issuer, asfAllowTrustLineClawback | asfNoFreeze),
+                    THISLINE);
                 env.close();
                 env(coverClawback(issuer),
                     loanBrokerID(brokerKeylet.key),
                     amount(vaultInfo.asset(2)),
-                    ter(tecNO_PERMISSION));
+                    ter(tecNO_PERMISSION),
+                    THISLINE);
             }
             else
             {
-                // preclaim: MPTCanClawback is not set or MPTCAnLock is not set
+                // preclaim: MPTCanClawback is not set or MPTCanLock is not set
                 env(coverClawback(issuer),
                     loanBrokerID(brokerKeylet.key),
                     amount(vaultInfo.asset(2)),
-                    ter(tecNO_PERMISSION));
+                    ter(tecNO_PERMISSION),
+                    THISLINE);
             }
             env.close();
         }
@@ -1056,30 +1073,36 @@ class LoanBroker_test : public beast::unit_test::suite
             env.fund(XRP(1'000), borrower);
             env(loan::set(borrower, brokerKeylet.key, asset(50).value()),
                 sig(sfCounterpartySignature, alice),
-                fee(env.current()->fees().base * 2));
+                fee(env.current()->fees().base * 2),
+                THISLINE);
 
             // preflight: temINVALID (empty/zero broker id)
             testZeroBrokerID([&]() { return del(alice, brokerKeylet.key); });
 
             // preclaim: tecHAS_OBLIGATIONS
-            env(del(alice, brokerKeylet.key), ter(tecHAS_OBLIGATIONS));
+            env(del(alice, brokerKeylet.key),
+                ter(tecHAS_OBLIGATIONS),
+                THISLINE);
 
             // Repay and delete the loan
             auto const loanKeylet = keylet::loan(brokerKeylet.key, 1);
-            env(loan::pay(borrower, loanKeylet.key, asset(50).value()));
+            env(loan::pay(borrower, loanKeylet.key, asset(50).value()),
-            env(loan::del(alice, loanKeylet.key));
+                THISLINE);
+            env(loan::del(alice, loanKeylet.key), THISLINE);
 
-            env(trust(issuer, asset(0), alice, tfSetFreeze | tfSetDeepFreeze));
+            env(trust(issuer, asset(0), alice, tfSetFreeze | tfSetDeepFreeze),
+                THISLINE);
             // preclaim: tecFROZEN (deep frozen)
-            env(del(alice, brokerKeylet.key), ter(tecFROZEN));
+            env(del(alice, brokerKeylet.key), ter(tecFROZEN), THISLINE);
             env(trust(
-                issuer, asset(0), alice, tfClearFreeze | tfClearDeepFreeze));
+                    issuer, asset(0), alice, tfClearFreeze | tfClearDeepFreeze),
+                THISLINE);
 
             // successful delete the loan broker object
-            env(del(alice, brokerKeylet.key), ter(tesSUCCESS));
+            env(del(alice, brokerKeylet.key), ter(tesSUCCESS), THISLINE);
         }
         else
-            env(del(alice, brokerKeylet.key));
+            env(del(alice, brokerKeylet.key), THISLINE);
 
         if (brokerTest == Set)
         {
@@ -1098,21 +1121,23 @@ class LoanBroker_test : public beast::unit_test::suite
 
             if (asset.holds<Issue>())
             {
-                env(fclear(issuer, asfDefaultRipple));
+                env(fclear(issuer, asfDefaultRipple), THISLINE);
                 env.close();
                 // preclaim: DefaultRipple is not set
-                env(set(alice, vaultInfo.vaultID), ter(terNO_RIPPLE));
+                env(set(alice, vaultInfo.vaultID), ter(terNO_RIPPLE), THISLINE);
 
-                env(fset(issuer, asfDefaultRipple));
+                env(fset(issuer, asfDefaultRipple), THISLINE);
                 env.close();
             }
 
             auto const amt = env.balance(alice) -
                 env.current()->fees().accountReserve(env.ownerCount(alice));
-            env(pay(alice, issuer, amt));
+            env(pay(alice, issuer, amt), THISLINE);
 
             // preclaim:: tecINSUFFICIENT_RESERVE
-            env(set(alice, vaultInfo.vaultID), ter(tecINSUFFICIENT_RESERVE));
+            env(set(alice, vaultInfo.vaultID),
+                ter(tecINSUFFICIENT_RESERVE),
+                THISLINE);
         }
     }
 
@@ -1135,7 +1160,7 @@ class LoanBroker_test : public beast::unit_test::suite
             auto jtx = env.jt(coverClawback(alice), amount(USD(100)));
 
             // holder == account
-            env(jtx, ter(temINVALID));
+            env(jtx, ter(temINVALID), THISLINE);
 
             // holder == beast::zero
             STAmount bad(Issue{USD.currency, beast::zero}, 100);
@@ -1164,17 +1189,6 @@ class LoanBroker_test : public beast::unit_test::suite
                 return mpt;
             },
             CoverClawback);
-        // MPTCanLock is not set
-        testLoanBroker(
-            [&](Env& env, Account const& issuer, Account const& alice) -> MPT {
-                MPTTester mpt(
-                    {.env = env,
-                     .issuer = issuer,
-                     .holders = {alice},
-                     .flags = MPTDEXFlags | tfMPTCanClawback});
-                return mpt;
-            },
-            CoverClawback);
     }
 
     void

src/test/app/TxQ_test.cpp

@@ -1054,7 +1054,7 @@ public:
         // Charlie - queue a transaction, with a higher fee
         // than default
         env(noop(charlie), fee(15), queued);
-        checkMetrics(*this, env, 6, initQueueMax, 4, 3);
+        checkMetrics(*this, env, 6, initQueueMax, 4, 3, 257);
 
         BEAST_EXPECT(env.seq(alice) == aliceSeq);
         BEAST_EXPECT(env.seq(bob) == bobSeq);
@@ -4330,7 +4330,7 @@ public:
         Account const ellie("ellie");
         Account const fiona("fiona");
 
-        constexpr int ledgersInQueue = 20;
+        constexpr int ledgersInQueue = 30;
         auto cfg = makeConfig(
             {{"minimum_txn_in_ledger_standalone", "1"},
              {"ledgers_in_queue", std::to_string(ledgersInQueue)},

src/test/jtx/Env_test.cpp

@@ -798,16 +798,18 @@ public:
 
         {
             // a Env FeatureBitset has *only* those features
-            Env env{*this, FeatureBitset{featureDynamicMPT | featureFlow}};
+            Env env{
+                *this, FeatureBitset{featureDynamicMPT | featureTokenEscrow}};
             BEAST_EXPECT(env.app().config().features.size() == 2);
             foreachFeature(supported, [&](uint256 const& f) {
-                bool const has = (f == featureDynamicMPT || f == featureFlow);
+                bool const has =
+                    (f == featureDynamicMPT || f == featureTokenEscrow);
                 this->BEAST_EXPECT(has == hasFeature(env, f));
             });
         }
 
         auto const missingSomeFeatures =
-            testable_amendments() - featureDynamicMPT - featureFlow;
+            testable_amendments() - featureDynamicMPT - featureTokenEscrow;
         BEAST_EXPECT(missingSomeFeatures.count() == (supported.count() - 2));
         {
             // a Env supported_features_except is missing *only* those features
@@ -815,7 +817,8 @@ public:
             BEAST_EXPECT(
                 env.app().config().features.size() == (supported.count() - 2));
             foreachFeature(supported, [&](uint256 const& f) {
-                bool hasnot = (f == featureDynamicMPT || f == featureFlow);
+                bool hasnot =
+                    (f == featureDynamicMPT || f == featureTokenEscrow);
                 this->BEAST_EXPECT(hasnot != hasFeature(env, f));
             });
         }
@@ -828,7 +831,9 @@ public:
             Env env{
                 *this,
                 FeatureBitset{
-                    featureDynamicMPT, featureFlow, *neverSupportedFeat}};
+                    featureDynamicMPT,
+                    featureTokenEscrow,
+                    *neverSupportedFeat}};
 
             // this app will have just 2 supported amendments and
             // one additional never supported feature flag
@@ -836,7 +841,7 @@ public:
             BEAST_EXPECT(hasFeature(env, *neverSupportedFeat));
 
             foreachFeature(supported, [&](uint256 const& f) {
-                bool has = (f == featureDynamicMPT || f == featureFlow);
+                bool has = (f == featureDynamicMPT || f == featureTokenEscrow);
                 this->BEAST_EXPECT(has == hasFeature(env, f));
             });
         }
@@ -856,7 +861,8 @@ public:
                 (supported.count() - 2 + 1));
             BEAST_EXPECT(hasFeature(env, *neverSupportedFeat));
             foreachFeature(supported, [&](uint256 const& f) {
-                bool hasnot = (f == featureDynamicMPT || f == featureFlow);
+                bool hasnot =
+                    (f == featureDynamicMPT || f == featureTokenEscrow);
                 this->BEAST_EXPECT(hasnot != hasFeature(env, f));
             });
         }

src/test/ledger/View_test.cpp

@@ -1117,7 +1117,7 @@ class GetAmendments_test : public beast::unit_test::suite
         // There should be at least 3 amendments.  Don't do exact comparison
         // to avoid maintenance as more amendments are added in the future.
         BEAST_EXPECT(i == 254);
-        BEAST_EXPECT(majorities.size() >= 3);
+        BEAST_EXPECT(majorities.size() >= 2);
 
         // None of the amendments should be enabled yet.
         auto enableds = getEnabledAmendments(*env.closed());
@@ -1135,7 +1135,7 @@ class GetAmendments_test : public beast::unit_test::suite
                 break;
         }
         BEAST_EXPECT(i == 255);
-        BEAST_EXPECT(enableds.size() >= 3);
+        BEAST_EXPECT(enableds.size() >= 2);
     }
 
     void

src/test/rpc/Feature_test.cpp

@@ -123,7 +123,7 @@ class Feature_test : public beast::unit_test::suite
         BEAST_EXPECT(
             featureToName(fixRemoveNFTokenAutoTrustLine) ==
             "fixRemoveNFTokenAutoTrustLine");
-        BEAST_EXPECT(featureToName(featureFlow) == "Flow");
+        BEAST_EXPECT(featureToName(featureBatch) == "Batch");
         BEAST_EXPECT(featureToName(featureDID) == "DID");
         BEAST_EXPECT(
             featureToName(fixIncludeKeyletFields) == "fixIncludeKeyletFields");
@@ -183,16 +183,16 @@ class Feature_test : public beast::unit_test::suite
         using namespace test::jtx;
         Env env{*this};
 
-        auto jrr = env.rpc("feature", "Flow")[jss::result];
+        auto jrr = env.rpc("feature", "fixAMMOverflowOffer")[jss::result];
         BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
         jrr.removeMember(jss::status);
         BEAST_EXPECT(jrr.size() == 1);
         BEAST_EXPECT(
-            jrr.isMember("740352F2412A9909880C23A559FCECEDA3BE2126FED62FC7660D6"
+            jrr.isMember("12523DF04B553A0B1AD74F42DDB741DE8DC06A03FC089A0EF197E"
-                         "28A06927F11"));
+                         "2A87F1D8107"));
         auto feature = *(jrr.begin());
 
-        BEAST_EXPECTS(feature[jss::name] == "Flow", "name");
+        BEAST_EXPECTS(feature[jss::name] == "fixAMMOverflowOffer", "name");
         BEAST_EXPECTS(!feature[jss::enabled].asBool(), "enabled");
         BEAST_EXPECTS(
             feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(),
@@ -200,7 +200,7 @@ class Feature_test : public beast::unit_test::suite
         BEAST_EXPECTS(feature[jss::supported].asBool(), "supported");
 
         // feature names are case-sensitive - expect error here
-        jrr = env.rpc("feature", "flow")[jss::result];
+        jrr = env.rpc("feature", "fMM")[jss::result];
         BEAST_EXPECT(jrr[jss::error] == "badFeature");
         BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid.");
     }
@@ -419,9 +419,9 @@ class Feature_test : public beast::unit_test::suite
                 break;
         }
 
-        // There should be at least 3 amendments.  Don't do exact comparison
+        // There should be at least 2 amendments.  Don't do exact comparison
         // to avoid maintenance as more amendments are added in the future.
-        BEAST_EXPECT(majorities.size() >= 3);
+        BEAST_EXPECT(majorities.size() >= 2);
         std::map<std::string, VoteBehavior> const& votes =
             ripple::detail::supportedAmendments();
 
@@ -476,8 +476,8 @@ class Feature_test : public beast::unit_test::suite
         testcase("Veto");
 
         using namespace test::jtx;
-        Env env{*this, FeatureBitset{featureFlow}};
+        Env env{*this, FeatureBitset{featurePriceOracle}};
-        constexpr char const* featureName = "Flow";
+        constexpr char const* featureName = "fixAMMOverflowOffer";
 
         auto jrr = env.rpc("feature", featureName)[jss::result];
         if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"))

src/xrpld/app/misc/LendingHelpers.h

@@ -27,22 +27,54 @@ roundPeriodicPayment(
     return roundToAsset(asset, periodicPayment, scale, Number::upward);
 }
 
-/// This structure is explained in the XLS-66 spec, section 3.2.4.4 (Failure
+/* Represents the breakdown of amounts to be paid and changes applied to the
-/// Conditions)
+ * Loan object while processing a loan payment.
+ *
+ * This structure is returned after processing a loan payment transaction and
+ * captures the amounts that need to be paid. The actual ledger entry changes
+ * are made in LoanPay based on this structure values.
+ *
+ * The sum of principalPaid, interestPaid, and feePaid represents the total
+ * amount to be deducted from the borrower's account. The valueChange field
+ * tracks whether the loan's total value increased or decreased beyond normal
+ * amortization.
+ *
+ * This structure is explained in the XLS-66 spec, section 3.2.4.2 (Payment
+ * Processing).
+ */
 struct LoanPaymentParts
 {
-    /// principal_paid is the amount of principal that the payment covered.
+    // The amount of principal paid that reduces the loan balance.
+    // This amount is subtracted from sfPrincipalOutstanding in the Loan object
+    // and paid to the Vault
     Number principalPaid = numZero;
-    /// interest_paid is the amount of interest that the payment covered.
+
+    // The total amount of interest paid to the Vault.
+    // This includes:
+    // - Tracked interest from the amortization schedule
+    // - Untracked interest (e.g., late payment penalty interest)
+    // This value is always non-negative.
     Number interestPaid = numZero;
-    /**
+
-     * value_change is the amount by which the total value of the Loan changed.
+    // The change in the loan's total value outstanding.
-     *  If value_change < 0, Loan value decreased.
+    // - If valueChange < 0: Loan value decreased
-     *  If value_change > 0, Loan value increased.
+    // - If valueChange > 0: Loan value increased
-     * This is 0 for regular payments.
+    // - If valueChange = 0: No value adjustment
-     */
+    //
+    // For regular on-time payments, this is always 0. Non-zero values occur
+    // when:
+    // - Overpayments reduce the loan balance beyond the scheduled amount
+    // - Late payments add penalty interest to the loan value
+    // - Early full payment may increase or decrease the loan value based on
+    // terms
     Number valueChange = numZero;
-    /// feePaid is amount of fee that is paid to the broker
+
+    /* The total amount of fees paid to the Broker.
+     * This includes:
+     * - Tracked management fees from the amortization schedule
+     * - Untracked fees (e.g., late payment fees, service fees, origination
+     * fees) This value is always non-negative.
+     */
     Number feePaid = numZero;
 
     LoanPaymentParts&
@@ -52,46 +84,71 @@ struct LoanPaymentParts
     operator==(LoanPaymentParts const& other) const;
 };
 
-/** This structure describes the initial "computed" properties of a loan.
+/* Describes the initial computed properties of a loan.
  *
- * It is used at loan creation and when the terms of a loan change, such as
+ * This structure contains the fundamental calculated values that define a
- * after an overpayment.
+ * loan's payment structure and amortization schedule. These properties are
+ * computed:
+ * - At loan creation (LoanSet transaction)
+ * - When loan terms change (e.g., after an overpayment that reduces the loan
+ * balance)
  */
 struct LoanProperties
 {
+    // The unrounded amount to be paid at each regular payment period.
+    // Calculated using the standard amortization formula based on principal,
+    // interest rate, and number of payments.
+    // The actual amount paid in the LoanPay transaction must be rounded up to
+    // the precision of the asset and loan.
     Number periodicPayment;
+
+    // The total amount the borrower will pay over the life of the loan.
+    // Equal to periodicPayment * paymentsRemaining.
+    // This includes principal, interest, and management fees.
     Number totalValueOutstanding;
+
+    // The total management fee that will be paid to the broker over the
+    // loan's lifetime. This is a percentage of the total interest (gross)
+    // as specified by the broker's management fee rate.
     Number managementFeeOwedToBroker;
+
+    // The scale (decimal places) used for rounding all loan amounts.
+    // This is the maximum of:
+    // - The asset's native scale
+    // - A minimum scale required to represent the periodic payment accurately
+    // All loan state values (principal, interest, fees) are rounded to this
+    // scale.
     std::int32_t loanScale;
+
+    // The principal portion of the first payment.
     Number firstPaymentPrincipal;
 };
 
-/** This structure captures the current state of a loan and all the
+/** This structure captures the parts of a loan state.
-   relevant parts.
+ *
-
+ *  Whether the values are raw (unrounded) or rounded will depend on how it was
-   Whether the values are raw (unrounded) or rounded will
+ * computed.
-   depend on how it was computed.
+ *
-
+ *  Many of the fields can be derived from each other, but they're all provided
-   Many of the fields can be derived from each other, but they're all provided
+ *  here to reduce code duplication and possible mistakes.
-   here to reduce code duplication and possible mistakes.
+ *   e.g.
-   e.g.
+ *     * interestOutstanding = valueOutstanding - principalOutstanding
-     * interestOutstanding = valueOutstanding - principalOutstanding
+ *     * interestDue = interestOutstanding - managementFeeDue
-     * interestDue = interestOutstanding - managementFeeDue
  */
 struct LoanState
 {
-    /// Total value still due to be paid by the borrower.
+    // Total value still due to be paid by the borrower.
     Number valueOutstanding;
-    /// Prinicipal still due to be paid by the borrower.
+    // Principal still due to be paid by the borrower.
     Number principalOutstanding;
-    /// Interest still due to be paid TO the Vault.
+    // Interest still due to be paid to the Vault.
     // This is a portion of interestOutstanding
     Number interestDue;
-    /// Management fee still due to be paid TO the broker.
+    // Management fee still due to be paid to the broker.
     // This is a portion of interestOutstanding
     Number managementFeeDue;
 
-    /// Interest still due to be paid by the borrower.
+    // Interest still due to be paid by the borrower.
     Number
     interestOutstanding() const
     {
@@ -199,42 +256,133 @@ namespace detail {
 
 enum class PaymentSpecialCase { none, final, extra };
 
-/// This structure is used internally to compute the breakdown of a
+/* Represents a single loan payment component parts.
-/// single loan payment
+
+* This structure captures the "delta" (change) values that will be applied to
+* the tracked fields in the Loan ledger object when a payment is processed.
+*
+* These are called "deltas" because they represent the amount by which each
+* corresponding field in the Loan object will be reduced.
+* They are "tracked" as they change tracked loan values.
+*/
 struct PaymentComponents
 {
-    // tracked values are rounded to the asset and loan scale, and correspond to
+    // The change in total value outstanding for this payment.
-    // fields in the Loan ledger object.
+    // This amount will be subtracted from sfTotalValueOutstanding in the Loan
-    // trackedValueDelta modifies sfTotalValueOutstanding.
+    // object. Equal to the sum of trackedPrincipalDelta,
+    // trackedInterestPart(), and trackedManagementFeeDelta.
     Number trackedValueDelta;
-    // trackedPrincipalDelta modifies sfPrincipalOutstanding.
+
+    // The change in principal outstanding for this payment.
+    // This amount will be subtracted from sfPrincipalOutstanding in the Loan
+    // object, representing the portion of the payment that reduces the
+    // original loan amount.
     Number trackedPrincipalDelta;
-    // trackedManagementFeeDelta modifies sfManagementFeeOutstanding. It will
+
-    // not include any "extra" fees that go directly to the broker, such as late
+    // The change in management fee outstanding for this payment.
-    // fees.
+    // This amount will be subtracted from sfManagementFeeOutstanding in the
+    // Loan object. This represents only the tracked management fees from the
+    // amortization schedule and does not include additional untracked fees
+    // (such as late payment fees) that go directly to the broker.
     Number trackedManagementFeeDelta;
 
+    // Indicates if this payment has special handling requirements.
+    // - none: Regular scheduled payment
+    // - final: The last payment that closes out the loan
+    // - extra: An additional payment beyond the regular schedule (overpayment)
     PaymentSpecialCase specialCase = PaymentSpecialCase::none;
 
+    // Calculates the tracked interest portion of this payment.
+    // This is derived from the other components as:
+    // trackedValueDelta - trackedPrincipalDelta - trackedManagementFeeDelta
+    //
+    // @return The amount of tracked interest included in this payment that
+    //         will be paid to the vault.
     Number
     trackedInterestPart() const;
 };
 
-// This structure describes the difference between two LoanState objects so that
+/* Extends PaymentComponents with untracked payment amounts.
-// the differences between components don't have to be tracked individually,
+ *
-// risking more errors. How that difference is used depends on the context.
+ * This structure adds untracked fees and interest to the base
+ * PaymentComponents, representing amounts that don't affect the Loan object's
+ * tracked state but are still part of the total payment due from the borrower.
+ *
+ * Untracked amounts include:
+ * - Late payment fees that go directly to the Broker
+ * - Late payment penalty interest that goes directly to the Vault
+ * - Service fees
+ *
+ * The key distinction is that tracked amounts reduce the Loan object's state
+ * (sfTotalValueOutstanding, sfPrincipalOutstanding,
+ * sfManagementFeeOutstanding), while untracked amounts are paid directly to the
+ * recipient without affecting the loan's amortization schedule.
+ */
+struct ExtendedPaymentComponents : public PaymentComponents
+{
+    // Additional management fees that go directly to the Broker.
+    // This includes fees not part of the standard amortization schedule
+    // (e.g., late fees, service fees, origination fees).
+    // This value may be negative, though the final value returned in
+    // LoanPaymentParts.feePaid will never be negative.
+    Number untrackedManagementFee;
+
+    // Additional interest that goes directly to the Vault.
+    // This includes interest not part of the standard amortization schedule
+    // (e.g., late payment penalty interest).
+    // This value may be negative, though the final value returned in
+    // LoanPaymentParts.interestPaid will never be negative.
+    Number untrackedInterest;
+
+    // The complete amount due from the borrower for this payment.
+    // Calculated as: trackedValueDelta + untrackedInterest +
+    // untrackedManagementFee
+    //
+    // This value is used to validate that the payment amount provided by the
+    // borrower is sufficient to cover all components of the payment.
+    Number totalDue;
+
+    ExtendedPaymentComponents(
+        PaymentComponents const& p,
+        Number fee,
+        Number interest = numZero)
+        : PaymentComponents(p)
+        , untrackedManagementFee(fee)
+        , untrackedInterest(interest)
+        , totalDue(
+              trackedValueDelta + untrackedInterest + untrackedManagementFee)
+    {
+    }
+};
+
+/* Represents the differences between two loan states.
+ *
+ * This structure is used to capture the change in each component of a loan's
+ * state, typically when computing the difference between two LoanState objects
+ * (e.g., before and after a payment). It is a convenient way to capture changes
+ * in each component. How that difference is used depends on the context.
+ */
 struct LoanStateDeltas
 {
+    // The difference in principal outstanding between two loan states.
     Number principal;
+
+    // The difference in interest due between two loan states.
     Number interest;
+
+    // The difference in management fee outstanding between two loan states.
     Number managementFee;
 
+    /* Calculates the total change across all components.
+     * @return The sum of principal, interest, and management fee deltas.
+     */
     Number
     total() const
     {
         return principal + interest + managementFee;
     }
 
+    // Ensures all delta values are non-negative.
     void
     nonNegative();
 };

src/xrpld/app/paths/RippleCalc.cpp

@@ -43,15 +43,6 @@ RippleCalc::rippleCalculate(
     PaymentSandbox flowSB(&view);
     auto j = l.journal("Flow");
 
-    if (!view.rules().enabled(featureFlow))
-    {
-        // The new payment engine was enabled several years ago. New transaction
-        // should never use the old rules. Assume this is a replay
-        j.fatal()
-            << "Old payment rules are required for this transaction. Assuming "
-               "this is a replay and running with the new rules.";
-    }
-
     {
         bool const defaultPaths =
             !pInputs ? true : pInputs->defaultPathsAllowed;

src/xrpld/app/paths/detail/StrandFlow.h

@@ -433,7 +433,7 @@ public:
         // add the strands in `next_` to `cur_`, sorted by theoretical quality.
         // Best quality first.
         cur_.clear();
-        if (v.rules().enabled(featureFlowSortStrands) && !next_.empty())
+        if (!next_.empty())
         {
             std::vector<std::pair<Quality, Strand const*>> strandQuals;
             strandQuals.reserve(next_.size());
@@ -719,46 +719,16 @@ flow(
                 continue;
             }
 
-            if (baseView.rules().enabled(featureFlowSortStrands))
+            XRPL_ASSERT(!best, "ripple::flow : best is unset");
-            {
+            if (!f.inactive)
-                XRPL_ASSERT(!best, "ripple::flow : best is unset");
+                activeStrands.push(strand);
-                if (!f.inactive)
+            best.emplace(f.in, f.out, std::move(*f.sandbox), *strand, q);
-                    activeStrands.push(strand);
+            activeStrands.pushRemainingCurToNext(strandIndex + 1);
-                best.emplace(f.in, f.out, std::move(*f.sandbox), *strand, q);
+            break;
-                activeStrands.pushRemainingCurToNext(strandIndex + 1);
-                break;
-            }
-
-            activeStrands.push(strand);
-
-            if (!best || best->quality < q ||
-                (best->quality == q && best->out < f.out))
-            {
-                // If this strand is inactive (because it consumed too many
-                // offers) and ends up having the best quality, remove it
-                // from the activeStrands. If it doesn't end up having the
-                // best quality, keep it active.
-
-                if (f.inactive)
-                {
-                    // This should be `nextSize`, not `size`. This issue is
-                    // fixed in featureFlowSortStrands.
-                    markInactiveOnUse = activeStrands.size() - 1;
-                }
-                else
-                {
-                    markInactiveOnUse.reset();
-                }
-
-                best.emplace(f.in, f.out, std::move(*f.sandbox), *strand, q);
-            }
         }
 
-        bool const shouldBreak = [&] {
+        bool const shouldBreak =
-            if (baseView.rules().enabled(featureFlowSortStrands))
+            !best || offersConsidered >= maxOffersToConsider;
-                return !best || offersConsidered >= maxOffersToConsider;
-            return !best;
-        }();
 
         if (best)
         {

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

@@ -1731,6 +1731,102 @@ ValidPermissionedDomain::finalize(
         (sleStatus_[1] ? check(*sleStatus_[1], j) : true);
 }
 
+//------------------------------------------------------------------------------
+
+void
+ValidPseudoAccounts::visitEntry(
+    bool isDelete,
+    std::shared_ptr<SLE const> const& before,
+    std::shared_ptr<SLE const> const& after)
+{
+    if (isDelete)
+        // Deletion is ignored
+        return;
+
+    if (after && after->getType() == ltACCOUNT_ROOT)
+    {
+        bool const isPseudo = [&]() {
+            // isPseudoAccount checks that any of the pseudo-account fields are
+            // set.
+            if (isPseudoAccount(after))
+                return true;
+            // Not all pseudo-accounts have a zero sequence, but all accounts
+            // with a zero sequence had better be pseudo-accounts.
+            if (after->at(sfSequence) == 0)
+                return true;
+
+            return false;
+        }();
+        if (isPseudo)
+        {
+            // Pseudo accounts must have the following properties:
+            // 1. Exactly one of the pseudo-account fields is set.
+            // 2. The sequence number is not changed.
+            // 3. The lsfDisableMaster, lsfDefaultRipple, and lsfDepositAuth
+            // flags are set.
+            // 4. The RegularKey is not set.
+            {
+                std::vector<SField const*> const& fields =
+                    getPseudoAccountFields();
+
+                auto const numFields = std::count_if(
+                    fields.begin(),
+                    fields.end(),
+                    [&after](SField const* sf) -> bool {
+                        return after->isFieldPresent(*sf);
+                    });
+                if (numFields != 1)
+                {
+                    std::stringstream error;
+                    error << "pseudo-account has " << numFields
+                          << " pseudo-account fields set";
+                    errors_.emplace_back(error.str());
+                }
+            }
+            if (before && before->at(sfSequence) != after->at(sfSequence))
+            {
+                errors_.emplace_back("pseudo-account sequence changed");
+            }
+            if (!after->isFlag(
+                    lsfDisableMaster | lsfDefaultRipple | lsfDepositAuth))
+            {
+                errors_.emplace_back("pseudo-account flags are not set");
+            }
+            if (after->isFieldPresent(sfRegularKey))
+            {
+                errors_.emplace_back("pseudo-account has a regular key");
+            }
+        }
+    }
+}
+
+bool
+ValidPseudoAccounts::finalize(
+    STTx const& tx,
+    TER const,
+    XRPAmount const,
+    ReadView const& view,
+    beast::Journal const& j)
+{
+    bool const enforce = view.rules().enabled(featureSingleAssetVault);
+    XRPL_ASSERT(
+        errors_.empty() || enforce,
+        "ripple::ValidPseudoAccounts::finalize : no bad "
+        "changes or enforce invariant");
+    if (!errors_.empty())
+    {
+        for (auto const& error : errors_)
+        {
+            JLOG(j.fatal()) << "Invariant failed: " << error;
+        }
+        if (enforce)
+            return false;
+    }
+    return true;
+}
+
+//------------------------------------------------------------------------------
+
 void
 ValidPermissionedDEX::visitEntry(
     bool,
@@ -2237,100 +2333,6 @@ NoModifiedUnmodifiableFields::finalize(
 
 //------------------------------------------------------------------------------
 
-void
-ValidPseudoAccounts::visitEntry(
-    bool isDelete,
-    std::shared_ptr<SLE const> const& before,
-    std::shared_ptr<SLE const> const& after)
-{
-    if (isDelete)
-        // Deletion is ignored
-        return;
-
-    if (after && after->getType() == ltACCOUNT_ROOT)
-    {
-        bool const isPseudo = [&]() {
-            // isPseudoAccount checks that any of the pseudo-account fields are
-            // set.
-            if (isPseudoAccount(after))
-                return true;
-            // Not all pseudo-accounts have a zero sequence, but all accounts
-            // with a zero sequence had better be pseudo-accounts.
-            if (after->at(sfSequence) == 0)
-                return true;
-
-            return false;
-        }();
-        if (isPseudo)
-        {
-            // Pseudo accounts must have the following properties:
-            // 1. Exactly one of the pseudo-account fields is set.
-            // 2. The sequence number is not changed.
-            // 3. The lsfDisableMaster, lsfDefaultRipple, and lsfDepositAuth
-            // flags are set.
-            // 4. The RegularKey is not set.
-            {
-                std::vector<SField const*> const& fields =
-                    getPseudoAccountFields();
-
-                auto const numFields = std::count_if(
-                    fields.begin(),
-                    fields.end(),
-                    [&after](SField const* sf) -> bool {
-                        return after->isFieldPresent(*sf);
-                    });
-                if (numFields != 1)
-                {
-                    std::stringstream error;
-                    error << "pseudo-account has " << numFields
-                          << " pseudo-account fields set";
-                    errors_.emplace_back(error.str());
-                }
-            }
-            if (before && before->at(sfSequence) != after->at(sfSequence))
-            {
-                errors_.emplace_back("pseudo-account sequence changed");
-            }
-            if (!after->isFlag(
-                    lsfDisableMaster | lsfDefaultRipple | lsfDepositAuth))
-            {
-                errors_.emplace_back("pseudo-account flags are not set");
-            }
-            if (after->isFieldPresent(sfRegularKey))
-            {
-                errors_.emplace_back("pseudo-account has a regular key");
-            }
-        }
-    }
-}
-
-bool
-ValidPseudoAccounts::finalize(
-    STTx const& tx,
-    TER const,
-    XRPAmount const,
-    ReadView const& view,
-    beast::Journal const& j)
-{
-    bool const enforce = view.rules().enabled(featureSingleAssetVault);
-    XRPL_ASSERT(
-        errors_.empty() || enforce,
-        "ripple::ValidPseudoAccounts::finalize : no bad "
-        "changes or enforce invariant");
-    if (!errors_.empty())
-    {
-        for (auto const& error : errors_)
-        {
-            JLOG(j.fatal()) << "Invariant failed: " << error;
-        }
-        if (enforce)
-            return false;
-    }
-    return true;
-}
-
-//------------------------------------------------------------------------------
-
 void
 ValidLoanBroker::visitEntry(
     bool isDelete,

src/xrpld/app/tx/detail/InvariantCheck.h

@@ -612,6 +612,34 @@ public:
         beast::Journal const&);
 };
 
+/**
+ * @brief Invariants: Pseudo-accounts have valid and consisent properties
+ *
+ * Pseudo-accounts have certain properties, and some of those properties are
+ * unique to pseudo-accounts. Check that all pseudo-accounts are following the
+ * rules, and that only pseudo-accounts look like pseudo-accounts.
+ *
+ */
+class ValidPseudoAccounts
+{
+    std::vector<std::string> errors_;
+
+public:
+    void
+    visitEntry(
+        bool,
+        std::shared_ptr<SLE const> const&,
+        std::shared_ptr<SLE const> const&);
+
+    bool
+    finalize(
+        STTx const&,
+        TER const,
+        XRPAmount const,
+        ReadView const&,
+        beast::Journal const&);
+};
+
 class ValidPermissionedDEX
 {
     bool regularOffers_ = false;
@@ -725,34 +753,6 @@ public:
         beast::Journal const&);
 };
 
-/**
- * @brief Invariants: Pseudo-accounts have valid and consisent properties
- *
- * Pseudo-accounts have certain properties, and some of those properties are
- * unique to pseudo-accounts. Check that all pseudo-accounts are following the
- * rules, and that only pseudo-accounts look like pseudo-accounts.
- *
- */
-class ValidPseudoAccounts
-{
-    std::vector<std::string> errors_;
-
-public:
-    void
-    visitEntry(
-        bool,
-        std::shared_ptr<SLE const> const&,
-        std::shared_ptr<SLE const> const&);
-
-    bool
-    finalize(
-        STTx const&,
-        TER const,
-        XRPAmount const,
-        ReadView const&,
-        beast::Journal const&);
-};
-
 /**
  * @brief Invariants: Loan brokers are internally consistent
  *

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

@@ -192,8 +192,7 @@ preclaimHelper<MPTIssue>(
     if (!sleIssuance)
         return tecOBJECT_NOT_FOUND;
 
-    if (!sleIssuance->isFlag(lsfMPTCanClawback) ||
+    if (!sleIssuance->isFlag(lsfMPTCanClawback))
-        !sleIssuance->isFlag(lsfMPTCanLock))
         return tecNO_PERMISSION;
 
     // With all the checking already done, this should be impossible
@@ -233,7 +232,6 @@ LoanBrokerCoverClawback::preclaim(PreclaimContext const& ctx)
         // LCOV_EXCL_STOP
     }
 
-
     auto const vaultAsset = vault->at(sfAsset);
 
     if (vaultAsset.native())

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

@@ -269,7 +269,9 @@ LoanPay::doApply()
     // Normally freeze status is checked in preflight, but we do it here to
     // avoid duplicating the check. It'll claim a fee either way.
     bool const sendBrokerFeeToOwner = [&]() {
-        // Always round the minimum required up.
+        // Round the minimum required cover up to be conservative. This ensures
+        // CoverAvailable never drops below the theoretical minimum, protecting
+        // the broker's solvency.
         NumberRoundModeGuard mg(Number::upward);
         return coverAvailableProxy >=
             roundToAsset(

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

@@ -465,7 +465,9 @@ LoanSet::doApply()
     }
     TenthBips32 const coverRateMinimum{brokerSle->at(sfCoverRateMinimum)};
     {
-        // Always round the minimum required up.
+        // Round the minimum required cover up to be conservative. This ensures
+        // CoverAvailable never drops below the theoretical minimum, protecting
+        // the broker's solvency.
         NumberRoundModeGuard mg(Number::upward);
         if (brokerSle->at(sfCoverAvailable) <
             tenthBipsOfValue(newDebtTotal, coverRateMinimum))