Page MenuHomePhabricator

[mempool] Track "unbroadcast" transactions
ClosedPublic

Authored by PiRK on Jan 21 2021, 11:31.

Details

Reviewers
majcosta
Group Reviewers
Restricted Project
Commits
rABC674a222fdbab: [mempool] Track "unbroadcast" transactions
Summary
  • Mempool tracks locally submitted transactions (wallet or rpc)
  • Transactions are removed from set when the node receives a GETDATA request from a peer, or if the transaction is removed from the mempool.

PR description:

This PR introduces mempool tracking of unbroadcast transactions and periodic reattempts at initial broadcast. This is a part of the rebroadcast project, and a standalone privacy win.

The current rebroadcast logic is terrible for privacy because 1. only the source wallet rebroadcasts transactions and 2. it does so quite frequently. In the current system, if a user submits a transaction that does not immediately get broadcast to the network (eg. they are offline), this "rebroadcast" behavior is the safety net that can actually serve as the initial broadcast. So, keeping the attempts frequent is important for initial delivery within a reasonable timespan.

This PR aims to improve # 2 by reducing the wallet rebroadcast frequency to ~1/day from ~1/15 min. It achieves this by separating the notion of initial broadcast from rebroadcasts. With these changes, the mempool tracks locally submitted transactions & periodically reattempts initial broadcast. Transactions submitted via the wallet or RPC are added to an "unbroadcast" set & are removed when a peer sends a getdata request, or the transaction is removed from the mempool. Every 10-15 minutes, the node reattempts an initial broadcast. This enables reducing the wallet rebroadcast frequency while ensuring the transactions will be propagated to the network.

For privacy improvements around # 1, please see PR16698.

This is a backport of Core PR18038 [1/7]
https://github.com/bitcoin/bitcoin/pull/18038/commits/89eeb4a3335f8e871cc3f5286af4546dff66172a

Test Plan

ninja all check-all

Event Timeline

PiRK requested review of this revision.Jan 21 2021, 11:32
majcosta requested changes to this revision.Jan 21 2021, 13:09
majcosta added a subscriber: majcosta.
majcosta added inline comments.
src/txmempool.cpp
475 ↗(On Diff #27168)

if this constant were to be used in more than one place I'd agree with defining it here, but since it's only passed to RemoveUnbroadcastTx a few lines below it's fine to just use it->GetTx().GetId() directly

This revision now requires changes to proceed.Jan 21 2021, 13:09
This revision is now accepted and ready to land.Jan 21 2021, 13:49