-
Notifications
You must be signed in to change notification settings - Fork 500
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
Add definitions.json changes to xrpl package #2214
base: main
Are you sure you want to change the base?
Conversation
const tx = typeof transaction === 'string' ? decode(transaction) : transaction | ||
function isSigned<T extends BaseTransaction = Transaction>( | ||
transaction: T | string, | ||
definitions?: InstanceType<typeof XrplDefinitionsBase>, |
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.
Should not be optional since this directly uses decode, so we should specify our default one layer above the binary codec across the board.
transaction: Transaction | string, | ||
function getLastLedgerSequence<T extends BaseTransaction = Transaction>( | ||
transaction: T | string, | ||
definitions?: InstanceType<typeof XrplDefinitionsBase>, |
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.
Same for this one.
const tx = typeof transaction === 'string' ? decode(transaction) : transaction | ||
function isAccountDelete<T extends BaseTransaction = Transaction>( | ||
transaction: T | string, | ||
definitions?: InstanceType<typeof XrplDefinitionsBase>, |
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.
And this one
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.
Nevermind (and for all above) - I think it makes the most sense to just have encode
and decode
be the only functions with default value of DEFAULT_DEFINITIONS
.
High Level Overview of Change
The user interface for defining custom definitions needs more work, so I split up #2127 to focus on the binary-codec, with this one focusing on the interface.
Context of Change
With sidechains and many amendments being worked on, we need a better way to define which types & transactions we're able to serialize/deserialize.
Type of Change
Before / After
Test Plan
CI Passes
Future Tasks
server_definitions