highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-03-30 18:35:54 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#26646: validation, bugfix: provide more info in *MempoolAcceptResult

Browse Source 
264f9ef17f [validation] return MempoolAcceptResult for every tx on PCKG_TX failure (glozow)
dae81e01e8 [refactor] rename variables in AcceptPackage for clarity (glozow)
da484bc738 [doc] release note effective-feerate and effective-includes RPC results (glozow)
5eab397b98 [validation] remove PackageMempoolAcceptResult::m_package_feerate (glozow)
601bac88cb [rpc] return effective-includes in testmempoolaccept and submitpackage (glozow)
1691eaa818 [rpc] return effective-feerate in testmempoolaccept and submitpackage (glozow)
d6c7b78ef2 [validation] return wtxids of other transactions whose fees were used (glozow)
1605886380 [validation] return effective feerate from mempool validation (glozow)
5d35b4a7de [test] package validation quits early due to non-policy, non-missing-inputs failure (glozow)
be2e4d94e5 [validation] when quitting early in AcceptPackage, set package_state and tx result (glozow)

Pull request description:

  This PR fixes a bug and improves the mempool accept interface to return information more predictably.

  Bug: In package validation, we first try the transactions individually (see doc/policy/packages.md for more explanation) and, if they all failed for missing inputs and policy-related (i.e. fee) reasons, we'll try package validation. Otherwise, we'll just "quit early" since, for example, if a transaction had an invalid signature, adding a child will not help make it valid. Currently, when we quit early, we're not setting the `package_state` to be invalid, so the caller might think it succeeded. Also, we're returning no results - it makes more sense to return the individual transaction failure. Thanks instagibbs for catching https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1013293248!

  Also, make the package results interface generally more useful/predictable:
  - Always return the feerate at which a transaction was considered for `CheckFeeRate` in `MempoolAcceptResult::m_effective_feerate` when it was successful. This can replace the current `PackageMempoolAcceptResult::m_package_feerate`, which only sometimes exists.
  - Always provide an entry for every transaction in `PackageMempoolAcceptResult::m_tx_results` when the error is `PCKG_TX`.

ACKs for top commit:
  instagibbs:
    reACK 264f9ef17f
  achow101:
    ACK 264f9ef17f
  naumenkogs:
    reACK 264f9ef17f

Tree-SHA512: ce7fd9927a80030317cc6157822596e85a540feff5dbf5eea7c62da2eb50c917cdddc9da1e2ff62cc18b98b27d360151811546bd9d498859679a04bbee090837
This commit is contained in:
glozow
2023-01-11 13:04:13 +00:00
parent dbca00ef76 264f9ef17f
commit 26002570ab
8 changed files with 266 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
test/functional/mempool_accept.py
View File

@@ -58,7 +58,11 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
"""Wrapper to check result of testmempoolaccept on node_0's mempool"""
result_test = self.nodes[0].testmempoolaccept(*args, **kwargs)
for r in result_test:
r.pop('wtxid') # Skip check for now
# Skip these checks for now
r.pop('wtxid')
if "fees" in r:
r["fees"].pop("effective-feerate")
r["fees"].pop("effective-includes")
assert_equal(result_expected, result_test)
assert_equal(self.nodes[0].getmempoolinfo()['size'], self.mempool_size) # Must not change mempool state

12
test/functional/p2p_segwit.py
View File

@@ -622,8 +622,10 @@ class SegWitTest(BitcoinTestFramework):
if not self.segwit_active:
# Just check mempool acceptance, but don't add the transaction to the mempool, since witness is disallowed
# in blocks and the tx is impossible to mine right now.
assert_equal(
self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]),
testres3 = self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()])
testres3[0]["fees"].pop("effective-feerate")
testres3[0]["fees"].pop("effective-includes")
assert_equal(testres3,
[{
'txid': tx3.hash,
'wtxid': tx3.getwtxid(),
@@ -639,8 +641,10 @@ class SegWitTest(BitcoinTestFramework):
tx3 = tx
tx3.vout = [tx3_out]
tx3.rehash()
assert_equal(
self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()]),
testres3_replaced = self.nodes[0].testmempoolaccept([tx3.serialize_with_witness().hex()])
testres3_replaced[0]["fees"].pop("effective-feerate")
testres3_replaced[0]["fees"].pop("effective-includes")
assert_equal(testres3_replaced,
[{
'txid': tx3.hash,
'wtxid': tx3.getwtxid(),

60
test/functional/rpc_packages.py
View File

@@ -20,6 +20,7 @@ from test_framework.util import (
assert_raises_rpc_error,
)
from test_framework.wallet import (
COIN,
DEFAULT_FEE,
MiniWallet,
)
@@ -239,11 +240,14 @@ class RPCPackagesTest(BitcoinTestFramework):
coin = self.wallet.get_utxo()
fee = Decimal("0.00125000")
replaceable_tx = self.wallet.create_self_transfer(utxo_to_spend=coin, sequence=MAX_BIP125_RBF_SEQUENCE, fee = fee)
testres_replaceable = node.testmempoolaccept([replaceable_tx["hex"]])
assert_equal(testres_replaceable, [
{"txid": replaceable_tx["txid"], "wtxid": replaceable_tx["wtxid"],
"allowed": True, "vsize": replaceable_tx["tx"].get_vsize(), "fees": { "base": fee }}
])
testres_replaceable = node.testmempoolaccept([replaceable_tx["hex"]])[0]
assert_equal(testres_replaceable["txid"], replaceable_tx["txid"])
assert_equal(testres_replaceable["wtxid"], replaceable_tx["wtxid"])
assert testres_replaceable["allowed"]
assert_equal(testres_replaceable["vsize"], replaceable_tx["tx"].get_vsize())
assert_equal(testres_replaceable["fees"]["base"], fee)
assert_fee_amount(fee, replaceable_tx["tx"].get_vsize(), testres_replaceable["fees"]["effective-feerate"])
assert_equal(testres_replaceable["fees"]["effective-includes"], [replaceable_tx["wtxid"]])
# Replacement transaction is identical except has double the fee
replacement_tx = self.wallet.create_self_transfer(utxo_to_spend=coin, sequence=MAX_BIP125_RBF_SEQUENCE, fee = 2 * fee)
@@ -287,11 +291,13 @@ class RPCPackagesTest(BitcoinTestFramework):
peer = node.add_p2p_connection(P2PTxInvStore())
package_txns = []
presubmitted_wtxids = set()
for _ in range(num_parents):
parent_tx = self.wallet.create_self_transfer(fee=DEFAULT_FEE)
package_txns.append(parent_tx)
if partial_submit and random.choice([True, False]):
node.sendrawtransaction(parent_tx["hex"])
presubmitted_wtxids.add(parent_tx["wtxid"])
child_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=[tx["new_utxo"] for tx in package_txns], fee_per_output=10000) #DEFAULT_FEE
package_txns.append(child_tx)
@@ -302,24 +308,19 @@ class RPCPackagesTest(BitcoinTestFramework):
for package_txn in package_txns:
tx = package_txn["tx"]
assert tx.getwtxid() in submitpackage_result["tx-results"]
tx_result = submitpackage_result["tx-results"][tx.getwtxid()]
assert_equal(tx_result, {
"txid": package_txn["txid"],
"vsize": tx.get_vsize(),
"fees": {
"base": DEFAULT_FEE,
}
})
wtxid = tx.getwtxid()
assert wtxid in submitpackage_result["tx-results"]
tx_result = submitpackage_result["tx-results"][wtxid]
assert_equal(tx_result["txid"], tx.rehash())
assert_equal(tx_result["vsize"], tx.get_vsize())
assert_equal(tx_result["fees"]["base"], DEFAULT_FEE)
if wtxid not in presubmitted_wtxids:
assert_fee_amount(DEFAULT_FEE, tx.get_vsize(), tx_result["fees"]["effective-feerate"])
assert_equal(tx_result["fees"]["effective-includes"], [wtxid])
# submitpackage result should be consistent with testmempoolaccept and getmempoolentry
self.assert_equal_package_results(node, testmempoolaccept_result, submitpackage_result)
# Package feerate is calculated for the remaining transactions after deduplication and
# individual submission. If only 0 or 1 transaction is left, e.g. because all transactions
# had high-feerates or were already in the mempool, no package feerate is provided.
# In this case, since all of the parents have high fees, each is accepted individually.
assert "package-feerate" not in submitpackage_result
# The node should announce each transaction. No guarantees for propagation.
peer.wait_for_broadcast([tx["tx"].getwtxid() for tx in package_txns])
self.generate(node, 1)
@@ -328,8 +329,11 @@ class RPCPackagesTest(BitcoinTestFramework):
node = self.nodes[0]
peer = node.add_p2p_connection(P2PTxInvStore())
# Package with 2 parents and 1 child. One parent pays for itself using modified fees, and
# another has 0 fees but is bumped by child.
tx_poor = self.wallet.create_self_transfer(fee=0, fee_rate=0)
tx_rich = self.wallet.create_self_transfer(fee=DEFAULT_FEE)
tx_rich = self.wallet.create_self_transfer(fee=0, fee_rate=0)
node.prioritisetransaction(tx_rich["txid"], 0, int(DEFAULT_FEE * COIN))
package_txns = [tx_rich, tx_poor]
coins = [tx["new_utxo"] for tx in package_txns]
tx_child = self.wallet.create_self_transfer_multi(utxos_to_spend=coins, fee_per_output=10000) #DEFAULT_FEE
@@ -340,14 +344,18 @@ class RPCPackagesTest(BitcoinTestFramework):
rich_parent_result = submitpackage_result["tx-results"][tx_rich["wtxid"]]
poor_parent_result = submitpackage_result["tx-results"][tx_poor["wtxid"]]
child_result = submitpackage_result["tx-results"][tx_child["tx"].getwtxid()]
assert_equal(rich_parent_result["fees"]["base"], DEFAULT_FEE)
assert_equal(rich_parent_result["fees"]["base"], 0)
assert_equal(poor_parent_result["fees"]["base"], 0)
assert_equal(child_result["fees"]["base"], DEFAULT_FEE)
# Package feerate is calculated for the remaining transactions after deduplication and
# individual submission. Since this package had a 0-fee parent, package feerate must have
# been used and returned.
assert "package-feerate" in submitpackage_result
assert_fee_amount(DEFAULT_FEE, rich_parent_result["vsize"] + child_result["vsize"], submitpackage_result["package-feerate"])
# The "rich" parent does not require CPFP so its effective feerate.
assert_fee_amount(DEFAULT_FEE, tx_rich["tx"].get_vsize(), rich_parent_result["fees"]["effective-feerate"])
assert_equal(rich_parent_result["fees"]["effective-includes"], [tx_rich["wtxid"]])
# The "poor" parent and child's effective feerates are the same, composed of the child's fee
# divided by their combined vsize.
assert_fee_amount(DEFAULT_FEE, tx_poor["tx"].get_vsize() + tx_child["tx"].get_vsize(), poor_parent_result["fees"]["effective-feerate"])
assert_fee_amount(DEFAULT_FEE, tx_poor["tx"].get_vsize() + tx_child["tx"].get_vsize(), child_result["fees"]["effective-feerate"])
assert_equal([tx_poor["wtxid"], tx_child["tx"].getwtxid()], poor_parent_result["fees"]["effective-includes"])
assert_equal([tx_poor["wtxid"], tx_child["tx"].getwtxid()], child_result["fees"]["effective-includes"])
# The node will broadcast each transaction, still abiding by its peer's fee filter
peer.wait_for_broadcast([tx["tx"].getwtxid() for tx in package_txns])