-
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
base: develop
Are you sure you want to change the base?
Changes from all commits
6d6fda2
aaf7fe5
f84e9ea
18515dd
aef1426
9136a89
16a029a
2beb2d9
3cc7003
069273c
86bcebd
b9cb9ca
8af452e
fab0c78
ea887bf
d9fc97f
98d419f
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 |
---|---|---|
@@ -0,0 +1,164 @@ | ||
//------------------------------------------------------------------------------ | ||
/* | ||
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; | ||
|
||
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ | |
#ifndef RIPPLE_BASICS_NUMBER_H_INCLUDED | ||
#define RIPPLE_BASICS_NUMBER_H_INCLUDED | ||
|
||
#include <xrpl/basics/MPTAmount.h> | ||
#include <xrpl/basics/XRPAmount.h> | ||
#include <cstdint> | ||
#include <limits> | ||
|
@@ -52,6 +53,7 @@ class Number | |
explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept; | ||
|
||
Number(XRPAmount const& x); | ||
Number(MPTAmount const& x); | ||
Comment on lines
55
to
+56
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. These both should probably be 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. This follows the original implementation by @HowardHinnant 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. The design is that conversions to As 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. Thanks for that clarification, @HowardHinnant ! I left the feedback originally because I assumed that we didn't want any accidental conversions to @gregtatcam Could you add a comment on these functions so that we have this info preserved for the future. It's probably fine to just copy Howard's comment here. |
||
|
||
constexpr rep | ||
mantissa() const noexcept; | ||
|
@@ -89,6 +91,7 @@ class Number | |
lowest() noexcept; | ||
|
||
explicit operator XRPAmount() const; // round to nearest, even on tie | ||
explicit operator MPTAmount() const; // round to nearest, even on tie | ||
explicit operator rep() const; // round to nearest, even on tie | ||
|
||
friend constexpr bool | ||
|
@@ -210,6 +213,10 @@ inline Number::Number(XRPAmount const& x) : Number{x.drops()} | |
{ | ||
} | ||
|
||
inline Number::Number(MPTAmount const& x) : Number{x.value()} | ||
{ | ||
} | ||
|
||
inline constexpr Number::rep | ||
Number::mantissa() const noexcept | ||
{ | ||
|
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.