feat: Make VaultID conditional on LoanBrokerSet

include/xrpl/protocol/detail/transactions.macro

@@ -955,7 +955,7 @@ TRANSACTION(ttLOAN_BROKER_SET, 74, LoanBrokerSet,
     Delegation::NotDelegable,
     featureLendingProtocol,
     CreatePseudoAcct | MayAuthorizeMpt, ({
-    {sfVaultID, SoeRequired},
+    {sfVaultID, SoeOptional},
     {sfLoanBrokerID, SoeOptional},
     {sfData, SoeOptional},
     {sfManagementFeeRate, SoeOptional},

include/xrpl/protocol_autogen/transactions/LoanBrokerSet.h

@@ -48,14 +48,29 @@ class LoanBrokerSet : public TransactionBase
     // Transaction-specific field getters
 
     /**
-     * @brief Get sfVaultID (SoeRequired)
-     * @return The field value.
+     * @brief Get sfVaultID (SoeOptional)
+     * @return The field value, or std::nullopt if not present.
      */
     [[nodiscard]]
-    SF_UINT256::type::value_type
+    protocol_autogen::Optional<SF_UINT256::type::value_type>
     getVaultID() const
     {
-        return this->tx_->at(sfVaultID);
+        if (hasVaultID())
+        {
+            return this->tx_->at(sfVaultID);
+        }
+        return std::nullopt;
+    }
+
+    /**
+     * @brief Check if sfVaultID is present.
+     * @return True if the field is present, false otherwise.
+     */
+    [[nodiscard]]
+    bool
+    hasVaultID() const
+    {
+        return this->tx_->isFieldPresent(sfVaultID);
     }
 
     /**
@@ -228,17 +243,15 @@ class LoanBrokerSetBuilder : public TransactionBuilderBase<LoanBrokerSetBuilder>
     /**
      * @brief Construct a new LoanBrokerSetBuilder with required fields.
      * @param account The account initiating the transaction.
-     * @param vaultID The sfVaultID field value.
      * @param sequence Optional sequence number for the transaction.
      * @param fee Optional fee for the transaction.
      */
     LoanBrokerSetBuilder(SF_ACCOUNT::type::value_type account,
-                     std::decay_t<typename SF_UINT256::type::value_type> const& vaultID,                    std::optional<SF_UINT32::type::value_type> sequence = std::nullopt,
+                    std::optional<SF_UINT32::type::value_type> sequence = std::nullopt,
                     std::optional<SF_AMOUNT::type::value_type> fee = std::nullopt
 )
         : TransactionBuilderBase<LoanBrokerSetBuilder>(ttLOAN_BROKER_SET, account, sequence, fee)
     {
-        setVaultID(vaultID);
     }
 
     /**
@@ -258,7 +271,7 @@ class LoanBrokerSetBuilder : public TransactionBuilderBase<LoanBrokerSetBuilder>
     /** @brief Transaction-specific field setters */
 
     /**
-     * @brief Set sfVaultID (SoeRequired)
+     * @brief Set sfVaultID (SoeOptional)
      * @return Reference to this builder for method chaining.
      */
     LoanBrokerSetBuilder&

src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp

@@ -1,14 +1,19 @@
 #include <xrpl/tx/transactors/lending/LoanBrokerSet.h>
 
+#include <xrpl/basics/Expected.h>
 #include <xrpl/basics/Log.h>
 #include <xrpl/basics/Number.h>
+#include <xrpl/basics/base_uint.h>
 #include <xrpl/beast/utility/Zero.h>
+#include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/ledger/View.h>
 #include <xrpl/ledger/helpers/AccountRootHelpers.h>
 #include <xrpl/ledger/helpers/LendingHelpers.h>
 #include <xrpl/ledger/helpers/TokenHelpers.h>
+#include <xrpl/protocol/AccountID.h>
 #include <xrpl/protocol/Asset.h>
+#include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
@@ -49,7 +54,9 @@
     if (!validNumericRange(tx[~sfDebtMaximum], Number(kMaxMpTokenAmount), Number(0)))
         return temINVALID;
 
-    if (tx.isFieldPresent(sfLoanBrokerID))
+    auto const isLoanBrokerUpdate = tx.isFieldPresent(sfLoanBrokerID);
+
+    if (isLoanBrokerUpdate)
     {
         // Fixed fields can not be specified if we're modifying an existing
         // LoanBroker Object
@@ -61,9 +68,27 @@
             return temINVALID;
     }
 
-    if (auto const vaultID = tx.at(~sfVaultID))
+    // Amendment-specific field presence rules
+    if (ctx.rules.enabled(featureLendingProtocolV1_1))
+    {
+        if (isLoanBrokerUpdate)
+        {
+            if (tx.isFieldPresent(sfVaultID))
+                return temINVALID;
+        }
+        else
+        {
+            if (!tx.isFieldPresent(sfVaultID) || tx[sfVaultID] == beast::kZero)
+                return temINVALID;
+        }
+    }
+    else
     {
-        if (*vaultID == beast::kZero)
+        // Pre-amendment: VaultID was soeREQUIRED, must always be present
+        if (!tx.isFieldPresent(sfVaultID))
+            return temINVALID;
+
+        if (tx[sfVaultID] == beast::kZero)
             return temINVALID;
     }
 
@@ -88,77 +113,157 @@
     return kValueFields;
 }
 
-TER
-LoanBrokerSet::preclaim(PreclaimContext const& ctx)
+/** Read and validate a vault, checking existence and ownership.
+ *
+ * @param ctx The preclaim context.
+ * @param account The expected vault owner.
+ * @param id The vault ID to look up.
+ * @return The vault SLE on success, or a TER error.
+ */
+[[nodiscard]] static Expected<std::shared_ptr<SLE const>, TER>
+readVault(PreclaimContext const& ctx, AccountID const& account, uint256 const& id)
 {
-    auto const& tx = ctx.tx;
-
-    auto const account = tx[sfAccount];
-    auto const vaultID = tx[sfVaultID];
-
-    auto const sleVault = ctx.view.read(keylet::vault(vaultID));
-    if (!sleVault)
+    auto const sle = ctx.view.read(keylet::vault(id));
+    if (!sle)
     {
         JLOG(ctx.j.warn()) << "Vault does not exist.";
-        return tecNO_ENTRY;
+        return Unexpected(tecNO_ENTRY);
     }
-    Asset const asset = sleVault->at(sfAsset);
-
-    if (account != sleVault->at(sfOwner))
+    if (account != sle->at(sfOwner))
     {
         JLOG(ctx.j.warn()) << "Account is not the owner of the Vault.";
-        return tecNO_PERMISSION;
+        return Unexpected(tecNO_PERMISSION);
     }
+    return sle;
+}
 
-    if (auto const brokerID = tx[~sfLoanBrokerID])
-    {
-        // Updating an existing Broker
+/** Preclaim validation for updating an existing LoanBroker.
+ *
+ * @param ctx The preclaim context.
+ * @param account The transaction submitter.
+ * @param brokerID The LoanBroker ID to update.
+ * @return The vault SLE on success, or a TER error.
+ */
+[[nodiscard]] static Expected<std::shared_ptr<SLE const>, TER>
+preclaimUpdate(PreclaimContext const& ctx, AccountID const& account, uint256 const& brokerID)
+{
+    auto const& tx = ctx.tx;
+    bool const fixEnabled = ctx.view.rules().enabled(featureLendingProtocolV1_1);
 
-        auto const sleBroker = ctx.view.read(keylet::loanbroker(*brokerID));
+    std::shared_ptr<SLE const> sleBroker;
+    std::shared_ptr<SLE const> sleVault;
+
+    if (fixEnabled)
+    {
+        // Post-amendment: VaultID is not in the tx, read it from broker
+        sleBroker = ctx.view.read(keylet::loanbroker(brokerID));
         if (!sleBroker)
         {
             JLOG(ctx.j.warn()) << "LoanBroker does not exist.";
-            return tecNO_ENTRY;
+            return Unexpected(tecNO_ENTRY);
         }
-        if (vaultID != sleBroker->at(sfVaultID))
-        {
-            JLOG(ctx.j.warn()) << "Can not change VaultID on an existing LoanBroker.";
-            return tecNO_PERMISSION;
-        }
-        if (account != sleBroker->at(sfOwner))
+
+        auto const vault = readVault(ctx, account, sleBroker->at(sfVaultID));
+        if (!vault)
+            return vault;
+        sleVault = *vault;
+    }
+    else
+    {
+        XRPL_ASSERT(
+            tx.isFieldPresent(sfVaultID),
+            "xrpl::LoanBrokerSet::preclaimUpdate : VaultID is present in the transaction");
+        // Pre-amendment: vault is validated before broker to preserve
+        // the original error ordering for historical transaction replay.
+        auto const vault = readVault(ctx, account, tx[sfVaultID]);
+        if (!vault)
+            return vault;
+        sleVault = *vault;
+
+        sleBroker = ctx.view.read(keylet::loanbroker(brokerID));
+        if (!sleBroker)
         {
-            JLOG(ctx.j.warn()) << "Account is not the owner of the LoanBroker.";
-            return tecNO_PERMISSION;
+            JLOG(ctx.j.warn()) << "LoanBroker does not exist.";
+            return Unexpected(tecNO_ENTRY);
         }
-
-        if (auto const debtMax = tx[~sfDebtMaximum])
+        if (tx[sfVaultID] != sleBroker->at(sfVaultID))
         {
-            // Can't reduce the debt maximum below the current total debt
-            auto const currentDebtTotal = sleBroker->at(sfDebtTotal);
-            if (*debtMax != 0 && *debtMax < currentDebtTotal)
-            {
-                JLOG(ctx.j.warn()) << "Cannot reduce DebtMaximum below current DebtTotal.";
-                return tecLIMIT_EXCEEDED;
-            }
+            JLOG(ctx.j.warn()) << "Can not change VaultID on an existing LoanBroker.";
+            return Unexpected(tecNO_PERMISSION);
         }
     }
-    else
+
+    XRPL_ASSERT(sleVault, "xrpl::LoanBrokerSet::preclaimUpdate : sleVault is initialized");
+
+    if (account != sleBroker->at(sfOwner))
     {
-        if (auto const ter = canAddHolding(ctx.view, asset))
-            return ter;
+        JLOG(ctx.j.warn()) << "Account is not the owner of the LoanBroker.";
+        return Unexpected(tecNO_PERMISSION);
+    }
 
-        if (auto const ter = checkFrozen(ctx.view, sleVault->at(sfAccount), sleVault->at(sfAsset)))
+    if (auto const debtMax = tx[~sfDebtMaximum])
+    {
+        auto const currentDebtTotal = sleBroker->at(sfDebtTotal);
+        if (*debtMax != 0 && *debtMax < currentDebtTotal)
         {
-            JLOG(ctx.j.warn()) << "Vault pseudo-account is frozen.";
-            return ter;
+            JLOG(ctx.j.warn()) << "Cannot reduce DebtMaximum below current DebtTotal.";
+            return Unexpected(tecLIMIT_EXCEEDED);
         }
     }
 
+    return sleVault;
+}
+
+/** Preclaim validation for creating a new LoanBroker.
+ *
+ * @param ctx The preclaim context.
+ * @param account The transaction submitter (vault owner).
+ * @return The vault SLE on success, or a TER error.
+ */
+[[nodiscard]] static Expected<std::shared_ptr<SLE const>, TER>
+preclaimCreate(PreclaimContext const& ctx, AccountID const& account)
+{
+    XRPL_ASSERT(
+        ctx.tx.isFieldPresent(sfVaultID),
+        "xrpl::LoanBrokerSet::preclaimCreate : VaultID is present in the transaction");
+    auto const vault = readVault(ctx, account, ctx.tx[sfVaultID]);
+    if (!vault)
+        return vault;
+    auto const& sleVault = *vault;
+
+    Asset const asset = sleVault->at(sfAsset);
+    if (auto const ter = canAddHolding(ctx.view, asset))
+        return Unexpected(ter);
+
+    if (auto const ter = checkFrozen(ctx.view, sleVault->at(sfAccount), sleVault->at(sfAsset)))
+    {
+        JLOG(ctx.j.warn()) << "Vault pseudo-account is frozen.";
+        return Unexpected(ter);
+    }
+
+    return sleVault;
+}
+
+TER
+LoanBrokerSet::preclaim(PreclaimContext const& ctx)
+{
+    auto const account = ctx.tx[sfAccount];
+
+    auto const maybeVault = [&]() -> Expected<std::shared_ptr<SLE const>, TER> {
+        if (auto const brokerID = ctx.tx[~sfLoanBrokerID])
+            return preclaimUpdate(ctx, account, *brokerID);
+        return preclaimCreate(ctx, account);
+    }();
+
+    if (!maybeVault)
+        return maybeVault.error();
+
     // Check that relevant values can be represented as the vault asset
-    // type. This is mostly only relevant for integral (non-IOU) types
+    // type. This is mostly only relevant for integral (non-IOU) types.
+    Asset const asset = (*maybeVault)->at(sfAsset);
     for (auto const& field : getValueFields())
     {
-        if (auto const value = tx[field]; value && STAmount{asset, *value} != *value)
+        if (auto const value = ctx.tx[field]; value && STAmount{asset, *value} != *value)
         {
             JLOG(ctx.j.warn()) << field.f->getName() << " (" << *value
                                << ") can not be represented as a(n) " << to_string(asset) << ".";