Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR broadens xrpl.js lending-protocol support by adding/adjusting Loan/Vault/LoanBroker transaction & ledger typing/validation, related hashing utilities, and wallet helper functions for counterparty signing; it also updates CI/tooling and release/docs to match the new workflows and dependency versions.
Changes:
- Add lending-protocol transaction models (Loan*) and ledger entry models (Loan/LoanBroker), plus new hash helpers (Vault/LoanBroker/Loan).
- Add Wallet helpers for counterparty signing of
LoanSet, and extend autofill fee calculation forLoanSet(counterparty signer count) andVaultCreate. - Update release documentation, CI workflows, lint/tooling configs, and ripple-binary-codec numeric/Issue handling + tests.
Reviewed changes
Copilot reviewed 115 out of 117 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| UNIQUE_SETUPS.md | Removes outdated Deno workaround documentation. |
| RELEASE.md | Expands release pipeline instructions (stable/beta) and workflow inputs. |
| README.md | Updates XRPL code samples link path. |
| packages/xrpl/test/wallet/counterpartySigner.test.ts | Adds unit tests for counterparty signing helpers (single + multisign combine). |
| packages/xrpl/test/utils/hashes.test.ts | Adds tests for new Vault/LoanBroker/Loan hash functions. |
| packages/xrpl/test/setupClient.ts | Allows passing ClientOptions into test client setup. |
| packages/xrpl/test/models/vaultCreate.test.ts | Updates MPTokenMetadata import; adds Asset/Scale validation tests. |
| packages/xrpl/test/models/utils.test.ts | Splits MPTokenMetadata fixtures; adds encode/decode tests and error cases. |
| packages/xrpl/test/models/MPTokenIssuanceCreate.test.ts | Updates MPTokenMetadata warnings source; adjusts warning expectations. |
| packages/xrpl/test/models/loanPay.test.ts | Adds unit tests for LoanPay validation and flag constraints. |
| packages/xrpl/test/models/loanManage.test.ts | Adds unit tests for LoanManage validation and flag conflicts. |
| packages/xrpl/test/models/loanBrokerSet.test.ts | Adds unit tests for LoanBrokerSet validation rules. |
| packages/xrpl/test/models/loanBrokerDelete.test.ts | Adds unit tests for LoanBrokerDelete validation rules. |
| packages/xrpl/test/models/loanBrokerCoverWithdraw.test.ts | Adds unit tests for LoanBrokerCoverWithdraw validation rules. |
| packages/xrpl/test/models/loanBrokerCoverDeposit.test.ts | Adds unit tests for LoanBrokerCoverDeposit validation rules. |
| packages/xrpl/test/models/loanBrokerCoverClawback.test.ts | Adds unit tests for LoanBrokerCoverClawback validation rules. |
| packages/xrpl/test/integration/utils.ts | Adds helper to check amendment enablement via feature command. |
| packages/xrpl/test/integration/transactions/singleAssetVault.test.ts | Updates vault flows (AssetsMaximum/Scale, destination fields, MPT vault path). |
| packages/xrpl/test/integration/transactions/oracleSet.test.ts | Uses validated ledger close time to derive LastUpdateTime. |
| packages/xrpl/test/integration/transactions/oracleDelete.test.ts | Uses validated ledger close time to derive LastUpdateTime. |
| packages/xrpl/test/integration/transactions/mptokenIssuanceCreate.test.ts | Uses encode/decode MPTokenMetadata in integration test. |
| packages/xrpl/test/integration/transactions/delegateSet.test.ts | Updates expected error code string for permission failure. |
| packages/xrpl/test/integration/transactions/ammBid.test.ts | Adjusts floating-point diff constant for expected price. |
| packages/xrpl/test/integration/requests/vaultInfo.test.ts | Expands vault_info assertions and adds IOU+Scale scenario. |
| packages/xrpl/test/integration/requests/getAggregatePrice.test.ts | Uses validated ledger close time to derive LastUpdateTime. |
| packages/xrpl/test/faucet/fundWallet.test.ts | Updates faucet error serialization expectation; ensures disconnect in finally. |
| packages/xrpl/test/connection.test.ts | Makes connection timeout configurable in tests; reduces waits. |
| packages/xrpl/test/client/autofill.test.ts | Adds autofill test for LoanSet fee calculation. |
| packages/xrpl/src/Wallet/utils.ts | Refactors signer comparison/tx decode helpers; adds exported computeSignature. |
| packages/xrpl/src/Wallet/index.ts | Switches Wallet signing to shared computeSignature; re-exports wallet helpers. |
| packages/xrpl/src/Wallet/fundWallet.ts | Adds faucetProtocol; improves error parsing/serialization. |
| packages/xrpl/src/Wallet/counterpartySigner.ts | Introduces LoanSet counterparty signing + multisign combination helpers. |
| packages/xrpl/src/Wallet/batchSigner.ts | Changes Wallet import to type-only to avoid runtime import. |
| packages/xrpl/src/Wallet/authorizeChannel.ts | Changes Wallet import to type-only to avoid runtime import. |
| packages/xrpl/src/utils/index.ts | Exposes encodeForSigningBatch and new lending-related hash helpers. |
| packages/xrpl/src/utils/hashes/ledgerSpaces.ts | Adds ledger namespace prefixes for vault/loanBroker/loan and updates reference link. |
| packages/xrpl/src/utils/hashes/index.ts | Adds hashVault/hashLoanBroker/hashLoan helpers. |
| packages/xrpl/src/sugar/getOrderbook.ts | Handles possible null result from BigNumber comparison in sort. |
| packages/xrpl/src/sugar/autofill.ts | Adds LoanSet counterparty-signer fee logic; treats VaultCreate as special fee. |
| packages/xrpl/src/models/utils/flags.ts | Registers flag maps for LoanManage and LoanPay. |
| packages/xrpl/src/models/transactions/vaultWithdraw.ts | Adds DestinationTag to VaultWithdraw validation/type. |
| packages/xrpl/src/models/transactions/vaultCreate.ts | Adds Scale field + validation; moves MPTokenMetadata validation imports. |
| packages/xrpl/src/models/transactions/transaction.ts | Registers new Loan* and LoanBroker* transaction types in validation union/switch. |
| packages/xrpl/src/models/transactions/MPTokenIssuanceCreate.ts | Moves MPTokenMetadata validation imports; updates doc comments to XLS-89. |
| packages/xrpl/src/models/transactions/loanSet.ts | Adds LoanSet transaction model + validation. |
| packages/xrpl/src/models/transactions/loanPay.ts | Adds LoanPay transaction model + validation (flag exclusivity). |
| packages/xrpl/src/models/transactions/loanManage.ts | Adds LoanManage transaction model + validation (flag conflict). |
| packages/xrpl/src/models/transactions/loanDelete.ts | Adds LoanDelete transaction model + validation. |
| packages/xrpl/src/models/transactions/loanBrokerSet.ts | Adds LoanBrokerSet transaction model + validation. |
| packages/xrpl/src/models/transactions/loanBrokerDelete.ts | Adds LoanBrokerDelete transaction model + validation. |
| packages/xrpl/src/models/transactions/loanBrokerCoverWithdraw.ts | Adds LoanBrokerCoverWithdraw transaction model + validation (+ dest/tag). |
| packages/xrpl/src/models/transactions/loanBrokerCoverDeposit.ts | Adds LoanBrokerCoverDeposit transaction model + validation. |
| packages/xrpl/src/models/transactions/loanBrokerCoverClawback.ts | Adds LoanBrokerCoverClawback transaction model + validation. |
| packages/xrpl/src/models/transactions/index.ts | Adjusts exports (GlobalFlags, Batch flags/types, Loan* types). |
| packages/xrpl/src/models/transactions/common.ts | Refactors currency/amount guards; adds isTokenAmount/isLedgerEntryId/validateHexMetadata; removes MPTokenMetadata validator (moved). |
| packages/xrpl/src/models/methods/vaultInfo.ts | Makes AssetsAvailable/AssetsTotal optional; adds AssetsMaximum/Data/Scale fields. |
| packages/xrpl/src/models/ledger/Vault.ts | Fixes Flags typing; makes totals optional; renames ShareMPTID; adds Scale + VaultFlags. |
| packages/xrpl/src/models/ledger/LoanBroker.ts | Adds LoanBroker ledger entry type definition. |
| packages/xrpl/src/models/ledger/Loan.ts | Adds Loan ledger entry type definition + LoanFlags. |
| packages/xrpl/src/models/ledger/LedgerEntry.ts | Adds Loan and LoanBroker to LedgerEntry unions/filters. |
| packages/xrpl/src/models/ledger/index.ts | Exports Loan/LoanFlags/LoanBroker and VaultFlags. |
| packages/xrpl/src/models/index.ts | Exports MPTokenMetadata encode/decode/validate utilities. |
| packages/xrpl/src/models/common/index.ts | Updates MPTokenMetadata types/docs to XLS-89 (uris, additional_info typing, etc.). |
| packages/xrpl/src/index.ts | Re-exports Wallet module surface (export * from './Wallet'). |
| packages/xrpl/src/client/index.ts | Updates fundWallet docs to reflect protocol/host usage; adds local faucet example. |
| packages/xrpl/package.json | Adds fast-json-stable-stringify; bumps typedoc. |
| packages/xrpl/HISTORY.md | Adds release notes entries describing newly added functionality. |
| packages/xrpl/eslint.config.js | Adjusts eslint-config import for default export. |
| packages/secret-numbers/eslint.config.js | Adjusts eslint-config import for default export. |
| packages/ripple-keypairs/eslint.config.js | Adjusts eslint-config import for default export. |
| packages/ripple-binary-codec/tools/generateDefinitions.js | Improves parsing regexes; adds DATATYPE rename mapping. |
| packages/ripple-binary-codec/test/uint.test.ts | Adds UInt decimal validation tests; extends imports. |
| packages/ripple-binary-codec/test/st-number.test.ts | Expands STNumber coverage for boundaries/rounding/overflow/underflow. |
| packages/ripple-binary-codec/test/signing-data-encoding.test.ts | Adds multisign encoding test for LoanSet with non-empty SigningPubKey. |
| packages/ripple-binary-codec/test/quality.test.ts | Updates bytesToHex import path to @transia/isomorphic. |
| packages/ripple-binary-codec/test/issue.test.ts | Updates Issue parser usage and error expectation. |
| packages/ripple-binary-codec/test/int.test.ts | Adds Int32 type tests + encode/decode coverage with LoanScale. |
| packages/ripple-binary-codec/test/fixtures/contract-call-binary.json | Updates serialized fixture blob. |
| packages/ripple-binary-codec/test/definitions.test.ts | Reformats/updates custom definitions tests and IDs. |
| packages/ripple-binary-codec/test/binary-parser.test.ts | Updates bytesToHex import path to @transia/isomorphic. |
| packages/ripple-binary-codec/src/types/uint-8.ts | Rejects non-integer numbers in UInt8.from. |
| packages/ripple-binary-codec/src/types/uint-64.ts | Rejects non-integer numbers in UInt64.from. |
| packages/ripple-binary-codec/src/types/uint-32.ts | Rejects non-integer numbers in UInt32.from. |
| packages/ripple-binary-codec/src/types/uint-16.ts | Rejects non-integer numbers; fixes error message text. |
| packages/ripple-binary-codec/src/types/st-object.ts | Refactors formatting; preserves X-address handling and field sorting/serialization logic. |
| packages/ripple-binary-codec/src/types/st-number.ts | Reworks STNumber normalization/serialization for larger mantissa range and rounding behavior. |
| packages/ripple-binary-codec/src/types/issue.ts | Refactors Issue encoding/parsing; adds MPT Issue serialization form. |
| packages/ripple-binary-codec/src/types/int.ts | Adds signed Int base type for comparisons/range checks. |
| packages/ripple-binary-codec/src/types/int-32.ts | Adds Int32 serialized type implementation. |
| packages/ripple-binary-codec/src/types/index.ts | Registers/exports Int32 in coreTypes. |
| packages/ripple-binary-codec/src/types/hash-192.ts | Uses optional chaining when checking zero-length bytes. |
| packages/ripple-binary-codec/src/types/hash-160.ts | Uses optional chaining when checking zero-length bytes. |
| packages/ripple-binary-codec/src/types/hash-128.ts | Uses optional chaining when checking zero-length bytes. |
| packages/ripple-binary-codec/src/index.ts | Allows encodeForMultisigning with non-empty SigningPubKey. |
| packages/ripple-binary-codec/package.json | Fixes test script to run all tests instead of a local absolute path. |
| packages/ripple-binary-codec/HISTORY.md | Adds release notes for Int32/STNumber/UInt decimal fixes. |
| packages/ripple-binary-codec/eslint.config.js | Adjusts eslint-config import for default export. |
| packages/ripple-address-codec/eslint.config.js | Adjusts eslint-config import for default export. |
| packages/isomorphic/eslint.config.js | Adjusts eslint-config import for default export. |
| package.json | Updates tooling deps (eslint-config, chai, expect, jest-mock, lerna, etc.). |
| lerna.json | Migrates lerna config from workspaces flag to explicit packages list + schema. |
| jest.config.base.js | Adjusts Jest transforms/ignore patterns to handle chai ESM packaging. |
| CONTRIBUTING.md | Updates docker run command and removes docgen step from release checklist. |
| .prettierignore | Ignores NX workspace data directory. |
| .github/workflows/nodejs.yml | Updates CI matrix (node versions), adds workflow inputs/toggles, splits docs job out. |
| .github/workflows/generate-documentation.yml | Adds standalone reusable workflow to build + deploy docs. |
| .github/workflows/faucet_test.yml | Adds input to optionally skip faucet tests. |
| .github/dependabot.yml | Changes update cadence to quarterly and increases PR limit. |
| .ci-config/rippled.cfg | Adjusts enabled amendments (adds LendingProtocol / PermissionDelegationV1_1; removes older NFT amendments). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+48
to
+51
| "expect": "^30.2.0", | ||
| "jest": "^29.3.1", | ||
| "jest-mock": "^29.3.1", | ||
| "lerna": "^4.0.0", | ||
| "jest-mock": "^30.2.0", | ||
| "lerna": "^9.0.4", |
There was a problem hiding this comment.
expect and jest-mock were bumped to major v30 while jest remains on v29. This version mismatch is likely to cause installation/peer-dependency issues or runtime incompatibilities in tests. Consider aligning these packages on the same major version (either upgrade jest/ts-jest to v30 or keep expect/jest-mock on v29).
Comment on lines
+1
to
+16
| import { bytesToHex } from "@transia/isomorphic/utils"; | ||
| import BigNumber from "bignumber.js"; | ||
| import { | ||
| decodeAccountID, | ||
| isValidXAddress, | ||
| xAddressToClassicAddress, | ||
| } from "@transia/ripple-address-codec"; | ||
| import { | ||
| decode, | ||
| encode, | ||
| encodeForMultisigning, | ||
| encodeForSigning, | ||
| } from "@transia/ripple-binary-codec"; | ||
| import { sign } from "@transia/ripple-keypairs"; | ||
|
|
||
| import { Transaction } from '../models' | ||
| import { Transaction } from "../models"; |
There was a problem hiding this comment.
This file switches to double quotes + semicolons while the surrounding codebase (e.g., packages/xrpl/src/Wallet/index.ts) uses single quotes and no semicolons. If ESLint/Prettier is configured for the existing style, this will create avoidable lint/format churn. Please reformat this file to match repository conventions (or apply the formatter to the whole package consistently).
Comment on lines
+46
to
+50
| /** | ||
| * The 1/10th basis point fee charged by the Lending Protocol Owner. Valid values are between 0 and 10000 inclusive (1% - 10%). | ||
| */ | ||
| ManagementFeeRate?: number | ||
|
|
There was a problem hiding this comment.
The JSDoc for ManagementFeeRate says the range 0–10000 corresponds to “(1% - 10%)”, but with 1/10th-basis-point units that range corresponds to 0%–10% (and 0 is explicitly allowed by the validator). Please correct the comment to avoid misleading API consumers.
Comment on lines
+13
to
+43
| **Stable release ** | ||
| 1. Create a release branch. A qualified branch name should start with "release-" or "release/", **case-insensitive**. e.g: `release/xrpl@4.3.8`, `release-xrpl-4.3.8`, `Release/xrpl@4.3.8`. | ||
| 2. Update the **`version`** field in `packages/<package_name>/package.json` to the intended release version. | ||
| ```json | ||
| { | ||
| "name": "<package_name>", | ||
| "version": "x.y.z" | ||
| } | ||
| ``` | ||
| 2. Run npm i to update the package-lock with the updated versions and commit the lock file to the releaes branch | ||
| 3. Run npm i to refresh package-lock.json and commit it. | ||
|
|
||
| **Beta release ** | ||
| 1. Create a release branch. There is no restriction for branch name. | ||
| 2. Update the **`version`** field in `packages/<package_name>/package.json` to the intended beta release version. | ||
| ```json | ||
| { | ||
| "name": "<package_name>", | ||
| "version": "x.y.z-<beta|rc>.a" | ||
| } | ||
| ``` | ||
| 3. Run `npm i` to refresh `package-lock.json` and commit it. | ||
|
|
||
| ### **Triggering a Release** | ||
|
|
||
| 1. Go to **GitHub → Actions → Release Pipeline → Run workflow** | ||
| 2. Fill in these fields: | ||
| - **package_name** → The folder name under `packages/`, e.g., `xrpl` or `ripple-address-codec`. | ||
| - **release_branch** → The Git branch the release is generated from, e.g., `release/xrpl@4.3.8`. | ||
| - **npmjs_dist_tag** → The npm distribution tag to publish under. Defaults to `latest`. | ||
| - Examples: | ||
| - `latest` → Standard production release | ||
| - `beta` → Pre-release for testing | ||
| - `rc` → Release candidate | ||
| 1. Go to **GitHub → Actions → Release Pipeline → Run workflow** (must be triggered from `main`). | ||
|
|
||
| 2. Triggering the workflow with following requied inputs: | ||
|
|
||
| - **Stable release** | ||
| - `release_branch_name`: e.g., `release/xrpl@4.3.8` or `release-xrpl-4.3.8` (must start with `release-`/`release/`, **case-insensitive**). | ||
| - `package_name`: e.g., `xrpl`. |
There was a problem hiding this comment.
Spelling/formatting issues in this section make the release guide harder to follow: extra space in “Stable release ” / “Beta release ” headings, “requied” → “required”, and “relase” → “release”. Please fix these typos.
Comment on lines
72
to
+86
| ### 2. **Run Tests** | ||
| - Triggers the `faucet_test.yml` and `nodejs.yml` workflows to run unit, integration, and faucet tests against the specified Git ref. | ||
| - Ensures the code at the given Git ref passes all tests. | ||
| - Triggers the `faucet_test.yml` and `nodejs.yml` workflows to run unit, integration, and faucet tests against the specified Git ref. Integration and taucet tests are skipped for beta release. | ||
| - Ensures the code at the given Git ref passes required tests. | ||
|
|
||
| --- | ||
|
|
||
| ### 3. **Pre-Release Steps** | ||
| - Builds the npm package. | ||
| - Generates a CycloneDX SBOM (Software Bill of Materials). | ||
| - Runs a vulnerability scan with Trivy. | ||
| - Uploads the SBOM to OWASP Dependency-Track for tracking vulnerabilities. | ||
| - Packages the module with Lerna and uploads the tarball as an artifact. | ||
| - Posts failure notifications to Slack.. | ||
| - Create a Github issue for detected vulnerabilities. | ||
| - Automatically raise a PR from relase branch to main branch for stable release |
There was a problem hiding this comment.
Typo: “taucet tests” should be “faucet tests”. Also there’s a double period in “Slack..”.
Comment on lines
+13
to
+16
| const borrowerWallet = Wallet.fromSeed('sEd7FqVHfNZ2UdGAwjssxPev2ujwJoT') | ||
| const singedLoanSet = { | ||
| TransactionType: 'LoanSet', | ||
| Flags: 0, |
There was a problem hiding this comment.
Minor typo in variable name: singedLoanSet reads like a misspelling of signedLoanSet, which makes the test harder to scan. Consider renaming for clarity.
Comment on lines
5
to
+12
| ## Unreleased | ||
|
|
||
| ## 4.6.0 (2026-02-12) | ||
|
|
||
| ### Added | ||
| * Add `faucetProtocol` (http or https) option to `fundWallet` method. Makes `fundWallet` work with locally running faucet servers. | ||
| * Add `signLoanSetByCounterparty` and `combineLoanSetCounterpartySigners` helper functions to sign and combine LoanSet transactions signed by the counterparty. | ||
| * Add newly added fields to `Loan`, `LoanBroker` and `Vault` ledger objects and lending protocol related transaction types. |
There was a problem hiding this comment.
The PR title/description (“Fix contract datatype” with an otherwise empty template) doesn’t reflect the actual scope here (release pipeline docs, CI workflow updates, new Loan* transaction models, Vault changes, binary-codec changes, etc.). Please update the PR description (and possibly title) to accurately summarize the changes and their motivation/risk.
Comment on lines
+386
to
+390
| ) | ||
| assert.equal(vault.Data, vaultCreateTx.Data) | ||
| assert.equal(assetsMaximum, '500') | ||
| assert.notEqual(vaultFlags, VaultFlags.lsfVaultPrivate) | ||
|
|
There was a problem hiding this comment.
assert.notEqual(vaultFlags, VaultFlags.lsfVaultPrivate) only checks for exact equality. If lsfVaultPrivate is set alongside other bits, this assertion will incorrectly pass. Use a bitwise check to ensure the private bit is not set (e.g., (vaultFlags & VaultFlags.lsfVaultPrivate) === 0).
Collaborator
Author
|
@dangell7 pretty sure the large diff is because of the squash commit on the merge, it's a 4-line fix otherwise |
2 tasks
mvadari
force-pushed
the
fix-contract-datatype
branch
from
b0dd429 to
df5cd97
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
Context of Change
Type of Change
Did you update HISTORY.md?
Test Plan