diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index c1fca1bfa..602be87a6 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -815,6 +815,7 @@ enum class SBoxCmp same, dustDiff, offerDelDiff, + xrpRound, diff }; @@ -828,6 +829,8 @@ static std::string to_string (SBoxCmp c) return "dust diffs"; case SBoxCmp::offerDelDiff: return "offer del diffs"; + case SBoxCmp::xrpRound: + return "XRP round to zero"; case SBoxCmp::diff: return "different"; } @@ -846,6 +849,19 @@ static SBoxCmp compareSandboxes (char const* name, ApplyContext const& ctx, if (diff.hasDiff()) { using namespace beast::severities; + // There is a special case of an offer with XRP on one side where + // the XRP gets rounded to zero. It mostly looks like dust-level + // differences. It is easier to detect if we look for it before + // removing the dust differences. + if (int const side = diff.xrpRoundToZero()) + { + char const* const whichSide = side > 0 ? "; Flow" : "; Taker"; + j.stream (kWarning) << "FlowCross: " << name << " different" << + whichSide << " XRP rounded to zero. tx: " << + ctx.tx.getTransactionID(); + return SBoxCmp::xrpRound; + } + c = SBoxCmp::dustDiff; Severity s = kInfo; std::string diffDesc = ", but only dust."; diff --git a/src/ripple/ledger/CashDiff.h b/src/ripple/ledger/CashDiff.h index 13ccb8f25..0b5b9e33b 100644 --- a/src/ripple/ledger/CashDiff.h +++ b/src/ripple/ledger/CashDiff.h @@ -83,6 +83,21 @@ public: // Returns true is there are any differences to report. bool hasDiff() const; + // Checks for the XRP round-to-zero case. Returns zero if not detected. + // Otherwise returns -1 if seen on lhs, +1 if seen on rhs. + // + // For tiny offers of TakerPays IOU and TakerGets XRP, cases have been + // observed where XRP rounding allows a tiny amount of IOU to be + // removed from an Offer while returning no XRP to the offer owner. + // That's because the XRP amount was rounded down to zero drops. + // + // The person submitting the tiny offer does not, however, get something + // for nothing. The transaction's fee is significantly larger than the + // value of the received IOU. + // + // This check should be made before calling rmDust(). + int xrpRoundToZero() const; + // Remove dust-sized differences. Returns true is dust was removed. bool rmDust(); @@ -123,7 +138,7 @@ private: // If v1 and v2 have different issues, then their difference is never dust. // If v1 < v2, smallness is computed as v1 / (v2 - v1). // The e10 argument says at least how big that ratio must be. Default is 10^6. -// If both v1 and v2 are XRP, any difference of 2 or smaller is considered dust. +// If both v1 and v2 are XRP, consider any diff of 2 drops or less to be dust. bool diffIsDust (STAmount const& v1, STAmount const& v2, std::uint8_t e10 = 6); } // ripple diff --git a/src/ripple/ledger/impl/CashDiff.cpp b/src/ripple/ledger/impl/CashDiff.cpp index ac82513e8..e92cda586 100644 --- a/src/ripple/ledger/impl/CashDiff.cpp +++ b/src/ripple/ledger/impl/CashDiff.cpp @@ -330,6 +330,8 @@ public: || rhsDiffs_.hasDiff(); } + int xrpRoundToZero () const; + // Filter out differences that are small enough to be in the floating // point noise. bool rmDust (); @@ -424,6 +426,46 @@ countKeys (detail::CashSummary const& lhs, detail::CashSummary const& rhs) return ret; } +int CashDiff::Impl::xrpRoundToZero () const +{ + // The case has one OfferChange that is present on both lhs_ and rhs_. + // That OfferChange should have XRP for TakerGets. There should be a 1 + // drop difference between the TakerGets of lhsDiffs_ and rhsDiffs_. + if (lhsDiffs_.offerChanges.size() != 1 || + rhsDiffs_.offerChanges.size() != 1) + return 0; + + if (! lhsDiffs_.offerChanges[0].second.takerGets().native() || + ! rhsDiffs_.offerChanges[0].second.takerGets().native()) + return 0; + + bool const lhsBigger = + lhsDiffs_.offerChanges[0].second.takerGets().mantissa() > + rhsDiffs_.offerChanges[0].second.takerGets().mantissa(); + + detail::CashSummary const& bigger = lhsBigger ? lhsDiffs_ : rhsDiffs_; + detail::CashSummary const& smaller = lhsBigger ? rhsDiffs_ : lhsDiffs_; + if (bigger.offerChanges[0].second.takerGets().mantissa() - + smaller.offerChanges[0].second.takerGets().mantissa() != 1) + return 0; + + // The side with the smaller XRP balance in the OfferChange should have + // two XRP differences. The other side should have no XRP differences. + if (smaller.xrpChanges.size() != 2) + return 0; + if (! bigger.xrpChanges.empty()) + return 0; + + // There should be no other differences. + if (!smaller.trustChanges.empty() || !bigger.trustChanges.empty() || + !smaller.trustDeletions.empty() || !bigger.trustDeletions.empty() || + !smaller.offerDeletions.empty() || !bigger.offerDeletions.empty()) + return 0; + + // Return which side exhibited the problem. + return lhsBigger ? -1 : 1; +} + // Function that compares two CashDiff::OfferAmounts and returns true if // the difference is dust-sized. static bool diffIsDust ( @@ -627,6 +669,11 @@ bool CashDiff::hasDiff() const return impl_->hasDiff(); } +int CashDiff::xrpRoundToZero() const +{ + return impl_->xrpRoundToZero(); +} + bool CashDiff::rmDust() { return impl_->rmDust(); @@ -678,7 +725,7 @@ bool diffIsDust (STAmount const& v1, STAmount const& v2, std::uint8_t e10) return true; static_assert (sizeof (1ULL) == sizeof (std::uint64_t), ""); - std::uint64_t ratio = s / (l - s); + std::uint64_t const ratio = s / (l - s); static constexpr std::uint64_t e10Lookup[] { 1ULL,