Introduce MPT V1 support (XLS-33d) #5108
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shawnxie999
changed the title
New Transactions: - MPTokenIssuanceCreate - MPTokenIssuanceDestory - MPTokenIssuanceSet - MPTokenAuthorize Modified Transactions: - Payment - Clawback New Objects: - MPTokenIssuance - MPTokenAuthorize API updates: - ledger_entry - account_objects - ledger_data Read full spec: https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0033d-multi-purpose-tokens --------- Co-authored-by: Shawn Xie <shawnxie920@gmail.com> Co-authored-by: Howard Hinnant <howard.hinnant@gmail.com>
gregtatcam
force-pushed
the
feature/mpt-v1
branch
from
69f2027
to
57c21a6
Compare
| r -= 1; | ||
| } | ||
| if (r > std::numeric_limits<MPTAmount::value_type>::max()) | ||
| Throw<std::overflow_error>("XRP mulRatio overflow"); |
There was a problem hiding this comment.
Nitpick: Overflow message should be MPT or MPTAmount instead of XRP.
| std::size_t constexpr maxMPTokenMetadataLength = 1024; | ||
|
|
||
| /** The maximum amount of MPTokenIssuance */ | ||
| std::uint64_t constexpr maxMPTokenAmount = 0x7FFFFFFFFFFFFFFFull; |
There was a problem hiding this comment.
Stylistic: I had to put in ' every four digits to convince myself that there was the proper number of Fs.
std::uint64_t constexpr maxMPTokenAmount = 0x7FFF'FFFF'FFFF'FFFFull;
|
List of warnings flagged by clang. None of these are caused by this commit. The first warning has already made its way onto develop. The rest are not yet on develop. |
| STAmount{ | ||
| std::get<Issue>(issue), mantissa, exponent, native, negative}}; | ||
| while (exponent-- > 0) | ||
| mantissa *= 10; |
There was a problem hiding this comment.
What if mantissa > maxMPTokenAmount prior to entering this loop, or becomes so after the *= 10 but before exponent == 0?
Unless there is other logic to prevent this I think we need to guard against mantissa *= 10 overflowing the uint64_t. This is only guaranteed if mantissa <= maxMPTokenAmount prior to the *= 10.
| } | ||
|
|
||
| while (exponent-- > 0) | ||
| mantissa *= 10; |
There was a problem hiding this comment.
Do we need overflow checks here?
* add issuance not found code * add issuance check to start of func * uses `exists()`
* fix maximum amt * add if check to return tecMPT_ISSUANCE_NOT_FOUND
* MPT amount base 10 * add assert * revert formatting * test * add test for maxamt * clang * parse as base 10 for all mpt amounts
|
Replaced with #5143 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
New Transactions:
Modified Transactions:
New Objects:
API updates:
Refactor:
replaces STAmount as a serializable type for amount fields.
Context of Change
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)Test Plan
Added test for new feature:
Future Tasks
Integrate MPT into XRPL DEX.