Make preflight1 and preflight2 private static Transactor functions

- They should never be called by derived classes.
This commit is contained in:
Ed Hennis
2025-07-11 18:42:39 -04:00
parent e9d2dfe329
commit d82461ea70
4 changed files with 82 additions and 93 deletions

View File

@@ -378,11 +378,6 @@ Batch::doPreflight(PreflightContext const& ctx)
requiredSigners.insert(innerAccount);
}
// LCOV_EXCL_START
if (auto const ret = detail::preflight2(ctx); !isTesSuccess(ret))
return ret;
// LCOV_EXCL_STOP
// Validation Batch Signers
std::unordered_set<AccountID> batchSigners;
if (ctx.tx.isFieldPresent(sfBatchSigners))

View File

@@ -650,14 +650,6 @@ EscrowFinish::doPreflight(PreflightContext const& ctx)
if (static_cast<bool>(cb) != static_cast<bool>(fb))
return temMALFORMED;
// Verify the transaction signature. If it doesn't work
// then don't do any more work.
{
auto const ret = detail::preflight2(ctx);
if (!isTesSuccess(ret))
return ret;
}
if (cb && fb)
{
auto& router = ctx.app.getHashRouter();

View File

@@ -112,69 +112,6 @@ preflightCheckSigningKey(STObject const& sigObject, beast::Journal j)
return tesSUCCESS;
}
/** Performs early sanity checks on the account and fee fields */
NotTEC
preflight1(PreflightContext const& ctx, std::uint32_t flagMask)
{
// This is inappropriate in preflight0, because only Change transactions
// skip this function, and those do not allow an sfTicketSequence field.
if (ctx.tx.isFieldPresent(sfTicketSequence) &&
!ctx.rules.enabled(featureTicketBatch))
{
return temMALFORMED;
}
if (ctx.tx.isFieldPresent(sfDelegate))
{
if (!ctx.rules.enabled(featurePermissionDelegation))
return temDISABLED;
if (ctx.tx[sfDelegate] == ctx.tx[sfAccount])
return temBAD_SIGNER;
}
if (auto const ret = preflight0(ctx, flagMask))
return ret;
auto const id = ctx.tx.getAccountID(sfAccount);
if (id == beast::zero)
{
JLOG(ctx.j.warn()) << "preflight1: bad account id";
return temBAD_SRC_ACCOUNT;
}
// No point in going any further if the transaction fee is malformed.
auto const fee = ctx.tx.getFieldAmount(sfFee);
if (!fee.native() || fee.negative() || !isLegalAmount(fee.xrp()))
{
JLOG(ctx.j.debug()) << "preflight1: invalid fee";
return temBAD_FEE;
}
if (auto const ret = preflightCheckSigningKey(ctx.tx, ctx.j))
return ret;
// An AccountTxnID field constrains transaction ordering more than the
// Sequence field. Tickets, on the other hand, reduce ordering
// constraints. Because Tickets and AccountTxnID work against one
// another the combination is unsupported and treated as malformed.
//
// We return temINVALID for such transactions.
if (ctx.tx.getSeqProxy().isTicket() &&
ctx.tx.isFieldPresent(sfAccountTxnID))
return temINVALID;
if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatch))
return temINVALID_FLAG;
XRPL_ASSERT(
ctx.tx.isFlag(tfInnerBatchTxn) == ctx.parentBatchId.has_value() ||
!ctx.rules.enabled(featureBatch),
"Inner batch transaction must have a parent batch ID.");
return tesSUCCESS;
}
std::optional<NotTEC>
preflightCheckSimulateKeys(
ApplyFlags flags,
@@ -217,11 +154,77 @@ preflightCheckSimulateKeys(
return {};
}
} // namespace detail
/** Performs early sanity checks on the account and fee fields */
NotTEC
Transactor::preflight1(PreflightContext const& ctx, std::uint32_t flagMask)
{
// This is inappropriate in preflight0, because only Change transactions
// skip this function, and those do not allow an sfTicketSequence field.
if (ctx.tx.isFieldPresent(sfTicketSequence) &&
!ctx.rules.enabled(featureTicketBatch))
{
return temMALFORMED;
}
if (ctx.tx.isFieldPresent(sfDelegate))
{
if (!ctx.rules.enabled(featurePermissionDelegation))
return temDISABLED;
if (ctx.tx[sfDelegate] == ctx.tx[sfAccount])
return temBAD_SIGNER;
}
if (auto const ret = preflight0(ctx, flagMask))
return ret;
auto const id = ctx.tx.getAccountID(sfAccount);
if (id == beast::zero)
{
JLOG(ctx.j.warn()) << "preflight1: bad account id";
return temBAD_SRC_ACCOUNT;
}
// No point in going any further if the transaction fee is malformed.
auto const fee = ctx.tx.getFieldAmount(sfFee);
if (!fee.native() || fee.negative() || !isLegalAmount(fee.xrp()))
{
JLOG(ctx.j.debug()) << "preflight1: invalid fee";
return temBAD_FEE;
}
if (auto const ret = detail::preflightCheckSigningKey(ctx.tx, ctx.j))
return ret;
// An AccountTxnID field constrains transaction ordering more than the
// Sequence field. Tickets, on the other hand, reduce ordering
// constraints. Because Tickets and AccountTxnID work against one
// another the combination is unsupported and treated as malformed.
//
// We return temINVALID for such transactions.
if (ctx.tx.getSeqProxy().isTicket() &&
ctx.tx.isFieldPresent(sfAccountTxnID))
return temINVALID;
if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatch))
return temINVALID_FLAG;
XRPL_ASSERT(
ctx.tx.isFlag(tfInnerBatchTxn) == ctx.parentBatchId.has_value() ||
!ctx.rules.enabled(featureBatch),
"Inner batch transaction must have a parent batch ID.");
return tesSUCCESS;
}
/** Checks whether the signature appears valid */
NotTEC
preflight2(PreflightContext const& ctx)
Transactor::preflight2(PreflightContext const& ctx)
{
if (auto const ret = preflightCheckSimulateKeys(ctx.flags, ctx.tx, ctx.j))
if (auto const ret =
detail::preflightCheckSimulateKeys(ctx.flags, ctx.tx, ctx.j))
// Skips following checks if the transaction is being simulated,
// regardless of success or failure
return *ret;
@@ -236,8 +239,6 @@ preflight2(PreflightContext const& ctx)
return tesSUCCESS;
}
} // namespace detail
//------------------------------------------------------------------------------
Transactor::Transactor(ApplyContext& ctx)

View File

@@ -319,6 +319,18 @@ private:
STObject const& sigObject);
void trapTransaction(uint256) const;
// Helper functions for preflight checks. Do not use directly.
/** Performs early sanity checks on the account and fee fields.
(And passes flagMask to preflight0)
*/
static NotTEC
preflight1(PreflightContext const& ctx, std::uint32_t flagMask);
/** Checks whether the signature appears valid */
static NotTEC
preflight2(PreflightContext const& ctx);
};
inline bool
@@ -340,13 +352,6 @@ namespace detail {
NotTEC
preflightCheckSigningKey(STObject const& sigObject, beast::Journal j);
/** Performs early sanity checks on the account and fee fields.
(And passes flagMask to preflight0)
*/
NotTEC
preflight1(PreflightContext const& ctx, std::uint32_t flagMask);
/** Checks the special signing key state needed for simulation
*
* Normally called from preflight2 with ctx.tx.
@@ -356,10 +361,6 @@ preflightCheckSimulateKeys(
ApplyFlags flags,
STObject const& sigObject,
beast::Journal j);
/** Checks whether the signature appears valid */
NotTEC
preflight2(PreflightContext const& ctx);
} // namespace detail
// Defined in Change.cpp
@@ -374,13 +375,13 @@ Transactor::preflight(PreflightContext const& ctx)
if (!T::isEnabled(ctx))
return temDISABLED;
if (auto const ret = ripple::detail::preflight1(ctx, T::getFlagsMask(ctx)))
if (auto const ret = preflight1(ctx, T::getFlagsMask(ctx)))
return ret;
if (auto const ret = T::doPreflight(ctx))
return ret;
return ripple::detail::preflight2(ctx);
return preflight2(ctx);
}
template <class T>