fix: Don't vote on gas settings before Smart Escrow is enabled

`doVoting` built the three gas votes unconditionally. Before the amendment,
the ledger reports zero for all three while the config targets are non-zero by
default, and the loop that collects validator votes is itself gated on the
amendment — so the vote map held only our own target and every one of the
three reported a change.

The result was a `SetFee` pseudo-transaction on every flag ledger, from every
node running this build, for as long as the amendment stayed disabled. The
transaction carried no gas fields, since that part was gated correctly, so it
proposed no change at all.

Gate the three flags on the amendment, matching `doValidation`, which already
had its gas votes inside the same check.

Reported by xrplf-ai-reviewer on #8214.
This commit is contained in:
Mayukha Vadari
2026-09-10 18:03:41 -04:00
parent e4476e5821
commit 052782a3d4
2 changed files with 74 additions and 2 deletions

View File

@@ -988,6 +988,71 @@ class FeeVote_test : public beast::unit_test::Suite
}
}
void
testDoVotingNoChangePreSmartEscrow()
{
testcase("doVoting votes for nothing before Smart Escrow");
using namespace jtx;
// A ledger from before the amendment reports zero for all three gas
// settings, while the config targets are non-zero by default. Those
// three must not count as a change, or every node emits a SetFee on
// every flag ledger for as long as the amendment is off.
Env env(*this, testableAmendments() - featureSmartEscrow);
FeeSetup setup;
setup.referenceFee = UNIT_TEST_REFERENCE_FEE;
setup.accountReserve = 200'000'000;
setup.ownerReserve = 50'000'000;
BEAST_EXPECT(setup.gasLimit != 0);
BEAST_EXPECT(setup.bytecodeSizeLimit != 0);
BEAST_EXPECT(setup.gasPrice != 0);
// The three-argument Fees leaves the gas settings at zero, which is
// what Ledger::setup() reads back from a pre-amendment FeeSettings.
Fees const ledgerFees{setup.referenceFee, setup.accountReserve, setup.ownerReserve};
auto feeVote = makeFeeVote(setup, env.app().getJournal("FeeVote"));
auto ledger = std::make_shared<Ledger>(
kCreateGenesis,
Rules{env.app().config().features},
ledgerFees,
std::vector<uint256>{},
env.app().getNodeFamily());
for (int i = 0; i < 256 - 1; ++i)
{
ledger = std::make_shared<Ledger>(*ledger, env.app().getTimeKeeper().closeTime());
}
BEAST_EXPECT(ledger->isFlagLedger());
BEAST_EXPECT(ledger->fees().gasLimit == 0);
std::vector<std::shared_ptr<STValidation>> validations;
for (int i = 0; i < 5; i++)
{
auto sec = randomSecretKey();
auto pub = derivePublicKey(KeyType::Secp256k1, sec);
auto val = std::make_shared<STValidation>(
env.app().getTimeKeeper().now(), pub, sec, calcNodeID(pub), [&](STValidation& v) {
v.setFieldU32(sfLedgerSequence, ledger->seq());
// Everyone is content with the fees as they stand.
v.setFieldAmount(sfBaseFeeDrops, XRPAmount{setup.referenceFee});
v.setFieldAmount(sfReserveBaseDrops, XRPAmount{setup.accountReserve});
v.setFieldAmount(sfReserveIncrementDrops, XRPAmount{setup.ownerReserve});
});
if ((i % 2) != 0)
val->setTrusted();
validations.push_back(val);
}
auto txSet = std::make_shared<SHAMap>(SHAMapType::TRANSACTION, env.app().getNodeFamily());
feeVote->doVoting(ledger, validations, txSet);
BEAST_EXPECT(getTxs(txSet).empty());
}
void
testDoVotingSmartEscrow()
{
@@ -1152,6 +1217,7 @@ class FeeVote_test : public beast::unit_test::Suite
testDoValidation();
testDoVoting();
testGenesisFeeSettings();
testDoVotingNoChangePreSmartEscrow();
testDoVotingSmartEscrow();
}
};

View File

@@ -336,8 +336,14 @@ FeeVoteImpl::doVoting(
auto const seq = lastClosedLedger->header().seq + 1;
// add transactions to our position
if (baseFeeChanged || baseReserveChanged || incReserveChanged || gasLimitChanged ||
bytecodeSizeLimitChanged || gasPriceChanged)
//
// The gas votes only count once the amendment is on. Before it is, the
// ledger reports zero for all three while the config targets are
// non-zero, so they would report a change on every flag ledger and have
// us emit a SetFee that carries no gas fields and changes nothing.
if (baseFeeChanged || baseReserveChanged || incReserveChanged ||
(rules.enabled(featureSmartEscrow) &&
(gasLimitChanged || bytecodeSizeLimitChanged || gasPriceChanged)))
{
JLOG(journal_.warn()) << "We are voting for a fee change: " << baseFee << "/" << baseReserve
<< "/" << incReserve;