Update the coverage

This commit is contained in:
Gregory Tsipenyuk
2026-07-08 09:27:22 -04:00
parent b247d2db29
commit f1290bfa4b
7 changed files with 113 additions and 9 deletions

View File

@@ -38,14 +38,21 @@ QualityFunction::outFromAvgQ(Quality const& quality)
return std::nullopt;
return out;
}
return std::nullopt;
// The sole caller (StrandFlow::limitOut) only invokes this on a non-const
// quality function, so m_ != 0 here, and a real payment/offer never yields
// a zero-rate limit quality (it would divide by zero above). This fallback
// is therefore unreachable in practice.
return std::nullopt; // LCOV_EXCL_LINE
}
bool
QualityFunction::satisfiesAvgQ(Quality const& quality, Number const& out) const
{
// satisfiesAvgQ is only reached from StrandFlow::limitOut *after*
// outFromAvgQ returned a value, which requires a non-zero rate. So a
// zero-rate quality never reaches here; this guard is defensive.
if (quality.rate() == beast::kZero)
return false;
return false; // LCOV_EXCL_LINE
return m_ * out + b_ >= 1 / quality.rate();
}

View File

@@ -1474,11 +1474,20 @@ roundNumberResult(
if (roundUp && !resultNegative && !result)
{
// Preserve existing mulRound/divRound behavior for a positive result
// too small to represent in the target asset.
// Intended to preserve existing mulRound/divRound behavior for a
// positive result too small to represent in the target asset.
//
// Unreachable in practice: when roundUp is set, roundMode() above
// selects Upward, and materializing a Number into an STAmount honors
// that mode (Number::operator rep()), so any positive value rounds up
// to at least the smallest representable unit. Hence, a positive result
// is never !result here; the only zero case is a zero operand, which
// the mulRound/divRound callers handle before reaching this function.
// LCOV_EXCL_START
if (asset.integral())
return STAmount{asset, 1};
return STAmount{asset, STAmount::kMinValue, STAmount::kMinOffset, false};
// LCOV_EXCL_STOP
}
return result;

View File

@@ -849,7 +849,11 @@ BookStep<TIn, TOut, TDerived>::forEachOffer(
removeOffer("Removing offer with overflowing amount calculation");
return true;
}
throw;
// An overflow can only be produced by a crafted MPT offer, and MPT
// offers require featureMPTokensV2 (enforced at OfferCreate
// preflight). So the amendment is always enabled when we get here
// and this legacy re-throw is unreachable in practice.
throw; // LCOV_EXCL_LINE
}
};
@@ -922,7 +926,13 @@ BookStep<TIn, TOut, TDerived>::consumeOffer(
// deleted by BookTip::step().
if (auto const err = checkCreateMPT(sb, book_.in.get<MPTIssue>(), offer.owner(), j_);
!isTesSuccess(err))
Throw<FlowException>(err);
{
// checkCreateMPT only fails on tecDIR_FULL (its source line is
// itself LCOV-excluded) or a missing offer-owner account, which
// cannot happen since that account owns the offer being
// consumed. Defensive and unreachable in practice.
Throw<FlowException>(err); // LCOV_EXCL_LINE
}
}
auto const dr = offer.send(

View File

@@ -330,7 +330,11 @@ TOfferStreamBase<TIn, TOut>::step()
offer_ = TOffer<TIn, TOut>{};
continue;
}
throw;
// The strict reduction only overflows for a crafted MPT offer, and
// MPT offers require featureMPTokensV2 (enforced at OfferCreate
// preflight). So the amendment is always enabled here and this
// legacy re-throw is unreachable in practice.
throw; // LCOV_EXCL_LINE
}
if (shouldRemoveSmallIncreasedQOffer)

View File

@@ -687,7 +687,18 @@ AMMWithdraw::withdraw(
!isTesSuccess(err))
{
if (authHandling != AuthHandling::IgnoreAuth || err != tecNO_AUTH)
return err;
{
// Unreachable in practice. Normal withdraws (authHandling
// != IgnoreAuth) are rejected for unauthorized holders in
// preclaim, so they never get here. Under clawback
// (IgnoreAuth) requireAuth returns a non-tecNO_AUTH error
// (e.g. tecEXPIRED) only for a domain-authorized MPT, but no
// such MPT can be in an AMM pool: a directly domain-gated
// RequireAuth MPT fails AMMCreate/deposit with tecNO_AUTH,
// and vault shares (whose recursive auth could yield
// tecEXPIRED) are rejected by AMMCreate with tecWRONG_ASSET.
return err; // LCOV_EXCL_LINE
}
// AMMClawback ignores authorization so the issuer can recover
// MPT locked in the pool even if the holder deleted their
@@ -708,7 +719,11 @@ AMMWithdraw::withdraw(
if (auto const err = checkCreateMPT(view, mptIssue, account, createFlags, journal);
!isTesSuccess(err))
{
return err;
// checkCreateMPT only fails on tecDIR_FULL (its source line is
// itself LCOV-excluded) or a missing account, which cannot
// happen since `account` is the withdrawing LP. Defensive and
// unreachable in practice.
return err; // LCOV_EXCL_LINE
}
}
return tesSUCCESS;

View File

@@ -1,4 +1,5 @@
#include <test/jtx/AMM.h>
#include <test/jtx/CaptureLogs.h>
#include <test/jtx/Env.h>
#include <test/jtx/PathSet.h>
#include <test/jtx/TestHelpers.h>
@@ -6,6 +7,7 @@
#include <test/jtx/acctdelete.h>
#include <test/jtx/amount.h>
#include <test/jtx/balance.h>
#include <test/jtx/envconfig.h>
#include <test/jtx/fee.h>
#include <test/jtx/jtx_json.h>
#include <test/jtx/mpt.h>
@@ -23,6 +25,7 @@
#include <test/jtx/txflags.h>
#include <xrpl/beast/unit_test/suite.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/core/ServiceRegistry.h>
#include <xrpl/json/json_value.h>
#include <xrpl/ledger/helpers/DirectoryHelpers.h>
@@ -47,6 +50,7 @@
#include <cstdint>
#include <functional>
#include <map>
#include <memory>
#include <optional>
#include <string>
#include <type_traits>
@@ -3656,6 +3660,46 @@ public:
BEAST_EXPECT(env.balance(poisonMaker, token) == token(funded));
BEAST_EXPECT(env.balance(taker, token) == token(0));
}
{
// Same overflow scenario as the ownerGives case above, but run with
// trace-level logging so BookStep::forEachOffer's removeOffer()
// emits its "Removing offer with overflowing amount calculation"
// trace line. This exercises the JLOG body inside removeOffer,
// which is skipped when logging is above trace severity.
std::string logs;
{
Env env{
*this,
envconfig(),
features,
std::make_unique<CaptureLogs>(&logs),
beast::Severity::Trace};
env.fund(XRP(10'000), issuer, taker);
env.close();
MPTTester const token{
{.env = env, .issuer = issuer, .holders = {taker}, .transferFee = 10'000}};
std::int64_t const poisonAmount = 8'500'000'000'000'000'000LL;
auto const poisonSeq = env.seq(issuer);
env(offer(issuer, XRP(1), token(poisonAmount)));
env.close();
auto const poisonKeylet = keylet::offer(issuer.id(), poisonSeq);
BEAST_EXPECT(env.le(poisonKeylet) != nullptr);
auto const takerSeq = env.seq(taker);
env(offer(taker, token(100), XRP(100)));
env.close();
BEAST_EXPECT(env.le(poisonKeylet) == nullptr);
BEAST_EXPECT(env.le(keylet::offer(taker.id(), takerSeq)) != nullptr);
}
BEAST_EXPECT(
logs.find("Removing offer with overflowing amount calculation") !=
std::string::npos);
}
}
void

View File

@@ -1102,6 +1102,21 @@ public:
BEAST_EXPECT(multiplyRound(largeAmount, transferRate, asset, true) == scaledAmount);
BEAST_EXPECT(divideRound(scaledAmount, transferRate, asset, true) == largeAmount);
}
{
// mulRound with an integral (XRP) operand whose mantissa is below
// kMinValue exercises the legacy value-scaling loop that normalizes
// the mantissa before multiply. The MPTokensV2 Number path is
// not taken here because the target asset is an IOU.
Issue const usd{Currency(0x5553440000000000), AccountID(0x4985601)};
STAmount const iouVal{usd, 5};
STAmount const xrpVal{XRPAmount{7}}; // integral, mantissa < kMinValue
auto const up = mulRound(iouVal, xrpVal, usd, /*roundUp*/ true);
auto const down = mulRound(iouVal, xrpVal, usd, /*roundUp*/ false);
BEAST_EXPECT(down.signum() > 0);
BEAST_EXPECT(up >= down);
}
}
void