Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add min_relay_fee checks for fee-estimation calls #1016

Closed
ariard opened this issue Jul 25, 2021 · 3 comments · Fixed by #1552
Closed

Add min_relay_fee checks for fee-estimation calls #1016

ariard opened this issue Jul 25, 2021 · 3 comments · Fixed by #1552

Comments

@ariard
Copy link

ariard commented Jul 25, 2021

See #985 (comment)

@dunxen
Copy link
Contributor

dunxen commented Jun 17, 2022

Working on this

@dunxen
Copy link
Contributor

dunxen commented Jun 20, 2022

Kind of related, but is this constant not supposed to be 250 for the default min relay fee in Bitcoin Core policy?

pub const MIN_RELAY_FEE_SAT_PER_1000_WEIGHT: u64 = 4000;

(Divide 1000 by 4 instead of multiplying?)

$$ {1000 sats \over 1000 vB} \times { 1 vB \over 4 WU } \times 1000 = 250 sats $$

I might be misunderstanding that variable name.

I'll create a const for the 253 sat/kvB min rate for the assertions needed for this issue. I think CLN calls it FEERATE_FLOOR to distinguish it from Bitcoin Core policy one they name BITCOIND_MINRELAYTXFEE_PER_KW which is 250.

@ariard
Copy link
Author

ariard commented Jun 21, 2022

If you would like the rational Bitcoin Core rounds up during the conversion from weight to vbytes : https://github.com/bitcoin/bitcoin/blob/master/src/policy/policy.cpp#L296

Therefore, if your transaction is let's say 1003 WU offering a feerate of 250 sats/kW because you think 1003 / WITNESS_SCALE_FACTOR = 250, you might think you're okay. You're not. Your transaction should be scaled to 251 vbytes and the required min fee by Core should be 251 satoshis (see CFeeRate::GetFee).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants