Fix a bug if the min fee is 0 for FeeFilterRounder #9288

pull morcos wants to merge 2 commits into bitcoin:master from morcos:fixFFRbug changing 3 files +4 −3

morcos commented at 2:52 AM on December 6, 2016: member

Thanks @dcousens and @gmaxwell
gmaxwell commented at 2:57 AM on December 6, 2016: contributor

ACK. This appears sensible and also fixes the bug.
dcousens commented at 3:13 AM on December 6, 2016: contributor

ACK 0e78469, ~~does it need to be put in place at https://github.com/bitcoin/bitcoin/blob/master/src/policy/fees.cpp#L304 also?~~

Probably not, since that is bounded by MIN_FEERATE...

Actually, probably is worth it since if someone changed MIN_FEERATE to 0, it would dead lock again.
laanwj commented at 8:56 AM on December 6, 2016: member

utACK 0e78469
MarcoFalke commented at 10:57 AM on December 6, 2016: member

if someone changed MIN_FEERATE to 0

I think we can't protect against developers randomly changing internal constants.

utACK 0e78469
sipa commented at 5:08 PM on December 6, 2016: member

@dcousens One way to deal with such constraints that some parts of the code have, is by adding a static_assert(MIN_FEERATE > 0, "Min feerate must be nonzero); in the place where it is used.
dcousens commented at 10:16 PM on December 6, 2016: contributor

@sipa sounds fine to me
fanquake added the label TX fees and policy on Dec 8, 2016
laanwj commented at 7:34 AM on December 8, 2016: member

Yes, static assertions would be the way to go then.
laanwj cross-referenced this on Dec 8, 2016 from issue Inline CTxInWitness inside CTxIn by sipa
morcos force-pushed on Dec 8, 2016
morcos commented at 2:19 PM on December 8, 2016: member

I added the static_assert, although I'm not sure it matters since the value used is actually max of MIN_FEERATE and minRelayTxFee which can't be 0. But it doesn't hurt.

I noticed another "bug" if a minRelayTxFee of 1 satoshi is used in the mempool min fee decay. Wouldn't mind someone else verifying the fix makes sense though..
fix a bug if the min fee is 0 for FeeFilterRounder eab8e1b172
Avoid rollingMinimumFeeRate never being able to decay below half f8d43b8081
morcos force-pushed on Dec 8, 2016
gmaxwell commented at 11:03 PM on December 12, 2016: contributor

re-ACK.
laanwj cross-referenced this on Dec 14, 2016 from issue Node crashed with assertion failure in network code by laanwj
laanwj commented at 11:28 AM on January 4, 2017: member

utACK f8d43b8
laanwj merged this on Jan 4, 2017
laanwj closed this on Jan 4, 2017
laanwj referenced this in commit 1ce7ede2a9 on Jan 4, 2017
codablock referenced this in commit bdd151f598 on Jan 18, 2018
andvgal referenced this in commit eb7947e9ca on Jan 6, 2019
CryptoCentric referenced this in commit 502d1ca720 on Feb 26, 2019
random-zebra cross-referenced this on Feb 5, 2021 from issue [Bug][Refactoring] Miscellaneous fixes by random-zebra
Fuzzbawls referenced this in commit 71ed429370 on Feb 14, 2021
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#8589 Inline CTxInWitness inside CTxIn #9348 Node crashed with assertion failure in network code