D4903.id15885.diff
No OneTemporary
Actions

Size

18 KB

Subscribers

None

D4903.id15885.diff
View Options

	diff --git a/src/miner.cpp b/src/miner.cpp
	--- a/src/miner.cpp
	+++ b/src/miner.cpp
	@@ -455,8 +455,17 @@
	}

	if (packageFees < blockMinFeeRate.GetFee(packageSize)) {
	- // Everything else we might consider has a lower fee rate
	- return;
	+ // Don't include this package, but don't stop yet because something
	+ // else we might consider may have a sufficient fee rate (since txes
	+ // are ordered by virtualsize feerate, not actual feerate).
	+ if (fUsingModified) {
	+ // Since we always look at the best entry in mapModifiedTx, we
	+ // must erase failed entries so that we can consider the next
	+ // best entry on the next loop iteration
	+ mapModifiedTx.get<ancestor_score>().erase(modit);
	+ failedTx.insert(iter);
	+ }
	+ continue;
	}

	// The following must not use virtual size since TestPackage relies on
	diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
	--- a/src/test/mempool_tests.cpp
	+++ b/src/test/mempool_tests.cpp
	@@ -324,6 +324,13 @@
	LOCK2(cs_main, pool.cs);
	TestMemPoolEntryHelper entry;

	+ /**
	+ * Remove the default nonzero sigops, since the below tests are focussing on
	+ * fee-based ordering and involve some artificially very tiny 21-byte
	+ * transactions without any inputs.
	+ */
	+ entry.SigOpCount(0);
	+
	/* 3rd highest fee */
	CMutableTransaction tx1 = CMutableTransaction();
	tx1.vout.resize(1);
	@@ -508,6 +515,13 @@
	LOCK2(cs_main, pool.cs);
	TestMemPoolEntryHelper entry;

	+ /**
	+ * Remove the default nonzero sigops, since the below tests are focussing on
	+ * fee-based ordering and involve some artificially very tiny 21-byte
	+ * transactions without any inputs.
	+ */
	+ entry.SigOpCount(0);
	+
	/* 3rd highest fee */
	CMutableTransaction tx1 = CMutableTransaction();
	tx1.vout.resize(1);
	diff --git a/src/txmempool.h b/src/txmempool.h
	--- a/src/txmempool.h
	+++ b/src/txmempool.h
	@@ -252,15 +252,17 @@
	double &size) const {
	// Compare feerate with descendants to feerate of the transaction, and
	// return the fee/size for the max.
	- double f1 = a.GetSizeWithDescendants() * (a.GetModifiedFee() / SATOSHI);
	- double f2 = a.GetTxSize() * (a.GetModFeesWithDescendants() / SATOSHI);
	+ double f1 =
	+ a.GetVirtualSizeWithDescendants() * (a.GetModifiedFee() / SATOSHI);
	+ double f2 =
	+ a.GetTxVirtualSize() * (a.GetModFeesWithDescendants() / SATOSHI);

	if (f2 > f1) {
	mod_fee = a.GetModFeesWithDescendants() / SATOSHI;
	- size = a.GetSizeWithDescendants();
	+ size = a.GetVirtualSizeWithDescendants();
	} else {
	mod_fee = a.GetModifiedFee() / SATOSHI;
	- size = a.GetTxSize();
	+ size = a.GetTxVirtualSize();
	}
	}
	};
	@@ -319,15 +321,17 @@
	void GetModFeeAndSize(const T &a, double &mod_fee, double &size) const {
	// Compare feerate with ancestors to feerate of the transaction, and
	// return the fee/size for the min.
	- double f1 = a.GetSizeWithAncestors() * (a.GetModifiedFee() / SATOSHI);
	- double f2 = a.GetTxSize() * (a.GetModFeesWithAncestors() / SATOSHI);
	+ double f1 =
	+ a.GetVirtualSizeWithAncestors() * (a.GetModifiedFee() / SATOSHI);
	+ double f2 =
	+ a.GetTxVirtualSize() * (a.GetModFeesWithAncestors() / SATOSHI);

	if (f1 > f2) {
	mod_fee = a.GetModFeesWithAncestors() / SATOSHI;
	- size = a.GetSizeWithAncestors();
	+ size = a.GetVirtualSizeWithAncestors();
	} else {
	mod_fee = a.GetModifiedFee() / SATOSHI;
	- size = a.GetTxSize();
	+ size = a.GetTxVirtualSize();
	}
	}
	};
	diff --git a/src/txmempool.cpp b/src/txmempool.cpp
	--- a/src/txmempool.cpp
	+++ b/src/txmempool.cpp
	@@ -1190,7 +1190,7 @@
	// mempool with feerate equal to txn which were removed with no block in
	// between.
	CFeeRate removed(it->GetModFeesWithDescendants(),
	- it->GetSizeWithDescendants());
	+ it->GetVirtualSizeWithDescendants());
	removed += MEMPOOL_FULL_FEE_INCREMENT;

	trackPackageRemoved(removed);
	diff --git a/src/validation.cpp b/src/validation.cpp
	--- a/src/validation.cpp
	+++ b/src/validation.cpp
	@@ -701,6 +701,7 @@
	CTxMemPoolEntry entry(ptx, nFees, nAcceptTime, chainActive.Height(),
	fSpendsCoinbase, nSigOpsCount, lp);
	unsigned int nSize = entry.GetTxSize();
	+ unsigned int nVirtualSize = entry.GetTxVirtualSize();

	// No transactions are allowed below minRelayTxFee except from
	// disconnected blocks.
	@@ -715,7 +716,7 @@
	pool.GetMinFee(
	gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) *
	1000000)
	- .GetFee(nSize);
	+ .GetFee(nVirtualSize);
	if (!bypass_limits && mempoolRejectFee > Amount::zero() &&
	nModifiedFees < mempoolRejectFee) {
	return state.DoS(
	diff --git a/test/functional/abc-sigops-mempool-mining.py b/test/functional/abc-sigops-mempool-mining.py
	new file mode 100644
	--- /dev/null
	+++ b/test/functional/abc-sigops-mempool-mining.py
	@@ -0,0 +1,320 @@
	+#!/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 mempool and mining priorities using high-sigops transactions, probing
	+how the virtualsize mechanism behaves.
	+
	+This test assumes:
	+ -bytespersigop default is 50
	+ -blockmintxfee default is 1000
	+ -minrelaytxfee default is 1000
	+ -MEMPOOL_FULL_FEE_INCREMENT is 1000
	+
	+based on mempool_limit.py
	+"""
	+
	+from test_framework.blocktools import (
	+ create_block,
	+ create_coinbase,
	+ make_conform_to_ctor,
	+)
	+from test_framework.messages import (
	+ CBlock,
	+ COutPoint,
	+ CTransaction,
	+ CTxIn,
	+ CTxOut,
	+ FromHex,
	+ ToHex,
	+)
	+from test_framework.mininode import (
	+ P2PDataStore,
	+)
	+from test_framework.script import (
	+ OP_CHECKMULTISIG,
	+ OP_RETURN,
	+ OP_TRUE,
	+)
	+from test_framework.test_framework import BitcoinTestFramework
	+from test_framework.util import (
	+ assert_equal,
	+ assert_raises_rpc_error,
	+ JSONRPCException
	+)
	+
	+from decimal import Decimal
	+from collections import deque
	+
	+SATOSHI = Decimal('0.00000001')
	+
	+
	+def create_var_transaction(spendfrom, custom_script, size_bytes, fee_sats):
	+ # Fund and sign a transaction to a given output, padding it to exactly
	+ # size = size_bytes and providing the given fee.
	+ # spendfrom should be a CTransaction with first output to OP_TRUE.
	+
	+ customout = CTxOut(0, custom_script)
	+ # set output amount to required dust
	+ customout.nValue = (len(customout.serialize()) + 148) * 3
	+
	+ ctx = CTransaction()
	+ ctx.vin.append(CTxIn(COutPoint(spendfrom.sha256, 0), b''))
	+ ctx.vout.append(
	+ CTxOut(spendfrom.vout[0].nValue - customout.nValue - fee_sats, bytes([OP_TRUE])))
	+ ctx.vout.append(customout)
	+ # two padding outputs
	+ ctx.vout.append(CTxOut(0, b''))
	+ ctx.vout.append(CTxOut(0, b''))
	+
	+ size_without_padding = len(ctx.serialize())
	+ padding_needed = size_bytes - size_without_padding
	+ assert padding_needed >= 0, (size_bytes, size_without_padding)
	+
	+ # Now insert padding. We need to be careful due to the flexible
	+ # var_int size.
	+ if padding_needed > 0xFFFF + 4:
	+ # We can fit all padding in one script.
	+ # 4 extra bytes will be used for var_int
	+ ctx.vout[2].scriptPubKey = bytes(
	+ [OP_RETURN]) * (padding_needed - 4)
	+ padding_needed = 0
	+ elif padding_needed >= 0xFD + 2:
	+ # We can pad most (probably all) using a script that is between
	+ # 253 and 65535 in length. Probably fit everything in one script.
	+ # 2 extra bytes will be used for var_int
	+ scriptsize = min(padding_needed - 2, 0xFFFF)
	+ ctx.vout[2].scriptPubKey = bytes([OP_RETURN]) * scriptsize
	+ padding_needed -= scriptsize + 2
	+ elif padding_needed >= 0xFD:
	+ # 0xFD or 0xFD+1. We can't pad with just one script, so just
	+ # take off some.
	+ ctx.vout[2].scriptPubKey = bytes([OP_RETURN]) * 50
	+ padding_needed -= 50
	+
	+ # extra padding on another output is needed if original padding_needed
	+ # was <= 0xfd+1, or was 0xffff+3 or 0xffff+4 .
	+ assert padding_needed < 0xfd
	+ ctx.vout[3].scriptPubKey = bytes([OP_RETURN]) * padding_needed
	+
	+ ctx.calc_sha256()
	+ assert_equal(len(ctx.serialize()), size_bytes)
	+ return ctx
	+
	+
	+class MempoolLimitSigopsTest(BitcoinTestFramework):
	+
	+ def set_test_params(self):
	+ self.setup_clean_chain = True
	+ self.num_nodes = 1
	+ self.extra_args = [["-maxmempool=5"]]
	+ self.block_heights = {}
	+
	+ def getbestblock(self, node):
	+ """Get the best block. Register its height so we can use build_block."""
	+ block_height = node.getblockcount()
	+ blockhash = node.getblockhash(block_height)
	+ block = FromHex(CBlock(), node.getblock(blockhash, 0))
	+ block.calc_sha256()
	+ self.block_heights[block.sha256] = block_height
	+ return block
	+
	+ def build_block(self, parent, transactions=(), nTime=None):
	+ """Make a new block with an OP_1 coinbase output.
	+
	+ Requires parent to have its height registered."""
	+ parent.calc_sha256()
	+ block_height = self.block_heights[parent.sha256] + 1
	+ block_time = (parent.nTime + 1) if nTime is None else nTime
	+
	+ block = create_block(
	+ parent.sha256, create_coinbase(block_height), block_time)
	+ block.nVersion = 4
	+ block.vtx.extend(transactions)
	+ make_conform_to_ctor(block)
	+ block.hashMerkleRoot = block.calc_merkle_root()
	+ block.solve()
	+ self.block_heights[block.sha256] = block_height
	+ return block
	+
	+ def run_test(self):
	+ (node,) = self.nodes
	+ node.add_p2p_connection(P2PDataStore())
	+
	+ tip = self.getbestblock(node)
	+
	+ self.log.info("Create some blocks with OP_1 coinbase for spending.")
	+ blocks = []
	+ for _ in range(600):
	+ tip = self.build_block(tip)
	+ blocks.append(tip)
	+ node.p2p.send_blocks_and_test(blocks, node, success=True)
	+ self.spendable_outputs = deque(block.vtx[0] for block in blocks)
	+
	+ self.log.info(
	+ "Mature the blocks and get out of IBD; also make some coinbases for the node wallet.")
	+ node.generate(120)
	+
	+ tip = self.getbestblock(node)
	+
	+ self.log.info(
	+ 'PART 1: testing of priority under congestion (mempool eviction priority and mining priority).')
	+
	+ self.log.info(
	+ "Put a regular transaction in mempool at 1 sat/byte.")
	+ txid1 = node.sendtoaddress(node.getnewaddress(), 0.001)
	+
	+ self.log.info(
	+ "Mempool accepts 1 sat/byte txes even with very many sigops")
	+ script_4000_sigops = bytes([OP_CHECKMULTISIG] * 200)
	+ # vsize = 200000, so 0.0015 sat/vbyte.
	+ ctx = create_var_transaction(
	+ self.spendable_outputs.popleft(), script_4000_sigops, 300, 300)
	+ txid_parent = node.sendrawtransaction(ToHex(ctx))
	+
	+ self.log.info(
	+ "Create a 1 sat/byte compensating child of a high-sigop parent")
	+ # package vsize = 200000, so 0.5015 sat/vbyte.
	+ ctx_child = create_var_transaction(
	+ ctx, b'', 100000, 100000)
	+ txid_child = node.sendrawtransaction(ToHex(ctx_child))
	+
	+ # make one with 270 sat/byte, but only 0.4 sat/vbyte.
	+ ctx = create_var_transaction(
	+ self.spendable_outputs.popleft(), script_4000_sigops, 300, 80000)
	+ txid_highsigops = node.sendrawtransaction(ToHex(ctx))
	+
	+ self.log.info(
	+ "Flood the mempool with 10-kB transactions @ 9.9 sat/byte and 0.495 sat/vbyte, until it is full")
	+ # About 500 should fill up our 5 MB mempool.
	+ # Note that even starting from an empty pool, somewhat fewer than 500
	+ # would be accepted depending on mempool data structure sizes.
	+ for i in range(500):
	+ spendfrom = self.spendable_outputs.popleft()
	+ ctx = create_var_transaction(
	+ spendfrom, script_4000_sigops, 10000, 99000)
	+ try:
	+ node.sendrawtransaction(ToHex(ctx))
	+ except JSONRPCException as e:
	+ if 'mempool full' in e.error['message'] or 'mempool min fee not met' in e.error['message']:
	+ self.log.info(
	+ "Mempool filled after {} transactions".format(i,))
	+ break
	+ raise
	+ else:
	+ raise RuntimeError("didn't fill mempool")
	+
	+ self.log.info(
	+ 'The flooding only caused eviction of lower low sat/vbyte packages')
	+
	+ mempool_txids = set(node.getrawmempool())
	+
	+ # The 9.9 sat/byte txes caused the 270 sat/byte tx to get evicted, because
	+ # the latter was more dense in sigops.
	+ assert txid_highsigops not in mempool_txids
	+
	+ # Other txes did get kept, including txid_parent which had ultralow
	+ # virtual feerate (but its child supplied enough fee).
	+ settxids = set([txid1, txid_parent, txid_child])
	+ assert_equal(settxids.difference(mempool_txids), set())
	+
	+ self.log.info('Mempool fee floor has jumped to 1.495 sat/vbyte')
	+ # The removed txes have feerate 495 sat/kvbyte and the fee floor jumps by
	+ # 1000 sat/kbyte (MEMPOOL_FULL_FEE_INCREMENT) on top of that:
	+ assert_equal(node.getmempoolinfo()[
	+ 'mempoolminfee'], Decimal('0.00001495'))
	+
	+ self.log.info(
	+ 'Broadcasting a regular tx still works, because wallet knows what fee to use.')
	+ txid2 = node.sendtoaddress(node.getnewaddress(), 0.001)
	+ settxids.add(txid2)
	+
	+ self.log.info(
	+ "But, a regular 1 sat/byte transaction can't get in now.")
	+ spendfrom = self.spendable_outputs.popleft()
	+ assert_raises_rpc_error(-26, "mempool min fee not met", node.sendrawtransaction,
	+ ToHex(create_var_transaction(spendfrom, b'', 500, 500)))
	+
	+ self.log.info(
	+ "Broadcasting regular transactions will push out the high-sigops txns.")
	+ # We can broadcast a bunch of regular txes. They need to pay a bit more fee (1.5 sat/vbyte) than the floor.
	+ for i in range(15):
	+ spendfrom = self.spendable_outputs.popleft()
	+ ctx = create_var_transaction(spendfrom, b'', 100000, 150000)
	+ settxids.add(node.sendrawtransaction(ToHex(ctx)))
	+
	+ self.log.info("Mining picks all the 'good' txes first")
	+ # These slightly higher-fee transactions also have more priority.
	+ [lastblockhash, ] = node.generate(1)
	+ blocktxes = set(node.getblock(lastblockhash, 1)['tx'])
	+ assert_equal(settxids.difference(blocktxes), set())
	+ # there are still hundreds of the flooded txes left in mempool.
	+ assert len(node.getrawmempool()) > 200
	+
	+ self.log.info(
	+ 'PART 2: The following tests focus on mineability according to blockmintxfee.')
	+
	+ self.log.info('Clear the mempool by mining out everything.')
	+ node.generate(100)
	+ assert_equal(node.getrawmempool(), [])
	+
	+ self.log.info(
	+ 'Reset the mempool fee floor (currently, restarting the node achieves this).')
	+ # could also be done by setting mocktime in future (the floor decays over a period of hours)
	+ self.restart_node(0, self.extra_args[0])
	+ (node,) = self.nodes
	+ node.add_p2p_connection(P2PDataStore())
	+ assert_equal(node.getmempoolinfo()[
	+ 'mempoolminfee'], Decimal('0.00001000'))
	+
	+ self.log.info(
	+ 'Get a 0.998 sat/byte transaction into mempool by having it mined then reorged out.')
	+ self.log.info(node.getbestblockhash())
	+ tip = self.getbestblock(node)
	+ self.log.info(tip.hash)
	+ ctx = create_var_transaction(
	+ self.spendable_outputs.popleft(), b'', 1000, 998)
	+ stucktxid = ctx.hash
	+ # check that indeed it can't get in normally:
	+ assert_raises_rpc_error(-26, "min relay fee not met",
	+ node.sendrawtransaction, ToHex(ctx))
	+ block = self.build_block(tip, [ctx])
	+ node.p2p.send_blocks_and_test([block], node, success=True)
	+ node.invalidateblock(block.hash)
	+ assert stucktxid in node.getrawmempool(), "should be returned to pool"
	+
	+ self.log.info(
	+ "Since blockmintxfee is 1 sat/byte, that 0.998 sat/byte tx won't get mined by default, and is stuck permanently.")
	+ node.generate(1)
	+ assert stucktxid in node.getrawmempool(), "should be stuck"
	+
	+ self.log.info(
	+ 'Get wallet to make an normal tx sending money to 12 people, which is just under 1 sat/vbyte.')
	+ txid = node.sendmany(
	+ "", {node.getnewaddress(): 0.01 for _ in range(12)})
	+
	+ # analyze the constructed tx just to make sure it's as expected
	+ ctx = FromHex(CTransaction(), node.getrawtransaction(txid))
	+ assert len(ctx.vin) == 1, "only a single input should be needed"
	+ assert len(ctx.vout) == 13, "12 recipients and 1 change"
	+ # tx size should be ~ 599 bytes and fees should be 599 sats.
	+ # 13 sigops and 50 sigops / byte = 650 vbytes.
	+ txsize = len(ctx.serialize())
	+ txvsize = max(txsize, 50 * len(ctx.vout))
	+ assert txvsize > txsize, "transactions is dense-sigops"
	+ fee_sats = node.getmempoolentry(txid)['fee'] / SATOSHI
	+ assert_equal(fee_sats, 599)
	+ assert fee_sats / txsize >= 1.0, "just over 1 sat/byte"
	+ assert fee_sats / txvsize < 0.95, "just under 1 sat/vbyte"
	+
	+ self.log.info(
	+ "Even though another transaction is stuck permanently, it should not prevent a normal tx from being mined.")
	+ [bhash, ] = node.generate(1)
	+ blocktxes = set(node.getblock(bhash, 1)['tx'])
	+ assert txid in blocktxes, "normal tx is not stuck"
	+ assert stucktxid not in blocktxes, "lowfee tx is still stuck"
	+
	+
	+if __name__ == '__main__':
	+ MempoolLimitSigopsTest().main()

File Metadata

Mime Type: text/plain
Expires: Sat, Mar 1, 11:41 (3 h, 20 m)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 5187665
Default Alt Text: D4903.id15885.diff (18 KB)

D4903.id15885.diffNo OneTemporaryActions

D4903.id15885.diffView Options

File Metadata

Event Timeline

D4903.id15885.diff
No OneTemporary
Actions

D4903.id15885.diff
View Options