-
Notifications
You must be signed in to change notification settings - Fork 523
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
additional amm flags are introduced #2667
Conversation
tfOneAssetWithdrawAll = 0x00040000, | ||
tfTwoAssetIfEmpty = 0x00800000, | ||
tfWithdrawSubTx = tfLPToken | | ||
tfSingleAsset | |
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.
Can I not use existing flags to designate new flags? I get the following lint error ://
error Explicit enum value must only be a literal value (string, number, boolean, etc)
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.
There shouldn't be any masks here - the enum should just be the flags.
I would use https://xrpl.org/docs/references/protocol/transactions/types/ammdeposit/#ammdeposit-flags as your source of truth instead of rippled.
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.
It's the dev's job to |
the flags together, not ours. rippled
reuses a lot of these combinations, which is why that code has some combinations. They aren't independent flags, though.
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.
ok thanks, fixed it
This should probably be released before AMM goes live tomorrow |
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.
Please update the changelog
Co-authored-by: Mayukha Vadari <[email protected]>
* additional amm flags are introduced Co-authored-by: Mayukha Vadari <[email protected]> --------- Co-authored-by: Mayukha Vadari <[email protected]>
* additional amm flags are introduced Co-authored-by: Mayukha Vadari <[email protected]> --------- Co-authored-by: Mayukha Vadari <[email protected]>
High Level Overview of Change
Fix #2666
This PR introduces additional flags used in the AMM-related transactions. I'm using these lines of code as the source of truth: https://github.com/XRPLF/rippled/blob/69143d71f8973e33b701d7becc19da2ad6a68b68/src/ripple/protocol/TxFlags.h#L166
Context of Change
This change brings forth certain AMMFlags into the xrpl-js client library.
Type of Change
Did you update HISTORY.md?
Do I need to update HISTORY file? this change implements what was promised in the AMM-transaction documentation.
Test Plan
I'm not familiar with the AMM-transactions to write unit tests for all these flags. Please let me know if you have any ideas.