Merge branch 'develop' into copilot/fix-vetoed-type-error

Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>

API-CHANGELOG.md

@@ -92,6 +92,10 @@ This release contains bug fixes only and no API changes.
 
 [Version 2.5.0](https://github.com/XRPLF/rippled/releases/tag/2.5.0) was released on Jun 24, 2025.
 
+### Breaking changes in 2.5.0
+
+- `feature`: The `vetoed` field in admin-mode responses is now always a boolean. Disabled obsolete amendments now have `"vetoed": true` instead of the previous `"vetoed": "Obsolete"` string value, which was not type-safe. A new `"obsolete"` boolean field is now included in all responses (both admin and non-admin), indicating whether an amendment is obsolete.
+
 ### Additions and bugfixes in 2.5.0
 
 - `tx`: Added `ctid` field to the response and improved error handling. ([#4738](https://github.com/XRPLF/rippled/pull/4738))

include/xrpl/protocol/jss.h

@@ -433,6 +433,7 @@ JSS(node_write_retries);          // out: GetCounts
 JSS(node_writes_delayed);         // out::GetCounts
 JSS(nth);                         // out: RPC server_definitions
 JSS(obligations);                 // out: GatewayBalances
+JSS(obsolete);                    // out: AmendmentTableImpl
 JSS(offers);                      // out: NetworkOPs, AccountOffers, Subscribe
 JSS(offer_id);                    // out: insertNFTokenOfferID
 JSS(offline);                     // in: TransactionSign

src/libxrpl/protocol/BuildInfo.cpp

@@ -23,7 +23,7 @@ namespace {
 //  and follow the format described at http://semver.org/
 //------------------------------------------------------------------------------
 // clang-format off
-char const* const versionString = "3.2.0-b3"
+char const* const versionString = "3.2.0-b0"
     // clang-format on
     ;
 

src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp

@@ -43,10 +43,10 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
     if (!vault)
         return tecNO_ENTRY;
 
-    auto const assets = ctx.tx[sfAmount];
+    auto const amount = ctx.tx[sfAmount];
     auto const vaultAsset = vault->at(sfAsset);
     auto const vaultShare = vault->at(sfShareMPTID);
-    if (assets.asset() != vaultAsset && assets.asset() != vaultShare)
+    if (amount.asset() != vaultAsset && amount.asset() != vaultShare)
         return tecWRONG_ASSET;
 
     auto const& vaultAccount = vault->at(sfAccount);
@@ -67,8 +67,53 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
         // LCOV_EXCL_STOP
     }
 
-    if (auto const ret = canWithdraw(ctx.view, ctx.tx))
-        return ret;
+    if (ctx.view.rules().enabled(fixSecurity3_1_3) && amount.asset() == vaultShare)
+    {
+        // Post-fixSecurity3_1_3: if the user specified shares, convert
+        // to the equivalent asset amount before checking withdrawal
+        // limits. Pre-amendment the limit check was skipped for
+        // share-denominated withdrawals.
+        auto const sleIssuance = ctx.view.read(keylet::mptIssuance(vaultShare));
+        if (!sleIssuance)
+        {
+            // LCOV_EXCL_START
+            JLOG(ctx.j.error()) << "VaultWithdraw: missing issuance of vault shares.";
+            return tefINTERNAL;
+            // LCOV_EXCL_STOP
+        }
+
+        try
+        {
+            auto const maybeAssets = sharesToAssetsWithdraw(vault, sleIssuance, amount);
+            if (!maybeAssets)
+                return tefINTERNAL;  // LCOV_EXCL_LINE
+
+            if (auto const ret = canWithdraw(
+                    ctx.view,
+                    account,
+                    dstAcct,
+                    *maybeAssets,
+                    ctx.tx.isFieldPresent(sfDestinationTag)))
+                return ret;
+        }
+        catch (std::overflow_error const&)
+        {
+            // It's easy to hit this exception from Number with large enough Scale
+            // so we avoid spamming the log and only use debug here.
+            JLOG(ctx.j.debug())  //
+                << "VaultWithdraw: overflow error with"
+                << " scale=" << (int)vault->at(sfScale)  //
+                << ", assetsTotal=" << vault->at(sfAssetsTotal)
+                << ", sharesTotal=" << sleIssuance->at(sfOutstandingAmount)
+                << ", amount=" << amount.value();
+            return tecPATH_DRY;
+        }
+    }
+    else
+    {
+        if (auto const ret = canWithdraw(ctx.view, ctx.tx))
+            return ret;
+    }
 
     // If sending to Account (i.e. not a transfer), we will also create (only
     // if authorized) a trust line or MPToken as needed, in doApply().

src/test/app/Vault_test.cpp

@@ -5231,6 +5231,102 @@ class Vault_test : public beast::unit_test::suite
         }
     }
 
+    // Reproduction: canWithdraw IOU limit check bypassed when
+    // withdrawal amount is specified in shares (MPT) rather than in assets.
+    void
+    testBug6_LimitBypassWithShares()
+    {
+        using namespace test::jtx;
+        testcase("Bug6 - limit bypass with share-denominated withdrawal");
+
+        auto const allAmendments = testable_amendments() | featureSingleAssetVault;
+
+        for (auto const& features : {allAmendments, allAmendments - fixSecurity3_1_3})
+        {
+            bool const withFix = features[fixSecurity3_1_3];
+
+            Env env{*this, features};
+            Account const owner{"owner"};
+            Account const issuer{"issuer"};
+            Account const depositor{"depositor"};
+            Account const charlie{"charlie"};
+            Vault const vault{env};
+
+            env.fund(XRP(1000), issuer, owner, depositor, charlie);
+            env(fset(issuer, asfAllowTrustLineClawback));
+            env.close();
+
+            PrettyAsset const asset = issuer["IOU"];
+            env.trust(asset(1000), owner);
+            env.trust(asset(1000), depositor);
+            env(pay(issuer, owner, asset(200)));
+            env(pay(issuer, depositor, asset(200)));
+            env.close();
+
+            // Charlie gets a LOW trustline limit of 5
+            env.trust(asset(5), charlie);
+            env.close();
+
+            auto const [tx, keylet] = vault.create({.owner = owner, .asset = asset});
+            env(tx);
+            env.close();
+
+            auto const depositTx =
+                vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)});
+            env(depositTx);
+            env.close();
+
+            // Get the share MPT info
+            auto const vaultSle = env.le(keylet);
+            if (!BEAST_EXPECT(vaultSle))
+                return;
+            auto const mptIssuanceID = vaultSle->at(sfShareMPTID);
+            MPTIssue const shares(mptIssuanceID);
+            PrettyAsset const share(shares);
+
+            // CONTROL: Withdraw 10 IOU (asset-denominated) to charlie.
+            // Charlie's limit is 5, so this should be rejected with tecNO_LINE
+            // regardless of the amendment.
+            {
+                auto withdrawTx =
+                    vault.withdraw({.depositor = depositor, .id = keylet.key, .amount = asset(10)});
+                withdrawTx[sfDestination] = charlie.human();
+                env(withdrawTx, ter{tecNO_LINE});
+                env.close();
+            }
+            auto const charlieBalanceBefore = env.balance(charlie, asset.raw().get<Issue>());
+
+            // Withdraw the equivalent amount in shares to charlie.
+            // Post-fix: rejected (tecNO_LINE) because the share amount is
+            //   converted to assets and the trustline limit is checked.
+            // Pre-fix: succeeds (tesSUCCESS) because the limit check was
+            //   skipped for share-denominated withdrawals.
+            {
+                auto withdrawTx = vault.withdraw(
+                    {.depositor = depositor,
+                     .id = keylet.key,
+                     .amount = STAmount(share, 10'000'000)});
+                withdrawTx[sfDestination] = charlie.human();
+                env(withdrawTx, ter{withFix ? TER{tecNO_LINE} : TER{tesSUCCESS}});
+                env.close();
+
+                auto const charlieBalanceAfter = env.balance(charlie, asset.raw().get<Issue>());
+                if (withFix)
+                {
+                    // Post-fix: charlie's balance is unchanged — the withdrawal
+                    // was correctly rejected despite being share-denominated.
+                    BEAST_EXPECT(charlieBalanceAfter == charlieBalanceBefore);
+                }
+                else
+                {
+                    // Pre-fix: charlie received the assets, bypassing the
+                    // trustline limit.
+                    BEAST_EXPECT(charlieBalanceAfter > charlieBalanceBefore);
+                }
+            }
+        }
+    }
+
 public:
     void
     run() override
@@ -5251,6 +5347,7 @@ public:
         testVaultClawbackBurnShares();
         testVaultClawbackAssets();
         testAssetsMaximum();
+        testBug6_LimitBypassWithShares();
     }
 };
 

src/test/rpc/Feature_test.cpp

@@ -151,12 +151,15 @@ class Feature_test : public beast::unit_test::suite
                 feature.isMember(jss::enabled) && !feature[jss::enabled].asBool(),
                 feature[jss::name].asString() + " enabled");
             BEAST_EXPECTS(
-                feature.isMember(jss::vetoed) && feature[jss::vetoed].isBool() == !expectObsolete &&
-                    (!feature[jss::vetoed].isBool() ||
-                     feature[jss::vetoed].asBool() == expectVeto) &&
-                    (feature[jss::vetoed].isBool() ||
-                     feature[jss::vetoed].asString() == "Obsolete"),
-                feature[jss::name].asString() + " vetoed");
+                feature.isMember(jss::vetoed) && feature[jss::vetoed].isBool(),
+                feature[jss::name].asString() + " vetoed is bool");
+            BEAST_EXPECTS(
+                feature[jss::vetoed].asBool() == (expectVeto || expectObsolete),
+                feature[jss::name].asString() + " vetoed value");
+            BEAST_EXPECTS(
+                feature.isMember(jss::obsolete) && feature[jss::obsolete].isBool() &&
+                    feature[jss::obsolete].asBool() == expectObsolete,
+                feature[jss::name].asString() + " obsolete");
             BEAST_EXPECTS(
                 feature.isMember(jss::supported) && feature[jss::supported].asBool(),
                 feature[jss::name].asString() + " supported");
@@ -288,6 +291,9 @@ class Feature_test : public beast::unit_test::suite
                     (*it).isMember(jss::supported) &&
                         (*it)[jss::supported].asBool() == expectSupported,
                     (*it)[jss::name].asString() + " supported");
+                BEAST_EXPECTS(
+                    (*it).isMember(jss::obsolete) && (*it)[jss::obsolete].isBool(),
+                    (*it)[jss::name].asString() + " unknown obsolete");
                 BEAST_EXPECT(!(*it).isMember(jss::vetoed));
                 BEAST_EXPECT(!(*it).isMember(jss::majority));
                 BEAST_EXPECT(!(*it).isMember(jss::count));
@@ -357,12 +363,15 @@ class Feature_test : public beast::unit_test::suite
             else
             {
                 BEAST_EXPECTS(
-                    (*it).isMember(jss::vetoed) && (*it)[jss::vetoed].isBool() == !expectObsolete &&
-                        (!(*it)[jss::vetoed].isBool() ||
-                         (*it)[jss::vetoed].asBool() == expectVeto) &&
-                        ((*it)[jss::vetoed].isBool() ||
-                         (*it)[jss::vetoed].asString() == "Obsolete"),
-                    (*it)[jss::name].asString() + " vetoed");
+                    (*it).isMember(jss::vetoed) && (*it)[jss::vetoed].isBool(),
+                    (*it)[jss::name].asString() + " vetoed is bool");
+                BEAST_EXPECTS(
+                    (*it)[jss::vetoed].asBool() == (expectVeto || expectObsolete),
+                    (*it)[jss::name].asString() + " vetoed value");
+                BEAST_EXPECTS(
+                    (*it).isMember(jss::obsolete) && (*it)[jss::obsolete].isBool() &&
+                        (*it)[jss::obsolete].asBool() == expectObsolete,
+                    (*it)[jss::name].asString() + " obsolete");
             }
             BEAST_EXPECTS(
                 (*it).isMember(jss::supported) && (*it)[jss::supported].asBool() == expectSupported,
@@ -432,12 +441,15 @@ class Feature_test : public beast::unit_test::suite
                 (expectVeto || expectObsolete) ^ feature.isMember(jss::majority),
                 feature[jss::name].asString() + " majority");
             BEAST_EXPECTS(
-                feature.isMember(jss::vetoed) && feature[jss::vetoed].isBool() == !expectObsolete &&
-                    (!feature[jss::vetoed].isBool() ||
-                     feature[jss::vetoed].asBool() == expectVeto) &&
-                    (feature[jss::vetoed].isBool() ||
-                     feature[jss::vetoed].asString() == "Obsolete"),
-                feature[jss::name].asString() + " vetoed");
+                feature.isMember(jss::vetoed) && feature[jss::vetoed].isBool(),
+                feature[jss::name].asString() + " vetoed is bool");
+            BEAST_EXPECTS(
+                feature[jss::vetoed].asBool() == (expectVeto || expectObsolete),
+                feature[jss::name].asString() + " vetoed value");
+            BEAST_EXPECTS(
+                feature.isMember(jss::obsolete) && feature[jss::obsolete].isBool() &&
+                    feature[jss::obsolete].asBool() == expectObsolete,
+                feature[jss::name].asString() + " obsolete");
             BEAST_EXPECTS(feature.isMember(jss::count), feature[jss::name].asString() + " count");
             BEAST_EXPECTS(
                 feature.isMember(jss::threshold), feature[jss::name].asString() + " threshold");
@@ -528,8 +540,9 @@ class Feature_test : public beast::unit_test::suite
         auto feature = *(jrr.begin());
         BEAST_EXPECTS(feature[jss::name] == featureName, "name");
         BEAST_EXPECTS(
-            feature[jss::vetoed].isString() && feature[jss::vetoed].asString() == "Obsolete",
-            "vetoed");
+            feature[jss::vetoed].isBool() && feature[jss::vetoed].asBool() == true, "vetoed");
+        BEAST_EXPECTS(
+            feature[jss::obsolete].isBool() && feature[jss::obsolete].asBool() == true, "obsolete");
 
         jrr = env.rpc("feature", featureName, "reject")[jss::result];
         if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"))
@@ -540,8 +553,9 @@ class Feature_test : public beast::unit_test::suite
         feature = *(jrr.begin());
         BEAST_EXPECTS(feature[jss::name] == featureName, "name");
         BEAST_EXPECTS(
-            feature[jss::vetoed].isString() && feature[jss::vetoed].asString() == "Obsolete",
-            "vetoed");
+            feature[jss::vetoed].isBool() && feature[jss::vetoed].asBool() == true, "vetoed");
+        BEAST_EXPECTS(
+            feature[jss::obsolete].isBool() && feature[jss::obsolete].asBool() == true, "obsolete");
 
         jrr = env.rpc("feature", featureName, "accept")[jss::result];
         if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"))
@@ -552,8 +566,9 @@ class Feature_test : public beast::unit_test::suite
         feature = *(jrr.begin());
         BEAST_EXPECTS(feature[jss::name] == featureName, "name");
         BEAST_EXPECTS(
-            feature[jss::vetoed].isString() && feature[jss::vetoed].asString() == "Obsolete",
-            "vetoed");
+            feature[jss::vetoed].isBool() && feature[jss::vetoed].asBool() == true, "vetoed");
+        BEAST_EXPECTS(
+            feature[jss::obsolete].isBool() && feature[jss::obsolete].asBool() == true, "obsolete");
 
         // anything other than accept or reject is an error
         jrr = env.rpc("feature", featureName, "maybe");

src/xrpld/app/misc/detail/AmendmentTable.cpp

@@ -924,18 +924,12 @@ AmendmentTableImpl::injectJson(
         v[jss::name] = fs.name;
 
     v[jss::supported] = fs.supported;
-    if (!fs.enabled && isAdmin)
-    {
-        if (fs.vote == AmendmentVote::obsolete)
-        {
-            v[jss::vetoed] = "Obsolete";
-        }
-        else
-        {
-            v[jss::vetoed] = fs.vote == AmendmentVote::down;
-        }
-    }
     v[jss::enabled] = fs.enabled;
+    v[jss::obsolete] = fs.vote == AmendmentVote::obsolete;
+    if (isAdmin)
+    {
+        v[jss::vetoed] = fs.vote == AmendmentVote::down || fs.vote == AmendmentVote::obsolete;
+    }
 
     if (!fs.enabled && lastVote_ && isAdmin)
     {