diff --git a/RELEASENOTES.md b/RELEASENOTES.md index f1710cf97..856fa7886 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -2,13 +2,34 @@ ![XRP](docs/images/xrp-text-mark-black-small@2x.png) -This document contains the release notes for `rippled`, the reference server implementation of the Ripple protocol. To learn more about how to build, run or update a `rippled` server, visit https://xrpl.org/install-rippled.html +This document contains the release notes for `rippled`, the reference server implementation of the XRP Ledger protocol. To learn more about how to build, run or update a `rippled` server, visit https://xrpl.org/install-rippled.html Have new ideas? Need help with setting up your node? Come visit us [here](https://github.com/ripple/rippled/issues/new/choose) # Releases +## Version 1.7.2 + +This the 1.7.2 release of rippled, the reference server implementation of the XRP Ledger protocol. This release protects against the security issue [CVE-2021-3499](https://www.openssl.org/news/secadv/20210325.txt) affecting OpenSSL, adds an amendment to fix an issue with small offers not being properly removed from order books in some cases, and includes various other minor fixes. +Version 1.7.2 supersedes version 1.7.1 and adds fixes for more issues that were discovered during the release cycle. + +## Action Required + +This release introduces a new amendment to the XRP Ledger protocol: `fixRmSmallIncreasedQOffers`. This amendments is now open for voting according to the XRP Ledger's amendment process, which enables protocol changes following two weeks of >80% support from trusted validators. +If you operate an XRP Ledger server, then you should upgrade to version 1.7.2 within two weeks, to ensure service continuity. The exact time that protocol changes take effect depends on the voting decisions of the decentralized network. +If you operate an XRP Ledger validator, please learn more about this amendment so you can make informed decisions about how your validator votes. If you take no action, your validator begins voting in favor of any new amendments as soon as it has been upgraded. + +### Bug Fixes + +- **fixRmSmallIncreasedQOffers Amendment:** This amendment fixes an issue where certain small offers can be left at the tip of an order book without being consumed or removed when appropriate and causes some payments and Offers to fail when they should have succeeded [(#3827)](https://github.com/ripple/rippled/pull/3827). +- **Adjust OpenSSL defaults and mitigate CVE-2021-3499:** Prior to this fix, servers compiled against a vulnerable version of OpenSSL could have a crash triggered by a malicious network connection. This fix disables renegotiation support in OpenSSL so that the rippled server is not vulnerable to this bug regardless of the OpenSSL version used to compile the server. This also removes support for deprecated TLS versions 1.0 and 1.1 and ciphers that are not part of TLS 1.2 [(#79e69da)](https://github.com/ripple/rippled/pull/3843/commits/79e69da3647019840dca49622621c3d88bc3883f). +- **Support HTTP health check in reporting mode:** Enables the Health Check special method when running the server in the new Reporting Mode introduced in 1.7.0 [(9c8cadd)](https://github.com/ripple/rippled/pull/3843/commits/9c8caddc5a197bdd642556f8beb14f06d53cdfd3). +- **Maintain compatibility for forwarded RPC responses:** Fixes a case in API responses from servers in Reporting Mode, where requests that were forwarded to a P2P-mode server would have the result field nested inside another result field [(8579eb0)](https://github.com/ripple/rippled/pull/3843/commits/8579eb0c191005022dcb20641444ab471e277f67). +- **Add load_factor in reporting mode:** Adds a load_factor value to the server info method response when running the server in Reporting Mode so that the response is compatible with the format returned by servers in P2P mode (the default) [(430802c)](https://github.com/ripple/rippled/pull/3843/commits/430802c1cf6d4179f2249a30bfab9eff8e1fa748). +- **Properly encode metadata from tx RPC command:** Fixes a problem where transaction metadata in the tx API method response would be in JSON format even when binary was requested [(7311629)](https://github.com/ripple/rippled/pull/3843/commits/73116297aa94c4acbfc74c2593d1aa2323b4cc52). +- **Updates to Windows builds:** When building on Windows, use vcpkg 2021 by default and add compatibility with MSVC 2019 [(36fe196)](https://github.com/ripple/rippled/pull/3843/commits/36fe1966c3cd37f668693b5d9910fab59c3f8b1f), [(30fd458)](https://github.com/ripple/rippled/pull/3843/commits/30fd45890b1d3d5f372a2091d1397b1e8e29d2ca). + ## Version 1.7.0 Ripple has released version 1.7.0 of `rippled`, the reference server implementation of the XRP Ledger protocol. diff --git a/bin/sh/install-vcpkg.sh b/bin/sh/install-vcpkg.sh index b77982987..8cf8f2d08 100755 --- a/bin/sh/install-vcpkg.sh +++ b/bin/sh/install-vcpkg.sh @@ -20,7 +20,7 @@ else if [[ -d "${VCPKG_DIR}" ]] ; then rm -rf "${VCPKG_DIR}" fi - git clone --branch 2019.12 https://github.com/Microsoft/vcpkg.git ${VCPKG_DIR} + git clone --branch 2021.04.30 https://github.com/Microsoft/vcpkg.git ${VCPKG_DIR} pushd ${VCPKG_DIR} BSARGS=() if [[ "$(uname)" == "Darwin" ]] ; then diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index e3c54e803..4ea804ba5 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -300,6 +300,25 @@ bool LedgerMaster::isCaughtUp(std::string& reason) { using namespace std::chrono_literals; + +#ifdef RIPPLED_REPORTING + if (app_.config().reporting()) + { + auto age = PgQuery(app_.getPgPool())("SELECT age()"); + if (!age || age.isNull()) + { + reason = "No ledgers in database"; + return false; + } + if (std::chrono::seconds{age.asInt()} > 3min) + { + reason = "No recently-published ledger"; + return false; + } + return true; + } +#endif + if (getPublishedLedgerAge() > 3min) { reason = "No recently-published ledger"; diff --git a/src/ripple/app/tx/impl/OfferStream.cpp b/src/ripple/app/tx/impl/OfferStream.cpp index 370ea46c5..58fd209ca 100644 --- a/src/ripple/app/tx/impl/OfferStream.cpp +++ b/src/ripple/app/tx/impl/OfferStream.cpp @@ -19,6 +19,7 @@ #include #include +#include namespace ripple { @@ -132,6 +133,73 @@ accountFundsHelper( view, id, issue.currency, issue.account, freezeHandling, j)); } +template +template +bool +TOfferStreamBase::shouldRmSmallIncreasedQOffer() const +{ + static_assert( + std::is_same_v || + std::is_same_v, + "STAmount is not supported"); + + static_assert( + std::is_same_v || + std::is_same_v, + "STAmount is not supported"); + + static_assert( + !std::is_same_v || + !std::is_same_v, + "Cannot have XRP/XRP offers"); + + if (!view_.rules().enabled(fixRmSmallIncreasedQOffers)) + return false; + + // Consider removing the offer if: + // o `TakerPays` is XRP (because of XRP drops granularity) or + // o `TakerPays` and `TakerGets` are both IOU and `TakerPays`<`TakerGets` + constexpr bool const inIsXRP = std::is_same_v; + constexpr bool const outIsXRP = std::is_same_v; + + if constexpr (outIsXRP) + { + // If `TakerGets` is XRP, the worst this offer's quality can change is + // to about 10^-81 `TakerPays` and 1 drop `TakerGets`. This will be + // remarkably good quality for any realistic asset, so these offers + // don't need this extra check. + return false; + } + + TAmounts const ofrAmts{ + toAmount(offer_.amount().in), + toAmount(offer_.amount().out)}; + + if constexpr (!inIsXRP && !outIsXRP) + { + if (ofrAmts.in >= ofrAmts.out) + return false; + } + + TTakerGets const ownerFunds = toAmount(*ownerFunds_); + + auto const effectiveAmounts = [&] { + if (offer_.owner() != offer_.issueOut().account && + ownerFunds < ofrAmts.out) + { + // adjust the amounts by owner funds + return offer_.quality().ceil_out(ofrAmts, ownerFunds); + } + return ofrAmts; + }(); + + if (effectiveAmounts.in > TTakerPays::minPositiveAmount()) + return false; + + Quality const effectiveQuality{effectiveAmounts}; + return effectiveQuality < offer_.quality(); +} + template bool TOfferStreamBase::step() @@ -222,6 +290,68 @@ TOfferStreamBase::step() << "Removing became unfunded offer " << entry->key(); } offer_ = TOffer{}; + // See comment at top of loop for how the offer is removed + continue; + } + + bool const rmSmallIncreasedQOffer = [&] { + bool const inIsXRP = isXRP(offer_.issueIn()); + bool const outIsXRP = isXRP(offer_.issueOut()); + if (inIsXRP && !outIsXRP) + { + // Without the `if constexpr`, the + // `shouldRmSmallIncreasedQOffer` template will be instantiated + // even if it is never used. This can cause compiler errors in + // some cases, hence the `if constexpr` guard. + // Note that TIn can be XRPAmount or STAmount, and TOut can be + // IOUAmount or STAmount. + if constexpr (!(std::is_same_v || + std::is_same_v)) + return shouldRmSmallIncreasedQOffer(); + } + if (!inIsXRP && outIsXRP) + { + // See comment above for `if constexpr` rationale + if constexpr (!(std::is_same_v || + std::is_same_v)) + return shouldRmSmallIncreasedQOffer(); + } + if (!inIsXRP && !outIsXRP) + { + // See comment above for `if constexpr` rationale + if constexpr (!(std::is_same_v || + std::is_same_v)) + return shouldRmSmallIncreasedQOffer(); + } + assert(0); // xrp/xrp offer!?! should never happen + return false; + }(); + + if (rmSmallIncreasedQOffer) + { + auto const original_funds = accountFundsHelper( + cancelView_, + offer_.owner(), + amount.out, + offer_.issueOut(), + fhZERO_IF_FROZEN, + j_); + + if (original_funds == *ownerFunds_) + { + permRmOffer(entry->key()); + JLOG(j_.trace()) + << "Removing tiny offer due to reduced quality " + << entry->key(); + } + else + { + JLOG(j_.trace()) << "Removing tiny offer that became tiny due " + "to reduced quality " + << entry->key(); + } + offer_ = TOffer{}; + // See comment at top of loop for how the offer is removed continue; } diff --git a/src/ripple/app/tx/impl/OfferStream.h b/src/ripple/app/tx/impl/OfferStream.h index 69946790c..9472bc4e7 100644 --- a/src/ripple/app/tx/impl/OfferStream.h +++ b/src/ripple/app/tx/impl/OfferStream.h @@ -85,6 +85,10 @@ protected: virtual void permRmOffer(uint256 const& offerIndex) = 0; + template + bool + shouldRmSmallIncreasedQOffer() const; + public: TOfferStreamBase( ApplyView& view, diff --git a/src/ripple/basics/IOUAmount.h b/src/ripple/basics/IOUAmount.h index 46a37d23a..7e9e50d7e 100644 --- a/src/ripple/basics/IOUAmount.h +++ b/src/ripple/basics/IOUAmount.h @@ -129,6 +129,9 @@ public: { return mantissa_; } + + static IOUAmount + minPositiveAmount(); }; std::string diff --git a/src/ripple/basics/XRPAmount.h b/src/ripple/basics/XRPAmount.h index c9f0d08ea..08f82b175 100644 --- a/src/ripple/basics/XRPAmount.h +++ b/src/ripple/basics/XRPAmount.h @@ -238,6 +238,12 @@ public: s >> val.drops_; return s; } + + static XRPAmount + minPositiveAmount() + { + return XRPAmount{1}; + } }; /** Number of drops per 1 XRP */ diff --git a/src/ripple/basics/impl/IOUAmount.cpp b/src/ripple/basics/impl/IOUAmount.cpp index bb3c39ef3..272546623 100644 --- a/src/ripple/basics/impl/IOUAmount.cpp +++ b/src/ripple/basics/impl/IOUAmount.cpp @@ -34,6 +34,12 @@ static std::int64_t const maxMantissa = 9999999999999999ull; static int const minExponent = -96; static int const maxExponent = 80; +IOUAmount +IOUAmount::minPositiveAmount() +{ + return IOUAmount(minMantissa, minExponent); +} + void IOUAmount::normalize() { @@ -353,7 +359,7 @@ mulRatio( { if (!result) { - return IOUAmount(minMantissa, minExponent); + return IOUAmount::minPositiveAmount(); } // This addition cannot overflow because the mantissa is already // normalized diff --git a/src/ripple/basics/impl/make_SSLContext.cpp b/src/ripple/basics/impl/make_SSLContext.cpp index 80dc698b7..67beb79d3 100644 --- a/src/ripple/basics/impl/make_SSLContext.cpp +++ b/src/ripple/basics/impl/make_SSLContext.cpp @@ -29,13 +29,21 @@ namespace ripple { namespace openssl { namespace detail { -// We limit the ciphers we request and allow to ensure that weak -// ciphers aren't used. While this isn't strictly necessary for -// the rippled server-server use case, where we only need MITM -// detection/prevention, we also have websocket and rpc scenarios -// and want to ensure weak ciphers can't be used. -std::string const defaultCipherList = - "HIGH:MEDIUM:!aNULL:!MD5:!DSS:!3DES:!RC4:!EXPORT"; +/** The default list of ciphers we accept over TLS. + + Generally we include cipher suites that are part of TLS v1.2, but + we specifically exclude: + + - the DSS cipher suites (!DSS); + - cipher suites using pre-shared keys (!PSK); + - cipher suites that don't offer encryption (!eNULL); and + - cipher suites that don't offer authentication (!aNULL). + + @note Server administrators can override this default list, on either a + global or per-port basis, using the `ssl_ciphers` directive in the + config file. + */ +std::string const defaultCipherList = "TLSv1.2:!DSS:!PSK:!eNULL:!aNULL"; template struct custom_delete; @@ -194,69 +202,6 @@ ssl_ctx_use_privatekey(SSL_CTX* const ctx, evp_pkey_ptr key) LogicError("SSL_CTX_use_PrivateKey failed"); } -#ifdef SSL_FLAGS_NO_RENEGOTIATE_CIPHERS -static bool -disallowRenegotiation(SSL const* ssl, bool isNew) -{ - // Track when SSL connections have last negotiated and - // do not allow a connection to renegotiate more than - // once every 4 minutes - struct StaticData - { - std::mutex lock; - beast::aged_unordered_set set; - - StaticData() : set(ripple::stopwatch()) - { - } - }; - - static StaticData sd; - std::lock_guard lock(sd.lock); - auto const expired(sd.set.clock().now() - std::chrono::minutes(4)); - - // Remove expired entries - for (auto iter(sd.set.chronological.begin()); - (iter != sd.set.chronological.end()) && (iter.when() <= expired); - iter = sd.set.chronological.begin()) - { - sd.set.erase(iter); - } - - auto iter = sd.set.find(ssl); - if (iter != sd.set.end()) - { - if (!isNew) - { - // This is a renegotiation and the last negotiation was recent - return true; - } - - sd.set.touch(iter); - } - else - { - sd.set.emplace(ssl); - } - - return false; -} - -static void -info_handler(SSL const* ssl, int event, int) -{ -#if OPENSSL_VERSION_NUMBER < 0x10100000L - if ((ssl->s3) && (event & SSL_CB_HANDSHAKE_START)) - { - if (disallowRenegotiation(ssl, SSL_in_before(ssl))) - ssl->s3->flags |= SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS; - } -#else - // empty, flag removed in OpenSSL 1.1 -#endif -} -#endif - static std::string error_message(std::string const& what, boost::system::error_code const& ec) { @@ -388,7 +333,10 @@ get_context(std::string const& cipherList) boost::asio::ssl::context::default_workarounds | boost::asio::ssl::context::no_sslv2 | boost::asio::ssl::context::no_sslv3 | - boost::asio::ssl::context::single_dh_use); + boost::asio::ssl::context::no_tlsv1 | + boost::asio::ssl::context::no_tlsv1_1 | + boost::asio::ssl::context::single_dh_use | + boost::asio::ssl::context::no_compression); { auto const& l = !cipherList.empty() ? cipherList : defaultCipherList; @@ -435,9 +383,11 @@ get_context(std::string const& cipherList) SSL_CTX_set_tmp_dh(c->native_handle(), dh.get()); -#ifdef SSL_FLAGS_NO_RENEGOTIATE_CIPHERS - SSL_CTX_set_info_callback(c->native_handle(), info_handler); -#endif + // Disable all renegotiation support in TLS v1.2. This can help prevent + // exploitation of the bug described in CVE-2021-3499 (for details see + // https://www.openssl.org/news/secadv/20210325.txt) when linking against + // OpenSSL versions prior to 1.1.1k. + SSL_CTX_set_options(c->native_handle(), SSL_OP_NO_RENEGOTIATION); return c; } diff --git a/src/ripple/protocol/AmountConversions.h b/src/ripple/protocol/AmountConversions.h index 899d313fb..f60b976d5 100644 --- a/src/ripple/protocol/AmountConversions.h +++ b/src/ripple/protocol/AmountConversions.h @@ -63,11 +63,7 @@ toSTAmount(XRPAmount const& xrp, Issue const& iss) template T -toAmount(STAmount const& amt) -{ - static_assert(sizeof(T) == -1, "Must use specialized function"); - return T(0); -} +toAmount(STAmount const& amt) = delete; template <> inline STAmount @@ -102,6 +98,28 @@ toAmount(STAmount const& amt) return XRPAmount(sMant); } +template +T +toAmount(IOUAmount const& amt) = delete; + +template <> +inline IOUAmount +toAmount(IOUAmount const& amt) +{ + return amt; +} + +template +T +toAmount(XRPAmount const& amt) = delete; + +template <> +inline XRPAmount +toAmount(XRPAmount const& amt) +{ + return amt; +} + } // namespace ripple #endif diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 6864ff49a..2fbcc9789 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -116,6 +116,7 @@ class FeatureCollections "TicketBatch", "FlowSortStrands", "fixSTAmountCanonicalize", + "fixRmSmallIncreasedQOffers", }; std::vector features; @@ -376,6 +377,7 @@ extern uint256 const featureNegativeUNL; extern uint256 const featureTicketBatch; extern uint256 const featureFlowSortStrands; extern uint256 const fixSTAmountCanonicalize; +extern uint256 const fixRmSmallIncreasedQOffers; } // namespace ripple diff --git a/src/ripple/protocol/Quality.h b/src/ripple/protocol/Quality.h index 27026b674..6324a8836 100644 --- a/src/ripple/protocol/Quality.h +++ b/src/ripple/protocol/Quality.h @@ -132,6 +132,13 @@ public: /** Create a quality from the ratio of two amounts. */ explicit Quality(Amounts const& amount); + /** Create a quality from the ratio of two amounts. */ + template + Quality(TAmounts const& amount) + : Quality(Amounts(toSTAmount(amount.in), toSTAmount(amount.out))) + { + } + /** Create a quality from the ratio of two amounts. */ template Quality(Out const& out, In const& in) diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 87e739e2e..9f29dd1ba 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -135,6 +135,7 @@ detail::supportedAmendments() "TicketBatch", "FlowSortStrands", "fixSTAmountCanonicalize", + "fixRmSmallIncreasedQOffers", }; return supported; } @@ -190,7 +191,8 @@ uint256 const featureNegativeUNL = *getRegisteredFeature("NegativeUNL"), featureTicketBatch = *getRegisteredFeature("TicketBatch"), featureFlowSortStrands = *getRegisteredFeature("FlowSortStrands"), - fixSTAmountCanonicalize = *getRegisteredFeature("fixSTAmountCanonicalize"); + fixSTAmountCanonicalize = *getRegisteredFeature("fixSTAmountCanonicalize"), + fixRmSmallIncreasedQOffers = *getRegisteredFeature("fixRmSmallIncreasedQOffers"); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. diff --git a/src/ripple/rpc/handlers/ServerInfo.cpp b/src/ripple/rpc/handlers/ServerInfo.cpp index 174315c08..80fde471e 100644 --- a/src/ripple/rpc/handlers/ServerInfo.cpp +++ b/src/ripple/rpc/handlers/ServerInfo.cpp @@ -17,7 +17,9 @@ */ //============================================================================== +#include #include +#include #include #include #include @@ -38,6 +40,12 @@ doServerInfo(RPC::JsonContext& context) context.params.isMember(jss::counters) && context.params[jss::counters].asBool()); + if (context.app.config().reporting()) + { + Json::Value const proxied = forwardToP2p(context); + auto const lf = proxied[jss::result][jss::info][jss::load_factor]; + ret[jss::info][jss::load_factor] = lf.isNull() ? 1 : lf; + } return ret; } diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index e3ea56d30..d64d58bcb 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -903,6 +903,17 @@ ServerHandlerImp::processRequest( reply.append(std::move(r)); else reply = std::move(r); + + if (reply.isMember(jss::result) && + reply[jss::result].isMember(jss::result)) + { + reply = reply[jss::result]; + if (reply.isMember(jss::status)) + { + reply[jss::result][jss::status] = reply[jss::status]; + reply.removeMember(jss::status); + } + } } auto response = to_string(reply); diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index f30fc19e1..e0d2de4e3 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -320,17 +320,20 @@ public: Env env{*this, features}; env.fund(XRP(10000), alice, bob, carol, dan, erin, gw); + env.close(); env.trust(USD(1000), alice, bob, carol, dan, erin); + env.close(); env(pay(gw, carol, USD(0.99999))); env(pay(gw, dan, USD(1))); env(pay(gw, erin, USD(1))); + env.close(); // Carol doesn't quite have enough funds for this offer // The amount left after this offer is taken will cause // STAmount to incorrectly round to zero when the next offer - // (at a good quality) is considered. (when the - // stAmountCalcSwitchover2 patch is inactive) - env(offer(carol, drops(1), USD(1))); + // (at a good quality) is considered. (when the now removed + // stAmountCalcSwitchover2 patch was inactive) + env(offer(carol, drops(1), USD(0.99999))); // Offer at a quality poor enough so when the input xrp is // calculated in the reverse pass, the amount is not zero. env(offer(dan, XRP(100), USD(1))); @@ -340,9 +343,9 @@ public: // It is considered after the offer from carol, which leaves a // tiny amount left to pay. When calculating the amount of xrp // needed for this offer, it will incorrectly compute zero in both - // the forward and reverse passes (when the stAmountCalcSwitchover2 - // is inactive.) - env(offer(erin, drops(2), USD(2))); + // the forward and reverse passes (when the now removed + // stAmountCalcSwitchover2 was inactive.) + env(offer(erin, drops(2), USD(1))); env(pay(alice, bob, USD(1)), path(~USD), @@ -356,6 +359,317 @@ public: env.require(balance(erin, USD(0.99999)), offers(erin, 1)); } + void + testRmSmallIncreasedQOffersXRP(FeatureBitset features) + { + testcase("Rm small increased q offers XRP"); + + // Carol places an offer, but cannot fully fund the offer. When her + // funding is taken into account, the offer's quality drops below its + // initial quality and has an input amount of 1 drop. This is removed as + // an offer that may block offer books. + + using namespace jtx; + using namespace std::chrono_literals; + auto const alice = Account{"alice"}; + auto const bob = Account{"bob"}; + auto const carol = Account{"carol"}; + auto const gw = Account{"gw"}; + + auto const USD = gw["USD"]; + + // Test offer crossing + for (auto crossBothOffers : {false, true}) + { + Env env{*this, features}; + + env.fund(XRP(10000), alice, bob, carol, gw); + env.close(); + env.trust(USD(1000), alice, bob, carol); + // underfund carol's offer + auto initialCarolUSD = USD(0.499); + env(pay(gw, carol, initialCarolUSD)); + env(pay(gw, bob, USD(100))); + env.close(); + // This offer is underfunded + env(offer(carol, drops(1), USD(1))); + env.close(); + // offer at a lower quality + env(offer(bob, drops(2), USD(1), tfPassive)); + env.close(); + env.require(offers(bob, 1), offers(carol, 1)); + + // alice places an offer that crosses carol's; depending on + // "crossBothOffers" it may cross bob's as well + auto aliceTakerGets = crossBothOffers ? drops(2) : drops(1); + env(offer(alice, USD(1), aliceTakerGets)); + env.close(); + + if (features[fixRmSmallIncreasedQOffers]) + { + env.require( + offers(carol, 0), + balance( + carol, + initialCarolUSD)); // offer is removed but not taken + if (crossBothOffers) + { + env.require( + offers(alice, 0), + balance(alice, USD(1))); // alice's offer is crossed + } + else + { + env.require( + offers(alice, 1), + balance( + alice, USD(0))); // alice's offer is not crossed + } + } + else + { + env.require( + offers(alice, 1), + offers(bob, 1), + offers(carol, 1), + balance(alice, USD(0)), + balance( + carol, + initialCarolUSD)); // offer is not crossed at all + } + } + + // Test payments + for (auto partialPayment : {false, true}) + { + Env env{*this, features}; + + env.fund(XRP(10000), alice, bob, carol, gw); + env.close(); + env.trust(USD(1000), alice, bob, carol); + env.close(); + auto const initialCarolUSD = USD(0.999); + env(pay(gw, carol, initialCarolUSD)); + env.close(); + env(pay(gw, bob, USD(100))); + env.close(); + env(offer(carol, drops(1), USD(1))); + env.close(); + env(offer(bob, drops(2), USD(2), tfPassive)); + env.close(); + env.require(offers(bob, 1), offers(carol, 1)); + + std::uint32_t const flags = partialPayment + ? (tfNoRippleDirect | tfPartialPayment) + : tfNoRippleDirect; + + TER const expectedTer = + partialPayment ? TER{tesSUCCESS} : TER{tecPATH_PARTIAL}; + + env(pay(alice, bob, USD(5)), + path(~USD), + sendmax(XRP(1)), + txflags(flags), + ter(expectedTer)); + env.close(); + + if (features[fixRmSmallIncreasedQOffers]) + { + if (expectedTer == tesSUCCESS) + { + env.require(offers(carol, 0)); + env.require(balance( + carol, + initialCarolUSD)); // offer is removed but not taken + } + else + { + // TODO: Offers are not removed when payments fail + // If that is addressed, the test should show that carol's + // offer is removed but not taken, as in the other branch of + // this if statement + } + } + else + { + if (partialPayment) + { + env.require(offers(carol, 0)); + env.require( + balance(carol, USD(0))); // offer is removed and taken + } + else + { + // offer is not removed or taken + BEAST_EXPECT(isOffer(env, carol, drops(1), USD(1))); + } + } + } + } + + void + testRmSmallIncreasedQOffersIOU(FeatureBitset features) + { + testcase("Rm small increased q offers IOU"); + + // Carol places an offer, but cannot fully fund the offer. When her + // funding is taken into account, the offer's quality drops below its + // initial quality and has an input amount of 1 drop. This is removed as + // an offer that may block offer books. + + using namespace jtx; + using namespace std::chrono_literals; + auto const alice = Account{"alice"}; + auto const bob = Account{"bob"}; + auto const carol = Account{"carol"}; + auto const gw = Account{"gw"}; + + auto const USD = gw["USD"]; + auto const EUR = gw["EUR"]; + + auto tinyAmount = [&](IOU const& iou) -> PrettyAmount { + STAmount amt( + iou.issue(), + /*mantissa*/ 1, + /*exponent*/ -81); + return PrettyAmount(amt, iou.account.name()); + }; + + // Test offer crossing + for (auto crossBothOffers : {false, true}) + { + Env env{*this, features}; + + env.fund(XRP(10000), alice, bob, carol, gw); + env.close(); + env.trust(USD(1000), alice, bob, carol); + env.trust(EUR(1000), alice, bob, carol); + // underfund carol's offer + auto initialCarolUSD = tinyAmount(USD); + env(pay(gw, carol, initialCarolUSD)); + env(pay(gw, bob, USD(100))); + env(pay(gw, alice, EUR(100))); + env.close(); + // This offer is underfunded + env(offer(carol, EUR(1), USD(10))); + env.close(); + // offer at a lower quality + env(offer(bob, EUR(1), USD(5), tfPassive)); + env.close(); + env.require(offers(bob, 1), offers(carol, 1)); + + // alice places an offer that crosses carol's; depending on + // "crossBothOffers" it may cross bob's as well + // Whatever + auto aliceTakerGets = crossBothOffers ? EUR(0.2) : EUR(0.1); + env(offer(alice, USD(1), aliceTakerGets)); + env.close(); + + if (features[fixRmSmallIncreasedQOffers]) + { + env.require( + offers(carol, 0), + balance( + carol, + initialCarolUSD)); // offer is removed but not taken + if (crossBothOffers) + { + env.require( + offers(alice, 0), + balance(alice, USD(1))); // alice's offer is crossed + } + else + { + env.require( + offers(alice, 1), + balance( + alice, USD(0))); // alice's offer is not crossed + } + } + else + { + env.require( + offers(alice, 1), + offers(bob, 1), + offers(carol, 1), + balance(alice, USD(0)), + balance( + carol, + initialCarolUSD)); // offer is not crossed at all + } + } + + // Test payments + for (auto partialPayment : {false, true}) + { + Env env{*this, features}; + + env.fund(XRP(10000), alice, bob, carol, gw); + env.close(); + env.trust(USD(1000), alice, bob, carol); + env.trust(EUR(1000), alice, bob, carol); + env.close(); + // underfund carol's offer + auto const initialCarolUSD = tinyAmount(USD); + env(pay(gw, carol, initialCarolUSD)); + env(pay(gw, bob, USD(100))); + env(pay(gw, alice, EUR(100))); + env.close(); + // This offer is underfunded + env(offer(carol, EUR(1), USD(2))); + env.close(); + env(offer(bob, EUR(2), USD(4), tfPassive)); + env.close(); + env.require(offers(bob, 1), offers(carol, 1)); + + std::uint32_t const flags = partialPayment + ? (tfNoRippleDirect | tfPartialPayment) + : tfNoRippleDirect; + + TER const expectedTer = + partialPayment ? TER{tesSUCCESS} : TER{tecPATH_PARTIAL}; + + env(pay(alice, bob, USD(5)), + path(~USD), + sendmax(EUR(10)), + txflags(flags), + ter(expectedTer)); + env.close(); + + if (features[fixRmSmallIncreasedQOffers]) + { + if (expectedTer == tesSUCCESS) + { + env.require(offers(carol, 0)); + env.require(balance( + carol, + initialCarolUSD)); // offer is removed but not taken + } + else + { + // TODO: Offers are not removed when payments fail + // If that is addressed, the test should show that carol's + // offer is removed but not taken, as in the other branch of + // this if statement + } + } + else + { + if (partialPayment) + { + env.require(offers(carol, 0)); + env.require( + balance(carol, USD(0))); // offer is removed and taken + } + else + { + // offer is not removed or taken + BEAST_EXPECT(isOffer(env, carol, EUR(1), USD(2))); + } + } + } + } + void testEnforceNoRipple(FeatureBitset features) { @@ -5387,7 +5701,7 @@ public: { // An assert was falsely triggering when computing rates for offers. // This unit test would trigger that assert (which has been removed). - testcase("false assert"); + testcase("incorrect assert fixed"); using namespace jtx; Env env{*this}; @@ -5459,6 +5773,8 @@ public: testTickSize(features); testTicketOffer(features); testTicketCancelOffer(features); + testRmSmallIncreasedQOffersXRP(features); + testRmSmallIncreasedQOffersIOU(features); } void @@ -5468,10 +5784,12 @@ public: FeatureBitset const all{supported_amendments()}; FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval}; + FeatureBitset const rmSmallIncreasedQOffers{fixRmSmallIncreasedQOffers}; testAll(all - takerDryOffer); testAll(all - flowCross - takerDryOffer); testAll(all - flowCross); + testAll(all - rmSmallIncreasedQOffers); testAll(all); testFalseAssert(); }