Skip to content

Commit c6fcce0

Browse files
ximinezximinez
authored
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.
1 parent 10c5abe commit c6fcce0

10 files changed

Lines changed: 215 additions & 260 deletions

File tree

‎include/xrpl/ledger/View.h‎

Lines changed: 24 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ enum FreezeHandling { fhIGNORE_FREEZE, fhZERO_IF_FROZEN };
6161
/** Controls the treatment of unauthorized MPT balances */
6262
enum AuthHandling { ahIGNORE_AUTH, ahZERO_IF_UNAUTHORIZED };
6363

64+
/** Controls whether to include the account's full spendable balance */
65+
enum SpendableHandling { shSIMPLE_BALANCE, shFULL_BALANCE };
66+
6467
[[nodiscard]] bool
6568
isGlobalFrozen(ReadView const& view, AccountID const& issuer);
6669

@@ -305,7 +308,17 @@ isLPTokenFrozen(
305308
Issue const& asset,
306309
Issue const& asset2);
307310

308-
// Returns the amount an account can spend without going into debt.
311+
// Returns the amount an account can spend.
312+
//
313+
// If shSIMPLE_BALANCE is specified, this is the amount the account can spend
314+
// without going into debt.
315+
//
316+
// If shFULL_BALANCE is specified, this is the amount the account can spend
317+
// total. Specifically:
318+
// * The account can go into debt if using a trust line, and the other side has
319+
// a non-zero limit.
320+
// * If the account is the asset issuer the limit is defined by the asset /
321+
// issuance.
309322
//
310323
// <-- saAmount: amount of currency held by account. May be negative.
311324
[[nodiscard]] STAmount
@@ -315,15 +328,17 @@ accountHolds(
315328
Currency const& currency,
316329
AccountID const& issuer,
317330
FreezeHandling zeroIfFrozen,
318-
beast::Journal j);
331+
beast::Journal j,
332+
SpendableHandling includeFullBalance = shSIMPLE_BALANCE);
319333

320334
[[nodiscard]] STAmount
321335
accountHolds(
322336
ReadView const& view,
323337
AccountID const& account,
324338
Issue const& issue,
325339
FreezeHandling zeroIfFrozen,
326-
beast::Journal j);
340+
beast::Journal j,
341+
SpendableHandling includeFullBalance = shSIMPLE_BALANCE);
327342

328343
[[nodiscard]] STAmount
329344
accountHolds(
@@ -332,7 +347,8 @@ accountHolds(
332347
MPTIssue const& mptIssue,
333348
FreezeHandling zeroIfFrozen,
334349
AuthHandling zeroIfUnauthorized,
335-
beast::Journal j);
350+
beast::Journal j,
351+
SpendableHandling includeFullBalance = shSIMPLE_BALANCE);
336352

337353
[[nodiscard]] STAmount
338354
accountHolds(
@@ -341,50 +357,8 @@ accountHolds(
341357
Asset const& asset,
342358
FreezeHandling zeroIfFrozen,
343359
AuthHandling zeroIfUnauthorized,
344-
beast::Journal j);
345-
346-
// Returns the amount an account can spend total.
347-
//
348-
// These functions use accountHolds, but unlike accountHolds:
349-
// * The account can go into debt.
350-
// * If the account is the asset issuer the only limit is defined by the asset /
351-
// issuance.
352-
//
353-
// <-- saAmount: amount of currency held by account. May be negative.
354-
[[nodiscard]] STAmount
355-
accountSpendable(
356-
ReadView const& view,
357-
AccountID const& account,
358-
Currency const& currency,
359-
AccountID const& issuer,
360-
FreezeHandling zeroIfFrozen,
361-
beast::Journal j);
362-
363-
[[nodiscard]] STAmount
364-
accountSpendable(
365-
ReadView const& view,
366-
AccountID const& account,
367-
Issue const& issue,
368-
FreezeHandling zeroIfFrozen,
369-
beast::Journal j);
370-
371-
[[nodiscard]] STAmount
372-
accountSpendable(
373-
ReadView const& view,
374-
AccountID const& account,
375-
MPTIssue const& mptIssue,
376-
FreezeHandling zeroIfFrozen,
377-
AuthHandling zeroIfUnauthorized,
378-
beast::Journal j);
379-
380-
[[nodiscard]] STAmount
381-
accountSpendable(
382-
ReadView const& view,
383-
AccountID const& account,
384-
Asset const& asset,
385-
FreezeHandling zeroIfFrozen,
386-
AuthHandling zeroIfUnauthorized,
387-
beast::Journal j);
360+
beast::Journal j,
361+
SpendableHandling includeFullBalance = shSIMPLE_BALANCE);
388362

389363
// Returns the amount an account can spend of the currency type saDefault, or
390364
// returns saDefault if this account is the issuer of the currency in
@@ -715,8 +689,8 @@ checkDestinationAndTag(SLE::const_ref toSle, bool hasDestinationTag);
715689
*/
716690
[[nodiscard]] TER
717691
canWithdraw(
718-
AccountID const& from,
719692
ReadView const& view,
693+
AccountID const& from,
720694
AccountID const& to,
721695
SLE::const_ref toSle,
722696
STAmount const& amount,
@@ -738,8 +712,8 @@ canWithdraw(
738712
*/
739713
[[nodiscard]] TER
740714
canWithdraw(
741-
AccountID const& from,
742715
ReadView const& view,
716+
AccountID const& from,
743717
AccountID const& to,
744718
STAmount const& amount,
745719
bool hasDestinationTag);

‎src/libxrpl/ledger/View.cpp‎

Lines changed: 60 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -465,19 +465,28 @@ accountHolds(
465465
Currency const& currency,
466466
AccountID const& issuer,
467467
FreezeHandling zeroIfFrozen,
468-
beast::Journal j)
468+
beast::Journal j,
469+
SpendableHandling includeFullBalance)
469470
{
470471
STAmount amount;
471472
if (isXRP(currency))
472473
{
473474
return {xrpLiquid(view, account, 0, j)};
474475
}
475476

477+
bool const returnSpendable = (includeFullBalance == shFULL_BALANCE);
478+
if (returnSpendable && account == issuer)
479+
// If the account is the issuer, then their limit is effectively
480+
// infinite
481+
return STAmount{
482+
Issue{currency, issuer}, STAmount::cMaxValue, STAmount::cMaxOffset};
483+
476484
// IOU: Return balance on trust line modulo freeze
477485
SLE::const_pointer const sle =
478486
getLineIfUsable(view, account, currency, issuer, zeroIfFrozen, j);
479487

480-
return getTrustLineBalance(view, sle, account, currency, issuer, false, j);
488+
return getTrustLineBalance(
489+
view, sle, account, currency, issuer, returnSpendable, j);
481490
}
482491

483492
STAmount
@@ -486,10 +495,17 @@ accountHolds(
486495
AccountID const& account,
487496
Issue const& issue,
488497
FreezeHandling zeroIfFrozen,
489-
beast::Journal j)
498+
beast::Journal j,
499+
SpendableHandling includeFullBalance)
490500
{
491501
return accountHolds(
492-
view, account, issue.currency, issue.account, zeroIfFrozen, j);
502+
view,
503+
account,
504+
issue.currency,
505+
issue.account,
506+
zeroIfFrozen,
507+
j,
508+
includeFullBalance);
493509
}
494510

495511
STAmount
@@ -499,8 +515,28 @@ accountHolds(
499515
MPTIssue const& mptIssue,
500516
FreezeHandling zeroIfFrozen,
501517
AuthHandling zeroIfUnauthorized,
502-
beast::Journal j)
518+
beast::Journal j,
519+
SpendableHandling includeFullBalance)
503520
{
521+
bool const returnSpendable = (includeFullBalance == shFULL_BALANCE);
522+
523+
if (returnSpendable && account == mptIssue.getIssuer())
524+
{
525+
// if the account is the issuer, and the issuance exists, their limit is
526+
// the issuance limit minus the outstanding value
527+
auto const issuance =
528+
view.read(keylet::mptIssuance(mptIssue.getMptID()));
529+
530+
if (!issuance)
531+
{
532+
return STAmount{mptIssue};
533+
}
534+
return STAmount{
535+
mptIssue,
536+
issuance->at(~sfMaximumAmount).value_or(maxMPTokenAmount) -
537+
issuance->at(sfOutstandingAmount)};
538+
}
539+
504540
STAmount amount;
505541

506542
auto const sleMpt =
@@ -548,108 +584,27 @@ accountHolds(
548584
Asset const& asset,
549585
FreezeHandling zeroIfFrozen,
550586
AuthHandling zeroIfUnauthorized,
551-
beast::Journal j)
587+
beast::Journal j,
588+
SpendableHandling includeFullBalance)
552589
{
553590
return std::visit(
554-
[&](auto const& value) {
555-
if constexpr (std::is_same_v<
556-
std::remove_cvref_t<decltype(value)>,
557-
Issue>)
591+
[&]<ValidIssueType TIss>(TIss const& value) {
592+
if constexpr (std::is_same_v<TIss, Issue>)
558593
{
559-
return accountHolds(view, account, value, zeroIfFrozen, j);
594+
return accountHolds(
595+
view, account, value, zeroIfFrozen, j, includeFullBalance);
560596
}
561-
return accountHolds(
562-
view, account, value, zeroIfFrozen, zeroIfUnauthorized, j);
563-
},
564-
asset.value());
565-
}
566-
567-
STAmount
568-
accountSpendable(
569-
ReadView const& view,
570-
AccountID const& account,
571-
Currency const& currency,
572-
AccountID const& issuer,
573-
FreezeHandling zeroIfFrozen,
574-
beast::Journal j)
575-
{
576-
if (isXRP(currency))
577-
return accountHolds(view, account, currency, issuer, zeroIfFrozen, j);
578-
579-
if (account == issuer)
580-
// If the account is the issuer, then their limit is effectively
581-
// infinite
582-
return STAmount{
583-
Issue{currency, issuer}, STAmount::cMaxValue, STAmount::cMaxOffset};
584-
585-
// IOU: Return balance on trust line modulo freeze
586-
SLE::const_pointer const sle =
587-
getLineIfUsable(view, account, currency, issuer, zeroIfFrozen, j);
588-
589-
return getTrustLineBalance(view, sle, account, currency, issuer, true, j);
590-
}
591-
592-
STAmount
593-
accountSpendable(
594-
ReadView const& view,
595-
AccountID const& account,
596-
Issue const& issue,
597-
FreezeHandling zeroIfFrozen,
598-
beast::Journal j)
599-
{
600-
return accountSpendable(
601-
view, account, issue.currency, issue.account, zeroIfFrozen, j);
602-
}
603-
604-
STAmount
605-
accountSpendable(
606-
ReadView const& view,
607-
AccountID const& account,
608-
MPTIssue const& mptIssue,
609-
FreezeHandling zeroIfFrozen,
610-
AuthHandling zeroIfUnauthorized,
611-
beast::Journal j)
612-
{
613-
if (account == mptIssue.getIssuer())
614-
{
615-
// if the account is the issuer, and the issuance exists, their limit is
616-
// the issuance limit minus the outstanding value
617-
auto const issuance =
618-
view.read(keylet::mptIssuance(mptIssue.getMptID()));
619-
620-
if (!issuance)
621-
{
622-
return STAmount{mptIssue};
623-
}
624-
return STAmount{
625-
mptIssue,
626-
issuance->at(~sfMaximumAmount).value_or(maxMPTokenAmount) -
627-
issuance->at(sfOutstandingAmount)};
628-
}
629-
630-
return accountHolds(
631-
view, account, mptIssue, zeroIfFrozen, zeroIfUnauthorized, j);
632-
}
633-
634-
[[nodiscard]] STAmount
635-
accountSpendable(
636-
ReadView const& view,
637-
AccountID const& account,
638-
Asset const& asset,
639-
FreezeHandling zeroIfFrozen,
640-
AuthHandling zeroIfUnauthorized,
641-
beast::Journal j)
642-
{
643-
return std::visit(
644-
[&](auto const& value) {
645-
if constexpr (std::is_same_v<
646-
std::remove_cvref_t<decltype(value)>,
647-
Issue>)
597+
else if constexpr (std::is_same_v<TIss, MPTIssue>)
648598
{
649-
return accountSpendable(view, account, value, zeroIfFrozen, j);
599+
return accountHolds(
600+
view,
601+
account,
602+
value,
603+
zeroIfFrozen,
604+
zeroIfUnauthorized,
605+
j,
606+
includeFullBalance);
650607
}
651-
return accountSpendable(
652-
view, account, value, zeroIfFrozen, zeroIfUnauthorized, j);
653608
},
654609
asset.value());
655610
}
@@ -1389,8 +1344,8 @@ withdrawToDestExceedsLimit(
13891344

13901345
[[nodiscard]] TER
13911346
canWithdraw(
1392-
AccountID const& from,
13931347
ReadView const& view,
1348+
AccountID const& from,
13941349
AccountID const& to,
13951350
SLE::const_ref toSle,
13961351
STAmount const& amount,
@@ -1413,15 +1368,15 @@ canWithdraw(
14131368

14141369
[[nodiscard]] TER
14151370
canWithdraw(
1416-
AccountID const& from,
14171371
ReadView const& view,
1372+
AccountID const& from,
14181373
AccountID const& to,
14191374
STAmount const& amount,
14201375
bool hasDestinationTag)
14211376
{
14221377
auto const toSle = view.read(keylet::account(to));
14231378

1424-
return canWithdraw(from, view, to, toSle, amount, hasDestinationTag);
1379+
return canWithdraw(view, from, to, toSle, amount, hasDestinationTag);
14251380
}
14261381

14271382
[[nodiscard]] TER
@@ -1431,7 +1386,7 @@ canWithdraw(ReadView const& view, STTx const& tx)
14311386
auto const to = tx[~sfDestination].value_or(from);
14321387

14331388
return canWithdraw(
1434-
from, view, to, tx[sfAmount], tx.isFieldPresent(sfDestinationTag));
1389+
view, from, to, tx[sfAmount], tx.isFieldPresent(sfDestinationTag));
14351390
}
14361391

14371392
TER

0 commit comments

Comments
 (0)