refactor: Introduce XRPL_ASSERT_IF for amendment-gated assertions

Add XRPL_ASSERT_IF(guard, ...) to instrumentation.h — a conditional
assertion that fires only when guard is true, using __VA_ARGS__ to
match the XRPL_ASSERT_PARTS variadic convention and avoid bare-comma
pitfalls in cond. guard is always evaluated (even in release builds
where the assertion body is stripped), so it should be side-effect-free.

Replace all amendment-gated XRPL_ASSERT / XRPL_ASSERT_PARTS call sites
in LendingHelpers.cpp and MPTokenHelpers.cpp with the new macro. In
doOverpayment, hoist rules.enabled(fixCleanup3_2_0) to a single bool
to avoid three repeated set lookups, and inline the three intermediate
Number values into a lambda so they are only computed when the guard
fires.
This commit is contained in:
Vito
2026-06-02 11:11:11 +02:00
parent d4cb68d5a1
commit cd04055559
3 changed files with 58 additions and 47 deletions

View File

@@ -21,6 +21,13 @@
#define XRPL_ASSERT ALWAYS_OR_UNREACHABLE
#define XRPL_ASSERT_PARTS(cond, function, description, ...) \
XRPL_ASSERT(cond, function " : " description)
// clang-format off
#define XRPL_ASSERT_IF(guard, ...) \
do { \
if ((guard)) \
XRPL_ASSERT(__VA_ARGS__); \
} while (false)
// clang-format on
// How to use the instrumentation macros:
//
@@ -29,6 +36,11 @@
// * XRPL_ASSERT_PARTS is for convenience, and works like XRPL_ASSERT, but
// splits the message param into "function" and "description", then joins
// them with " : " before passing to XRPL_ASSERT.
// * XRPL_ASSERT_IF(guard, cond, message) fires the assertion only when guard
// is true (e.g. an amendment is enabled). Equivalent to
// `if (guard) XRPL_ASSERT(cond, message)` but safe to use in all statement
// contexts. NOTE: guard is always evaluated — even in release builds where
// the assertion itself is stripped — so keep it side-effect-free and cheap.
// * ALWAYS if cond must be true _and_ the line must be reached during fuzzing.
// Same like `assert` in normal use.
// * REACHABLE if the line must be reached during fuzzing

View File

@@ -798,44 +798,45 @@ doOverpayment(
// (P * factor) / factor round-trip can leave the new principal one
// scale-unit high, so these equalities do not hold on the pre-amendment
// code path and must be gated to match the fix they verify.
if (rules.enabled(fixCleanup3_2_0))
{
// The valueChange returned by tryOverpayment satisfies
// valueChange = (newInterestDue - oldInterestDue) + untrackedInterest.
// Using the loan-state identity v = p + i + m and the adjacent
// `principal change agrees` assertion (dp = oldP - newP), this
// rearranges into three independently-computable terms:
//
// 1. TVO change beyond what principal repayment alone explains:
// newTVO - (oldTVO - dp)
// 2. Management fee released by re-amortization (positive when
// mfee decreased; zero when managementFeeRate == 0):
// oldMfee - newMfee
// 3. The overpayment's penalty interest part (= untrackedInterest
// for the overpayment path; see computeOverpaymentComponents):
// trackedInterestPart()
[[maybe_unused]] Number const tvoChange = newRoundedLoanState.valueOutstanding -
(totalValueOutstandingProxy - overpaymentComponents.trackedPrincipalDelta);
[[maybe_unused]] Number const managementFeeReleased =
managementFeeOutstandingProxy - newRoundedLoanState.managementFeeDue;
[[maybe_unused]] Number const interestPart = overpaymentComponents.trackedInterestPart();
//
// The valueChange returned by tryOverpayment satisfies
// valueChange = (newInterestDue - oldInterestDue) + untrackedInterest.
// Using the loan-state identity v = p + i + m and the adjacent
// `principal change agrees` assertion (dp = oldP - newP), this
// rearranges into three independently-computable terms:
//
// 1. TVO change beyond what principal repayment alone explains:
// newTVO - (oldTVO - dp)
// 2. Management fee released by re-amortization (positive when
// mfee decreased; zero when managementFeeRate == 0):
// oldMfee - newMfee
// 3. The overpayment's penalty interest part (= untrackedInterest
// for the overpayment path; see computeOverpaymentComponents):
// trackedInterestPart()
bool const fix320Enabled = rules.enabled(fixCleanup3_2_0);
XRPL_ASSERT_IF(
fix320Enabled,
overpaymentComponents.trackedPrincipalDelta ==
principalOutstandingProxy - newRoundedLoanState.principalOutstanding,
"xrpl::detail::doOverpayment : principal change agrees");
XRPL_ASSERT_PARTS(
overpaymentComponents.trackedPrincipalDelta ==
principalOutstandingProxy - newRoundedLoanState.principalOutstanding,
"xrpl::detail::doOverpayment",
"principal change agrees");
XRPL_ASSERT_IF(
fix320Enabled,
loanPaymentParts.valueChange ==
[&] {
Number const tvoChange = newRoundedLoanState.valueOutstanding -
(totalValueOutstandingProxy - overpaymentComponents.trackedPrincipalDelta);
Number const managementFeeReleased =
managementFeeOutstandingProxy - newRoundedLoanState.managementFeeDue;
Number const interestPart = overpaymentComponents.trackedInterestPart();
return tvoChange + managementFeeReleased + interestPart;
}(),
"xrpl::detail::doOverpayment : interest paid agrees");
XRPL_ASSERT_PARTS(
loanPaymentParts.valueChange == tvoChange + managementFeeReleased + interestPart,
"xrpl::detail::doOverpayment",
"interest paid agrees");
XRPL_ASSERT_PARTS(
overpaymentComponents.trackedPrincipalDelta == loanPaymentParts.principalPaid,
"xrpl::detail::doOverpayment",
"principal payment matches");
}
XRPL_ASSERT_IF(
fix320Enabled,
overpaymentComponents.trackedPrincipalDelta == loanPaymentParts.principalPaid,
"xrpl::detail::doOverpayment : principal payment matches");
// All validations passed, so update the proxy objects (which will
// modify the actual Loan ledger object)
@@ -1326,13 +1327,11 @@ computeOverpaymentComponents(
TenthBips32 const overpaymentFeeRate,
TenthBips16 const managementFeeRate)
{
if (rules.enabled(fixCleanup3_2_0))
{
XRPL_ASSERT(
overpayment > 0 && isRounded(asset, overpayment, loanScale),
"xrpl::detail::computeOverpaymentComponents : valid overpayment "
"amount");
}
XRPL_ASSERT_IF(
rules.enabled(fixCleanup3_2_0),
overpayment > 0 && isRounded(asset, overpayment, loanScale),
"xrpl::detail::computeOverpaymentComponents : valid overpayment "
"amount");
// First, deduct the fixed overpayment fee from the total amount.
// This reduces the effective payment that will be applied to the loan.

View File

@@ -736,10 +736,10 @@ unlockEscrowMPT(
STAmount const& grossAmount,
beast::Journal j)
{
if (!view.rules().enabled(fixTokenEscrowV1))
{
XRPL_ASSERT(netAmount == grossAmount, "xrpl::unlockEscrowMPT : netAmount == grossAmount");
}
XRPL_ASSERT_IF(
!view.rules().enabled(fixTokenEscrowV1),
netAmount == grossAmount,
"xrpl::unlockEscrowMPT : netAmount == grossAmount");
auto const& issuer = netAmount.getIssuer();
auto const& mptIssue = netAmount.get<MPTIssue>();