From a1a86bd3622fc8d79c64f59ee248ad8f3be38cdd Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 23 Sep 2026 01:58:38 +0530 Subject: [PATCH] refactor: Add std::optional overload for isZeroId Addresses bthomee's review suggestion: about a third of isZeroId call sites are the optional-field pattern 'x && isZeroId(*x)'. Add an overload taking std::optional const& and collapse the 5 direct call sites (Payment, LoanSet, LoanBrokerCoverClawback, OfferCreate, PermissionedDomainSet). The remaining if-let uses are left alone because collapsing them would move unrelated code out of the outer block. The negated helpers (!isPositiveAmount etc.) are intentionally not overloaded: with an optional overload, !isPositiveAmount(opt) would be true when the optional is empty, which is the opposite of the existing 'opt && !isPositiveAmount(*opt)' semantics. --- include/xrpl/tx/helpers/PreflightHelpers.h | 11 +++++++++++ src/libxrpl/tx/transactors/dex/OfferCreate.cpp | 2 +- .../transactors/lending/LoanBrokerCoverClawback.cpp | 2 +- src/libxrpl/tx/transactors/lending/LoanSet.cpp | 2 +- src/libxrpl/tx/transactors/payment/Payment.cpp | 2 +- .../permissioned_domain/PermissionedDomainSet.cpp | 2 +- 6 files changed, 16 insertions(+), 5 deletions(-) diff --git a/include/xrpl/tx/helpers/PreflightHelpers.h b/include/xrpl/tx/helpers/PreflightHelpers.h index f618ac0ba1..4d01733dbf 100644 --- a/include/xrpl/tx/helpers/PreflightHelpers.h +++ b/include/xrpl/tx/helpers/PreflightHelpers.h @@ -5,6 +5,7 @@ #include #include +#include namespace xrpl { @@ -41,6 +42,16 @@ isZeroId(T const& id) return id == beast::kZero; } +// Optional-field overload: true iff the field is present and its value is +// unset/zero. Simplifies the `field && isZeroId(*field)` pattern into a +// single call. +template +inline bool +isZeroId(std::optional const& id) +{ + return id.has_value() && isZeroId(*id); +} + // Checks whether an amount is a strictly positive XRP amount. inline bool isPositiveXRPAmount(STAmount const& amount) diff --git a/src/libxrpl/tx/transactors/dex/OfferCreate.cpp b/src/libxrpl/tx/transactors/dex/OfferCreate.cpp index dba30ce62f..a140327067 100644 --- a/src/libxrpl/tx/transactors/dex/OfferCreate.cpp +++ b/src/libxrpl/tx/transactors/dex/OfferCreate.cpp @@ -101,7 +101,7 @@ OfferCreate::preflight(PreflightContext const& ctx) // A zero DomainID is invalid for a PermissionedDomain ledger entry because // keylet::permissionedDomain(uint256) uses the DomainID as the ledger key. if (auto const domainID = tx[~sfDomainID]; - ctx.rules.enabled(fixCleanup3_2_0) && domainID && isZeroId(*domainID)) + ctx.rules.enabled(fixCleanup3_2_0) && isZeroId(domainID)) return temMALFORMED; bool const bImmediateOrCancel(tx.isFlag(tfImmediateOrCancel)); diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverClawback.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverClawback.cpp index 6716e52e9b..fc8be65e8c 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverClawback.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverClawback.cpp @@ -49,7 +49,7 @@ LoanBrokerCoverClawback::preflight(PreflightContext const& ctx) if (!brokerID && !amount) return temINVALID; - if (brokerID && isZeroId(*brokerID)) + if (isZeroId(brokerID)) return temINVALID; if (amount) diff --git a/src/libxrpl/tx/transactors/lending/LoanSet.cpp b/src/libxrpl/tx/transactors/lending/LoanSet.cpp index c9910c44ff..bfcfd36b8a 100644 --- a/src/libxrpl/tx/transactors/lending/LoanSet.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanSet.cpp @@ -147,7 +147,7 @@ LoanSet::preflight(PreflightContext const& ctx) return *ret; } - if (auto const brokerID = ctx.tx[~sfLoanBrokerID]; brokerID && isZeroId(*brokerID)) + if (auto const brokerID = ctx.tx[~sfLoanBrokerID]; isZeroId(brokerID)) return temINVALID; return tesSUCCESS; diff --git a/src/libxrpl/tx/transactors/payment/Payment.cpp b/src/libxrpl/tx/transactors/payment/Payment.cpp index 166ec217fa..c49949dcf3 100644 --- a/src/libxrpl/tx/transactors/payment/Payment.cpp +++ b/src/libxrpl/tx/transactors/payment/Payment.cpp @@ -144,7 +144,7 @@ Payment::preflight(PreflightContext const& ctx) // A zero DomainID is invalid for a PermissionedDomain ledger entry because // keylet::permissionedDomain(uint256) uses the DomainID as the ledger key. if (auto const domainID = tx[~sfDomainID]; - ctx.rules.enabled(fixCleanup3_2_0) && domainID && isZeroId(*domainID)) + ctx.rules.enabled(fixCleanup3_2_0) && isZeroId(domainID)) return temMALFORMED; bool const partialPaymentAllowed = tx.isFlag(tfPartialPayment); diff --git a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp index 0739be5385..7284333880 100644 --- a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp +++ b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp @@ -40,7 +40,7 @@ PermissionedDomainSet::preflight(PreflightContext const& ctx) return err; auto const domain = ctx.tx.at(~sfDomainID); - if (domain && isZeroId(*domain)) + if (isZeroId(domain)) return temMALFORMED; return tesSUCCESS;