Feature: Psbt fee checks

[junderw]

Closes #2061

These new methods on Psbt will add checks for high fees by default. The threshold for "high fees" is currently set to 25000 sat/vbyte, which is about 20x higher than the highest next block fees seen on the "Mempool" website.

The primary goal of this change is to prevent users of the library from accidentally sending absurd amounts of fees.

(ie. Recently in September 2023 there was a transaction that sent an absurd amount of fees and made news in the Bitcoin world. Luckily the mining pool gave it back, but some might not be so lucky.)

There are variants of the method that allow for users to set their own "absurd" threshold using a FeeRate value. And there is a method that performs no checks, and the method name is alarming enough to draw attention in a review, so at least developers will be aware of the concept.

[junderw]

Also, I keep getting these annoying errors in rust-analyzer and I can't figure out how to configure RA to make them go away.

Anyone know what's going on? (The terminal never complains, cargo check, cargo test, build whatever are all fine... it's just RA)

[sanket1729]

I have learned to live with those rust-analyzer errors. Also interested in seeing if anyone else has a solution for those.

[apoelstra]

63f500e looks good to me, except that I think we should simply drop the "fee ratio" stuff. The network doesn't respect any sort of value-to-fee ratio.

It's also not obvious which direction this ratio goes, nor how the "value" is determined in a multi-output transaction, so I think people are likely to use this incorrectly.

Though if we were to have such a thing, I agree that "fees are 10x the value" is a good starting point..

[junderw]

It's also not obvious which direction this ratio goes, nor how the "value" is determined in a multi-output transaction, so I think people are likely to use this incorrectly.

I think this is the biggest issue. Since "fee rate" is a concept that 99 out of 100 devs will understand without much explanation, but this new "fee ratio" concept being "the fee in satoshis divided by the total value in satoshis of all outputs including change if any" is a new concept... and because of that, it's hard to grasp what's a "very large" number for that.

I myself had to run some scripts on a local indexed set of transactions and got medians, means, and distributions of fee ratio before I came to the conclusion of 10x being high enough to reduce false positives.

Going back to genesis, in general people sent larger BTC amounts (when you include change, even moreso) and fees were usually fixed at 0.0001 BTC until around 2017. So fee ratio averaged out to about 0.03 ish.

In more recent times, there's a lot more variance, but the median is still around 0.1 and the mean is around 0.3 (meaning if the sum of outputs is 0.1 BTC, your fee is 0.03 BTC)

When I looked into outliers, I found that 10.0 and above tend to jump up to 90.0, 120.0, etc. (I think the famous over-pay from the other day was a fee ratio of about 150.0) and there were not a lot of instances of txes in the 10.0 - 50.0 range.

---

That said, I think that maybe this can be fixed with better documentation. The point of this new code is not to represent some sort of network phenomena, but rather to prevent fat-fingered sends of large fees (usually stemming from forgetting / clobbering the change output somehow).

I am still open to removing the fee ratio part, but I'd like to hear some other opinions about it before I remove it.

[tcharding]

If this is, as stated, just fat-finger protection then I don't think we need the added complexity of a fee ration - just pick a sane default for the max fee rate.

[junderw]

1. Removed Ratio
2. Use FeeRate for fee rate
3. Rebased on master

[junderw]

Removed the new internal functions and instead use the existing fee() method.

[apoelstra]

ACK 7f6980e

[junderw]

Any comments on #2064 (comment) this?

"An absurdly high fee rate of 6250000 sats / kwu"

vs

"An absurdly high fee rate of 25000 sats / vByte"

Considering sat/vByte is the accepted unit right now..... I think using the latter would be better.

Thoughts?

[apoelstra]

I have very little preference, but agree that "sat/vbyte" is probably better and more familiar.

I'd also like to stick a .00 onto the number, basically as a PSA that "1 sat/vbyte" is not actually the minimum possible fee and that feerates are not discretized in terms of feerates. This has been a mildly annoying confusion ever since mempool websites became popular.

[tcharding]

I was reviewing while you force pushed so I might have commented on old state.

[apoelstra]

ACK a64c3ad -- though @tcharding's comments all sound reasonable to me.

[junderw]

I'm going to add tests later today.

[junderw]

Maybe tomorrow. heh.

[apoelstra]

ACK dac627c

[tcharding]

Looking good man, can you re-write the PR description please (since there is not a descriptive commit log on the patch we are relying on the PR description for context on this work).

[junderw]

Updated the main post.

[tcharding]

ACK dac627c

[tcharding]

FWIW I rekon this is ok to go in before release although it is not explicitly mentioned in the release tracking discussion.