Merge #15681: [mempool] Allow one extra single-ancestor transaction per package

50cede3f5a4d4fbfbb7c420b94e661a6a159bced [mempool] Allow one extra single-ancestor transaction per package (Matt Corallo)

Pull request description:

  This implements the proposed policy change from [1], which allows
  certain classes of contract protocols involving revocation
  punishments to use CPFP. Note that some such use-cases may still
  want some form of one-deep package relay, though even this alone
  may greatly simplify some lightning fee negotiation.

  [1] https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html

ACKs for top commit:
  ajtowns:
    ACK 50cede3f5a4d4fbfbb7c420b94e661a6a159bced -- looked over code again, compared with previous commit, compiles, etc.
  sdaftuar:
    ACK 50cede3f5a4d4fbfbb7c420b94e661a6a159bced
  ryanofsky:
    utACK 50cede3f5a4d4fbfbb7c420b94e661a6a159bced. Changes since last review: adding EXTRA_DESCENDANT_TX_SIZE_LIMIT constant, changing max ancestor size from 1,000,000 to nLimitAncestorSize constant (101,000), fixing test comment and getting rid of unused test node.

Tree-SHA512: b052c2a0f384855572b4579310131897b612201214b5abbb225167224e4f550049e300b471dbf320928652571e92ca2d650050b7cf39ac92b3bc1d2bcd386c1c
This commit is contained in:
Wladimir J. van der Laan 2019-07-19 19:56:12 +02:00 committed by munkybooty
parent 35f8ed4d80
commit 507c871ed5
4 changed files with 108 additions and 1 deletions

View File

@ -708,8 +708,22 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
size_t nLimitDescendantSize = gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000; size_t nLimitDescendantSize = gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000;
std::string errString; std::string errString;
if (!pool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) { if (!pool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) {
setAncestors.clear();
// If the new transaction is relatively small (up to 40k weight)
// and has at most one ancestor (ie ancestor limit of 2, including
// the new transaction), allow it if its parent has exactly the
// descendant limit descendants.
//
// This allows protocols which rely on distrusting counterparties
// being able to broadcast descendants of an unconfirmed transaction
// to be secure by simply only having two immediately-spendable
// outputs - one for each counterparty. For more info on the uses for
// this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT ||
!pool.CalculateMemPoolAncestors(entry, setAncestors, 2, nLimitAncestorSize, nLimitDescendants + 1, nLimitDescendantSize + EXTRA_DESCENDANT_TX_SIZE_LIMIT, errString)) {
return state.DoS(0, false, REJECT_NONSTANDARD, "too-long-mempool-chain", false, errString); return state.DoS(0, false, REJECT_NONSTANDARD, "too-long-mempool-chain", false, errString);
} }
}
// check special TXs after all the other checks. If we'd do this before the other checks, we might end up // check special TXs after all the other checks. If we'd do this before the other checks, we might end up
// DoS scoring a node for non-critical errors, e.g. duplicate keys because a TX is received that was already // DoS scoring a node for non-critical errors, e.g. duplicate keys because a TX is received that was already

View File

@ -67,6 +67,12 @@ static const unsigned int DEFAULT_ANCESTOR_SIZE_LIMIT = 101;
static const unsigned int DEFAULT_DESCENDANT_LIMIT = 25; static const unsigned int DEFAULT_DESCENDANT_LIMIT = 25;
/** Default for -limitdescendantsize, maximum kilobytes of in-mempool descendants */ /** Default for -limitdescendantsize, maximum kilobytes of in-mempool descendants */
static const unsigned int DEFAULT_DESCENDANT_SIZE_LIMIT = 101; static const unsigned int DEFAULT_DESCENDANT_SIZE_LIMIT = 101;
/**
* An extra transaction can be added to a package, as long as it only has one
* ancestor and is no larger than this. Not really any reason to make this
* configurable as it doesn't materially change DoS parameters.
*/
static const unsigned int EXTRA_DESCENDANT_TX_SIZE_LIMIT = 10000;
/** Default for -mempoolexpiry, expiration time for mempool transactions in hours */ /** Default for -mempoolexpiry, expiration time for mempool transactions in hours */
static const unsigned int DEFAULT_MEMPOOL_EXPIRY = 336; static const unsigned int DEFAULT_MEMPOOL_EXPIRY = 336;
/** The maximum size of a blk?????.dat file (since 0.8) */ /** The maximum size of a blk?????.dat file (since 0.8) */

View File

@ -0,0 +1,86 @@
#!/usr/bin/env python3
# Copyright (c) 2014-2019 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test descendant package tracking carve-out allowing one final transaction in
an otherwise-full package as long as it has only one parent and is <= 10k in
size.
"""
from decimal import Decimal
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal, assert_raises_rpc_error, satoshi_round
MAX_ANCESTORS = 25
MAX_DESCENDANTS = 25
class MempoolPackagesTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
self.extra_args = [["-maxorphantxsize=100000"]]
def skip_test_if_missing_module(self):
self.skip_if_no_wallet()
# Build a transaction that spends parent_txid:vout
# Return amount sent
def chain_transaction(self, node, parent_txids, vouts, value, fee, num_outputs):
send_value = satoshi_round((value - fee)/num_outputs)
inputs = []
for (txid, vout) in zip(parent_txids, vouts):
inputs.append({'txid' : txid, 'vout' : vout})
outputs = {}
for i in range(num_outputs):
outputs[node.getnewaddress()] = send_value
rawtx = node.createrawtransaction(inputs, outputs)
signedtx = node.signrawtransactionwithwallet(rawtx)
txid = node.sendrawtransaction(signedtx['hex'])
fulltx = node.getrawtransaction(txid, 1)
assert len(fulltx['vout']) == num_outputs # make sure we didn't generate a change output
return (txid, send_value)
def run_test(self):
# Mine some blocks and have them mature.
self.nodes[0].generate(101)
utxo = self.nodes[0].listunspent(10)
txid = utxo[0]['txid']
vout = utxo[0]['vout']
value = utxo[0]['amount']
fee = Decimal("0.0002")
# MAX_ANCESTORS transactions off a confirmed tx should be fine
chain = []
for _ in range(4):
(txid, sent_value) = self.chain_transaction(self.nodes[0], [txid], [vout], value, fee, 2)
vout = 0
value = sent_value
chain.append([txid, value])
for _ in range(MAX_ANCESTORS - 4):
(txid, sent_value) = self.chain_transaction(self.nodes[0], [txid], [0], value, fee, 1)
value = sent_value
chain.append([txid, value])
(second_chain, second_chain_value) = self.chain_transaction(self.nodes[0], [utxo[1]['txid']], [utxo[1]['vout']], utxo[1]['amount'], fee, 1)
# Check mempool has MAX_ANCESTORS + 1 transactions in it
assert_equal(len(self.nodes[0].getrawmempool(True)), MAX_ANCESTORS + 1)
# Adding one more transaction on to the chain should fail.
assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], [txid], [0], value, fee, 1)
# ...even if it chains on from some point in the middle of the chain.
assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], [chain[2][0]], [1], chain[2][1], fee, 1)
assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], [chain[1][0]], [1], chain[1][1], fee, 1)
# ...even if it chains on to two parent transactions with one in the chain.
assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], [chain[0][0], second_chain], [1, 0], chain[0][1] + second_chain_value, fee, 1)
# ...especially if its > 40k weight
assert_raises_rpc_error(-26, "too-long-mempool-chain", self.chain_transaction, self.nodes[0], [chain[0][0]], [1], chain[0][1], fee, 350)
# But not if it chains directly off the first transaction
self.chain_transaction(self.nodes[0], [chain[0][0]], [1], chain[0][1], fee, 1)
# and the second chain should work just fine
self.chain_transaction(self.nodes[0], [second_chain], [0], second_chain_value, fee, 1)
# Finally, check that we added two transactions
assert_equal(len(self.nodes[0].getrawmempool(True)), MAX_ANCESTORS + 3)
if __name__ == '__main__':
MempoolPackagesTest().main()

View File

@ -166,6 +166,7 @@ BASE_SCRIPTS = [
'rpc_invalidateblock.py', 'rpc_invalidateblock.py',
'feature_txindex.py', 'feature_txindex.py',
'mempool_packages.py', 'mempool_packages.py',
'mempool_package_onemore.py',
'feature_versionbits_warning.py', 'feature_versionbits_warning.py',
'rpc_preciousblock.py', 'rpc_preciousblock.py',
'wallet_importprunedfunds.py', 'wallet_importprunedfunds.py',