sweep: handle zero fee rate delta in LinearFeeFunction#10614
sweep: handle zero fee rate delta in LinearFeeFunction#10614darioAnongba wants to merge 1 commit intolightningnetwork:masterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue in the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🔴 PR Severity: CRITICAL
🔴 Critical (1 file)
🧪 Test-only (excluded from classification)
AnalysisThis PR modifies No severity bump was triggered: only 1 non-test file was changed (threshold: >20) and only 20 non-test lines were modified (threshold: >500). To override, add a |
There was a problem hiding this comment.
Code Review
This pull request gracefully handles the case where the fee rate delta is zero in NewLinearFeeFunction by creating a flat-rate fee function instead of returning an error. The change is logical, well-implemented, and includes corresponding updates to the tests. My main feedback is a suggestion to refactor the updated test case to use a table-driven approach, which is preferred by the repository's style guide and would improve maintainability.
1b0ed19 to
93ad93b
Compare
Fix
NewLinearFeeFunctionto handle the case where starting and ending fee rates are equal (delta=0) by creating a flat-rate fee function instead of returningErrZeroFeeRateDeltaProblem
When the fee estimator returns a rate higher than the budget-derived max fee rate, both the starting fee rate (estimated, clamped to max) and ending fee rate (budget max) become equal. This causes
delta = (end - start) / width = 0,andNewLinearFeeFunctionrejects the initialization withErrZeroFeeRateDelta.This means the sweep is never even attempted, even though the budget is sufficient to pay the fee at the max budget rate. The sweeper logs repeated errors like:
This can happen in practice when:
Fix
Instead of returning an error when
delta == 0 && width > 1, we now:The sweep will be attempted at the budget-max fee rate. If it passes mempool acceptance, it succeeds. If a fee bump is needed later,
Increment()immediately returnsErrMaxPosition, which the fee bumper handles by retrying with a different grouping strategy.