Fix estimatesmartfee rounding display issue #11303
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We were saving a div by caching the inverse as a float, but this ended up requiring a int -> float -> int conversion, which takes almost as much time as the difference between float mul and div. There are lots of other more pressing issues with the bench framework which probably require simply removing the adaptive iteration count stuff anyway.
TheBlueMatt
force-pushed
the
2017-09-estimatesmartfee-round
branch
from
c572f2a
to
40467e2
Compare
|
I'm scared that we use floats for monetary values in these places, even when just for display. This is ok for a workaround, but in the long run it would be preferable for these to be converted to fixed point math. |
This resolves an issue where estimatesmartfee would return 999 sat/byte instead of 1000, due to floating point loss of precision Thanks to sipa for suggesting is_integral.
TheBlueMatt
force-pushed
the
2017-09-estimatesmartfee-round
branch
from
40467e2
to
1789e46
Compare
sipa
reviewed
Sep 12, 2017
| @@ -55,21 +55,20 @@ bool benchmark::State::KeepRunning() | |||
| else { | |||
| now = gettimedouble(); | |||
| double elapsed = now - lastTime; | |||
| double elapsedOne = elapsed * countMaskInv; | |||
| double elapsedOne = elapsed / (countMask + 1); | |||
There was a problem hiding this comment.
In commit "Remove countMaskInv caching in bench framework".
Isn't countMask always a power of two minus one? If so, we could just remember the number of bits in it, and use a shift instead of a division?
EDIT: this doesn't apply here, as you can't shift a double, but does in line 64.
There was a problem hiding this comment.
I'm gonna leave this be, I think we need to re-write the bench crap anyway (there is a very massive difference between runs based on the number of iterations, in many of my tests, so all of this garbage needs to just be removed).
| @@ -1043,5 +1043,5 @@ CAmount FeeFilterRounder::round(CAmount currentMinFee) | |||
| if ((it != feeset.begin() && insecure_rand.rand32() % 3 != 0) || it == feeset.end()) { | |||
| it--; | |||
| } | |||
| return *it; | |||
| return static_cast<CAmount>(*it); | |||
There was a problem hiding this comment.
In commit "Make float <-> int casts explicit outside of test, qt, CFeeRate"
Nit: for primitive types we usually use C-style casts for brevity (in many places in this commit).
There was a problem hiding this comment.
Heh, I was assuming based on #10498 that we want to migrate. I dont care either way.
There was a problem hiding this comment.
Note that #10498 is only about non-primitive types.
I don't care strongly either (it's not in the developer notes) - just pointing out that it differs from common style.
There was a problem hiding this comment.
Ah, heh, well CAmount we stradle the line...cause its a primitive typedef we often pass it by reference. I'll leave it for now since its a bit more clear what going on.
|
Concept ACK 1789e46. I think it is fine to use double for fee estimation, as it is an approximation anyway. The rounding should take care of not violating the edge cases. |
|
The bench changes seem unrelated to estimatefee. Shouldn't those go to another pull? |
|
+1 splitting the changes (EDIT: maybe not worth it now, but i would have preferred that) @laanwj I don't think it makes sense to use integer math for fee estimation. The bug here wasn't necessarily caused by floating point math but by the fact that we were implicitly taking a floor to round the answer to an integer whereas we should have been rounding or ceiling. But we would still need to have made a similar choice even if we weren't using floating point. |
|
All of the changes are from -Wfloat-conversion, so I figured I'd dump them all together, otherwise I would have just skipped the bench/wallet/etc changes and only done the CFeeRate ones. |
|
ACK 1789e46 This doesn't fix all cases of 999 estimates though, my guess is the rest are caused by tracking fee estimation for transactions which enter mempool via reorg. Will fix that separately. |
maflcko
pushed a commit
that referenced
this pull request
Sep 29, 2017
bf64c3c Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos) 04f78ab Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos) fd849e1 Change AcceptToMemoryPool function signature (Alex Morcos) Pull request description: First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior. Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](#9602 (comment)) Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in #11303. Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c
|
Tested ACK 1789e46 |
maflcko
pushed a commit
that referenced
this pull request
Sep 30, 2017
1789e46 Force explicit double -> int conversion for CFeeRate constructor (Matt Corallo) 53a6590 Make float <-> int casts explicit outside of test, qt, CFeeRate (Matt Corallo) 0b1b914 Remove countMaskInv caching in bench framework (Matt Corallo) Pull request description: This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion. Tree-SHA512: 66087b08e5dfca67506da54ae057c2f9d86184415e8fa4fa0199e38839e06a3ce96c836fcb7593b7d960065f5240c594ff3a0cfa14333ac528421f5aeac835c9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 17, 2020
1789e46 Force explicit double -> int conversion for CFeeRate constructor (Matt Corallo) 53a6590 Make float <-> int casts explicit outside of test, qt, CFeeRate (Matt Corallo) 0b1b914 Remove countMaskInv caching in bench framework (Matt Corallo) Pull request description: This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion. Tree-SHA512: 66087b08e5dfca67506da54ae057c2f9d86184415e8fa4fa0199e38839e06a3ce96c836fcb7593b7d960065f5240c594ff3a0cfa14333ac528421f5aeac835c9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 22, 2020
1789e46 Force explicit double -> int conversion for CFeeRate constructor (Matt Corallo) 53a6590 Make float <-> int casts explicit outside of test, qt, CFeeRate (Matt Corallo) 0b1b914 Remove countMaskInv caching in bench framework (Matt Corallo) Pull request description: This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion. Tree-SHA512: 66087b08e5dfca67506da54ae057c2f9d86184415e8fa4fa0199e38839e06a3ce96c836fcb7593b7d960065f5240c594ff3a0cfa14333ac528421f5aeac835c9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 22, 2020
1789e46 Force explicit double -> int conversion for CFeeRate constructor (Matt Corallo) 53a6590 Make float <-> int casts explicit outside of test, qt, CFeeRate (Matt Corallo) 0b1b914 Remove countMaskInv caching in bench framework (Matt Corallo) Pull request description: This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion. Tree-SHA512: 66087b08e5dfca67506da54ae057c2f9d86184415e8fa4fa0199e38839e06a3ce96c836fcb7593b7d960065f5240c594ff3a0cfa14333ac528421f5aeac835c9
zkbot
added a commit
to zcash/zcash
that referenced
this pull request
Jan 24, 2020
Micro-benchmarking framework part 1 Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6733 - bitcoin/bitcoin#6770 - bitcoin/bitcoin#6892 - Excluding changes to `src/policy/policy.h` which we don't have yet. - bitcoin/bitcoin#7934 - Just the benchmark, not the performance improvements. - bitcoin/bitcoin#8039 - bitcoin/bitcoin#8107 - bitcoin/bitcoin#8115 - bitcoin/bitcoin#8914 - Required resolving several merge conflicts in code that had been refactored upstream. The changes were simple enough that I decided it was okay to impose merge conflicts on pulling in those refactors later. - bitcoin/bitcoin#9200 - bitcoin/bitcoin#9202 - Adds support for measuring CPU cycles, which is later removed in an upstream PR after the refactor. I am including it to reduce future merge conflicts. - bitcoin/bitcoin#9281 - Only changes to `src/bench/bench.cpp` - bitcoin/bitcoin#9498 - bitcoin/bitcoin#9712 - bitcoin/bitcoin#9547 - bitcoin/bitcoin#9505 - Just the benchmark, not the performance improvements. - bitcoin/bitcoin#9792 - Just the benchmark, not the performance improvements. - bitcoin/bitcoin#10272 - bitcoin/bitcoin#10395 - Only changes to `src/bench/` - bitcoin/bitcoin#10735 - Only changes to `src/bench/base58.cpp` - bitcoin/bitcoin#10963 - bitcoin/bitcoin#11303 - Only the benchmark backend change. - bitcoin/bitcoin#11562 - bitcoin/bitcoin#11646 - bitcoin/bitcoin#11654 This pulls in all changes to the micro-benchmark framework prior to December 2017, when it was rewritten. The rewrite depends on other upstream PRs we have not pulled in yet. This does not pull in all benchmarks prior to December 2017. It leaves out benchmarks that either test code we do not have yet (except for the `FastRandomContext` refactor, which I decided to pull in), or would require rewrites to work with our changes to the codebase.
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 29, 2020
1789e46 Force explicit double -> int conversion for CFeeRate constructor (Matt Corallo) 53a6590 Make float <-> int casts explicit outside of test, qt, CFeeRate (Matt Corallo) 0b1b914 Remove countMaskInv caching in bench framework (Matt Corallo) Pull request description: This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion. Tree-SHA512: 66087b08e5dfca67506da54ae057c2f9d86184415e8fa4fa0199e38839e06a3ce96c836fcb7593b7d960065f5240c594ff3a0cfa14333ac528421f5aeac835c9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 29, 2020
1789e46 Force explicit double -> int conversion for CFeeRate constructor (Matt Corallo) 53a6590 Make float <-> int casts explicit outside of test, qt, CFeeRate (Matt Corallo) 0b1b914 Remove countMaskInv caching in bench framework (Matt Corallo) Pull request description: This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion. Tree-SHA512: 66087b08e5dfca67506da54ae057c2f9d86184415e8fa4fa0199e38839e06a3ce96c836fcb7593b7d960065f5240c594ff3a0cfa14333ac528421f5aeac835c9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 31, 2020
1789e46 Force explicit double -> int conversion for CFeeRate constructor (Matt Corallo) 53a6590 Make float <-> int casts explicit outside of test, qt, CFeeRate (Matt Corallo) 0b1b914 Remove countMaskInv caching in bench framework (Matt Corallo) Pull request description: This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion. Tree-SHA512: 66087b08e5dfca67506da54ae057c2f9d86184415e8fa4fa0199e38839e06a3ce96c836fcb7593b7d960065f5240c594ff3a0cfa14333ac528421f5aeac835c9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Mar 14, 2020
bf64c3c Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos) 04f78ab Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos) fd849e1 Change AcceptToMemoryPool function signature (Alex Morcos) Pull request description: First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior. Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](bitcoin#9602 (comment)) Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in bitcoin#11303. Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Mar 15, 2020
bf64c3c Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos) 04f78ab Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos) fd849e1 Change AcceptToMemoryPool function signature (Alex Morcos) Pull request description: First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior. Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](bitcoin#9602 (comment)) Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in bitcoin#11303. Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Mar 16, 2020
bf64c3c Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos) 04f78ab Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos) fd849e1 Change AcceptToMemoryPool function signature (Alex Morcos) Pull request description: First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior. Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](bitcoin#9602 (comment)) Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in bitcoin#11303. Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c fix 11309 Signed-off-by: Pasta <pasta@dashboost.org>
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Mar 16, 2020
bf64c3c Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos) 04f78ab Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos) fd849e1 Change AcceptToMemoryPool function signature (Alex Morcos) Pull request description: First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior. Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](bitcoin#9602 (comment)) Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in bitcoin#11303. Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c fix 11309 Signed-off-by: Pasta <pasta@dashboost.org> fix & Signed-off-by: Pasta <pasta@dashboost.org>
furszy
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Jun 8, 2020
3f3edde [Bench] Use PIVX address in Base58Decode test (random-zebra) 5a1be90 [Travis] Disable benchmark framework for trusty test (random-zebra) 1bd89ac Initialize recently introduced non-static class member lastCycles to zero in constructor (random-zebra) ec60671 Require a steady clock for bench with at least micro precision (random-zebra) 84069ce bench: prefer a steady clock if the resolution is no worse (random-zebra) 38367b1 bench: switch to std::chrono for time measurements (random-zebra) a24633a Remove countMaskInv caching in bench framework (random-zebra) 9e9bc22 Restore default format state of cout after printing with std::fixed/setprecision (random-zebra) 3dd559d Avoid static analyzer warnings regarding uninitialized arguments (random-zebra) e85f224 Replace boost::function with std::function (C++11) (random-zebra) 98c0857 Prevent warning: variable 'x' is uninitialized (random-zebra) 7f0d4b3 FastRandom benchmark (random-zebra) d9fa0c6 Add prevector destructor benchmark (random-zebra) e1527ba Assert that what might look like a possible division by zero is actually unreachable (random-zebra) e94cf15 bench: Fix initialization order in registration (random-zebra) 151c25f Basic CCheckQueue Benchmarks (random-zebra) 51aedbc Use std:thread:hardware_concurrency, instead of Boost, to determine available cores (random-zebra) d447613 Use real number of cores for default -par, ignore virtual cores (random-zebra) 9162a56 [Refactoring] Removed using namespace <xxx> from bench/ sources (random-zebra) 5c07f67 bench: Add support for measuring CPU cycles (random-zebra) 41ce1ed bench: Fix subtle counting issue when rescaling iteration count (random-zebra) 68ea794 Avoid integer division in the benchmark inner-most loop. (random-zebra) 3fa4f27 bench: Added base58 encoding/decoding benchmarks (random-zebra) 4442118 bench: Add crypto hash benchmarks (random-zebra) a5179b6 [Trivial] ensure minimal header conventions (random-zebra) 8607d6b Support very-fast-running benchmarks (random-zebra) 4aebb60 Simple benchmarking framework (random-zebra) Pull request description: Introduces the benchmarking framework, loosely based on google's micro-benchmarking library (https://github.com/google/benchmark), ported from Bitcoin, up to 0.16. The benchmark framework is hard-coded to run each benchmark for one wall-clock second, and then spits out .csv-format timing information to stdout. Backported PR: - bitcoin#6733 - bitcoin#6770 - bitcoin#6892 - bitcoin#8039 - bitcoin#8107 - bitcoin#8115 - bitcoin#9200 - bitcoin#9202 - bitcoin#9281 - bitcoin#6361 - bitcoin#10271 - bitcoin#9498 - bitcoin#9712 - bitcoin#9547 - bitcoin#9505 (benchmark only. Rest was in #1557) - bitcoin#9792 (benchmark only. Rest was in #643) - bitcoin#10272 - bitcoin#10395 (base58 only) - bitcoin#10963 - bitcoin#11303 (first commit) - bitcoin#11562 - bitcoin#11646 - bitcoin#11654 Current output of `src/bench/bench_pivx`: ``` #Benchmark,count,min(ns),max(ns),average(ns),min_cycles,max_cycles,average_cycles Base58CheckEncode,131072,7697,8065,7785,20015,20971,20242 Base58Decode,294912,3305,3537,3454,8595,9198,8981 Base58Encode,180224,5498,6020,5767,14297,15652,14994 CCheckQueueSpeed,320,3159960,3535173,3352787,8216030,9191602,8717388 CCheckQueueSpeedPrevectorJob,96,9184484,11410840,10823070,23880046,29668680,28140445 FastRandom_1bit,320,3143690,4838162,3199156,8173726,12579373,8317941 FastRandom_32bit,60,17097612,17923669,17367440,44454504,46602306,45156079 PrevectorClear,3072,334741,366618,346731,870340,953224,901516 PrevectorDestructor,2816,344233,368912,357281,895022,959187,928948 RIPEMD160,288,3404503,3693917,3577774,8851850,9604334,9302363 SHA1,384,2718128,2891558,2802513,7067238,7518184,7286652 SHA256,176,6133760,6580005,6239866,15948035,17108376,16223916 SHA512,240,4251468,4358706,4313463,11054006,11332826,11215186 Sleep100ms,10,100221470,100302411,100239073,260580075,260790726,260625870 ``` NOTE: Not all the tests have been pulled yet (as we might not have the code being tested, or it would require rewrites to work with our different code base), but the framework is updated to December 2017. ACKs for top commit: Fuzzbawls: ACK 3f3edde Tree-SHA512: c283311a9accf6d2feeb93b185afa08589ebef3f18b6e86980dbc3647b9845f75ac9ecce2f1b08738d25ceac36596a2c89d41e4dbf3b463502aa695611aa1f8e
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Jun 24, 2021
1789e46 Force explicit double -> int conversion for CFeeRate constructor (Matt Corallo) 53a6590 Make float <-> int casts explicit outside of test, qt, CFeeRate (Matt Corallo) 0b1b914 Remove countMaskInv caching in bench framework (Matt Corallo) Pull request description: This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion. Tree-SHA512: 66087b08e5dfca67506da54ae057c2f9d86184415e8fa4fa0199e38839e06a3ce96c836fcb7593b7d960065f5240c594ff3a0cfa14333ac528421f5aeac835c9
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Jun 30, 2021
bf64c3c Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos) 04f78ab Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos) fd849e1 Change AcceptToMemoryPool function signature (Alex Morcos) Pull request description: First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior. Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](bitcoin#9602 (comment)) Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in bitcoin#11303. Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c fix 11309 Signed-off-by: Pasta <pasta@dashboost.org> fix & Signed-off-by: Pasta <pasta@dashboost.org>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes an issue where estimatesmartfee which matches at the min relay fee will return 999 sat/byte instead of 1000 sat/byte due to a float rounding issue. I went ahead and made all float <-> int conversion outside of test/qt explicit (test only had one or two more, Qt had quite a few, including many in the Qt headers themselves) and added overloads to CFeeRate to force callers to do an explicit round themselves. Easy to test with -Wfloat-conversion.