Conversation
Introduces a dedicated `LoanAccept` transaction (type 84) for the two-step loan creation flow instead of overloading `LoanSet` with `tfLoanSetAccept`. Clarifies that expired pending loans require an explicit `LoanDelete` to reclaim `Vault.AssetsReserved`.
Keep expired acceptances from mutating the pending Loan, assign LoanAccept type 83, and apply V1.1 cash-basis deltas on proposal and delete.
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overall the two-step loan flow (LoanAccept) and AssetsReserved additions are documented thoroughly, and the extensive section renumbering (3.9→3.12) plus internal anchor links were re-verified and are all correctly updated. TransactionType values (LoanAccept=83, LoanPay bumped to 84) avoid collisions, and the reserve/Vault bookkeeping for propose→accept→delete is symmetric. The one notable gap is that LoanAccept's state changes never recompute Loan.StartDate/NextPaymentDueDate after acceptance, even though the two-step flow's StartDate is described as an acceptance deadline rather than the loan's actual start.
There was a problem hiding this comment.
Documentation-only change adding XLS-65 AssetsReserved and the XLS-66 two-step LoanSet/LoanAccept flow. The spec text is largely internally consistent (flag bitmasks don't collide, new anchors resolve correctly, reserve/vault bookkeeping nets out correctly across proposal/accept/delete). Two things worth double-checking before merge: the TransactionType reassignment for LoanPay, and whether interest should be recognized in Vault.AssetsTotal before a loan proposal is actually accepted.
|
@ai-review --aggressive |
There was a problem hiding this comment.
🟡 Changes recommended
The updated specs contain concrete field-table inaccuracies (e.g., incorrect internal types) and a mismatch between the PR description and the documented LoanSet additions that should be resolved before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the XLS-65 and XLS-66 specifications to document the LendingProtocolV1_1 amendment, adding asset reservation accounting in Vaults and introducing a two-step loan creation flow (proposal + acceptance) using a new LoanAccept transaction.
Changes:
- XLS-65: Adds
AssetsReservedto theVaultledger entry andvault_infoRPC response to track assets reserved for pending loans. - XLS-66: Introduces pending-loan semantics (
lsfLoanPending), addsLoanAccept, and updatesLoanSet,LoanDelete,LoanManage, andLoanPayrules to support the two-step flow. - Expands/adjusts failure conditions and state changes to cover proposal expiration, acceptance, reserve transfer, and pending-loan deletion.
File summaries
| File | Description |
|---|---|
XLS-0066-lending-protocol/README.md |
Documents the two-step loan creation flow and pending-loan lifecycle, adds LoanAccept, and updates related ledger/transaction behaviors. |
XLS-0065-single-asset-vault/README.md |
Adds AssetsReserved to the Vault ledger entry and vault_info RPC response to support pending-loan asset reservation tracking. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
This is a well-specified two-step loan creation flow for XLS-66 plus the AssetsReserved field for XLS-65. Cross-references, failure-condition numbering, and state-change bookkeeping (reserve/OwnerCount/Vault balances) are internally consistent between LoanSet proposal, LoanAccept, and pending-Loan deletion. One spec-table inconsistency stands out: AssetsReserved is marked as an unconditionally Required field on the Vault ledger entry even though it's gated behind the LendingProtocolV1_1 amendment (and is correctly marked optional in the vault_info RPC table), which mirrors the exact conditional treatment this same diff correctly applied to Loan.OwnerNode.
There was a problem hiding this comment.
🟡 Changes recommended
The spec introduces a transaction-type ID conflict (renumbering LoanPay) and has a missing data-verification rule for StartDate without Borrower, which must be resolved for protocol correctness and implementability.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
XLS-0066-lending-protocol/README.md:1547
LoanPayis currently specified as transaction type84, but transaction type IDs are protocol constants andLoanPaywas already assigned83in this spec. KeepLoanPayat83and assign the new ID (84) toLoanAcceptinstead.
| `TransactionType` | Yes | `string` | `UINT16` | `84` | The transaction type. |
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
| | `LoanID` | Yes | `string` | `HASH256` | `N/A` | The ID of the Loan object to be deleted. | | ||
| | Field Name | Required | JSON Type | Internal Type | Default Value | Description | | ||
| | ----------------- | :------: | :-------: | :-----------: | :-----------: | :--------------------------------------------- | | ||
| | `TransactionType` | Yes | `string` | `UINT16` | `83` | The transaction type. | |
| 19. `StartDate` is specified and either the `Counterparty` or the `CounterpartySignature` field is specified. (`temINVALID`) | ||
| 20. `Borrower` is specified and `StartDate` is not specified. (`temINVALID`) |
| | `LateInterestRate` | Yes | Yes | `number` | `UINT32` | `N/A` | A premium is added to the interest rate for late payments in 1/10th basis points. Valid values are between 0 and 100000 inclusive. (0 - 100%) | | ||
| | `CloseInterestRate` | Yes | Yes | `number` | `UINT32` | `N/A` | An interest rate charged for repaying the Loan early in 1/10th basis points. Valid values are between 0 and 100000 inclusive. (0 - 100%) | | ||
| | `OverpaymentInterestRate` | Yes | Yes | `number` | `UINT32` | `N/A` | An interest rate charged on overpayments in 1/10th basis points. Valid values are between 0 and 100000 inclusive. (0 - 100%) | | ||
| | `StartDate` | Yes | Yes | `number` | `UINT32` | `CurrentLedgerTimestamp` | The timestamp of when the Loan started [Ripple Epoch](https://xrpl.org/docs/references/protocol/data-types/basic-data-types/#specifying-time). | |
| 7. The LoanBroker _pseudo-account_ is deep frozen for the asset. (`tecFROZEN` for IOUs, `tecLOCKED` for MPTs) | ||
| 8. The Borrower is frozen for the asset. (`tecFROZEN` for IOUs, `tecLOCKED` for MPTs) | ||
| 9. The `LoanBroker.Owner` is deep frozen for the asset. (`tecFROZEN` for IOUs, `tecLOCKED` for MPTs) | ||
| 10. Cannot add asset holding for the `Vault.Asset` (e.g., MPToken or TrustLine issues). (`tecNO_PERMISSION`) |
There was a problem hiding this comment.
We currently return terNO_RIPPLE for IOU and tecNO_AUTH for MPT on develop so maybe we should update this and also for other transactors.
Summary
AssetsReservedto the Vault ledger entry andvault_infoRPC response underLendingProtocolV1_1. This field tracks assets reserved for pending Loans that have not yet been accepted.LendingProtocolV1_1that eliminates the custom dual-signature requirement:BorrowerandStartDatefields toLoanSetfor creating a pending Loan proposalLoanAccepttransaction whose required transaction-specific field isLoanIDlsfLoanPendingflag to theLoanledger entryOwnerNodeconditional until acceptanceLoanManageandLoanPayMotivation
The current
LoanSetdual-signature workflow creates custody lock-in and limits adoption to custodians willing to build bespoke integrations. The two-step flow (propose + accept) uses standard XRPL transactions, enabling any wallet or custodian to participate.Test plan