phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-24 18:23:26 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Remove -mempoolfullrbf option

Browse source
This commit is contained in:
Greg Sanders 2024-08-01 12:26:47 -04:00
parent fc642c33ef
commit 111a23d9b3
8 changed files with 8 additions and 230 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/init.cpp
View file

@ -633,7 +633,6 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
"is of this size or less (default: %u)",
MAX_OP_RETURN_RELAY),
ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-mempoolfullrbf", strprintf("(DEPRECATED) Accept transaction replace-by-fee without requiring replaceability signaling (default: %u)", DEFAULT_MEMPOOL_FULL_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-permitbaremultisig", strprintf("Relay transactions creating non-P2SH multisig outputs (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY,
OptionsCategory::NODE_RELAY);
argsman.AddArg("-minrelaytxfee=<amt>", strprintf("Fees (in %s/kvB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)",

3
src/kernel/mempool_options.h
View file

@ -21,8 +21,6 @@ static constexpr unsigned int DEFAULT_MAX_MEMPOOL_SIZE_MB{300};
static constexpr unsigned int DEFAULT_BLOCKSONLY_MAX_MEMPOOL_SIZE_MB{5};
/** Default for -mempoolexpiry, expiration time for mempool transactions in hours */
static constexpr unsigned int DEFAULT_MEMPOOL_EXPIRY_HOURS{336};
/** Default for -mempoolfullrbf, if the transaction replaceability signaling is ignored */
static constexpr bool DEFAULT_MEMPOOL_FULL_RBF{true};
/** Whether to fall back to legacy V1 serialization when writing mempool.dat */
static constexpr bool DEFAULT_PERSIST_V1_DAT{false};
/** Default for -acceptnonstdtxn */
@ -55,7 +53,6 @@ struct MemPoolOptions {
std::optional<unsigned> max_datacarrier_bytes{DEFAULT_ACCEPT_DATACARRIER ? std::optional{MAX_OP_RETURN_RELAY} : std::nullopt};
bool permit_bare_multisig{DEFAULT_PERMIT_BAREMULTISIG};
bool require_standard{true};
bool full_rbf{DEFAULT_MEMPOOL_FULL_RBF};
bool persist_v1_dat{DEFAULT_PERSIST_V1_DAT};
MemPoolLimits limits{};

5
src/node/mempool_args.cpp
View file

@ -92,11 +92,6 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& argsman, const CChainP
return util::Error{strprintf(Untranslated("acceptnonstdtxn is not currently supported for %s chain"), chainparams.GetChainTypeString())};
}
mempool_opts.full_rbf = argsman.GetBoolArg("-mempoolfullrbf", mempool_opts.full_rbf);
if (!mempool_opts.full_rbf) {
LogInfo("Warning: mempoolfullrbf=0 set but deprecated and will be removed in a future release\n");
}
mempool_opts.persist_v1_dat = argsman.GetBoolArg("-persistmempoolv1", mempool_opts.persist_v1_dat);
ApplyArgsManOptions(argsman, mempool_opts.limits);

2
src/rpc/mempool.cpp
View file

@ -679,7 +679,7 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool)
ret.pushKV("minrelaytxfee", ValueFromAmount(pool.m_opts.min_relay_feerate.GetFeePerK()));
ret.pushKV("incrementalrelayfee", ValueFromAmount(pool.m_opts.incremental_relay_feerate.GetFeePerK()));
ret.pushKV("unbroadcastcount", uint64_t{pool.GetUnbroadcastTxs().size()});
ret.pushKV("fullrbf", pool.m_opts.full_rbf);
ret.pushKV("fullrbf", true);
return ret;
}

24
src/validation.cpp
View file

@ -820,30 +820,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
const CTransaction* ptxConflicting = m_pool.GetConflictTx(txin.prevout);
if (ptxConflicting) {
if (!args.m_allow_replacement) {
// Transaction conflicts with a mempool tx, but we're not allowing replacements.
// Transaction conflicts with a mempool tx, but we're not allowing replacements in this context.
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "bip125-replacement-disallowed");
}
if (!ws.m_conflicts.count(ptxConflicting->GetHash()))
{
// Transactions that don't explicitly signal replaceability are
// *not* replaceable with the current logic, even if one of their
// unconfirmed ancestors signals replaceability. This diverges
// from BIP125's inherited signaling description (see CVE-2021-31876).
// Applications relying on first-seen mempool behavior should
// check all unconfirmed ancestors; otherwise an opt-in ancestor
// might be replaced, causing removal of this descendant.
//
// All TRUC transactions are considered replaceable.
//
// Replaceability signaling of the original transactions may be
// ignored due to node setting.
const bool allow_rbf{m_pool.m_opts.full_rbf || SignalsOptInRBF(*ptxConflicting) || ptxConflicting->version == TRUC_VERSION};
if (!allow_rbf) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict");
}
ws.m_conflicts.insert(ptxConflicting->GetHash());
}
ws.m_conflicts.insert(ptxConflicting->GetHash());
}
}

161
test/functional/feature_rbf.py
View file

@ -9,7 +9,6 @@ from decimal import Decimal
from test_framework.messages import (
MAX_BIP125_RBF_SEQUENCE,
COIN,
SEQUENCE_FINAL,
)
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
@ -26,18 +25,15 @@ class ReplaceByFeeTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 2
# both nodes disable full-rbf to test BIP125 signaling
self.extra_args = [
[
"-mempoolfullrbf=0",
"-limitancestorcount=50",
"-limitancestorsize=101",
"-limitdescendantcount=200",
"-limitdescendantsize=101",
],
# second node has default mempool parameters, besides mempoolfullrbf being disabled
# second node has default mempool parameters
[
"-mempoolfullrbf=0",
],
]
self.supports_cli = False
@ -69,18 +65,12 @@ class ReplaceByFeeTest(BitcoinTestFramework):
self.log.info("Running test too many replacements using default mempool params...")
self.test_too_many_replacements_with_default_mempool_params()
self.log.info("Running test opt-in...")
self.test_opt_in()
self.log.info("Running test RPC...")
self.test_rpc()
self.log.info("Running test prioritised transactions...")
self.test_prioritised_transactions()
self.log.info("Running test no inherited signaling...")
self.test_no_inherited_signaling()
self.log.info("Running test replacement relay fee...")
self.test_replacement_relay_fee()
@ -426,14 +416,12 @@ class ReplaceByFeeTest(BitcoinTestFramework):
for graph_num in range(num_tx_graphs):
root_utxos.append(wallet.get_utxo())
optin_parent_tx = wallet.send_self_transfer_multi(
parent_tx = wallet.send_self_transfer_multi(
from_node=normal_node,
sequence=MAX_BIP125_RBF_SEQUENCE,
utxos_to_spend=[root_utxos[graph_num]],
num_outputs=txs_per_graph,
)
assert_equal(True, normal_node.getmempoolentry(optin_parent_tx['txid'])['bip125-replaceable'])
new_utxos = optin_parent_tx['new_utxos']
new_utxos = parent_tx['new_utxos']
for utxo in new_utxos:
# Create spends for each output from the "root" of this graph.
@ -470,81 +458,6 @@ class ReplaceByFeeTest(BitcoinTestFramework):
self.generate(normal_node, 1)
self.wallet.rescan_utxos()
def test_opt_in(self):
"""Replacing should only work if orig tx opted in"""
tx0_outpoint = self.make_utxo(self.nodes[0], int(1.1 * COIN))
# Create a non-opting in transaction
tx1a_utxo = self.wallet.send_self_transfer(
from_node=self.nodes[0],
utxo_to_spend=tx0_outpoint,
sequence=SEQUENCE_FINAL,
fee=Decimal("0.1"),
)["new_utxo"]
# This transaction isn't shown as replaceable
assert_equal(self.nodes[0].getmempoolentry(tx1a_utxo["txid"])['bip125-replaceable'], False)
# Shouldn't be able to double-spend
tx1b_hex = self.wallet.create_self_transfer(
utxo_to_spend=tx0_outpoint,
sequence=0,
fee=Decimal("0.2"),
)["hex"]
# This will raise an exception
assert_raises_rpc_error(-26, "txn-mempool-conflict", self.nodes[0].sendrawtransaction, tx1b_hex, 0)
tx1_outpoint = self.make_utxo(self.nodes[0], int(1.1 * COIN))
# Create a different non-opting in transaction
tx2a_utxo = self.wallet.send_self_transfer(
from_node=self.nodes[0],
utxo_to_spend=tx1_outpoint,
sequence=0xfffffffe,
fee=Decimal("0.1"),
)["new_utxo"]
# Still shouldn't be able to double-spend
tx2b_hex = self.wallet.create_self_transfer(
utxo_to_spend=tx1_outpoint,
sequence=0,
fee=Decimal("0.2"),
)["hex"]
# This will raise an exception
assert_raises_rpc_error(-26, "txn-mempool-conflict", self.nodes[0].sendrawtransaction, tx2b_hex, 0)
# Now create a new transaction that spends from tx1a and tx2a
# opt-in on one of the inputs
# Transaction should be replaceable on either input
tx3a_txid = self.wallet.send_self_transfer_multi(
from_node=self.nodes[0],
utxos_to_spend=[tx1a_utxo, tx2a_utxo],
sequence=[SEQUENCE_FINAL, 0xfffffffd],
fee_per_output=int(0.1 * COIN),
)["txid"]
# This transaction is shown as replaceable
assert_equal(self.nodes[0].getmempoolentry(tx3a_txid)['bip125-replaceable'], True)
self.wallet.send_self_transfer(
from_node=self.nodes[0],
utxo_to_spend=tx1a_utxo,
sequence=0,
fee=Decimal("0.4"),
)
# If tx3b was accepted, tx3c won't look like a replacement,
# but make sure it is accepted anyway
self.wallet.send_self_transfer(
from_node=self.nodes[0],
utxo_to_spend=tx2a_utxo,
sequence=0,
fee=Decimal("0.4"),
)
def test_prioritised_transactions(self):
# Ensure that fee deltas used via prioritisetransaction are
# correctly used by replacement logic
@ -629,69 +542,6 @@ class ReplaceByFeeTest(BitcoinTestFramework):
assert_equal(json0["vin"][0]["sequence"], 4294967293)
assert_equal(json1["vin"][0]["sequence"], 4294967294)
def test_no_inherited_signaling(self):
confirmed_utxo = self.wallet.get_utxo()
# Create an explicitly opt-in parent transaction
optin_parent_tx = self.wallet.send_self_transfer(
from_node=self.nodes[0],
utxo_to_spend=confirmed_utxo,
sequence=MAX_BIP125_RBF_SEQUENCE,
fee_rate=Decimal('0.01'),
)
assert_equal(True, self.nodes[0].getmempoolentry(optin_parent_tx['txid'])['bip125-replaceable'])
replacement_parent_tx = self.wallet.create_self_transfer(
utxo_to_spend=confirmed_utxo,
sequence=MAX_BIP125_RBF_SEQUENCE,
fee_rate=Decimal('0.02'),
)
# Test if parent tx can be replaced.
res = self.nodes[0].testmempoolaccept(rawtxs=[replacement_parent_tx['hex']])[0]
# Parent can be replaced.
assert_equal(res['allowed'], True)
# Create an opt-out child tx spending the opt-in parent
parent_utxo = self.wallet.get_utxo(txid=optin_parent_tx['txid'])
optout_child_tx = self.wallet.send_self_transfer(
from_node=self.nodes[0],
utxo_to_spend=parent_utxo,
sequence=SEQUENCE_FINAL,
fee_rate=Decimal('0.01'),
)
# Reports true due to inheritance
assert_equal(True, self.nodes[0].getmempoolentry(optout_child_tx['txid'])['bip125-replaceable'])
replacement_child_tx = self.wallet.create_self_transfer(
utxo_to_spend=parent_utxo,
sequence=SEQUENCE_FINAL,
fee_rate=Decimal('0.02'),
)
# Broadcast replacement child tx
# BIP 125 :
# 1. The original transactions signal replaceability explicitly or through inheritance as described in the above
# Summary section.
# The original transaction (`optout_child_tx`) doesn't signal RBF but its parent (`optin_parent_tx`) does.
# The replacement transaction (`replacement_child_tx`) should be able to replace the original transaction.
# See CVE-2021-31876 for further explanations.
assert_equal(True, self.nodes[0].getmempoolentry(optin_parent_tx['txid'])['bip125-replaceable'])
assert_raises_rpc_error(-26, 'txn-mempool-conflict', self.nodes[0].sendrawtransaction, replacement_child_tx["hex"], 0)
self.log.info('Check that the child tx can still be replaced (via a tx that also replaces the parent)')
replacement_parent_tx = self.wallet.send_self_transfer(
from_node=self.nodes[0],
utxo_to_spend=confirmed_utxo,
sequence=SEQUENCE_FINAL,
fee_rate=Decimal('0.03'),
)
# Check that child is removed and update wallet utxo state
assert_raises_rpc_error(-5, 'Transaction not in mempool', self.nodes[0].getmempoolentry, optout_child_tx['txid'])
self.wallet.get_utxo(txid=optout_child_tx['txid'])
def test_replacement_relay_fee(self):
tx = self.wallet.send_self_transfer(from_node=self.nodes[0])['tx']
@ -702,12 +552,12 @@ class ReplaceByFeeTest(BitcoinTestFramework):
assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx.serialize().hex())
def test_fullrbf(self):
# BIP125 signaling is not respected
confirmed_utxo = self.make_utxo(self.nodes[0], int(2 * COIN))
self.restart_node(0, extra_args=["-mempoolfullrbf=1"])
assert self.nodes[0].getmempoolinfo()["fullrbf"]
# Create an explicitly opt-out transaction
# Create an explicitly opt-out BIP125 transaction, which will be ignored
optout_tx = self.wallet.send_self_transfer(
from_node=self.nodes[0],
utxo_to_spend=confirmed_utxo,
@ -718,7 +568,6 @@ class ReplaceByFeeTest(BitcoinTestFramework):
conflicting_tx = self.wallet.create_self_transfer(
utxo_to_spend=confirmed_utxo,
sequence=SEQUENCE_FINAL,
fee_rate=Decimal('0.02'),
)

13
test/functional/mempool_accept.py
View file

@ -55,7 +55,6 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
self.num_nodes = 1
self.extra_args = [[
'-txindex','-permitbaremultisig=0',
'-mempoolfullrbf=0',
]] * self.num_nodes
self.supports_cli = False
@ -155,25 +154,13 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
self.log.info('A transaction that replaces a mempool transaction')
tx = tx_from_hex(raw_tx_0)
tx.vout[0].nValue -= int(fee * COIN) # Double the fee
tx.vin[0].nSequence = MAX_BIP125_RBF_SEQUENCE + 1 # Now, opt out of RBF
raw_tx_0 = tx.serialize().hex()
txid_0 = tx.rehash()
self.check_mempool_result(
result_expected=[{'txid': txid_0, 'allowed': True, 'vsize': tx.get_vsize(), 'fees': {'base': (2 * fee)}}],
rawtxs=[raw_tx_0],
)
self.log.info('A transaction that conflicts with an unconfirmed tx')
# Send the transaction that replaces the mempool transaction and opts out of replaceability
node.sendrawtransaction(hexstring=tx.serialize().hex(), maxfeerate=0)
# take original raw_tx_0
tx = tx_from_hex(raw_tx_0)
tx.vout[0].nValue -= int(4 * fee * COIN) # Set more fee
self.check_mempool_result(
result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'txn-mempool-conflict'}],
rawtxs=[tx.serialize().hex()],
maxfeerate=0,
)
self.log.info('A transaction with missing inputs, that never existed')
tx = tx_from_hex(raw_tx_0)

29
test/functional/mempool_truc.py
View file

@ -4,9 +4,6 @@
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
from decimal import Decimal
from test_framework.messages import (
MAX_BIP125_RBF_SEQUENCE,
)
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
@ -115,7 +112,6 @@ class MempoolTRUC(BitcoinTestFramework):
from_node=node,
fee_rate=DEFAULT_FEE,
utxo_to_spend=utxo_v3_bip125,
sequence=MAX_BIP125_RBF_SEQUENCE,
version=3
)
self.check_mempool([tx_v3_bip125["txid"]])
@ -163,30 +159,6 @@ class MempoolTRUC(BitcoinTestFramework):
self.check_mempool([tx_v3_bip125_rbf_v2["txid"], tx_v3_parent["txid"], tx_v3_child["txid"]])
@cleanup(extra_args=["-mempoolfullrbf=0"])
def test_truc_bip125(self):
node = self.nodes[0]
self.log.info("Test TRUC transactions that don't signal BIP125 are replaceable")
assert_equal(node.getmempoolinfo()["fullrbf"], False)
utxo_v3_no_bip125 = self.wallet.get_utxo()
tx_v3_no_bip125 = self.wallet.send_self_transfer(
from_node=node,
fee_rate=DEFAULT_FEE,
utxo_to_spend=utxo_v3_no_bip125,
sequence=MAX_BIP125_RBF_SEQUENCE + 1,
version=3
)
self.check_mempool([tx_v3_no_bip125["txid"]])
assert not node.getmempoolentry(tx_v3_no_bip125["txid"])["bip125-replaceable"]
tx_v3_no_bip125_rbf = self.wallet.send_self_transfer(
from_node=node,
fee_rate=DEFAULT_FEE * 2,
utxo_to_spend=utxo_v3_no_bip125,
version=3
)
self.check_mempool([tx_v3_no_bip125_rbf["txid"]])
@cleanup(extra_args=["-datacarriersize=40000"])
def test_truc_reorg(self):
node = self.nodes[0]
@ -631,7 +603,6 @@ class MempoolTRUC(BitcoinTestFramework):
self.test_truc_max_vsize()
self.test_truc_acceptance()
self.test_truc_replacement()
self.test_truc_bip125()
self.test_truc_reorg()
self.test_nondefault_package_limits()
self.test_truc_ancestors_package()