Skip to content
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
merged 61 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 55 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
6d6fda2
Introduce MPT support (XLS-33d):
gregtatcam Jun 28, 2024
aaf7fe5
Merge branch 'develop' into feature/mpt-v1-var-issues
gregtatcam Sep 30, 2024
f84e9ea
Apply suggestions from code review
gregtatcam Oct 1, 2024
18515dd
Address reviewer's feedback
gregtatcam Oct 1, 2024
aef1426
Allow MPT SendMax in Payment tx.
gregtatcam Oct 1, 2024
9136a89
Apply suggestions from code review
gregtatcam Oct 2, 2024
16a029a
Address reviewer's feedback
gregtatcam Oct 2, 2024
2beb2d9
Manually align jss and disable clang-format
gregtatcam Oct 3, 2024
3cc7003
Apply suggestions from code review
gregtatcam Oct 3, 2024
069273c
Fix clang format
gregtatcam Oct 3, 2024
86bcebd
Apply suggestions from code review
gregtatcam Oct 3, 2024
b9cb9ca
Address reviewer's feedback
gregtatcam Oct 3, 2024
8af452e
remove unneeded fields (#36)
shawnxie999 Oct 3, 2024
fab0c78
rename codes (#37)
shawnxie999 Oct 3, 2024
ea887bf
Address reviewer feedback
gregtatcam Oct 3, 2024
d9fc97f
Check Payment's sendMax and amount have the same issue
gregtatcam Oct 4, 2024
98d419f
Apply suggestions from code review
gregtatcam Oct 4, 2024
3b7861d
Address reviewer's feedback
gregtatcam Oct 7, 2024
93c545d
Fix static_assert in STVar and other changes
gregtatcam Oct 7, 2024
aacb1c8
comment (#38)
shawnxie999 Oct 7, 2024
560cf91
Fix SendMax with no transfer fee
gregtatcam Oct 7, 2024
6a7a8c2
Combine Issue/MPTIssue handling in Payment transactor plus other changes
gregtatcam Oct 9, 2024
8796641
Fix non-unity build
gregtatcam Oct 9, 2024
757b55a
Update src/libxrpl/protocol/TxFormats.cpp
gregtatcam Oct 9, 2024
11aa41c
New SField flag to override hex input/output formatting to use decima…
ximinez Oct 10, 2024
eac438c
Change maxAmount in the test to uint64 for better visualization
gregtatcam Oct 10, 2024
9d51389
Refactor Clawback plus other minor refactoring
gregtatcam Oct 10, 2024
8774b90
Fix requireAuth (#40)
shawnxie999 Oct 11, 2024
4bcf463
Address reviewer's feedback
gregtatcam Oct 11, 2024
9b8564e
Fix metadata and add metadata unit-test
gregtatcam Oct 12, 2024
ac45342
Address reviewer's feedback:
gregtatcam Oct 14, 2024
af29f41
use mptJson
shawnxie999 Oct 15, 2024
9e3cfef
Add += and -= operators to ValueProxy
gregtatcam Oct 15, 2024
4bbf613
ledger_entry test (#41)
shawnxie999 Oct 15, 2024
3bb44c2
Simplify constraint for +=, -= of ValueProxy
gregtatcam Oct 16, 2024
0bf5b42
Address reviewer's feedback on mpt uni-test helper
gregtatcam Oct 16, 2024
924dd9c
refactor mpt_issuance_id into SLE::getJson (#42)
shawnxie999 Oct 16, 2024
31b3be3
Merge branch 'develop' into feature/mpt-v1-var-issues
gregtatcam Oct 16, 2024
c3f2bd4
Fix clang-format
gregtatcam Oct 16, 2024
29c6f5d
Merge with the latest develop
gregtatcam Oct 17, 2024
bd97cc9
Address reviewer's feedback
gregtatcam Oct 17, 2024
38781df
Fix inline isFrozen definition
gregtatcam Oct 17, 2024
e239fcb
Address reviewer feedback
gregtatcam Oct 17, 2024
fe6fa60
Fix clang-format
gregtatcam Oct 18, 2024
f4bc120
Address reviewer's feedback
gregtatcam Oct 18, 2024
780af7a
removed mptHolders (#43)
shawnxie999 Oct 18, 2024
e4009c8
address comments (#44)
shawnxie999 Oct 18, 2024
5f33c80
move flags (#45)
shawnxie999 Oct 21, 2024
e8c8329
Address reviewer's feedback - refactor mpt.[h,cpp]
gregtatcam Oct 21, 2024
c53df59
Apply suggestions from code review - update MPT unit-tests
gregtatcam Oct 21, 2024
b8f3b83
Fix clang-format
gregtatcam Oct 21, 2024
5349827
Address reviewer's feedback - add MPT unit-tests
gregtatcam Oct 21, 2024
0905e37
Apply suggestions from code review
gregtatcam Oct 22, 2024
9d84f3b
Address reviewer's feedback
gregtatcam Oct 22, 2024
e8201f3
address comments (#46)
shawnxie999 Oct 23, 2024
5aa36e4
Apply suggestions from code review
gregtatcam Oct 24, 2024
7a6272e
Address reviewer's feedback
gregtatcam Oct 24, 2024
3439bfa
comments (#48)
shawnxie999 Oct 25, 2024
b636836
Rename mpt holder (#49)
shawnxie999 Oct 28, 2024
7c08b58
Address reviewer's feedback
gregtatcam Oct 29, 2024
9ce2061
Ed's comments (#50)
shawnxie999 Oct 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Builds/levelization/results/loops.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Loop: test.jtx test.unit_test
test.unit_test == test.jtx

Loop: xrpl.basics xrpl.json
xrpl.json ~= xrpl.basics
xrpl.json == xrpl.basics

Loop: xrpld.app xrpld.core
xrpld.app > xrpld.core
Expand Down
166 changes: 166 additions & 0 deletions include/xrpl/basics/MPTAmount.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
//------------------------------------------------------------------------------
/*
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>,
Copy link
Collaborator

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 to XRPAmount 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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My opinion:

  • They should be called MPTNumber and XRPNumber because they represent quantities, while STAmount represents a quantity plus an asset / issue / unit (however you want to think of it). These are more like Number, and convert directly to and from it.
  • All of the arithmetic is moving to Number after the switchover anyway. In due time, when we retire that amendment, effectively locking it in permanently, then I think we can remove MPTAmount and XRPAmount.

Copy link
Collaborator Author

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 that Amount 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. And XRPAmount 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 thinks Number is better. There are over 300 instances of XRPAmount, MPTAmount, IOUAmount.

Copy link
Collaborator Author

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 and XRPAmount to use a common code + renaming.

Copy link
Collaborator

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 that STAmount has quantity + issue, while these {XRP,IOU,MPT}Amounts only have quantity, like Number, which is their common representation. The alternative fix is to rename STAmount to a different suffix, but I think that would be even more disruptive, especially to external projects.

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
13 changes: 13 additions & 0 deletions include/xrpl/basics/Number.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -52,6 +53,7 @@ class Number
explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept;

Number(XRPAmount const& x);
Number(MPTAmount const& x);
ximinez marked this conversation as resolved.
Show resolved Hide resolved

constexpr rep
mantissa() const noexcept;
Expand Down Expand Up @@ -88,9 +90,16 @@ class Number
static constexpr Number
lowest() noexcept;

/** Conversions to Number are implicit and conversions away from Number
* are explicit. This design encourages and facilitates the use of Number
* as the preferred type for floating point arithmetic as it makes
* "mixed mode" more convenient, e.g. MPTAmount + Number.
*/
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
Expand Down Expand Up @@ -212,6 +221,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
{
Expand Down
4 changes: 4 additions & 0 deletions include/xrpl/basics/XRPAmount.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ class XRPAmount : private boost::totally_ordered<XRPAmount>,
return dropsAs<Dest>().value_or(defaultValue.drops());
}

/* Clips a 64-bit value to a 32-bit JSON number. It is only used
* in contexts that don't expect the value to ever approach
* the 32-bit limits (i.e. fees and reserves).
*/
Json::Value
jsonClipped() const
{
Expand Down
2 changes: 2 additions & 0 deletions include/xrpl/basics/base_uint.h
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ class base_uint
using uint128 = base_uint<128>;
using uint160 = base_uint<160>;
using uint256 = base_uint<256>;
using uint192 = base_uint<192>;

template <std::size_t Bits, class Tag>
[[nodiscard]] inline constexpr std::strong_ordering
Expand Down Expand Up @@ -634,6 +635,7 @@ operator<<(std::ostream& out, base_uint<Bits, Tag> const& u)
#ifndef __INTELLISENSE__
static_assert(sizeof(uint128) == 128 / 8, "There should be no padding bytes");
static_assert(sizeof(uint160) == 160 / 8, "There should be no padding bytes");
static_assert(sizeof(uint192) == 192 / 8, "There should be no padding bytes");
static_assert(sizeof(uint256) == 256 / 8, "There should be no padding bytes");
#endif

Expand Down
8 changes: 1 addition & 7 deletions include/xrpl/protocol/AmountConversions.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,7 @@ toSTAmount(IOUAmount const& iou, Issue const& iss)
{
bool const isNeg = iou.signum() < 0;
std::uint64_t const umant = isNeg ? -iou.mantissa() : iou.mantissa();
return STAmount(
iss,
umant,
iou.exponent(),
/*native*/ false,
isNeg,
STAmount::unchecked());
return STAmount(iss, umant, iou.exponent(), isNeg, STAmount::unchecked());
}

inline STAmount
Expand Down
Loading
Loading