RBF Coop Close using sat/kw instead of sat/vb#10425
RBF Coop Close using sat/kw instead of sat/vb#10425MPins wants to merge 4 commits intolightningnetwork:masterfrom
Conversation
Summary of ChangesHello @MPins, 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 refactors the RBF cooperative close process to align all fee rate calculations and representations with the Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request refactors the RBF cooperative close logic to use sat/kw instead of sat/vb for fee rates. The changes are consistent across the codebase, including protocol definitions, RPC server logic, and tests. My feedback includes a suggestion to deprecate the old fee_per_vbyte field in the protobuf definition for API clarity and an improvement to a comment to better align with the style guide by explaining the intention of the code.
Abdulkbk
left a comment
There was a problem hiding this comment.
Had a first pass through, this looks good. Also confirmed locally that the changes did not break existing tests.
|
Thank you for taking the time to review this PR @Abdulkbk |
3a6a379 to
80815c9
Compare
|
could you resolve the conflict so I can do a final review?. |
|
Hello @Abdulkbk rebase done 👍 |
Yes, that makes sense. Updated.
Done
Thank you again! |
Change rbf coop close flow to use sat/kw insted of sat/vb, and update the unit tests accordingly.
b21997c to
9640b0c
Compare
|
@MPins, remember to re-request review from reviewers when ready |
Changed the RBF Coop Close to make it use internally sat/kw instead of sat/vb.
This PR must be merged before PR#10067 as it provides the necessary groundwork.