Skip to content

Commit e3f416d

Browse files
committed
Merge bitcoin/bitcoin#32463: test: fix an incorrect feature_fee_estimation.py subtest
9b75cfd test: retain the intended behavior of `feature_fee_estimation.py` nodes (ismaelsadeeq) 5c1236f test: fix incorrect subtest in `feature_fee_estimation.py` (ismaelsadeeq) Pull request description: Attempt to fix #32461 In the `estimatesmartfee` RPC, we return the maximum of the following: the feerate estimate for the target, `minrelaytxfee`, and `mempoolminfee`. https://github.com/bitcoin/bitcoin/blob/9a05b45da60d214cb1e5a50c3d2293b1defc9bb0/src/rpc/fees.cpp#L85 The test `test_feerate_mempoolminfee`, originally introduced in bitcoin/bitcoin@ea31caf, is incorrect. It should calculate the fee rate ceiling by taking the maximum of the custom `minrelaytxfee`, `mempoolminfee`, and the highest fee rate observed during the test (`check_smart_estimates`). This is necessary because: * There is no guarantee that the generated fee rates will exceed both `minrelaytxfee` and `mempoolminfee`. * Users can start a node with custom fee settings. Due to the non-deterministic nature of the `feature_fee_estimation.py` test, it often passes by chance. The randomly generated fees typically include a value higher than the custom `minrelaytxfee`, inadvertently hiding the issue. Issue #32461 identified a random seeds that consistently fails the test because the generated fees never exceed the custom `minrelaytxfee`: e.g ``` build/test/functional/feature_fee_estimation.py --random=3450808900320758527 ``` This PR has two commits which : * Correctly fixes the test by calculating the fee rate ceiling as the maximum of the node `minrelaytxfee`, `mempoolminfee`, and the highest seen fee rate, when verifying smart fee estimates. * Improves the subtest name and comment for clarity. * Restores the original test behavior by appending 4000 WU to the custom `blockmaxweight`. ACKs for top commit: achow101: ACK 9b75cfd glozow: ACK 9b75cfd theStack: Light ACK 9b75cfd Tree-SHA512: 0f7fb0496b50a399b58f6fb1afd95414fad454795fbc0046e22dfc54a2062ae0c519a12ebfeb6ad7ef547438868d99eca2351c0d19d0346adaadb500eff6f15f
2 parents ea42857 + 9b75cfd commit e3f416d

File tree

1 file changed

+12
-11
lines changed

1 file changed

+12
-11
lines changed

test/functional/feature_fee_estimation.py

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,19 +90,20 @@ def check_smart_estimates(node, fees_seen):
9090
"""Call estimatesmartfee and verify that the estimates meet certain invariants."""
9191

9292
delta = 1.0e-6 # account for rounding error
93-
last_feerate = float(max(fees_seen))
9493
all_smart_estimates = [node.estimatesmartfee(i) for i in range(1, 26)]
9594
mempoolMinFee = node.getmempoolinfo()["mempoolminfee"]
9695
minRelaytxFee = node.getmempoolinfo()["minrelaytxfee"]
96+
feerate_ceiling = max(max(fees_seen), float(mempoolMinFee), float(minRelaytxFee))
97+
last_feerate = feerate_ceiling
9798
for i, e in enumerate(all_smart_estimates): # estimate is for i+1
9899
feerate = float(e["feerate"])
99100
assert_greater_than(feerate, 0)
100101
assert_greater_than_or_equal(feerate, float(mempoolMinFee))
101102
assert_greater_than_or_equal(feerate, float(minRelaytxFee))
102103

103-
if feerate + delta < min(fees_seen) or feerate - delta > max(fees_seen):
104+
if feerate + delta < min(fees_seen) or feerate - delta > feerate_ceiling:
104105
raise AssertionError(
105-
f"Estimated fee ({feerate}) out of range ({min(fees_seen)},{max(fees_seen)})"
106+
f"Estimated fee ({feerate}) out of range ({min(fees_seen)},{feerate_ceiling})"
106107
)
107108
if feerate - delta > last_feerate:
108109
raise AssertionError(
@@ -144,8 +145,8 @@ def set_test_params(self):
144145
self.noban_tx_relay = True
145146
self.extra_args = [
146147
[],
147-
["-blockmaxweight=68000"],
148-
["-blockmaxweight=32000"],
148+
["-blockmaxweight=72000"],
149+
["-blockmaxweight=36000"],
149150
]
150151

151152
def setup_network(self):
@@ -237,10 +238,10 @@ def sanity_check_estimates_range(self):
237238
self.log.info("Final estimates after emptying mempools")
238239
check_estimates(self.nodes[1], self.fees_per_kb)
239240

240-
def test_feerate_mempoolminfee(self):
241-
high_val = 3 * self.nodes[1].estimatesmartfee(1)["feerate"]
241+
def test_estimates_with_highminrelaytxfee(self):
242+
high_val = 3 * self.nodes[1].estimatesmartfee(2)["feerate"]
242243
self.restart_node(1, extra_args=[f"-minrelaytxfee={high_val}"])
243-
check_estimates(self.nodes[1], self.fees_per_kb)
244+
check_smart_estimates(self.nodes[1], self.fees_per_kb)
244245
self.restart_node(1)
245246

246247
def sanity_check_rbf_estimates(self, utxos):
@@ -451,11 +452,11 @@ def run_test(self):
451452
self.log.info("Test fee_estimates.dat is flushed periodically")
452453
self.test_estimate_dat_is_flushed_periodically()
453454

454-
# check that the effective feerate is greater than or equal to the mempoolminfee even for high mempoolminfee
455+
# check that estimatesmartfee feerate is greater than or equal to maximum of mempoolminfee and minrelaytxfee
455456
self.log.info(
456-
"Test fee rate estimation after restarting node with high MempoolMinFee"
457+
"Test fee rate estimation after restarting node with high minrelaytxfee"
457458
)
458-
self.test_feerate_mempoolminfee()
459+
self.test_estimates_with_highminrelaytxfee()
459460

460461
self.log.info("Test acceptstalefeeestimates option")
461462
self.test_acceptstalefeeestimates_option()

0 commit comments

Comments
 (0)