Anti-fee snipe in bumpfee with time-based locktimes #35628

I've been looking at the part of the code responsible for anti-fee sniping in wallet/spend.cpp. I noticed that when creating replacement transactions via fee bumping, we don't preserve the previous locktime. As a result, the locktime type can change across replacements. For example, if you create a transaction with a time-based locktime and then replace it without explicitly specifying a locktime, the replacement may end up using a height-based locktime. I wrote this test to reproduce the behavior I described.

Initially, I thought this was undesirable because changing the locktime type makes it much easier to identify that we're using anti-fee sniping. The solutions I considered were either not setting a locktime at all or preserving the locktime from the original transaction. I implemented the last approach here.

However, I'm no longer convinced this is the right direction. Preserving the original locktime would effectively disable anti-fee sniping for transactions that already use a time-based locktime. It would also introduce different behavior for RBF replacements depending on whether the original transaction used a time-based or height-based locktime, and I'm not sure that's desirable.

dupe of #26526?

dupe of #26526?

The issue #26526 is to not have a higher locktime in the original transaction than in the replacement as it would indicate that we are anti-fee sniping from either Core or Electrum.

My issue is focused on changing the type of locktime if the original transaction has a time-based lockitme, as if this is the case right now we would end up with a time-based locktime in the original transaction and a height-based locktime in the replacement. I don't think this should be the current behaviour as it can be used to identify we are antifee sniping, however I am not super convinced that the solution I proposed (second paragraph) are better than the current approach.

I hope I explained myself clearly.

Yeah, I see. I mostly think that either both issues will be fixed at the same time, or closed at the same time as wontfix.

When it comes to privacy, let's recall that time-based locktimes are already used way less than 1% of the time (https://mainnet.observer/charts/transactions-timestamp-based-locktime/), and height-based locktimes are sitting right now at less than 5%: https://mainnet.observer/charts/transactions-height-based-locktime/.

Yeah, I see. I mostly think that either both issues will be fixed at the same time, or closed at the same time as wontfix.

Yes, I was working to fix the backdating in bumpfee and then saw this issue with the time-based locktime. I just wanted to discuss what is the best approach before opening a PR.

When it comes to privacy, let's recall that time-based locktimes are already used way less than 1% of the time (https://mainnet.observer/charts/transactions-timestamp-based-locktime/), and height-based locktimes are sitting right now at less than 5%: https://mainnet.observer/charts/transactions-height-based-locktime/.

Abolutely, especially time-based locktime are almost never used. And if we counted how many replacements had a time-based locktime in the original transaction, and then no locktime set in the replacement it would be even less. However, I still think it is worth it paying some attetion to this cases since the fix it is not very complicated.