Skip to content

Commit

Permalink
experiment: protect orphans from outbounds with 1 missing parent
Browse files Browse the repository at this point in the history
This is better than protecting ones with 1 reconsiderable rejected
parent, because it means the parent doesn't need to be received first
for protection to happen.
However, it can be problematic for orphans that spend confirmed UTXOs
since they are not considered AlreadyHaveTx unless they are in
RecentConfirmedTransactionsFilter.
  • Loading branch information
glozow committed Aug 30, 2024
1 parent b8f8041 commit 0215926
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
7 changes: 3 additions & 4 deletions src/node/txdownloadman_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,11 +463,10 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
m_orphan_resolution_tracker.ReceivedInv(nodeid, GenTxid::Wtxid(wtxid), info.m_preferred, now + *delay);

// If this could be a 1p1c package, attempt to protect this orphan from eviction
// (only peers preferred for download have the protection ability). We give
// orphans with exactly 1 parent in m_lazy_recent_rejects_reconsiderable special
// (only peers preferred for download have the protection ability). We give them
// treatment because orphan resolution is potentially the only way we will
// accept these transactions (the parent will not be accepted by itself).
if (rejected_parent_reconsiderable.has_value()) {
// accept these transactions (low feerate parent will not be accepted by itself).
if (unique_parents.size() == 1) {
MaybeProtectOrphan(nodeid, wtxid);
}

Expand Down
22 changes: 20 additions & 2 deletions test/functional/p2p_opportunistic_1p1c.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,16 @@ def test_basic_child_then_parent(self, adversarial_peers):
low_fee_parent = self.create_tx_below_mempoolminfee(self.wallet)
high_fee_child = self.wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=20*FEERATE_1SAT_VB)

peer_sender = node.add_p2p_connection(P2PInterface())
# When there are no adversarial_peers, test that 1p1c works with inbound peers, which are
# given fewer privileges than outbounds.
# When there are adversarial_peers, make peer_sender an outbound connection to test that the
# preferential download logic works properly. We are not testing that inbound connections
# work under adversarial conditions, because there are no guarantees in those situations.
if len(adversarial_peers) > 0:
peer_sender = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=5, connection_type="outbound-full-relay")
else:
peer_sender = node.add_p2p_connection(P2PInterface())

for peer in adversarial_peers:
node.add_p2p_connection(peer)

Expand Down Expand Up @@ -180,7 +189,10 @@ def test_basic_parent_then_child(self, wallet, adversarial_peers):
low_fee_parent = self.create_tx_below_mempoolminfee(wallet)
high_fee_child = wallet.create_self_transfer(utxo_to_spend=low_fee_parent["new_utxo"], fee_rate=20*FEERATE_1SAT_VB)

peer_sender = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=1, connection_type="outbound-full-relay")
if len(adversarial_peers) > 0:
peer_sender = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=1, connection_type="outbound-full-relay")
else:
peer_sender = node.add_p2p_connection(P2PInterface())
peer_ignored = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=2, connection_type="outbound-full-relay")
for peer in adversarial_peers:
node.add_p2p_connection(peer)
Expand Down Expand Up @@ -488,8 +500,14 @@ def run_test(self):
adversaries_many += [PeerSendsLargeOrphans(self.wallet_adversaries) for _ in range(5)]

self.log.info("Check 1p1c (parent sent before child) with large mix of adversaries")
# Protecting orphans with 1 rejected reconsiderable parent makes this work.
# But this is probably the rarest case of 1p1c.
self.test_basic_parent_then_child(self.wallet_nonsegwit, adversaries_many)

# Protecting orphans with 1 parent makes these work.
self.test_basic_child_then_parent(adversaries_many)
self.test_basic_parent_then_child(self.wallet, adversaries_many)


if __name__ == '__main__':
PackageRelayTest(__file__).main()

0 comments on commit 0215926

Please sign in to comment.