Tracking issue for discussion in #20391 (review)
Cleanup CFeeRate constructor (sat/vB vs BTC/kvB) #23129
issue maflcko opened this issue on September 29, 2021-
maflcko commented at 8:17 AM on September 29, 2021: member
-
-
maflcko commented at 8:55 AM on September 29, 2021: member
Yes, I think so. That pull only fixed the AmountFromValue bug.
Cleaning up the constructor would be more of a refactor.
- maflcko added the label Refactoring on Sep 29, 2021
-
-
willcl-ark commented at 7:41 AM on June 26, 2025: member
@jonatack @murchandamus is this issue effectively solved via previous changes + #24305
-
murchandamus commented at 10:02 PM on June 30, 2025: contributor
@willcl-ark: I don’t think this is resolved. The state in
src/policy/feerate.cppstill matches @maflcko’s comment from the linked PR that predates this issue by a day. I think #32750 is making some progress towards improvement, though. -
polespinasa commented at 1:39 AM on September 22, 2025: contributor
Is this still relevant after #32750 is merged?
-
maflcko commented at 5:58 PM on September 24, 2025: member
@polespinasa I don't think this is resolved. To explain it a bit better, looking at the code:
AmountFromValueis a function that returns aCAmount(i64 of satoshis). However, theCFeeRateconstructor that is called takes a feerate per kvB:So calling
CFeeRate{AmountFromValue("0.0000'0001")}makes sense to call the BTC/kvB constructor.However, calling
CFeeRate{AmountFromValue("0.001", 3)}feels like a layer and type violation to parse sat/vB.I guess if I am the only one bothered by it, the issue can be closed. The code behaves correctly and this is more in the style/refactor territory.
Maybe this can also be closed and transferred into #32093?
-
maflcko commented at 6:39 AM on September 25, 2025: member
Closing for now. Seems fine to just organically change this, the next time the code is touched.
- maflcko closed this on Sep 25, 2025
