From d02f53498769bcbe91b8e8fe66873464214dcfb5 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Sat, 21 Mar 2026 16:17:18 +0100 Subject: [PATCH] refactor: Rename fixLendingProtocolV1_1 to featureLendingProtocolV1_1 and remove THISLINE --- .../transactions/VaultDelete.h | 37 ++++++++++++++ src/libxrpl/tx/invariants/VaultInvariant.cpp | 5 +- .../tx/transactors/vault/VaultDeposit.cpp | 11 +++-- src/test/app/Invariants_test.cpp | 2 +- src/test/app/Vault_test.cpp | 37 +++++++------- .../transactions/VaultDeleteTests.cpp | 49 +++++++++++++++++++ 6 files changed, 115 insertions(+), 26 deletions(-) diff --git a/include/xrpl/protocol_autogen/transactions/VaultDelete.h b/include/xrpl/protocol_autogen/transactions/VaultDelete.h index 89a4ef2a2f..bc89ce2bb7 100644 --- a/include/xrpl/protocol_autogen/transactions/VaultDelete.h +++ b/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 + 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 const& value) + { + object_[sfMemoData] = value; + return *this; + } + /** * @brief Build and return the VaultDelete wrapper. * @param publicKey The public key for signing. diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp index ed22bbeead..a609d2782a 100644 --- a/src/libxrpl/tx/invariants/VaultInvariant.cpp +++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp @@ -419,7 +419,8 @@ ValidVault::finalize( return std::nullopt; }(); - bool const isDonate = view.rules().enabled(fixLendingProtocolV1_1) && tx.isFlag(tfVaultDonate); + bool const isDonate = + view.rules().enabled(featureLendingProtocolV1_1) && tx.isFlag(tfVaultDonate); bool const shouldUpdateShares = // Vault Asset donation is the only operation that can succeed without updating shares ((tx.getTxnType() == ttVAULT_DEPOSIT && !isDonate) || // @@ -710,7 +711,7 @@ ValidVault::finalize( } // If assets are donated, check share invariants - if (view.rules().enabled(fixLendingProtocolV1_1) && tx.isFlag(tfVaultDonate)) + if (view.rules().enabled(featureLendingProtocolV1_1) && tx.isFlag(tfVaultDonate)) { auto const accountDeltaShares = deltaShares(tx[sfAccount]); if (accountDeltaShares) diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index abb0e3bfc5..79a6e58093 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -17,7 +17,7 @@ namespace xrpl { std::uint32_t VaultDeposit::getFlagsMask(PreflightContext const& ctx) { - if (ctx.rules.enabled(fixLendingProtocolV1_1)) + if (ctx.rules.enabled(featureLendingProtocolV1_1)) return tfVaultDepositMask; return tfVaultDepositMask | tfVaultDonate; @@ -77,7 +77,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } - if (ctx.view.rules().enabled(fixLendingProtocolV1_1) && ctx.tx.isFlag(tfVaultDonate)) + if (ctx.view.rules().enabled(featureLendingProtocolV1_1) && ctx.tx.isFlag(tfVaultDonate)) { if (account != vault->at(sfOwner)) { @@ -169,7 +169,7 @@ VaultDeposit::doApply() } auto const isDonate = - ctx_.view().rules().enabled(fixLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate); + ctx_.view().rules().enabled(featureLendingProtocolV1_1) && ctx_.tx.isFlag(tfVaultDonate); auto const& vaultAccount = vault->at(sfAccount); // Note, vault owner is always authorized @@ -233,8 +233,11 @@ VaultDeposit::doApply() auto const maybeAssets = sharesToAssetsDeposit(vault, sleIssuance, sharesCreated); if (!maybeAssets) + { return tecINTERNAL; // LCOV_EXCL_LINE - else if (*maybeAssets > amount) + } + + if (*maybeAssets > amount) { // LCOV_EXCL_START JLOG(j_.error()) << "VaultDeposit: would take more than offered."; diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 80ac8a7129..ef36bc6780 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -3380,7 +3380,7 @@ class Invariants_test : public beast::unit_test::suite TxAccount::A2); doInvariantCheck( - Env{*this, testable_amendments() - fixLendingProtocolV1_1}, + Env{*this, testable_amendments() - featureLendingProtocolV1_1}, {"deposit must change depositor shares"}, [&](Account const& A1, Account const& A2, ApplyContext& ac) { auto const keylet = keylet::vault(A1.id(), ac.view().seq()); diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index a9cd920854..3c088993df 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -5252,7 +5252,7 @@ class Vault_test : public beast::unit_test::suite testcase("VaultDelete data featureLendingProtocolV1_1 disabled"); env.disableFeature(featureLendingProtocolV1_1); delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A')); - env(delTx, ter(temDISABLED), THISLINE); + env(delTx, ter(temDISABLED)); env.close(); env.enableFeature(featureLendingProtocolV1_1); } @@ -5261,7 +5261,7 @@ class Vault_test : public beast::unit_test::suite { testcase("VaultDelete data featureLendingProtocolV1_1 enabled data too large"); delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength + 1, 'A')); - env(delTx, ter(temMALFORMED), THISLINE); + env(delTx, ter(temMALFORMED)); env.close(); } @@ -5269,7 +5269,7 @@ class Vault_test : public beast::unit_test::suite { testcase("VaultDelete data featureLendingProtocolV1_1 enabled data empty"); delTx[sfMemoData] = strHex(std::string(0, 'A')); - env(delTx, ter(temMALFORMED), THISLINE); + env(delTx, ter(temMALFORMED)); env.close(); } @@ -5277,12 +5277,12 @@ class Vault_test : public beast::unit_test::suite 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), THISLINE); + 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(maxDataPayloadLength, 'A')); - env(delTx, ter(tesSUCCESS), THISLINE); + env(delTx, ter(tesSUCCESS)); env.close(); } } @@ -5321,21 +5321,21 @@ class Vault_test : public beast::unit_test::suite auto const depositAmount = XRP(10); auto const [tx, keylet] = vault.create({.owner = owner, .asset = xrpIssue()}); - env(tx, ter(tesSUCCESS), THISLINE); + env(tx, ter(tesSUCCESS)); env.close(); - // With fixLendingProtocolV1_1 disabled, donations fail + // With featureLendingProtocolV1_1 disabled, donations fail { - testcase(prefix + " fails with fixLendingProtocolV1_1 disabled"); - env.disableFeature(fixLendingProtocolV1_1); + testcase(prefix + " fails with featureLendingProtocolV1_1 disabled"); + env.disableFeature(featureLendingProtocolV1_1); auto const tx = vault.deposit({ .depositor = owner, .id = keylet.key, .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{temINVALID_FLAG}, THISLINE); - env.enableFeature(fixLendingProtocolV1_1); + env(tx, ter{temINVALID_FLAG}); + env.enableFeature(featureLendingProtocolV1_1); env.close(); } @@ -5348,7 +5348,7 @@ class Vault_test : public beast::unit_test::suite .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{tecNO_PERMISSION}, THISLINE); + env(tx, ter{tecNO_PERMISSION}); env.close(); } @@ -5358,8 +5358,7 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = depositAmount, }), - ter{tesSUCCESS}, - THISLINE); + ter{tesSUCCESS}); env.close(); // Donation is not allowed by a non-owner @@ -5371,7 +5370,7 @@ class Vault_test : public beast::unit_test::suite .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{tecNO_PERMISSION}, THISLINE); + env(tx, ter{tecNO_PERMISSION}); env.close(); } @@ -5383,7 +5382,7 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, }); tx[sfAssetsMaximum] = XRP(30).number(); - env(tx, ter{tesSUCCESS}, THISLINE); + env(tx, ter{tesSUCCESS}); tx = vault.deposit({ .depositor = owner, @@ -5392,7 +5391,7 @@ class Vault_test : public beast::unit_test::suite .flags = tfVaultDonate, }); - env(tx, ter{tecLIMIT_EXCEEDED}, THISLINE); + env(tx, ter{tecLIMIT_EXCEEDED}); env.close(); } @@ -5407,7 +5406,7 @@ class Vault_test : public beast::unit_test::suite .amount = depositAmount, .flags = tfVaultDonate, }); - env(tx, ter{tesSUCCESS}, THISLINE); + env(tx, ter{tesSUCCESS}); env.close(); auto const shareBalanceAfterDeposit = vaultShareBalance(keylet); @@ -5426,7 +5425,7 @@ class Vault_test : public beast::unit_test::suite Asset shareAsset(sleVault->at(sfShareMPTID)); tx = vault.withdraw( {.depositor = depositor, .id = keylet.key, .amount = shareAsset(shareBalance)}); - env(tx, ter{tesSUCCESS}, THISLINE); + env(tx, ter{tesSUCCESS}); auto const shareBalanceAfterWithdraw = vaultShareBalance(keylet); auto const [assetsAvailableAfterWithdraw, assetsTotalAfterWithdraw] = diff --git a/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp index 24c89d249a..2b4ec486a6 100644 --- a/src/tests/libxrpl/protocol_autogen/transactions/VaultDeleteTests.cpp +++ b/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()); +} }