From 58a3a4e5ba8a5abd4dbb50b974ad4209a0090fe6 Mon Sep 17 00:00:00 2001 From: pwang200 <354723+pwang200@users.noreply.github.com> Date: Wed, 23 Sep 2026 16:03:06 -0400 Subject: [PATCH] gas price must > 0 (#8270) --- include/xrpl/protocol/Fees.h | 10 +- src/libxrpl/tx/transactors/system/Change.cpp | 3 +- src/test/app/FeeVote_test.cpp | 189 ++++++++++++++++++- src/xrpld/app/misc/FeeVoteImpl.cpp | 37 +++- src/xrpld/core/detail/Config.cpp | 2 +- 5 files changed, 220 insertions(+), 21 deletions(-) diff --git a/include/xrpl/protocol/Fees.h b/include/xrpl/protocol/Fees.h index 9f6357d214..0ebe080efd 100644 --- a/include/xrpl/protocol/Fees.h +++ b/include/xrpl/protocol/Fees.h @@ -11,15 +11,17 @@ namespace xrpl { inline constexpr std::uint32_t kFeeUnitsDeprecated = 10; // Number of micro-drops in one drop. -constexpr std::uint32_t microDropsPerDrop{1'000'000}; +inline constexpr std::uint32_t microDropsPerDrop{1'000'000}; /** - * Hard protocol ceilings on the Feature Extension fee settings. A voted value - * can never exceed these, so `preflight`, which has no view, may bound against - * them. + * Hard protocol bounds on the Feature Extension fee settings. A voted value + * can never fall outside these, so `preflight`, which has no view, may bound + * against them. Gas and bytecode limits are capped; gas price has a floor + * (zero would make WASM execution effectively free). */ inline constexpr std::uint32_t kMaxGasLimit{2'000'000}; inline constexpr std::uint32_t kMaxBytecodeSizeLimit{200'000}; +inline constexpr std::uint32_t kMinGasPrice{1}; // The following default values of fee settings will seed into FeeSettings // and write to the ledger on featureSmartEscrow activation. diff --git a/src/libxrpl/tx/transactors/system/Change.cpp b/src/libxrpl/tx/transactors/system/Change.cpp index f466dbe538..0300bdeae4 100644 --- a/src/libxrpl/tx/transactors/system/Change.cpp +++ b/src/libxrpl/tx/transactors/system/Change.cpp @@ -131,7 +131,8 @@ Change::preclaim(PreclaimContext const& ctx) !ctx.tx.isFieldPresent(sfGasPrice)) return temMALFORMED; if (ctx.tx[sfGasLimit] > kMaxGasLimit || - ctx.tx[sfBytecodeSizeLimit] > kMaxBytecodeSizeLimit) + ctx.tx[sfBytecodeSizeLimit] > kMaxBytecodeSizeLimit || + ctx.tx[sfGasPrice] < kMinGasPrice) return temBAD_FEE; } else diff --git a/src/test/app/FeeVote_test.cpp b/src/test/app/FeeVote_test.cpp index e4c7186bab..f36a90e876 100644 --- a/src/test/app/FeeVote_test.cpp +++ b/src/test/app/FeeVote_test.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include #include @@ -351,6 +352,20 @@ class FeeVote_test : public beast::unit_test::Suite BEAST_EXPECT(setup.gasLimit == kMaxGasLimit); BEAST_EXPECT(setup.bytecodeSizeLimit == kMaxBytecodeSizeLimit); } + { + // Zero is below kMinGasPrice, so the default is kept. + Section config; + config.append("gas_price = 0"); + auto const setup = setupFeeVote(config); + BEAST_EXPECT(setup.gasPrice == defaultSetup.gasPrice); + } + { + // The floor is inclusive: a configured price of 1 is accepted. + Section config; + config.append({"gas_price = " + std::to_string(kMinGasPrice)}); + auto const setup = setupFeeVote(config); + BEAST_EXPECT(setup.gasPrice == kMinGasPrice); + } } void @@ -447,7 +462,7 @@ class FeeVote_test : public beast::unit_test::Suite BEAST_EXPECT(verifyFeeObject(ledger, ledger->rules(), fields)); } - // Test that Smart Escrow limits reject values above their maximums. + // Test that Smart Escrow limits reject values outside their bounds. { jtx::Env env(*this, jtx::testableAmendments()); auto ledger = std::make_shared( @@ -479,6 +494,43 @@ class FeeVote_test : public beast::unit_test::Suite .gasLimit = kMaxGasLimit, .bytecodeSizeLimit = kMaxBytecodeSizeLimit + 1, .gasPrice = 300}); + // gasPrice == 0 is temBAD_FEE; gasLimit == 0 remains a valid kill + // switch. + testBadFields( + {.baseFeeDrops = XRPAmount{10}, + .reserveBaseDrops = XRPAmount{200000}, + .reserveIncrementDrops = XRPAmount{50000}, + .gasLimit = kMaxGasLimit, + .bytecodeSizeLimit = kMaxBytecodeSizeLimit, + .gasPrice = 0}); + } + + // ttFEE at exactly kMinGasPrice applies and is stored. + { + jtx::Env env(*this, jtx::testableAmendments()); + auto ledger = std::make_shared( + kCreateGenesis, + Rules{env.app().config().features}, + env.app().config().fees.toFees(), + std::vector{}, + env.app().getNodeFamily()); + + ledger = std::make_shared(*ledger, env.app().getTimeKeeper().closeTime()); + + FeeSettingsFields const fields{ + .baseFeeDrops = XRPAmount{10}, + .reserveBaseDrops = XRPAmount{200000}, + .reserveIncrementDrops = XRPAmount{50000}, + .gasLimit = 100, + .bytecodeSizeLimit = 200, + .gasPrice = kMinGasPrice}; + auto feeTx = createFeeTx(ledger->rules(), ledger->seq(), fields); + + OpenView accum(ledger.get()); + BEAST_EXPECT(isTesSuccess(applyFeeAndTestResult(env, accum, feeTx))); + accum.apply(*ledger); + + BEAST_EXPECT(verifyFeeObject(ledger, ledger->rules(), fields)); } // Test that the Smart Escrow fields are rejected if the @@ -846,6 +898,34 @@ class FeeVote_test : public beast::unit_test::Suite BEAST_EXPECT(val->isFieldPresent(sfBaseFee)); BEAST_EXPECT(val->getFieldU64(sfBaseFee) == setup.referenceFee); } + + // A local target of 0 is not emitted on the validation; the field is + // omitted so peers treat it as noVote. + { + Env env(*this, testableAmendments()); + FeeSetup zeroPrice = setup; + zeroPrice.gasPrice = 0; + auto feeVote = makeFeeVote(zeroPrice, env.app().getJournal("FeeVote")); + + auto ledger = std::make_shared( + kCreateGenesis, + Rules{env.app().config().features}, + env.app().config().fees.toFees(), + std::vector{}, + env.app().getNodeFamily()); + + auto sec = randomSecretKey(); + auto pub = derivePublicKey(KeyType::Secp256k1, sec); + + auto val = std::make_shared( + env.app().getTimeKeeper().now(), pub, sec, calcNodeID(pub), [](STValidation& v) { + v.setFieldU32(sfLedgerSequence, 12345); + }); + + feeVote->doValidation(ledger->fees(), ledger->rules(), *val); + + BEAST_EXPECT(!val->isFieldPresent(sfGasPrice)); + } } void @@ -952,8 +1032,18 @@ class FeeVote_test : public beast::unit_test::Suite BEAST_EXPECT(env.current()->fees().bytecodeSizeLimit == kDefaultBytecodeSizeLimit); BEAST_EXPECT(env.current()->fees().gasPrice == kDefaultGasPrice); + struct SeVoteOpts + { + // If set, each validation uses the returned price; nullopt omits + // sfGasPrice. If unset, every validation uses setup.gasPrice. + std::function(int)> gasPrice; + int nValidations = 5; + bool trustAll = false; + }; + auto const createFeeTxFromVoting = - [&](FeeSetup const& setup) -> std::pair> { + [&](FeeSetup const& setup, + SeVoteOpts const& opts = {}) -> std::pair> { auto feeVote = makeFeeVote(setup, env.app().getJournal("FeeVote")); auto ledger = std::make_shared( kCreateGenesis, @@ -974,7 +1064,7 @@ class FeeVote_test : public beast::unit_test::Suite // Create some mock validations with fee votes std::vector> validations; - for (int i = 0; i < 5; i++) + for (int i = 0; i < opts.nValidations; i++) { auto sec = randomSecretKey(); auto pub = derivePublicKey(KeyType::Secp256k1, sec); @@ -992,9 +1082,17 @@ class FeeVote_test : public beast::unit_test::Suite v.setFieldAmount(sfReserveIncrementDrops, XRPAmount{setup.ownerReserve}); v.setFieldU32(sfGasLimit, setup.gasLimit); v.setFieldU32(sfBytecodeSizeLimit, setup.bytecodeSizeLimit); - v.setFieldU32(sfGasPrice, setup.gasPrice); + if (opts.gasPrice) + { + if (auto const price = opts.gasPrice(i)) + v.setFieldU32(sfGasPrice, *price); + } + else + { + v.setFieldU32(sfGasPrice, setup.gasPrice); + } }); - if (i % 2) + if (opts.trustAll || (i % 2)) val->setTrusted(); validations.push_back(val); } @@ -1083,6 +1181,87 @@ class FeeVote_test : public beast::unit_test::Suite setup.bytecodeSizeLimit = ledger->fees().bytecodeSizeLimit; checkFeeTx(setup, feeTx, ledger); } + + // Local and peer votes of 0 are ignored; the fee tx keeps the ledger + // gas price. Other fee fields still change, so a ttFEE is produced. + { + FeeSetup setup; + setup.referenceFee = 42; + setup.accountReserve = 1234567; + setup.ownerReserve = 7654321; + setup.gasLimit = 100; + setup.bytecodeSizeLimit = 200; + setup.gasPrice = 0; + auto const [feeTx, ledger] = createFeeTxFromVoting(setup); + + setup.gasPrice = ledger->fees().gasPrice; + checkFeeTx(setup, feeTx, ledger); + } + + // Absent sfGasPrice is an abstention (noVote), not a vote for 0. + { + FeeSetup setup; + setup.referenceFee = 42; + setup.accountReserve = 1234567; + setup.ownerReserve = 7654321; + setup.gasLimit = 100; + setup.bytecodeSizeLimit = 200; + setup.gasPrice = 300; + auto const [feeTx, ledger] = createFeeTxFromVoting( + setup, + {.gasPrice = [](int) -> std::optional { return std::nullopt; }}); + + setup.gasPrice = ledger->fees().gasPrice; + checkFeeTx(setup, feeTx, ledger); + } + + // Invalid (0) votes are noVote (weight on current), not dropped. + // Four zeros and one 300: current outweighs 300. If zeros were + // ignored, the local 300 target would still win. + { + FeeSetup setup; + setup.referenceFee = 42; + setup.accountReserve = 1234567; + setup.ownerReserve = 7654321; + setup.gasLimit = 100; + setup.bytecodeSizeLimit = 200; + setup.gasPrice = 300; + auto const [feeTx, ledger] = createFeeTxFromVoting( + setup, + {.gasPrice = [](int i) -> std::optional { return i == 0 ? 300 : 0; }, + .trustAll = true}); + + setup.gasPrice = ledger->fees().gasPrice; + checkFeeTx(setup, feeTx, ledger); + } + + // doVote accepts the inclusive floor (field >= kMinGasPrice). + { + FeeSetup setup; + setup.referenceFee = 42; + setup.accountReserve = 1234567; + setup.ownerReserve = 7654321; + setup.gasLimit = 100; + setup.bytecodeSizeLimit = 200; + setup.gasPrice = kMinGasPrice; + auto const [feeTx, ledger] = createFeeTxFromVoting(setup); + + checkFeeTx(setup, feeTx, ledger); + } + + // There is no protocol max for gas price; UINT32_MAX is a legal vote. + { + FeeSetup setup; + setup.referenceFee = 42; + setup.accountReserve = 1234567; + setup.ownerReserve = 7654321; + setup.gasLimit = 100; + setup.bytecodeSizeLimit = 200; + setup.gasPrice = std::numeric_limits::max(); + auto const [feeTx, ledger] = createFeeTxFromVoting(setup); + + checkFeeTx(setup, feeTx, ledger); + } } // Activation cannot be driven through consensus here, so the ledger is diff --git a/src/xrpld/app/misc/FeeVoteImpl.cpp b/src/xrpld/app/misc/FeeVoteImpl.cpp index 26bf6bd24b..f7e95f13e1 100644 --- a/src/xrpld/app/misc/FeeVoteImpl.cpp +++ b/src/xrpld/app/misc/FeeVoteImpl.cpp @@ -184,7 +184,10 @@ FeeVoteImpl::doValidation(Fees const& lastFees, Rules const& rules, STValidation "bytecode size limit", sfBytecodeSizeLimit); } - vote(lastFees.gasPrice, target_.gasPrice, "gas price", sfGasPrice); + if (target_.gasPrice >= kMinGasPrice) + { + vote(lastFees.gasPrice, target_.gasPrice, "gas price", sfGasPrice); + } } } @@ -205,22 +208,30 @@ FeeVoteImpl::doVoting( detail::VotableValue incReserveVote(lastClosedLedger->fees().increment, target_.ownerReserve); - auto validOrCurrent = [](std::uint32_t target, std::uint32_t max, std::uint32_t current) { - return target <= max ? target : current; - }; + auto validOrCurrent = + [](std::uint32_t target, std::uint32_t min, std::uint32_t max, std::uint32_t current) { + return (target >= min && target <= max) ? target : current; + }; detail::VotableValue gasLimitVote( lastClosedLedger->fees().gasLimit, - validOrCurrent(target_.gasLimit, kMaxGasLimit, lastClosedLedger->fees().gasLimit)); + validOrCurrent(target_.gasLimit, 0, kMaxGasLimit, lastClosedLedger->fees().gasLimit)); detail::VotableValue bytecodeSizeLimitVote( lastClosedLedger->fees().bytecodeSizeLimit, validOrCurrent( target_.bytecodeSizeLimit, + 0, kMaxBytecodeSizeLimit, lastClosedLedger->fees().bytecodeSizeLimit)); - detail::VotableValue gasPriceVote(lastClosedLedger->fees().gasPrice, target_.gasPrice); + detail::VotableValue gasPriceVote( + lastClosedLedger->fees().gasPrice, + validOrCurrent( + target_.gasPrice, + kMinGasPrice, + std::numeric_limits::max(), + lastClosedLedger->fees().gasPrice)); auto const& rules = lastClosedLedger->rules(); if (rules.enabled(featureXRPFees)) @@ -297,10 +308,11 @@ FeeVoteImpl::doVoting( auto doVote = [](std::shared_ptr const& val, detail::VotableValue& value, SF_UINT32 const& sfield, + std::uint32_t minValue, std::uint32_t maxValue) { if (auto const field = ~val->at(~sfield); field) { - if (field.value() <= maxValue) + if (field.value() >= minValue && field.value() <= maxValue) { value.addVote(field.value()); } @@ -319,9 +331,14 @@ FeeVoteImpl::doVoting( { if (!val->isTrusted()) continue; - doVote(val, gasLimitVote, sfGasLimit, kMaxGasLimit); - doVote(val, bytecodeSizeLimitVote, sfBytecodeSizeLimit, kMaxBytecodeSizeLimit); - doVote(val, gasPriceVote, sfGasPrice, std::numeric_limits::max()); + doVote(val, gasLimitVote, sfGasLimit, 0, kMaxGasLimit); + doVote(val, bytecodeSizeLimitVote, sfBytecodeSizeLimit, 0, kMaxBytecodeSizeLimit); + doVote( + val, + gasPriceVote, + sfGasPrice, + kMinGasPrice, + std::numeric_limits::max()); } } diff --git a/src/xrpld/core/detail/Config.cpp b/src/xrpld/core/detail/Config.cpp index d8002f483c..9a6caa13e2 100644 --- a/src/xrpld/core/detail/Config.cpp +++ b/src/xrpld/core/detail/Config.cpp @@ -1258,7 +1258,7 @@ setupFeeVote(Section const& section) setup.gasLimit = temp; if (set(temp, Keys::kBytecodeSizeLimit, section) && temp <= kMaxBytecodeSizeLimit) setup.bytecodeSizeLimit = temp; - if (set(temp, Keys::kGasPrice, section)) + if (set(temp, Keys::kGasPrice, section) && temp >= kMinGasPrice) setup.gasPrice = temp; } return setup;