Skip to content

Conversation

@Tapanito
Copy link
Collaborator

Discussion thread can be found here: #190
Development branch can be found here: TBD

@sappenin
Copy link
Collaborator

@Tapanito this PR seems ready to merge -- do you agree?

(Note that per the DRAFTS section of the CONTRIBUTING guidelines, merging does not mean endorsement or even a completed spec, but instead offers a way for spec editors to more easily edit using PRs).

@Tapanito
Copy link
Collaborator Author

Tapanito commented Dec 4, 2024

Before merging, I'd like to have at least one review by an engineer to ensure everything is in order. In addition, similarly to Single Asset Vault, it is simpler/more convenient to edit a PR with various small design changes (e.g. names), rather than open a new PR for each of them. As a result, keeping a PR open allows to have a spec. that is more readily in-alignment with implementation.

@sappenin
Copy link
Collaborator

sappenin commented Dec 4, 2024

Makes sense, especially if it's more convenient. Just didn't want you (or anyone reading along) to think that merging here implies "finished product" (that's why we have a d designator, to indicate draft status).

Tapanito and others added 7 commits January 27, 2025 16:40
- Adds VaultNode to LoanBroker object to track in which owner directory of the Vaults pseudo-account the LoanBroker object is referenced.
- Adds LoanBrokerNode to Loan object to track in which owner directory of the LoanBroker object the Loan is references.
- Replaces CurrentTime to LastClosedLedger.CloseTime.
- Changes the LoanBroker.Delete transaction to automatically return any outstanding Cover to the LoanBroker.Owner.
- Adds a balance check to the LoanBrokerCoverDeposit transaction when depositing XRP.
- Adds a check to LoanBrokerCoverWithdraw to ensure the CoverAvailable does not drop below Mimimum Cover Required.
Co-authored-by: Ed Hennis <ed@ripple.com>
@Tapanito
Copy link
Collaborator Author

Tapanito commented Jan 28, 2025 via email

@Tapanito
Copy link
Collaborator Author

Tapanito commented Jan 28, 2025 via email

Copy link
Collaborator

@ximinez ximinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updates look good.

Co-authored-by: Ed Hennis <ed@ripple.com>
@Tapanito
Copy link
Collaborator Author

Tapanito commented Mar 19, 2025 via email

@ximinez
Copy link
Collaborator

ximinez commented Mar 20, 2025

No, since we introduced a pseudo-account for the LoanBroker, the deposit will go there

Right, I wasn't thinking about that, but if there is a loan default, and funds have to be paid out of the Cover Amount, wouldn't that go to the SAV?

@Tapanito
Copy link
Collaborator Author

No, since we introduced a pseudo-account for the LoanBroker, the deposit will go there

Right, I wasn't thinking about that, but if there is a loan default, and funds have to be paid out of the Cover Amount, wouldn't that go to the SAV?

Yes, 100%, the cover amount liquidation, in essence, will become a transfer of asset from one pseduo-account to another, with some accounting state changes.

@ximinez
Copy link
Collaborator

ximinez commented Mar 21, 2025

No, since we introduced a pseudo-account for the LoanBroker, the deposit will go there

Right, I wasn't thinking about that, but if there is a loan default, and funds have to be paid out of the Cover Amount, wouldn't that go to the SAV?

Yes, 100%, the cover amount liquidation, in essence, will become a transfer of asset from one pseduo-account to another, with some accounting state changes.

Right, so they should be the same currency, no? I'm fine with it being an STAmount as long as we're on the same page that the transaction should fail if it's not the same currency as the SAV.


The following is the pseudo-code for handling a Loan payment transaction.

```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is value_change? Can as well add description for the arguments in and return.

}

return (total_principal_paid, total_interest_paid, loan_value_change, total_fee_paid)
```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few functions referred to in the pseudo-code but it's not always obvious what they refer to:

  • compute_late_payment(): section 3.2.4.1.2 Late Payment
  • compute_periodic_payment(): section 3.2.4.1.1 Regular Payment
  • compute_full_payment(): is it section 3.2.4.1.4 Early Full Repayment?
  • compute_current_value(): is it section 3.2.4.2 Total Loan Value Calculation?
    Please describe at the start of the pseudo-code what sections each function refers to.
    Also, it's not obvious what these functions return. For instance, when compute_periodic_payment() is called for the first time, it returns a struct with at least one member interest. When it's called a second time, it looks like it returns just one variable periodic_payment amount but then later in the code periodic_payment is used as a struct with {principal, interest}. Please describe the return value of each of the functions and match this value(s) with the calculated values in a relevant section. Also, use the return value consistently. I.e. don't change the type. Lastly, IMO it's better to use hungarian rather than snake notation and use the field names as they are defined in the corresponding ledger objects to make it consistent with the rest of the specs. For instance, pseudo-code refers to loan.payment_interval. Elsewhere in the specs this referred to as Loan.PaymentInterest.


- Total paid, and what portion goes to the vault:

- `totalPaid = principal_paid + interest_paid + fee_paid`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to mix hungarian and snake notation throughout the specs? IMHO it should be consistent and use hungarian notation (except for the field names, which should be capitalized).

Comment on lines +1253 to +1264
- Initial TotalValueOutstanding = 11 MPT
- PaymentTotal = 10
- Amortization produces periodicPayment = 1.1 MPT
- Asset supports only whole units ⇒ each scheduled payment is truncated to 1 MPT
Progress:
After 9 payments: Paid = 9 MPT, Remaining TotalValueOutstanding = 2 MPT
If we applied the formula again for the 10th payment:
periodicPayment (unrounded) = 1.1 MPT → truncated to 1 MPT
Remaining after payment = 1 MPT (cannot be repaid; no payments left)
Adjustment:
Because PaymentRemaining = 1, set periodicPayment = TotalValueOutstanding = 2 MPT
Final payment = 2 MPT clears the loan exactly (PrincipalOutstanding = 0, TotalValueOutstanding = 0).
Copy link
Collaborator

@ximinez ximinez Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been looking at rounding the interest calculations up instead of truncating (down). Nothing is set in stone yet, but let's look at how that would work. I estimated an actual rate to show how the payments change over time.

  • Initial principal = 10 MPT

  • PeriodicRate = 0.0178 (Regardless of payment period)

  • PaymentTotal = 10

  • PeriodicPayment = 10 * 0.0178 * (1.0178^10) / (1.0178^10 - 1) ~= 1.10048958

  • Initial TotalValueOutstanding = PeriodicPayment * 10 = 11.0048958, which rounds up to 12 MPT (though we could round down or to nearest. This shouldn't really matter, since 1 MPT should not represent a significant amount)

  • Asset supports only whole units ⇒ first scheduled payment is rounded up to 2 MPT

  • After first payment:

    • TotalValueOutstanding = 10
    • PrincipalOutstanding = 9
    • PaymentsRemaining = 9
  • For the second payment, we recompute the periodic payment

    • PeriodicPayment = 9 * 0.0178 * (1.0178^9) / (1.0178^9 - 1) ~= 1.0910928, which rounds up to 2 MPT
    • TotalValueOutstanding = 8
    • PrincipalOutstanding = 8
    • PaymentsRemaining = 8
  • At this point, all of the interest has been paid!

  • What would the periodic payment be for the third payment?

    • PeriodicPayment = 8 * 0.0178 * (1.0178^8) / (1.0178^8 - 1) ~= 1.08174822, which would also round up to 2 MPT, and end up over paying.
  • Instead I propose that we add another rule:

if(PaymentRemaining > 1 && PrincipalOutstanding == TotalValueOutstanding)
	PeriodicPayment = PrincipalOutstanding / PaymentRemaining;
  • This is the same as the special case rule where the interest rate is 0, which makes sense, because if the interest rate is 0, the TotalValueOutstanding and PrincipalOutstanding will be identical for the life of the loan!
  • This would be in addition to the rule you outlined:
if (PaymentRemaining == 1)
	PeriodicPayment = TotalValueOutstanding;

And an invariant that PrincipalOutstanding <= TotalValueOutstanding. (As an aside, this might let me get rid of the OriginalPrincipal field I added to help with rounding, and simplify a lot of the rounding operations, because any accumulated dust will be wiped out in the last payment. Maybe not, though, because those amounts are taken out of other fields like DebtTotal, and rounding might get weird there.)

I propose we move forward with this scheme for these reasons:

  1. It more realistically maps to the real world (or at least my real world experience) where, for example, my last mortgage payment is scheduled to be less than all the mortgage payments leading up to it.
  2. I think people will complain if they've been making smaller payments all along, and then have it balloon at the end. I also think Vault shareholders would like to get their interest up front.
  3. In a situation like this one where the interest is paid off early, the borrower can continue paying the same amount an potentially pay their loan off early (either through overpayments or extra payments).
  4. It simplifies the math for later payments, because I only have to divide once, instead of taking powers, multiplying, and dividing multiple times, which might be ever so slightly more performant.

Comment on lines +885 to +887
The total fee for the transaction will be increased due to the extra signatures that need to be processed, similar to the additional fees for multisigning. The minimum fee will be $(|signatures| + 1) \times base_fee$ where $|signatures| == max(1, |tx.CounterPartySignature.Signers|)$

The total fee calculation for signatures will now be $(1 + |tx.Signers| + |signatures|) \times base_fee$. In other words, even without a `tx.Signers` list, the minimum fee will be $2 \times base_fee$.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The total fee for the transaction will be increased due to the extra signatures that need to be processed, similar to the additional fees for multisigning. The minimum fee will be $(|signatures| + 1) \times base_fee$ where $|signatures| == max(1, |tx.CounterPartySignature.Signers|)$
The total fee calculation for signatures will now be $(1 + |tx.Signers| + |signatures|) \times base_fee$. In other words, even without a `tx.Signers` list, the minimum fee will be $2 \times base_fee$.
The total fee for the transaction will be increased due to the extra signatures that need to be processed, similar to the additional fees for multisigning. The minimum fee will be $(|signatures| + 1) \times base\_fee$ where $|signatures| == max(1, |tx.CounterPartySignature.Signers|)$
The total fee calculation for signatures will now be $(1 + |tx.Signers| + |signatures|) \times base\_fee$. In other words, even without a `tx.Signers` list, the minimum fee will be $2 \times base\_fee$.

I think this is correct LaTeX to get the underscore to appear right.

Copy link
Collaborator

@ximinez ximinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment...

Comment on lines 1216 to 1218
$$
latePaymentInterest = principalOutstanding \times \frac{lateInterestRate \times secondsSinceLastPayment}{365 \times 24 \times 60 \times 60}
$$
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the correct calculation is:

secondsSinceLastPayment = lastLedgerCloseTime - Loan.nextPaymentDate
``

I was thinking about this yesterday, when we were looking at this. I agree.

@amarantha-k
Copy link
Collaborator

@Tapanito Just a quick note to drop the "d" from the directory name before merging. (XLS-0066d-lending-protocol/README.md -> XLS-0066-lending-protocol/README.md).

@mvadari mvadari changed the title XLS-0066d Lending Protocol XLS-0066 Lending Protocol Oct 22, 2025

##### 2.2.2.5 PeriodicPayment

The periodic payment amount represents the precise sum the Borrower must pay during each payment cycle. For practical implementation, this value should be rounded UP when processing payments. The system automatically recalculates the PeriodicPayment following any overpayment by the borrower. For instance, when dealing with MPT loans, the calculated `PeriodicPayment` may be `10.251`. However, since MPTs only support whole number representations, the borrower would need to pay `12` units. The system maintains the precise periodic payment value at maximum accuracy since it is frequently referenced throughout loan payment computations.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11 units, not 12.


##### 2.2.2.2 TotalValueOutstanding

The total outstanding value of the Loan, including all fees. To calculate the outstanding interest portion, use this formula: `TotalInterestOutstanding = TotalValueOutstanding - PrincipalOutstanding - ManagementFeeOutstanding`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it includes all fees then why only ManagementFeeOutstanding is subtracted?

The loan's financial state is tracked through three key components:

- **PrincipalOutstanding**: Represents the remaining principal balance that the borrower must repay to satisfy the original loan amount.
- **TotalValueOutstanding**: Encompasses the complete remaining loan obligation, comprising both the outstanding principal and all scheduled interest payments based on the original amortization schedule. This value excludes any additional interest charges resulting from late payments.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should mention the fees. Per section 2.2.2.2 TotalValueOutstanding also includes the fees.


- **PrincipalOutstanding**: Represents the remaining principal balance that the borrower must repay to satisfy the original loan amount.
- **TotalValueOutstanding**: Encompasses the complete remaining loan obligation, comprising both the outstanding principal and all scheduled interest payments based on the original amortization schedule. This value excludes any additional interest charges resulting from late payments.
- **InterestOutstanding**: The total scheduled interest remaining on the loan, derived as `TotalValueOutstanding - PrincipalOutstanding`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the fee not subtracted? This is different from section 2.2.2.2.

- **TotalValueOutstanding**: Encompasses the complete remaining loan obligation, comprising both the outstanding principal and all scheduled interest payments based on the original amortization schedule. This value excludes any additional interest charges resulting from late payments.
- **InterestOutstanding**: The total scheduled interest remaining on the loan, derived as `TotalValueOutstanding - PrincipalOutstanding`.

**Asset-Specific Precision Handling**: For discrete asset types (MPTs and XRP denominated in drops), both `TotalValueOutstanding` and `PrincipalOutstanding` values are truncated to whole numbers to ensure compatibility with the underlying asset precision requirements.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the values always truncated or rounded up or down?

| ----------------- | :----------------: | :-------: | :-----------: | :-----------: | :---------------------------------------------------------------------- |
| `TransactionType` | :heavy_check_mark: | `string` | `UINT16` | `77` | Transaction type. |
| `LoanBrokerID` | :heavy_check_mark: | `string` | `HASH256` | `N/A` | The Loan Broker ID from which to withdraw First-Loss Capital. |
| `Amount` | :heavy_check_mark: | `object` | `AMOUNT` | 0 | The Fist-Loss Capital amount to withdraw. |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be NUMBER type? We are going to access Vault anyways so will now specific Asset type. NUMBER is going to make the tx size smaller by 40 bytes(IOU/XRP) or 24 bytes (MPT). It's also fewer checks in preclaim. This also applies to LoanBrokerCoverDeposit, LoanBrokerCoverWithdraw, and LoanPay. By the way, PrincipalRequested in LoanSet is NUMBER. I think it makes sense to make all the Amount fields to have NUMBER type.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be NUMBER type? We are going to access Vault anyways so will now specific Asset type. NUMBER is going to make the tx size smaller by 40 bytes(IOU/XRP) or 24 bytes (MPT). It's also fewer checks in preclaim. This also applies to LoanBrokerCoverDeposit, LoanBrokerCoverWithdraw, and LoanPay. By the way, PrincipalRequested in LoanSet is NUMBER. I think it makes sense to make all the Amount fields to have NUMBER type.

There was an earlier discussion on this topic. #240 (comment)

tl;dr Because of how SFields are defined, any field name Amount must be AMOUNT. It was decided to keep the familiar name rather than create a new field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants