-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Lending Protocol: Fix Overpayment ValueChange calculation #6114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: ximinez/lending-XLS-66-ongoing
Are you sure you want to change the base?
Changes from 7 commits
7216120
e27fecd
cf33c9d
f9438cc
d7c1fbb
7f25661
e47d691
aa1386f
9a95502
478cb46
5546ffa
5e4f593
d3dd60b
f45e972
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -620,10 +620,117 @@ class LendingHelpers_test : public beast::unit_test::suite | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| void | ||||||
| testTryOverpaymentValueChange() | ||||||
| { | ||||||
| // This test ensures that overpayment value change is computed | ||||||
| // correctly. | ||||||
| testcase("tryOverpayment - Value Change is the decrease in interest"); | ||||||
|
|
||||||
| using namespace jtx; | ||||||
| using namespace ripple::detail; | ||||||
|
|
||||||
| Env env{*this}; | ||||||
| Account const issuer{"issuer"}; | ||||||
| PrettyAsset const asset = issuer["USD"]; | ||||||
|
|
||||||
| // Interest delta is 40 (100 - 50 - 10) | ||||||
| ExtendedPaymentComponents const overpaymentComponents = { | ||||||
| PaymentComponents{ | ||||||
| .trackedValueDelta = Number{50, 0}, | ||||||
| .trackedPrincipalDelta = Number{50, 0}, | ||||||
| .trackedManagementFeeDelta = Number{0, 0}, | ||||||
| .specialCase = PaymentSpecialCase::extra, | ||||||
| }, | ||||||
| numZero, | ||||||
| numZero, | ||||||
| }; | ||||||
|
|
||||||
| TenthBips16 managementFeeRate{20'000}; // 10% | ||||||
| TenthBips32 loanInterestRate{10'000}; // 20% | ||||||
| Number loanPrincipal{1'000}; | ||||||
| std::uint32_t paymentInterval = 30 * 24 * 60 * 60; | ||||||
| std::uint32_t paymentsRemaining = 10; | ||||||
| std::int32_t loanScale = -5; | ||||||
| auto const periodicRate = | ||||||
| loanPeriodicRate(loanInterestRate, paymentInterval); | ||||||
|
|
||||||
| auto loanProperites = computeLoanProperties( | ||||||
| asset, | ||||||
| loanPrincipal, | ||||||
| loanInterestRate, | ||||||
| paymentInterval, | ||||||
| paymentsRemaining, | ||||||
| managementFeeRate, | ||||||
| loanScale); | ||||||
| std::cout << loanProperites.periodicPayment << std::endl; | ||||||
| std::cout << loanProperites.loanState.valueOutstanding << std::endl; | ||||||
| std::cout << loanProperites.loanState.interestOutstanding() | ||||||
| << std::endl; | ||||||
|
||||||
|
|
||||||
| Number periodicPayment = loanProperites.periodicPayment; | ||||||
|
|
||||||
| auto const ret = tryOverpayment( | ||||||
| asset, | ||||||
| loanScale, | ||||||
| overpaymentComponents, | ||||||
| loanProperites.loanState, | ||||||
| periodicPayment, | ||||||
| periodicRate, | ||||||
| paymentsRemaining, | ||||||
| managementFeeRate, | ||||||
| env.journal); | ||||||
|
|
||||||
| BEAST_EXPECT(ret); | ||||||
|
|
||||||
| auto const& [actualPaymentParts, newLoanProperties] = *ret; | ||||||
| auto const newState = newLoanProperties.loanState; | ||||||
|
||||||
| auto const newState = newLoanProperties.loanState; | |
| auto const& newState = newLoanProperties.loanState; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -700,17 +700,17 @@ class Loan_test : public beast::unit_test::suite | |
| interval, | ||
| total, | ||
| feeRate, | ||
| asset(brokerParams.vaultDeposit).number().exponent(), | ||
| env.journal); | ||
| asset(brokerParams.vaultDeposit).number().exponent()); | ||
| log << "Loan properties:\n" | ||
| << "\tPrincipal: " << principal << std::endl | ||
| << "\tInterest rate: " << interest << std::endl | ||
| << "\tPayment interval: " << interval << std::endl | ||
| << "\tManagement Fee Rate: " << feeRate << std::endl | ||
| << "\tTotal Payments: " << total << std::endl | ||
| << "\tPeriodic Payment: " << props.periodicPayment << std::endl | ||
| << "\tTotal Value: " << props.totalValueOutstanding << std::endl | ||
| << "\tManagement Fee: " << props.managementFeeOwedToBroker | ||
| << "\tTotal Value: " << props.loanState.valueOutstanding | ||
| << std::endl | ||
| << "\tManagement Fee: " << props.loanState.managementFeeDue | ||
| << std::endl | ||
| << "\tLoan Scale: " << props.loanScale << std::endl | ||
| << "\tFirst payment principal: " << props.firstPaymentPrincipal | ||
|
|
@@ -860,9 +860,6 @@ class Loan_test : public beast::unit_test::suite | |
| using namespace std::chrono_literals; | ||
| using d = NetClock::duration; | ||
|
|
||
| // Account const evan{"evan"}; | ||
| // Account const alice{"alice"}; | ||
|
|
||
| bool const showStepBalances = paymentParams.showStepBalances; | ||
|
|
||
| auto const currencyLabel = getCurrencyLabel(broker.asset); | ||
|
|
@@ -1482,17 +1479,16 @@ class Loan_test : public beast::unit_test::suite | |
| state.paymentInterval, | ||
| state.paymentRemaining, | ||
| broker.params.managementFeeRate, | ||
| state.loanScale, | ||
| env.journal); | ||
| state.loanScale); | ||
|
|
||
| verifyLoanStatus( | ||
| 0, | ||
| startDate + *loanParams.payInterval, | ||
| *loanParams.payTotal, | ||
| state.loanScale, | ||
| loanProperties.totalValueOutstanding, | ||
| loanProperties.loanState.valueOutstanding, | ||
| principalRequestAmount, | ||
| loanProperties.managementFeeOwedToBroker, | ||
| loanProperties.loanState.managementFeeDue, | ||
| loanProperties.periodicPayment, | ||
| loanFlags | 0); | ||
|
|
||
|
|
@@ -1547,9 +1543,9 @@ class Loan_test : public beast::unit_test::suite | |
| nextDueDate, | ||
| *loanParams.payTotal, | ||
| loanProperties.loanScale, | ||
| loanProperties.totalValueOutstanding, | ||
| loanProperties.loanState.valueOutstanding, | ||
| principalRequestAmount, | ||
| loanProperties.managementFeeOwedToBroker, | ||
| loanProperties.loanState.managementFeeDue, | ||
| loanProperties.periodicPayment, | ||
| loanFlags | 0); | ||
|
|
||
|
|
@@ -4466,15 +4462,6 @@ class Loan_test : public beast::unit_test::suite | |
| }; | ||
| } | ||
|
|
||
| void | ||
| testBasicMath() | ||
| { | ||
| // Test the functions defined in LendingHelpers.h | ||
| testcase("Basic Math"); | ||
|
|
||
| pass(); | ||
| } | ||
|
Comment on lines
-4470
to
-4477
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Woops. That one slipped through the cracks. LOL. |
||
|
|
||
| void | ||
| testIssuerLoan() | ||
| { | ||
|
|
@@ -7019,11 +7006,8 @@ class Loan_test : public beast::unit_test::suite | |
| env.close(); | ||
|
|
||
| PaymentParameters paymentParams{ | ||
| //.overpaymentFactor = Number{15, -1}, | ||
| //.overpaymentExtra = Number{1, -6}, | ||
| //.flags = tfLoanOverpayment, | ||
| .showStepBalances = true, | ||
| //.validateBalances = false, | ||
| .showStepBalances = false, | ||
| .validateBalances = true, | ||
| }; | ||
|
|
||
| makeLoanPayments( | ||
|
|
@@ -7038,6 +7022,69 @@ class Loan_test : public beast::unit_test::suite | |
| paymentParams); | ||
| } | ||
|
|
||
| void | ||
| testOverpaymentManagementFee() | ||
| { | ||
| testcase("testOverpaymentManagementFee"); | ||
|
|
||
| using namespace jtx; | ||
| using namespace loan; | ||
|
|
||
| Env env(*this, all); | ||
|
|
||
| Account const lender{"lender"}, borrower{"borrower"}; | ||
|
|
||
| env.fund(XRP(10'000'000), lender, borrower); | ||
| env.close(); | ||
|
|
||
| PrettyAsset const asset{xrpIssue(), 1000}; | ||
|
|
||
| auto const result = createVaultAndBroker( | ||
| env, | ||
| asset, | ||
| lender, | ||
| { | ||
| .vaultDeposit = asset(100'000).value(), | ||
| .managementFeeRate = TenthBips16(10'000), | ||
| }); | ||
|
|
||
| auto const loanSetFee = fee(env.current()->fees().base * 2); | ||
|
|
||
| auto const loanKeylet = keylet::loan( | ||
| result.brokerKeylet().key, | ||
| (env.le(result.brokerKeylet()))->at(sfLoanSequence)); | ||
| env(loan::set( | ||
| borrower, | ||
| result.brokerKeylet().key, | ||
| asset(10'000).value(), | ||
| tfLoanOverpayment), | ||
| sig(sfCounterpartySignature, lender), | ||
| loan::paymentInterval(86400 * 30), | ||
| loan::paymentTotal(3), | ||
| loan::overpaymentInterestRate( | ||
| TenthBips32(percentageToTenthBips(20))), | ||
| loanSetFee); | ||
|
|
||
| // From calculator | ||
| auto const expectedOverpaymentManagementFee = Number{33333, 0}; | ||
| auto const loanBrokerBalanceBefore = env.balance(lender); | ||
|
|
||
| auto const loanPayFee = fee(env.current()->fees().base * 2); | ||
| env(pay(borrower, | ||
| loanKeylet.key, | ||
| asset(5'000).value(), | ||
| tfLoanOverpayment), | ||
| loanPayFee); | ||
| env.close(); | ||
|
|
||
| BEAST_EXPECTS( | ||
| env.balance(lender) - loanBrokerBalanceBefore == | ||
| expectedOverpaymentManagementFee, | ||
| "overpayment management fee missmatch; expected:" + | ||
| to_string(expectedOverpaymentManagementFee) + " got: " + | ||
| to_string(env.balance(lender) - loanBrokerBalanceBefore)); | ||
| } | ||
|
|
||
| public: | ||
| void | ||
| run() override | ||
|
|
@@ -7057,8 +7104,6 @@ class Loan_test : public beast::unit_test::suite | |
| testServiceFeeOnBrokerDeepFreeze(); | ||
|
|
||
| testRPC(); | ||
| testBasicMath(); | ||
|
|
||
| testInvalidLoanDelete(); | ||
| testInvalidLoanManage(); | ||
| testInvalidLoanPay(); | ||
|
|
@@ -7086,6 +7131,7 @@ class Loan_test : public beast::unit_test::suite | |
| testBorrowerIsBroker(); | ||
| testIssuerIsBorrower(); | ||
| testLimitExceeded(); | ||
| testOverpaymentManagementFee(); | ||
| } | ||
| }; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comments on these values are switched.