Repository navigation
fees: never quote below mempoolminfee, and hold the top rate in a fee spike - #1251
niteshbalusu11 wants to merge 6 commits into
Conversation
parse_conf_target cast the JSON number to u32 before the 1..=1008 check, so 4294967297 became target 1 and estimatesmartfee answered it. Core v31.1 reads conf_target with getInt<int>: a value outside the int range, or a fraction, is -1 "JSON integer out of range", and an int outside 1..=1008 (negatives included) is -8. The parser now does the same for estimatesmartfee and estimaterawfee. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
min_rate_for_capacity returned None when even 1000 sat/vB, the top of the flow ladder, did not fit the next N blocks. The refresh passed that None to flow_for_depth, which read it as "no flow data": the flow rate became the frontier, or min relay for a near depth whose pool was thinner than N blocks. The monotone pass then capped every later target at that rate. Three blocks of 2000 sat/vB arrivals on a one-transaction pool quoted 0.1 sat/vB for the next block. The search now answers the ladder top when nothing fits, so a near target quotes at least 1000 sat/vB in that case. projected is None only when the flow meter lock is poisoned, and the refresh then publishes the cold quote alone (alpha 0), so the min-relay fallback for a live pool without a projected rate is deleted with its parameters. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
estimatesmartfee raised its answer to mempoolminfee in the RPC layer. Electrum blockchain.estimatefee, Esplora /fee-estimates, and estimaterawfee read the hub's fee table, which was floored at the static 0.1 sat/vB only. With -minrelaytxfee raised, or a full pool's rolling minimum above the estimate, those surfaces quoted a rate this node's own mempool rejects. FeeSnapshot::rate_btc_per_kb now takes a floor, and both hub accessors pass mempool_min_fee_sat_kvb, read under a short read lock when they answer. All four surfaces share that one floored path, and the RPC-only clamp in smart_fee_json is deleted. The floor is read per request, not stored in the table, so a raised floor applies even while another request's refresh runs, and it does not depend on every floor change calling publish_fee_floor: a package rollback that restores a conflict into a near-full pool raises the floor without a publish. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
warmup_blend dropped a cold quote at alpha 1 when the warm side had no rate, though its rustdoc and mempool-fee-estimation.md say a missing warm side leaves the cold quote. published_depth_rate cannot reach that case (history defines both sides), so the arm now matches the contract and folds into one. Comments that disagreed with the code are corrected: target 0, not 0-2, maps to the default depth; CONFIDENCE_FAR is flow capacity confidence (history reads its own 95% quantile); confirm-memory holds each entry's own feerate, not a package rate. The duplicated estimatesmartfee doc line is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![rearden-grok[bot]](https://avatars.githubusercontent.com/u/730881?s=60&v=4){kind=small}
There was a problem hiding this comment.
REQUEST_CHANGES
min_rate_for_capacity quotes the ladder top after that rate has failed the capacity check, and wallets will pay it. See the comment on fee_est.rs.
Nit: parse_conf_target treats a whole-valued JSON float as an int32. Core rejects it.
estimaterawfee picking up the mempoolminfee floor matches the documented 10-minute product, the same answer as estimatesmartfee. Core's estimaterawfee is a different, unfloored, per-horizon result; COMPAT.md already does not claim that shape. The new tx_relay tests insert history rows under a mutex and reuse the shared 3-block maturity pad. They do not sleep or mine a chain, so they stay inside the 2s per-test budget.
Core v31.1 answers estimatesmartfee 6.0 and 1e1 with -1 "JSON integer out of range". rbitcoin-rpc builds serde_json with arbitrary_precision, so Number::as_i64 parses the raw token and rejects both already. The chain-ops journey now sends those tokens so that stays pinned. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The previous commit on this branch answered the 1000 sat/vB ladder top when no candidate rate fit the next N blocks. That rate had just failed the capacity check, so it is a price the projection says will not clear, and about 150,000 vB admitted above it kept targets 1-5 (and, with no history, every held target) at 1000 sat/vB while the admit EMA decayed. min_rate_for_capacity returns None again when nothing fits. The min-relay fallback in flow_for_depth stays deleted, so that None no longer becomes 0.1 sat/vB: the depth falls back to the frontier, then to block history, and has no estimate when neither has a rate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Note 🤖 Claude Opus 5.5 responding on behalf of niteshbalusu11 @reardencode ready for another look. rearden-grok's review is addressed: 612e27f leaves a depth with no fitting ladder rate to the frontier and block history instead of quoting 1000 sat/vB, and 608067b pins |
There was a problem hiding this comment.
APPROVE
Both earlier findings are fixed. min_rate_for_capacity returns None when every rung fails the capacity check, and flow_for_depth no longer substitutes min relay. With no frontier, a near target uses block history, and with no history the quote stays negative. arrivals_above_the_ladder_leave_near_targets_to_history checks both: targets 1 and 2 are below zero before any history, then exactly 2,000 sat/kvB after the history rows, which is neither min relay nor the 1000 sat/vB top. A defined rate still passes through rate_btc_per_kb, so Electrum, Esplora, and both fee RPCs keep the mempoolminfee floor. A missing rate stays a missing rate.
serde_json 1.0.154 with arbitrary_precision stores the token and as_i64 is str::parse::<i64>, which rejects 6.0 and 1e1. The RPC server parses the body with that same serde_json (from_slice in rpc_post). The new test builds those tokens with from_str and sends them through dispatch for estimatesmartfee and estimaterawfee, asserting -1 and JSON integer out of range.
The new history loop is the same in-memory record_fee_sample insert the other fee tests use. No sleeps, polling, or extra chain.
Electrum
blockchain.estimatefeeand Esplora/fee-estimatescould quote a feerate this node rejects. Onlyestimatesmartfeewas raised tomempoolminfee; the others stopped at 0.1 sat/vB. And when arrivals priced above 1000 sat/vB overfilled the next blocks, a pool holding less than a block quoted 0.1 sat/vB for the next block, and every longer target was capped at that.estimatesmartfee,estimaterawfee, Electrum, and Esplora read one hub path that floors each rate at the livemempoolminfeewhen it answers. The RPC-only clamp is deleted. Reading the floor per request also covers a request that lands while another refresh runs, and a package rollback that raises the floor without republishing it.flow_for_depthno longer turns that into 0.1 sat/vB; it is deleted. That depth falls back to the frontier, then block history, and has no estimate when neither has a rate.conf_target: parsed as Core's int32.4294967297used to be cut to target 1. A value outside int32, or a fraction, is now-1JSON integer out of range, and-1is-8, as Core v31.1 answers on regtest.warmup_blendkeeps the cold quote when the warm side is missing, as its rustdoc says, and comments that disagreed with the code are fixed.Proof: each fix started from a test that failed first. A 2000 sat/vB arrival burst on a thin pool quoted 0.1 sat/vB for the next block and now quotes block history, or no estimate without history. Raising min relay to 5000 sat/kvB left Electrum/Esplora at 2000, including with a refresh in flight, and now gives 5000. A package rollback into a near-full pool quoted 149 against a 200 floor and now gives 200.
4294967297answered as target 1 and is now an error;6.0and1e1stay errors. Local fmt, clippy, deny, ast-grep, unwrap-ratchet, OS smoke, and the workspace suite pass, except two timing tests that pass alone:integration_multinode::mocktime_generate_keeps_ponging_peer(it also fails under load on master, 1 of 3rbitcoin-testruns on5cc40ff05) andpeer::disconnect_after_tip_sync_clears_far_side_within_5s.Found in the same review and left for later:
FeeFlowMeter::note_admitdecays the bucket it updates twice, a snapshot read changes the next EMA sample, and CPFP members are metered at their own feerates rather than the chunk rate.Reviewed with sol-loop: 4 rounds with gpt-6.1-sol on high. Sol verdict: approved.
Created with Claude Opus 5.5 in Claude Code (T3 Code).
🤖 Generated with Claude Code