Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?

I propose reworking the rebroadcast logic to

  1. Significantly improve privacy
  2. Create a cleaner divide between wallet & node responsibilities

Background: Currently, a node will only rebroadcast a transaction if it is the originating wallet. This is awful for privacy.

Conceptual changes

  • Instead of the wallet directly relaying transactions to peers, the wallet will submit unconfirmed txns to the node, and the node will apply logic to trigger txn rebroadcasts.
  • The node will not keep track of “my” transactions but instead, apply rebroadcast conditions to all transactions.
  • The wallet will attempt to resubmit unconfirmed transactions to the node on a scheduled timer. This is to ensure the txn is not dropped from the local mempool.

New rebroadcast conditions:

  • I define “top transactions” as the max fee rate packages in the mempool.
  • When it is time to rebroadcast, calculate top transactions, filter out transactions that are recent. Queue the remaining up for rebroadcasting (and add timing delay logic).

Params & initially proposed values

  • Frequency of resubmission attempt from wallet to node -> wallet resubmits once / day
  • Frequency of triggering rebroadcast -> once / hour
  • Defining highest priority transactions (top of mempool for potential txns to rebroadcast ) -> 3/4 block worth of txns based on package fee rate
  • Define what “recent” transaction means -> only rebroadcast if txn is >30 minutes old.
  • Add a max of [# of rebroadcasts per duration] as a safety net -> 1000 txns / hour

Implementation (high level)

UPDATE: This implementation plan is currently out of date and needs to be updated.

  1. Update how the wallet submits transactions to the node

    • Changes mentioned here build off of PR #15713.
    • Rework CWallet::ResendWalletTransaction to resubmit txns to the node instead of rebroadcasting them to peers.
    • Remove the relay bool being passed around (SubmitMemoryPool&Relay, broadcastTransaction, BroadcastTransaction). With ResendWalletTransaction simply submitting wallet txns to the node, the checks in BroadcastTransaction will determine if the node should relay the txns.
  2. Add rebroadcast logic to the mempool

    • Introduce new function CTxMemPool::RebroadcastTransactions to identify txn set of rebroadcast possibilities, apply filtering conditions as per logic above, and initiate relay to peers.
    • Kick off on automated timer.
    • Use ancestor fee rate traversal to identify rebroadcast possibilities via a version of BlockAssembler::addPackageTxs -> return iterator of txns instead of adding to block.
    • Relay to peers via net_processing#RelayTransaction. Likely will introduce another queue between identifying txns to rebroadcast and calling this method to introduce more time delays.

Other considerations & open questions

  • Avoid introducing extreme bandwidth spikes (aka spam the network). Currently proposed mechanisms to mitigate are: 1. poisson distribution of when rebroadcasts occur & 2. robust filtering logic. Some further possibilities are mentioned in this section.
  • There is an inherent tradeoff between defining param [top of mempool] vs param [age of transactions filtered out]. The values I have proposed opt to reduce #1 allow for leniency for #2. Having more recent transactions enables txns evicted from the mempool during volatility to be rebroadcasted and thus confirmed sooner.
  • We could do a simple fee rate traversal of the mempool instead of looking at transaction packages. One dependency for this to work is transaction package relay which is currently a WIP. However, mimic-ing the miner logic as closely as possible seems to reduce room for error.
  • Possibility: node stores rebroadcast data, such as last_rebroadcast_at time or list of rebroadcasted txns per peer. Tradeoff between bandwidth vs memory. While not storing this data means two peers could rebroadcast the same txn to one another within a short time span, I believe the bandwidth usage of sending unnecessary INV messages is a lower cost than the memory usage and code complexity of effectively storing rebroadcasted txn per peer.
  • Open question: what are privacy implications for when nodes have varied mempool expiry settings? For example, is it a privacy leak if you expire txns quicker, your wallet resubmits the txn, and you rebroadcast sooner than default 2-week expiration?
  • To consider: when policy rules are updated, nodes that are not upgraded can have their mempools cluttered with txns that will never be mined. While this is already the case (it just takes 1 node to rebroadcast), these proposed changes increase these chances.
  • Possibility: for more accurate filtering, cache the [txn fee rate for block inclusion] to capture the value right before disconnecting block txns from the mempool. I've gone through many iterations of this idea and have some viable implementation options to explore if we see signs of concern around bandwidth usage.

Thank you to jnewbery, ajtowns, MarcoFalke & sdaftuar for lots of incredibly valuable input.

@amitiuttarwar

This comment has been minimized.

Copy link
Owner Author

commented Jul 25, 2019

todo: update proposal with notes from meeting

  • add per-tx flag to mempool to keep track of ever_broadcast if tx pushed to at least one peer
    -> save flag to mempool.dat to avoid rebroadcasting own txns every time we restart
  • to consider: add cap of # rebroadcast / txn
  • to consider: per-txn "point system" to let txns earn weight before rebroadcasting
  • implementation: sort on actual fee rate, not modified fee rate -> avoid privacy leak for when prioritizetransaction is used
@amitiuttarwar

This comment has been minimized.

Copy link
Owner Author

commented Aug 1, 2019

update: going to sort on modified fee rate, not actual fee rate. this means there will be a privacy leak if prioritisetransaction is used. this seems reasonable because it would be a giveaway when a miner mines a txn with a lower fee rate regardless, and the function is not meaningful for normal users. simply going to update the help text to warn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.