phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-25 10:43:19 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge bitcoin/bitcoin#24310: docs / fixups from RBF and packages

Browse source 
77202f0554 [doc] package deduplication (glozow)
d35a3cb396 [doc] clarify inaccurate comment about replacements paying higher feerate (glozow)
5ae187f876 [validation] look up transaction by txid (glozow)

Pull request description:

  - Use txid, not wtxid, for `mempool.GetIter()`: https://github.com/bitcoin/bitcoin/pull/22674#discussion_r772934994
  - Fix a historically inaccurate comment about RBF during the refactors: https://github.com/bitcoin/bitcoin/pull/22855#discussion_r777130441
  - Add a section about package deduplication to policy/packages.md: https://github.com/bitcoin/bitcoin/pull/24152#discussion_r802955759 and https://github.com/bitcoin/bitcoin/pull/24152#discussion_r802723149

  (I'm intending for this to be in v23 since it's fixups for things that are already merged, which is why I split it from #24152)

ACKs for top commit:
  t-bast:
    LGTM, ACK 77202f0554
  darosior:
    ACK 77202f0554
  LarryRuane:
    ACK 77202f0554

Tree-SHA512: a428e791dfa59c359d3ccc67e8d3a4c1239815d2f6b29898e129700079271c00b3a45f091f70b65a6e54aa00a3d5b678b6da29d2a76b6cd6f946eaa7082ea696
This commit is contained in:
fanquake 2022-02-22 09:16:04 +00:00
commit bc49650b7c
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
3 changed files with 36 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
doc/policy/packages.md
View file

@ -57,3 +57,18 @@ test accepts):
- Warning: Batched fee-bumping may be unsafe for some use cases. Users and application developers
should take caution if utilizing multi-parent packages.
* Transactions in the package that have the same txid as another transaction already in the mempool
will be removed from the package prior to submission ("deduplication").
- *Rationale*: Node operators are free to set their mempool policies however they please, nodes
may receive transactions in different orders, and malicious counterparties may try to take
advantage of policy differences to pin or delay propagation of transactions. As such, it's
possible for some package transaction(s) to already be in the mempool, and there is no need to
repeat validation for those transactions or double-count them in fees.
- *Rationale*: We want to prevent potential censorship vectors. We should not reject entire
packages because we already have one of the transactions. Also, if an attacker first broadcasts
a competing package or transaction with a mutated witness, even though the two
same-txid-different-witness transactions are conflicting and cannot replace each other, the
honest package should still be considered for acceptance.

11
src/test/txpackage_tests.cpp
View file

@ -413,6 +413,17 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(ptx_child2->GetHash())));
BOOST_CHECK(!m_node.mempool->exists(GenTxid::Wtxid(ptx_child2->GetWitnessHash())));
// Deduplication should work when wtxid != txid. Submit package with the already-in-mempool
// transactions again, which should not fail.
const auto submit_segwit_dedup = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool,
{ptx_parent, ptx_child1}, /*test_accept=*/ false);
BOOST_CHECK_MESSAGE(submit_segwit_dedup.m_state.IsValid(),
"Package validation unexpectedly failed: " << submit_segwit_dedup.m_state.GetRejectReason());
auto it_parent_dup = submit_segwit_dedup.m_tx_results.find(ptx_parent->GetWitnessHash());
auto it_child_dup = submit_segwit_dedup.m_tx_results.find(ptx_child1->GetWitnessHash());
BOOST_CHECK(it_parent_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY);
BOOST_CHECK(it_child_dup->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY);
}
// Try submitting Package1{child2, grandchild} where child2 is same-txid-different-witness as

17
src/validation.cpp
View file

@ -915,12 +915,15 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws)
TxValidationState& state = ws.m_state;
CFeeRate newFeeRate(ws.m_modified_fees, ws.m_vsize);
// It's possible that the replacement pays more fees than its direct conflicts but not more
// than all conflicts (i.e. the direct conflicts have high-fee descendants). However, if the
// replacement doesn't pay more fees than its direct conflicts, then we can be sure it's not
// more economically rational to mine. Before we go digging through the mempool for all
// transactions that would need to be removed (direct conflicts and all descendants), check
// that the replacement transaction pays more than its direct conflicts.
// The replacement transaction must have a higher feerate than its direct conflicts.
// - The motivation for this check is to ensure that the replacement transaction is preferable for
// block-inclusion, compared to what would be removed from the mempool.
// - This logic predates ancestor feerate-based transaction selection, which is why it doesn't
// consider feerates of descendants.
// - Note: Ancestor feerate-based transaction selection has made this comparison insufficient to
// guarantee that this is incentive-compatible for miners, because it is possible for a
// descendant transaction of a direct conflict to pay a higher feerate than the transaction that
// might replace them, under these rules.
if (const auto err_string{PaysMoreThanConflicts(ws.m_iters_conflicting, newFeeRate, hash)}) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string);
}
@ -1318,7 +1321,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
// we know is that the inputs aren't available.
if (m_pool.exists(GenTxid::Wtxid(wtxid))) {
// Exact transaction already exists in the mempool.
auto iter = m_pool.GetIter(wtxid);
auto iter = m_pool.GetIter(txid);
assert(iter != std::nullopt);
results.emplace(wtxid, MempoolAcceptResult::MempoolTx(iter.value()->GetTxSize(), iter.value()->GetFee()));
} else if (m_pool.exists(GenTxid::Txid(txid))) {