-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Introduce MPT support (XLS-33d): #5143
Merged
+7,084
−1,057
Merged
Changes from 12 commits
Commits
Show all changes
61 commits
Select commit
Hold shift + click to select a range
6d6fda2
Introduce MPT support (XLS-33d):
gregtatcam aaf7fe5
Merge branch 'develop' into feature/mpt-v1-var-issues
gregtatcam f84e9ea
Apply suggestions from code review
gregtatcam 18515dd
Address reviewer's feedback
gregtatcam aef1426
Allow MPT SendMax in Payment tx.
gregtatcam 9136a89
Apply suggestions from code review
gregtatcam 16a029a
Address reviewer's feedback
gregtatcam 2beb2d9
Manually align jss and disable clang-format
gregtatcam 3cc7003
Apply suggestions from code review
gregtatcam 069273c
Fix clang format
gregtatcam 86bcebd
Apply suggestions from code review
gregtatcam b9cb9ca
Address reviewer's feedback
gregtatcam 8af452e
remove unneeded fields (#36)
shawnxie999 fab0c78
rename codes (#37)
shawnxie999 ea887bf
Address reviewer feedback
gregtatcam d9fc97f
Check Payment's sendMax and amount have the same issue
gregtatcam 98d419f
Apply suggestions from code review
gregtatcam 3b7861d
Address reviewer's feedback
gregtatcam 93c545d
Fix static_assert in STVar and other changes
gregtatcam aacb1c8
comment (#38)
shawnxie999 560cf91
Fix SendMax with no transfer fee
gregtatcam 6a7a8c2
Combine Issue/MPTIssue handling in Payment transactor plus other changes
gregtatcam 8796641
Fix non-unity build
gregtatcam 757b55a
Update src/libxrpl/protocol/TxFormats.cpp
gregtatcam 11aa41c
New SField flag to override hex input/output formatting to use decima…
ximinez eac438c
Change maxAmount in the test to uint64 for better visualization
gregtatcam 9d51389
Refactor Clawback plus other minor refactoring
gregtatcam 8774b90
Fix requireAuth (#40)
shawnxie999 4bcf463
Address reviewer's feedback
gregtatcam 9b8564e
Fix metadata and add metadata unit-test
gregtatcam ac45342
Address reviewer's feedback:
gregtatcam af29f41
use mptJson
shawnxie999 9e3cfef
Add += and -= operators to ValueProxy
gregtatcam 4bbf613
ledger_entry test (#41)
shawnxie999 3bb44c2
Simplify constraint for +=, -= of ValueProxy
gregtatcam 0bf5b42
Address reviewer's feedback on mpt uni-test helper
gregtatcam 924dd9c
refactor mpt_issuance_id into SLE::getJson (#42)
shawnxie999 31b3be3
Merge branch 'develop' into feature/mpt-v1-var-issues
gregtatcam c3f2bd4
Fix clang-format
gregtatcam 29c6f5d
Merge with the latest develop
gregtatcam bd97cc9
Address reviewer's feedback
gregtatcam 38781df
Fix inline isFrozen definition
gregtatcam e239fcb
Address reviewer feedback
gregtatcam fe6fa60
Fix clang-format
gregtatcam f4bc120
Address reviewer's feedback
gregtatcam 780af7a
removed mptHolders (#43)
shawnxie999 e4009c8
address comments (#44)
shawnxie999 5f33c80
move flags (#45)
shawnxie999 e8c8329
Address reviewer's feedback - refactor mpt.[h,cpp]
gregtatcam c53df59
Apply suggestions from code review - update MPT unit-tests
gregtatcam b8f3b83
Fix clang-format
gregtatcam 5349827
Address reviewer's feedback - add MPT unit-tests
gregtatcam 0905e37
Apply suggestions from code review
gregtatcam 9d84f3b
Address reviewer's feedback
gregtatcam e8201f3
address comments (#46)
shawnxie999 5aa36e4
Apply suggestions from code review
gregtatcam 7a6272e
Address reviewer's feedback
gregtatcam 3439bfa
comments (#48)
shawnxie999 b636836
Rename mpt holder (#49)
shawnxie999 7c08b58
Address reviewer's feedback
gregtatcam 9ce2061
Ed's comments (#50)
shawnxie999 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,167 @@ | ||
//------------------------------------------------------------------------------ | ||
/* | ||
This file is part of rippled: https://github.com/ripple/rippled | ||
Copyright (c) 2024 Ripple Labs Inc. | ||
|
||
Permission to use, copy, modify, and/or distribute this software for any | ||
purpose with or without fee is hereby granted, provided that the above | ||
copyright notice and this permission notice appear in all copies. | ||
|
||
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES | ||
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF | ||
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR | ||
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES | ||
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN | ||
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF | ||
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. | ||
*/ | ||
//============================================================================== | ||
|
||
#ifndef RIPPLE_BASICS_MPTAMOUNT_H_INCLUDED | ||
#define RIPPLE_BASICS_MPTAMOUNT_H_INCLUDED | ||
|
||
#include <xrpl/basics/contract.h> | ||
#include <xrpl/basics/safe_cast.h> | ||
#include <xrpl/beast/utility/Zero.h> | ||
#include <xrpl/json/json_value.h> | ||
|
||
#include <boost/multiprecision/cpp_int.hpp> | ||
#include <boost/operators.hpp> | ||
|
||
#include <cstdint> | ||
#include <optional> | ||
#include <string> | ||
#include <type_traits> | ||
|
||
namespace ripple { | ||
|
||
class MPTAmount : private boost::totally_ordered<MPTAmount>, | ||
private boost::additive<MPTAmount>, | ||
private boost::equality_comparable<MPTAmount, std::int64_t>, | ||
private boost::additive<MPTAmount, std::int64_t> | ||
{ | ||
public: | ||
using value_type = std::int64_t; | ||
|
||
protected: | ||
value_type value_; | ||
|
||
public: | ||
MPTAmount() = default; | ||
constexpr MPTAmount(MPTAmount const& other) = default; | ||
constexpr MPTAmount& | ||
operator=(MPTAmount const& other) = default; | ||
|
||
constexpr explicit MPTAmount(value_type value); | ||
|
||
constexpr MPTAmount& operator=(beast::Zero); | ||
|
||
MPTAmount& | ||
operator+=(MPTAmount const& other); | ||
|
||
MPTAmount& | ||
operator-=(MPTAmount const& other); | ||
|
||
MPTAmount | ||
operator-() const; | ||
|
||
bool | ||
operator==(MPTAmount const& other) const; | ||
|
||
bool | ||
operator==(value_type other) const; | ||
|
||
bool | ||
operator<(MPTAmount const& other) const; | ||
|
||
/** Returns true if the amount is not zero */ | ||
explicit constexpr operator bool() const noexcept; | ||
|
||
/** Return the sign of the amount */ | ||
constexpr int | ||
signum() const noexcept; | ||
|
||
/** Returns the underlying value. Code SHOULD NOT call this | ||
function unless the type has been abstracted away, | ||
e.g. in a templated function. | ||
*/ | ||
constexpr value_type | ||
value() const; | ||
|
||
friend std::istream& | ||
operator>>(std::istream& s, MPTAmount& val); | ||
|
||
thejohnfreeman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
static MPTAmount | ||
minPositiveAmount(); | ||
}; | ||
|
||
constexpr MPTAmount::MPTAmount(value_type value) : value_(value) | ||
{ | ||
} | ||
|
||
constexpr MPTAmount& MPTAmount::operator=(beast::Zero) | ||
{ | ||
value_ = 0; | ||
return *this; | ||
} | ||
|
||
/** Returns true if the amount is not zero */ | ||
constexpr MPTAmount::operator bool() const noexcept | ||
{ | ||
return value_ != 0; | ||
} | ||
|
||
/** Return the sign of the amount */ | ||
constexpr int | ||
MPTAmount::signum() const noexcept | ||
{ | ||
return (value_ < 0) ? -1 : (value_ ? 1 : 0); | ||
} | ||
|
||
/** Returns the underlying value. Code SHOULD NOT call this | ||
function unless the type has been abstracted away, | ||
e.g. in a templated function. | ||
*/ | ||
constexpr MPTAmount::value_type | ||
MPTAmount::value() const | ||
{ | ||
return value_; | ||
} | ||
|
||
inline std::string | ||
to_string(MPTAmount const& amount) | ||
{ | ||
return std::to_string(amount.value()); | ||
} | ||
|
||
inline MPTAmount | ||
mulRatio( | ||
MPTAmount const& amt, | ||
std::uint32_t num, | ||
std::uint32_t den, | ||
bool roundUp) | ||
{ | ||
using namespace boost::multiprecision; | ||
|
||
if (!den) | ||
Throw<std::runtime_error>("division by zero"); | ||
|
||
int128_t const amt128(amt.value()); | ||
auto const neg = amt.value() < 0; | ||
auto const m = amt128 * num; | ||
auto r = m / den; | ||
if (m % den) | ||
{ | ||
if (!neg && roundUp) | ||
r += 1; | ||
if (neg && !roundUp) | ||
r -= 1; | ||
} | ||
if (r > std::numeric_limits<MPTAmount::value_type>::max()) | ||
Throw<std::overflow_error>("MPT mulRatio overflow"); | ||
return MPTAmount(r.convert_to<MPTAmount::value_type>()); | ||
} | ||
|
||
} // namespace ripple | ||
|
||
#endif // RIPPLE_BASICS_MPTAMOUNT_H_INCLUDED |
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Seeing how similar
MPTAmount
is toXRPAmount
it's a pity that they aren't implemented with common code. I.e. through a base class, or a template or something. Rather than diving down the potential rabbit hole of implementing it, I'm just going to leave this note here for someone else or for future reference.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.
My opinion:
MPTNumber
andXRPNumber
because they represent quantities, whileSTAmount
represents a quantity plus an asset / issue / unit (however you want to think of it). These are more likeNumber
, and convert directly to and from it.Number
after the switchover anyway. In due time, when we retire that amendment, effectively locking it in permanently, then I think we can removeMPTAmount
andXRPAmount
.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
IOUAmount
change too? I think thatAmount
better communicates what the value is. I don't think you'd say number when talking about tokens or currencies even if the values don't have associate unit. AndXRPAmount
doesn't need an issue, it's implicit. Also doesn't seem like this refactoring has to be done in MPT PR. But this is just my opinion. I'll change if everyone thinksNumber
is better. There are over 300 instances ofXRPAmount
,MPTAmount
,IOUAmount
.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.
I created a ticket to refactor
MPTAmount
andXRPAmount
to use a common code + renaming.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.
Yeah, I'm not asking it to be done here, or at all even. Was just stating my position. My opinion is that
IOUAmount
should be renamed too (in an ideal world), and my only reason for these renames is thatSTAmount
has quantity + issue, while these{XRP,IOU,MPT}Amount
s only have quantity, likeNumber
, which is their common representation. The alternative fix is to renameSTAmount
to a different suffix, but I think that would be even more disruptive, especially to external projects.