fix: Make calculateBaseFee exception-safe

This commit is contained in:
Jingchen
2026-09-09 17:35:07 +02:00
committed by Bart
parent 76da5d4475
commit 3e4e56d6bb
8 changed files with 162 additions and 45 deletions

View File

@@ -12,6 +12,7 @@
#include <xrpl/protocol/XRPAmount.h>
#include <cstdint>
#include <expected>
#include <optional>
#include <utility>
@@ -393,16 +394,21 @@ preclaim(PreflightResult const& preflightResult, ServiceRegistry& registry, Open
*
* No validation is done or implied by this function.
*
* Caller is responsible for handling any exceptions.
* Since none should be thrown, that will usually
* mean terminating.
*
* Callers do not expect this function to throw; exceptions from a transactor's
* `calculateBaseFee` are caught and reported as an error instead.
* @param view The current open ledger.
* @param tx The transaction to be checked.
*
* @return The base fee.
* @return The base fee on success. Returns `std::unexpected(temUNKNOWN)` if the transaction
* type is not recognized, and `std::unexpected(tefEXCEPTION)` if the transactor's
* `calculateBaseFee` threw.
*
* @note Failure is reported as an error rather than a fee of zero because a
* zero (or default) fee would pass checkFee and let the transaction be
* applied for less than it owes. Callers that only need a fee hint may fall
* back to a default; callers deciding whether to apply should reject.
*/
XRPAmount
[[nodiscard]] std::expected<XRPAmount, TER>
calculateBaseFee(ReadView const& view, STTx const& tx);
/**

View File

@@ -17,6 +17,7 @@
#include <cstdint>
#include <exception>
#include <expected>
#include <memory>
#include <optional>
#include <utility>
@@ -195,7 +196,12 @@ invokePreclaim(PreclaimContext const& ctx)
}())
return preSigResult;
if (TER const result = T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx)))
// We can't check the fee if we can't compute it, so reject.
auto const baseFee = calculateBaseFee(ctx.view, ctx.tx);
if (!baseFee)
return baseFee.error();
if (TER const result = T::checkFee(ctx, *baseFee))
return result;
}
@@ -223,13 +229,12 @@ invokePreclaim(PreclaimContext const& ctx)
*
* @param view The ledger view to use for fee calculation.
* @param tx The transaction for which the base fee is to be calculated.
* @return The calculated base fee as an XRPAmount.
* @return The calculated base fee. Returns `std::unexpected(temUNKNOWN)` if the transaction
* type is not recognized, and `std::unexpected(tefEXCEPTION)` if the transactor's
* `calculateBaseFee` threw.
*
* @throws std::exception If an error occurs during fee calculation, including
* but not limited to unknown transaction types or internal errors, the function
* logs an error and returns an XRPAmount of zero.
*/
static XRPAmount
static std::expected<XRPAmount, TER>
invokeCalculateBaseFee(ReadView const& view, STTx const& tx)
{
try
@@ -238,13 +243,25 @@ invokeCalculateBaseFee(ReadView const& view, STTx const& tx)
return T::calculateBaseFee(view, tx);
});
}
catch (UnknownTxnType const& e)
catch (UnknownTxnType const&)
{
// LCOV_EXCL_START
UNREACHABLE("xrpl::invoke_calculateBaseFee : unknown transaction type");
return XRPAmount{0};
return std::unexpected(temUNKNOWN);
// LCOV_EXCL_STOP
}
catch (std::exception const& e)
{
JLOG(debugLog().error()) << "calculateBaseFee: " << tx.getTransactionID()
<< " threw an exception: " << e.what();
return std::unexpected(tefEXCEPTION);
}
catch (...)
{
JLOG(debugLog().error()) << "calculateBaseFee: " << tx.getTransactionID()
<< " threw an unknown exception";
return std::unexpected(tefEXCEPTION);
}
}
TxConsequences::TxConsequences(NotTEC pfResult)
@@ -416,7 +433,7 @@ preclaim(PreflightResult const& preflightResult, ServiceRegistry& registry, Open
}
}
XRPAmount
std::expected<XRPAmount, TER>
calculateBaseFee(ReadView const& view, STTx const& tx)
{
return invokeCalculateBaseFee(view, tx);
@@ -441,13 +458,26 @@ doApply(PreclaimResult const& preclaimResult, ServiceRegistry& registry, OpenVie
{
if (!preclaimResult.likelyToClaimFee)
return {preclaimResult.ter, false};
// For any tx with a real account, preclaim already computed this fee
// successfully against this same view.
auto const baseFee = calculateBaseFee(view, preclaimResult.tx);
if (!baseFee)
{
// LCOV_EXCL_START
JLOG(preclaimResult.j.error())
<< "apply: could not compute base fee: " << transToken(baseFee.error());
return {tefINTERNAL, false};
// LCOV_EXCL_STOP
}
ApplyContext ctx(
registry,
view,
preclaimResult.parentBatchId,
preclaimResult.tx,
preclaimResult.ter,
calculateBaseFee(view, preclaimResult.tx),
*baseFee,
preclaimResult.flags,
preclaimResult.j);
return invokeApply(ctx);

View File

@@ -36,6 +36,15 @@
namespace xrpl {
namespace {
// Returns true if the transaction's payment amount is malformed. A loan
// payment must be strictly positive: zero would move nothing, and a negative
// amount is not a payment at all.
bool
isPaymentAmountInvalid(STAmount const& amount)
{
return amount <= beast::kZero;
}
// Returns the account's true, unclamped balance in `asset`, for use only in
// fund-conservation checks. accountHolds(..., SpendableHandling::FullBalance)
// cannot be used for this: for XRP it always defers to xrpLiquid, which
@@ -81,7 +90,7 @@ LoanPay::preflight(PreflightContext const& ctx)
if (ctx.tx[sfLoanID] == beast::kZero)
return temINVALID;
if (ctx.tx[sfAmount] <= beast::kZero)
if (isPaymentAmountInvalid(ctx.tx[sfAmount]))
return temBAD_AMOUNT;
// The loan payment flags are all mutually exclusive. If more than one is
@@ -103,10 +112,19 @@ LoanPay::preflight(PreflightContext const& ctx)
XRPAmount
LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx)
{
auto fixEnabled313 = view.rules().enabled(fixCleanup3_1_3);
auto fixEnabled340 = view.rules().enabled(fixCleanup3_4_0);
using namespace lending;
auto const normalCost = Transactor::calculateBaseFee(view, tx);
if (fixEnabled340 && isPaymentAmountInvalid(tx[sfAmount]))
{
// Let preflight worry about the error for this
return normalCost;
}
if (tx.isFlag(tfLoanFullPayment) || tx.isFlag(tfLoanLatePayment))
{
// The loan will be making one set of calculations for one full or late
@@ -179,8 +197,7 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx)
static constexpr std::int64_t kMaxFeeIncrements =
kLoanMaximumPaymentsPerTransaction / kLoanPaymentsPerFeeIncrement;
if (view.rules().enabled(fixCleanup3_1_3) &&
amount >= regularPayment * kLoanMaximumPaymentsPerTransaction)
if (fixEnabled313 && amount >= regularPayment * kLoanMaximumPaymentsPerTransaction)
{
// The payment handler will never process more than
// loanMaximumPaymentsPerTransaction payments (including overpayments),

View File

@@ -73,14 +73,23 @@ Batch::calculateBaseFeeImpl(ReadView const& view, STTx const& tx)
for (auto const& stx : tx.getBatchTransactions())
{
auto const fee = xrpl::calculateBaseFee(view, *stx);
// LCOV_EXCL_START
if (txnFees > maxAmount - fee)
if (!fee)
{
JLOG(debugLog().error())
<< "BatchTrace: base fee of inner transaction " << stx->getTransactionID()
<< " could not be computed: " << transToken(fee.error());
return std::nullopt;
}
// LCOV_EXCL_START
if (txnFees > maxAmount - *fee)
{
UNREACHABLE("XRPAmount overflow in txnFees calculation");
JLOG(debugLog().error()) << "BatchTrace: XRPAmount overflow in txnFees calculation.";
return std::nullopt;
}
// LCOV_EXCL_STOP
txnFees += fee;
txnFees += *fee;
}
// Calculate the Signers/BatchSigners Fees

View File

@@ -1886,11 +1886,21 @@ NetworkOPsImp::apply(std::unique_lock<std::mutex>& batchLock)
if (validatedLedgerIndex)
{
auto [fee, accountSeq, availableSeq] =
registry_.get().getTxQ().getTxRequiredFeeAndSeq(
*newOL, e.transaction->getSTransaction());
e.transaction->setCurrentLedgerState(
*validatedLedgerIndex, fee, accountSeq, availableSeq);
auto maybeFeeAndSeq = registry_.get().getTxQ().getTxRequiredFeeAndSeq(
*newOL, e.transaction->getSTransaction());
if (maybeFeeAndSeq.has_value())
{
auto [fee, accountSeq, availableSeq] = *maybeFeeAndSeq;
e.transaction->setCurrentLedgerState(
*validatedLedgerIndex, fee, accountSeq, availableSeq);
}
else
{
JLOG(journal_.debug())
<< "Unable to compute current ledger state for tx "
<< e.transaction->getID() << " in validated ledger "
<< *validatedLedgerIndex << ": " << transToken(maybeFeeAndSeq.error());
}
}
}
}

View File

@@ -23,6 +23,7 @@
#include <cstddef>
#include <cstdint>
#include <expected>
#include <limits>
#include <map>
#include <memory>
@@ -390,10 +391,10 @@ public:
* and first available sequence for transaction
* @param view current open ledger
* @param tx the transaction
* @return minimum required fee, first sequence in the ledger
* @return minimum required fee or an error, first sequence in the ledger
* and first available sequence
*/
FeeAndSeq
std::expected<FeeAndSeq, TER>
getTxRequiredFeeAndSeq(OpenView const& view, std::shared_ptr<STTx const> const& tx) const;
/**

View File

@@ -38,6 +38,7 @@
#include <algorithm>
#include <cstddef>
#include <cstdint>
#include <expected>
#include <iterator>
#include <limits>
#include <memory>
@@ -54,22 +55,29 @@ namespace xrpl {
//////////////////////////////////////////////////////////////////////////
static FeeLevel64
/**
* Compute the fee level that a transaction pays.
* @return The fee level paid, or the error reported by `calculateBaseFee`.
*/
static std::expected<FeeLevel64, TER>
getFeeLevelPaid(ReadView const& view, STTx const& tx)
{
auto const [baseFee, effectiveFeePaid] = [&view, &tx]() {
XRPAmount const baseFee = calculateBaseFee(view, tx);
auto const computedBaseFee = calculateBaseFee(view, tx);
if (!computedBaseFee)
return std::unexpected(computedBaseFee.error());
auto const [baseFee, effectiveFeePaid] = [&view, &tx, fee = *computedBaseFee]() {
XRPAmount const feePaid = tx[sfFee].xrp();
// If baseFee is 0 then the cost of a basic transaction is free, but we
// need the effective fee level to be non-zero.
XRPAmount const mod = [&view, &tx, baseFee]() {
if (baseFee.signum() > 0)
XRPAmount const mod = [&view, &tx, fee]() {
if (fee.signum() > 0)
return XRPAmount{0};
auto def = calculateDefaultBaseFee(view, tx);
return def.signum() == 0 ? XRPAmount{1} : def;
}();
return std::pair{baseFee + mod, feePaid + mod};
return std::pair{fee + mod, feePaid + mod};
}();
XRPL_ASSERT(baseFee.signum() > 0, "xrpl::getFeeLevelPaid : positive fee");
@@ -112,10 +120,20 @@ TxQ::FeeMetrics::update(
auto const size = std::distance(txBegin, txEnd);
feeLevels.reserve(size);
std::for_each(txBegin, txEnd, [&](auto const& tx) {
feeLevels.push_back(getFeeLevelPaid(view, *tx.first));
auto const maybeFeeLevel = getFeeLevelPaid(view, *tx.first);
if (maybeFeeLevel.has_value())
{
feeLevels.push_back(*maybeFeeLevel);
}
else
{
// Excluded from the median sample below.
JLOG(j_.warn()) << "Unable to compute the fee level for a validated transaction "
<< tx.first->getTransactionID() << " in ledger " << view.header().seq
<< ": " << transToken(maybeFeeLevel.error());
}
});
std::ranges::sort(feeLevels);
XRPL_ASSERT(size == feeLevels.size(), "xrpl::TxQ::FeeMetrics::update : fee levels size");
JLOG((timeLeap ? j_.warn() : j_.debug()))
<< "Ledger " << view.header().seq << " has " << size << " transactions. "
@@ -159,7 +177,10 @@ TxQ::FeeMetrics::update(
txnsExpected_ = std::min(next, maximumTxnCount_.value_or(next));
}
if (size == 0)
// The median is taken over the transactions whose fee level could be
// computed, while txnsExpected_ above deliberately uses the full
// transaction count.
if (feeLevels.empty())
{
escalationMultiplier_ = setup.minimumEscalationMultiplier;
}
@@ -169,8 +190,9 @@ TxQ::FeeMetrics::update(
// evaluates to the middle element; for an even
// number of elements, it will add the two elements
// on either side of the "middle" and average them.
auto const count = feeLevels.size();
escalationMultiplier_ =
(feeLevels[size / 2] + feeLevels[(size - 1) / 2] + FeeLevel64{1}) / 2;
(feeLevels[count / 2] + feeLevels[(count - 1) / 2] + FeeLevel64{1}) / 2;
escalationMultiplier_ = std::max(escalationMultiplier_, setup.minimumEscalationMultiplier);
}
JLOG(j_.debug()) << "Expected transactions updated to " << txnsExpected_
@@ -878,7 +900,14 @@ TxQ::apply(
// We may need the base fee for multiple transactions or transaction
// replacement, so just pull it up now.
auto const metricsSnapshot = feeMetrics_.getSnapshot();
auto const feeLevelPaid = getFeeLevelPaid(view, *tx);
auto const computedFeeLevelPaid = getFeeLevelPaid(view, *tx);
// Without a fee level there is no way to tell whether the transaction
// pays enough, so it can be neither applied nor queued.
if (!computedFeeLevelPaid.has_value())
{
return {computedFeeLevelPaid.error(), false};
}
FeeLevel64 const feeLevelPaid = *computedFeeLevelPaid;
auto const requiredFeeLevel = getRequiredFeeLevel(view, flags, metricsSnapshot, lock);
// Is there a blocker already in the account's queue? If so, don't
@@ -1684,7 +1713,14 @@ TxQ::tryDirectApply(
// If the transaction's fee is high enough we may be able to put the
// transaction straight into the ledger.
FeeLevel64 const feeLevelPaid = getFeeLevelPaid(view, *tx);
auto const computedFeeLevelPaid = getFeeLevelPaid(view, *tx);
// The fee level is unknown, so the transaction cannot be applied here,
// and queueing it would only run into the same failure. Reject it.
if (!computedFeeLevelPaid.has_value())
{
return ApplyResult{computedFeeLevelPaid.error(), false};
}
FeeLevel64 const feeLevelPaid = *computedFeeLevelPaid;
if (feeLevelPaid >= requiredFeeLevel)
{
@@ -1768,7 +1804,7 @@ TxQ::getMetrics(OpenView const& view) const
return result;
}
TxQ::FeeAndSeq
std::expected<TxQ::FeeAndSeq, TER>
TxQ::getTxRequiredFeeAndSeq(OpenView const& view, std::shared_ptr<STTx const> const& tx) const
{
auto const account = (*tx)[sfAccount];
@@ -1776,14 +1812,19 @@ TxQ::getTxRequiredFeeAndSeq(OpenView const& view, std::shared_ptr<STTx const> co
std::scoped_lock const lock(mutex_);
auto const snapshot = feeMetrics_.getSnapshot();
auto const baseFee = calculateBaseFee(view, *tx);
auto const maybeBaseFee = calculateBaseFee(view, *tx);
if (!maybeBaseFee.has_value())
{
return std::unexpected(maybeBaseFee.error());
}
auto const baseFee = *maybeBaseFee;
auto const fee = FeeMetrics::scaleFeeLevel(snapshot, view);
auto const sle = view.read(keylet::account(account));
std::uint32_t const accountSeq = sle ? (*sle)[sfSequence] : 0;
std::uint32_t const availableSeq = nextQueuableSeqImpl(sle, lock).value();
return {
return FeeAndSeq{
.fee = mulDiv(fee, baseFee, kBaseLevel)
.value_or(XRPAmount(std::numeric_limits<std::int64_t>::max())),
.accountSeq = accountSeq,

View File

@@ -887,7 +887,10 @@ getTxFee(Application const& app, Config const& config, json::Value tx)
if (!passesLocalChecks(stTx, reason))
return config.fees.referenceFee;
return calculateBaseFee(*app.getOpenLedger().current(), stTx);
// This fee is only a suggestion returned to the caller, so fall back to
// the reference fee as the other failure paths in this function do.
return calculateBaseFee(*app.getOpenLedger().current(), stTx)
.value_or(config.fees.referenceFee);
}
catch (std::exception& e)
{