-
Notifications
You must be signed in to change notification settings - Fork 520
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(xrpl): custom definitions support #2683
base: main
Are you sure you want to change the base?
Changes from all commits
23411ac
9249824
b529149
a618fb9
b283fc3
4689b02
59c2074
6089035
7021e63
02a8503
3153659
b047f04
eeb9f43
2900ab3
2c60a1e
071beb5
04446ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ Functions to encode/decode to/from the ripple [binary serialization format](http | |
``` | ||
|
||
|
||
### decode(binary: string): object | ||
### decode(binary: string, definitions?: XrplDefinitionsBase): object | ||
Decode a hex-string into a transaction object. | ||
```js | ||
> api.decode('1100612200000000240000000125000000072D0000000055DF530FB14C5304852F20080B0A8EEF3A6BDD044F41F4EBBD68B8B321145FE4FF6240000002540BE4008114D0F5430B66E06498D4CEEC816C7B3337F9982337') | ||
|
@@ -26,7 +26,7 @@ Decode a hex-string into a transaction object. | |
} | ||
``` | ||
|
||
### encode(json: object): string | ||
### encode(json: object, definitions?: XrplDefinitionsBase): string | ||
Encode a transaction object into a hex-string. Note that encode filters out fields with undefined values. | ||
```js | ||
> api.encode({ | ||
|
@@ -37,12 +37,12 @@ Encode a transaction object into a hex-string. Note that encode filters out fiel | |
OwnerCount: 0, | ||
PreviousTxnID: 'DF530FB14C5304852F20080B0A8EEF3A6BDD044F41F4EBBD68B8B321145FE4FF', | ||
Balance: '10000000000', | ||
Account: 'rLs1MzkFWCxTbuAHgjeTZK4fcCDDnf2KRv' | ||
Account: 'rLs1MzkFWCxTbuAHgjeTZK4fcCDDnf2KRv' | ||
}) | ||
'1100612200000000240000000125000000072D0000000055DF530FB14C5304852F20080B0A8EEF3A6BDD044F41F4EBBD68B8B321145FE4FF6240000002540BE4008114D0F5430B66E06498D4CEEC816C7B3337F9982337' | ||
``` | ||
|
||
#### X-Address Compatibility | ||
#### X-Address Compatibility | ||
* ripple-binary-codec handles X-addresses by looking for a few specific files (Account/SourceTag, Destination/DestinationTag). | ||
* If other fields (in the future) must to support X-addresses with tags, this library will need to be updated. | ||
* When decoding rippled binary, the output will always output classic address + tag, with no X-addresses. X-address support only applies when encoding to binary. | ||
|
@@ -54,15 +54,15 @@ Encode a transaction object into a hex-string. Note that encode filters out fiel | |
* When _decoding_, if a currency code is three uppercase letters or numbers (`/^[A-Z0-9]{3}$/`), then it will be decoded into that string. For example,`0000000000000000000000004142430000000000` decodes as `ABC`. | ||
* When decoding, if a currency code is does not match the regex, then it is not considered to be an ISO 4217 or pseudo-ISO currency. ripple-binary-codec will return a 160-bit hex-string (40 hex characters). For example, `0000000000000000000000006142430000000000` (`aBC`) decodes as `0000000000000000000000006142430000000000` because it contains a lowercase letter. | ||
|
||
### encodeForSigning(json: object): string | ||
### encodeForSigning(json: object, definitions?: XrplDefinitionsBase): string | ||
|
||
Encode the transaction object for signing. | ||
|
||
### encodeForSigningClaim(json: object): string | ||
|
||
Encode the transaction object for payment channel claim. | ||
|
||
### encodeForMultisigning(json: object, signer: string): string | ||
### encodeForMultisigning(json: object, signer: string, definitions?: XrplDefinitionsBase): string | ||
|
||
Encode the transaction object for multi-signing. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello,
Aren't custom definitions useful for these methods? |
||
|
||
|
@@ -72,7 +72,7 @@ Encode the transaction object for multi-signing. | |
'5D06F4C3362FE1D0' | ||
``` | ||
|
||
### decodeQuality(value: string): string | ||
### decodeQuality(value: string): string | ||
```js | ||
> api.decodeQuality('5D06F4C3362FE1D0') | ||
'195796912.5171664' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr | |
|
||
### Added | ||
* parseTransactionFlags as a utility function in the xrpl package to streamline transactions flags-to-map conversion | ||
* Adds support for Custom Definitions to `client.submit()` and `client.submitAndWait()` | ||
|
||
## 4.0.0 (2024-07-15) | ||
|
||
|
@@ -38,6 +39,9 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr | |
* Add missing `lsfAMMNode` flag to `RippleState` ledger object | ||
* Add `PreviousFields` to `DeletedNode` metadata type | ||
|
||
### Added | ||
elmurci marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* Custom definitions support for `util.encode`, `util.decode`, `util.encodeForSignning` and `Wallet.sign`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo in |
||
|
||
## 3.0.0 (2024-02-01) | ||
|
||
### BREAKING CHANGES | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,6 +2,7 @@ | |||||||||||||||||
|
||||||||||||||||||
/* eslint-disable max-lines -- Client is a large file w/ lots of imports/exports */ | ||||||||||||||||||
import { EventEmitter } from 'eventemitter3' | ||||||||||||||||||
import { XrplDefinitionsBase } from 'ripple-binary-codec' | ||||||||||||||||||
|
||||||||||||||||||
import { | ||||||||||||||||||
RippledError, | ||||||||||||||||||
|
@@ -218,6 +219,12 @@ class Client extends EventEmitter<EventTypes> { | |||||||||||||||||
*/ | ||||||||||||||||||
public buildVersion: string | undefined | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Custom rippled types to use instead of the default. Used for sidechains and amendments. | ||||||||||||||||||
* | ||||||||||||||||||
*/ | ||||||||||||||||||
public definitions: XrplDefinitionsBase | undefined | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* API Version used by the server this client is connected to | ||||||||||||||||||
* | ||||||||||||||||||
|
@@ -526,6 +533,33 @@ class Client extends EventEmitter<EventTypes> { | |||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Get Definitions from server_definitions | ||||||||||||||||||
* | ||||||||||||||||||
* @returns void | ||||||||||||||||||
* @example | ||||||||||||||||||
* ```ts | ||||||||||||||||||
* const { Client } = require('xrpl') | ||||||||||||||||||
* const client = new Client('wss://s.altnet.rippletest.net:51233') | ||||||||||||||||||
* await client.getDefinitions() | ||||||||||||||||||
* console.log(client.definitions) | ||||||||||||||||||
* ``` | ||||||||||||||||||
*/ | ||||||||||||||||||
public async getDefinitions(): Promise<void> { | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello, where else is this method used? I couldn't find invocations of this method in this PR. |
||||||||||||||||||
try { | ||||||||||||||||||
const response = await this.request({ | ||||||||||||||||||
command: 'server_definitions', | ||||||||||||||||||
}) | ||||||||||||||||||
this.definitions = new XrplDefinitionsBase( | ||||||||||||||||||
JSON.parse(JSON.stringify(response.result)), | ||||||||||||||||||
{}, | ||||||||||||||||||
) | ||||||||||||||||||
} catch (error) { | ||||||||||||||||||
// eslint-disable-next-line no-console -- Print the error to console but allows client to be connected. | ||||||||||||||||||
console.error(error) | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
Comment on lines
+548
to
+561
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve error handling in Currently, errors in
|
||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Tells the Client instance to connect to its rippled server. | ||||||||||||||||||
* | ||||||||||||||||||
|
@@ -760,7 +794,10 @@ class Client extends EventEmitter<EventTypes> { | |||||||||||||||||
wallet?: Wallet | ||||||||||||||||||
}, | ||||||||||||||||||
): Promise<SubmitResponse> { | ||||||||||||||||||
const signedTx = await getSignedTx(this, transaction, opts) | ||||||||||||||||||
const signedTx = await getSignedTx(this, transaction, { | ||||||||||||||||||
...opts, | ||||||||||||||||||
definitions: this.definitions, | ||||||||||||||||||
}) | ||||||||||||||||||
Comment on lines
+797
to
+800
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid overwriting user-provided definitions in By spreading const signedTx = await getSignedTx(this, transaction, {
...opts,
- definitions: this.definitions,
+ definitions: opts?.definitions ?? this.definitions,
}) This change ensures that if the user provides 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
return submitRequest(this, signedTx, opts?.failHard) | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -834,7 +871,10 @@ class Client extends EventEmitter<EventTypes> { | |||||||||||||||||
wallet?: Wallet | ||||||||||||||||||
}, | ||||||||||||||||||
): Promise<TxResponse<T>> { | ||||||||||||||||||
const signedTx = await getSignedTx(this, transaction, opts) | ||||||||||||||||||
const signedTx = await getSignedTx(this, transaction, { | ||||||||||||||||||
...opts, | ||||||||||||||||||
definitions: this.definitions, | ||||||||||||||||||
}) | ||||||||||||||||||
Comment on lines
+874
to
+877
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consistent handling of definitions in Similar to the const signedTx = await getSignedTx(this, transaction, {
...opts,
- definitions: this.definitions,
+ definitions: opts?.definitions ?? this.definitions,
}) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
const lastLedger = getLastLedgerSequence(signedTx) | ||||||||||||||||||
if (lastLedger == null) { | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { isValidClassicAddress, isValidXAddress } from 'ripple-address-codec' | ||
import { TRANSACTION_TYPES } from 'ripple-binary-codec' | ||
import { TRANSACTION_TYPES, XrplDefinitionsBase } from 'ripple-binary-codec' | ||
|
||
import { ValidationError } from '../../errors' | ||
import { | ||
|
@@ -351,6 +351,30 @@ export function validateBaseTransaction(common: Record<string, unknown>): void { | |
validateOptionalField(common, 'NetworkID', isNumber) | ||
} | ||
|
||
/** | ||
* Validate that the passed transaction is a valid type against the types provided by the custom definitions. | ||
* | ||
* @param tx - A Transaction. | ||
* @param definitions - Custom definitions | ||
* @throws When the passed transaction type is not found in the definitions. | ||
*/ | ||
export function validateTxAgainstCustomDefinitions( | ||
tx: Record<string, unknown>, | ||
definitions: XrplDefinitionsBase, | ||
): void { | ||
// Validate just transaction type for now, leaving it open for further validations against the custom definition spec. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you explain what else is remaining as far as validation is concerned? Are you referring to validation of the |
||
const txType = tx.TransactionType | ||
if (typeof txType !== 'string') { | ||
throw new ValidationError( | ||
'TransactionType field is not specified or not a string', | ||
) | ||
} | ||
|
||
if (!definitions.transactionType[txType]) { | ||
throw new ValidationError(`Invalid transaction type: ${txType}`) | ||
} | ||
} | ||
|
||
/** | ||
* Parse the value of an amount, expressed either in XRP or as an Issued Currency, into a number. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
/* eslint-disable max-lines -- need to work with a lot of transactions in a switch statement */ | ||
/* eslint-disable max-lines-per-function -- need to work with a lot of Tx verifications */ | ||
|
||
import { XrplDefinitionsBase } from 'ripple-binary-codec' | ||
|
||
import { ValidationError } from '../../errors' | ||
import { IssuedCurrencyAmount, Memo } from '../common' | ||
import { isHex } from '../utils' | ||
|
@@ -18,7 +20,11 @@ import { CheckCancel, validateCheckCancel } from './checkCancel' | |
import { CheckCash, validateCheckCash } from './checkCash' | ||
import { CheckCreate, validateCheckCreate } from './checkCreate' | ||
import { Clawback, validateClawback } from './clawback' | ||
import { BaseTransaction, isIssuedCurrency } from './common' | ||
import { | ||
BaseTransaction, | ||
isIssuedCurrency, | ||
validateTxAgainstCustomDefinitions, | ||
} from './common' | ||
import { DepositPreauth, validateDepositPreauth } from './depositPreauth' | ||
import { DIDDelete, validateDIDDelete } from './DIDDelete' | ||
import { DIDSet, validateDIDSet } from './DIDSet' | ||
|
@@ -170,10 +176,14 @@ export interface TransactionAndMetadata< | |
* Encode/decode and individual type validation. | ||
* | ||
* @param transaction - A Transaction. | ||
* @param customDefinitions - Optional parameter to validate against a custom definition. | ||
* @throws ValidationError When the Transaction is malformed. | ||
* @category Utilities | ||
*/ | ||
export function validate(transaction: Record<string, unknown>): void { | ||
export function validate( | ||
transaction: Record<string, unknown>, | ||
customDefinitions?: XrplDefinitionsBase, | ||
): void { | ||
const tx = { ...transaction } | ||
if (tx.TransactionType == null) { | ||
throw new ValidationError('Object does not have a `TransactionType`') | ||
|
@@ -407,8 +417,12 @@ export function validate(transaction: Record<string, unknown>): void { | |
break | ||
|
||
default: | ||
throw new ValidationError( | ||
`Invalid field TransactionType: ${tx.TransactionType}`, | ||
) | ||
if (customDefinitions) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I happen to modify the behavior of an existing transaction (say The existing Is my understanding correct ? |
||
validateTxAgainstCustomDefinitions(tx, customDefinitions) | ||
} else { | ||
throw new ValidationError( | ||
elmurci marked this conversation as resolved.
Show resolved
Hide resolved
|
||
`Invalid field TransactionType: ${tx.TransactionType}`, | ||
) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Documentation needed for the new
definitions
parameterThe new
definitions
parameter has been added to multiple methods, but there's no explanation of:XrplDefinitionsBase
objectConsider adding a new section that explains:
Also applies to: 29-29, 57-57, 65-65