From 315775dc217502ec3f15176366b4734d75e4e909 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 1 Sep 2026 17:56:58 -0400 Subject: [PATCH] Revert isPositive -> isNonPositive rename isNonPositiveXRPAmount(amount) = !isXRP(amount) || amount <= 0, which reads misleadingly (returns true for non-XRP amounts entirely, not just non-positive XRP ones). Restore isPositiveXRPAmount/ isPositiveAmount and the negation at call sites. --- include/xrpl/tx/helpers/PreflightHelpers.h | 12 ++++++------ src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp | 7 +++---- .../transactors/lending/LoanBrokerCoverDeposit.cpp | 2 +- .../transactors/lending/LoanBrokerCoverWithdraw.cpp | 2 +- src/libxrpl/tx/transactors/lending/LoanPay.cpp | 2 +- src/libxrpl/tx/transactors/payment/Payment.cpp | 4 ++-- .../payment_channel/PaymentChannelClaim.cpp | 4 ++-- .../payment_channel/PaymentChannelCreate.cpp | 2 +- .../payment_channel/PaymentChannelFund.cpp | 2 +- src/libxrpl/tx/transactors/token/Clawback.cpp | 4 ++-- src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp | 2 +- 11 files changed, 21 insertions(+), 22 deletions(-) diff --git a/include/xrpl/tx/helpers/PreflightHelpers.h b/include/xrpl/tx/helpers/PreflightHelpers.h index a250c4cc5b..f618ac0ba1 100644 --- a/include/xrpl/tx/helpers/PreflightHelpers.h +++ b/include/xrpl/tx/helpers/PreflightHelpers.h @@ -41,18 +41,18 @@ isZeroId(T const& id) return id == beast::kZero; } -// Checks whether an amount is not a strictly positive XRP amount. +// Checks whether an amount is a strictly positive XRP amount. inline bool -isNonPositiveXRPAmount(STAmount const& amount) +isPositiveXRPAmount(STAmount const& amount) { - return !isXRP(amount) || amount <= beast::kZero; + return isXRP(amount) && amount > beast::kZero; } -// Checks whether an amount (of any asset type) is not strictly positive. +// Checks whether an amount (of any asset type) is strictly positive. inline bool -isNonPositiveAmount(STAmount const& amount) +isPositiveAmount(STAmount const& amount) { - return amount <= beast::kZero; + return amount > beast::kZero; } // Checks whether a currency code is the reserved "bad"/XRP currency code, diff --git a/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp b/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp index 27fdec713b..30a36d1a72 100644 --- a/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp +++ b/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp @@ -101,7 +101,7 @@ NotTEC escrowCreatePreflightHelper(PreflightContext const& ctx) { STAmount const amount = ctx.tx[sfAmount]; - if (amount.native() || isNonPositiveAmount(amount)) + if (amount.native() || !isPositiveAmount(amount)) return temBAD_AMOUNT; if (isBadCurrency(amount.get().currency)) @@ -118,8 +118,7 @@ escrowCreatePreflightHelper(PreflightContext const& ctx) return temDISABLED; auto const amount = ctx.tx[sfAmount]; - if (amount.native() || amount.mpt() > MPTAmount{kMaxMpTokenAmount} || - isNonPositiveAmount(amount)) + if (amount.native() || amount.mpt() > MPTAmount{kMaxMpTokenAmount} || !isPositiveAmount(amount)) return temBAD_AMOUNT; return tesSUCCESS; @@ -142,7 +141,7 @@ EscrowCreate::preflight(PreflightContext const& ctx) } else { - if (isNonPositiveXRPAmount(amount)) + if (!isPositiveXRPAmount(amount)) return temBAD_AMOUNT; } diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp index 7387fee595..02fc26596a 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp @@ -32,7 +32,7 @@ LoanBrokerCoverDeposit::preflight(PreflightContext const& ctx) return temINVALID; auto const dstAmount = ctx.tx[sfAmount]; - if (isNonPositiveAmount(dstAmount)) + if (!isPositiveAmount(dstAmount)) return temBAD_AMOUNT; if (!isLegalNet(dstAmount)) diff --git a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp index dcff9e277b..45cfd3600d 100644 --- a/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp @@ -35,7 +35,7 @@ LoanBrokerCoverWithdraw::preflight(PreflightContext const& ctx) return temINVALID; auto const dstAmount = ctx.tx[sfAmount]; - if (isNonPositiveAmount(dstAmount)) + if (!isPositiveAmount(dstAmount)) return temBAD_AMOUNT; if (!isLegalNet(dstAmount)) diff --git a/src/libxrpl/tx/transactors/lending/LoanPay.cpp b/src/libxrpl/tx/transactors/lending/LoanPay.cpp index 3e3be9df8a..1c99b11dd1 100644 --- a/src/libxrpl/tx/transactors/lending/LoanPay.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanPay.cpp @@ -52,7 +52,7 @@ LoanPay::preflight(PreflightContext const& ctx) if (isZeroId(ctx.tx[sfLoanID])) return temINVALID; - if (isNonPositiveAmount(ctx.tx[sfAmount])) + if (!isPositiveAmount(ctx.tx[sfAmount])) return temBAD_AMOUNT; // The loan payment flags are all mutually exclusive. If more than one is diff --git a/src/libxrpl/tx/transactors/payment/Payment.cpp b/src/libxrpl/tx/transactors/payment/Payment.cpp index 388026de23..4210bded59 100644 --- a/src/libxrpl/tx/transactors/payment/Payment.cpp +++ b/src/libxrpl/tx/transactors/payment/Payment.cpp @@ -184,13 +184,13 @@ Payment::preflight(PreflightContext const& ctx) << "Payment destination account not specified."; return temDST_NEEDED; } - if (hasMax && isNonPositiveAmount(maxSourceAmount)) + if (hasMax && !isPositiveAmount(maxSourceAmount)) { JLOG(j.trace()) << "Malformed transaction: bad max amount: " << maxSourceAmount.getFullText(); return temBAD_AMOUNT; } - if (isNonPositiveAmount(dstAmount)) + if (!isPositiveAmount(dstAmount)) { JLOG(j.trace()) << "Malformed transaction: bad dst amount: " << dstAmount.getFullText(); return temBAD_AMOUNT; diff --git a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp index 43783cad4b..3c8a4e8150 100644 --- a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp +++ b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp @@ -47,11 +47,11 @@ PaymentChannelClaim::preflight(PreflightContext const& ctx) return temMALFORMED; auto const bal = ctx.tx[~sfBalance]; - if (bal && isNonPositiveXRPAmount(*bal)) + if (bal && !isPositiveXRPAmount(*bal)) return temBAD_AMOUNT; auto const amt = ctx.tx[~sfAmount]; - if (amt && isNonPositiveXRPAmount(*amt)) + if (amt && !isPositiveXRPAmount(*amt)) return temBAD_AMOUNT; if (bal && amt && *bal > *amt) diff --git a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp index 554e2b8ea1..f77243f0f9 100644 --- a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp +++ b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp @@ -57,7 +57,7 @@ PaymentChannelCreate::makeTxConsequences(PreflightContext const& ctx) NotTEC PaymentChannelCreate::preflight(PreflightContext const& ctx) { - if (isNonPositiveXRPAmount(ctx.tx[sfAmount])) + if (!isPositiveXRPAmount(ctx.tx[sfAmount])) return temBAD_AMOUNT; if (ctx.tx[sfAccount] == ctx.tx[sfDestination]) diff --git a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp index 58d4d66f50..78a59012b7 100644 --- a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp +++ b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp @@ -34,7 +34,7 @@ PaymentChannelFund::preflight(PreflightContext const& ctx) if (ctx.rules.enabled(fixCleanup3_2_0) && isZeroId(ctx.tx[sfChannel])) return temMALFORMED; - if (isNonPositiveXRPAmount(ctx.tx[sfAmount])) + if (!isPositiveXRPAmount(ctx.tx[sfAmount])) return temBAD_AMOUNT; return tesSUCCESS; diff --git a/src/libxrpl/tx/transactors/token/Clawback.cpp b/src/libxrpl/tx/transactors/token/Clawback.cpp index 904afcd76a..5231116c64 100644 --- a/src/libxrpl/tx/transactors/token/Clawback.cpp +++ b/src/libxrpl/tx/transactors/token/Clawback.cpp @@ -45,7 +45,7 @@ preflightHelper(PreflightContext const& ctx) // The issuer field is used for the token holder instead AccountID const& holder = clawAmount.getIssuer(); - if (issuer == holder || isXRP(clawAmount) || isNonPositiveAmount(clawAmount)) + if (issuer == holder || isXRP(clawAmount) || !isPositiveAmount(clawAmount)) return temBAD_AMOUNT; return tesSUCCESS; @@ -68,7 +68,7 @@ preflightHelper(PreflightContext const& ctx) if (ctx.tx[sfAccount] == *mptHolder) return temMALFORMED; - if (clawAmount.mpt() > MPTAmount{kMaxMpTokenAmount} || isNonPositiveAmount(clawAmount)) + if (clawAmount.mpt() > MPTAmount{kMaxMpTokenAmount} || !isPositiveAmount(clawAmount)) return temBAD_AMOUNT; return tesSUCCESS; diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index def2c94e9f..c335157cd2 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -49,7 +49,7 @@ VaultWithdraw::preflight(PreflightContext const& ctx) return temMALFORMED; } - if (isNonPositiveAmount(ctx.tx[sfAmount])) + if (!isPositiveAmount(ctx.tx[sfAmount])) return temBAD_AMOUNT; if (auto const destination = ctx.tx[~sfDestination])