Skip to content

Commit 91d239f

Browse files
ximinezTapanitoshawnxie999a1q123456gregtatcam
authored
Improve and fix bugs in Lending Protocol - XLS-66 (#6156)
- Fix overpayment asserts (#6084) - MPTTester::operator() parameter should be std::int64_t - Originally defined as uint64_t, but the testIssuerLoan() test called it with a negative number, causing an overflow to a very large number that in some circumstances could be silently cast back to an int64_t, but might not be. I believe this is UB, and we don't want to rely on that. - Fix Overpayment Calculation (#6087) - Adds additional unit tests to cover math calculations. - Removes unused methods. - Fix LoanBrokerSet debtMaximum limits (#6116) - Fix some minor bugs in Lending Protocol (#6101) - add nodiscard to unimpairLoan, and check result in LoanPay - add a check to verify that issuer exists - improve LoanManage error code for dust amounts - In overpayment results, the management fee was being calculated twice: once as part of the value change, and as part of the fees paid. Exclude it from the value change. - Round the initial total value computation upward, unless there is 0-interest. - Rename getVaultScale to getAssetsTotalScale, and convert one incorrect computation to use it. - Use adjustImpreciseNumber for LossUnrealized. - Add some logging to computeLoanProperties. - Check permissions in LoanSet and LoanPay (#6108) - Disallow pseudo accounts to be Destination for LoanBrokerCoverWithdraw (#6106) - Ensure vault asset cap is not exceeded (#6124) - Fix Overpayment ValueChange calculation in Lending Protocol (#6114) - Adds loan state to LoanProperties. - Cleans up computeLoanProperties. - Fixes missing management fee from overpayment. - fix: Enable LP Deposits when the broker is the asset issuer (#6119) - Add a few minor changes (#6158) - Updates or fixes a couple of things I noticed while reviewing changes to the spec. - Rename sfPreviousPaymentDate to sfPreviousPaymentDueDate. - Make the vault asset cap check added in #6124 a little more robust: 1. Check in preflight if the vault is _already_ over the limit. 2. Prevent overflow when checking with the loan value. (Subtract instead of adding, in case the values are near maxint. Both return the same result. Also add a unit test so each case is covered. - Add minimum grace period validation (#6133) - Fix bugs: frozen pseudo-account, and FLC cutoff (#6170) - Fixes LoanManage tfBAD_LEDGER case by capping the amount of FLC to use to cover a loss at the amount of cover available. - Check if the Vault pseudo-account is frozen in LoanBrokerSet - refactor: Rename raw state to theoretical state (#6187) - Check if a withdrawal amount exceeds any applicable receiving limit. (#6117) - Check the trust line limit is not exceeded for a withdraw to a third party Destination account. - Fix test failures from interaction between #6120 and #6133 - LoanSet transaction added in #6120 failed the minimum grace period added by #6133. - Fix overpayment result calculation (#6195) - Address review feedback from Lending Protocol re-review (#6161) - Reduce code duplication in LoanBrokerDelete - Reorder "canWithdraw" parameters to put "view" first - Combine accountSpendable into accountHolds - Avoid copies by taking a reference for the claw amount - Return function results directly - Fix typo for "parseLoan" in ledger_entry RPC - Improve some comments and unused variables - No need for late payment components lambda - Add explanatory comment for computeLoanProperties - Add comment linking computeRawLoanState to spec - Fix typo: TrueTotalPrincipalOutstanding - Fix missed ripple -> xrpl update - Remove unnecessary "else"s. - Clean up std::visit in accountHolds. --------- Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Co-authored-by: Shawn Xie <35279399+shawnxie999@users.noreply.github.com> Co-authored-by: Jingchen <a1q123456@users.noreply.github.com> Co-authored-by: Gregory Tsipenyuk <gregtatcam@users.noreply.github.com>
1 parent 418ce68 commit 91d239f

30 files changed

+3238
-745
lines changed
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
*/
1818
//==============================================================================
1919

20-
#ifndef RIPPLE_APP_PATHS_CREDIT_H_INCLUDED
21-
#define RIPPLE_APP_PATHS_CREDIT_H_INCLUDED
20+
#ifndef RIPPLE_LEDGER_CREDIT_H_INCLUDED
21+
#define RIPPLE_LEDGER_CREDIT_H_INCLUDED
2222

2323
#include <xrpl/ledger/View.h>
2424
#include <xrpl/protocol/IOUAmount.h>

include/xrpl/ledger/View.h

Lines changed: 33 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ enum FreezeHandling { fhIGNORE_FREEZE, fhZERO_IF_FROZEN };
8080
/** Controls the treatment of unauthorized MPT balances */
8181
enum AuthHandling { ahIGNORE_AUTH, ahZERO_IF_UNAUTHORIZED };
8282

83+
/** Controls whether to include the account's full spendable balance */
84+
enum SpendableHandling { shSIMPLE_BALANCE, shFULL_BALANCE };
85+
8386
[[nodiscard]] bool
8487
isGlobalFrozen(ReadView const& view, AccountID const& issuer);
8588

@@ -324,7 +327,17 @@ isLPTokenFrozen(
324327
Issue const& asset,
325328
Issue const& asset2);
326329

327-
// Returns the amount an account can spend without going into debt.
330+
// Returns the amount an account can spend.
331+
//
332+
// If shSIMPLE_BALANCE is specified, this is the amount the account can spend
333+
// without going into debt.
334+
//
335+
// If shFULL_BALANCE is specified, this is the amount the account can spend
336+
// total. Specifically:
337+
// * The account can go into debt if using a trust line, and the other side has
338+
// a non-zero limit.
339+
// * If the account is the asset issuer the limit is defined by the asset /
340+
// issuance.
328341
//
329342
// <-- saAmount: amount of currency held by account. May be negative.
330343
[[nodiscard]] STAmount
@@ -334,15 +347,17 @@ accountHolds(
334347
Currency const& currency,
335348
AccountID const& issuer,
336349
FreezeHandling zeroIfFrozen,
337-
beast::Journal j);
350+
beast::Journal j,
351+
SpendableHandling includeFullBalance = shSIMPLE_BALANCE);
338352

339353
[[nodiscard]] STAmount
340354
accountHolds(
341355
ReadView const& view,
342356
AccountID const& account,
343357
Issue const& issue,
344358
FreezeHandling zeroIfFrozen,
345-
beast::Journal j);
359+
beast::Journal j,
360+
SpendableHandling includeFullBalance = shSIMPLE_BALANCE);
346361

347362
[[nodiscard]] STAmount
348363
accountHolds(
@@ -351,7 +366,8 @@ accountHolds(
351366
MPTIssue const& mptIssue,
352367
FreezeHandling zeroIfFrozen,
353368
AuthHandling zeroIfUnauthorized,
354-
beast::Journal j);
369+
beast::Journal j,
370+
SpendableHandling includeFullBalance = shSIMPLE_BALANCE);
355371

356372
[[nodiscard]] STAmount
357373
accountHolds(
@@ -360,50 +376,8 @@ accountHolds(
360376
Asset const& asset,
361377
FreezeHandling zeroIfFrozen,
362378
AuthHandling zeroIfUnauthorized,
363-
beast::Journal j);
364-
365-
// Returns the amount an account can spend total.
366-
//
367-
// These functions use accountHolds, but unlike accountHolds:
368-
// * The account can go into debt.
369-
// * If the account is the asset issuer the only limit is defined by the asset /
370-
// issuance.
371-
//
372-
// <-- saAmount: amount of currency held by account. May be negative.
373-
[[nodiscard]] STAmount
374-
accountSpendable(
375-
ReadView const& view,
376-
AccountID const& account,
377-
Currency const& currency,
378-
AccountID const& issuer,
379-
FreezeHandling zeroIfFrozen,
380-
beast::Journal j);
381-
382-
[[nodiscard]] STAmount
383-
accountSpendable(
384-
ReadView const& view,
385-
AccountID const& account,
386-
Issue const& issue,
387-
FreezeHandling zeroIfFrozen,
388-
beast::Journal j);
389-
390-
[[nodiscard]] STAmount
391-
accountSpendable(
392-
ReadView const& view,
393-
AccountID const& account,
394-
MPTIssue const& mptIssue,
395-
FreezeHandling zeroIfFrozen,
396-
AuthHandling zeroIfUnauthorized,
397-
beast::Journal j);
398-
399-
[[nodiscard]] STAmount
400-
accountSpendable(
401-
ReadView const& view,
402-
AccountID const& account,
403-
Asset const& asset,
404-
FreezeHandling zeroIfFrozen,
405-
AuthHandling zeroIfUnauthorized,
406-
beast::Journal j);
379+
beast::Journal j,
380+
SpendableHandling includeFullBalance = shSIMPLE_BALANCE);
407381

408382
// Returns the amount an account can spend of the currency type saDefault, or
409383
// returns saDefault if this account is the issuer of the currency in
@@ -674,7 +648,7 @@ createPseudoAccount(
674648
uint256 const& pseudoOwnerKey,
675649
SField const& ownerField);
676650

677-
// Returns true iff sleAcct is a pseudo-account or specific
651+
// Returns true if and only if sleAcct is a pseudo-account or specific
678652
// pseudo-accounts in pseudoFieldFilter.
679653
//
680654
// Returns false if sleAcct is
@@ -729,13 +703,16 @@ checkDestinationAndTag(SLE::const_ref toSle, bool hasDestinationTag);
729703
* - If withdrawing to self, succeed.
730704
* - If not, checks if the receiver requires deposit authorization, and if
731705
* the sender has it.
706+
* - Checks that the receiver will not exceed the limit (IOU trustline limit
707+
* or MPT MaximumAmount).
732708
*/
733709
[[nodiscard]] TER
734710
canWithdraw(
735-
AccountID const& from,
736711
ReadView const& view,
712+
AccountID const& from,
737713
AccountID const& to,
738714
SLE::const_ref toSle,
715+
STAmount const& amount,
739716
bool hasDestinationTag);
740717

741718
/** Checks that can withdraw funds from an object to itself or a destination.
@@ -749,12 +726,15 @@ canWithdraw(
749726
* - If withdrawing to self, succeed.
750727
* - If not, checks if the receiver requires deposit authorization, and if
751728
* the sender has it.
729+
* - Checks that the receiver will not exceed the limit (IOU trustline limit
730+
* or MPT MaximumAmount).
752731
*/
753732
[[nodiscard]] TER
754733
canWithdraw(
755-
AccountID const& from,
756734
ReadView const& view,
735+
AccountID const& from,
757736
AccountID const& to,
737+
STAmount const& amount,
758738
bool hasDestinationTag);
759739

760740
/** Checks that can withdraw funds from an object to itself or a destination.
@@ -768,6 +748,8 @@ canWithdraw(
768748
* - If withdrawing to self, succeed.
769749
* - If not, checks if the receiver requires deposit authorization, and if
770750
* the sender has it.
751+
* - Checks that the receiver will not exceed the limit (IOU trustline limit
752+
* or MPT MaximumAmount).
771753
*/
772754
[[nodiscard]] TER
773755
canWithdraw(ReadView const& view, STTx const& tx);

include/xrpl/protocol/detail/ledger_entries.macro

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ LEDGER_ENTRY(ltLOAN, 0x0089, Loan, loan, ({
560560
{sfStartDate, soeREQUIRED},
561561
{sfPaymentInterval, soeREQUIRED},
562562
{sfGracePeriod, soeDEFAULT},
563-
{sfPreviousPaymentDate, soeDEFAULT},
563+
{sfPreviousPaymentDueDate, soeDEFAULT},
564564
{sfNextPaymentDueDate, soeDEFAULT},
565565
// The loan object tracks these values:
566566
//

include/xrpl/protocol/detail/sfields.macro

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ TYPED_SFIELD(sfMutableFlags, UINT32, 53)
121121
TYPED_SFIELD(sfStartDate, UINT32, 54)
122122
TYPED_SFIELD(sfPaymentInterval, UINT32, 55)
123123
TYPED_SFIELD(sfGracePeriod, UINT32, 56)
124-
TYPED_SFIELD(sfPreviousPaymentDate, UINT32, 57)
124+
TYPED_SFIELD(sfPreviousPaymentDueDate, UINT32, 57)
125125
TYPED_SFIELD(sfNextPaymentDueDate, UINT32, 58)
126126
TYPED_SFIELD(sfPaymentRemaining, UINT32, 59)
127127
TYPED_SFIELD(sfPaymentTotal, UINT32, 60)

0 commit comments

Comments
 (0)