Remove wallet `relayTransaction` #15734

Originally posted as a comment here: #15713 (comment)

We should better define the relationship between the wallet and the node, ie answer the question "What services should the node offer to the wallet?" Currently, the node exposes two methods to the wallet for sending a transaction:

1. submitToMemoryPool(const CTransactionRef& tx, CAmount absurd_fee, CValidationState& state): Add transaction to memory pool if the transaction fee is below the amount specified by absurd_fee. Returns false if the transaction could not be added due to the fee or for another reason. This is only called in CWalletTx::AcceptToMemoryPool()
2. relayTransaction(const uint256& txid): adds the txid to the INV queue for each peer. This is only called in CWalletTx::RelayWalletTransaction() (which also calls CWalletTx::AcceptToMemoryPool())

Note that to relay an INV for the transaction to peers (2), the transaction must already be in the mempool. See:

https://github.com/bitcoin/bitcoin/blob/ba54342c9dd3f2e5cdeed9ac57f1924f0d885cc6/src/net_processing.cpp#L3804

relayTransaction() is only ever called if InMempool() || AcceptToMemoryPool() is true.

The wallet currently calls CWalletTx::AcceptToMemoryPool() in the following places:

1a. In CWallet::ReacceptWalletTransactions(), called after the wallet has started up to make sure that its transactions are added to the mempool at start. 2a. In CWalletTx::RelayWalletTransaction(), mentioned above. 3a. In CWallet::CommitTransaction(), when a transaction is to be broadcast to the network for the first time.

The wallet currently calls CWalletTx::RelayWalletTransaction() in the following places:

2a. In CWallet::ResendWalletTransactions(), called on a timer to rebroadcast unconfirmed transactions. 2b. In CWallet::CommitTransaction(), but only if the transaction is successfully added to the mempool.

I've been thinking about this in the context of the wallet rebroadcasting transactions (see discussion here: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2019/bitcoin-core-dev.2019-03-29-19.10.log.html#l-9). My view now is that the wallet should not have any access to relay functionality. It should be able to submit transactions to the node's mempool, and it should then be the mempool's responsibility to broadcast/rebroadcast the transaction (see comment here: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2019/bitcoin-core-dev.2019-03-29-19.10.log.html#l-92). That seems like a much cleaner divide of responsibility.

it should then be the mempool's responsibility to broadcast/rebroadcast the transaction

Concept ACK

The mempool should probably only re-send transactions in two cases:

• Your wallet-txs every two weeks to keep them in other node's mempools
• Any (also third-party) tx that has high enough fee to make it into the previous block and has been in the mempool for at least 5 minutes. I.e. any tx that could replace (due to higher fee) another tx from the previous block that is in getblocktemplate of the same size as the previous block. This re-send could happen with a higher poisson timeout to avoid network-wide spikes in bandwidth events.

The initial relay of wallet transactions should happen in a relay mechanism completely separate from the current mempool/inv logic. Txs are added to the mempool when they "come back" from other peers. Only if they never come back, the classical ATMP+fluff should be used.

I've written a proposal for better rebroadcast logic & a cleaner node / wallet interface: gist

I've written a proposal for better rebroadcast logic & a cleaner node / wallet interface: gist

The dev wiki is a good place for design docs, so could consider moving the gist there at some point. Does step 1 "Update how the wallet submits transactions to the node" in the implementation section overlap with #15713?

👍 happy to post to dev wiki once the gist is in a steady state.

the current proposed implementation conflicts with #15713, I will update to build off of that PR instead.

#16698

ref #18038 which lays the groundwork for #16698

This is tracked by the rebroadcast project at #16698