Ensure payment does not exceed Loan balance

- Also disallow extra parameters on the ALWAYS_OR_UNREACHABLE macro,
  which should prevent me from mixing up XRPL_ASSERT and
  XRPL_ASSERT_PARTS.
This commit is contained in:
Ed Hennis
2025-10-28 15:56:57 -04:00
parent 310852ba2d
commit ec60dcf90d
3 changed files with 31 additions and 17 deletions

View File

@@ -32,7 +32,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
// The duplication is because Visual Studio 2019 cannot compile that header // The duplication is because Visual Studio 2019 cannot compile that header
// even with the option -Zc:__cplusplus added. // even with the option -Zc:__cplusplus added.
#define ALWAYS(cond, message, ...) assert((message) && (cond)) #define ALWAYS(cond, message, ...) assert((message) && (cond))
#define ALWAYS_OR_UNREACHABLE(cond, message, ...) assert((message) && (cond)) #define ALWAYS_OR_UNREACHABLE(cond, message) assert((message) && (cond))
#define SOMETIMES(cond, message, ...) #define SOMETIMES(cond, message, ...)
#define REACHABLE(message, ...) #define REACHABLE(message, ...)
#define UNREACHABLE(message, ...) assert((message) && false) #define UNREACHABLE(message, ...) assert((message) && false)

View File

@@ -299,7 +299,7 @@ LedgerReplayTask::addDelta(std::shared_ptr<LedgerDeltaAcquire> const& delta)
deltas_.empty() || deltas_.empty() ||
deltas_.back()->ledgerSeq_ + 1 == delta->ledgerSeq_, deltas_.back()->ledgerSeq_ + 1 == delta->ledgerSeq_,
"ripple::LedgerReplayTask::addDelta : no deltas or consecutive " "ripple::LedgerReplayTask::addDelta : no deltas or consecutive "
"sequence", ); "sequence");
deltas_.push_back(delta); deltas_.push_back(delta);
} }
} }

View File

@@ -1111,15 +1111,15 @@ computePaymentComponents(
// If there's only one payment left, we need to pay off each of the loan // If there's only one payment left, we need to pay off each of the loan
// parts. // parts.
XRPL_ASSERT( XRPL_ASSERT_PARTS(
deltas.valueDelta == totalValueOutstanding, deltas.valueDelta == totalValueOutstanding,
"ripple::detail::computePaymentComponents", "ripple::detail::computePaymentComponents",
"last payment total value agrees"); "last payment total value agrees");
XRPL_ASSERT( XRPL_ASSERT_PARTS(
deltas.principalDelta == principalOutstanding, deltas.principalDelta == principalOutstanding,
"ripple::detail::computePaymentComponents", "ripple::detail::computePaymentComponents",
"last payment principal agrees"); "last payment principal agrees");
XRPL_ASSERT( XRPL_ASSERT_PARTS(
deltas.managementFeeDueDelta == managementFeeOutstanding, deltas.managementFeeDueDelta == managementFeeOutstanding,
"ripple::detail::computePaymentComponents", "ripple::detail::computePaymentComponents",
"last payment management fee agrees"); "last payment management fee agrees");
@@ -1227,14 +1227,6 @@ computePaymentComponents(
"payment parts fit within payment limit"); "payment parts fit within payment limit");
#endif #endif
// Make sure the parts don't add up to too much
Number shortage = roundedPeriodicPayment - deltas.valueDelta;
XRPL_ASSERT_PARTS(
isRounded(asset, shortage, scale),
"ripple::detail::computePaymentComponents",
"excess is rounded");
// The shortage must never be negative, which indicates that the parts are // The shortage must never be negative, which indicates that the parts are
// trying to take more than the whole payment. The excess can be positive, // trying to take more than the whole payment. The excess can be positive,
// which indicates that we're not going to take the whole payment amount, // which indicates that we're not going to take the whole payment amount,
@@ -1256,13 +1248,35 @@ computePaymentComponents(
"ripple::detail::computePaymentComponents", "ripple::detail::computePaymentComponents",
"excess non-negative"); "excess non-negative");
}; };
auto addressExcess = [&takeFrom](LoanDeltas& deltas, Number& excess) {
takeFrom(deltas.valueDelta, deltas.interestDueDelta, excess);
takeFrom(deltas.valueDelta, deltas.managementFeeDueDelta, excess);
takeFrom(deltas.valueDelta, deltas.principalDelta, excess);
};
Number totalOverpayment =
deltas.valueDelta - currentLedgerState.valueOutstanding;
if (totalOverpayment > 0)
{
// LCOV_EXCL_START
UNREACHABLE(
"ripple::detail::computePaymentComponents : payment exceeded loan "
"state");
addressExcess(deltas, totalOverpayment);
// LCOV_EXCL_STOP
}
// Make sure the parts don't add up to too much
Number shortage = roundedPeriodicPayment - deltas.valueDelta;
XRPL_ASSERT_PARTS(
isRounded(asset, shortage, scale),
"ripple::detail::computePaymentComponents",
"shortage is rounded");
if (shortage < beast::zero) if (shortage < beast::zero)
{ {
Number excess = -shortage; Number excess = -shortage;
takeFrom(deltas.valueDelta, deltas.principalDelta, excess); addressExcess(deltas, excess);
takeFrom(deltas.valueDelta, deltas.interestDueDelta, excess);
takeFrom(deltas.valueDelta, deltas.managementFeeDueDelta, excess);
shortage = -excess; shortage = -excess;
} }
@@ -1279,7 +1293,7 @@ computePaymentComponents(
"ripple::detail::computePaymentComponents", "ripple::detail::computePaymentComponents",
"excess is extremely small"); "excess is extremely small");
XRPL_ASSERT( XRPL_ASSERT_PARTS(
deltas.valueDelta == deltas.valueDelta ==
deltas.principalDelta + deltas.interestDueDelta + deltas.principalDelta + deltas.interestDueDelta +
deltas.managementFeeDueDelta, deltas.managementFeeDueDelta,