Resolve merge conflicts, and refactor

gregtatcam · Dec 10, 2024 · cf58834 · cf58834
1 parent 5771e20
commit cf58834
Show file tree

Hide file tree

Showing 43 changed files with 309 additions and 417 deletions.
diff --git a/include/xrpl/protocol/Asset.h b/include/xrpl/protocol/Asset.h
@@ -27,6 +27,15 @@
 
 namespace ripple {
 
+template <typename T>
+    requires(
+        std::is_same_v<T, XRPAmount> || std::is_same_v<T, IOUAmount> ||
+        std::is_same_v<T, MPTAmount>)
+struct AmountType
+{
+    using amount_type = T;
+};
+
 /* Asset is an abstraction of three different issue types: XRP, IOU, MPT.
  * For historical reasons, two issue types XRP and IOU are wrapped in Issue
  * type. Many functions and classes there were first written for Issue
@@ -92,14 +101,14 @@ class Asset
         return holds<Issue>() && get<Issue>().native();
     }
 
-    friend constexpr bool
-    operator==(Asset const& lhs, Asset const& rhs);
-
-    friend constexpr bool
-    operator!=(Asset const& lhs, Asset const& rhs);
+    std::variant<
+        AmountType<XRPAmount>,
+        AmountType<IOUAmount>,
+        AmountType<MPTAmount>>
+    getAmountType() const;
 
     friend constexpr bool
-    operator<(Asset const& lhs, Asset const& rhs);
+    operator==(Asset const& lhs, Asset const& rhs);
 
     friend constexpr std::weak_ordering
     operator<=>(Asset const& lhs, Asset const& rhs);
@@ -160,27 +169,6 @@ operator==(Asset const& lhs, Asset const& rhs)
         rhs.issue_);
 }
 
-constexpr bool
-operator!=(Asset const& lhs, Asset const& rhs)
-{
-    return !(lhs == rhs);
-}
-
-constexpr bool
-operator<(Asset const& lhs, Asset const& rhs)
-{
-    return std::visit(
-        [&]<typename TLhs, typename TRhs>(
-            TLhs const& issLhs, TRhs const& issRhs) {
-            if constexpr (std::is_same_v<TLhs, TRhs>)
-                return issLhs < issRhs;
-            else
-                return false;
-        },
-        lhs.issue_,
-        rhs.issue_);
-}
-
 constexpr bool
 operator==(Currency const& lhs, Asset const& rhs)
 {

diff --git a/include/xrpl/protocol/MPTAmount.h b/include/xrpl/protocol/MPTAmount.h
@@ -94,9 +94,6 @@ class MPTAmount : private boost::totally_ordered<MPTAmount>,
     constexpr int
     signum() const noexcept;
 
-    Json::Value
-    jsonClipped() const;
-
     /** Returns the underlying value. Code SHOULD NOT call this
         function unless the type has been abstracted away,
         e.g. in a templated function.

diff --git a/include/xrpl/protocol/MPTIssue.h b/include/xrpl/protocol/MPTIssue.h
@@ -53,39 +53,16 @@ class MPTIssue
     void
     setJson(Json::Value& jv) const;
 
-    auto
+    constexpr std::weak_ordering
     operator<=>(MPTIssue const&) const = default;
 
     bool
     native() const
     {
         return false;
     }
-
-    friend constexpr std::weak_ordering
-    operator<=>(MPTIssue const& lhs, MPTIssue const& rhs);
 };
 
-constexpr bool
-operator==(MPTIssue const& lhs, MPTIssue const& rhs)
-{
-    return lhs.mptID_ == rhs.mptID_;
-}
-
-constexpr bool
-operator!=(MPTIssue const& lhs, MPTIssue const& rhs)
-{
-    return !(lhs == rhs);
-}
-
-constexpr std::weak_ordering
-operator<=>(MPTIssue const& lhs, MPTIssue const& rhs)
-{
-    if (auto const c{lhs.mptID_ <=> rhs.mptID_}; c != 0)
-        return c;
-    return lhs.mptID_ <=> rhs.mptID_;
-}
-
 /** MPT is a non-native token.
  */
 inline bool

diff --git a/include/xrpl/protocol/PathAsset.h b/include/xrpl/protocol/PathAsset.h
@@ -25,13 +25,16 @@
 
 namespace ripple {
 
+/* Represent STPathElement's asset, which can be Currency or MPTID.
+ */
 class PathAsset
 {
 private:
     std::variant<Currency, MPTID> easset_;
 
 public:
     PathAsset() = default;
+    // Enables comparing Asset and PathAsset
     PathAsset(Asset const& asset);
     PathAsset(Currency const& currency) : easset_(currency)
     {
@@ -54,12 +57,6 @@ class PathAsset
     constexpr std::variant<Currency, MPTID> const&
     value() const;
 
-    static PathAsset
-    toPathAsset(Asset const& asset);
-
-    static std::optional<PathAsset>
-    toPathAsset(std::optional<Asset> const& asset);
-
     friend constexpr bool
     operator==(PathAsset const& lhs, PathAsset const& rhs);
 };
@@ -101,7 +98,9 @@ PathAsset::value() const
 constexpr bool
 PathAsset::isXRP() const
 {
-    return holds<Currency>() && get<Currency>() == xrpCurrency();
+    return std::visit(
+        [&]<ValidPathAsset A>(A const& a) { return ripple::isXRP(a); },
+        easset_);
 }
 
 constexpr bool
@@ -139,12 +138,6 @@ to_string(PathAsset const& asset);
 std::ostream&
 operator<<(std::ostream& os, PathAsset const& x);
 
-bool
-equalAssets(PathAsset const& asset1, Asset const& asset2);
-
-bool
-equalAssets(Asset const& asset1, PathAsset const& asset2);
-
 }  // namespace ripple
 
 #endif  // RIPPLE_APP_PATHASSET_H_INCLUDED
diff --git a/include/xrpl/protocol/STPathSet.h b/include/xrpl/protocol/STPathSet.h
@@ -44,9 +44,6 @@ class STPathElement final : public CountedObject<STPathElement>
     std::size_t hash_value_;
 
 public:
-    struct PathAssetTag
-    {
-    };
     enum Type {
         typeNone = 0x00,
         typeAccount =
@@ -65,34 +62,16 @@ class STPathElement final : public CountedObject<STPathElement>
     STPathElement&
     operator=(STPathElement const&) = default;
 
-    STPathElement(
-        std::optional<AccountID> const& account,
-        std::optional<Asset> const& asset,
-        std::optional<AccountID> const& issuer);
-
     STPathElement(
         std::optional<AccountID> const& account,
         std::optional<PathAsset> const& asset,
-        std::optional<AccountID> const& issuer,
-        PathAssetTag);
-
-    STPathElement(
-        AccountID const& account,
-        Asset const& asset,
-        AccountID const& issuer,
-        bool forceCurrency = false);
+        std::optional<AccountID> const& issuer);
 
     STPathElement(
         AccountID const& account,
         PathAsset const& asset,
         AccountID const& issuer,
-        bool forceCurrency = false);
-
-    STPathElement(
-        unsigned int uType,
-        AccountID const& account,
-        Asset const& asset,
-        AccountID const& issuer);
+        bool forceAsset = false);
 
     STPathElement(
         unsigned int uType,
@@ -174,12 +153,6 @@ class STPath final : public CountedObject<STPath>
     void
     emplace_back(Args&&... args);
 
-    bool
-    hasSeen(
-        AccountID const& account,
-        Asset const& asset,
-        AccountID const& issuer) const;
-
     bool
     hasSeen(
         AccountID const& account,
@@ -285,23 +258,10 @@ inline STPathElement::STPathElement() : mType(typeNone), is_offer_(true)
     hash_value_ = get_hash(*this);
 }
 
-inline STPathElement::STPathElement(
-    std::optional<AccountID> const& account,
-    std::optional<Asset> const& asset,
-    std::optional<AccountID> const& issuer)
-    : STPathElement(
-          account,
-          PathAsset::toPathAsset(asset),
-          issuer,
-          PathAssetTag{})
-{
-}
-
 inline STPathElement::STPathElement(
     std::optional<AccountID> const& account,
     std::optional<PathAsset> const& asset,
-    std::optional<AccountID> const& issuer,
-    PathAssetTag)
+    std::optional<AccountID> const& issuer)
     : mType(typeNone)
 {
     if (!account)
@@ -336,24 +296,11 @@ inline STPathElement::STPathElement(
     hash_value_ = get_hash(*this);
 }
 
-inline STPathElement::STPathElement(
-    AccountID const& account,
-    Asset const& asset,
-    AccountID const& issuer,
-    bool forceCurrency)
-    : STPathElement(
-          account,
-          PathAsset::toPathAsset(asset),
-          issuer,
-          forceCurrency)
-{
-}
-
 inline STPathElement::STPathElement(
     AccountID const& account,
     PathAsset const& asset,
     AccountID const& issuer,
-    bool forceCurrency)
+    bool forceAsset)
     : mType(typeNone)
     , mAccountID(account)
     , mAssetID(asset)
@@ -363,28 +310,15 @@ inline STPathElement::STPathElement(
     if (!is_offer_)
         mType |= typeAccount;
 
-    if (!asset.holds<MPTID>() &&
-        (forceCurrency || !isXRP(mAssetID.get<Currency>())))
-        mType |= typeCurrency;
+    if (forceAsset || !isXRP(mAssetID))
+        mType |= asset.holds<MPTID>() ? typeMPT : typeCurrency;
 
     if (!isXRP(issuer))
         mType |= typeIssuer;
 
-    if (asset.holds<MPTID>())
-        mType |= typeMPT;
-
     hash_value_ = get_hash(*this);
 }
 
-inline STPathElement::STPathElement(
-    unsigned int uType,
-    AccountID const& account,
-    Asset const& asset,
-    AccountID const& issuer)
-    : STPathElement(uType, account, PathAsset::toPathAsset(asset), issuer)
-{
-}
-
 inline STPathElement::STPathElement(
     unsigned int uType,
     AccountID const& account,
@@ -396,6 +330,8 @@ inline STPathElement::STPathElement(
     , mIssuerID(issuer)
     , is_offer_(isXRP(mAccountID))
 {
+    // uType could be assetType; i.e. either Currency or MPTID.
+    // Get the actual type.
     if (!asset.holds<MPTID>())
         mType = mType & (~Type::typeMPT);
     else if (mAssetID.holds<Currency>() && isXRP(mAssetID.get<Currency>()))

diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro
@@ -236,8 +236,8 @@ TRANSACTION(ttCLAWBACK, 30, Clawback, ({
 /** This transaction claws back tokens from an AMM pool. */
 TRANSACTION(ttAMM_CLAWBACK, 31, AMMClawback, ({
     {sfHolder, soeREQUIRED},
-    {sfAsset, soeREQUIRED},
-    {sfAsset2, soeREQUIRED},
+    {sfAsset, soeREQUIRED, soeMPTSupported},
+    {sfAsset2, soeREQUIRED, soeMPTSupported},
     {sfAmount, soeOPTIONAL, soeMPTSupported},
 }))
 
@@ -250,8 +250,8 @@ TRANSACTION(ttAMM_CREATE, 35, AMMCreate, ({
 
 /** This transaction type deposits into an AMM instance */
 TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, ({
-    {sfAsset, soeREQUIRED},
-    {sfAsset2, soeREQUIRED},
+    {sfAsset, soeREQUIRED, soeMPTSupported},
+    {sfAsset2, soeREQUIRED, soeMPTSupported},
     {sfAmount, soeOPTIONAL, soeMPTSupported},
     {sfAmount2, soeOPTIONAL, soeMPTSupported},
     {sfEPrice, soeOPTIONAL},
@@ -261,8 +261,8 @@ TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, ({
 
 /** This transaction type withdraws from an AMM instance */
 TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, ({
-    {sfAsset, soeREQUIRED},
-    {sfAsset2, soeREQUIRED},
+    {sfAsset, soeREQUIRED, soeMPTSupported},
+    {sfAsset2, soeREQUIRED, soeMPTSupported},
     {sfAmount, soeOPTIONAL, soeMPTSupported},
     {sfAmount2, soeOPTIONAL, soeMPTSupported},
     {sfEPrice, soeOPTIONAL},
@@ -271,24 +271,24 @@ TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, ({
 
 /** This transaction type votes for the trading fee */
 TRANSACTION(ttAMM_VOTE, 38, AMMVote, ({
-    {sfAsset, soeREQUIRED},
-    {sfAsset2, soeREQUIRED},
+    {sfAsset, soeREQUIRED, soeMPTSupported},
+    {sfAsset2, soeREQUIRED, soeMPTSupported},
     {sfTradingFee, soeREQUIRED},
 }))
 
 /** This transaction type bids for the auction slot */
 TRANSACTION(ttAMM_BID, 39, AMMBid, ({
-    {sfAsset, soeREQUIRED},
-    {sfAsset2, soeREQUIRED},
+    {sfAsset, soeREQUIRED, soeMPTSupported},
+    {sfAsset2, soeREQUIRED, soeMPTSupported},
     {sfBidMin, soeOPTIONAL},
     {sfBidMax, soeOPTIONAL},
     {sfAuthAccounts, soeOPTIONAL},
 }))
 
 /** This transaction type deletes AMM in the empty state */
 TRANSACTION(ttAMM_DELETE, 40, AMMDelete, ({
-    {sfAsset, soeREQUIRED},
-    {sfAsset2, soeREQUIRED},
+    {sfAsset, soeREQUIRED, soeMPTSupported},
+    {sfAsset2, soeREQUIRED, soeMPTSupported},
 }))
 
 /** This transactions creates a crosschain sequence number */