-
Notifications
You must be signed in to change notification settings - Fork 575
feat(xrpl): custom definitions support #3229
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
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
2452093
4731f60
5db85fe
006a3cb
62a765d
0373902
137f82e
6ae5337
3168faa
caa5fdf
0360c4e
b5aff79
777a8e2
7f5b146
fd0465e
f579a9f
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 |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ | |
|
|
||
| /* eslint-disable max-lines -- Client is a large file w/ lots of imports/exports */ | ||
| import { EventEmitter } from 'eventemitter3' | ||
| import { XrplDefinitions, XrplDefinitionsBase } from 'ripple-binary-codec' | ||
| import type { DefinitionsData } from 'ripple-binary-codec/dist/enums/xrpl-definitions-base' | ||
|
|
||
| import { | ||
| RippledError, | ||
|
|
@@ -225,6 +227,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 | ||
| * | ||
|
|
@@ -642,6 +650,14 @@ class Client extends EventEmitter<EventTypes> { | |
| return this.connection.isConnected() | ||
| } | ||
|
|
||
| public async getDefinitions(): Promise<void> { | ||
| const response = await this.request({ | ||
| command: 'server_definitions', | ||
| }) | ||
| // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- response.result is the DefinitionsData | ||
| this.definitions = new XrplDefinitions(response.result as DefinitionsData) | ||
| } | ||
|
|
||
| /** | ||
| * Autofills fields in a transaction. This will set `Sequence`, `Fee`, | ||
| * `lastLedgerSequence` according to the current state of the server this Client | ||
|
|
@@ -801,8 +817,11 @@ class Client extends EventEmitter<EventTypes> { | |
| wallet?: Wallet | ||
| }, | ||
| ): Promise<SubmitResponse> { | ||
| const signedTx = await getSignedTx(this, transaction, opts) | ||
| return submitRequest(this, signedTx, opts?.failHard) | ||
| const signedTx = await getSignedTx(this, transaction, { | ||
| ...opts, | ||
| definitions: this.definitions, | ||
| }) | ||
| return submitRequest(this, signedTx, opts?.failHard, this.definitions) | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -875,7 +894,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, | ||
|
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. 🟡 Severity: MEDIUM
💡 Fix SuggestionSuggestion: Pass
This ensures that custom definitions are used for the |
||
| }) | ||
|
|
||
| const lastLedger = getLastLedgerSequence(signedTx) | ||
| if (lastLedger == null) { | ||
|
|
||
| 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 { convertTxFlagsToNumber } from '../utils/flags' | ||
|
|
||
|
|
@@ -252,14 +254,18 @@ 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 { | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| const tx = { ...transaction } | ||
|
|
||
| // should already be done in the tx-specific validation, but doesn't hurt to check again | ||
| validateBaseTransaction(tx) | ||
| validateBaseTransaction(tx, customDefinitions) | ||
|
|
||
| Object.keys(tx).forEach((key) => { | ||
| const standard_currency_code_len = 3 | ||
|
|
@@ -325,8 +331,11 @@ export function validate(transaction: Record<string, unknown>): void { | |
| // @ts-expect-error -- already checked | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-call -- already checked above | ||
| tx.RawTransactions.forEach((innerTx: Record<string, unknown>) => { | ||
| // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- already checked above | ||
| validate(innerTx.RawTransaction as Record<string, unknown>) | ||
| validate( | ||
| // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- already checked above | ||
| innerTx.RawTransaction as Record<string, unknown>, | ||
| customDefinitions, | ||
| ) | ||
| }) | ||
| break | ||
|
|
||
|
|
@@ -575,8 +584,10 @@ export function validate(transaction: Record<string, unknown>): void { | |
| break | ||
|
|
||
| default: | ||
| throw new ValidationError( | ||
| `Invalid field TransactionType: ${tx.TransactionType}`, | ||
| ) | ||
| if (!customDefinitions?.transactionNames.includes(tx.TransactionType)) { | ||
|
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. Custom transaction types skip all field-level validation — only |
||
| throw new ValidationError( | ||
| `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 | 🟠 Major
Client.definitionsis never wired intoClientOptions.The new public field is declared but there is no way to set it at construction — users must mutate
client.definitions = ...afternew Client(...). Consider exposing it throughClientOptions(e.g.,options.definitions) and assigning it in the constructor, mirroringfeeCushion/maxFeeXRP. This also makes the behavior documentable and consistent with how the property is threaded intosubmit/submitAndWait.Suggested change
export interface ClientOptions extends ConnectionUserOptions { ... timeout?: number + /** + * Custom rippled transaction type definitions for sidechains/amendments. + */ + definitions?: XrplDefinitionsBase }public definitions: XrplDefinitionsBase | undefined ... public constructor(server: string, options: ClientOptions = {}) { ... this.feeCushion = options.feeCushion ?? DEFAULT_FEE_CUSHION this.maxFeeXRP = options.maxFeeXRP ?? DEFAULT_MAX_FEE_XRP + this.definitions = options.definitions🤖 Prompt for AI Agents