Add support for lending protocol xls-66d #3138
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request adds comprehensive support for the Lending Protocol (XLS-66d) to the xrpl.js SDK. The changes include new ledger entry types (Loan, LoanBroker), nine new transaction types for loan and broker management, field definitions in the binary codec, validation utilities, and hash functions for vault and loan broker lookups, along with integration tests demonstrating multi-signature loan flows. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
packages/xrpl/src/models/transactions/loanManage.ts (1)
33-48: Consider adding mutual exclusivity validation for flags.The flags
tfLoanDefault,tfLoanImpair, andtfLoanUnimpairappear to represent mutually exclusive loan state transitions. If this is the case, consider adding validation to ensure only one flag is set at a time. However, if rippled enforces this constraint, this may be optional per the SDK's test philosophy (rippled-level behaviors are tested in rippled).Based on learnings, the SDK generally defers complex validation to rippled. Please verify whether mutual exclusivity of these flags should be validated client-side or if rippled handles this.
packages/xrpl/src/models/transactions/loanBrokerSet.ts (1)
42-44: Clarify the Data field length documentation.The comment says "limited to 256 bytes" but the validation uses
MAX_DATA_LENGTH = 512characters. Since this is hex-encoded data, 512 hex characters equals 256 bytes, so the behavior is correct. Consider updating the comment to mention the hex character limit for clarity:/** - * Arbitrary metadata in hex format. The field is limited to 256 bytes. + * Arbitrary metadata in hex format. The field is limited to 256 bytes (512 hex characters). */ Data?: stringpackages/xrpl/src/models/transactions/loanSet.ts (2)
57-60: Clarify the Data field length documentation.Same as in
loanBrokerSet.ts, the comment says "limited to 256 bytes" but validation uses 512 hex characters (which is equivalent). Consider clarifying:/** - * Arbitrary metadata in hex format. The field is limited to 256 bytes. + * Arbitrary metadata in hex format. The field is limited to 256 bytes (512 hex characters). */ Data?: string
130-145: Consider deeper validation forCounterpartySignature.The
CounterpartySignatureis validated only asisRecord(line 181), but its inner structure (SigningPubKey,TxnSignature,Signers) is not validated. If these fields have specific format requirements, consider adding validation helpers for them. However, if rippled performs this validation, this may be acceptable per the SDK's approach.Based on learnings, the SDK generally defers complex validation to rippled. Please verify whether
CounterpartySignaturestructure validation should be done client-side.packages/xrpl/src/models/transactions/loanBrokerCoverClawback.ts (1)
59-61: Potential type narrowing issue withtx.Amount.valueaccess.After
validateOptionalField(tx, 'Amount', isTokenAmount)passes,tx.Amountis typed asunknown. The direct access totx.Amount.valuerelies on runtime behavior but TypeScript won't narrow the type properly here.Consider adding a type guard or casting:
- if (tx.Amount != null && new BigNumber(tx.Amount.value).isLessThan(0)) { + if (tx.Amount != null) { + const amount = tx.Amount as { value: string } + if (new BigNumber(amount.value).isLessThan(0)) { + throw new ValidationError(`LoanBrokerCoverClawback: Amount must be >= 0`) + } + } - throw new ValidationError(`LoanBrokerCoverClawback: Amount must be >= 0`) - }Alternatively, this pattern is used elsewhere in the codebase for similar validations, so it may be intentionally accepted. Based on learnings from other validation functions in this repository, runtime type checks are acceptable in validators.
packages/xrpl/src/models/transactions/loanPay.ts (1)
32-37: Minor grammar issue in JSDoc.The article should be "a" instead of "an" before "LoanPay".
/** - * Verify the form and type of an LoanPay at runtime. + * Verify the form and type of a LoanPay at runtime. * * @param tx - LoanPay Transaction. * @throws When LoanPay is Malformed. */packages/xrpl/src/models/transactions/loanBrokerDelete.ts (1)
25-30: Minor grammar issue in JSDoc.Same issue as in other files - use "a" instead of "an" before "LoanBrokerDelete".
/** - * Verify the form and type of an LoanBrokerDelete at runtime. + * Verify the form and type of a LoanBrokerDelete at runtime. * * @param tx - LoanBrokerDelete Transaction. * @throws When LoanBrokerDelete is Malformed. */packages/xrpl/src/models/transactions/loanBrokerCoverDeposit.ts (1)
32-37: Minor grammar issue in JSDoc.Use "a" instead of "an" before "LoanBrokerCoverDeposit".
/** - * Verify the form and type of an LoanBrokerCoverDeposit at runtime. + * Verify the form and type of a LoanBrokerCoverDeposit at runtime. * * @param tx - LoanBrokerCoverDeposit Transaction. * @throws When LoanBrokerCoverDeposit is Malformed. */packages/xrpl/src/models/transactions/loanBrokerCoverWithdraw.ts (1)
40-45: Minor grammar issue in JSDoc.Use "a" instead of "an" before "LoanBrokerCoverWithdraw".
/** - * Verify the form and type of an LoanBrokerCoverWithdraw at runtime. + * Verify the form and type of a LoanBrokerCoverWithdraw at runtime. * * @param tx - LoanBrokerCoverWithdraw Transaction. * @throws When LoanBrokerCoverWithdraw is Malformed. */
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
.ci-config/rippled.cfg(1 hunks)packages/ripple-binary-codec/HISTORY.md(1 hunks)packages/ripple-binary-codec/src/enums/definitions.json(10 hunks)packages/xrpl/HISTORY.md(1 hunks)packages/xrpl/src/models/ledger/LedgerEntry.ts(3 hunks)packages/xrpl/src/models/ledger/Loan.ts(1 hunks)packages/xrpl/src/models/ledger/LoanBroker.ts(1 hunks)packages/xrpl/src/models/ledger/index.ts(2 hunks)packages/xrpl/src/models/transactions/common.ts(3 hunks)packages/xrpl/src/models/transactions/index.ts(1 hunks)packages/xrpl/src/models/transactions/loanBrokerCoverClawback.ts(1 hunks)packages/xrpl/src/models/transactions/loanBrokerCoverDeposit.ts(1 hunks)packages/xrpl/src/models/transactions/loanBrokerCoverWithdraw.ts(1 hunks)packages/xrpl/src/models/transactions/loanBrokerDelete.ts(1 hunks)packages/xrpl/src/models/transactions/loanBrokerSet.ts(1 hunks)packages/xrpl/src/models/transactions/loanDelete.ts(1 hunks)packages/xrpl/src/models/transactions/loanManage.ts(1 hunks)packages/xrpl/src/models/transactions/loanPay.ts(1 hunks)packages/xrpl/src/models/transactions/loanSet.ts(1 hunks)packages/xrpl/src/models/transactions/transaction.ts(3 hunks)packages/xrpl/src/utils/hashes/index.ts(1 hunks)packages/xrpl/src/utils/hashes/ledgerSpaces.ts(2 hunks)packages/xrpl/src/utils/index.ts(2 hunks)packages/xrpl/test/integration/transactions/lendingProtocol.test.ts(1 hunks)packages/xrpl/test/models/loanBrokerCoverClawback.test.ts(1 hunks)packages/xrpl/test/models/loanBrokerCoverDeposit.test.ts(1 hunks)packages/xrpl/test/models/loanBrokerCoverWithdraw.test.ts(1 hunks)packages/xrpl/test/models/loanBrokerDelete.test.ts(1 hunks)packages/xrpl/test/models/loanBrokerSet.test.ts(1 hunks)packages/xrpl/test/utils/hashes.test.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (18)
📓 Common learnings
Learnt from: ckeshava
Repo: XRPLF/xrpl.js PR: 2874
File: packages/xrpl/test/integration/transactions/permissionedDomain.test.ts:25-80
Timestamp: 2025-01-08T02:12:28.489Z
Learning: The rippled C++ implementation (PR #5161) includes comprehensive test coverage for PermissionedDomain (XLS-80d) error cases. The JS SDK tests focus on the happy path since the error cases are already validated at the rippled level, following the principle of not duplicating complex validation testing across SDK implementations.
Learnt from: shawnxie999
Repo: XRPLF/xrpl.js PR: 2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Learnt from: Patel-Raj11
Repo: XRPLF/xrpl.js PR: 3117
File: packages/xrpl/src/models/transactions/common.ts:1133-1180
Timestamp: 2025-10-28T14:16:24.585Z
Learning: The MPTokenMetadata TypeScript interface in xrpl.js reflects the XLS-89 standard (long-form fields only) to provide correct type hints for IDEs and TypeScript users, even though JavaScript users may provide non-standard structures with compact fields or both forms present. This design provides proper tooling support while maintaining runtime flexibility.
📚 Learning: 2024-12-05T16:48:12.951Z
Learnt from: achowdhry-ripple
Repo: XRPLF/xrpl.js PR: 2661
File: packages/xrpl/src/models/transactions/MPTokenIssuanceCreate.ts:69-102
Timestamp: 2024-12-05T16:48:12.951Z
Learning: When adding validation in `validate*` functions in `packages/xrpl/src/models/transactions/`, utilize existing helper functions (e.g., `validateOptionalField`, `validateType`, `isNumber`, `isInteger`) for type checking and validation where appropriate.
Applied to files:
packages/xrpl/src/models/transactions/loanBrokerCoverClawback.tspackages/xrpl/src/models/transactions/loanPay.tspackages/xrpl/src/models/transactions/loanBrokerSet.tspackages/xrpl/src/models/transactions/loanDelete.tspackages/xrpl/src/models/transactions/loanBrokerCoverWithdraw.tspackages/xrpl/src/models/transactions/common.tspackages/xrpl/src/models/transactions/loanManage.tspackages/xrpl/src/models/transactions/transaction.tspackages/xrpl/src/models/transactions/loanSet.ts
📚 Learning: 2024-12-06T19:27:11.147Z
Learnt from: shawnxie999
Repo: XRPLF/xrpl.js PR: 2661
File: packages/xrpl/test/integration/transactions/clawback.test.ts:165-178
Timestamp: 2024-12-06T19:27:11.147Z
Learning: In the integration tests for `clawback.test.ts`, it's acceptable to use `ts-expect-error` to bypass type checking when verifying ledger entries, and no additional type safety improvements are needed.
Applied to files:
packages/xrpl/src/models/transactions/loanBrokerCoverClawback.tspackages/xrpl/test/models/loanBrokerCoverWithdraw.test.tspackages/xrpl/src/models/ledger/LedgerEntry.tspackages/xrpl/test/models/loanBrokerDelete.test.tspackages/xrpl/test/models/loanBrokerCoverClawback.test.tspackages/xrpl/test/models/loanBrokerSet.test.tspackages/xrpl/test/models/loanBrokerCoverDeposit.test.tspackages/xrpl/test/integration/transactions/lendingProtocol.test.ts
📚 Learning: 2024-10-02T15:47:02.491Z
Learnt from: mvadari
Repo: XRPLF/xrpl.js PR: 2690
File: packages/xrpl/tools/generateModels.js:52-52
Timestamp: 2024-10-02T15:47:02.491Z
Learning: In `generateModels.js`, the regex used to match `SubmittableTransaction` in `transaction.ts` is expected to always succeed because the pattern is present in the source code. If it fails, the code needs to be updated.
Applied to files:
packages/xrpl/src/models/transactions/loanBrokerCoverClawback.tspackages/xrpl/test/models/loanBrokerCoverWithdraw.test.tspackages/xrpl/test/models/loanBrokerDelete.test.tspackages/xrpl/test/models/loanBrokerCoverClawback.test.tspackages/xrpl/test/models/loanBrokerSet.test.tspackages/xrpl/test/models/loanBrokerCoverDeposit.test.tspackages/xrpl/src/models/transactions/transaction.ts
📚 Learning: 2025-10-30T12:09:55.784Z
Learnt from: ckeshava
Repo: XRPLF/xrpl.js PR: 3081
File: packages/xrpl/src/models/transactions/MPTokenIssuanceCreate.ts:298-298
Timestamp: 2025-10-30T12:09:55.784Z
Learning: In XRPL's MPTokenIssuanceCreate transaction validation, MaximumAmount must be greater than 0 (not greater than or equal to). The rippled C++ implementation at MPTokenIssuanceCreate.cpp explicitly rejects MaximumAmount == 0 with temMALFORMED, so the JavaScript validation should use `<= 0` to match this behavior.
Applied to files:
packages/xrpl/src/models/transactions/loanBrokerCoverClawback.tspackages/xrpl/src/models/transactions/common.ts
📚 Learning: 2024-12-06T18:44:55.095Z
Learnt from: shawnxie999
Repo: XRPLF/xrpl.js PR: 2661
File: packages/xrpl/test/models/MPTokenAuthorize.test.ts:60-71
Timestamp: 2024-12-06T18:44:55.095Z
Learning: In the XRPL.js library's TypeScript test file `packages/xrpl/test/models/MPTokenAuthorize.test.ts`, negative test cases for invalid `Account` address format, invalid `Holder` address format, invalid `MPTokenIssuanceID` format, and invalid flag combinations are not necessary.
Applied to files:
packages/xrpl/test/models/loanBrokerCoverWithdraw.test.tspackages/xrpl/test/models/loanBrokerDelete.test.tspackages/xrpl/test/models/loanBrokerCoverClawback.test.tspackages/xrpl/test/models/loanBrokerSet.test.tspackages/xrpl/test/models/loanBrokerCoverDeposit.test.tspackages/xrpl/src/models/transactions/common.tspackages/xrpl/test/integration/transactions/lendingProtocol.test.tspackages/xrpl/src/models/transactions/transaction.ts
📚 Learning: 2024-12-06T19:25:15.376Z
Learnt from: shawnxie999
Repo: XRPLF/xrpl.js PR: 2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Applied to files:
packages/xrpl/test/models/loanBrokerCoverWithdraw.test.tspackages/xrpl/test/models/loanBrokerDelete.test.tspackages/xrpl/test/models/loanBrokerCoverClawback.test.tspackages/xrpl/src/models/transactions/loanBrokerCoverWithdraw.tspackages/xrpl/test/models/loanBrokerSet.test.tspackages/xrpl/test/models/loanBrokerCoverDeposit.test.tspackages/xrpl/test/integration/transactions/lendingProtocol.test.tspackages/xrpl/src/models/transactions/transaction.tspackages/xrpl/src/models/transactions/index.ts
📚 Learning: 2025-02-12T23:30:40.622Z
Learnt from: mvadari
Repo: XRPLF/xrpl.js PR: 2895
File: packages/xrpl/test/models/DIDDelete.test.ts:28-31
Timestamp: 2025-02-12T23:30:40.622Z
Learning: In JavaScript/TypeScript transaction validation tests, object key validation can be performed using:
1. Object.keys() comparison with expected set
2. TypeScript interfaces with strict object literal checks
3. Object sanitization by filtering to allowed keys only
Applied to files:
packages/xrpl/test/models/loanBrokerCoverWithdraw.test.tspackages/xrpl/test/models/loanBrokerDelete.test.tspackages/xrpl/test/models/loanBrokerCoverClawback.test.tspackages/xrpl/test/models/loanBrokerSet.test.tspackages/xrpl/test/models/loanBrokerCoverDeposit.test.tspackages/xrpl/src/models/transactions/common.ts
📚 Learning: 2025-02-27T19:08:13.676Z
Learnt from: mvadari
Repo: XRPLF/xrpl.js PR: 2895
File: packages/xrpl/test/models/XChainCreateBridge.test.ts:44-44
Timestamp: 2025-02-27T19:08:13.676Z
Learning: Using `delete` operator in model tests is acceptable as it better represents a field being completely absent, which is the intention in validation tests.
Applied to files:
packages/xrpl/test/models/loanBrokerDelete.test.ts
📚 Learning: 2025-02-12T23:28:55.377Z
Learnt from: mvadari
Repo: XRPLF/xrpl.js PR: 2895
File: packages/xrpl/test/models/clawback.test.ts:0-0
Timestamp: 2025-02-12T23:28:55.377Z
Learning: The `validate` function in xrpl.js is synchronous and should be tested using `assert.doesNotThrow` rather than async assertions.
Applied to files:
packages/xrpl/test/models/loanBrokerCoverClawback.test.tspackages/xrpl/test/models/loanBrokerCoverDeposit.test.ts
📚 Learning: 2025-01-08T02:12:28.489Z
Learnt from: ckeshava
Repo: XRPLF/xrpl.js PR: 2874
File: packages/xrpl/test/integration/transactions/permissionedDomain.test.ts:25-80
Timestamp: 2025-01-08T02:12:28.489Z
Learning: The rippled C++ implementation (PR #5161) includes comprehensive test coverage for PermissionedDomain (XLS-80d) error cases. The JS SDK tests focus on the happy path since the error cases are already validated at the rippled level, following the principle of not duplicating complex validation testing across SDK implementations.
Applied to files:
packages/xrpl/test/models/loanBrokerCoverClawback.test.ts
📚 Learning: 2025-10-28T14:16:24.585Z
Learnt from: Patel-Raj11
Repo: XRPLF/xrpl.js PR: 3117
File: packages/xrpl/src/models/transactions/common.ts:1133-1180
Timestamp: 2025-10-28T14:16:24.585Z
Learning: The MPTokenMetadata TypeScript interface in xrpl.js reflects the XLS-89 standard (long-form fields only) to provide correct type hints for IDEs and TypeScript users, even though JavaScript users may provide non-standard structures with compact fields or both forms present. This design provides proper tooling support while maintaining runtime flexibility.
Applied to files:
packages/xrpl/src/models/ledger/LoanBroker.tspackages/xrpl/src/models/transactions/common.tspackages/xrpl/src/models/transactions/loanSet.ts
📚 Learning: 2025-07-10T22:04:59.994Z
Learnt from: ckeshava
Repo: XRPLF/xrpl.js PR: 3032
File: packages/xrpl/src/models/transactions/common.ts:689-698
Timestamp: 2025-07-10T22:04:59.994Z
Learning: In the XRPL.js library, the validateDomainID function in `packages/xrpl/src/models/transactions/common.ts` should only validate the length (64 characters) of domain IDs, not hex encoding. The rippled C++ implementation does not enforce any regex check on domain ID values, so additional hex validation is not required in the JS implementation.
Applied to files:
packages/xrpl/src/models/transactions/common.ts
📚 Learning: 2025-10-28T14:16:24.585Z
Learnt from: Patel-Raj11
Repo: XRPLF/xrpl.js PR: 3117
File: packages/xrpl/src/models/transactions/common.ts:1133-1180
Timestamp: 2025-10-28T14:16:24.585Z
Learning: In xrpl.js MPTokenMetadata handling: encodeMPTokenMetadata and decodeMPTokenMetadata are designed to preserve the original structure of the input, only converting between compact and long forms when a single form is present (not both). If both compact (e.g., 'us') and long (e.g., 'uris') forms exist in the input, both are preserved in the output. This is intentional to avoid modifying the user's data. Validation (validateMPTokenMetadata) separately checks and reports XLS-89 compliance.
Applied to files:
packages/xrpl/src/models/transactions/common.ts
📚 Learning: 2025-07-11T21:22:07.809Z
Learnt from: ckeshava
Repo: XRPLF/xrpl.js PR: 3032
File: packages/xrpl/src/models/transactions/common.ts:689-698
Timestamp: 2025-07-11T21:22:07.809Z
Learning: Domain ID validation in XRPL.js is handled at the serialization/deserialization layer through Hash types in the ripple-binary-codec package, not at the transaction validation layer. The validateDomainID function only needs to validate length (64 characters) as hex validation occurs when Hash types are constructed during serialization.
Applied to files:
packages/xrpl/src/models/transactions/common.ts
📚 Learning: 2025-01-31T17:46:25.375Z
Learnt from: ckeshava
Repo: XRPLF/xrpl.js PR: 2873
File: packages/xrpl/test/integration/transactions/trustSet.test.ts:0-0
Timestamp: 2025-01-31T17:46:25.375Z
Learning: For the XRPL implementation, extensive test cases for deep freeze behavior (high/low side interactions, clearing flags, etc.) are maintained in the C++ implementation and don't need to be duplicated in the JavaScript implementation.
Applied to files:
packages/xrpl/test/integration/transactions/lendingProtocol.test.ts
📚 Learning: 2025-01-08T13:08:52.688Z
Learnt from: ckeshava
Repo: XRPLF/xrpl.js PR: 2873
File: packages/xrpl/src/models/transactions/trustSet.ts:33-36
Timestamp: 2025-01-08T13:08:52.688Z
Learning: For trust-set transactions in XRPL, validation rules for flags should be implemented comprehensively rather than cherry-picking specific rules, as there are many interdependent validation rules associated with these flags.
Applied to files:
packages/xrpl/src/models/transactions/transaction.tspackages/xrpl/src/models/transactions/loanSet.ts
📚 Learning: 2024-10-30T01:05:33.583Z
Learnt from: ckeshava
Repo: XRPLF/xrpl.js PR: 2812
File: packages/xrpl/test/integration/transactions/payment.test.ts:41-43
Timestamp: 2024-10-30T01:05:33.583Z
Learning: In tests involving fee calculations for XRPL, avoid using default fee amounts. If unable to retrieve fee values from the latest validated ledger, throw an error instead of defaulting to a specific amount.
Applied to files:
.ci-config/rippled.cfg
🧬 Code graph analysis (12)
packages/xrpl/test/models/loanBrokerCoverWithdraw.test.ts (2)
packages/xrpl/test/testUtils.ts (2)
assertTxIsValid(66-69)assertTxValidationError(78-85)packages/xrpl/src/models/transactions/loanBrokerCoverWithdraw.ts (1)
validateLoanBrokerCoverWithdraw(46-60)
packages/xrpl/src/models/ledger/LedgerEntry.ts (2)
packages/xrpl/src/models/ledger/LoanBroker.ts (1)
LoanBroker(10-84)packages/xrpl/src/models/ledger/index.ts (1)
LoanBroker(66-66)
packages/xrpl/src/models/transactions/loanDelete.ts (1)
packages/xrpl/src/models/transactions/common.ts (5)
BaseTransaction(503-578)validateBaseTransaction(589-654)validateRequiredField(428-454)isString(147-149)isLedgerEntryId(391-393)
packages/xrpl/test/models/loanBrokerDelete.test.ts (2)
packages/xrpl/test/testUtils.ts (2)
assertTxIsValid(66-69)assertTxValidationError(78-85)packages/xrpl/src/models/transactions/loanBrokerDelete.ts (1)
validateLoanBrokerDelete(31-41)
packages/xrpl/src/models/ledger/LoanBroker.ts (2)
packages/xrpl/src/models/ledger/BaseLedgerEntry.ts (2)
BaseLedgerEntry(1-3)HasPreviousTxnID(5-16)packages/xrpl/src/models/transactions/common.ts (1)
XRPLNumber(317-317)
packages/xrpl/src/models/transactions/loanBrokerCoverWithdraw.ts (3)
packages/xrpl/src/models/transactions/common.ts (8)
BaseTransaction(503-578)Account(308-308)validateRequiredField(428-454)isString(147-149)isAmount(338-344)validateOptionalField(468-488)isAccount(325-330)isLedgerEntryId(391-393)packages/xrpl/src/models/common/index.ts (1)
MPTAmount(27-30)packages/xrpl/src/errors.ts (1)
ValidationError(156-156)
packages/xrpl/test/models/loanBrokerSet.test.ts (2)
packages/xrpl/test/testUtils.ts (2)
assertTxIsValid(66-69)assertTxValidationError(78-85)packages/xrpl/src/models/transactions/loanBrokerSet.ts (1)
validateLoanBrokerSet(76-138)
packages/xrpl/src/models/transactions/common.ts (2)
packages/xrpl/src/models/common/index.ts (2)
IssuedCurrencyAmount(23-25)MPTAmount(27-30)packages/xrpl/src/models/utils/index.ts (1)
isHex(60-62)
packages/xrpl/src/models/ledger/Loan.ts (2)
packages/xrpl/src/models/ledger/BaseLedgerEntry.ts (2)
BaseLedgerEntry(1-3)HasPreviousTxnID(5-16)packages/xrpl/src/models/ledger/index.ts (1)
LoanFlags(65-65)
packages/xrpl/src/models/transactions/transaction.ts (10)
packages/xrpl/src/models/transactions/index.ts (9)
LoanBrokerSet(58-58)LoanBrokerCoverClawback(62-62)LoanBrokerCoverDeposit(60-60)LoanBrokerCoverWithdraw(61-61)LoanBrokerDelete(59-59)LoanSet(63-63)LoanDelete(64-64)LoanManage(66-66)LoanPay(70-70)packages/xrpl/src/models/transactions/loanBrokerCoverClawback.ts (2)
LoanBrokerCoverClawback(22-37)validateLoanBrokerCoverClawback(45-68)packages/xrpl/src/models/transactions/loanBrokerCoverDeposit.ts (2)
LoanBrokerCoverDeposit(18-30)validateLoanBrokerCoverDeposit(38-51)packages/xrpl/src/models/transactions/loanBrokerCoverWithdraw.ts (2)
LoanBrokerCoverWithdraw(21-38)validateLoanBrokerCoverWithdraw(46-60)packages/xrpl/src/models/transactions/loanBrokerDelete.ts (2)
LoanBrokerDelete(16-23)validateLoanBrokerDelete(31-41)packages/xrpl/src/models/transactions/loanSet.ts (2)
LoanSet(34-125)validateLoanSet(176-280)packages/xrpl/src/models/transactions/loanDelete.ts (2)
LoanDelete(16-23)validateLoanDelete(31-41)packages/xrpl/src/models/transactions/loanManage.ts (2)
LoanManage(17-26)validateLoanManage(68-78)packages/xrpl/src/models/transactions/loanPay.ts (2)
LoanPay(18-30)validateLoanPay(38-49)packages/xrpl/src/models/transactions/loanBrokerSet.ts (1)
validateLoanBrokerSet(76-138)
packages/xrpl/src/models/transactions/loanBrokerCoverDeposit.ts (3)
packages/xrpl/src/models/transactions/common.ts (6)
BaseTransaction(503-578)validateBaseTransaction(589-654)validateRequiredField(428-454)isString(147-149)isAmount(338-344)isLedgerEntryId(391-393)packages/xrpl/src/models/common/index.ts (1)
MPTAmount(27-30)packages/xrpl/src/errors.ts (1)
ValidationError(156-156)
packages/xrpl/src/models/transactions/loanSet.ts (4)
packages/xrpl/src/models/transactions/common.ts (14)
BaseTransaction(503-578)XRPLNumber(317-317)Account(308-308)GlobalFlagsInterface(496-498)validateBaseTransaction(589-654)validateRequiredField(428-454)isString(147-149)isXRPLNumber(196-203)validateOptionalField(468-488)isRecord(137-139)isAccount(325-330)isNumber(157-159)isLedgerEntryId(391-393)validateHexMetadata(402-412)packages/xrpl/src/models/common/index.ts (1)
Signer(43-49)packages/xrpl/src/models/transactions/index.ts (1)
LoanSetFlags(63-63)packages/xrpl/src/errors.ts (1)
ValidationError(156-156)
🔇 Additional comments (40)
.ci-config/rippled.cfg (1)
196-196: LGTM!The
LendingProtocolamendment is correctly added to the 2.5.0 Amendments block, following the existing pattern.packages/xrpl/src/models/transactions/common.ts (4)
27-27: LGTM!The constant correctly represents the hex length of a SHA-512/2 hash (256 bits = 64 hex characters).
346-356: LGTM!Clean type guard that correctly combines the existing
isIssuedCurrencyAmountandisMPTAmountchecks to validate token amounts (IOU or MPT).
385-393: LGTM!The ledger entry ID validation correctly checks for a 64-character hexadecimal string, consistent with how XRPL ledger object IDs are represented.
395-412: LGTM!The hex metadata validation function properly validates non-empty hex strings with a maximum length constraint. This aligns with the existing validation patterns in the codebase.
packages/xrpl/src/models/transactions/loanManage.ts (1)
68-77: LGTM!The validation logic correctly follows the established patterns: validates base transaction, then required fields with type guards, and finally applies additional format constraints for the ledger entry ID.
packages/xrpl/src/models/transactions/loanBrokerSet.ts (1)
76-137: LGTM!The validation logic is thorough and follows established patterns. Using
BigNumberforDebtMaximumcomparison correctly handles string-based XRPL numbers. The bounds checks for rate fields are consistent with the interface documentation.packages/xrpl/src/models/transactions/loanSet.ts (1)
176-201: LGTM on the overall validation structure.The validation correctly handles required fields (
LoanBrokerID,PrincipalRequested), optional fields, and applies appropriate format/bounds checks. The use of existing helper functions (validateRequiredField,validateOptionalField) follows established patterns.packages/xrpl/src/utils/hashes/ledgerSpaces.ts (1)
32-33: The namespace characters are correct and verified.Both
'V'forvaultand'l'forloanBrokermatch the rippled implementation inIndexes.cpp. The entries correspond toVAULT = 'V'andLOAN_BROKER = 'l'from rippled'sLedgerNameSpaceenum.packages/ripple-binary-codec/src/enums/definitions.json (5)
1123-1512: Ledger node and ID field definitions follow XRPL conventions.The new fields follow established XRPL patterns:
VaultNodeandLoanBrokerNodeuse UInt64 (consistent withOwnerNode,BookNode)LoanBrokerIDandLoanIDuse Hash256 (consistent withCheckID,AMMID)
2293-2312: Account fields use standard XRPL patterns.The
BorrowerandCounterpartyAccountID fields correctly useisVLEncoded: true, which is standard for all AccountID fields in XRPL.
3660-3669: Transaction type IDs appear properly allocated.The new loan-related transaction types use IDs 74-84, which don't conflict with existing transaction types and follow a logical grouping.
3720-3721: New Int32 and Int64 type mappings added.The addition of
Int32(type code 10) andInt64(type code 11) expands the binary codec's type system to support signed integers, which may be needed for certain lending protocol calculations.Verify that these type codes (10, 11) don't conflict with the rippled implementation and that signed integer handling is implemented in the codec's serialization/deserialization logic.
183-852: Verify field definitions match rippled implementation before merging.The definitions.json file is newly added for the XLS-66d lending protocol. According to the repository's own documentation, definitions should be generated from rippled's SField.cpp using the generateDefinitions.js tool. Before this change is approved:
- Confirm these field definitions (ManagementFeeRate, LoanSequence, InterestRate, etc.) are generated from or validated against rippled's current lending protocol branch
- Verify nth values don't conflict within their type scopes (UInt16 vs UInt32)
- Ensure all serialization flags (isSerialized, isSigningField, isVLEncoded) match rippled's definitions
The json structure and field organization are well-formed, but validation against the rippled source is essential for binary codec compatibility.
packages/ripple-binary-codec/HISTORY.md (1)
5-7: HISTORY.md entry looks good.The changelog entry correctly documents the new Lending Protocol support under the Unreleased section, following the existing format.
packages/xrpl/HISTORY.md (1)
9-9: HISTORY.md entry is appropriate.The changelog correctly documents the Lending Protocol support addition.
packages/xrpl/src/models/ledger/index.ts (1)
23-24: New ledger exports follow established patterns.The additions of
Loan,LoanFlags, andLoanBrokerto the ledger index follow the same pattern as other ledger types (e.g.,OfferwithOfferFlags).Also applies to: 64-66
packages/xrpl/src/utils/index.ts (1)
49-50: Hash utility exports properly integrated.The new
hashVaultandhashLoanBrokerfunctions are correctly imported and added to the exportedhashesobject, following the established pattern.Also applies to: 184-185
packages/xrpl/test/models/loanBrokerSet.test.ts (1)
1-100: Comprehensive validation test coverage.The test suite properly covers:
- Valid transaction scenario
- Required field validation (VaultID)
- Optional field format validation (LoanBrokerID, Data)
- Numeric range validation (ManagementFeeRate, CoverRateMinimum, CoverRateLiquidation)
- Non-negative value validation (DebtMaximum)
The tests follow established patterns from the codebase and provide clear, specific error messages.
packages/xrpl/test/models/loanBrokerCoverClawback.test.ts (1)
1-57: LoanBrokerCoverClawback validation tests are well-structured.The test suite covers the essential validation scenarios:
- Valid transaction with MPT amount
- LoanBrokerID format validation
- Amount type validation (ensures MPT amount format)
- Mutual requirement validation (at least one of LoanBrokerID or Amount must be present)
The tests follow established patterns and provide appropriate error messages.
packages/xrpl/test/utils/hashes.test.ts (1)
154-172: Tests correctly follow established patterns and ledger space conventions.The tests for
hashVaultandhashLoanBrokerfollow the same structure as existing hash tests (e.g.,calcEscrowEntryHash). Both hash functions are properly implemented using the correct ledger space prefixes ('V' and 'l') and follow the sha512Half pattern. The integration test inlendingProtocol.test.tsvalidates these functions work correctly with real transaction data.packages/xrpl/test/models/loanBrokerCoverWithdraw.test.ts (1)
1-52: LGTM! Comprehensive validation tests covering key paths.The test suite properly validates:
- Valid transaction passes validation
- Invalid LoanBrokerID format (non-64-char hex) triggers appropriate error
- Missing LoanBrokerID field triggers missing field error
- Malformed Amount (using
mpt_issuanceIdinstead ofmpt_issuance_id) triggers invalid field error- Missing Amount triggers missing field error
The test structure is consistent with other LoanBroker transaction tests in this PR.
packages/xrpl/test/models/loanBrokerCoverDeposit.test.ts (1)
1-51: LGTM! Test coverage aligns with LoanBrokerCoverDeposit interface.The test correctly omits the
Destinationfield (which is only present inLoanBrokerCoverWithdraw) and covers the same validation paths as the sibling test file. The error messages correctly reference the transaction type.packages/xrpl/test/models/loanBrokerDelete.test.ts (1)
1-36: LGTM! Appropriate test coverage for LoanBrokerDelete.The test correctly validates the only additional required field (
LoanBrokerID) beyond the base transaction fields. The test cases cover both invalid format and missing field scenarios.packages/xrpl/src/models/transactions/loanDelete.ts (1)
1-41: LGTM! Clean implementation following established patterns.The
LoanDeleteinterface andvalidateLoanDeletefunction follow the same structure asLoanBrokerDelete, correctly usingvalidateRequiredFieldbefore the format check withisLedgerEntryId.packages/xrpl/src/models/transactions/loanBrokerCoverClawback.ts (1)
45-68: Overall validation logic is correct.The validator properly:
- Validates base transaction structure
- Validates optional fields with appropriate type guards
- Enforces LoanBrokerID format when present
- Enforces non-negative Amount when present
- Requires at least one of LoanBrokerID or Amount
This is the only clawback transaction that allows both fields to be optional (with at least one required), which aligns with the interface documentation stating the conditions under which each is required.
packages/xrpl/src/models/transactions/transaction.ts (3)
37-54: LGTM! Imports properly added for all loan-related transactions.All nine loan-related transaction types and their validators are correctly imported following the established import style with destructuring.
178-186: LGTM! SubmittableTransaction union properly extended.All nine new loan transaction types are added to the union, maintaining the structure's consistency with other transaction types.
389-423: LGTM! Validation switch cases correctly implemented.Each loan transaction type has a corresponding case that calls its validator function. The cases are placed in alphabetical order (Loan* after Escrow*, before MPToken*), consistent with the existing code organization.
packages/xrpl/src/utils/hashes/index.ts (1)
188-218: LGTM! Consistent implementation with existing hash utilities.Both
hashVaultandhashLoanBrokerfollow the established patterns used byhashEscrowandhashPaymentChannel, correctly using the ledger space prefix, account address, and zero-padded sequence number. The requiredvault('V') andloanBroker('l') keys are properly defined inledgerSpaces.ts.packages/xrpl/src/models/transactions/loanPay.ts (1)
38-49: Validation logic looks good.The validator correctly validates the base transaction, required fields (LoanID as string, Amount as valid amount), and enforces the 64-character hexadecimal format for LoanID using the
isLedgerEntryIdhelper.packages/xrpl/src/models/transactions/loanBrokerDelete.ts (1)
31-41: Validation implementation is correct.The validator properly validates the base transaction, ensures LoanBrokerID is a string, and enforces the 64-character hexadecimal format.
packages/xrpl/src/models/ledger/Loan.ts (2)
1-127: Ledger entry definition is well-structured.The
Loaninterface properly extendsBaseLedgerEntryandHasPreviousTxnID, and includes comprehensive JSDoc documentation for all fields. The field types align with XRPL ledger entry conventions.
129-144: LoanFlags enum is correctly defined.The flag values follow the XRPL convention with proper bit positions (0x00010000, 0x00020000, 0x00040000) and clear documentation.
packages/xrpl/src/models/transactions/loanBrokerCoverDeposit.ts (1)
38-51: Validation implementation is correct.The validator properly validates all required fields and enforces the LoanBrokerID format.
packages/xrpl/src/models/ledger/LoanBroker.ts (1)
1-84: LoanBroker ledger entry is well-defined.The interface correctly extends the base types, uses
XRPLNumber(string type) appropriately for large numeric values (DebtTotal, DebtMaximum, CoverAvailable), and includes comprehensive JSDoc documentation for all fields.packages/xrpl/src/models/transactions/loanBrokerCoverWithdraw.ts (1)
46-60: Validation implementation is correct.The validator properly handles all fields including the optional
Destinationfield withisAccountvalidation.packages/xrpl/src/models/transactions/index.ts (1)
58-70: New transaction exports are properly organized.All loan-related transaction types and their associated flags/interfaces are correctly exported and follow the established pattern in the index file.
packages/xrpl/test/integration/transactions/lendingProtocol.test.ts (2)
209-237: LGTM!The helper function correctly creates an MPT issuance and vault, then derives the vault object ID using the hash utility.
239-265: LGTM!The helper function correctly creates an MPToken and extracts the issuance ID from the transaction metadata.
| [ | ||
| "DummyInt32", | ||
| { | ||
| "isSerialized": true, | ||
| "isSigningField": true, | ||
| "isVLEncoded": false, | ||
| "nth": 1, | ||
| "type": "Int32" | ||
| } | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
XLS-66d DummyInt32 ripple lending protocol specification
💡 Result:
I found the XRPL lending protocol specification XLS‑66d: it defines an on‑ledger, fixed‑term, interest‑accruing lending protocol that leverages XLS‑65d vaults/pools, uses off‑chain underwriting with on‑chain loan objects and agreements, supports pool delegates and optional first‑loss capital, and focuses on uncollateralized, institution-friendly lending workflows. [1][2]
I could not find any references to the term "DummyInt32" within the XLS‑66d descriptions or related XRPL documents and coverage. If you mean a specific field/type inside the XLS‑66d ledger objects, tell me where you saw "DummyInt32" (or I can fetch the full XLS‑66d draft/technical text and search it for that term). [1][2][3]
Sources:
[1] Ripple — Institutional DeFi on the XRP Ledger (XLS‑66d summary).
[2] Ripple — The Next Phase of Institutional DeFi on XRPL (XLS‑65/66 overview).
[3] Coverage of the XLS‑66d proposal (news article summarizing the spec).
🏁 Script executed:
# Search for DummyInt32 usage in the codebase
rg "DummyInt32" --type json --type js --type ts -B 2 -A 2Repository: XRPLF/xrpl.js
Length of output: 407
🏁 Script executed:
# Check git history for when DummyInt32 was added and any related commits
git log --oneline --all -- "**/definitions.json" | head -20Repository: XRPLF/xrpl.js
Length of output: 180
🏁 Script executed:
# Search for any comments or documentation about DummyInt32
rg "DummyInt32|Dummy" --type md --type txt -B 2 -A 2Repository: XRPLF/xrpl.js
Length of output: 39
🏁 Script executed:
# Search for all references to DummyInt32 in all file types
rg "DummyInt32" -B 3 -A 3Repository: XRPLF/xrpl.js
Length of output: 559
🏁 Script executed:
# Check if there are any test files that reference DummyInt32
fd "test|spec" -type f | xargs rg "DummyInt32" 2>/dev/null || echo "No test references found"Repository: XRPLF/xrpl.js
Length of output: 320
🏁 Script executed:
# Look at the git blame for this specific section to understand when/why it was added
git blame packages/ripple-binary-codec/src/enums/definitions.json | grep -A 10 -B 10 "DummyInt32" | head -30Repository: XRPLF/xrpl.js
Length of output: 1574
🏁 Script executed:
# Check if there are any commits that mention "dummy" or reference this field
git log --all --oneline -- packages/ripple-binary-codec/src/enums/definitions.json | xargs -I {} git show {} --stat | grep -i dummy || echo "No commits mentioning dummy found"Repository: XRPLF/xrpl.js
Length of output: 395
Remove the DummyInt32 field from definitions.json before release.
The field is not referenced anywhere in the codebase, does not appear in official XLS-66d specifications, and the "Dummy" prefix indicates it is a placeholder or testing artifact. It should be removed.
🤖 Prompt for AI Agents
packages/ripple-binary-codec/src/enums/definitions.json lines 2453-2462: remove
the entire DummyInt32 array entry (the placeholder field) from the JSON file,
ensure the surrounding JSON remains valid (no trailing commas or broken arrays),
search the repo for any references to "DummyInt32" and remove or update them if
found, then run the project's JSON lint/tests/build to confirm nothing else
depends on this field.
| [ | ||
| "CounterpartySignature", | ||
| { | ||
| "isSerialized": true, | ||
| "isSigningField": false, | ||
| "isVLEncoded": false, | ||
| "nth": 37, | ||
| "type": "STObject" | ||
| } | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for CounterpartySignature usage across the codebase
rg "CounterpartySignature" --type json --type js --type ts -A 3 -B 3Repository: XRPLF/xrpl.js
Length of output: 3780
🏁 Script executed:
# Search for validation logic and field checking
rg "isSigningField|STObject" packages/ripple-binary-codec/src/enums/definitions.json -A 2 -B 2 | head -100Repository: XRPLF/xrpl.js
Length of output: 2059
🏁 Script executed:
# Look for XLS-66d references or lending protocol documentation
rg "XLS-66d|XLS66d|lending.*protocol|CounterpartySignature" -i --type md --type txt --type jsonRepository: XRPLF/xrpl.js
Length of output: 294
🏁 Script executed:
# Search for validation or constraint logic in the codebase
rg "CounterpartySignature|nth.*37" --type js --type ts -B 5 -A 5Repository: XRPLF/xrpl.js
Length of output: 4886
🏁 Script executed:
# Get the full LoanSet interface and validation logic
cat -n packages/xrpl/src/models/transactions/loanSet.tsRepository: XRPLF/xrpl.js
Length of output: 9808
🏁 Script executed:
# Search for documentation or comments about when CounterpartySignature is required
rg "CounterpartySignature|Lending.*Protocol" packages/xrpl/src --type ts -B 5 -A 5Repository: XRPLF/xrpl.js
Length of output: 7071
🏁 Script executed:
# Check if there's any conditional logic or business rules
rg "LoanSet|CounterpartySignature" packages/xrpl/src/models/transactions --type ts -B 2 -A 10Repository: XRPLF/xrpl.js
Length of output: 17886
Update description to accurately reflect field purpose and limitations.
The CounterpartySignature field stores the counterparty's signature(s) in the XLS-66d Lending Protocol for LoanSet transactions. It contains SigningPubKey, TxnSignature, and Signers (an array for multi-signer approval), but the field itself is optional with no business logic enforcing when it must be present—only type validation when included.
🤖 Prompt for AI Agents
packages/ripple-binary-codec/src/enums/definitions.json around lines 2803 to
2812: update the `CounterpartySignature` field description to state that it
stores the counterparty's signature(s) for the XLS-66d Lending Protocol LoanSet
transactions, and that the object may include SigningPubKey, TxnSignature, and
Signers (an array for multi-signer approval); also make clear the field is
optional and there is no business-rule enforcement of its presence—only
type/schema validation when present. Ensure the wording is concise and accurate
about structure and limitation.
| import Escrow from './Escrow' | ||
| import FeeSettings from './FeeSettings' | ||
| import LedgerHashes from './LedgerHashes' | ||
| import LoanBroker from './LoanBroker' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if Loan ledger entry exists and is imported
rg -n "import.*Loan" packages/xrpl/src/models/ledger/LedgerEntry.ts
# Check if 'loan' exists in the file
rg -n "'loan" packages/xrpl/src/models/ledger/LedgerEntry.ts
# Check if Loan.ts file exists
fd -t f "Loan\.ts" packages/xrpl/src/models/ledger/Repository: XRPLF/xrpl.js
Length of output: 158
🏁 Script executed:
# Check Loan.ts content
cat -n packages/xrpl/src/models/ledger/Loan.ts
# Check the union type in LedgerEntry.ts around line 39
sed -n '30,50p' packages/xrpl/src/models/ledger/LedgerEntry.ts
# Check the filter in LedgerEntry.ts around line 66
sed -n '60,75p' packages/xrpl/src/models/ledger/LedgerEntry.ts
# List all imports in LedgerEntry.ts to see the pattern
head -40 packages/xrpl/src/models/ledger/LedgerEntry.tsRepository: XRPLF/xrpl.js
Length of output: 6116
Loan ledger entry type is missing and should be added alongside LoanBroker.
Loan.ts exists and is a properly defined ledger entry. It needs to be imported and added to the LedgerEntry union type and LedgerEntryFilter union alongside LoanBroker:
- Add
import Loan from './Loan'at line 12 (before LoanBroker import) - Add
| Loanto the union type before| LoanBroker - Add
| 'loan'to the filter before| 'loan_broker'
🤖 Prompt for AI Agents
In packages/xrpl/src/models/ledger/LedgerEntry.ts around line 12-13, the Loan
ledger entry type is not imported or included in the union types; add "import
Loan from './Loan'" at line 12 (before the existing LoanBroker import), extend
the LedgerEntry union to include "| Loan" immediately before "| LoanBroker", and
extend the LedgerEntryFilter union to include "| 'loan'" immediately before "|
'loan_broker'".
| /** | ||
| * The Fist-Loss Capital amount to deposit. | ||
| */ | ||
| Amount: Amount | MPTAmount |
There was a problem hiding this comment.
Typo in JSDoc comment.
"Fist-Loss" should be "First-Loss".
/**
- * The Fist-Loss Capital amount to deposit.
+ * The First-Loss Capital amount to deposit.
*/
Amount: Amount | MPTAmount📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * The Fist-Loss Capital amount to deposit. | |
| */ | |
| Amount: Amount | MPTAmount | |
| /** | |
| * The First-Loss Capital amount to deposit. | |
| */ | |
| Amount: Amount | MPTAmount |
🤖 Prompt for AI Agents
In packages/xrpl/src/models/transactions/loanBrokerCoverDeposit.ts around lines
26 to 29, update the JSDoc comment for the Amount property to fix the typo:
change "Fist-Loss" to "First-Loss" so the comment correctly reads "The
First-Loss Capital amount to deposit." Leave the code and type declarations
unchanged.
| /** | ||
| * The Fist-Loss Capital amount to withdraw. | ||
| */ | ||
| Amount: Amount | MPTAmount |
There was a problem hiding this comment.
Typo in JSDoc comment.
Same typo as in LoanBrokerCoverDeposit - "Fist-Loss" should be "First-Loss".
/**
- * The Fist-Loss Capital amount to withdraw.
+ * The First-Loss Capital amount to withdraw.
*/
Amount: Amount | MPTAmount📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * The Fist-Loss Capital amount to withdraw. | |
| */ | |
| Amount: Amount | MPTAmount | |
| /** | |
| * The First-Loss Capital amount to withdraw. | |
| */ | |
| Amount: Amount | MPTAmount |
🤖 Prompt for AI Agents
In packages/xrpl/src/models/transactions/loanBrokerCoverWithdraw.ts around lines
29 to 32, the JSDoc comment contains a typo "Fist-Loss" — update the comment to
"First-Loss" so it matches correct terminology and the other file
(LoanBrokerCoverDeposit); ensure spelling is corrected in the comment only
without changing the property type or signature.
| if ( | ||
| tx.OverpaymentFee != null && | ||
| (tx.OverpaymentFee < 0 || tx.OverpaymentFee > MAX_OVER_PAYMENT_FEE_RATE) | ||
| ) { | ||
| throw new ValidationError( | ||
| `LoanSet: OverpaymentFee must be must be between 0 and ${MAX_OVER_PAYMENT_FEE_RATE} inclusive`, | ||
| ) | ||
| } | ||
|
|
||
| if ( | ||
| tx.InterestRate != null && | ||
| (tx.InterestRate < 0 || tx.InterestRate > MAX_INTEREST_RATE) | ||
| ) { | ||
| throw new ValidationError( | ||
| `LoanSet: InterestRate must be must be between 0 and ${MAX_INTEREST_RATE} inclusive`, | ||
| ) | ||
| } | ||
|
|
||
| if ( | ||
| tx.LateInterestRate != null && | ||
| (tx.LateInterestRate < 0 || tx.LateInterestRate > MAX_LATE_INTEREST_RATE) | ||
| ) { | ||
| throw new ValidationError( | ||
| `LoanSet: LateInterestRate must be must be between 0 and ${MAX_LATE_INTEREST_RATE} inclusive`, | ||
| ) | ||
| } | ||
|
|
||
| if ( | ||
| tx.CloseInterestRate != null && | ||
| (tx.CloseInterestRate < 0 || tx.CloseInterestRate > MAX_CLOSE_INTEREST_RATE) | ||
| ) { | ||
| throw new ValidationError( | ||
| `LoanSet: CloseInterestRate must be must be between 0 and ${MAX_CLOSE_INTEREST_RATE} inclusive`, | ||
| ) | ||
| } | ||
|
|
||
| if ( | ||
| tx.OverpaymentInterestRate != null && | ||
| (tx.OverpaymentInterestRate < 0 || | ||
| tx.OverpaymentInterestRate > MAX_OVER_PAYMENT_INTEREST_RATE) | ||
| ) { | ||
| throw new ValidationError( | ||
| `LoanSet: OverpaymentInterestRate must be must be between 0 and ${MAX_OVER_PAYMENT_INTEREST_RATE} inclusive`, | ||
| ) | ||
| } | ||
|
|
||
| if ( | ||
| tx.OverpaymentInterestRate != null && | ||
| (tx.OverpaymentInterestRate < 0 || | ||
| tx.OverpaymentInterestRate > MAX_OVER_PAYMENT_INTEREST_RATE) | ||
| ) { | ||
| throw new ValidationError( | ||
| `LoanSet: OverpaymentInterestRate must be must be between 0 and ${MAX_OVER_PAYMENT_INTEREST_RATE} inclusive`, | ||
| ) | ||
| } | ||
|
|
||
| if (tx.PaymentInterval != null && tx.PaymentInterval < MIN_PAYMENT_INTERVAL) { | ||
| throw new ValidationError( | ||
| `LoanSet: PaymentInterval must be must be greater than or equal to ${MIN_PAYMENT_INTERVAL}`, | ||
| ) |
There was a problem hiding this comment.
Fix typo "must be must be" in error messages.
Multiple error messages contain the duplicate phrase "must be must be". This affects lines 214, 223, 232, 241, 251, and 267.
Example fix for one of them:
- `LoanSet: OverpaymentFee must be must be between 0 and ${MAX_OVER_PAYMENT_FEE_RATE} inclusive`,
+ `LoanSet: OverpaymentFee must be between 0 and ${MAX_OVER_PAYMENT_FEE_RATE} inclusive`,Apply the same fix to all affected error messages.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| tx.OverpaymentFee != null && | |
| (tx.OverpaymentFee < 0 || tx.OverpaymentFee > MAX_OVER_PAYMENT_FEE_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: OverpaymentFee must be must be between 0 and ${MAX_OVER_PAYMENT_FEE_RATE} inclusive`, | |
| ) | |
| } | |
| if ( | |
| tx.InterestRate != null && | |
| (tx.InterestRate < 0 || tx.InterestRate > MAX_INTEREST_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: InterestRate must be must be between 0 and ${MAX_INTEREST_RATE} inclusive`, | |
| ) | |
| } | |
| if ( | |
| tx.LateInterestRate != null && | |
| (tx.LateInterestRate < 0 || tx.LateInterestRate > MAX_LATE_INTEREST_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: LateInterestRate must be must be between 0 and ${MAX_LATE_INTEREST_RATE} inclusive`, | |
| ) | |
| } | |
| if ( | |
| tx.CloseInterestRate != null && | |
| (tx.CloseInterestRate < 0 || tx.CloseInterestRate > MAX_CLOSE_INTEREST_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: CloseInterestRate must be must be between 0 and ${MAX_CLOSE_INTEREST_RATE} inclusive`, | |
| ) | |
| } | |
| if ( | |
| tx.OverpaymentInterestRate != null && | |
| (tx.OverpaymentInterestRate < 0 || | |
| tx.OverpaymentInterestRate > MAX_OVER_PAYMENT_INTEREST_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: OverpaymentInterestRate must be must be between 0 and ${MAX_OVER_PAYMENT_INTEREST_RATE} inclusive`, | |
| ) | |
| } | |
| if ( | |
| tx.OverpaymentInterestRate != null && | |
| (tx.OverpaymentInterestRate < 0 || | |
| tx.OverpaymentInterestRate > MAX_OVER_PAYMENT_INTEREST_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: OverpaymentInterestRate must be must be between 0 and ${MAX_OVER_PAYMENT_INTEREST_RATE} inclusive`, | |
| ) | |
| } | |
| if (tx.PaymentInterval != null && tx.PaymentInterval < MIN_PAYMENT_INTERVAL) { | |
| throw new ValidationError( | |
| `LoanSet: PaymentInterval must be must be greater than or equal to ${MIN_PAYMENT_INTERVAL}`, | |
| ) | |
| if ( | |
| tx.OverpaymentFee != null && | |
| (tx.OverpaymentFee < 0 || tx.OverpaymentFee > MAX_OVER_PAYMENT_FEE_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: OverpaymentFee must be between 0 and ${MAX_OVER_PAYMENT_FEE_RATE} inclusive`, | |
| ) | |
| } | |
| if ( | |
| tx.InterestRate != null && | |
| (tx.InterestRate < 0 || tx.InterestRate > MAX_INTEREST_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: InterestRate must be between 0 and ${MAX_INTEREST_RATE} inclusive`, | |
| ) | |
| } | |
| if ( | |
| tx.LateInterestRate != null && | |
| (tx.LateInterestRate < 0 || tx.LateInterestRate > MAX_LATE_INTEREST_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: LateInterestRate must be between 0 and ${MAX_LATE_INTEREST_RATE} inclusive`, | |
| ) | |
| } | |
| if ( | |
| tx.CloseInterestRate != null && | |
| (tx.CloseInterestRate < 0 || tx.CloseInterestRate > MAX_CLOSE_INTEREST_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: CloseInterestRate must be between 0 and ${MAX_CLOSE_INTEREST_RATE} inclusive`, | |
| ) | |
| } | |
| if ( | |
| tx.OverpaymentInterestRate != null && | |
| (tx.OverpaymentInterestRate < 0 || | |
| tx.OverpaymentInterestRate > MAX_OVER_PAYMENT_INTEREST_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: OverpaymentInterestRate must be between 0 and ${MAX_OVER_PAYMENT_INTEREST_RATE} inclusive`, | |
| ) | |
| } | |
| if ( | |
| tx.OverpaymentInterestRate != null && | |
| (tx.OverpaymentInterestRate < 0 || | |
| tx.OverpaymentInterestRate > MAX_OVER_PAYMENT_INTEREST_RATE) | |
| ) { | |
| throw new ValidationError( | |
| `LoanSet: OverpaymentInterestRate must be between 0 and ${MAX_OVER_PAYMENT_INTEREST_RATE} inclusive`, | |
| ) | |
| } | |
| if (tx.PaymentInterval != null && tx.PaymentInterval < MIN_PAYMENT_INTERVAL) { | |
| throw new ValidationError( | |
| `LoanSet: PaymentInterval must be greater than or equal to ${MIN_PAYMENT_INTERVAL}`, | |
| ) |
🤖 Prompt for AI Agents
In packages/xrpl/src/models/transactions/loanSet.ts around lines 209 to 268,
several ValidationError messages contain the duplicated phrase "must be must
be"; update each message to read correctly (e.g., "LoanSet: OverpaymentFee must
be between 0 and ${MAX_OVER_PAYMENT_FEE_RATE} inclusive") for the checks at
lines ~214, ~223, ~232, ~241, ~251, and ~267 so all error strings remove the
extra "must be" and match the same concise phrasing.
| if ( | ||
| tx.OverpaymentInterestRate != null && | ||
| (tx.OverpaymentInterestRate < 0 || | ||
| tx.OverpaymentInterestRate > MAX_OVER_PAYMENT_INTEREST_RATE) | ||
| ) { | ||
| throw new ValidationError( | ||
| `LoanSet: OverpaymentInterestRate must be must be between 0 and ${MAX_OVER_PAYMENT_INTEREST_RATE} inclusive`, | ||
| ) | ||
| } | ||
|
|
||
| if ( | ||
| tx.OverpaymentInterestRate != null && | ||
| (tx.OverpaymentInterestRate < 0 || | ||
| tx.OverpaymentInterestRate > MAX_OVER_PAYMENT_INTEREST_RATE) | ||
| ) { | ||
| throw new ValidationError( | ||
| `LoanSet: OverpaymentInterestRate must be must be between 0 and ${MAX_OVER_PAYMENT_INTEREST_RATE} inclusive`, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Remove duplicate validation block for OverpaymentInterestRate.
The validation for OverpaymentInterestRate is duplicated (lines 245-253 and 255-263). Remove the second occurrence.
if (
tx.OverpaymentInterestRate != null &&
(tx.OverpaymentInterestRate < 0 ||
tx.OverpaymentInterestRate > MAX_OVER_PAYMENT_INTEREST_RATE)
) {
throw new ValidationError(
`LoanSet: OverpaymentInterestRate must be must be between 0 and ${MAX_OVER_PAYMENT_INTEREST_RATE} inclusive`,
)
}
-
- if (
- tx.OverpaymentInterestRate != null &&
- (tx.OverpaymentInterestRate < 0 ||
- tx.OverpaymentInterestRate > MAX_OVER_PAYMENT_INTEREST_RATE)
- ) {
- throw new ValidationError(
- `LoanSet: OverpaymentInterestRate must be must be between 0 and ${MAX_OVER_PAYMENT_INTEREST_RATE} inclusive`,
- )
- }
if (tx.PaymentInterval != null && tx.PaymentInterval < MIN_PAYMENT_INTERVAL) {🤖 Prompt for AI Agents
In packages/xrpl/src/models/transactions/loanSet.ts around lines 245 to 263
there is a duplicated validation block for tx.OverpaymentInterestRate (the same
if-check and ValidationError throw appears twice); remove the second occurrence
(lines 255–263) so the validation only exists once, keeping the original message
and bounds check unchanged.
| afterEach(() => { | ||
| teardownClient(testContext) | ||
| }, TIMEOUT) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find teardownClient definition
rg -n "teardownClient" packages/xrpl/test/integration/setup.ts -A 10
# Also check if there are other afterEach patterns in integration tests
rg -n "afterEach" packages/xrpl/test/integration/transactions/ -A 3 | head -50Repository: XRPLF/xrpl.js
Length of output: 4221
Add async and await to properly handle the async teardownClient cleanup.
The teardownClient function is async (returns Promise<void>) and must be awaited in the afterEach hook. Change line 47-49 from:
afterEach(() => {
teardownClient(testContext)
}, TIMEOUT)
to:
afterEach(async () => {
await teardownClient(testContext)
}, TIMEOUT)
All other integration test files use this pattern correctly (e.g., mptokenIssuanceCreate.test.ts, delegateSet.test.ts). Without awaiting, client cleanup is not guaranteed to complete before the next test runs, potentially causing resource leaks and test interference.
🤖 Prompt for AI Agents
In packages/xrpl/test/integration/transactions/lendingProtocol.test.ts around
lines 47 to 49, the afterEach hook currently calls the async teardownClient
without awaiting it; change the hook to be async and await
teardownClient(testContext) so the Promise is resolved before proceeding to the
next test, ensuring proper cleanup and preventing test interference.
| console.log(JSON.stringify(loanSetTx)) | ||
| console.log(signer1.classicAddress) | ||
| console.log(signer1.privateKey) | ||
| console.log(signer1.seed) | ||
| console.log(sign1) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statements.
These debug logging statements should be removed before merging. They leak potentially sensitive test data (private keys, seeds) and clutter test output.
- console.log(JSON.stringify(loanSetTx))
- console.log(signer1.classicAddress)
- console.log(signer1.privateKey)
- console.log(signer1.seed)
- console.log(sign1)
-
loanSetTx.CounterpartySignature = {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log(JSON.stringify(loanSetTx)) | |
| console.log(signer1.classicAddress) | |
| console.log(signer1.privateKey) | |
| console.log(signer1.seed) | |
| console.log(sign1) | |
| loanSetTx.CounterpartySignature = {} |
🤖 Prompt for AI Agents
In packages/xrpl/test/integration/transactions/lendingProtocol.test.ts around
lines 174 to 178, remove the debug console.log statements that print loanSetTx,
signer1.classicAddress, signer1.privateKey, signer1.seed, and sign1; replace
them with no-ops or assertions if needed (e.g., assert expected values) or use a
non-sensitive test logger, ensuring no private keys or seeds are logged and test
output remains clean.
| // eslint-disable-next-line max-params -- required here for three wallets | ||
| async function setupMultiSigning( | ||
| testContext: XrplIntegrationTestContext, | ||
| wallet: Wallet, | ||
| singer1: Wallet, | ||
| singer2: Wallet, | ||
| ): Promise<void> { | ||
| const transaction: SignerListSet = { | ||
| TransactionType: 'SignerListSet', | ||
| Account: wallet.address, | ||
| SignerQuorum: 2, | ||
| SignerEntries: [ | ||
| { SignerEntry: { Account: singer1.address, SignerWeight: 1 } }, | ||
| { SignerEntry: { Account: singer2.address, SignerWeight: 1 } }, | ||
| ], | ||
| } | ||
|
|
||
| await testTransaction(testContext.client, transaction, wallet) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix parameter name typos: singer → signer.
The parameters are misspelled as singer1 and singer2 instead of signer1 and signer2.
// eslint-disable-next-line max-params -- required here for three wallets
async function setupMultiSigning(
testContext: XrplIntegrationTestContext,
wallet: Wallet,
- singer1: Wallet,
- singer2: Wallet,
+ signer1: Wallet,
+ signer2: Wallet,
): Promise<void> {
const transaction: SignerListSet = {
TransactionType: 'SignerListSet',
Account: wallet.address,
SignerQuorum: 2,
SignerEntries: [
- { SignerEntry: { Account: singer1.address, SignerWeight: 1 } },
- { SignerEntry: { Account: singer2.address, SignerWeight: 1 } },
+ { SignerEntry: { Account: signer1.address, SignerWeight: 1 } },
+ { SignerEntry: { Account: signer2.address, SignerWeight: 1 } },
],
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // eslint-disable-next-line max-params -- required here for three wallets | |
| async function setupMultiSigning( | |
| testContext: XrplIntegrationTestContext, | |
| wallet: Wallet, | |
| singer1: Wallet, | |
| singer2: Wallet, | |
| ): Promise<void> { | |
| const transaction: SignerListSet = { | |
| TransactionType: 'SignerListSet', | |
| Account: wallet.address, | |
| SignerQuorum: 2, | |
| SignerEntries: [ | |
| { SignerEntry: { Account: singer1.address, SignerWeight: 1 } }, | |
| { SignerEntry: { Account: singer2.address, SignerWeight: 1 } }, | |
| ], | |
| } | |
| await testTransaction(testContext.client, transaction, wallet) | |
| } | |
| // eslint-disable-next-line max-params -- required here for three wallets | |
| async function setupMultiSigning( | |
| testContext: XrplIntegrationTestContext, | |
| wallet: Wallet, | |
| signer1: Wallet, | |
| signer2: Wallet, | |
| ): Promise<void> { | |
| const transaction: SignerListSet = { | |
| TransactionType: 'SignerListSet', | |
| Account: wallet.address, | |
| SignerQuorum: 2, | |
| SignerEntries: [ | |
| { SignerEntry: { Account: signer1.address, SignerWeight: 1 } }, | |
| { SignerEntry: { Account: signer2.address, SignerWeight: 1 } }, | |
| ], | |
| } | |
| await testTransaction(testContext.client, transaction, wallet) | |
| } |
🤖 Prompt for AI Agents
In packages/xrpl/test/integration/transactions/lendingProtocol.test.ts around
lines 267 to 285, the function parameters and internal usages are misspelled as
"singer1" and "singer2" instead of "signer1" and "signer2"; rename the
parameters in the function signature to signer1 and signer2 and update every use
inside the function (SignerEntries and any references) to the new names, then
update any call sites in the test suite that pass those arguments to use the
corrected parameter names.
High Level Overview of Change
Placeholder PR
Context of Change
Type of Change
Did you update HISTORY.md?
Test Plan