feat: add Single Asset Vault (XLS-65) #3008
Conversation
WalkthroughThis update introduces comprehensive support for the Single Asset Vault (XLS-65) feature in the XRPL codebase. It adds new transaction models, validation logic, serialization types, and API methods for vault operations, updates configuration and changelogs, and provides extensive unit and integration tests covering the full lifecycle and API surface of vault functionality. Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
* fix AccountRoot ledger object * update HISTORY.md * fix import and lint errors * request a validated ledger
Co-authored-by: achowdhry-ripple <[email protected]>
Bumps [webpack](https://github.com/webpack/webpack) from 5.98.0 to 5.99.9. - [Release notes](https://github.com/webpack/webpack/releases) - [Commits](webpack/webpack@v5.98.0...v5.99.9) --- updated-dependencies: - dependency-name: webpack dependency-version: 5.99.9 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: achowdhry-ripple <[email protected]> Co-authored-by: Omar Khan <[email protected]>
* add conditional check for PermissionValue * update HISTORY * remove release date * add unit test
* update HISTORY * update package.json files * update package-lock.json
* refactor(tests): separate faucet tests from local integration tests * feat: add Faucet test workflow and documentation * fix: Add missing test:faucet script * fix: execute tests one time Co-authored-by: Raj Patel <[email protected]> * fix: remove docker steps from faucet test workflow * docs: update faucet tests section in CONTRIBUTING.md * fix: remove comment from contributing.md Co-authored-by: Raj Patel <[email protected]> * fix: remove test:all from root package Co-authored-by: Raj Patel <[email protected]> * Update CONTRIBUTING.md --------- Co-authored-by: Raj Patel <[email protected]>
This reverts commit 416c1e1.
khancode
force-pushed
the
single-asset-vault
branch
from
7aac152 to
320a45e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/xrpl/src/models/transactions/vaultDeposit.ts (1)
11-28: Fix the typo in JSDoc comment.The interface definition is correct, but there's a typo in the comment.
- * The VaultDeposit transaction adds liqudity in exchange for vault shares. + * The VaultDeposit transaction adds liquidity in exchange for vault shares.packages/xrpl/src/models/ledger/Vault.ts (1)
20-22: Consider usingnumbertype for Flags field.The Flags field is currently typed as
0, which is overly restrictive. Vault objects may have various flag combinations.Consider changing the type to allow for different flag values:
- Flags: 0 + Flags: numberpackages/ripple-binary-codec/src/types/st-number.ts (1)
145-145: Use class name instead ofthisin static context.Using
thisin a static method can be confusing as it refers to the class itself.- return this.fromValue(value) + return STNumber.fromValue(value)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
.ci-config/rippled.cfg(1 hunks)packages/ripple-binary-codec/HISTORY.md(1 hunks)packages/ripple-binary-codec/src/types/index.ts(2 hunks)packages/ripple-binary-codec/src/types/st-number.ts(1 hunks)packages/ripple-binary-codec/src/types/st-object.ts(1 hunks)packages/ripple-binary-codec/src/utils.ts(2 hunks)packages/ripple-binary-codec/test/st-number.test.ts(1 hunks)packages/xrpl/HISTORY.md(1 hunks)packages/xrpl/src/models/common/index.ts(1 hunks)packages/xrpl/src/models/ledger/LedgerEntry.ts(3 hunks)packages/xrpl/src/models/ledger/Vault.ts(1 hunks)packages/xrpl/src/models/ledger/index.ts(2 hunks)packages/xrpl/src/models/methods/index.ts(5 hunks)packages/xrpl/src/models/methods/vaultInfo.ts(1 hunks)packages/xrpl/src/models/transactions/clawback.ts(4 hunks)packages/xrpl/src/models/transactions/common.ts(4 hunks)packages/xrpl/src/models/transactions/index.ts(1 hunks)packages/xrpl/src/models/transactions/transaction.ts(3 hunks)packages/xrpl/src/models/transactions/vaultClawback.ts(1 hunks)packages/xrpl/src/models/transactions/vaultCreate.ts(1 hunks)packages/xrpl/src/models/transactions/vaultDelete.ts(1 hunks)packages/xrpl/src/models/transactions/vaultDeposit.ts(1 hunks)packages/xrpl/src/models/transactions/vaultSet.ts(1 hunks)packages/xrpl/src/models/transactions/vaultWithdraw.ts(1 hunks)packages/xrpl/src/models/utils/flags.ts(2 hunks)packages/xrpl/test/integration/requests/vaultInfo.test.ts(1 hunks)packages/xrpl/test/integration/transactions/singleAssetVault.test.ts(1 hunks)packages/xrpl/test/models/clawback.test.ts(2 hunks)packages/xrpl/test/models/vaultClawback.test.ts(1 hunks)packages/xrpl/test/models/vaultCreate.test.ts(1 hunks)packages/xrpl/test/models/vaultDelete.test.ts(1 hunks)packages/xrpl/test/models/vaultDeposit.test.ts(1 hunks)packages/xrpl/test/models/vaultSet.test.ts(1 hunks)packages/xrpl/test/models/vaultWithdraw.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/xrpl/test/models/clawback.test.ts (1)
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#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.
🧬 Code Graph Analysis (14)
packages/xrpl/src/models/utils/flags.ts (1)
packages/xrpl/src/models/transactions/index.ts (1)
VaultCreateFlags(109-109)
packages/ripple-binary-codec/src/types/index.ts (1)
packages/ripple-binary-codec/src/types/st-number.ts (1)
STNumber(114-232)
packages/xrpl/src/models/ledger/LedgerEntry.ts (2)
packages/xrpl/src/models/ledger/Vault.ts (1)
Vault(11-83)packages/xrpl/src/models/ledger/index.ts (1)
Vault(79-79)
packages/xrpl/src/models/transactions/vaultDelete.ts (2)
packages/xrpl/src/models/transactions/index.ts (2)
VaultDelete(113-113)BaseTransaction(1-1)packages/xrpl/src/models/transactions/common.ts (3)
validateBaseTransaction(452-513)validateRequiredField(291-317)isString(92-94)
packages/xrpl/src/models/methods/index.ts (1)
packages/xrpl/src/models/methods/vaultInfo.ts (2)
VaultInfoRequest(11-28)VaultInfoResponse(35-193)
packages/xrpl/src/models/transactions/vaultDeposit.ts (2)
packages/xrpl/src/models/transactions/index.ts (2)
VaultDeposit(114-114)BaseTransaction(1-1)packages/xrpl/src/models/transactions/common.ts (4)
validateBaseTransaction(452-513)validateRequiredField(291-317)isString(92-94)isAmount(242-248)
packages/xrpl/src/models/transactions/vaultWithdraw.ts (2)
packages/xrpl/src/models/transactions/index.ts (2)
VaultWithdraw(116-116)BaseTransaction(1-1)packages/xrpl/src/models/transactions/common.ts (6)
validateBaseTransaction(452-513)validateRequiredField(291-317)isString(92-94)isAmount(242-248)validateOptionalField(331-351)isAccount(229-234)
packages/xrpl/src/models/methods/vaultInfo.ts (1)
packages/xrpl/src/models/methods/index.ts (4)
VaultInfoRequest(660-660)BaseRequest(510-510)VaultInfoResponse(661-661)BaseResponse(511-511)
packages/xrpl/src/models/transactions/clawback.ts (2)
packages/xrpl/src/models/common/index.ts (1)
ClawbackAmount(34-34)packages/xrpl/src/models/transactions/common.ts (2)
validateRequiredField(291-317)isClawbackAmount(205-207)
packages/xrpl/src/models/ledger/Vault.ts (3)
packages/xrpl/src/models/ledger/index.ts (1)
Vault(79-79)packages/xrpl/src/models/ledger/BaseLedgerEntry.ts (2)
BaseLedgerEntry(1-3)HasPreviousTxnID(5-16)packages/ripple-binary-codec/src/types/index.ts (1)
Currency(55-55)
packages/xrpl/src/models/transactions/common.ts (2)
packages/xrpl/src/models/common/index.ts (1)
ClawbackAmount(34-34)packages/xrpl/src/models/transactions/index.ts (1)
isMPTAmount(1-1)
packages/xrpl/src/models/transactions/vaultSet.ts (3)
packages/xrpl/src/models/transactions/common.ts (8)
BaseTransaction(366-441)XRPLNumber(221-221)validateBaseTransaction(452-513)validateRequiredField(291-317)isString(92-94)validateOptionalField(331-351)isXRPLNumber(119-126)DATA_MAX_BYTE_LENGTH(26-26)packages/xrpl/src/models/utils/index.ts (1)
isHex(60-62)packages/xrpl/src/errors.ts (1)
ValidationError(156-156)
packages/xrpl/test/integration/transactions/singleAssetVault.test.ts (7)
packages/xrpl/test/integration/setup.ts (1)
XrplIntegrationTestContext(38-41)packages/xrpl/test/integration/utils.ts (2)
generateFundedWallet(182-186)testTransaction(240-290)packages/xrpl/src/models/transactions/index.ts (3)
AccountSetAsfFlags(11-11)TrustSetFlags(104-104)VaultWithdrawalPolicy(111-111)packages/xrpl/src/models/transactions/vaultCreate.ts (1)
VaultCreate(54-86)packages/xrpl/src/models/ledger/Vault.ts (1)
Vault(11-83)packages/xrpl/src/models/common/index.ts (1)
XRP(7-10)packages/xrpl/src/models/transactions/vaultClawback.ts (1)
VaultClawback(24-41)
packages/xrpl/src/models/transactions/vaultCreate.ts (4)
packages/xrpl/src/models/transactions/common.ts (11)
GlobalFlagsInterface(359-361)BaseTransaction(366-441)XRPLNumber(221-221)validateBaseTransaction(452-513)validateRequiredField(291-317)isCurrency(134-143)validateOptionalField(331-351)isString(92-94)isXRPLNumber(119-126)isNumber(102-104)DATA_MAX_BYTE_LENGTH(26-26)packages/ripple-binary-codec/src/types/index.ts (1)
Currency(55-55)packages/xrpl/src/models/utils/index.ts (2)
isHex(60-62)hasFlag(40-52)packages/xrpl/src/errors.ts (1)
ValidationError(156-156)
🪛 Biome (1.9.4)
packages/xrpl/test/models/vaultDelete.test.ts
[error] 30-30: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/ripple-binary-codec/test/st-number.test.ts
[error] 4-4: Do not shadow the global "Number" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
packages/xrpl/test/models/vaultWithdraw.test.ts
[error] 31-31: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 55-55: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/xrpl/test/models/vaultSet.test.ts
[error] 30-30: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/xrpl/test/models/vaultClawback.test.ts
[error] 36-36: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 60-60: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/xrpl/test/models/vaultCreate.test.ts
[error] 36-36: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/xrpl/test/models/vaultDeposit.test.ts
[error] 31-31: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 47-47: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/ripple-binary-codec/src/types/st-number.ts
[error] 145-145: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🔇 Additional comments (54)
.ci-config/rippled.cfg (1)
193-193: Enable SingleAssetVault amendment
TheSingleAssetVaultflag is correctly added under the 2.5.0 Amendments section to activate the new XLS-65 feature.packages/xrpl/test/models/clawback.test.ts (2)
45-49: Consistent error message for invalid Amount
The updated test now expects'Clawback: invalid field Amount', aligning with the validation utility’s standardized phrasing.
57-61: Consistent error message for invalid string Amount
Similarly, the string-typed invalid Amount test is updated to'Clawback: invalid field Amount'for consistency.packages/xrpl/src/models/ledger/index.ts (2)
34-34: Import Vault ledger entry model
TheVaultmodel is correctly imported to support the new vault ledger object type.
79-80: Export Vault in ledger models index
IncludingVaultin the export list ensures it’s available in theLedgerEntryunion and related interfaces. The placement maintains alphabetical order.packages/xrpl/HISTORY.md (1)
7-9: Document Vault feature in changelog
The Unreleased section correctly notes the addition of Single Asset Vault support (XLS-65).packages/ripple-binary-codec/src/types/index.ts (1)
10-10: LGTM! STNumber type integration is correct.The import and registration of
STNumberas the "Number" type incoreTypesproperly integrates XRPL's fixed-point number format into the binary codec. This enables serialization/deserialization of numeric fields in vault-related transactions.Also applies to: 33-33
packages/xrpl/src/models/common/index.ts (1)
34-34: LGTM! ClawbackAmount type definition is well-structured.The union type correctly combines
IssuedCurrencyAmountandMPTAmountfor vault clawback operations, providing type safety for different asset types that can be clawed back.packages/xrpl/src/models/ledger/LedgerEntry.ts (1)
21-21: LGTM! Vault ledger entry integration is comprehensive.The import, union type addition, and filter addition properly integrate the new
Vaultledger entry type. The changes follow the existing patterns and maintain consistency with other ledger entry types.Also applies to: 46-46, 75-75
packages/xrpl/src/models/utils/flags.ts (1)
23-23: LGTM! VaultCreate flag support is correctly implemented.The import and mapping addition properly integrates
VaultCreateFlagsinto the flag parsing system, enabling proper flag handling for VaultCreate transactions consistent with other transaction types.Also applies to: 67-67
packages/ripple-binary-codec/src/types/st-object.ts (1)
154-160: LGTM! Error handling improvement enhances robustness.The addition of the
associatedType?.fromcheck with a descriptive error message is excellent defensive programming. This prevents runtime errors when types are missing the requiredfrommethod and provides clear debugging information.packages/xrpl/src/models/transactions/vaultDelete.ts (3)
1-6: LGTM: Proper imports for transaction validation.The imports correctly include all necessary validation utilities from the common module.
8-20: LGTM: Well-documented interface definition.The interface properly extends BaseTransaction and includes:
- Correct TransactionType literal
- Required VaultID field with appropriate type
- Clear JSDoc documentation
22-32: LGTM: Correct validation implementation.The validation function follows established patterns:
- Validates base transaction fields first
- Validates required VaultID field with appropriate type checker
- Uses proper error handling via ValidationError
packages/xrpl/src/models/methods/index.ts (5)
192-192: LGTM: Proper import of VaultInfo types.The import correctly includes both request and response types for VaultInfo method.
251-252: LGTM: Correct integration into Request union type.VaultInfoRequest is properly added to the Request union type with appropriate grouping comment.
312-313: LGTM: Correct integration into Response union type.VaultInfoResponse is properly added to the Response union type with matching comment structure.
472-473: LGTM: Proper type mapping in RequestResponseMap.The conditional type mapping correctly maps VaultInfoRequest to VaultInfoResponse, maintaining type safety.
659-661: LGTM: Complete export of VaultInfo types.Both request and response types are properly exported with appropriate grouping comment.
packages/xrpl/test/models/vaultDelete.test.ts (5)
1-6: LGTM: Proper test imports and setup.The imports correctly include all necessary testing utilities and the VaultDelete types.
7-21: LGTM: Well-structured test setup.The test suite properly:
- Uses descriptive naming and documentation
- Sets up a valid transaction in beforeEach
- Includes all required fields for VaultDelete
23-26: LGTM: Comprehensive validation testing.The test correctly verifies that both the specific
validateVaultDeletefunction and the genericvalidatefunction accept valid transactions.
28-34: LGTM: Proper missing field validation.The test correctly verifies that missing VaultID field throws appropriate ValidationError with both validation functions.
36-42: LGTM: Proper invalid type validation.The test correctly verifies that invalid VaultID type throws appropriate ValidationError with both validation functions.
packages/xrpl/src/models/transactions/vaultDeposit.ts (2)
1-9: LGTM: Proper imports for transaction validation.The imports correctly include all necessary validation utilities, including
isAmountfor Amount field validation.
30-41: LGTM: Correct validation implementation.The validation function properly:
- Validates base transaction fields first
- Validates required VaultID field with string type checker
- Validates required Amount field with appropriate amount type checker
packages/xrpl/test/models/vaultDeposit.test.ts (5)
1-6: LGTM: Proper test imports and setup.The imports correctly include all necessary testing utilities and the VaultDeposit types.
7-22: LGTM: Well-structured test setup.The test suite properly:
- Uses descriptive naming and documentation
- Sets up a valid transaction in beforeEach
- Includes all required fields (VaultID and Amount) for VaultDeposit
24-27: LGTM: Comprehensive validation testing.The test correctly verifies that both the specific
validateVaultDepositfunction and the genericvalidatefunction accept valid transactions.
29-43: LGTM: Thorough VaultID validation testing.The tests correctly verify that:
- Missing VaultID field throws appropriate ValidationError
- Invalid VaultID type throws appropriate ValidationError
- Both validation functions are tested
45-59: LGTM: Thorough Amount validation testing.The tests correctly verify that:
- Missing Amount field throws appropriate ValidationError
- Invalid Amount type throws appropriate ValidationError
- Both validation functions are tested
packages/ripple-binary-codec/test/st-number.test.ts (1)
6-114: Excellent test coverage for STNumber serialization.The test suite comprehensively covers various numeric formats, edge cases, normalization behavior, and error handling. The tests validate roundtrip conversions, scientific notation handling, and proper error messages for invalid inputs.
packages/xrpl/test/models/vaultClawback.test.ts (2)
36-36: Delete operator usage is acceptable in test context.While static analysis flags the
deleteoperator for performance concerns, its usage here is intentional and appropriate for testing validation of missing fields. The performance impact is negligible in test scenarios.Also applies to: 60-60
12-93: Comprehensive validation test coverage.The test suite effectively validates both positive and negative scenarios for the VaultClawback transaction, ensuring proper error handling for missing or invalid required fields (VaultID, Holder) and validation of the optional Amount field.
packages/xrpl/test/models/vaultWithdraw.test.ts (2)
31-31: Delete operator usage is acceptable in test context.The
deleteoperator usage here is intentional for testing validation of missing fields. This is appropriate and necessary for comprehensive validation testing despite the static analysis warning.Also applies to: 55-55
12-94: Well-structured validation tests for VaultWithdraw.The test suite properly validates required fields (VaultID, Amount), optional fields (Destination), and includes appropriate error handling for invalid field types. The tests ensure both positive and negative validation scenarios are covered.
packages/xrpl/src/models/transactions/index.ts (1)
98-99: Well-organized export additions for Vault transactions.The reorganization of PermissionedDomain exports and addition of comprehensive Vault transaction exports is well-structured. All necessary transaction types, flags, and interfaces are properly exported to support the Single Asset Vault functionality.
Also applies to: 106-116
packages/xrpl/test/models/vaultSet.test.ts (2)
30-30: Delete operator usage is acceptable in test context.The
deleteoperator usage is intentional for testing validation of missing fields. This is appropriate for comprehensive validation testing despite the static analysis warning.
12-80: Comprehensive validation tests with excellent field-specific validation.The test suite provides thorough coverage of VaultSet transaction validation, including:
- Required field validation (VaultID)
- Optional field validation (Data, AssetsMaximum, DomainID)
- Hex string validation for Data field
- Byte length limits (256 bytes) for Data field
- Proper error handling for invalid field types
The validation logic is robust and well-tested.
packages/xrpl/test/integration/requests/vaultInfo.test.ts (1)
1-132: Excellent comprehensive integration test for vault_info API method.This integration test provides thorough coverage of the
vault_infoAPI method by:
- Creating a complete vault with all relevant parameters
- Testing both query methods (
vault_idandowner/seq)- Validating all vault properties including asset type, withdrawal policy, totals, and shares data
- Ensuring consistency between different query approaches
The test structure follows best practices with proper setup/teardown and comprehensive assertions.
packages/xrpl/src/models/transactions/vaultWithdraw.ts (1)
1-51: Clean and well-structured VaultWithdraw transaction model.The implementation follows established patterns correctly:
- Proper interface definition extending
BaseTransaction- Appropriate use of
Amounttype for flexible asset amounts- Optional
Destinationfield for withdrawal flexibility- Comprehensive validation using standard utilities
- Clear documentation with JSDoc comments
The model integrates well with the existing transaction framework.
packages/xrpl/src/models/transactions/clawback.ts (1)
2-2: Excellent refactoring to improve type consistency and validation.These changes improve the codebase by:
- Using the centralized
ClawbackAmounttype alias instead of inline union types- Leveraging
validateRequiredFieldwithisClawbackAmountfor consistent validation patterns- Maintaining the same validation behavior while improving maintainability
The refactoring aligns well with the standard validation patterns used throughout the transaction models.
Also applies to: 11-12, 31-31, 47-47
packages/xrpl/src/models/transactions/vaultClawback.ts (1)
1-56: Well-designed VaultClawback transaction model with clear semantics.The implementation demonstrates thoughtful design:
- Proper interface definition with required
VaultIDandHolderfields- Optional
Amountfield allows flexible clawback operations (including full clawback)- Comprehensive documentation explaining the conceptual behavior as a VaultWithdraw on behalf of the Holder
- Consistent validation using standard utilities
- Appropriate use of
ClawbackAmounttype for consistency with regular clawback operationsThe model effectively bridges vault and clawback functionality.
packages/xrpl/src/models/transactions/transaction.ts (1)
95-100: Complete and consistent integration of vault transaction types.The integration properly adds all six vault transaction types:
- All imports are present for transaction models and validation functions
- All types are correctly added to the
SubmittableTransactionunion- All validation cases are included in the switch statement with proper function calls
- Implementation follows the exact same pattern as existing transaction types
The integration ensures comprehensive support for the Single Asset Vault feature within the transaction framework.
Also applies to: 182-187, 491-513
packages/xrpl/src/models/transactions/vaultSet.ts (1)
1-72: LGTM! Well-structured VaultSet transaction implementation.The VaultSet transaction model is correctly implemented with:
- Proper interface definition extending BaseTransaction
- Comprehensive validation function following established patterns
- Appropriate field validation including hex string and byte length checks
- Consistent error handling and messaging
The implementation aligns well with other vault transaction models in the codebase.
packages/xrpl/test/models/vaultCreate.test.ts (1)
17-103: Excellent test coverage for VaultCreate validation.The test suite comprehensively covers:
- Required field validation
- Type checking for all fields
- Hex string validation and size limits
- Flag-dependent validation logic
- Both specific and generic validation function testing
This provides robust validation testing for the VaultCreate transaction type.
packages/xrpl/src/models/methods/vaultInfo.ts (1)
1-194: Comprehensive vault_info method implementation.The vault_info method interfaces are well-designed with:
- Clear request parameters allowing flexible vault querying
- Comprehensive response structure capturing all vault details
- Proper TypeScript documentation and type safety
- Consistent with existing XRPL API method patterns
The nested shares object structure provides complete information about vault share issuance.
packages/ripple-binary-codec/src/utils.ts (1)
53-137: Solid implementation of signed integer utilities.The new signed integer read/write functions are well-implemented with:
- Proper use of DataView for correct byte order handling
- Consistent function signatures with existing utilities
- Clear JSDoc documentation
- Support for both 32-bit and 64-bit signed integers
These utilities provide the foundation for STNumber type serialization in the Single Asset Vault feature.
packages/xrpl/src/models/ledger/Vault.ts (1)
11-83: Well-designed Vault ledger entry interface.The Vault interface comprehensively defines the ledger entry structure with:
- Proper extension of base interfaces
- All necessary vault properties with appropriate types
- Clear documentation for each field
- Correct handling of optional fields like AssetsMaximum and Data
This provides a solid foundation for the Single Asset Vault feature in the XRPL ecosystem.
packages/xrpl/src/models/transactions/common.ts (1)
10-10: LGTM! Well-structured additions for Vault support.The new constants, type guards, and type aliases are properly implemented:
DATA_MAX_BYTE_LENGTHconstant is appropriately sized for metadata fieldsisXRPLNumberregex pattern comprehensively handles integer, decimal, and scientific notation formatsisClawbackAmountcorrectly delegates to existing type guards- Clear documentation for all additions
Also applies to: 25-27, 106-126, 196-207, 214-222
packages/xrpl/test/integration/transactions/singleAssetVault.test.ts (1)
46-287: Well-structured integration test covering the complete vault lifecycle.The test comprehensively validates:
- Vault creation with metadata and policies
- Vault updates via VaultSet
- Deposit and withdrawal operations
- Clawback functionality
- Vault deletion
Good practices observed:
- Proper wallet setup and teardown
- State verification after each operation
- Use of BigInt for precise arithmetic calculations
- Clear test structure with descriptive operations
packages/xrpl/src/models/transactions/vaultCreate.ts (2)
88-142: Excellent validation implementation with proper bounds checking.The validation function thoroughly checks:
- All field types and requirements
- Hex string format validation for Data and MPTokenMetadata
- Byte length limits with clear error messages
- Business rule enforcement (DomainID requires tfVaultPrivate flag)
The modular use of validation helpers and descriptive error messages will help developers quickly identify issues.
24-26: Verify the correct enum value for vaultStrategyFirstComeFirstServe.The enum defines
vaultStrategyFirstComeFirstServe = 0x0001, but the integration test and the index.ts snippet show it as0. This inconsistency needs to be resolved.#!/bin/bash # Description: Check for VaultWithdrawalPolicy enum definitions and usage # Search for VaultWithdrawalPolicy enum definitions echo "=== Searching for VaultWithdrawalPolicy definitions ===" ast-grep --pattern 'enum VaultWithdrawalPolicy { $$$ }' # Search for usage of vaultStrategyFirstComeFirstServe echo -e "\n=== Searching for vaultStrategyFirstComeFirstServe usage ===" rg -A 2 "vaultStrategyFirstComeFirstServe"packages/ripple-binary-codec/src/types/st-number.ts (1)
1-232: Robust implementation of XRPL Number type with excellent edge case handling.The implementation demonstrates:
- Comprehensive number parsing supporting integer, decimal, and scientific notation
- Proper normalization to XRPL constraints with clear bounds checking
- Canonical zero handling following XRPL conventions
- Well-documented regex pattern with detailed explanation
- Appropriate error messages for invalid inputs
The serialization/deserialization logic correctly handles signed integers in big-endian format.
packages/xrpl/test/integration/transactions/singleAssetVault.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ripple-binary-codec/src/types/st-number.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/ripple-binary-codec/src/types/st-number.ts (2)
packages/ripple-binary-codec/src/types/serialized-type.ts (1)
SerializedType(122-122)packages/ripple-binary-codec/src/utils.ts (4)
writeInt64BE(79-89)writeInt32BE(60-70)readInt64BE(131-137)readInt32BE(116-122)
🪛 Biome (1.9.4)
packages/ripple-binary-codec/src/types/st-number.ts
[error] 145-145: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: integration (22.x)
- GitHub Check: browser (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: unit (22.x)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
packages/ripple-binary-codec/src/types/st-number.ts (6)
1-14: LGTM! Clean imports and well-defined constants.The imports are appropriate and the constants are well-documented with clear constraints for XRPL Number specification compliance.
27-69: Robust number parsing implementation.The regex pattern and parsing logic correctly handle integer, decimal, and scientific notation formats. The leading zero normalization and component extraction are implemented correctly.
82-101: Solid normalization logic with proper bounds checking.The function correctly normalizes mantissa and exponent to fit XRPL constraints, with appropriate error handling for out-of-range values.
114-233: Well-structured STNumber class implementation.The class correctly extends SerializedType and provides comprehensive functionality for XRPL Number serialization/deserialization. The constructor validation, static factory methods, and JSON conversion are well-implemented with appropriate error handling.
213-232: ```shell
#!/bin/bashLocate the decimal rendering block in st-number.ts
echo "Finding exponent handling in st-number.ts..."
rg -n "exponent < -25" -t ts packages/ripple-binary-codec/src/types/st-number.tsecho "Printing surrounding lines for context..."
sed -n '180,260p' packages/ripple-binary-codec/src/types/st-number.ts--- `165-171`: ```shell #!/bin/bash file="packages/ripple-binary-codec/src/types/st-number.ts" echo "=== extractNumberPartsFromString definition and usage ===" grep -R "extractNumberPartsFromString" -n "$file" -A10 -B5
| return value | ||
| } | ||
| if (typeof value === 'string') { | ||
| return this.fromValue(value) |
There was a problem hiding this comment.
Fix static method context issue.
The static analysis tool correctly identifies that using this in a static context is confusing and should be replaced with the class name.
- if (typeof value === 'string') {
- return this.fromValue(value)
- }
+ if (typeof value === 'string') {
+ return STNumber.fromValue(value)
+ }📝 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.
| return this.fromValue(value) | |
| if (typeof value === 'string') { | |
| return STNumber.fromValue(value) | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 145-145: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🤖 Prompt for AI Agents
In packages/ripple-binary-codec/src/types/st-number.ts at line 145, replace the
use of 'this' in the static method with the actual class name to avoid confusion
and clarify the static context. Change 'this.fromValue(value)' to
'StNumber.fromValue(value)' or the appropriate class name to fix the static
method context issue.
There was a problem hiding this comment.
@khancode this suggestion appears to be be correct
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| return value | ||
| } | ||
| if (typeof value === 'string') { | ||
| return this.fromValue(value) |
There was a problem hiding this comment.
@khancode this suggestion appears to be be correct
| @@ -151,7 +151,13 @@ class STObject extends SerializedType { | |||
| ? STArray.from(xAddressDecoded[field.name], definitions) | |||
| : field.type.name === 'UInt64' | |||
| ? UInt64.from(xAddressDecoded[field.name], field.name) | |||
There was a problem hiding this comment.
: field.type.name === 'STNumber'
? STNumber.from(xAddressDecoded[field.name], field.name)
Don't we need this type of if-else condition? Is this case being handled elsewhere?
| @@ -4,6 +4,10 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| ### Added | |||
| * Support for `Single Asset Vault` (XLS-65) | |||
| * Add new amount types: `XRPLNumber` and `ClawbackAmount` | |||
There was a problem hiding this comment.
| * Add new amount types: `XRPLNumber` and `ClawbackAmount` | |
| * Add new amount types: `STNumber` and `ClawbackAmount` |
Since ClawbackAmount is an internal type variable, does it need to be declared in the HISTORY.md file? It is difficult to understand this Changelog line without digging through the codebase.
| @@ -21,6 +22,9 @@ export const MAX_AUTHORIZED_CREDENTIALS = 8 | |||
| const MAX_CREDENTIAL_BYTE_LENGTH = 64 | |||
| const MAX_CREDENTIAL_TYPE_LENGTH = MAX_CREDENTIAL_BYTE_LENGTH * 2 | |||
|
|
|||
| // Used for Vault transactions | |||
| export const DATA_MAX_BYTE_LENGTH = 256 | |||
There was a problem hiding this comment.
Can we instead use a variable name that indicates that this is associated with Vaults? That will help with readability. For ex: VAULT_DATA_MAX_LEN
| assert.equal( | ||
| vault.ShareMPTID, | ||
| vault.shares.mpt_issuance_id, | ||
| 'ShareMPTID should match mpt_issuance_id', | ||
| ) |
There was a problem hiding this comment.
The currency of the asset in the Vault is XRP. However, this assert refers to MPT. Can you explain this line? Are the shares of a Vault necessarily MPT tokens?
| @@ -0,0 +1,114 @@ | |||
| import { BinaryParser } from '../src/binary' | |||
There was a problem hiding this comment.
I think it's good to have a VaultCreate transaction added in packages/ripple-binary-codec/test/fixtures/codec-fixtures.json to get the encode and decode tested.
High Level Overview of Change
Adds support for Single Asset Vault (XLS-65) and
STNumbertype for serialization.Context of Change
rippled PR: XRPLF/rippled#5224
rippled
STNumbertype PR: https://github.com/XRPLF/rippled/pull/5121/filesNote: Vault definitions were already merged in so no changes needed in
definitions.jsonType of Change
Did you update HISTORY.md?
Test Plan
Adds unit/integ tests.