Merge branch 'tapanito/lending-fix-amendment' into tapanito/loan-broker-set

Resolved conflicts in LoanBrokerSet.cpp, transactions.macro, LoanBrokerSet.h,
TestHelpers (h/cpp), and LoanBroker_test.cpp.

Key decisions:
- sfVaultID remains SoeOptional (our change) with new capitalization style
- Amendment-gated preflight logic preserved alongside kZero renames
- testZeroVaultID lambda removed (field is optional on update under V1_1)
- Both testLoanBrokerSetVaultIDAmendment and testCoverPrecisionGuard included
- All k-prefix helper renames from lending-fix-amendment applied

include/xrpl/protocol/detail/features.macro

@@ -15,6 +15,7 @@
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
 
+XRPL_FEATURE(LendingProtocolV1_1,         Supported::No, VoteBehavior::DefaultNo)
 XRPL_FIX    (Cleanup3_2_0,                Supported::Yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(MPTokensV2,                  Supported::No,  VoteBehavior::DefaultNo)
 XRPL_FIX    (Cleanup3_1_3,                Supported::Yes, VoteBehavior::DefaultYes)

include/xrpl/protocol/detail/transactions.macro

@@ -887,6 +887,7 @@ TRANSACTION(ttVAULT_DELETE, 67, VaultDelete,
     MustDeleteAcct | DestroyMptIssuance | MustModifyVault,
     ({
     {sfVaultID, SoeRequired},
+    {sfMemoData, SoeOptional},
 }))
 
 /** This transaction trades assets for shares with a vault. */
@@ -954,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 @@ public:
     // 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 @@ public:
     /**
      * @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 @@ public:
     /** @brief Transaction-specific field setters */
 
     /**
-     * @brief Set sfVaultID (SoeRequired)
+     * @brief Set sfVaultID (SoeOptional)
      * @return Reference to this builder for method chaining.
      */
     LoanBrokerSetBuilder&

include/xrpl/protocol_autogen/transactions/VaultDelete.h

@@ -57,6 +57,32 @@ public:
     {
         return this->tx_->at(sfVaultID);
     }
+
+    /**
+     * @brief Get sfMemoData (SoeOptional)
+     * @return The field value, or std::nullopt if not present.
+     */
+    [[nodiscard]]
+    protocol_autogen::Optional<SF_VL::type::value_type>
+    getMemoData() const
+    {
+        if (hasMemoData())
+        {
+            return this->tx_->at(sfMemoData);
+        }
+        return std::nullopt;
+    }
+
+    /**
+     * @brief Check if sfMemoData is present.
+     * @return True if the field is present, false otherwise.
+     */
+    [[nodiscard]]
+    bool
+    hasMemoData() const
+    {
+        return this->tx_->isFieldPresent(sfMemoData);
+    }
 };
 
 /**
@@ -112,6 +138,17 @@ public:
         return *this;
     }
 
+    /**
+     * @brief Set sfMemoData (SoeOptional)
+     * @return Reference to this builder for method chaining.
+     */
+    VaultDeleteBuilder&
+    setMemoData(std::decay_t<typename SF_VL::type::value_type> const& value)
+    {
+        object_[sfMemoData] = value;
+        return *this;
+    }
+
     /**
      * @brief Build and return the VaultDelete wrapper.
      * @param publicKey The public key for signing.

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 @@ LoanBrokerSet::preflight(PreflightContext const& ctx)
     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 @@ LoanBrokerSet::preflight(PreflightContext const& ctx)
             return temINVALID;
     }
 
-    if (auto const vaultID = tx.at(~sfVaultID))
+    // Amendment-specific field presence rules
+    if (ctx.rules.enabled(featureLendingProtocolV1_1))
     {
-        if (*vaultID == beast::kZero)
+        if (isLoanBrokerUpdate)
+        {
+            if (tx.isFieldPresent(sfVaultID))
+                return temINVALID;
+        }
+        else
+        {
+            if (!tx.isFieldPresent(sfVaultID) || tx[sfVaultID] == beast::kZero)
+                return temINVALID;
+        }
+    }
+    else
+    {
+        // 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 @@ LoanBrokerSet::getValueFields()
     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])
+/** 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);
+
+    std::shared_ptr<SLE const> sleBroker;
+    std::shared_ptr<SLE const> sleVault;
+
+    if (fixEnabled)
     {
-        // Updating an existing Broker
-
-        auto const sleBroker = ctx.view.read(keylet::loanbroker(*brokerID));
+        // 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;
-        }
-        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))
-        {
-            JLOG(ctx.j.warn()) << "Account is not the owner of the LoanBroker.";
-            return tecNO_PERMISSION;
+            return Unexpected(tecNO_ENTRY);
         }
 
-        if (auto const debtMax = tx[~sfDebtMaximum])
-        {
-            // 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;
-            }
-        }
+        auto const vault = readVault(ctx, account, sleBroker->at(sfVaultID));
+        if (!vault)
+            return vault;
+        sleVault = *vault;
     }
     else
     {
-        if (auto const ter = canAddHolding(ctx.view, asset))
-            return ter;
+        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;
 
-        if (auto const ter = checkFrozen(ctx.view, sleVault->at(sfAccount), sleVault->at(sfAsset)))
+        sleBroker = ctx.view.read(keylet::loanbroker(brokerID));
+        if (!sleBroker)
         {
-            JLOG(ctx.j.warn()) << "Vault pseudo-account is frozen.";
-            return ter;
+            JLOG(ctx.j.warn()) << "LoanBroker does not exist.";
+            return Unexpected(tecNO_ENTRY);
+        }
+        if (tx[sfVaultID] != sleBroker->at(sfVaultID))
+        {
+            JLOG(ctx.j.warn()) << "Can not change VaultID on an existing LoanBroker.";
+            return Unexpected(tecNO_PERMISSION);
         }
     }
 
+    XRPL_ASSERT(sleVault, "xrpl::LoanBrokerSet::preclaimUpdate : sleVault is initialized");
+
+    if (account != sleBroker->at(sfOwner))
+    {
+        JLOG(ctx.j.warn()) << "Account is not the owner of the LoanBroker.";
+        return Unexpected(tecNO_PERMISSION);
+    }
+
+    if (auto const debtMax = tx[~sfDebtMaximum])
+    {
+        auto const currentDebtTotal = sleBroker->at(sfDebtTotal);
+        if (*debtMax != 0 && *debtMax < currentDebtTotal)
+        {
+            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) << ".";

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

@@ -7,8 +7,10 @@
 #include <xrpl/ledger/helpers/MPTokenHelpers.h>
 #include <xrpl/ledger/helpers/TokenHelpers.h>
 #include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/MPTIssue.h>
+#include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STLedgerEntry.h>
 #include <xrpl/protocol/STNumber.h>  // IWYU pragma: keep
@@ -28,6 +30,14 @@ VaultDelete::preflight(PreflightContext const& ctx)
         return temMALFORMED;
     }
 
+    if (ctx.tx.isFieldPresent(sfMemoData) && !ctx.rules.enabled(featureLendingProtocolV1_1))
+        return temDISABLED;
+
+    // The sfMemoData field is an optional field used to record the deletion reason.
+    if (auto const data = ctx.tx[~sfMemoData];
+        data && !validDataLength(data, kMaxDataPayloadLength))
+        return temMALFORMED;
+
     return tesSUCCESS;
 }
 

src/test/app/LoanBroker_test.cpp

@@ -463,7 +463,7 @@ class LoanBroker_test : public beast::unit_test::Suite
             }
 
             // no-op
-            env(set(alice, vault.vaultID), kLoanBrokerId(keylet.key));
+            env(set(alice), kLoanBrokerId(keylet.key));
             env.close();
 
             // Make modifications to the broker
@@ -479,10 +479,7 @@ class LoanBroker_test : public beast::unit_test::Suite
             // Verify that fields get removed when set to default values
             // Debt maximum: explicit 0
             // Data: explicit empty
-            env(set(alice, vault.vaultID),
-                kLoanBrokerId(broker->key()),
-                kDebtMaximum(Number(0)),
-                kData(""));
+            env(set(alice), kLoanBrokerId(broker->key()), kDebtMaximum(Number(0)), kData(""));
             env.close();
 
             // Check the updated fields
@@ -739,26 +736,25 @@ class LoanBroker_test : public beast::unit_test::Suite
                     auto const nextKeylet = keylet::loanbroker(alice.id(), env.seq(alice));
 
                     // fields that can't be changed
+
                     // LoanBrokerID
-                    env(set(alice, vault.vaultID), kLoanBrokerId(nextKeylet.key), Ter(tecNO_ENTRY));
-                    // VaultID
-                    env(set(alice, nextKeylet.key), kLoanBrokerId(broker->key()), Ter(tecNO_ENTRY));
+                    env(set(alice), kLoanBrokerId(nextKeylet.key), Ter(tecNO_ENTRY));
+                    // VaultID (rejected in preflight when amendment is active)
+                    env(set(alice, nextKeylet.key), kLoanBrokerId(broker->key()), Ter(temINVALID));
                     // Owner
-                    env(set(evan, vault.vaultID),
-                        kLoanBrokerId(broker->key()),
-                        Ter(tecNO_PERMISSION));
+                    env(set(evan), kLoanBrokerId(broker->key()), Ter(tecNO_PERMISSION));
                     // ManagementFeeRate
-                    env(set(alice, vault.vaultID),
+                    env(set(alice),
                         kLoanBrokerId(broker->key()),
                         kManagementFeeRate(kMaxManagementFeeRate),
                         Ter(temINVALID));
                     // CoverRateMinimum
-                    env(set(alice, vault.vaultID),
+                    env(set(alice),
                         kLoanBrokerId(broker->key()),
                         kCoverRateMinimum(kMaxManagementFeeRate),
                         Ter(temINVALID));
                     // CoverRateLiquidation
-                    env(set(alice, vault.vaultID),
+                    env(set(alice),
                         kLoanBrokerId(broker->key()),
                         kCoverRateLiquidation(kMaxManagementFeeRate),
                         Ter(temINVALID));
@@ -766,20 +762,20 @@ class LoanBroker_test : public beast::unit_test::Suite
                     // fields that can be changed
                     testData = "Test Data 1234";
                     // Bad data: too long
-                    env(set(alice, vault.vaultID),
+                    env(set(alice),
                         kLoanBrokerId(broker->key()),
                         kData(std::string(kMaxDataPayloadLength + 1, 'W')),
                         Ter(temINVALID));
 
                     // Bad debt maximum
-                    env(set(alice, vault.vaultID),
+                    env(set(alice),
                         kLoanBrokerId(broker->key()),
                         kDebtMaximum(Number(-175, -1)),
                         Ter(temINVALID));
                     Number debtMax{175, -1};
                     if (vault.asset.integral())
                     {
-                        env(set(alice, vault.vaultID),
+                        env(set(alice),
                             kLoanBrokerId(broker->key()),
                             kData(testData),
                             kDebtMaximum(debtMax),
@@ -787,7 +783,7 @@ class LoanBroker_test : public beast::unit_test::Suite
                         roundToAsset(vault.asset, debtMax);
                     }
                     // Data & Debt maximum
-                    env(set(alice, vault.vaultID),
+                    env(set(alice),
                         kLoanBrokerId(broker->key()),
                         kData(testData),
                         kDebtMaximum(debtMax));
@@ -833,7 +829,7 @@ class LoanBroker_test : public beast::unit_test::Suite
                 },
                 [&](SLE::const_ref broker) {
                     // Reset Data & Debt maximum to default values
-                    env(set(alice, vault.vaultID),
+                    env(set(alice),
                         kLoanBrokerId(broker->key()),
                         kData(""),
                         kDebtMaximum(Number(0)));
@@ -911,17 +907,6 @@ class LoanBroker_test : public beast::unit_test::Suite
             // test
             env(jv, Txflags(tfFullyCanonicalSig), Ter(temINVALID));
         };
-        auto testZeroVaultID = [&](auto&& getTxJv) {
-            auto jv = getTxJv();
-            // empty broker ID
-            jv[sfVaultID] = "";
-            env(jv, Ter(temINVALID));
-            // 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));
-        };
 
         if (brokerTest == LoanBrokerTest::CoverDeposit)
         {
@@ -1074,13 +1059,8 @@ class LoanBroker_test : public beast::unit_test::Suite
         if (brokerTest == LoanBrokerTest::Set)
         {
             // preflight: temINVALID (empty/zero broker id)
-            testZeroBrokerID([&]() {
-                return env.json(set(alice, vaultInfo.vaultID), kLoanBrokerId(brokerKeylet.key));
-            });
-            // preflight: temINVALID (empty/zero vault id)
-            testZeroVaultID([&]() {
-                return env.json(set(alice, vaultInfo.vaultID), kLoanBrokerId(brokerKeylet.key));
-            });
+            testZeroBrokerID(
+                [&]() { return env.json(set(alice), kLoanBrokerId(brokerKeylet.key)); });
 
             if (asset.holds<Issue>())
             {
@@ -1431,7 +1411,7 @@ class LoanBroker_test : public beast::unit_test::Suite
         BEAST_EXPECT(broker->at(sfDebtTotal) == 50);
         auto debtTotal = broker->at(sfDebtTotal);
 
-        auto tx2 = set(alice, vaultInfo.vaultID);
+        auto tx2 = set(alice);
         tx2[sfLoanBrokerID] = to_string(brokerKeylet.key);
         tx2[sfDebtMaximum] = debtTotal - 1;
         env(tx2, Ter(tecLIMIT_EXCEEDED));
@@ -2240,10 +2220,207 @@ class LoanBroker_test : public beast::unit_test::Suite
         runTestCases(all_ - fixCleanup3_2_0);
     }
 
+    void
+    testLoanBrokerSetVaultIDAmendment()
+    {
+        using namespace jtx;
+        using namespace loanBroker;
+
+        Account const issuer{"issuer"};
+        Account const alice{"alice"};
+        Account const evan{"evan"};
+
+        // Helper to set up a vault and broker for testing
+        auto const setup = [&](Env& env) {
+            Vault const vault{env};
+            env.fund(XRP(100'000), issuer, alice, evan);
+            env.close();
+
+            env(trust(alice, issuer["IOU"](1'000'000)));
+            env.close();
+            PrettyAsset const asset = issuer["IOU"];
+            env(pay(issuer, alice, asset(100'000)));
+            env.close();
+
+            auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset});
+            env(tx);
+            env.close();
+
+            env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50)}));
+            env.close();
+
+            auto const brokerKeylet = keylet::loanbroker(alice.id(), env.seq(alice));
+            env(set(alice, vaultKeylet.key));
+            env.close();
+
+            struct Result
+            {
+                uint256 vaultID;
+                Keylet brokerKeylet;
+            };
+            return Result{.vaultID = vaultKeylet.key, .brokerKeylet = brokerKeylet};
+        };
+
+        // Post-amendment: VaultID must not be present on update
+        {
+            testcase("LoanBrokerSet post-amendment: VaultID rejected on update");
+            Env env(*this);
+            BEAST_EXPECT(env.enabled(featureLendingProtocolV1_1));
+            auto const [vaultID, brokerKL] = setup(env);
+
+            // Update with VaultID → temINVALID
+            env(set(alice, vaultID), kLoanBrokerId(brokerKL.key), Ter(temINVALID));
+
+            // Update without VaultID succeeds
+            env(set(alice), kLoanBrokerId(brokerKL.key), kData("post-amendment update"));
+            env.close();
+
+            auto const broker = env.le(brokerKL);
+            BEAST_EXPECT(broker);
+            if (broker)
+                BEAST_EXPECT(checkVL(broker->at(sfData), "post-amendment update"));
+        }
+
+        // Post-amendment: Create requires VaultID
+        {
+            testcase("LoanBrokerSet post-amendment: VaultID required on create");
+            Env env(*this);
+            BEAST_EXPECT(env.enabled(featureLendingProtocolV1_1));
+            env.fund(XRP(100'000), issuer, alice);
+            env.close();
+
+            // Create without VaultID → temINVALID
+            env(set(alice), Ter(temINVALID));
+
+            // Create with zero VaultID → temINVALID
+            env(set(alice, uint256{}), Ter(temINVALID));
+        }
+
+        // Post-amendment: Update by wrong owner → tecNO_PERMISSION
+        {
+            testcase("LoanBrokerSet post-amendment: wrong owner rejected");
+            Env env(*this);
+            auto const [vaultID, brokerKL] = setup(env);
+
+            env(set(evan), kLoanBrokerId(brokerKL.key), Ter(tecNO_PERMISSION));
+        }
+
+        // Post-amendment: Update non-existent broker → tecNO_ENTRY
+        {
+            testcase("LoanBrokerSet post-amendment: non-existent broker");
+            Env env(*this);
+            env.fund(XRP(100'000), alice);
+            env.close();
+
+            env(set(alice), kLoanBrokerId(uint256{1}), Ter(tecNO_ENTRY));
+        }
+
+        // Pre-amendment: VaultID required on both create and update
+        {
+            testcase("LoanBrokerSet pre-amendment: VaultID required on update");
+            Env env(*this);
+            env.disableFeature(featureLendingProtocolV1_1);
+            BEAST_EXPECT(!env.enabled(featureLendingProtocolV1_1));
+            auto const [vaultID, brokerKL] = setup(env);
+
+            // Update without VaultID → temINVALID (pre-amendment requires it)
+            env(set(alice), kLoanBrokerId(brokerKL.key), Ter(temINVALID));
+
+            // Update with matching VaultID succeeds (old behavior)
+            env(set(alice, vaultID), kLoanBrokerId(brokerKL.key), kData("pre-amendment update"));
+            env.close();
+
+            auto const broker = env.le(brokerKL);
+            BEAST_EXPECT(broker);
+            if (broker)
+                BEAST_EXPECT(checkVL(broker->at(sfData), "pre-amendment update"));
+        }
+
+        // Pre-amendment: mismatched VaultID on update → tecNO_PERMISSION
+        {
+            testcase("LoanBrokerSet pre-amendment: mismatched VaultID");
+            Env env(*this);
+            env.disableFeature(featureLendingProtocolV1_1);
+
+            Vault const vault{env};
+            env.fund(XRP(100'000), issuer, alice);
+            env.close();
+            env(trust(alice, issuer["IOU"](1'000'000)));
+            env.close();
+            PrettyAsset const asset = issuer["IOU"];
+            env(pay(issuer, alice, asset(100'000)));
+            env.close();
+
+            // Create two vaults
+            auto [tx1, vaultKL1] = vault.create({.owner = alice, .asset = asset});
+            env(tx1);
+            env.close();
+            auto [tx2, vaultKL2] = vault.create({.owner = alice, .asset = asset});
+            env(tx2);
+            env.close();
+
+            env(vault.deposit({.depositor = alice, .id = vaultKL1.key, .amount = asset(50)}));
+            env.close();
+
+            auto const brokerKL = keylet::loanbroker(alice.id(), env.seq(alice));
+            env(set(alice, vaultKL1.key));
+            env.close();
+
+            // Update with different vault → tecNO_PERMISSION
+            env(set(alice, vaultKL2.key), kLoanBrokerId(brokerKL.key), Ter(tecNO_PERMISSION));
+        }
+
+        // Pre-amendment: non-existent vault on update → tecNO_ENTRY
+        {
+            testcase("LoanBrokerSet pre-amendment: non-existent vault on update");
+            Env env(*this);
+            env.disableFeature(featureLendingProtocolV1_1);
+            auto const [vaultID, brokerKL] = setup(env);
+
+            // Update with a VaultID that doesn't exist
+            env(set(alice, uint256{1}), kLoanBrokerId(brokerKL.key), Ter(tecNO_ENTRY));
+        }
+
+        // Pre-amendment: Create without VaultID → temINVALID
+        {
+            testcase("LoanBrokerSet pre-amendment: create requires VaultID");
+            Env env(*this);
+            env.disableFeature(featureLendingProtocolV1_1);
+            env.fund(XRP(100'000), issuer, alice);
+            env.close();
+
+            env(set(alice), Ter(temINVALID));
+        }
+
+        // Pre-amendment: immutable fields still rejected on update
+        {
+            testcase("LoanBrokerSet pre-amendment: immutable fields on update");
+            Env env(*this);
+            env.disableFeature(featureLendingProtocolV1_1);
+            auto const [vaultID, brokerKL] = setup(env);
+
+            env(set(alice, vaultID),
+                kLoanBrokerId(brokerKL.key),
+                kManagementFeeRate(TenthBips16(1)),
+                Ter(temINVALID));
+        }
+
+        // Pre-amendment: zero VaultID on update → temINVALID
+        {
+            testcase("LoanBrokerSet pre-amendment: zero VaultID on update");
+            Env env(*this);
+            env.disableFeature(featureLendingProtocolV1_1);
+            auto const [vaultID, brokerKL] = setup(env);
+
+            env(set(alice, uint256{}), kLoanBrokerId(brokerKL.key), Ter(temINVALID));
+        }
+    }
+
 public:
     void
     run() override
     {
+        testLoanBrokerSetVaultIDAmendment();
         testCoverPrecisionGuard();
 
         testLoanBrokerSetDebtMaximum();

src/test/app/Vault_test.cpp

@@ -8027,6 +8027,63 @@ class Vault_test : public beast::unit_test::Suite
         }
     }
 
+    void
+    testVaultDeleteData()
+    {
+        using namespace test::jtx;
+
+        Env env{*this};
+
+        Account const owner{"owner"};
+        env.fund(XRP(1'000'000), owner);
+        env.close();
+
+        Vault const vault{env};
+
+        auto const keylet = keylet::vault(owner.id(), 1);
+        auto delTx = vault.del({.owner = owner, .id = keylet.key});
+
+        // Test VaultDelete with featureLendingProtocolV1_1 disabled
+        // Transaction fails if the data field is provided
+        {
+            testcase("VaultDelete data featureLendingProtocolV1_1 disabled");
+            env.disableFeature(featureLendingProtocolV1_1);
+            delTx[sfMemoData] = strHex(std::string(kMaxDataPayloadLength, 'A'));
+            env(delTx, Ter(temDISABLED));
+            env.close();
+            env.enableFeature(featureLendingProtocolV1_1);
+        }
+
+        // Transaction fails if the data field is too large
+        {
+            testcase("VaultDelete data featureLendingProtocolV1_1 enabled data too large");
+            delTx[sfMemoData] = strHex(std::string(kMaxDataPayloadLength + 1, 'A'));
+            env(delTx, Ter(temMALFORMED));
+            env.close();
+        }
+
+        // Transaction fails if the data field is set, but is empty
+        {
+            testcase("VaultDelete data featureLendingProtocolV1_1 enabled data empty");
+            delTx[sfMemoData] = strHex(std::string(0, 'A'));
+            env(delTx, Ter(temMALFORMED));
+            env.close();
+        }
+
+        {
+            testcase("VaultDelete data featureLendingProtocolV1_1 enabled data valid");
+            PrettyAsset const xrpAsset = xrpIssue();
+            auto [tx, keylet] = vault.create({.owner = owner, .asset = xrpAsset});
+            env(tx, Ter(tesSUCCESS));
+            env.close();
+            // Recreate the transaction as the vault keylet changed
+            auto delTx = vault.del({.owner = owner, .id = keylet.key});
+            delTx[sfMemoData] = strHex(std::string(kMaxDataPayloadLength, 'A'));
+            env(delTx, Ter(tesSUCCESS));
+            env.close();
+        }
+    }
+
 public:
     void
     run() override
@@ -8055,6 +8112,7 @@ public:
         testVaultClawbackAssets();
         testVaultEscrowedMPT();
         testAssetsMaximum();
+        testVaultDeleteData();
         testBug6LimitBypassWithShares();
         testRemoveEmptyHoldingLockedAmount();
 

src/test/jtx/TestHelpers.h

@@ -832,7 +832,9 @@ checkMetrics(
 namespace loanBroker {
 
 json::Value
-set(AccountID const& account, uint256 const& vaultId, std::uint32_t flags = 0);
+set(AccountID const& account,
+    std::optional<uint256> const& vaultId = std::nullopt,
+    std::uint32_t flags = 0);
 
 // Use "del" because "delete" is a reserved word in C++.
 json::Value

src/test/jtx/impl/TestHelpers.cpp

@@ -728,12 +728,13 @@ issueHelperMPT(IssuerArgs const& args)
 namespace loanBroker {
 
 json::Value
-set(AccountID const& account, uint256 const& vaultId, uint32_t flags)
+set(AccountID const& account, std::optional<uint256> const& vaultId, uint32_t flags)
 {
     json::Value jv;
     jv[sfTransactionType] = jss::LoanBrokerSet;
     jv[sfAccount] = to_string(account);
-    jv[sfVaultID] = to_string(vaultId);
+    if (vaultId)
+        jv[sfVaultID] = to_string(*vaultId);
     jv[sfFlags] = flags;
     return jv;
 }

src/tests/libxrpl/protocol_autogen/transactions/LoanBrokerSetTests.cpp

@@ -39,12 +39,12 @@ TEST(TransactionsLoanBrokerSetTests, BuilderSettersRoundTrip)
 
     LoanBrokerSetBuilder builder{
         accountValue,
-        vaultIDValue,
         sequenceValue,
         feeValue
     };
 
     // Set optional fields
+    builder.setVaultID(vaultIDValue);
     builder.setLoanBrokerID(loanBrokerIDValue);
     builder.setData(dataValue);
     builder.setManagementFeeRate(managementFeeRateValue);
@@ -67,13 +67,15 @@ TEST(TransactionsLoanBrokerSetTests, BuilderSettersRoundTrip)
     EXPECT_EQ(tx.getFee(), feeValue);
 
     // Verify required fields
+    // Verify optional fields
     {
         auto const& expected = vaultIDValue;
-        auto const actual = tx.getVaultID();
-        expectEqualField(expected, actual, "sfVaultID");
+        auto const actualOpt = tx.getVaultID();
+        ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfVaultID should be present";
+        expectEqualField(expected, *actualOpt, "sfVaultID");
+        EXPECT_TRUE(tx.hasVaultID());
     }
 
-    // Verify optional fields
     {
         auto const& expected = loanBrokerIDValue;
         auto const actualOpt = tx.getLoanBrokerID();
@@ -149,11 +151,11 @@ TEST(TransactionsLoanBrokerSetTests, BuilderFromStTxRoundTrip)
     // Build an initial transaction
     LoanBrokerSetBuilder initialBuilder{
         accountValue,
-        vaultIDValue,
         sequenceValue,
         feeValue
     };
 
+    initialBuilder.setVaultID(vaultIDValue);
     initialBuilder.setLoanBrokerID(loanBrokerIDValue);
     initialBuilder.setData(dataValue);
     initialBuilder.setManagementFeeRate(managementFeeRateValue);
@@ -177,13 +179,14 @@ TEST(TransactionsLoanBrokerSetTests, BuilderFromStTxRoundTrip)
     EXPECT_EQ(rebuiltTx.getFee(), feeValue);
 
     // Verify required fields
+    // Verify optional fields
     {
         auto const& expected = vaultIDValue;
-        auto const actual = rebuiltTx.getVaultID();
-        expectEqualField(expected, actual, "sfVaultID");
+        auto const actualOpt = rebuiltTx.getVaultID();
+        ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfVaultID should be present";
+        expectEqualField(expected, *actualOpt, "sfVaultID");
     }
 
-    // Verify optional fields
     {
         auto const& expected = loanBrokerIDValue;
         auto const actualOpt = rebuiltTx.getLoanBrokerID();
@@ -269,11 +272,9 @@ TEST(TransactionsLoanBrokerSetTests, OptionalFieldsReturnNullopt)
     auto const feeValue = canonical_AMOUNT();
 
     // Transaction-specific required field values
-    auto const vaultIDValue = canonical_UINT256();
 
     LoanBrokerSetBuilder builder{
         accountValue,
-        vaultIDValue,
         sequenceValue,
         feeValue
     };
@@ -283,6 +284,8 @@ TEST(TransactionsLoanBrokerSetTests, OptionalFieldsReturnNullopt)
     auto tx = builder.build(publicKey, secretKey);
 
     // Verify optional fields are not present
+    EXPECT_FALSE(tx.hasVaultID());
+    EXPECT_FALSE(tx.getVaultID().has_value());
     EXPECT_FALSE(tx.hasLoanBrokerID());
     EXPECT_FALSE(tx.getLoanBrokerID().has_value());
     EXPECT_FALSE(tx.hasData());

src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp

@@ -30,6 +30,7 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip)
 
     // Transaction-specific field values
     auto const vaultIDValue = canonical_UINT256();
+    auto const memoDataValue = canonical_VL();
 
     VaultDeleteBuilder builder{
         accountValue,
@@ -39,6 +40,7 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip)
     };
 
     // Set optional fields
+    builder.setMemoData(memoDataValue);
 
     auto tx = builder.build(publicKey, secretKey);
 
@@ -62,6 +64,14 @@ TEST(TransactionsVaultDeleteTests, BuilderSettersRoundTrip)
     }
 
     // Verify optional fields
+    {
+        auto const& expected = memoDataValue;
+        auto const actualOpt = tx.getMemoData();
+        ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfMemoData should be present";
+        expectEqualField(expected, *actualOpt, "sfMemoData");
+        EXPECT_TRUE(tx.hasMemoData());
+    }
+
 }
 
 // 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper,
@@ -79,6 +89,7 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip)
 
     // Transaction-specific field values
     auto const vaultIDValue = canonical_UINT256();
+    auto const memoDataValue = canonical_VL();
 
     // Build an initial transaction
     VaultDeleteBuilder initialBuilder{
@@ -88,6 +99,7 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip)
         feeValue
     };
 
+    initialBuilder.setMemoData(memoDataValue);
 
     auto initialTx = initialBuilder.build(publicKey, secretKey);
 
@@ -112,6 +124,13 @@ TEST(TransactionsVaultDeleteTests, BuilderFromStTxRoundTrip)
     }
 
     // Verify optional fields
+    {
+        auto const& expected = memoDataValue;
+        auto const actualOpt = rebuiltTx.getMemoData();
+        ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfMemoData should be present";
+        expectEqualField(expected, *actualOpt, "sfMemoData");
+    }
+
 }
 
 // 3) Verify wrapper throws when constructed from wrong transaction type.
@@ -142,5 +161,35 @@ TEST(TransactionsVaultDeleteTests, BuilderThrowsOnWrongTxType)
     EXPECT_THROW(VaultDeleteBuilder{wrongTx.getSTTx()}, std::runtime_error);
 }
 
+// 5) Build with only required fields and verify optional fields return nullopt.
+TEST(TransactionsVaultDeleteTests, OptionalFieldsReturnNullopt)
+{
+    // Generate a deterministic keypair for signing
+    auto const [publicKey, secretKey] =
+        generateKeyPair(KeyType::Secp256k1, generateSeed("testVaultDeleteNullopt"));
+
+    // Common transaction fields
+    auto const accountValue = calcAccountID(publicKey);
+    std::uint32_t const sequenceValue = 3;
+    auto const feeValue = canonical_AMOUNT();
+
+    // Transaction-specific required field values
+    auto const vaultIDValue = canonical_UINT256();
+
+    VaultDeleteBuilder builder{
+        accountValue,
+        vaultIDValue,
+        sequenceValue,
+        feeValue
+    };
+
+    // Do NOT set optional fields
+
+    auto tx = builder.build(publicKey, secretKey);
+
+    // Verify optional fields are not present
+    EXPECT_FALSE(tx.hasMemoData());
+    EXPECT_FALSE(tx.getMemoData().has_value());
+}
 
 }