refactor: Rename fixLendingProtocolV1_1 to featureLendingProtocolV1_1 and remove THISLINE

This commit is contained in:
Vito
2026-03-21 16:17:18 +01:00
parent 5d538ca59a
commit d02f534987
6 changed files with 115 additions and 26 deletions

View File

@@ -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.

View File

@@ -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)

View File

@@ -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.";

View File

@@ -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());

View File

@@ -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] =

View File

@@ -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());
}
}