diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index b6950891ea9..f243607feb0 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/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}, diff --git a/include/xrpl/protocol_autogen/transactions/LoanBrokerSet.h b/include/xrpl/protocol_autogen/transactions/LoanBrokerSet.h index 854022242dd..34cfb90667b 100644 --- a/include/xrpl/protocol_autogen/transactions/LoanBrokerSet.h +++ b/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 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 /** * @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 const& vaultID, std::optional sequence = std::nullopt, + std::optional sequence = std::nullopt, std::optional fee = std::nullopt ) : TransactionBuilderBase(ttLOAN_BROKER_SET, account, sequence, fee) { - setVaultID(vaultID); } /** @@ -258,7 +271,7 @@ class LoanBrokerSetBuilder : public TransactionBuilderBase /** @brief Transaction-specific field setters */ /** - * @brief Set sfVaultID (SoeRequired) + * @brief Set sfVaultID (SoeOptional) * @return Reference to this builder for method chaining. */ LoanBrokerSetBuilder& diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp index 2dc003eb7fe..7412ac324e1 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp @@ -1,14 +1,19 @@ #include +#include #include #include +#include #include +#include #include #include #include #include #include +#include #include +#include #include #include #include @@ -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 (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 @@ 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, 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, 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 sleBroker; + std::shared_ptr 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, 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, 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) << "."; diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 0edb955b902..e6754b6c64b 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/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()) { @@ -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(); diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index 27c54d830be..0643cdcb4df 100644 --- a/src/test/jtx/TestHelpers.h +++ b/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 const& vaultId = std::nullopt, + std::uint32_t flags = 0); // Use "del" because "delete" is a reserved word in C++. json::Value diff --git a/src/test/jtx/impl/TestHelpers.cpp b/src/test/jtx/impl/TestHelpers.cpp index a8ec899c8a2..d4c8c95f280 100644 --- a/src/test/jtx/impl/TestHelpers.cpp +++ b/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 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; } diff --git a/src/tests/libxrpl/protocol_autogen/transactions/LoanBrokerSetTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/LoanBrokerSetTests.cpp index bcf0a65755a..004c0d84a50 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/LoanBrokerSetTests.cpp +++ b/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());