Exclude RBF replacement txs from fee estimation

morcos commented at 8:27 pm on January 11, 2017: member

This has little effect now as less than 0.1% of transactions are replacement transactions, but until we’re confident that opt-in-RBF is widely accepted as miner policy, it’s probably better not to consider replacement transactions for fee estimation.

in src/validation.cpp: in 20418bdf79 outdated

787@@ -788,13 +788,15 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
788         size_t nConflictingSize = 0;
789         uint64_t nConflictingCount = 0;
790         CTxMemPool::setEntries allConflicting;
791+        bool fReplacementTransaction = false;

JeremyRubin commented at 9:05 pm on January 11, 2017:

nit: you may as well make this const bool fReplacementTransaction = setConflicts.size() under the lock & make the if below check fReplacementTransaction.

in src/validation.cpp: in 20418bdf79 outdated

961-        // transactions in the mempool
962-        bool validForFeeEstimation = IsCurrentForFeeEstimation() && pool.HasNoInputsOf(tx);
963+        // the node is not behind, it is not dependent on any other
964+        // transactions in the mempool, and it isn't a BIP 125
965+        // replacement transaction (may not be widely supported).
966+        bool validForFeeEstimation = IsCurrentForFeeEstimation() && pool.HasNoInputsOf(tx) && !fReplacementTransaction;

JeremyRubin commented at 9:06 pm on January 11, 2017:

nit: !fReplacementTransaction is a lot cheaper of a check than the others, so may as well make it the first one in the group of &s. I don’t think the optimizer would do that automatically?

jtimon commented at 7:18 pm on January 19, 2017:

No, the optimizer shouldn’t do that automatically, because it could be done on purpose (ie to guarantee IsCurrentForFeeEstimation() is always called, even if fReplacementTransaction, assuming IsCurrentForFeeEstimation() changes some state). So, yeah, +1 on the nit.

JeremyRubin commented at 9:29 pm on January 11, 2017: contributor

My conceptual worry is this makes possible some kind of attack where all clients are now incentivized to do RBF for transactions they want to go through quickly because it will not affect network fee estimates. E.g.: I want to pay 2x normal fee for a faster transaction. First, I broadcast at 0.5 normal fee and wait a few seconds for it to propagate. Next, I broadcast at the 2x normal fee (which I was always willing to pay), but now this code does not include my fee in the estimate.

One way to address this vector would be to not mark it as invalid for fee estimate if the conflicting transaction comes in within a certain time window of the original. This way, the user derives minimal benefit from the above.

If the above is not actually an issue, this is a utACK from me (+ 2 minor code style nits).

luke-jr commented at 9:46 pm on January 11, 2017: member

Too much special-casing here… Estimates shouldn’t break when 100% of the network adopts RBF support. But even more importantly, estimates shouldn’t break when miners use unknown policies. There needs to be some tolerance of transactions being left unmined for reasons beyond the estimator’s awareness.

morcos commented at 10:39 pm on January 11, 2017: member

@JeremyRubin

re: nits, seems kind of unnecessary optimization if you ask me..
re: concern, I won’t go so far as to say that’s not a concern at all, but the debug information for fee estimates already lets you know how many of the transactions in blocks you’re tracking. So if that number drops significantly it’ll be worth revisiting. Beyond that, it shouldn’t matter if a bunch of high fee txs aren’t tracked as long as enough still are to give you the data you need. @luke-jr sigh… sure, estimates should be magical and give you perfect answers. But whether this is the optimal algorithm is not the question here. The question is whether this is a reasonable improvement and I believe it is.

fanquake added the label Validation on Jan 11, 2017

jonasschnelli added the label TX fees and policy on Jan 12, 2017

jonasschnelli removed the label Validation on Jan 12, 2017

laanwj added this to the milestone 0.14.0 on Jan 12, 2017

sdaftuar commented at 9:46 pm on January 12, 2017: member

utACK

dcousens commented at 4:28 am on January 13, 2017: contributor

it’s probably better not to

Why not consider them?

morcos commented at 2:04 pm on January 13, 2017: member

@dcousens The issue is that if some significant fraction of miners don’t accept RBF replacements but your node does, then you would expect them to get mined quickly based on their fee rate but in reality it will take longer. In the worst case situation this can lead fee estimation to think that all transactions of that fee rate would take longer to be confirmed.

If you know that some miners are not considering a certain subset of transactions for block inclusion, its better to eliminate those as potential data points for fee estimation. There are two opposing arguments to this:

You’ve eliminated so many transactions that you don’t have a big enough sample of data points. (Not a concern here)
What if the particular transaction you are placing is of the type that is chosen to be excluded for fee estimation because its disadvantaged. This is a reason to not overly narrow the policy profile required for fee estimation. But in the case of replacement transactions, I think its relatively well known that not all miners might accept your replacement, and in the case they don’t, the alternative is your original transaction is still mined, so its probably ok if fee estimates are maybe not perfectly tailored to your particular transaction.

JeremyRubin commented at 10:28 pm on January 13, 2017: contributor

I am convinced now that the issue I brought up is unlikely to be workable, full utACK!

sipa commented at 11:06 pm on January 13, 2017: member

Concept ACK

dcousens commented at 11:13 pm on January 13, 2017: contributor

Thanks for the explanation @morcos.

I still think given the fact this has near zero effect (0.1%), and any increased usage of RBF would also incite miner acceptance… I don’t think this is necessary, but, if it was an issue impacting estimates substantially (and negatively), I would agree this is a good temporary fix.

Weak NACK

edit: I also think, if #9527 is to be enabled… these numbers could change dramatically. If the belief is that miners aren’t accepting RBF… then yes, concept ACK.

dcousens commented at 11:16 pm on January 13, 2017: contributor

Do we have any data/estimates on widespread RBF policy usage [by miners]?

MarcoFalke commented at 10:36 am on January 14, 2017: member

utACK 20418bdf7976b8ff807dc6ed9f2a8e6b7f068ec3

laanwj commented at 6:46 am on January 17, 2017: member

I have the same worry as @luke-jr here, the special-casing seems surprising and unexpected and will distort estimates once RBF is more widely adopted.

Once they are in the block chain, RBF transactions are the same as non-RBF transactions. Also the fee from a RBF transaction is worth just as much as from a non-RBF transaction. So I’m not sure why a miner would adopt a anti-RBF policy in the first place. But if they do, I don’t think we should special-case our code in it.

Especially if we want to generate RBF transactions ourselves later it looks like something that will come back to bite us.

Do we have any data/estimates on widespread RBF policy usage [by miners]?

I’d like to see this too.

morcos commented at 1:23 pm on January 17, 2017: member

To address the question about distorting estimates, I don’t really believe that is a concern.

Fundamentally if RBF replacements have the same confirmation profile as regular transactions, then none of this matters as long as we have enough total data points, which I think we are nowhere near worrying about even if all txs are default replaceable (only actual replacements are excluded).

If they have a different confirmation profile than regular transactions, then you have to look at the two sets of data separately or you can only give an answer for the subset of data that you look at. In our case there certainly isn’t enough data now to give an answer for replacements on their own, so we might as well exclude replacements from the regular transaction data so we are giving an unpolluted answer for regular transactions at least.

In other words its no harm done if they aren’t really subject to different policy. If they are, then this is playing it safe to the best degree we are able.

btcdrak commented at 10:43 am on January 18, 2017: contributor

Current usage stats of RBF transaction produced is about 0.2%-0.3%

TheBlueMatt commented at 6:36 pm on January 18, 2017: member

I dont buy the “miners will be more incentivized to run RBF” argument. While this is true, miners (in this case pools) have shown both a) long lags from incentivization to when they start adopting things which pay them more (eg many miners are still leaving lots on the table by not using 0.13 to do CPFP), and b) willingness to ignore incentivization when there is community drama - its not their income that they’re losing out on, its their clients.

On the flip side, I do buy that opt-in RBF will continue to get some additional adoption because of the big UX win for some wallets by using it, and while its useable today because there is some adoption from miners, the inconsistency of said adoption makes including it for fee esimation very painful.

Finally, the way this is written, it is trivial to revert even in a point release if we decide things have upgraded faster than we thought.

Strong Concept ACK.

TheBlueMatt commented at 6:50 pm on January 18, 2017: member

utACK 20418bdf7976b8ff807dc6ed9f2a8e6b7f068ec3

btcdrak commented at 7:18 pm on January 19, 2017: contributor

utACK 20418bd

jtimon commented at 7:30 pm on January 19, 2017: contributor

utACK 20418bd reverting this when we feel it makes sense should be equally simple anyway. I don’t see the reason not to fix @JeremyRubin ’s nit. It’s trivial to fix, and if anything, it will prevent someone else from bringing up the same micro-optimization again, or just think about it when reading the code.

morcos commented at 1:18 am on January 20, 2017: member

OK added @JeremyRubin’s optimization suggestions

Exclude RBF txs from fee estimation de1ae324bf

morcos force-pushed on Jan 20, 2017

morcos commented at 8:06 pm on January 20, 2017: member

Squashed bd1f849 -> de1ae32

jtimon commented at 9:50 pm on January 20, 2017: contributor

re utACK de1ae32

petertodd commented at 4:38 am on January 21, 2017: contributor

If you want a source of opt-in RBF confirmation data, my OpenTimestamps calendars use opt-in RBF for all transactions; the fee strategy there is to start with the lowest possible fee, and then keep incrementing the fees with replacements over time. So any OpenTimestamps transaction that is mined is pretty much guaranteed to have been due to a replacement on default Bitcoin Core mempool settings.

Here’s a sample tx: 6c13d8bcd5d4e397300302e635e3e13a1b7af929fb00ce6c7b71645e95b7b548

All prior OP_RETURN txs in that chain will have been from OpenTimestamps. I took a quick look myself, and it looks like those transactions are getting mined by a large percentage of hashing power, including large pools like F2Pool, AntPool, BitFury, etc.

TheBlueMatt commented at 6:14 pm on January 21, 2017: member

@petertodd I’m not sure that is required? Because blocks are not always full, if you get a transaction propagated its virtually guaranteed to get mined eventually still. What is your exact policy - is it possible that you’re simply seeing the first relayed transaction get confirmed (keeping in mind that relay of things with low fee can be super unreliable)?

sdaftuar commented at 8:45 pm on January 23, 2017: member

re-utACK

dcousens commented at 2:03 am on January 24, 2017: contributor

Could we just compare the median “time to confirm” for RBF v non-RBF to compare probability of miner acceptance?

btcdrak commented at 11:25 am on January 24, 2017: contributor

re-utACK de1ae32

btcdrak approved

jtimon approved

gmaxwell commented at 10:48 pm on January 25, 2017: contributor

ACK. The only downside of this is that if replacements become common, we may not end up with enough data. But if replacements were common, we could assume miners were mining them, and it would be harmless to remove this. We know that right now that they do have a different confirmation profile, so I think this is the correct change to make.

dcousens commented at 4:13 am on January 26, 2017: contributor

We know that right now that they do have a different confirmation profile, so I think this is the correct change to make.

How do we know this though? I’ve been recording data for the last week in regards to this so I can verify myself, but I haven’t seen any data presented in this thread.

edit:

We know they constitute ~0.2%: http://p2sh.info/dashboard/db/replace-by-fee But their confirmation profile?

laanwj merged this on Jan 26, 2017

laanwj closed this on Jan 26, 2017

laanwj referenced this in commit 9b4d2673b7 on Jan 26, 2017

dcousens commented at 12:33 pm on February 3, 2017: contributor

I collected 5181335 transactions between blocks 448207 and 451298, observing the txId and the height of the blockchain when first seen, later taking confirmed_height - seen_height to determine how many blocks each transaction waited until confirmation.

In that time, 14101 transactions were seen signalling RBF (~0.27%), with a median wait of 1 block until confirmation and 78 bytes satoshis/byte. This matches the confirmation profile of non-RBF transactions, which had a median fee of 80 satoshis/byte.

The mean wait time for standard transactions [seen in this time period] was 6.87137 blocks, with RBF transactions on average confirming within 5.38064 blocks.

Data available if needed, happy to query in any way necessary.

edit: after discussion with @gmaxwell, it appears the suggested problematic confirmation profile isn’t in regards to RBF signalling transactions, but their actual replacements. The above analysis isn’t presently accounting for that, so I’ll have to look into it further :+1: .

codablock referenced this in commit 32f1010bb0 on Jan 19, 2018

codablock referenced this in commit e77c6219d6 on Jan 20, 2018

codablock referenced this in commit 6070cf111b on Jan 21, 2018

andvgal referenced this in commit e967de655c on Jan 6, 2019

CryptoCentric referenced this in commit ce8c9288d2 on Feb 27, 2019

MarcoFalke locked this on Sep 8, 2021

Exclude RBF replacement txs from fee estimation #9519