ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-12-06 17:27:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

More review feedback from @gregtatcam

Browse Source 
- Also fix a few build errors that I missed earlier.
- Updated freeze check rules for LoanSet.
- Fixed the debt total calculation and check in LoanSet.
- Removed _some_ unnecessary includes.
This commit is contained in:
Ed Hennis
2025-09-05 20:00:10 -04:00
parent 582f56487d
commit 6db14ccb13
6 changed files with 91 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
include/xrpl/protocol/detail/ledger_entries.macro
View File

@@ -558,7 +558,7 @@ LEDGER_ENTRY(ltLOAN, 0x0089, Loan, loan, ({
{sfPreviousPaymentDate, soeREQUIRED},
{sfNextPaymentDueDate, soeREQUIRED},
{sfPaymentRemaining, soeREQUIRED},
{sfAssetsAvailable, soeREQUIRED},
{sfAssetsAvailable, soeDEFAULT},
{sfPrincipalOutstanding, soeREQUIRED},
// Save the original request amount for rounding / scaling of
// other computations, particularly for IOUs

68
src/test/app/Loan_test.cpp
View File

@@ -925,6 +925,17 @@ class Loan_test : public beast::unit_test::suite
if (!BEAST_EXPECT(brokerSle))
return;
auto const vaultPseudo = [&]() {
auto const vaultSle =
env.le(keylet::vault(brokerSle->at(sfVaultID)));
if (!BEAST_EXPECT(vaultSle))
// This will be wrong, but the test has failed anyway.
return lender;
auto const vaultPseudo =
Account("Vault pseudo-account", vaultSle->at(sfAccount));
return vaultPseudo;
}();
auto const [freeze, deepfreeze, unfreeze, expectedResult] =
[&]() -> std::tuple<
std::function<void(Account const& holder)>,
@@ -976,8 +987,8 @@ class Loan_test : public beast::unit_test::suite
}
}();
// Try freezing both the lender and the pseudo-account
for (auto const& account : {lender, pseudoAcct})
// Try freezing the accounts that can't be frozen
for (auto const& account : {vaultPseudo, evan, pseudoAcct})
{
if (freeze)
{
@@ -1020,29 +1031,42 @@ class Loan_test : public beast::unit_test::suite
// automatically.)
env(trust(evan, issuer[iouCurrency](100'000)));
// Freeze evan
deepfreeze(evan);
for (auto const& account :
{// these accounts can't be frozen, which deep freeze
// implies
vaultPseudo,
evan,
pseudoAcct,
// these accounts can't be deep frozen
lender})
{
// Freeze evan
deepfreeze(evan);
// Try to create a loan with a deep frozen line
env(set(evan, broker.brokerID, debtMaximumRequest, startDate),
sig(sfCounterpartySignature, lender),
loanSetFee,
ter(expectedResult));
// Try to create a loan with a deep frozen line
env(set(evan,
broker.brokerID,
debtMaximumRequest,
startDate),
sig(sfCounterpartySignature, lender),
loanSetFee,
ter(expectedResult));
// Unfreeze evan
BEAST_EXPECT(unfreeze);
unfreeze(evan);
// Unfreeze evan
BEAST_EXPECT(unfreeze);
unfreeze(evan);
// Ensure the line is unfrozen with a request that is fine
// except too it requests more principal than the broker can
// carry
env(set(evan,
broker.brokerID,
debtMaximumRequest + 1,
startDate),
sig(sfCounterpartySignature, lender),
loanSetFee,
ter(tecLIMIT_EXCEEDED));
// Ensure the line is unfrozen with a request that is fine
// except too it requests more principal than the broker can
// carry
env(set(evan,
broker.brokerID,
debtMaximumRequest + 1,
startDate),
sig(sfCounterpartySignature, lender),
loanSetFee,
ter(tecLIMIT_EXCEEDED));
}
}
}

2
src/xrpld/app/tx/detail/Batch.cpp
View File

@@ -418,7 +418,7 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
requiredSigners.insert(innerAccount);
// Some transactions have a Counterparty, who must also sign the
// transaction if they are not the outer account
if (auto const counterparty = stx.at(~sfCounterparty);
if (auto const counterparty = rb.at(~sfCounterparty);
counterparty && counterparty != outerAccount)
requiredSigners.insert(*counterparty);
}

2
src/xrpld/app/tx/detail/LoanBrokerCoverClawback.cpp
View File

@@ -133,6 +133,8 @@ determineAsset(
// This check only applies to IOUs
auto const holder = amount.getIssuer();
// holder can be the submitting account (the issuer of the asset) if a
// LoanBrokerID was provided in the transaction.
if (holder == account)
{
return amount.asset();

15
src/xrpld/app/tx/detail/LoanBrokerSet.cpp
View File

@@ -20,28 +20,14 @@
#include <xrpld/app/tx/detail/LoanBrokerSet.h>
//
#include <xrpld/app/misc/LendingHelpers.h>
#include <xrpld/app/tx/detail/SignerEntries.h>
#include <xrpld/app/tx/detail/VaultCreate.h>
#include <xrpld/ledger/ApplyView.h>
#include <xrpld/ledger/View.h>
#include <xrpl/basics/Log.h>
#include <xrpl/basics/Number.h>
#include <xrpl/basics/chrono.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/PublicKey.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAmount.h>
#include <xrpl/protocol/STNumber.h>
#include <xrpl/protocol/STObject.h>
#include <xrpl/protocol/STXChainBridge.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFlags.h>
#include <xrpl/protocol/XRPAmount.h>
namespace ripple {
@@ -151,6 +137,7 @@ LoanBrokerSet::doApply()
else
{
// Create a new LoanBroker pointing back to the given Vault
auto const vaultID = tx[sfVaultID];
auto const sleVault = view.read(keylet::vault(vaultID));
auto const vaultPseudoID = sleVault->at(sfAccount);
auto const sequence = tx.getSeqValue();

61
src/xrpld/app/tx/detail/LoanSet.cpp
View File

@@ -20,26 +20,15 @@
#include <xrpld/app/tx/detail/LoanSet.h>
//
#include <xrpld/app/misc/LendingHelpers.h>
#include <xrpld/app/tx/detail/SignerEntries.h>
#include <xrpld/app/tx/detail/VaultCreate.h>
#include <xrpld/ledger/ApplyView.h>
#include <xrpld/ledger/View.h>
#include <xrpl/basics/Log.h>
#include <xrpl/basics/Number.h>
#include <xrpl/basics/chrono.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/Protocol.h>
#include <xrpl/protocol/PublicKey.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAmount.h>
#include <xrpl/protocol/STNumber.h>
#include <xrpl/protocol/STObject.h>
#include <xrpl/protocol/STXChainBridge.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFlags.h>
#include <xrpl/protocol/XRPAmount.h>
@@ -231,28 +220,41 @@ LoanSet::preclaim(PreclaimContext const& ctx)
// Should be impossible
return tefBAD_LEDGER; // LCOV_EXCL_LINE
Asset const asset = vault->at(sfAsset);
auto const vaultPseudo = vault->at(sfAccount);
if (auto const ter = canAddHolding(ctx.view, asset))
return ter;
if (auto const ret = checkFrozen(ctx.view, brokerOwner, asset))
// vaultPseudo is going to send funds, so it can't be frozen.
if (auto const ret = checkFrozen(ctx.view, vaultPseudo, asset))
{
JLOG(ctx.j.warn()) << "Broker owner account is frozen.";
JLOG(ctx.j.warn()) << "Vault pseudo-account is frozen.";
return ret;
}
// borrower is eventually going to have to pay back the loan, so it can't be
// frozen now. It is also going to receive funds, so it can't be deep
// frozen, but being frozen is a prerequisite for being deep frozen, so
// checking the one is sufficient.
if (auto const ret = checkFrozen(ctx.view, borrower, asset))
{
JLOG(ctx.j.warn()) << "Borrower account is frozen.";
return ret;
}
// TODO: Remove when LoanDraw is combined with LoanSet
// brokerPseudo is eventually going to send funds to the borrower, so it
// can't be frozen now. It is also going to receive funds, so it can't be
// deep frozen, but being frozen is a prerequisite for being deep frozen, so
// checking the one is sufficient.
if (auto const ret = checkFrozen(ctx.view, brokerPseudo, asset))
{
JLOG(ctx.j.warn()) << "Broker pseudo-account account is frozen.";
return ret;
}
if (auto const ret = checkDeepFrozen(ctx.view, borrower, asset))
// brokerOwner is going to receive funds if there's an origination fee, so
// it can't be deep frozen
if (auto const ret = checkDeepFrozen(ctx.view, brokerOwner, asset))
{
JLOG(ctx.j.warn()) << "Borrower account is deep frozen.";
return ret;
}
if (auto const ret = checkDeepFrozen(ctx.view, brokerPseudo, asset))
{
JLOG(ctx.j.warn()) << "Broker pseudo-account account is deep frozen.";
JLOG(ctx.j.warn()) << "Broker owner account is frozen.";
return ret;
}
@@ -264,7 +266,24 @@ LoanSet::preclaim(PreclaimContext const& ctx)
<< "Insufficient assets available in the Vault to fund the loan.";
return tecINSUFFICIENT_FUNDS;
}
auto const newDebtTotal = brokerSle->at(sfDebtTotal) + principalRequested;
TenthBips32 const interestRate{tx[~sfInterestRate].value_or(0)};
auto const paymentInterval =
tx[~sfPaymentInterval].value_or(defaultPaymentInterval);
auto const paymentTotal = tx[~sfPaymentTotal].value_or(defaultPaymentTotal);
TenthBips32 const managementFeeRate{brokerSle->at(sfManagementFeeRate)};
auto const totalInterest = loanInterestOutstandingMinusFee(
asset,
principalRequested,
principalRequested,
interestRate,
paymentInterval,
paymentTotal,
managementFeeRate);
auto const newDebtTotal =
brokerSle->at(sfDebtTotal) + principalRequested + totalInterest;
if (auto const debtMaximum = brokerSle->at(sfDebtMaximum);
debtMaximum != 0 && debtMaximum < newDebtTotal)
{