Merge bitcoin/bitcoin#33189: rpc: followups for 33106

daa40a3 doc fixups for 33106 (glozow)
c568511 test fixup for incremental feerate (glozow)
636fa21 test fixups (glozow)
9169a50 [rpc] expose blockmintxfee via getmininginfo (glozow)

Pull request description:

  Followups from #33106:
  - bitcoin/bitcoin#33106 (comment)
  - bitcoin/bitcoin#33106 (comment)
  - bitcoin/bitcoin#33106 (comment)
  - bitcoin/bitcoin#33106 (comment)
  - bitcoin/bitcoin#33106 (comment)
  - bitcoin/bitcoin#33106 (comment)
  - bitcoin/bitcoin#33106 (comment)
  - bitcoin/bitcoin#33106 (comment)
  - bitcoin/bitcoin#33106 (comment)
  - bitcoin/bitcoin#33106 (comment)
  - bitcoin/bitcoin#33106 (comment)

ACKs for top commit:
  ajtowns:
    ACK daa40a3 ; cursory review, seems reasonable
  davidgumberg:
    ACK bitcoin/bitcoin@daa40a3
  instagibbs:
    ACK daa40a3

Tree-SHA512: d6f0ae5d00dadfbaf0998ac332c8536c997628de4f2b9947eb57712f05d3afa19a823c9cc007435be320640cd13a4c500db20c9606988cdd371934496dec009d

Author: fanquake

doc/release-notes-33106.md

@@ -9,8 +9,7 @@ changed to 100 satoshis per kvB. They can still be changed using their respectiv
 recommended to change both together if you decide to do so.
 
 Other minimum feerates (e.g. the dust feerate, the minimum returned by the fee estimator, and all feerates used by the
-wallet) remain unchanged. The mempool minimum feerate still changes in response to high volume but more gradually, as a
-result of the change to the incremental relay feerate.
+wallet) remain unchanged. The mempool minimum feerate still changes in response to high volume.
 
 Note that unless these lower defaults are widely adopted across the network, transactions created with lower fee rates
 are not guaranteed to propagate or confirm. The wallet feerates remain unchanged; `-mintxfee` must be changed before

src/rpc/mining.cpp

@@ -429,6 +429,7 @@ static RPCHelpMan getmininginfo()
                         {RPCResult::Type::STR_HEX, "target", "The current target"},
                         {RPCResult::Type::NUM, "networkhashps", "The network hashes per second"},
                         {RPCResult::Type::NUM, "pooledtx", "The size of the mempool"},
+                        {RPCResult::Type::STR_AMOUNT, "blockmintxfee", "Minimum feerate of packages selected for block inclusion in " + CURRENCY_UNIT + "/kvB"},
                         {RPCResult::Type::STR, "chain", "current network name (" LIST_CHAIN_NAMES ")"},
                         {RPCResult::Type::STR_HEX, "signet_challenge", /*optional=*/true, "The block challenge (aka. block script), in hexadecimal (only present if the current network is a signet)"},
                         {RPCResult::Type::OBJ, "next", "The next block",
@@ -469,6 +470,9 @@ static RPCHelpMan getmininginfo()
     obj.pushKV("target", GetTarget(tip, chainman.GetConsensus().powLimit).GetHex());
     obj.pushKV("networkhashps",    getnetworkhashps().HandleRequest(request));
     obj.pushKV("pooledtx",         (uint64_t)mempool.size());
+    BlockAssembler::Options assembler_options;
+    ApplyArgsManOptions(*node.args, assembler_options);
+    obj.pushKV("blockmintxfee", ValueFromAmount(assembler_options.blockMinFeeRate.GetFeePerK()));
     obj.pushKV("chain", chainman.GetParams().GetChainTypeString());
 
     UniValue next(UniValue::VOBJ);

src/test/miner_tests.cpp

@@ -218,7 +218,8 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vout[0].nValue = 5000000000LL - 100000000;
     tx.vout[1].nValue = 100000000; // 1BTC output
     // Increase size to avoid rounding errors: when the feerate is extremely small (i.e. 1sat/kvB), evaluating the fee
-    // at a smaller transaction size gives us a rounded value of 0.
+    // at smaller sizes gives us rounded values that are equal to each other, which means we incorrectly include
+    // hashFreeTx2 + hashLowFeeTx2.
     BulkTransaction(tx, 4000);
     Txid hashFreeTx2 = tx.GetHash();
     AddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(true).FromTx(tx));

test/functional/feature_rbf.py

@@ -13,7 +13,6 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    assert_greater_than,
     assert_greater_than_or_equal,
     assert_raises_rpc_error,
     get_fee,
@@ -584,7 +583,7 @@ def test_replacement_relay_fee(self):
         tx = self.wallet.send_self_transfer(from_node=self.nodes[0])['tx']
 
         # Higher fee, higher feerate, different txid, but the replacement does not provide a relay
-        # fee conforming to node's `incrementalrelayfee` policy of 1000 sat per KB.
+        # fee conforming to node's `incrementalrelayfee` policy of 100 sat per KB.
         assert_equal(self.nodes[0].getmempoolinfo()["incrementalrelayfee"], Decimal("0.000001"))
         tx.vout[0].nValue -= 1
         assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx.serialize().hex())
@@ -594,7 +593,7 @@ def test_incremental_relay_feerates(self):
         node = self.nodes[0]
         for incremental_setting in (0, 5, 10, 50, 100, 234, 1000, 5000, 21000):
             incremental_setting_decimal = incremental_setting / Decimal(COIN)
-            self.log.info(f"-> Test -incrementalrelayfee={incremental_setting_decimal:.8f}sat/kvB...")
+            self.log.info(f"-> Test -incrementalrelayfee={incremental_setting:.8f}sat/kvB...")
             self.restart_node(0, extra_args=[f"-incrementalrelayfee={incremental_setting_decimal:.8f}", "-persistmempool=0"])
 
             # When incremental relay feerate is higher than min relay feerate, min relay feerate is automatically increased.
@@ -603,23 +602,27 @@ def test_incremental_relay_feerates(self):
 
             low_feerate = min_relay_feerate * 2
             confirmed_utxo = self.wallet.get_utxo(confirmed_only=True)
-            replacee_tx = self.wallet.create_self_transfer(utxo_to_spend=confirmed_utxo, fee_rate=low_feerate, target_vsize=5000)
+            # Use different versions to avoid creating an identical transaction when failed_replacement_tx is created.
+            # Use a target vsize that is small, but something larger than the minimum so that we can create a transaction that is 1vB smaller later.
+            replacee_tx = self.wallet.create_self_transfer(utxo_to_spend=confirmed_utxo, fee_rate=low_feerate, version=3, target_vsize=200)
             node.sendrawtransaction(replacee_tx['hex'])
 
-            replacement_placeholder_tx = self.wallet.create_self_transfer(utxo_to_spend=confirmed_utxo)
+            replacement_placeholder_tx = self.wallet.create_self_transfer(utxo_to_spend=confirmed_utxo, target_vsize=200)
             replacement_expected_size = replacement_placeholder_tx['tx'].get_vsize()
             replacement_required_fee = get_fee(replacement_expected_size, incremental_setting_decimal) + replacee_tx['fee']
 
-            # Should always be required to pay additional fees
-            if incremental_setting > 0:
-                assert_greater_than(replacement_required_fee, replacee_tx['fee'])
-
-            # 1 satoshi shy of the required fee
-            failed_replacement_tx = self.wallet.create_self_transfer(utxo_to_spend=confirmed_utxo, fee=replacement_required_fee - Decimal("0.00000001"))
+            # Show that replacement fails when paying 1 satoshi shy of the required fee
+            failed_replacement_tx = self.wallet.create_self_transfer(utxo_to_spend=confirmed_utxo, fee=replacement_required_fee - Decimal("0.00000001"), version=2, target_vsize=200)
             assert_raises_rpc_error(-26, "insufficient fee", node.sendrawtransaction, failed_replacement_tx['hex'])
+            replacement_tx = self.wallet.create_self_transfer(utxo_to_spend=confirmed_utxo, fee=replacement_required_fee, version=2, target_vsize=200)
 
-            replacement_tx = self.wallet.create_self_transfer(utxo_to_spend=confirmed_utxo, fee=replacement_required_fee)
-            node.sendrawtransaction(replacement_tx['hex'])
+            if incremental_setting == 0:
+                # When incremental relay feerate is 0, additional fees are not required, but higher feerate is still required.
+                assert_raises_rpc_error(-26, "insufficient fee", node.sendrawtransaction, replacement_tx['hex'])
+                replacement_tx_smaller = self.wallet.create_self_transfer(utxo_to_spend=confirmed_utxo, fee=replacement_required_fee, version=2, target_vsize=199)
+                node.sendrawtransaction(replacement_tx_smaller['hex'])
+            else:
+                node.sendrawtransaction(replacement_tx['hex'])
 
     def test_fullrbf(self):
         # BIP125 signaling is not respected

test/functional/mempool_package_rbf.py

@@ -168,7 +168,7 @@ def test_package_rbf_additional_fees(self):
         failure_package_hex3, failure_package_txns3 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_CHILD_FEE + incremental_sats_short)
         assert_equal(package_3_size, sum([tx.get_vsize() for tx in failure_package_txns3]))
         pkg_results3 = node.submitpackage(failure_package_hex3)
-        assert_equal(f"package RBF failed: insufficient anti-DoS fees, rejecting replacement {failure_package_txns3[1].txid_hex}, not enough additional fees to relay; {incremental_sats_short:8f} < {incremental_sats_required:8f}", pkg_results3["package_msg"])
+        assert_equal(f"package RBF failed: insufficient anti-DoS fees, rejecting replacement {failure_package_txns3[1].txid_hex}, not enough additional fees to relay; {incremental_sats_short:.8f} < {incremental_sats_required:.8f}", pkg_results3["package_msg"])
         self.assert_mempool_contents(expected=package_txns1)
 
         success_package_hex3, success_package_txns3 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_CHILD_FEE + incremental_sats_required)

test/functional/mempool_truc.py

@@ -617,6 +617,10 @@ def test_minrelay_in_package_combos(self):
                 assert_greater_than(get_fee(tx_v3_0fee_parent["tx"].get_vsize(), minrelayfeerate), 0)
                 # Always need to pay at least 1 satoshi for entry, even if minimum feerate is very low
                 assert_greater_than(total_v3_fee, 0)
+                # Also create a version where the child is at minrelaytxfee
+                tx_v3_child_minrelay = self.wallet.create_self_transfer(utxo_to_spend=tx_v3_0fee_parent["new_utxo"], fee_rate=minrelayfeerate, version=3)
+                result_truc_minrelay = node.submitpackage([tx_v3_0fee_parent["hex"], tx_v3_child_minrelay["hex"]])
+                assert_equal(result_truc_minrelay["package_msg"], "transaction failed")
 
             tx_v2_0fee_parent = self.wallet.create_self_transfer(fee=0, fee_rate=0, confirmed_only=True, version=2)
             tx_v2_child = self.wallet.create_self_transfer(utxo_to_spend=tx_v2_0fee_parent["new_utxo"], fee_rate=high_feerate, version=2)

test/functional/mining_basic.py

@@ -152,18 +152,20 @@ def test_blockmintxfee_parameter(self):
                 blockmintxfee_parameter = f"-blockmintxfee={blockmintxfee_btc_kvb:.8f}"
                 self.log.info(f"-> Test {blockmintxfee_parameter} ({blockmintxfee_sat_kvb} sat/kvB)...")
                 self.restart_node(0, extra_args=[blockmintxfee_parameter, '-minrelaytxfee=0', '-persistmempool=0'])
-                self.wallet.rescan_utxos()  # to avoid spending outputs of txs that are not in mempool anymore after restart
+            assert_equal(node.getmininginfo()['blockmintxfee'], blockmintxfee_btc_kvb)
 
             # submit one tx with exactly the blockmintxfee rate, and one slightly below
             tx_with_min_feerate = self.wallet.send_self_transfer(from_node=node, fee_rate=blockmintxfee_btc_kvb, confirmed_only=True)
             assert_equal(tx_with_min_feerate["fee"], get_fee(tx_with_min_feerate["tx"].get_vsize(), blockmintxfee_btc_kvb))
-            if blockmintxfee_sat_kvb > 5:
+            if blockmintxfee_sat_kvb >= 10:
                 lowerfee_btc_kvb = blockmintxfee_btc_kvb - Decimal(10)/COIN  # 0.01 sat/vbyte lower
+                assert_greater_than(blockmintxfee_btc_kvb, lowerfee_btc_kvb)
+                assert_greater_than_or_equal(lowerfee_btc_kvb, 0)
                 tx_below_min_feerate = self.wallet.send_self_transfer(from_node=node, fee_rate=lowerfee_btc_kvb, confirmed_only=True)
                 assert_equal(tx_below_min_feerate["fee"], get_fee(tx_below_min_feerate["tx"].get_vsize(), lowerfee_btc_kvb))
             else:  # go below zero fee by using modified fees
                 tx_below_min_feerate = self.wallet.send_self_transfer(from_node=node, fee_rate=blockmintxfee_btc_kvb, confirmed_only=True)
-                node.prioritisetransaction(tx_below_min_feerate["txid"], 0, -1)
+                node.prioritisetransaction(tx_below_min_feerate["txid"], 0, -11)
 
             # check that tx below specified fee-rate is neither in template nor in the actual block
             block_template = node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS)

test/functional/wallet_bumpfee.py

@@ -848,12 +848,12 @@ def test_bumpfee_with_feerate_ignores_walletincrementalrelayfee(self, rbf_node,
     assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, tx["txid"], {"fee_rate": 1})
     assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, tx["txid"], {"fee_rate": 2})
 
-    # Ensure you can not fee bump if the fee_rate is more than original fee_rate but the total fee from new fee_rate is
-    # less than (original fee + incrementalrelayfee)
-    assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, tx["txid"], {"fee_rate": 2.05})
+    # Ensure you can not fee bump if the fee_rate is more than original fee_rate but the additional fee does
+    # not cover incrementalrelayfee for the size of the replacement transaction
+    assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, tx["txid"], {"fee_rate": 2.09})
 
     # You can fee bump as long as the new fee set from fee_rate is at least (original fee + incrementalrelayfee)
-    rbf_node.bumpfee(tx["txid"], {"fee_rate": 3})
+    rbf_node.bumpfee(tx["txid"], {"fee_rate": 2.1})
     self.clear_mempool()