diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 2b3cc84faf..f279c8dec2 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -162,6 +162,9 @@ class Invariants_test : public beast::unit_test::suite auto sle = ac.view().peek(keylet::account(A1.id())); if (!sle) return false; + // Clear the balance so the "account deletion left behind a + // non-zero balance" check doesn't trip earlier than the desired + // check. sle->at(sfBalance) = beast::zero; ac.view().erase(sle); return true; @@ -190,6 +193,9 @@ class Invariants_test : public beast::unit_test::suite auto sleA2 = ac.view().peek(keylet::account(A2.id())); if (!sleA1 || !sleA2) return false; + // Clear the balance so the "account deletion left behind a + // non-zero balance" check doesn't trip earlier than the desired + // check. sleA1->at(sfBalance) = beast::zero; sleA2->at(sfBalance) = beast::zero; ac.view().erase(sleA1); @@ -232,6 +238,9 @@ class Invariants_test : public beast::unit_test::suite auto const sleA1 = ac.view().peek(keylet::account(a1)); if (!sleA1) return false; + // Clear the balance so the "account deletion left behind a + // non-zero balance" check doesn't trip earlier than the desired + // check. sleA1->at(sfBalance) = beast::zero; BEAST_EXPECT(sleA1->at(sfOwnerCount) == 0); adjustOwnerCount(ac.view(), sleA1, 1, ac.journal); @@ -270,6 +279,9 @@ class Invariants_test : public beast::unit_test::suite auto const key = std::invoke(keyletfunc, a1); auto const newSLE = std::make_shared(key); ac.view().insert(newSLE); + // Clear the balance so the "account deletion left behind a + // non-zero balance" check doesn't trip earlier than the + // desired check. sleA1->at(sfBalance) = beast::zero; ac.view().erase(sleA1); @@ -287,6 +299,9 @@ class Invariants_test : public beast::unit_test::suite auto sle = ac.view().peek(keylet::account(A1.id())); if (!sle) return false; + // Clear the balance so the "account deletion left behind a + // non-zero balance" check doesn't trip earlier than the desired + // check. sle->at(sfBalance) = beast::zero; sle->at(sfOwnerCount) = 0; ac.view().erase(sle); @@ -319,6 +334,9 @@ class Invariants_test : public beast::unit_test::suite BEAST_EXPECT(sle->at(~sfAMMID)); BEAST_EXPECT(sle->at(~sfAMMID) == ammKey); + // Clear the balance so the "account deletion left behind a + // non-zero balance" check doesn't trip earlier than the desired + // check. sle->at(sfBalance) = beast::zero; sle->at(sfOwnerCount) = 0; ac.view().erase(sle); @@ -383,6 +401,9 @@ class Invariants_test : public beast::unit_test::suite !ac.view().exists(ownerDirKeylet) || ac.view().emptyDirDelete(ownerDirKeylet)); + // Clear the balance so the "account deletion left behind a + // non-zero balance" check doesn't trip earlier than the desired + // check. sle->at(sfBalance) = beast::zero; sle->at(sfOwnerCount) = 0; ac.view().erase(sle); diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index d0345b517b..7627289dc0 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -55,7 +55,7 @@ class Loan_test : public beast::unit_test::suite // Single Asset Vault depends on MPTokensV1, but don't test every combo // of that. using namespace jtx; - auto failAll = [this](FeatureBitset features, bool goodVault = false) { + auto failAll = [this](FeatureBitset features) { Env env(*this, features); Account const alice{"alice"}; @@ -96,7 +96,7 @@ class Loan_test : public beast::unit_test::suite failAll(all - featureMPTokensV1); failAll(all - featureSingleAssetVault - featureLendingProtocol); failAll(all - featureSingleAssetVault); - failAll(all - featureLendingProtocol, true); + failAll(all - featureLendingProtocol); } struct BrokerInfo @@ -122,6 +122,9 @@ class Loan_test : public beast::unit_test::suite TenthBips32 const interestRate{}; }; + /** Helper class to compare the expected state of a loan and loan broker + * against the data in the ledger. + */ struct VerifyLoanStatus { public: @@ -145,6 +148,8 @@ class Loan_test : public beast::unit_test::suite { } + /** Checks the expected broker state against the ledger + */ void checkBroker( Number const& principalRequested, @@ -194,6 +199,9 @@ class Loan_test : public beast::unit_test::suite if (ownerCount == 0) { // Allow some slop for rounding IOUs + + // TODO: This needs to be an exact match once all the + // other rounding issues are worked out. auto const total = vaultSle->at(sfAssetsTotal); auto const available = vaultSle->at(sfAssetsAvailable); env.test.BEAST_EXPECT( @@ -210,21 +218,7 @@ class Loan_test : public beast::unit_test::suite } } - void - checkBroker( - LoanState const& state, - TenthBips32 interestRate, - std::uint32_t ownerCount) const - { - checkBroker( - state.principalRequested, - state.principalOutstanding, - interestRate, - state.paymentInterval, - state.paymentRemaining, - ownerCount); - } - + /** Checks both the loan and broker expect states against the ledger */ void operator()( std::uint32_t previousPaymentDate, @@ -300,6 +294,7 @@ class Loan_test : public beast::unit_test::suite } } + /** Checks both the loan and broker expect states against the ledger */ void operator()(LoanState const& state) const { @@ -442,6 +437,16 @@ class Loan_test : public beast::unit_test::suite return true; } + /** Runs through the complete lifecycle of a loan + * + * 1. Create a loan. + * 2. Test a bunch of transaction failure conditions. + * 3. Use the `toEndOfLife` callback to take the loan to 0. How that is done + * depends on the callback. e.g. Default, Early payoff, make all the + * normal payments, etc. + * 4. Delete the loan. The loan will alternate between being deleted by the + * lender and the borrower. + */ void lifecycle( std::string const& caseLabel, @@ -745,6 +750,14 @@ class Loan_test : public beast::unit_test::suite } } + /** Wrapper to run a series of lifecycle tests for a given asset and loan + * amount + * + * Will be used in the future to vary the loan parameters. For now, it is + * only called once. + * + * Tests a bunch of LoanSet failure conditions before lifecycle. + */ template void testCaseWrapper( diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index efe7e0c83b..f9220eb3fa 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -746,21 +746,19 @@ set(AccountID const& account, uint256 const& vaultId, std::uint32_t flags = 0); // Use "del" because "delete" is a reserved word in C++. Json::Value -del(AccountID const& account, - uint256 const& loanBrokerID, - std::uint32_t flags = 0); +del(AccountID const& account, uint256 const& brokerID, std::uint32_t flags = 0); Json::Value coverDeposit( AccountID const& account, - uint256 const& loanBrokerID, + uint256 const& brokerID, STAmount const& amount, std::uint32_t flags = 0); Json::Value coverWithdraw( AccountID const& account, - uint256 const& loanBrokerID, + uint256 const& brokerID, STAmount const& amount, std::uint32_t flags = 0); diff --git a/src/test/jtx/impl/TestHelpers.cpp b/src/test/jtx/impl/TestHelpers.cpp index 05959dadfb..656e195303 100644 --- a/src/test/jtx/impl/TestHelpers.cpp +++ b/src/test/jtx/impl/TestHelpers.cpp @@ -388,12 +388,12 @@ set(AccountID const& account, uint256 const& vaultId, uint32_t flags) } Json::Value -del(AccountID const& account, uint256 const& loanBrokerID, uint32_t flags) +del(AccountID const& account, uint256 const& brokerID, uint32_t flags) { Json::Value jv; jv[sfTransactionType] = jss::LoanBrokerDelete; jv[sfAccount] = to_string(account); - jv[sfLoanBrokerID] = to_string(loanBrokerID); + jv[sfLoanBrokerID] = to_string(brokerID); jv[sfFlags] = flags; return jv; } @@ -401,14 +401,14 @@ del(AccountID const& account, uint256 const& loanBrokerID, uint32_t flags) Json::Value coverDeposit( AccountID const& account, - uint256 const& loanBrokerID, + uint256 const& brokerID, STAmount const& amount, uint32_t flags) { Json::Value jv; jv[sfTransactionType] = jss::LoanBrokerCoverDeposit; jv[sfAccount] = to_string(account); - jv[sfLoanBrokerID] = to_string(loanBrokerID); + jv[sfLoanBrokerID] = to_string(brokerID); jv[sfAmount] = amount.getJson(JsonOptions::none); jv[sfFlags] = flags; return jv; @@ -417,14 +417,14 @@ coverDeposit( Json::Value coverWithdraw( AccountID const& account, - uint256 const& loanBrokerID, + uint256 const& brokerID, STAmount const& amount, uint32_t flags) { Json::Value jv; jv[sfTransactionType] = jss::LoanBrokerCoverWithdraw; jv[sfAccount] = to_string(account); - jv[sfLoanBrokerID] = to_string(loanBrokerID); + jv[sfLoanBrokerID] = to_string(brokerID); jv[sfAmount] = amount.getJson(JsonOptions::none); jv[sfFlags] = flags; return jv; diff --git a/src/test/jtx/impl/sig.cpp b/src/test/jtx/impl/sig.cpp index 2763df5271..6ea1c153cb 100644 --- a/src/test/jtx/impl/sig.cpp +++ b/src/test/jtx/impl/sig.cpp @@ -29,19 +29,19 @@ sig::operator()(Env&, JTx& jt) const { if (!manual_) return; - if (!subField) + if (!subField_) jt.fill_sig = false; if (account_) { // VFALCO Inefficient pre-C++14 auto const account = *account_; - auto callback = [subField = subField, account](Env&, JTx& jtx) { + auto callback = [subField = subField_, account](Env&, JTx& jtx) { // Where to put the signature. Supports sfCounterPartySignature. auto& sigObject = subField ? jtx[*subField] : jtx.jv; jtx::sign(jtx.jv, account, sigObject); }; - if (!subField) + if (!subField_) jt.mainSigners.emplace_back(callback); else jt.postSigners.emplace_back(callback); diff --git a/src/test/jtx/multisign.h b/src/test/jtx/multisign.h index aedc660d92..7a035a9ff0 100644 --- a/src/test/jtx/multisign.h +++ b/src/test/jtx/multisign.h @@ -67,7 +67,13 @@ class msig { public: std::vector signers; + /** Alternative transaction object field in which to place the signer list. + * + * subField is only supported if an account_ is provided as well. + */ SField const* const subField = nullptr; + /// Used solely as a convenience placeholder for ctors that do _not_ specify + /// a subfield. static constexpr SField* const topLevel = nullptr; msig(SField const* subField_, std::vector signers_) diff --git a/src/test/jtx/sig.h b/src/test/jtx/sig.h index c101d9e491..b96a306a37 100644 --- a/src/test/jtx/sig.h +++ b/src/test/jtx/sig.h @@ -35,9 +35,19 @@ class sig { private: bool manual_ = true; - /// subField only supported with explicit account - SField const* const subField = nullptr; + /** Alternative transaction object field in which to place the signature. + * + * subField is only supported if an account_ is provided as well. + */ + SField const* const subField_ = nullptr; + /** Account that will generate the signature. + * + * If not provided, no signature will be added by this helper. See also + * Env::autofill_sig. + */ std::optional account_; + /// Used solely as a convenience placeholder for ctors that do _not_ specify + /// a subfield. static constexpr SField* const topLevel = nullptr; public: @@ -49,13 +59,13 @@ public: { } - explicit sig(SField const* subField_, Account const& account) - : subField(subField_), account_(account) + explicit sig(SField const* subField, Account const& account) + : subField_(subField), account_(account) { } - explicit sig(SField const& subField_, Account const& account) - : sig(&subField_, account) + explicit sig(SField const& subField, Account const& account) + : sig(&subField, account) { } diff --git a/src/xrpld/app/misc/LendingHelpers.h b/src/xrpld/app/misc/LendingHelpers.h index bc8e5c3b9a..cea123d75d 100644 --- a/src/xrpld/app/misc/LendingHelpers.h +++ b/src/xrpld/app/misc/LendingHelpers.h @@ -237,19 +237,21 @@ loanLatePaymentInterest( originalPrincipal); } -struct PeriodicPaymentParts +struct PaymentParts { Number interest; Number principal; + Number fee; }; template -PeriodicPaymentParts +PaymentParts computePeriodicPaymentParts( A const& asset, Number const& originalPrincipal, Number const& principalOutstanding, Number const& periodicPaymentAmount, + Number const& serviceFee, Number const& periodicRate, std::uint32_t paymentRemaining) { @@ -257,6 +259,8 @@ computePeriodicPaymentParts( * This function is derived from the XLS-66 spec, section 3.2.4.1.1 (Regular * Payment) */ + Number const roundedFee = + roundToAsset(asset, serviceFee, originalPrincipal); if (paymentRemaining == 1) { // If there's only one payment left, we need to pay off the principal. @@ -265,7 +269,10 @@ computePeriodicPaymentParts( periodicPaymentAmount - principalOutstanding, originalPrincipal, Number::upward); - return {interest, principalOutstanding}; + return { + .interest = interest, + .principal = principalOutstanding, + .fee = roundedFee}; } /* * From the spec, once the periodicPayment is computed: @@ -320,7 +327,7 @@ computePeriodicPaymentParts( "ripple::computePeriodicPaymentParts", "valid principal"); - return {interest, principal}; + return {.interest = interest, .principal = principal, .fee = roundedFee}; } // This structure is explained in the XLS-66 spec, section 3.2.4.4 (Failure @@ -342,16 +349,18 @@ struct LoanPaymentParts Number feePaid; }; -struct LatePaymentParams -{ -}; - /* Handle possible late payments. * * If this function processed a late payment, the return value will be * a LoanPaymentParts object. If the loan is not late, the return will be an * Unexpected(tesSUCCESS). Otherwise, it'll be an Unexpected with the error code * the caller is expected to return. + * + * + * This function is an implementation of the XLS-66 spec, based on + * * section 3.2.4.3 (Transaction Pseudo-code), specifically the bit + * labeled "the payment is late" + * * section 3.2.4.1.2 (Late Payment) */ template Expected @@ -362,12 +371,11 @@ handleLatePayment( Int32Proxy& paymentRemainingProxy, Int32Proxy& prevPaymentDateProxy, Int32Proxy& nextDueDateProxy, - PeriodicPaymentParts const& periodic, + PaymentParts const& periodic, std::uint32_t const startDate, std::uint32_t const paymentInterval, TenthBips32 const lateInterestRate, Number const& originalPrincipalRequested, - Number const& periodicPaymentAmount, Number const& latePaymentFee, STAmount const& amount, beast::Journal j) @@ -376,6 +384,8 @@ handleLatePayment( return Unexpected(tesSUCCESS); // the payment is late + // Late payment interest is only the part of the interest that comes from + // being late, as computed by 3.2.4.1.2. auto const latePaymentInterest = loanLatePaymentInterest( asset, principalOutstandingProxy, @@ -387,20 +397,23 @@ handleLatePayment( XRPL_ASSERT( latePaymentInterest >= 0, "ripple::handleLatePayment : valid late interest"); - auto const latePaymentAmount = - periodicPaymentAmount + latePaymentInterest + latePaymentFee; + PaymentParts const late{ + .interest = latePaymentInterest + periodic.interest, + .principal = periodic.principal, + .fee = latePaymentFee + periodic.fee}; + auto const totalDue = late.principal + late.interest + late.fee; - if (amount < latePaymentAmount) + if (amount < totalDue) { JLOG(j.warn()) << "Late loan payment amount is insufficient. Due: " - << latePaymentAmount << ", paid: " << amount; + << totalDue << ", paid: " << amount; return Unexpected(tecINSUFFICIENT_PAYMENT); } paymentRemainingProxy -= 1; // A single payment always pays the same amount of principal. Only the // interest and fees are extra for a late payment - principalOutstandingProxy -= periodic.principal; + principalOutstandingProxy -= late.principal; // Make sure this does an assignment prevPaymentDateProxy = nextDueDateProxy; @@ -409,10 +422,10 @@ handleLatePayment( // A late payment increases the value of the loan by the difference // between periodic and late payment interest return LoanPaymentParts{ - periodic.principal, - latePaymentInterest + periodic.interest, - latePaymentInterest, - latePaymentFee}; + .principalPaid = late.principal, + .interestPaid = late.interest, + .valueChange = latePaymentInterest, + .feePaid = late.fee}; } /* Handle possible full payments. @@ -459,21 +472,21 @@ handleFullPayment( XRPL_ASSERT( accruedInterest >= 0, "ripple::handleFullPayment : valid accrued interest"); - auto const closePrepaymentInterest = roundToAsset( + auto const prepaymentPenalty = roundToAsset( asset, tenthBipsOfValue(principalOutstandingProxy.value(), closeInterestRate), originalPrincipalRequested); XRPL_ASSERT( - closePrepaymentInterest >= 0, + prepaymentPenalty >= 0, "ripple::handleFullPayment : valid prepayment " "interest"); - auto const totalInterest = accruedInterest + closePrepaymentInterest; + auto const totalInterest = accruedInterest + prepaymentPenalty; auto const closeFullPayment = principalOutstandingProxy + totalInterest + closePaymentFee; if (amount < closeFullPayment) // If the payment is less than the full payment amount, it's not - // sufficient to be a full payment. + // sufficient to be a full payment, but that's not an error. return Unexpected(tesSUCCESS); // Make a full payment @@ -484,7 +497,10 @@ handleFullPayment( auto const valueChange = totalInterest - totalInterestOutstanding; LoanPaymentParts const result{ - principalOutstandingProxy, totalInterest, valueChange, closePaymentFee}; + .principalPaid = principalOutstandingProxy, + .interestPaid = totalInterest, + .valueChange = valueChange, + .feePaid = closePaymentFee}; paymentRemainingProxy = 0; principalOutstandingProxy = 0; @@ -556,6 +572,7 @@ loanMakePayment( originalPrincipalRequested, principalOutstandingProxy, periodicPaymentAmount, + serviceFee, periodicRate, paymentRemainingProxy); @@ -596,7 +613,6 @@ loanMakePayment( paymentInterval, lateInterestRate, originalPrincipalRequested, - periodicPaymentAmount, latePaymentFee, amount, j)) @@ -631,11 +647,7 @@ loanMakePayment( // if the payment is not late nor if it's a full payment, then it must // be a periodic one, with possible overpayments - auto const totalDue = roundToAsset( - asset, - periodicPaymentAmount + serviceFee, - originalPrincipalRequested, - Number::upward); + auto const totalDue = periodic.interest + periodic.principal + periodic.fee; std::optional mg(Number::downward); std::int64_t const fullPeriodicPayments = [&]() { @@ -654,7 +666,7 @@ loanMakePayment( if (fullPeriodicPayments < 1) { JLOG(j.warn()) << "Periodic loan payment amount is insufficient. Due: " - << periodicPaymentAmount << ", paid: " << amount; + << totalDue << ", paid: " << amount; return Unexpected(tecINSUFFICIENT_PAYMENT); } @@ -665,7 +677,7 @@ loanMakePayment( Number totalInterestPaid = 0; Number loanValueChange = 0; - std::optional future = periodic; + std::optional future = periodic; for (int i = 0; i < fullPeriodicPayments; ++i) { // Only do the work if we need to @@ -675,6 +687,7 @@ loanMakePayment( originalPrincipalRequested, principalOutstandingProxy, periodicPaymentAmount, + serviceFee, periodicRate, paymentRemainingProxy); XRPL_ASSERT( @@ -763,7 +776,10 @@ loanMakePayment( totalFeePaid, "ripple::loanMakePayment : totalFeePaid rounded"); return LoanPaymentParts{ - totalPrincipalPaid, totalInterestPaid, loanValueChange, totalFeePaid}; + .principalPaid = totalPrincipalPaid, + .interestPaid = totalInterestPaid, + .valueChange = loanValueChange, + .feePaid = totalFeePaid}; } } // namespace ripple diff --git a/src/xrpld/app/tx/detail/InvariantCheck.h b/src/xrpld/app/tx/detail/InvariantCheck.h index c3725cbc51..b2527c53e0 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.h +++ b/src/xrpld/app/tx/detail/InvariantCheck.h @@ -176,7 +176,7 @@ public: class AccountRootsDeletedClean { // Pair is . Before is used for most of the checks, so that - // if, for example, and object ID field is cleared, but the object is not + // if, for example, an object ID field is cleared, but the object is not // deleted, it can still be found. After is used specifically for any checks // that are expected as part of the deletion, such as zeroing out the // balance. diff --git a/src/xrpld/app/tx/detail/LoanManage.cpp b/src/xrpld/app/tx/detail/LoanManage.cpp index 2e2815db6a..38efff1570 100644 --- a/src/xrpld/app/tx/detail/LoanManage.cpp +++ b/src/xrpld/app/tx/detail/LoanManage.cpp @@ -133,14 +133,13 @@ LoanManage::preclaim(PreclaimContext const& ctx) } TER -defaultLoan( +LoanManage::defaultLoan( ApplyView& view, SLE::ref loanSle, SLE::ref brokerSle, SLE::ref vaultSle, Number const& principalOutstanding, Number const& interestOutstanding, - std::uint32_t paymentInterval, Asset const& vaultAsset, beast::Journal j) { @@ -260,15 +259,12 @@ defaultLoan( } TER -impairLoan( +LoanManage::impairLoan( ApplyView& view, SLE::ref loanSle, - SLE::ref brokerSle, SLE::ref vaultSle, Number const& principalOutstanding, Number const& interestOutstanding, - std::uint32_t paymentInterval, - Asset const& vaultAsset, beast::Journal j) { // Update the Vault object(set "paper loss") @@ -300,15 +296,13 @@ impairLoan( } TER -unimpairLoan( +LoanManage::unimpairLoan( ApplyView& view, SLE::ref loanSle, - SLE::ref brokerSle, SLE::ref vaultSle, Number const& principalOutstanding, Number const& interestOutstanding, std::uint32_t paymentInterval, - Asset const& vaultAsset, beast::Journal j) { // Update the Vault object(clear "paper loss") @@ -394,7 +388,6 @@ LoanManage::doApply() vaultSle, principalOutstanding, interestOutstanding, - paymentInterval, vaultAsset, j_)) return ter; @@ -404,12 +397,9 @@ LoanManage::doApply() if (auto const ter = impairLoan( view, loanSle, - brokerSle, vaultSle, principalOutstanding, interestOutstanding, - paymentInterval, - vaultAsset, j_)) return ter; } @@ -418,12 +408,10 @@ LoanManage::doApply() if (auto const ter = unimpairLoan( view, loanSle, - brokerSle, vaultSle, principalOutstanding, interestOutstanding, paymentInterval, - vaultAsset, j_)) return ter; } diff --git a/src/xrpld/app/tx/detail/LoanManage.h b/src/xrpld/app/tx/detail/LoanManage.h index ade02f4c20..92c5854115 100644 --- a/src/xrpld/app/tx/detail/LoanManage.h +++ b/src/xrpld/app/tx/detail/LoanManage.h @@ -45,6 +45,42 @@ public: static TER preclaim(PreclaimContext const& ctx); + /** Helper function that might be needed by other transactors + */ + static TER + defaultLoan( + ApplyView& view, + SLE::ref loanSle, + SLE::ref brokerSle, + SLE::ref vaultSle, + Number const& principalOutstanding, + Number const& interestOutstanding, + Asset const& vaultAsset, + beast::Journal j); + + /** Helper function that might be needed by other transactors + */ + static TER + impairLoan( + ApplyView& view, + SLE::ref loanSle, + SLE::ref vaultSle, + Number const& principalOutstanding, + Number const& interestOutstanding, + beast::Journal j); + + /** Helper function that might be needed by other transactors + */ + static TER + unimpairLoan( + ApplyView& view, + SLE::ref loanSle, + SLE::ref vaultSle, + Number const& principalOutstanding, + Number const& interestOutstanding, + std::uint32_t paymentInterval, + beast::Journal j); + TER doApply() override; }; diff --git a/src/xrpld/app/tx/detail/LoanPay.cpp b/src/xrpld/app/tx/detail/LoanPay.cpp index 92dbbd2044..fe58c66711 100644 --- a/src/xrpld/app/tx/detail/LoanPay.cpp +++ b/src/xrpld/app/tx/detail/LoanPay.cpp @@ -20,6 +20,7 @@ #include // #include +#include namespace ripple { @@ -148,6 +149,37 @@ LoanPay::doApply() // Loan object state changes Number const originalPrincipalRequested = loanSle->at(sfPrincipalRequested); + // Unimpair the loan if it was impaired. Do this before the payment is + // attempted, so the original values can be used. If the payment fails, this + // change will be discarded. + if (loanSle->isFlag(lsfLoanImpaired)) + { + TenthBips32 const interestRate{loanSle->at(sfInterestRate)}; + auto const principalOutstanding = loanSle->at(sfPrincipalOutstanding); + + TenthBips32 const managementFeeRate{brokerSle->at(sfManagementFeeRate)}; + auto const paymentInterval = loanSle->at(sfPaymentInterval); + auto const paymentsRemaining = loanSle->at(sfPaymentRemaining); + + auto const interestOutstanding = loanInterestOutstandingMinusFee( + asset, + originalPrincipalRequested, + principalOutstanding.value(), + interestRate, + paymentInterval, + paymentsRemaining, + managementFeeRate); + + LoanManage::unimpairLoan( + view, + loanSle, + vaultSle, + principalOutstanding, + interestOutstanding, + paymentInterval, + j_); + } + Expected paymentParts = loanMakePayment(asset, view, loanSle, amount, j_); @@ -158,9 +190,6 @@ LoanPay::doApply() // has been modified. view.update(loanSle); - // If the loan was impaired, it isn't anymore. - loanSle->clearFlag(lsfLoanImpaired); - XRPL_ASSERT_PARTS( paymentParts->principalPaid > 0, "ripple::LoanPay::doApply",