[bitcoindev] Transaction expiration should be based on the most recent transaction in a group, not the first

[bitcoindev] Transaction expiration should be based on the most recent transaction in a group, not the first

[Peter Todd]

[-- Attachment #1: Type: text/plain, Size: 2798 bytes --]

Disclaimer: I haven't actually tested this. So there is a chance I'm
understanding the code entirely wrong. If so, feel free to make fun of
me for being too lazy to actually test this.


In Bitcoin Core, mempool expiration is done by:

	int CTxMemPool::Expire(std::chrono::seconds time)
	{
	    AssertLockHeld(cs);
	    indexed_transaction_set::index<entry_time>::type::iterator it = mapTx.get<entry_time>().begin();
	    setEntries toremove;
	    while (it != mapTx.get<entry_time>().end() && it->GetTime() < time) {
		toremove.insert(mapTx.project<0>(it));
		it++;
	    }    
	    setEntries stage;
	    for (txiter removeit : toremove) {
		CalculateDescendants(removeit, stage);
	    }    
	    RemoveStaged(stage, false, MemPoolRemovalReason::EXPIRY);
	    return stage.size();
	}

	https://github.com/bitcoin/bitcoin/blob/b432e367427f1f9fe0f0a5800e31e496f00cd38d/src/txmempool.cpp#L1086

This function is expiring transactions based on their entry time into
the mempool, a value that is set once and never changed. Transactions
are removed unconditionally on expiration, whether or not they have
descendents. That means that if you broadcast A, wait just prior to A's
expiration, and broadcast B, a transaction spending an output of A, B
will be evicted immediately when A's expiration time is reached.

There's at least three problems with this:

1) It's dumb. If I do a CPFP on an old transaction, I want that
   transaction to get mined and am willing to pay money. It's silly to make
   me jump through the hoop of rebroadcasting it again when it expires.

2) It's a free-relay DoS attack: just prior to A expiring, I could
   broadcast B, a very large transaction, and use up bandwidth for "free".
   Frankly, I'm not very concerned about this. But if you care, you
   should fix this.

3) Expiration could maybe be leveraged in transaction cycling attacks:
   https://stacker.news/items/866680

Personally, I'm not convinced that transaction expiration is actually a
good idea. The best argument for it IMO is in the case of some
soft-fork-style screwup where you're allowing stuff into your mempool
that will never get mined. But that means something is seriously wrong
to begin with - you probably should fix that. Otherwise, it's not
uncommon for transactions that are months old to eventually get mined.
Do we really need to waste bandwidth re-relaying them in the meantime?

-- 
https://petertodd.org 'peter'[:-1]@petertodd.org

-- 
You received this message because you are subscribed to the Google Groups "Bitcoin Development Mailing List" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bitcoindev+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/bitcoindev/Z5lZc28t9-tCxdHN%40petertodd.org.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [bitcoindev] Transaction expiration should be based on the most recent transaction in a group, not the first

['ArmchairCryptologist' via Bitcoin Development Mailing List]

Is expiration-based mempool eviction necessary or even desirable anymore? I'm consistently seeing unconfirmed transactions from months ago being rebroadcast and (now that the mempool is draining) eventually confirming, without anyone even trying to exploit anything. So from what I can tell, the only thing this really accomplishes is wasting CPU cycles and bandwidth evicting and later re-accepting the transactions in question.

You were never able to rely on unconfirmed transactions ever going away without double-spending one of the inputs in the first place, and full-RBF is even a thing now, so this will always be possible.

The mempool is capped by size anyway, so while I may be missing something, I cannot honestly see any good reasons to keep this mechanism at all, especially if it can be used as a vector for attacks.

The only drawback I can think of is that abandontransaction currently does not work if a transaction is in the mempool, but it would probably be better to improve it so it actually evicts the transaction from the mempool of the local node if necessary.

--
Best,
ArmchairCryptologist

On Tuesday, January 28th, 2025 at 11:25 PM, Peter Todd <pete@petertodd.org> wrote:

> Disclaimer: I haven't actually tested this. So there is a chance I'm
> understanding the code entirely wrong. If so, feel free to make fun of
> me for being too lazy to actually test this.
> 
> 
> In Bitcoin Core, mempool expiration is done by:
> 
> int CTxMemPool::Expire(std::chrono::seconds time)
> {
> AssertLockHeld(cs);
> indexed_transaction_set::index<entry_time>::type::iterator it = mapTx.get<entry_time>().begin();
> 
> setEntries toremove;
> while (it != mapTx.get<entry_time>().end() && it->GetTime() < time) {
> 
> toremove.insert(mapTx.project<0>(it));
> 
> it++;
> }
> setEntries stage;
> for (txiter removeit : toremove) {
> CalculateDescendants(removeit, stage);
> }
> RemoveStaged(stage, false, MemPoolRemovalReason::EXPIRY);
> return stage.size();
> }
> 
> https://github.com/bitcoin/bitcoin/blob/b432e367427f1f9fe0f0a5800e31e496f00cd38d/src/txmempool.cpp#L1086
> 
> This function is expiring transactions based on their entry time into
> the mempool, a value that is set once and never changed. Transactions
> are removed unconditionally on expiration, whether or not they have
> descendents. That means that if you broadcast A, wait just prior to A's
> expiration, and broadcast B, a transaction spending an output of A, B
> will be evicted immediately when A's expiration time is reached.
> 
> There's at least three problems with this:
> 
> 1) It's dumb. If I do a CPFP on an old transaction, I want that
> transaction to get mined and am willing to pay money. It's silly to make
> me jump through the hoop of rebroadcasting it again when it expires.
> 
> 2) It's a free-relay DoS attack: just prior to A expiring, I could
> broadcast B, a very large transaction, and use up bandwidth for "free".
> Frankly, I'm not very concerned about this. But if you care, you
> should fix this.
> 
> 3) Expiration could maybe be leveraged in transaction cycling attacks:
> https://stacker.news/items/866680
> 
> Personally, I'm not convinced that transaction expiration is actually a
> good idea. The best argument for it IMO is in the case of some
> soft-fork-style screwup where you're allowing stuff into your mempool
> that will never get mined. But that means something is seriously wrong
> to begin with - you probably should fix that. Otherwise, it's not
> uncommon for transactions that are months old to eventually get mined.
> Do we really need to waste bandwidth re-relaying them in the meantime?
> 
> --
> https://petertodd.org 'peter'[:-1]@petertodd.org
> 
> --
> You received this message because you are subscribed to the Google Groups "Bitcoin Development Mailing List" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to bitcoindev+unsubscribe@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/bitcoindev/Z5lZc28t9-tCxdHN%40petertodd.org.

-- 
You received this message because you are subscribed to the Google Groups "Bitcoin Development Mailing List" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bitcoindev+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/bitcoindev/rqLkuDekAhYC8mO9BMrqXGQjv-Oq1a1jA2EDLdiohq9kHIQpE-MpzqFjqY15Cc5flU_GUrE7NoSxB_4wgkQg8Mv8IQdtzlXLtuxK76n7YRQ%3D%40protonmail.com.