fix: bugs from merge

This commit is contained in:
Vito
2026-03-31 14:09:33 +02:00
parent 73fe6e113a
commit 20d6e93b57
4 changed files with 67 additions and 61 deletions

View File

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

View File

@@ -46,7 +46,7 @@ LoanBrokerSet::preflight(PreflightContext const& ctx)
}
// Amendment-specific field presence rules
if (ctx.rules.enabled(fixLendingProtocolV1_1))
if (ctx.rules.enabled(featureLendingProtocolV1_1))
{
if (isLoanBrokerUpdate)
{
@@ -125,7 +125,7 @@ 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(fixLendingProtocolV1_1);
bool const fixEnabled = ctx.view.rules().enabled(featureLendingProtocolV1_1);
std::shared_ptr<SLE const> sleBroker;
std::shared_ptr<SLE const> sleVault;

View File

@@ -1775,22 +1775,21 @@ class LoanBroker_test : public beast::unit_test::suite
env.fund(XRP(100'000), issuer, alice, evan);
env.close();
env(trust(alice, issuer["IOU"](1'000'000)), THISLINE);
env(trust(alice, issuer["IOU"](1'000'000)));
env.close();
PrettyAsset const asset = issuer["IOU"];
env(pay(issuer, alice, asset(100'000)), THISLINE);
env(pay(issuer, alice, asset(100'000)));
env.close();
auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset});
env(tx, THISLINE);
env(tx);
env.close();
env(vault.deposit({.depositor = alice, .id = vaultKeylet.key, .amount = asset(50)}),
THISLINE);
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), THISLINE);
env(set(alice, vaultKeylet.key));
env.close();
struct Result
@@ -1805,14 +1804,14 @@ class LoanBroker_test : public beast::unit_test::suite
{
testcase("LoanBrokerSet post-amendment: VaultID rejected on update");
Env env(*this);
BEAST_EXPECT(env.enabled(fixLendingProtocolV1_1));
BEAST_EXPECT(env.enabled(featureLendingProtocolV1_1));
auto const [vaultID, brokerKL] = setup(env);
// Update with VaultID → temINVALID
env(set(alice, vaultID), loanBrokerID(brokerKL.key), ter(temINVALID), THISLINE);
env(set(alice, vaultID), loanBrokerID(brokerKL.key), ter(temINVALID));
// Update without VaultID succeeds
env(set(alice), loanBrokerID(brokerKL.key), data("post-amendment update"), THISLINE);
env(set(alice), loanBrokerID(brokerKL.key), data("post-amendment update"));
env.close();
auto const broker = env.le(brokerKL);
@@ -1825,15 +1824,15 @@ class LoanBroker_test : public beast::unit_test::suite
{
testcase("LoanBrokerSet post-amendment: VaultID required on create");
Env env(*this);
BEAST_EXPECT(env.enabled(fixLendingProtocolV1_1));
BEAST_EXPECT(env.enabled(featureLendingProtocolV1_1));
env.fund(XRP(100'000), issuer, alice);
env.close();
// Create without VaultID → temINVALID
env(set(alice), ter(temINVALID), THISLINE);
env(set(alice), ter(temINVALID));
// Create with zero VaultID → temINVALID
env(set(alice, uint256{}), ter(temINVALID), THISLINE);
env(set(alice, uint256{}), ter(temINVALID));
}
// Post-amendment: Update by wrong owner → tecNO_PERMISSION
@@ -1842,7 +1841,7 @@ class LoanBroker_test : public beast::unit_test::suite
Env env(*this);
auto const [vaultID, brokerKL] = setup(env);
env(set(evan), loanBrokerID(brokerKL.key), ter(tecNO_PERMISSION), THISLINE);
env(set(evan), loanBrokerID(brokerKL.key), ter(tecNO_PERMISSION));
}
// Post-amendment: Update non-existent broker → tecNO_ENTRY
@@ -1852,25 +1851,22 @@ class LoanBroker_test : public beast::unit_test::suite
env.fund(XRP(100'000), alice);
env.close();
env(set(alice), loanBrokerID(uint256{1}), ter(tecNO_ENTRY), THISLINE);
env(set(alice), loanBrokerID(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(fixLendingProtocolV1_1);
BEAST_EXPECT(!env.enabled(fixLendingProtocolV1_1));
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), loanBrokerID(brokerKL.key), ter(temINVALID), THISLINE);
env(set(alice), loanBrokerID(brokerKL.key), ter(temINVALID));
// Update with matching VaultID succeeds (old behavior)
env(set(alice, vaultID),
loanBrokerID(brokerKL.key),
data("pre-amendment update"),
THISLINE);
env(set(alice, vaultID), loanBrokerID(brokerKL.key), data("pre-amendment update"));
env.close();
auto const broker = env.le(brokerKL);
@@ -1883,73 +1879,68 @@ class LoanBroker_test : public beast::unit_test::suite
{
testcase("LoanBrokerSet pre-amendment: mismatched VaultID");
Env env(*this);
env.disableFeature(fixLendingProtocolV1_1);
env.disableFeature(featureLendingProtocolV1_1);
Vault vault{env};
env.fund(XRP(100'000), issuer, alice);
env.close();
env(trust(alice, issuer["IOU"](1'000'000)), THISLINE);
env(trust(alice, issuer["IOU"](1'000'000)));
env.close();
PrettyAsset const asset = issuer["IOU"];
env(pay(issuer, alice, asset(100'000)), THISLINE);
env(pay(issuer, alice, asset(100'000)));
env.close();
// Create two vaults
auto [tx1, vaultKL1] = vault.create({.owner = alice, .asset = asset});
env(tx1, THISLINE);
env(tx1);
env.close();
auto [tx2, vaultKL2] = vault.create({.owner = alice, .asset = asset});
env(tx2, THISLINE);
env(tx2);
env.close();
env(vault.deposit({.depositor = alice, .id = vaultKL1.key, .amount = asset(50)}),
THISLINE);
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), THISLINE);
env(set(alice, vaultKL1.key));
env.close();
// Update with different vault → tecNO_PERMISSION
env(set(alice, vaultKL2.key),
loanBrokerID(brokerKL.key),
ter(tecNO_PERMISSION),
THISLINE);
env(set(alice, vaultKL2.key), loanBrokerID(brokerKL.key), ter(tecNO_PERMISSION));
}
// Pre-amendment: Create without VaultID → temINVALID
{
testcase("LoanBrokerSet pre-amendment: create requires VaultID");
Env env(*this);
env.disableFeature(fixLendingProtocolV1_1);
env.disableFeature(featureLendingProtocolV1_1);
env.fund(XRP(100'000), issuer, alice);
env.close();
env(set(alice), ter(temINVALID), THISLINE);
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(fixLendingProtocolV1_1);
env.disableFeature(featureLendingProtocolV1_1);
auto const [vaultID, brokerKL] = setup(env);
env(set(alice, vaultID),
loanBrokerID(brokerKL.key),
managementFeeRate(TenthBips16(1)),
ter(temINVALID),
THISLINE);
ter(temINVALID));
}
// Pre-amendment: zero VaultID on update → temINVALID
{
testcase("LoanBrokerSet pre-amendment: zero VaultID on update");
Env env(*this);
env.disableFeature(fixLendingProtocolV1_1);
env.disableFeature(featureLendingProtocolV1_1);
auto const [vaultID, brokerKL] = setup(env);
env(set(alice, uint256{}), loanBrokerID(brokerKL.key), ter(temINVALID), THISLINE);
env(set(alice, uint256{}), loanBrokerID(brokerKL.key), ter(temINVALID));
}
}
@@ -1957,7 +1948,6 @@ public:
void
run() override
{
testFixAmendmentEnabled();
testLoanBrokerSetVaultIDAmendment();
testFeatureLendingProtocolV1_1enabled();
testLoanBrokerSetDebtMaximum();

View File

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