From e0ffb63cf7b5498b4b69c2be8cb88cde862f0c02 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Fri, 22 Nov 2024 11:40:38 -0500 Subject: [PATCH 01/10] Add MPTIssue to STIssue --- include/xrpl/protocol/Asset.h | 25 +++++ include/xrpl/protocol/Indexes.h | 2 +- include/xrpl/protocol/MPTIssue.h | 19 +--- include/xrpl/protocol/SOTemplate.h | 12 ++- include/xrpl/protocol/STAmount.h | 5 - include/xrpl/protocol/STIssue.h | 74 ++++++++----- include/xrpl/protocol/STXChainBridge.h | 4 +- include/xrpl/protocol/detail/features.macro | 2 +- src/libxrpl/protocol/Indexes.cpp | 8 +- src/libxrpl/protocol/STIssue.cpp | 87 ++++++++++++---- src/libxrpl/protocol/STTx.cpp | 6 +- src/test/app/MPToken_test.cpp | 109 ++++++++++++++------ src/xrpld/app/ledger/OrderBookDB.cpp | 4 +- src/xrpld/app/misc/detail/AMMUtils.cpp | 8 +- src/xrpld/app/tx/detail/AMMBid.cpp | 3 +- src/xrpld/app/tx/detail/AMMClawback.cpp | 12 +-- src/xrpld/app/tx/detail/AMMDelete.cpp | 4 +- src/xrpld/app/tx/detail/AMMDeposit.cpp | 8 +- src/xrpld/app/tx/detail/AMMVote.cpp | 3 +- src/xrpld/app/tx/detail/AMMWithdraw.cpp | 11 +- src/xrpld/rpc/handlers/AMMInfo.cpp | 4 +- 21 files changed, 273 insertions(+), 137 deletions(-) diff --git a/include/xrpl/protocol/Asset.h b/include/xrpl/protocol/Asset.h index 2cccc28bd41..62afb6aee94 100644 --- a/include/xrpl/protocol/Asset.h +++ b/include/xrpl/protocol/Asset.h @@ -26,10 +26,17 @@ namespace ripple { +class Asset; + template concept ValidIssueType = std::is_same_v || std::is_same_v; +template +concept AssetType = + std::is_convertible_v || std::is_convertible_v || + std::is_convertible_v || std::is_convertible_v; + /* 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 @@ -95,6 +102,9 @@ class Asset friend constexpr bool operator!=(Asset const& lhs, Asset const& rhs); + friend constexpr bool + operator<(Asset const& lhs, Asset const& rhs); + friend constexpr bool operator==(Currency const& lhs, Asset const& rhs); @@ -157,6 +167,21 @@ operator!=(Asset const& lhs, Asset const& rhs) return !(lhs == rhs); } +constexpr bool +operator<(Asset const& lhs, Asset const& rhs) +{ + return std::visit( + [&]( + TLhs const& issLhs, TRhs const& issRhs) { + if constexpr (std::is_same_v) + 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/Indexes.h b/include/xrpl/protocol/Indexes.h index 72cf0b527b1..c4b32f3863b 100644 --- a/include/xrpl/protocol/Indexes.h +++ b/include/xrpl/protocol/Indexes.h @@ -273,7 +273,7 @@ nft_sells(uint256 const& id) noexcept; /** AMM entry */ Keylet -amm(Issue const& issue1, Issue const& issue2) noexcept; +amm(Asset const& issue1, Asset const& issue2) noexcept; Keylet amm(uint256 const& amm) noexcept; diff --git a/include/xrpl/protocol/MPTIssue.h b/include/xrpl/protocol/MPTIssue.h index 06f55686caf..46f28dd87e1 100644 --- a/include/xrpl/protocol/MPTIssue.h +++ b/include/xrpl/protocol/MPTIssue.h @@ -51,11 +51,8 @@ class MPTIssue void setJson(Json::Value& jv) const; - friend constexpr bool - operator==(MPTIssue const& lhs, MPTIssue const& rhs); - - friend constexpr bool - operator!=(MPTIssue const& lhs, MPTIssue const& rhs); + auto + operator<=>(MPTIssue const&) const = default; bool native() const @@ -64,18 +61,6 @@ class MPTIssue } }; -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); -} - /** MPT is a non-native token. */ inline bool diff --git a/include/xrpl/protocol/SOTemplate.h b/include/xrpl/protocol/SOTemplate.h index 95cd35fead2..547ca302a59 100644 --- a/include/xrpl/protocol/SOTemplate.h +++ b/include/xrpl/protocol/SOTemplate.h @@ -40,7 +40,7 @@ enum SOEStyle { }; /** Amount fields that can support MPT */ -enum SOETxMPTAmount { soeMPTNone, soeMPTSupported, soeMPTNotSupported }; +enum SOETxMPTIssue { soeMPTNone, soeMPTSupported, soeMPTNotSupported }; //------------------------------------------------------------------------------ @@ -50,7 +50,7 @@ class SOElement // Use std::reference_wrapper so SOElement can be stored in a std::vector. std::reference_wrapper sField_; SOEStyle style_; - SOETxMPTAmount supportMpt_ = soeMPTNone; + SOETxMPTIssue supportMpt_ = soeMPTNone; private: void @@ -72,10 +72,12 @@ class SOElement { init(fieldName); } + template + requires(std::is_same_v || std::is_same_v) SOElement( - TypedField const& fieldName, + TypedField const& fieldName, SOEStyle style, - SOETxMPTAmount supportMpt = soeMPTNotSupported) + SOETxMPTIssue supportMpt = soeMPTNotSupported) : sField_(fieldName), style_(style), supportMpt_(supportMpt) { init(fieldName); @@ -93,7 +95,7 @@ class SOElement return style_; } - SOETxMPTAmount + SOETxMPTIssue supportMPT() const { return supportMpt_; diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index e0a6c1eca08..c0d357b6b9b 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -34,11 +34,6 @@ namespace ripple { -template -concept AssetType = - std::is_same_v || std::is_convertible_v || - std::is_convertible_v || std::is_convertible_v; - // Internal form: // 1: If amount is zero, then value is zero and offset is -100 // 2: Otherwise: diff --git a/include/xrpl/protocol/STIssue.h b/include/xrpl/protocol/STIssue.h index a0dfbd4faec..e97c2ab38ae 100644 --- a/include/xrpl/protocol/STIssue.h +++ b/include/xrpl/protocol/STIssue.h @@ -21,7 +21,7 @@ #define RIPPLE_PROTOCOL_STISSUE_H_INCLUDED #include -#include +#include #include #include #include @@ -31,31 +31,40 @@ namespace ripple { class STIssue final : public STBase, CountedObject { private: - Issue issue_{xrpIssue()}; + Asset asset_{xrpIssue()}; public: - using value_type = Issue; + using value_type = Asset; STIssue() = default; explicit STIssue(SerialIter& sit, SField const& name); - explicit STIssue(SField const& name, Issue const& issue); + template + explicit STIssue(SField const& name, A const& issue); explicit STIssue(SField const& name); - Issue const& - issue() const; + template + TIss const& + get() const; - Issue const& + template + bool + holds() const; + + value_type const& value() const noexcept; void - setIssue(Issue const& issue); + setIssue(Asset const& issue); SerializedTypeID getSType() const override; + std::string + getText() const override; + Json::Value getJson(JsonOptions) const override; void @@ -76,35 +85,54 @@ class STIssue final : public STBase, CountedObject friend class detail::STVar; }; +template +STIssue::STIssue(SField const& name, A const& asset) + : STBase{name}, asset_{asset} +{ + if (holds() && !isConsistent(asset_.get())) + Throw( + "Invalid asset: currency and account native mismatch"); +} + STIssue issueFromJson(SField const& name, Json::Value const& v); -inline Issue const& -STIssue::issue() const +template +bool +STIssue::holds() const +{ + return std::holds_alternative(asset_.value()); +} + +template +TIss const& +STIssue::get() const { - return issue_; + if (!holds(asset_)) + Throw("Asset doesn't hold the requested issue"); + return std::get(asset_); } -inline Issue const& +inline STIssue::value_type const& STIssue::value() const noexcept { - return issue_; + return asset_; } inline void -STIssue::setIssue(Issue const& issue) +STIssue::setIssue(Asset const& asset) { - if (isXRP(issue_.currency) != isXRP(issue_.account)) + if (holds() && !isConsistent(asset_.get())) Throw( - "invalid issue: currency and account native mismatch"); + "Invalid asset: currency and account native mismatch"); - issue_ = issue; + asset_ = asset; } inline bool operator==(STIssue const& lhs, STIssue const& rhs) { - return lhs.issue() == rhs.issue(); + return lhs.value() == rhs.value(); } inline bool @@ -116,19 +144,19 @@ operator!=(STIssue const& lhs, STIssue const& rhs) inline bool operator<(STIssue const& lhs, STIssue const& rhs) { - return lhs.issue() < rhs.issue(); + return lhs.value() < rhs.value(); } inline bool -operator==(STIssue const& lhs, Issue const& rhs) +operator==(STIssue const& lhs, Asset const& rhs) { - return lhs.issue() == rhs; + return lhs.value() == rhs; } inline bool -operator<(STIssue const& lhs, Issue const& rhs) +operator<(STIssue const& lhs, Asset const& rhs) { - return lhs.issue() < rhs; + return lhs.value() < rhs; } } // namespace ripple diff --git a/include/xrpl/protocol/STXChainBridge.h b/include/xrpl/protocol/STXChainBridge.h index 38db1912c70..813bcc44437 100644 --- a/include/xrpl/protocol/STXChainBridge.h +++ b/include/xrpl/protocol/STXChainBridge.h @@ -170,7 +170,7 @@ STXChainBridge::lockingChainDoor() const inline Issue const& STXChainBridge::lockingChainIssue() const { - return lockingChainIssue_.value(); + return lockingChainIssue_.value().get(); }; inline AccountID const& @@ -182,7 +182,7 @@ STXChainBridge::issuingChainDoor() const inline Issue const& STXChainBridge::issuingChainIssue() const { - return issuingChainIssue_.value(); + return issuingChainIssue_.value().get(); }; inline STXChainBridge::value_type const& diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 24c6e72ae34..31fc90cef80 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -32,9 +32,9 @@ XRPL_FEATURE(Credentials, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(AMMClawback, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (AMMv1_2, Supported::yes, VoteBehavior::DefaultNo) +XRPL_FEATURE(MPTokensV1, Supported::yes, VoteBehavior::DefaultNo) // InvariantsV1_1 will be changes to Supported::yes when all the // invariants expected to be included under it are complete. -XRPL_FEATURE(MPTokensV1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(InvariantsV1_1, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (NFTokenPageLinks, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (InnerObjTemplate2, Supported::yes, VoteBehavior::DefaultNo) diff --git a/src/libxrpl/protocol/Indexes.cpp b/src/libxrpl/protocol/Indexes.cpp index 12142879ad5..d9fec8ab509 100644 --- a/src/libxrpl/protocol/Indexes.cpp +++ b/src/libxrpl/protocol/Indexes.cpp @@ -17,6 +17,7 @@ */ //============================================================================== +#include #include #include #include @@ -411,9 +412,12 @@ nft_sells(uint256 const& id) noexcept } Keylet -amm(Issue const& issue1, Issue const& issue2) noexcept +amm(Asset const& issue1, Asset const& issue2) noexcept { - auto const& [minI, maxI] = std::minmax(issue1, issue2); + if (!issue1.holds() || !issue2.holds()) + Throw("Asset doesn't hold issue"); + auto const& [minI, maxI] = + std::minmax(issue1.get(), issue2.get()); return amm(indexHash( LedgerNameSpace::AMM, minI.account, diff --git a/src/libxrpl/protocol/STIssue.cpp b/src/libxrpl/protocol/STIssue.cpp index 00fe86b3287..7fc5074caa7 100644 --- a/src/libxrpl/protocol/STIssue.cpp +++ b/src/libxrpl/protocol/STIssue.cpp @@ -40,23 +40,44 @@ STIssue::STIssue(SField const& name) : STBase{name} STIssue::STIssue(SerialIter& sit, SField const& name) : STBase{name} { - issue_.currency = sit.get160(); - if (!isXRP(issue_.currency)) - issue_.account = sit.get160(); - else - issue_.account = xrpAccount(); - - if (isXRP(issue_.currency) != isXRP(issue_.account)) - Throw( - "invalid issue: currency and account native mismatch"); -} + auto const currencyOrAccount = sit.get160(); -STIssue::STIssue(SField const& name, Issue const& issue) - : STBase{name}, issue_{issue} -{ - if (isXRP(issue_.currency) != isXRP(issue_.account)) - Throw( - "invalid issue: currency and account native mismatch"); + if (isXRP(static_cast(currencyOrAccount))) + { + asset_ = xrpIssue(); + } + // Check if MPT + else + { + // MPT is serialized as: + // - 160 bits MPT issuer account + // - 160 bits black hole account + // - 32 bits sequence + AccountID account = static_cast(sit.get160()); + // MPT + if (noAccount() == account) + { + uint192 mptID; + std::uint32_t sequence = sit.get32(); + memcpy(mptID.data(), &sequence, sizeof(sequence)); + memcpy( + mptID.data() + sizeof(sequence), + currencyOrAccount.data(), + sizeof(currencyOrAccount)); + MPTIssue issue{mptID}; + asset_ = issue; + } + else + { + Issue issue; + issue.currency = currencyOrAccount; + issue.account = account; + if (!isConsistent(issue)) + Throw( + "invalid issue: currency and account native mismatch"); + asset_ = issue; + } + } } SerializedTypeID @@ -65,18 +86,40 @@ STIssue::getSType() const return STI_ISSUE; } +std::string +STIssue::getText() const +{ + return asset_.getText(); +} + Json::Value STIssue::getJson(JsonOptions) const { - return to_json(issue_); + Json::Value jv; + asset_.setJson(jv); + return jv; } void STIssue::add(Serializer& s) const { - s.addBitString(issue_.currency); - if (!isXRP(issue_.currency)) - s.addBitString(issue_.account); + if (holds()) + { + s.addBitString(asset_.get().currency); + if (!isXRP(asset_.get().currency)) + s.addBitString(asset_.get().account); + } + else + { + s.addBitString(asset_.get().getIssuer()); + s.addBitString(noAccount()); + std::uint32_t sequence; + memcpy( + &sequence, + asset_.get().getMptID().data(), + sizeof(sequence)); + s.add32(sequence); + } } bool @@ -89,7 +132,7 @@ STIssue::isEquivalent(const STBase& t) const bool STIssue::isDefault() const { - return issue_ == xrpIssue(); + return holds() && asset_.get() == xrpIssue(); } STBase* @@ -107,7 +150,7 @@ STIssue::move(std::size_t n, void* buf) STIssue issueFromJson(SField const& name, Json::Value const& v) { - return STIssue{name, issueFromJson(v)}; + return STIssue{name, assetFromJson(v)}; } } // namespace ripple diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index 7bd25246c53..c0631141a2e 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -557,8 +557,10 @@ invalidMPTAmountInTx(STObject const& tx) if (tx.isFieldPresent(e.sField()) && e.supportMPT() != soeMPTNone) { if (auto const& field = tx.peekAtField(e.sField()); - field.getSType() == STI_AMOUNT && - static_cast(field).holds()) + (field.getSType() == STI_AMOUNT && + static_cast(field).holds()) || + (field.getSType() == STI_ISSUE && + static_cast(field).holds())) { if (e.supportMPT() != soeMPTSupported) return true; diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 796a3f14c88..9fd4927d5eb 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -1470,19 +1470,19 @@ class MPToken_test : public beast::unit_test::suite void testMPTInvalidInTx(FeatureBitset features) { - testcase("MPT Amount Invalid in Transaction"); + testcase("MPT Issue Invalid in Transaction"); using namespace test::jtx; - // Validate that every transaction with an amount field, + // Validate that every transaction with an amount/issue field, // which doesn't support MPT, fails. - // keyed by transaction + amount field + // keyed by transaction + amount/issue field std::set txWithAmounts; for (auto const& format : TxFormats::getInstance()) { for (auto const& e : format.getSOTemplate()) { - // Transaction has amount fields. + // Transaction has amount/issue fields. // Exclude pseudo-transaction SetFee. Don't consider // the Fee field since it's included in every transaction. if (e.supportMPT() == soeMPTNotSupported && @@ -1508,9 +1508,9 @@ class MPToken_test : public beast::unit_test::suite env.fund(XRP(1'000), alice); env.fund(XRP(1'000), carol); auto test = [&](Json::Value const& jv, - std::string const& amtField) { + std::string const& mptField) { txWithAmounts.erase( - jv[jss::TransactionType].asString() + amtField); + jv[jss::TransactionType].asString() + mptField); // tx is signed auto jtx = env.jt(jv); @@ -1530,8 +1530,27 @@ class MPToken_test : public beast::unit_test::suite jrr = env.rpc("json", "sign", to_string(jv1)); BEAST_EXPECT(jrr[jss::result][jss::error] == "invalidParams"); }; - // All transactions with sfAmount, which don't support MPT - // and transactions with amount fields, which can't be MPT + auto toSFieldRef = [](SField const& field) { + return std::ref(field); + }; + auto setMPTFields = [&](SField const& field, + Json::Value& jv, + bool withAmount = true) { + jv[jss::Asset] = to_json(xrpIssue()); + jv[jss::Asset2] = to_json(USD.issue()); + if (withAmount) + jv[field.fieldName] = + USD(10).value().getJson(JsonOptions::none); + if (field == sfAsset) + jv[jss::Asset] = to_json(mpt.get()); + else if (field == sfAsset2) + jv[jss::Asset2] = to_json(mpt.get()); + else + jv[field.fieldName] = mpt.getJson(JsonOptions::none); + }; + // All transactions with sfAmount, which don't support MPT. + // Transactions with amount fields, which can't be MPT. + // Transactions with issue fields, which can't be MPT. // AMMCreate auto ammCreate = [&](SField const& field) { @@ -1554,58 +1573,84 @@ class MPToken_test : public beast::unit_test::suite Json::Value jv; jv[jss::TransactionType] = jss::AMMDeposit; jv[jss::Account] = alice.human(); - jv[jss::Asset] = to_json(xrpIssue()); - jv[jss::Asset2] = to_json(USD.issue()); - jv[field.fieldName] = mpt.getJson(JsonOptions::none); jv[jss::Flags] = tfSingleAsset; + setMPTFields(field, jv); test(jv, field.fieldName); }; for (SField const& field : - {std::ref(sfAmount), - std::ref(sfAmount2), - std::ref(sfEPrice), - std::ref(sfLPTokenOut)}) + {toSFieldRef(sfAmount), + toSFieldRef(sfAmount2), + toSFieldRef(sfEPrice), + toSFieldRef(sfLPTokenOut), + toSFieldRef(sfAsset), + toSFieldRef(sfAsset2)}) ammDeposit(field); // AMMWithdraw auto ammWithdraw = [&](SField const& field) { Json::Value jv; jv[jss::TransactionType] = jss::AMMWithdraw; jv[jss::Account] = alice.human(); - jv[jss::Asset] = to_json(xrpIssue()); - jv[jss::Asset2] = to_json(USD.issue()); jv[jss::Flags] = tfSingleAsset; - jv[field.fieldName] = mpt.getJson(JsonOptions::none); + setMPTFields(field, jv); test(jv, field.fieldName); }; ammWithdraw(sfAmount); for (SField const& field : - {std::ref(sfAmount2), - std::ref(sfEPrice), - std::ref(sfLPTokenIn)}) + {toSFieldRef(sfAmount2), + toSFieldRef(sfEPrice), + toSFieldRef(sfLPTokenIn), + toSFieldRef(sfAsset), + toSFieldRef(sfAsset2)}) ammWithdraw(field); // AMMBid auto ammBid = [&](SField const& field) { Json::Value jv; jv[jss::TransactionType] = jss::AMMBid; jv[jss::Account] = alice.human(); - jv[jss::Asset] = to_json(xrpIssue()); - jv[jss::Asset2] = to_json(USD.issue()); - jv[field.fieldName] = mpt.getJson(JsonOptions::none); + setMPTFields(field, jv); test(jv, field.fieldName); }; - ammBid(sfBidMin); - ammBid(sfBidMax); + for (SField const& field : + {toSFieldRef(sfBidMin), + toSFieldRef(sfBidMax), + toSFieldRef(sfAsset), + toSFieldRef(sfAsset2)}) + ammBid(field); // AMMClawback - { + auto ammClawback = [&](SField const& field) { Json::Value jv; jv[jss::TransactionType] = jss::AMMClawback; jv[jss::Account] = alice.human(); jv[jss::Holder] = carol.human(); - jv[jss::Asset] = to_json(xrpIssue()); - jv[jss::Asset2] = to_json(USD.issue()); - jv[jss::Amount] = mpt.getJson(JsonOptions::none); - test(jv, jss::Amount.c_str()); - } + setMPTFields(field, jv); + test(jv, field.fieldName); + }; + for (SField const& field : + {toSFieldRef(sfAmount), + toSFieldRef(sfAsset), + toSFieldRef(sfAsset2)}) + ammClawback(field); + // AMMDelete + auto ammDelete = [&](SField const& field) { + Json::Value jv; + jv[jss::TransactionType] = jss::AMMDelete; + jv[jss::Account] = alice.human(); + setMPTFields(field, jv, false); + test(jv, field.fieldName); + }; + ammDelete(sfAsset); + ammDelete(sfAsset2); + // AMMVote + auto ammVote = [&](SField const& field) { + Json::Value jv; + jv[jss::TransactionType] = jss::AMMVote; + jv[jss::Account] = alice.human(); + jv[jss::TradingFee] = 100; + setMPTFields(field, jv, false); + test(jv, field.fieldName); + }; + ammVote(sfAsset); + ammVote(sfAsset2); // CheckCash auto checkCash = [&](SField const& field) { Json::Value jv; diff --git a/src/xrpld/app/ledger/OrderBookDB.cpp b/src/xrpld/app/ledger/OrderBookDB.cpp index d0eddadbacb..802d1507f31 100644 --- a/src/xrpld/app/ledger/OrderBookDB.cpp +++ b/src/xrpld/app/ledger/OrderBookDB.cpp @@ -129,8 +129,8 @@ OrderBookDB::update(std::shared_ptr const& ledger) } else if (sle->getType() == ltAMM) { - auto const issue1 = (*sle)[sfAsset]; - auto const issue2 = (*sle)[sfAsset2]; + auto const issue1 = (*sle)[sfAsset].get(); + auto const issue2 = (*sle)[sfAsset2].get(); auto addBook = [&](Issue const& in, Issue const& out) { allBooks[in].insert(out); diff --git a/src/xrpld/app/misc/detail/AMMUtils.cpp b/src/xrpld/app/misc/detail/AMMUtils.cpp index efc80cf17b6..e305db53067 100644 --- a/src/xrpld/app/misc/detail/AMMUtils.cpp +++ b/src/xrpld/app/misc/detail/AMMUtils.cpp @@ -51,8 +51,8 @@ ammHolds( beast::Journal const j) { auto const issues = [&]() -> std::optional> { - auto const issue1 = ammSle[sfAsset]; - auto const issue2 = ammSle[sfAsset2]; + auto const issue1 = ammSle[sfAsset].get(); + auto const issue2 = ammSle[sfAsset2].get(); if (optIssue1 && optIssue2) { if (invalidAMMAssetPair( @@ -134,8 +134,8 @@ ammLPHolds( { return ammLPHolds( view, - ammSle[sfAsset].currency, - ammSle[sfAsset2].currency, + ammSle[sfAsset].get().currency, + ammSle[sfAsset2].get().currency, ammSle[sfAccount], lpAccount, j); diff --git a/src/xrpld/app/tx/detail/AMMBid.cpp b/src/xrpld/app/tx/detail/AMMBid.cpp index 9de3762d2e3..7b5a7e002e2 100644 --- a/src/xrpld/app/tx/detail/AMMBid.cpp +++ b/src/xrpld/app/tx/detail/AMMBid.cpp @@ -46,7 +46,8 @@ AMMBid::preflight(PreflightContext const& ctx) return temINVALID_FLAG; } - if (auto const res = invalidAMMAssetPair(ctx.tx[sfAsset], ctx.tx[sfAsset2])) + if (auto const res = invalidAMMAssetPair( + ctx.tx[sfAsset].get(), ctx.tx[sfAsset2].get())) { JLOG(ctx.j.debug()) << "AMM Bid: Invalid asset pair."; return res; diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 64150ded6da..cc7a0bcde4b 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -56,7 +56,7 @@ AMMClawback::preflight(PreflightContext const& ctx) } std::optional const clawAmount = ctx.tx[~sfAmount]; - auto const asset = ctx.tx[sfAsset]; + auto const asset = ctx.tx[sfAsset].get(); if (isXRP(asset)) return temMALFORMED; @@ -68,7 +68,7 @@ AMMClawback::preflight(PreflightContext const& ctx) return temMALFORMED; } - if (clawAmount && clawAmount->issue() != asset) + if (clawAmount && clawAmount->get() != asset) { JLOG(ctx.j.trace()) << "AMMClawback: Amount's issuer/currency subfield " "does not match Asset field"; @@ -84,8 +84,8 @@ AMMClawback::preflight(PreflightContext const& ctx) TER AMMClawback::preclaim(PreclaimContext const& ctx) { - auto const asset = ctx.tx[sfAsset]; - auto const asset2 = ctx.tx[sfAsset2]; + auto const asset = ctx.tx[sfAsset].get(); + auto const asset2 = ctx.tx[sfAsset2].get(); auto const sleIssuer = ctx.view.read(keylet::account(ctx.tx[sfAccount])); if (!sleIssuer) return terNO_ACCOUNT; // LCOV_EXCL_LINE @@ -138,8 +138,8 @@ AMMClawback::applyGuts(Sandbox& sb) std::optional const clawAmount = ctx_.tx[~sfAmount]; AccountID const issuer = ctx_.tx[sfAccount]; AccountID const holder = ctx_.tx[sfHolder]; - Issue const asset = ctx_.tx[sfAsset]; - Issue const asset2 = ctx_.tx[sfAsset2]; + Issue const asset = ctx_.tx[sfAsset].get(); + Issue const asset2 = ctx_.tx[sfAsset2].get(); auto ammSle = sb.peek(keylet::amm(asset, asset2)); if (!ammSle) diff --git a/src/xrpld/app/tx/detail/AMMDelete.cpp b/src/xrpld/app/tx/detail/AMMDelete.cpp index 89ce34052d2..430ac17e87b 100644 --- a/src/xrpld/app/tx/detail/AMMDelete.cpp +++ b/src/xrpld/app/tx/detail/AMMDelete.cpp @@ -72,8 +72,8 @@ AMMDelete::doApply() // as we go on processing transactions. Sandbox sb(&ctx_.view()); - auto const ter = - deleteAMMAccount(sb, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_); + auto const ter = deleteAMMAccount( + sb, ctx_.tx[sfAsset].get(), ctx_.tx[sfAsset2].get(), j_); if (ter == tesSUCCESS || ter == tecINCOMPLETE) sb.apply(ctx_.rawView()); diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index 3448401eb79..5d95945a277 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -100,8 +100,8 @@ AMMDeposit::preflight(PreflightContext const& ctx) return temMALFORMED; } - auto const asset = ctx.tx[sfAsset]; - auto const asset2 = ctx.tx[sfAsset2]; + auto const asset = ctx.tx[sfAsset].get(); + auto const asset2 = ctx.tx[sfAsset2].get(); if (auto const res = invalidAMMAssetPair(asset, asset2)) { JLOG(ctx.j.debug()) << "AMM Deposit: invalid asset pair."; @@ -268,10 +268,10 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) return tesSUCCESS; }; - if (auto const ter = checkAsset(ctx.tx[sfAsset])) + if (auto const ter = checkAsset(ctx.tx[sfAsset].get())) return ter; - if (auto const ter = checkAsset(ctx.tx[sfAsset2])) + if (auto const ter = checkAsset(ctx.tx[sfAsset2].get())) return ter; } diff --git a/src/xrpld/app/tx/detail/AMMVote.cpp b/src/xrpld/app/tx/detail/AMMVote.cpp index c4b6c612c63..31b3e8a04e7 100644 --- a/src/xrpld/app/tx/detail/AMMVote.cpp +++ b/src/xrpld/app/tx/detail/AMMVote.cpp @@ -38,7 +38,8 @@ AMMVote::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (auto const res = invalidAMMAssetPair(ctx.tx[sfAsset], ctx.tx[sfAsset2])) + if (auto const res = invalidAMMAssetPair( + ctx.tx[sfAsset].get(), ctx.tx[sfAsset2].get())) { JLOG(ctx.j.debug()) << "AMM Vote: invalid asset pair."; return res; diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 118262905c1..d840933d620 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -100,8 +100,8 @@ AMMWithdraw::preflight(PreflightContext const& ctx) return temMALFORMED; } - auto const asset = ctx.tx[sfAsset]; - auto const asset2 = ctx.tx[sfAsset2]; + auto const asset = ctx.tx[sfAsset].get(); + auto const asset2 = ctx.tx[sfAsset2].get(); if (auto const res = invalidAMMAssetPair(asset, asset2)) { JLOG(ctx.j.debug()) << "AMM Withdraw: Invalid asset pair."; @@ -418,7 +418,12 @@ AMMWithdraw::applyGuts(Sandbox& sb) return {result, false}; auto const res = deleteAMMAccountIfEmpty( - sb, ammSle, newLPTokenBalance, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_); + sb, + ammSle, + newLPTokenBalance, + ctx_.tx[sfAsset].get(), + ctx_.tx[sfAsset2].get(), + j_); // LCOV_EXCL_START if (!res.second) return {res.first, false}; diff --git a/src/xrpld/rpc/handlers/AMMInfo.cpp b/src/xrpld/rpc/handlers/AMMInfo.cpp index 9d5b20f1d63..8f1e9e2c5dc 100644 --- a/src/xrpld/rpc/handlers/AMMInfo.cpp +++ b/src/xrpld/rpc/handlers/AMMInfo.cpp @@ -162,8 +162,8 @@ doAMMInfo(RPC::JsonContext& context) return Unexpected(rpcACT_NOT_FOUND); if (!issue1 && !issue2) { - issue1 = (*amm)[sfAsset]; - issue2 = (*amm)[sfAsset2]; + issue1 = (*amm)[sfAsset].get(); + issue2 = (*amm)[sfAsset2].get(); } return ValuesFromContextParams{ From 8f3bbc4a2c7a8d82f8d8cc7ec9dbba188b4e462e Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Fri, 22 Nov 2024 13:14:02 -0500 Subject: [PATCH 02/10] Don't need to check/throw if not Issue. It's already done in get<>(). --- src/libxrpl/protocol/Indexes.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libxrpl/protocol/Indexes.cpp b/src/libxrpl/protocol/Indexes.cpp index d9fec8ab509..f592a99856f 100644 --- a/src/libxrpl/protocol/Indexes.cpp +++ b/src/libxrpl/protocol/Indexes.cpp @@ -414,8 +414,6 @@ nft_sells(uint256 const& id) noexcept Keylet amm(Asset const& issue1, Asset const& issue2) noexcept { - if (!issue1.holds() || !issue2.holds()) - Throw("Asset doesn't hold issue"); auto const& [minI, maxI] = std::minmax(issue1.get(), issue2.get()); return amm(indexHash( From ea1ecc4a69ae82880aae9cb85b019bf4d45db52a Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Tue, 10 Dec 2024 07:09:53 -0500 Subject: [PATCH 03/10] Address reviewer's feedback. --- include/xrpl/protocol/Asset.h | 28 +++++++++++----------------- include/xrpl/protocol/SOTemplate.h | 1 + include/xrpl/protocol/STIssue.h | 2 +- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/include/xrpl/protocol/Asset.h b/include/xrpl/protocol/Asset.h index 62afb6aee94..f33494c70dd 100644 --- a/include/xrpl/protocol/Asset.h +++ b/include/xrpl/protocol/Asset.h @@ -99,11 +99,8 @@ class Asset friend constexpr bool operator==(Asset const& lhs, Asset const& rhs); - friend constexpr bool - operator!=(Asset const& lhs, Asset const& rhs); - - friend constexpr bool - operator<(Asset const& lhs, Asset const& rhs); + friend constexpr std::weak_ordering + operator<=>(Asset const& lhs, Asset const& rhs); friend constexpr bool operator==(Currency const& lhs, Asset const& rhs); @@ -161,22 +158,19 @@ 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) +constexpr std::weak_ordering +operator<=>(Asset const& lhs, Asset const& rhs) { return std::visit( - [&]( - TLhs const& issLhs, TRhs const& issRhs) { + []( + TLhs const& lhs_, TRhs const& rhs_) { if constexpr (std::is_same_v) - return issLhs < issRhs; + return std::weak_ordering(lhs_ <=> rhs_); + else if constexpr ( + std::is_same_v && std::is_same_v) + return std::weak_ordering::greater; else - return false; + return std::weak_ordering::less; }, lhs.issue_, rhs.issue_); diff --git a/include/xrpl/protocol/SOTemplate.h b/include/xrpl/protocol/SOTemplate.h index 547ca302a59..b86fb63b858 100644 --- a/include/xrpl/protocol/SOTemplate.h +++ b/include/xrpl/protocol/SOTemplate.h @@ -72,6 +72,7 @@ class SOElement { init(fieldName); } + template requires(std::is_same_v || std::is_same_v) SOElement( diff --git a/include/xrpl/protocol/STIssue.h b/include/xrpl/protocol/STIssue.h index e97c2ab38ae..c9a5245eb84 100644 --- a/include/xrpl/protocol/STIssue.h +++ b/include/xrpl/protocol/STIssue.h @@ -101,7 +101,7 @@ template bool STIssue::holds() const { - return std::holds_alternative(asset_.value()); + return asset_.holds(); } template From 44029511f2f1e17f8c9c9338ebba31be0f0c26eb Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Tue, 10 Dec 2024 10:36:38 -0500 Subject: [PATCH 04/10] Address reviewer's feedback. --- include/xrpl/protocol/Asset.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/xrpl/protocol/Asset.h b/include/xrpl/protocol/Asset.h index f33494c70dd..0d12cd40580 100644 --- a/include/xrpl/protocol/Asset.h +++ b/include/xrpl/protocol/Asset.h @@ -44,8 +44,10 @@ concept AssetType = */ class Asset { -private: +public: using value_type = std::variant; + +private: value_type issue_; public: From cc4a9c0508641beeba90d0e0fc80a948f3492f38 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Tue, 10 Dec 2024 16:18:32 -0500 Subject: [PATCH 05/10] Address reviewer's feedback. --- include/xrpl/protocol/Issue.h | 5 ++++- include/xrpl/protocol/MPTIssue.h | 19 +++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/include/xrpl/protocol/Issue.h b/include/xrpl/protocol/Issue.h index 60182698ee6..83ef337c357 100644 --- a/include/xrpl/protocol/Issue.h +++ b/include/xrpl/protocol/Issue.h @@ -58,6 +58,9 @@ class Issue bool native() const; + + friend constexpr std::weak_ordering + operator<=>(Issue const& lhs, Issue const& rhs); }; bool @@ -95,7 +98,7 @@ operator==(Issue const& lhs, Issue const& rhs) /** Strict weak ordering. */ /** @{ */ -[[nodiscard]] inline constexpr std::weak_ordering +[[nodiscard]] constexpr std::weak_ordering operator<=>(Issue const& lhs, Issue const& rhs) { if (auto const c{lhs.currency <=> rhs.currency}; c != 0) diff --git a/include/xrpl/protocol/MPTIssue.h b/include/xrpl/protocol/MPTIssue.h index 46f28dd87e1..028051ab1ae 100644 --- a/include/xrpl/protocol/MPTIssue.h +++ b/include/xrpl/protocol/MPTIssue.h @@ -51,8 +51,11 @@ class MPTIssue void setJson(Json::Value& jv) const; - auto - operator<=>(MPTIssue const&) const = default; + friend constexpr bool + operator==(MPTIssue const& lhs, MPTIssue const& rhs); + + friend constexpr std::weak_ordering + operator<=>(MPTIssue const& lhs, MPTIssue const& rhs); bool native() const @@ -61,6 +64,18 @@ class MPTIssue } }; +constexpr bool +operator==(MPTIssue const& lhs, MPTIssue const& rhs) +{ + return lhs.mptID_ == rhs.mptID_; +} + +constexpr std::weak_ordering +operator<=>(MPTIssue const& lhs, MPTIssue const& rhs) +{ + return lhs.mptID_ <=> rhs.mptID_; +} + /** MPT is a non-native token. */ inline bool From 10ffdf1d0c42a3c8ac2671060a655ced691c558b Mon Sep 17 00:00:00 2001 From: gregtatcam Date: Wed, 11 Dec 2024 11:45:24 -0500 Subject: [PATCH 06/10] Address reviewer's feedback --- include/xrpl/protocol/STIssue.h | 37 +++++++++++++++------------------ 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/include/xrpl/protocol/STIssue.h b/include/xrpl/protocol/STIssue.h index c9a5245eb84..f5c183fc0ce 100644 --- a/include/xrpl/protocol/STIssue.h +++ b/include/xrpl/protocol/STIssue.h @@ -76,6 +76,15 @@ class STIssue final : public STBase, CountedObject bool isDefault() const override; + friend constexpr std::weak_ordering + operator<=>(STIssue const& lhs, STIssue const& rhs); + + friend constexpr bool + operator==(STIssue const& lhs, Asset const& rhs); + + friend constexpr std::weak_ordering + operator<=>(STIssue const& lhs, Asset const& rhs); + private: STBase* copy(std::size_t n, void* buf) const override; @@ -129,34 +138,22 @@ STIssue::setIssue(Asset const& asset) asset_ = asset; } -inline bool -operator==(STIssue const& lhs, STIssue const& rhs) -{ - return lhs.value() == rhs.value(); -} - -inline bool -operator!=(STIssue const& lhs, STIssue const& rhs) -{ - return !operator==(lhs, rhs); -} - -inline bool -operator<(STIssue const& lhs, STIssue const& rhs) +constexpr std::weak_ordering +operator<=>(STIssue const& lhs, STIssue const& rhs) { - return lhs.value() < rhs.value(); + return lhs.asset_ <=> rhs.asset_; } -inline bool +constexpr bool operator==(STIssue const& lhs, Asset const& rhs) { - return lhs.value() == rhs; + return lhs.asset_ == rhs; } -inline bool -operator<(STIssue const& lhs, Asset const& rhs) +constexpr std::weak_ordering +operator<=>(STIssue const& lhs, Asset const& rhs) { - return lhs.value() < rhs; + return lhs.asset_ <=> rhs; } } // namespace ripple From cca71333ff9a701388f8474adb30a777feb2357a Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Wed, 11 Dec 2024 13:01:39 -0500 Subject: [PATCH 07/10] Apply suggestions from code review Co-authored-by: Bronek Kozicki --- src/libxrpl/protocol/STIssue.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libxrpl/protocol/STIssue.cpp b/src/libxrpl/protocol/STIssue.cpp index 7fc5074caa7..1d7d71b0152 100644 --- a/src/libxrpl/protocol/STIssue.cpp +++ b/src/libxrpl/protocol/STIssue.cpp @@ -58,7 +58,10 @@ STIssue::STIssue(SerialIter& sit, SField const& name) : STBase{name} if (noAccount() == account) { uint192 mptID; + MPTID mptID; std::uint32_t sequence = sit.get32(); + static_assert( + MPTID::size() == sizeof(sequence) + sizeof(currencyOrAccount)); memcpy(mptID.data(), &sequence, sizeof(sequence)); memcpy( mptID.data() + sizeof(sequence), @@ -108,17 +111,18 @@ STIssue::add(Serializer& s) const s.addBitString(asset_.get().currency); if (!isXRP(asset_.get().currency)) s.addBitString(asset_.get().account); + auto const& issue = asset_.get(); + s.addBitString(issue.currency); + if (!isXRP(issue.currency)) + s.addBitString(issue.account); } else { - s.addBitString(asset_.get().getIssuer()); + auto const& issue = asset_.get(); + s.addBitString(issue.getIssuer()); s.addBitString(noAccount()); std::uint32_t sequence; - memcpy( - &sequence, - asset_.get().getMptID().data(), - sizeof(sequence)); - s.add32(sequence); + memcpy(&sequence, issue.getMptID().data(), sizeof(sequence)); } } From 1ae94a818532381d9f5a2354d045229131d328a7 Mon Sep 17 00:00:00 2001 From: gregtatcam Date: Wed, 11 Dec 2024 13:24:34 -0500 Subject: [PATCH 08/10] Address reviewer's feedback --- src/libxrpl/protocol/STIssue.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libxrpl/protocol/STIssue.cpp b/src/libxrpl/protocol/STIssue.cpp index 1d7d71b0152..5a2008cb178 100644 --- a/src/libxrpl/protocol/STIssue.cpp +++ b/src/libxrpl/protocol/STIssue.cpp @@ -57,7 +57,6 @@ STIssue::STIssue(SerialIter& sit, SField const& name) : STBase{name} // MPT if (noAccount() == account) { - uint192 mptID; MPTID mptID; std::uint32_t sequence = sit.get32(); static_assert( @@ -108,9 +107,6 @@ STIssue::add(Serializer& s) const { if (holds()) { - s.addBitString(asset_.get().currency); - if (!isXRP(asset_.get().currency)) - s.addBitString(asset_.get().account); auto const& issue = asset_.get(); s.addBitString(issue.currency); if (!isXRP(issue.currency)) @@ -123,6 +119,7 @@ STIssue::add(Serializer& s) const s.addBitString(noAccount()); std::uint32_t sequence; memcpy(&sequence, issue.getMptID().data(), sizeof(sequence)); + s.add32(sequence); } } From 0e8c0621e8bc075ec0ce0453d2dd7eace2729817 Mon Sep 17 00:00:00 2001 From: gregtatcam Date: Wed, 11 Dec 2024 15:56:38 -0500 Subject: [PATCH 09/10] Address reviewer's feedback - add STIssue_test unit-test --- src/test/protocol/STIssue_test.cpp | 164 +++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 src/test/protocol/STIssue_test.cpp diff --git a/src/test/protocol/STIssue_test.cpp b/src/test/protocol/STIssue_test.cpp new file mode 100644 index 00000000000..4c9eeb0ba1b --- /dev/null +++ b/src/test/protocol/STIssue_test.cpp @@ -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. +*/ +//============================================================================== + +#include +#include +#include + +namespace ripple { +namespace test { + +class STIssue_test : public beast::unit_test::suite +{ +public: + void + testConstructor() + { + testcase("Constructor"); + using namespace jtx; + Account const alice{"alice"}; + auto const USD = alice["USD"]; + Issue issue; + + try + { + issue = xrpIssue(); + issue.account = alice; + STIssue stissue(sfAsset, Asset{issue}); + fail("Inconsistent XRP Issue doesn't fail"); + } + catch (...) + { + pass(); + } + + try + { + issue = USD; + issue.account = xrpAccount(); + STIssue stissue(sfAsset, Asset{issue}); + fail("Inconsistent IOU Issue doesn't fail"); + } + catch (...) + { + pass(); + } + + try + { + // Currency is USD but account is XRP + auto const data = + "00000000000000000000000055534400000000000000000000000000000000" + "000000000000000000"; + base_uint<320> uint; + (void)uint.parseHex(data); + SerialIter iter(Slice(uint.data(), uint.size())); + STIssue stissue(iter, sfAsset); + fail("Inconsistent IOU Issue doesn't fail on serializer"); + } + catch (...) + { + pass(); + } + + try + { + STIssue stissue(sfAsset, Asset{xrpIssue()}); + } + catch (...) + { + fail("XRP issue failed"); + } + + try + { + STIssue stissue(sfAsset, Asset{USD}); + } + catch (...) + { + fail("USD issue failed"); + } + + try + { + auto const data = + "0000000000000000000000005553440000000000ae123a8556f3cf91154711" + "376afb0f894f832b3d"; + base_uint<320> uint; + (void)uint.parseHex(data); + SerialIter iter(Slice(uint.data(), uint.size())); + STIssue stissue(iter, sfAsset); + BEAST_EXPECT(stissue.value() == USD); + } + catch (...) + { + fail("USD Issue fails on serializer"); + } + + try + { + auto const data = "0000000000000000000000000000000000000000"; + base_uint<160> uint; + (void)uint.parseHex(data); + SerialIter iter(Slice(uint.data(), uint.size())); + STIssue stissue(iter, sfAsset); + BEAST_EXPECT(stissue.value() == xrpCurrency()); + } + catch (...) + { + fail("XRP Issue fails on serializer"); + } + } + + void + testCompare() + { + testcase("Compare"); + using namespace jtx; + Account const alice{"alice"}; + auto const USD = alice["USD"]; + Asset const asset1{xrpIssue()}; + Asset const asset2{USD}; + Asset const asset3{MPTID{2}}; + + BEAST_EXPECT(STIssue(sfAsset, asset1) != asset2); + BEAST_EXPECT(STIssue(sfAsset, asset1) != asset3); + BEAST_EXPECT(STIssue(sfAsset, asset1) == asset1); + BEAST_EXPECT(STIssue(sfAsset, asset1).getText() == "XRP"); + BEAST_EXPECT( + STIssue(sfAsset, asset2).getText() == + "USD/rG1QQv2nh2gr7RCZ1P8YYcBUKCCN633jCn"); + BEAST_EXPECT( + STIssue(sfAsset, asset3).getText() == + "000000000000000000000000000000000000000000000002"); + } + + void + run() override + { + // compliments other unit tests to ensure complete coverage + testConstructor(); + testCompare(); + } +}; + +BEAST_DEFINE_TESTSUITE(STIssue, ripple_data, ripple); + +} // namespace test +} // namespace ripple \ No newline at end of file From 97f6d8a79df2cc581395e14173b195f20e8b798f Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Thu, 12 Dec 2024 07:58:19 -0500 Subject: [PATCH 10/10] Address reviewer's feedback. --- include/xrpl/protocol/STIssue.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/xrpl/protocol/STIssue.h b/include/xrpl/protocol/STIssue.h index f5c183fc0ce..08812c15aec 100644 --- a/include/xrpl/protocol/STIssue.h +++ b/include/xrpl/protocol/STIssue.h @@ -76,6 +76,9 @@ class STIssue final : public STBase, CountedObject bool isDefault() const override; + friend constexpr bool + operator==(STIssue const& lhs, STIssue const& rhs); + friend constexpr std::weak_ordering operator<=>(STIssue const& lhs, STIssue const& rhs); @@ -138,6 +141,12 @@ STIssue::setIssue(Asset const& asset) asset_ = asset; } +constexpr bool +operator==(STIssue const& lhs, STIssue const& rhs) +{ + return lhs.asset_ == rhs.asset_; +} + constexpr std::weak_ordering operator<=>(STIssue const& lhs, STIssue const& rhs) {