Avoid serving stale fees #27555

1. 
   TheBlueMatt commented at 3:26 pm on May 2, 2023: contributor

   After conversation on irc it appears there’s some cases where Bitcoin core will serve feerate information even though it loaded a fairly old fee estimates file and is still syncing the chain. This is pretty dangerous for lightning nodes, at least on legacy channels or pre-package-relay.

   The immediate improvement would be to store fee estimates to disk once an hour or so to reduce the chance of having an old file. From there this case could probably be detected and refuse to serve estimates until we sync.

2. maflcko added the label TX fees and policy on May 2, 2023
3. ismaelsadeeq referenced this in commit 55f19449a6 on May 5, 2023
4. ismaelsadeeq referenced this in commit ceb134d8f7 on May 5, 2023
5. ismaelsadeeq referenced this in commit 2414830467 on May 5, 2023
6. ismaelsadeeq referenced this in commit 125357ac10 on May 5, 2023
7. 
   instagibbs commented at 0:34 am on May 8, 2023: member

   looks like one of my nodes had a similar issue with a clean shutdown and restart

   https://twitter.com/CoreFeeHelper/status/1655286815291588611 after restart, then an hour or so later: https://twitter.com/CoreFeeHelper/status/1655301916593668097

   back up to expected min fee

8. 
   instagibbs commented at 3:00 pm on May 8, 2023: member

   Here’s my guess on how this happens and explaining the behavior I’m seeing after reproducing it. It seems to be 100% reproducible on nodes, and repeatable after a few minutes of warmup when things fix themselves.

   1. On node restart, there is no persistence of mempoolminfee
   2. While mempool is importing, node receives transactions well below the old mempoolminfee, enters into mempool
   3. mempool finishes loading(with some failures), resulting in no trimming on startup
   4. mempoolminfee seems to be feeding into estimatesmartfee as a floor, so if node restarts, it starts naively giving out min incremental feerate
   5. In estimatesmartfee, fee_estimator.estimateSmartFee is apparently returning a non-0 CFeeRate value, but which is lower than 1 sat/vbyte, which then allows mempoolminfee to take over the value.
   6. Over time, if the mempool is actually busy, enough transactions enter the mempool to start trimming, stepping from min incremental relay all the way to the “actual” value once enough has been trimmed.

   So you have this gap on restart where longer term estimates are way out of whack, and we’re being sent INVs we will promptly evict anyways. It self-resolves in a few minutes it seems in the happy path.

   I think #22722 has ended up hiding the particular estimatefee issue, since prior my guess is it would have simply not returned anything if was below min relay.

   Two mitigations:

   1. persist mempoolminfee across restarts to at least tell user a plausible value to stay in the mempool for a bit
   2. investigate why estimateSmartFee is returning bogus values; require more certainty to return a result? … or get package relay deployed :P

   edit:

   #19699 (comment) 3-year ago me asking the same question as (2). oops.

   edit2:

   Looks like the estimator is giving something “real” from a bucket: 1000.0000000000035, so basically during these situations we’re relying on mempoolminfee to save us, and it’s failing after restart

9. glozow closed this on Jun 20, 2023

   ---

10. sidhujag referenced this in commit 5d0a209488 on Jun 21, 2023
11. BlackcoinDev referenced this in commit f80db62b2d on Feb 5, 2024
12. bitcoin locked this on Jun 19, 2024

Contributors
TheBlueMatt instagibbs

Labels
TX fees and policy