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/transactors/vault/VaultCreate.cpp b/src/libxrpl/tx/transactors/vault/VaultCreate.cpp index 28c38c43db..36093582a6 100644 --- a/src/libxrpl/tx/transactors/vault/VaultCreate.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultCreate.cpp @@ -32,7 +32,7 @@ VaultCreate::checkExtraFeatures(PreflightContext const& ctx) std::uint32_t VaultCreate::getFlagsMask(PreflightContext const& ctx) { - if (ctx.rules.enabled(fixLendingProtocolV1_1)) + if (ctx.rules.enabled(featureLendingProtocolV1_1)) return tfVaultCreateMask; return tfVaultCreateMask | tfVaultOwnerCanBlockDeposit; @@ -193,7 +193,8 @@ VaultCreate::doApply() if (tx.isFlag(tfVaultPrivate)) vault->setFlag(lsfVaultPrivate); - if (view().rules().enabled(fixLendingProtocolV1_1) && tx.isFlag(tfVaultOwnerCanBlockDeposit)) + if (view().rules().enabled(featureLendingProtocolV1_1) && + tx.isFlag(tfVaultOwnerCanBlockDeposit)) vault->setFlag(lsfVaultOwnerCanBlockDeposit); vault->at(sfSequence) = sequence; diff --git a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp index 20ca98aaa6..f4c4133404 100644 --- a/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultDeposit.cpp @@ -77,7 +77,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } - if (ctx.view.rules().enabled(fixLendingProtocolV1_1)) + if (ctx.view.rules().enabled(featureLendingProtocolV1_1)) { // Perform these checks early to avoid unnecessary processing @@ -85,7 +85,7 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) if (isVaultInsolvent(vault, sleShareIssuance)) { JLOG(ctx.j.debug()) << "VaultDeposit: Vault is insolvent, deposits are not allowed"; - return tecNO_PERMISSION; + return tecLOCKED; } if (vault->isFlag(lsfVaultDepositBlocked)) diff --git a/src/libxrpl/tx/transactors/vault/VaultSet.cpp b/src/libxrpl/tx/transactors/vault/VaultSet.cpp index 776fd22ae3..41801436ed 100644 --- a/src/libxrpl/tx/transactors/vault/VaultSet.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultSet.cpp @@ -23,7 +23,7 @@ VaultSet::checkExtraFeatures(PreflightContext const& ctx) std::uint32_t VaultSet::getFlagsMask(PreflightContext const& ctx) { - if (ctx.rules.enabled(fixLendingProtocolV1_1)) + if (ctx.rules.enabled(featureLendingProtocolV1_1)) return tfVaultSetMask; // Add tfVaultDepositBlock and tfVaultDepositUnblock flags to indicate they are disabled @@ -36,6 +36,8 @@ isValidVaultUpdate(PreflightContext const& ctx) auto const atLeastOneFieldPresent = ctx.tx.isFieldPresent(sfDomainID) || ctx.tx.isFieldPresent(sfAssetsMaximum) || ctx.tx.isFieldPresent(sfData); + // Mask of valid, non-universal flags: any bit set here means the + // transaction is requesting a meaningful flag change. auto const expectedFlags = ~(VaultSet::getFlagsMask(ctx) | tfUniversal); return atLeastOneFieldPresent || (ctx.tx.getFlags() & expectedFlags); @@ -134,7 +136,7 @@ VaultSet::preclaim(PreclaimContext const& ctx) } } - if (ctx.view.rules().enabled(fixLendingProtocolV1_1)) + if (ctx.view.rules().enabled(featureLendingProtocolV1_1)) { // The Vault is not configured to support deposit blocking if (!vault->isFlag(lsfVaultOwnerCanBlockDeposit) && @@ -214,7 +216,7 @@ VaultSet::doApply() view().update(sleIssuance); } - if (view().rules().enabled(fixLendingProtocolV1_1)) + if (view().rules().enabled(featureLendingProtocolV1_1)) { if (tx.isFlag(tfVaultDepositBlock)) vault->setFlag(lsfVaultDepositBlocked); diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index a65bad14e1..19aaa803ae 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -669,11 +669,11 @@ class Vault_test : public beast::unit_test::suite env(tx, ter{temINVALID_FLAG}); { - env.disableFeature(fixLendingProtocolV1_1); + env.disableFeature(featureLendingProtocolV1_1); auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); tx[sfFlags] = tfVaultOwnerCanBlockDeposit; env(tx, ter{temINVALID_FLAG}); - env.enableFeature(fixLendingProtocolV1_1); + env.enableFeature(featureLendingProtocolV1_1); } { @@ -1067,18 +1067,18 @@ class Vault_test : public beast::unit_test::suite testCase( [&](Env& env, Account const&, Account const& owner, Asset const& asset, Vault& vault) { - testcase("set flags fail without fixLendingProtocolV1_1"); + testcase("set flags fail without featureLendingProtocolV1_1"); auto [tx, keylet] = vault.create({.owner = owner, .asset = asset}); { - env.disableFeature(fixLendingProtocolV1_1); + env.disableFeature(featureLendingProtocolV1_1); env(vault.set({.owner = owner, .id = keylet.key, .flags = tfVaultDepositBlock}), ter(temINVALID_FLAG)); env(vault.set( {.owner = owner, .id = keylet.key, .flags = tfVaultDepositUnblock}), ter(temINVALID_FLAG)); - env.enableFeature(fixLendingProtocolV1_1); + env.enableFeature(featureLendingProtocolV1_1); } }); @@ -2371,7 +2371,7 @@ class Vault_test : public beast::unit_test::suite { auto const tx = vault.deposit( {.depositor = depositor, .id = vaultKeylet.key, .amount = depositAmount}); - env(tx, ter{tesSUCCESS}, THISLINE); + env(tx, ter{tesSUCCESS}); env.close(); } @@ -2383,7 +2383,7 @@ class Vault_test : public beast::unit_test::suite using namespace loanBroker; using namespace loan; - env(set(owner, vaultKeylet.key), THISLINE); + env(set(owner, vaultKeylet.key)); env.close(); // Create a simple Loan for the full amount of Vault assets @@ -2393,11 +2393,10 @@ class Vault_test : public beast::unit_test::suite paymentTotal(1), sig(sfCounterpartySignature, owner), fee(env.current()->fees().base * 2), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); env.close(std::chrono::seconds{120 + 60}); - env(manage(owner, loanKeylet.key, tfLoanDefault), ter(tesSUCCESS), THISLINE); + env(manage(owner, loanKeylet.key, tfLoanDefault), ter(tesSUCCESS)); env.close(); auto const sleVault = env.le(vaultKeylet); @@ -2424,7 +2423,7 @@ class Vault_test : public beast::unit_test::suite { auto const tx = vault.deposit( {.depositor = depositor, .id = vaultKeylet.key, .amount = asset(20)}); - env(tx, ter{tecNO_PERMISSION}, THISLINE); + env(tx, ter{tecNO_PERMISSION}); env.close(); } @@ -2440,14 +2439,13 @@ class Vault_test : public beast::unit_test::suite .id = vaultKeylet.key, .holder = depositor, .amount = share(0)}), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); env.close(); } { - env(loan::del(owner, loanKeylet.key), ter(tesSUCCESS), THISLINE); - env(loanBroker::del(owner, brokerKeylet.key), ter(tesSUCCESS), THISLINE); + env(loan::del(owner, loanKeylet.key), ter(tesSUCCESS)); + env(loanBroker::del(owner, brokerKeylet.key), ter(tesSUCCESS)); env(vault.del({.owner = owner, .id = vaultKeylet.key})); env.close(); } @@ -3173,7 +3171,7 @@ class Vault_test : public beast::unit_test::suite { auto const tx = vault.deposit( {.depositor = issuer, .id = vaultKeylet.key, .amount = depositAmount}); - env(tx, ter{tesSUCCESS}, THISLINE); + env(tx, ter{tesSUCCESS}); env.close(); } @@ -3185,7 +3183,7 @@ class Vault_test : public beast::unit_test::suite using namespace loanBroker; using namespace loan; - env(set(owner, vaultKeylet.key), ter{tesSUCCESS}, THISLINE); + env(set(owner, vaultKeylet.key), ter{tesSUCCESS}); env.close(); // Create a simple Loan for the full amount of Vault assets @@ -3195,11 +3193,10 @@ class Vault_test : public beast::unit_test::suite paymentTotal(1), sig(sfCounterpartySignature, owner), fee(env.current()->fees().base * 2), - ter{tesSUCCESS}, - THISLINE); + ter{tesSUCCESS}); env.close(std::chrono::seconds{120 + 60}); - env(manage(owner, loanKeylet.key, tfLoanDefault), ter(tesSUCCESS), THISLINE); + env(manage(owner, loanKeylet.key, tfLoanDefault), ter(tesSUCCESS)); env.close(); auto const sleVault = env.le(vaultKeylet); @@ -3226,7 +3223,7 @@ class Vault_test : public beast::unit_test::suite { auto const tx = vault.deposit( {.depositor = issuer, .id = vaultKeylet.key, .amount = asset(20)}); - env(tx, ter{tecNO_PERMISSION}, THISLINE); + env(tx, ter{tecNO_PERMISSION}); env.close(); } @@ -3242,14 +3239,13 @@ class Vault_test : public beast::unit_test::suite .id = vaultKeylet.key, .holder = issuer, .amount = share(0)}), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); env.close(); } { - env(loan::del(owner, loanKeylet.key), ter(tesSUCCESS), THISLINE); - env(loanBroker::del(owner, brokerKeylet.key), ter(tesSUCCESS), THISLINE); + env(loan::del(owner, loanKeylet.key), ter(tesSUCCESS)); + env(loanBroker::del(owner, brokerKeylet.key), ter(tesSUCCESS)); env(vault.del({.owner = owner, .id = vaultKeylet.key})); env.close(); } @@ -5585,30 +5581,28 @@ class Vault_test : public beast::unit_test::suite auto const blockVault = [&](TER expectedTer, Keylet const& keylet) { env(vault.set({.owner = owner, .id = keylet.key, .flags = tfVaultDepositBlock}), - ter(expectedTer), - THISLINE); + ter(expectedTer)); }; auto const unblockVault = [&](TER expectedTer, Keylet const& keylet) { env(vault.set({.owner = owner, .id = keylet.key, .flags = tfVaultDepositUnblock}), - ter(expectedTer), - THISLINE); + ter(expectedTer)); }; // Blocking Vault with the amendment disabled fails { testcase(prefix + "block/unblock fails when amendment is disabled"); - env.disableFeature(fixLendingProtocolV1_1); + env.disableFeature(featureLendingProtocolV1_1); auto const [tx, keylet] = vault.create( {.owner = owner, .asset = asset, .flags = tfVaultOwnerCanBlockDeposit}); - env(tx, ter(temINVALID_FLAG), THISLINE); + env(tx, ter(temINVALID_FLAG)); env.close(); blockVault(temINVALID_FLAG, keylet); unblockVault(temINVALID_FLAG, keylet); - env.enableFeature(fixLendingProtocolV1_1); + env.enableFeature(featureLendingProtocolV1_1); } // Block Vault deposits fails if the vault is not configured to allow blocking deposits @@ -5621,7 +5615,7 @@ class Vault_test : public beast::unit_test::suite blockVault(tecNO_PERMISSION, keylet); unblockVault(tecNO_PERMISSION, keylet); - env(vault.del({.owner = owner, .id = keylet.key}), ter(tesSUCCESS), THISLINE); + env(vault.del({.owner = owner, .id = keylet.key}), ter(tesSUCCESS)); env.close(); } @@ -5638,15 +5632,13 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = XRP(10'000), }), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); env(vault.deposit({ .depositor = other, .id = keylet.key, .amount = XRP(10'000), }), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); blockVault(tesSUCCESS, keylet); @@ -5656,8 +5648,7 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = XRP(10'000), }), - ter(tecNO_PERMISSION), - THISLINE); + ter(tecNO_PERMISSION)); // Other accounts are also blocked from depositing to the vault env(vault.deposit({ @@ -5665,8 +5656,7 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = XRP(10'000), }), - ter(tecNO_PERMISSION), - THISLINE); + ter(tecNO_PERMISSION)); // Block vault withdrawal works as normal env(vault.withdraw({ @@ -5674,16 +5664,14 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = XRP(10'000), }), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); env(vault.withdraw({ .depositor = other, .id = keylet.key, .amount = XRP(10'000), }), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); unblockVault(tesSUCCESS, keylet); @@ -5692,16 +5680,14 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = XRP(10'000), }), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); env(vault.deposit({ .depositor = other, .id = keylet.key, .amount = XRP(10'000), }), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); // Withdraw to keep the vault empty env(vault.withdraw({ @@ -5709,30 +5695,26 @@ class Vault_test : public beast::unit_test::suite .id = keylet.key, .amount = XRP(10'000), }), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); env(vault.withdraw({ .depositor = other, .id = keylet.key, .amount = XRP(10'000), }), - ter(tesSUCCESS), - THISLINE); + ter(tesSUCCESS)); } { testcase(prefix + "block/unblock fails when caller is not owner"); env(vault.set({.owner = other, .id = keylet.key, .flags = tfVaultDepositBlock}), - ter(tecNO_PERMISSION), - THISLINE); + ter(tecNO_PERMISSION)); blockVault(tesSUCCESS, keylet); env(vault.set({.owner = other, .id = keylet.key, .flags = tfVaultDepositUnblock}), - ter(tecNO_PERMISSION), - THISLINE); + ter(tecNO_PERMISSION)); unblockVault(tesSUCCESS, keylet); } @@ -5772,7 +5754,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); } @@ -5781,7 +5763,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(); } @@ -5789,7 +5771,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(); } @@ -5797,12 +5779,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(); } } 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()); +} }