mapRelay is a map containing all transactions that we’ve relayed to any peer recently.
It is accompanied by g_relay_expiration, a sorted list of expiration times for mapRelay entries.
Entries stay in mapRelay and g_relay_expirationfor 15 minutes.
When a peer asks for a transaction by sending a getdata message but the
transaction is no longer in the mempool, it can be served from mapRelay.
mapRelay has been around for a long time, it was already present in the
first github commit.
While it was essential back then, its scope has been reduced over time:
For example, Bitcoin Core now first tries to fetch transactions directly from the mempool.
There are other reasons why mapRelay wasn’t removed earlier
(see this comment
for an overview), but most of these have been made obsolete by other improvements.
This PR removes mapRelay and instead introduces
m_most_recent_block_txs to keep track of only the transactions from the most recent block.
Why is the memory usage of mapRelay hard to determine? (Hint: see
this comment)
What problem is solved by introducing m_most_recent_block_txs?
Do you think it is necessary to introduce it (as opposed to just removing mapRelay
without any replacement)?
What are the memory requirements for m_most_recent_block_txs compared to mapRelay?
Are there scenarios in which transactions would be made available for a shorter or longer time
than before as a result of this change?
Can you think of any other possible downsides of removing mapRelay?
<abubakarsadiq> because their is no utility for the transactions in mapRelay, all transaction dropped from mempool except for BLOCK reason does not need to be relayed anymore?
<lightlike> emzy, stickies-v: yes, exactly. I'd say it just gives an uneasy feeling having an unbounded structure, even if the actual memory consumption may not be that high.
<lightlike> abubakarsadiq: yes, that too. I think it's kind of a relic from the past when it was the main mechanism for tx transaction and we woldn't look up txns from the mempool.
<stickies-v> mapRelay and mempool share the same `std::shared_ptr<const CTransaction>` pointers, so the memory overhead of having an extra transaction in mapRelay depends on whether or not the transaction is still in mempool
<lightlike> stickies-v: exactly! Due to the use of a shared_ptr, if we have the tx stored in the mempool, the overhead of adding it also to mapRelay is small. If mapRelay is the only place the tx is stored, the additional size is much larger.
<lightlike> so the size doesn't only depend on the number of transactions, but also on how many of its txns are no longer in the mempool (which is hard to predict)
<lightlike> stickies-v: yes, that's true,, it would be much worse if we did that. But what is not nice is that if for some reason there is a situation with a lot of mempool turnover, mapRelay could become very large in memory consumption.
<stickies-v> (regardless: it highlights the limitations that mapRelay had, e.g. it could easily be abused with high RBF turnover, so perhaps I'm just arguing semantics)
<lightlike> stickies-v: yes, just bounding it in size (e.g. by evicting earlier when it gets too large) would've been an alternative possibility, but in my opionion that would only make sense if mapRelay had an important use case in the first place (which doesn't seem to be the case anymore)
<abubakarsadiq> I think to be able to relay transactions that was mined in the most recent block, because it will be dropped from the mempool, hence m_most_recent_block_txs will help
<stickies-v> lightlike: when a new block comes in and we validate it, we remove those transactions from our mempool, but we still want to be able to relay those recent txs to our peers, because they're very relevant
<stickies-v> isn't that helpful for block propagation speed? if you receive a block but don't yet have all txs, you'd want to have as many peers as possible that can relay you that tx?
<lightlike> I wonder about the timing here. When we have received and connected a new block, wouldn't we immediately announce it to our peers, so they'd get the transactions from us anyway during compact block download?
<stickies-v> hmmm. would this be helpful during a chain split? a peer on a different tip would still benefit from being able to get the tx, and potentially won't have received it as part of a block?
<stickies-v> i don't have the numbers, but it's bounded to the (in-memory) size of the `CTransaction`s in the previous block, plus a bit of overhead for the index etc?
<lightlike> I think it's even less. We save the last block anyway in m_most_recent_block, which includes CTransactionRef (the shared pointer) to all its txns. So even after the txns are removed from the mempool, I think there won't be extra storage requirement for the actual transaction data.
<lightlike> next q: Are there scenarios in which transactions would be made available for a shorter or longer time than before as a result of this change?
<stickies-v> with `RELAY_TX_CACHE_TIME = 15min`, it looks like txs were expired from mapRelay after 15 mins, so I suppose they're available longer whenever there is more than 15 mins since the last block?
<instagibbs> only I think in the case of a reorg race? if you've marked it conflicted, you're now on a heavier chaintip, and you want peers to get stuff on that heavier chaintip
<stickies-v> instagibbs: I don't actually mean from a relay pov, just for yourself to more quickly be able to validate the new chaintip? since you still have the tx you don't need to fetch it from a peer?
<stickies-v> yeah sorry no idea :-D also this is quite edge case and probably (definitely?) not worth optimizing for, just the only thing i could come up with
<instagibbs> spending time avoiding reorgs through faster prop seems better value than handling them slightly faster... gut feeling :) thanks lightlike