-
Notifications
You must be signed in to change notification settings - Fork 137
Orphan UTXO filter and limit #1905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @darioAnongba, 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 significantly enhances the robustness and usability of orphan UTXO management. It addresses potential transaction signing failures by filtering out improperly stored UTXOs and prevents transaction bloat by limiting the number of swept UTXOs. Furthermore, it streamlines the process by enabling automatic orphan UTXO sweeping by default, ensuring a cleaner and more efficient wallet operation. 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
|
17421c3 to
02e110a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces several improvements to orphan UTXO sweeping, including filtering out UTXOs with missing signing information, limiting the number of UTXOs fetched, and enabling sweeping by default. The changes are logical and well-implemented, and the new functionality is covered by tests. I have a few suggestions to improve the new tests and align the logging with the repository's style guide.
02e110a to
c82b4dd
Compare
Pull Request Test Coverage Report for Build 20035562459Details
💛 - Coveralls |
c82b4dd to
6edfe6a
Compare
jtobin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Nit: I would avoid long lines in commit messages, e.g. in 4539224.)
LGTM. 👍 👍
GeorgeTsagk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cACK, approach looks good, will take a look at the test in a while before approving
GeorgeTsagk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
have some suggestions to extend the tests, otherwise looking good 🟢
Added filtering to exclude orphan UTXOs with missing signing information (KeyFamily=0 and KeyIndex=0). These UTXOs were created in prior versions that didn't store this information, causing LND to fail when signing.
6edfe6a to
7102e04
Compare
Filter out orphan UTXOs with missing signing information: In prior versions, we didn't store
KeyFamilyandKeyIndexfor orphan UTXOs. When both values are 0, LND fails to sign the transaction. This PR filters out these UTXOs to prevent signing failures.Limit the number of orphan UTXOs fetched: Previously, there was no limit on how many orphan UTXOs could be included in a transaction. If too many were fetched, the transaction could become too large. This PR adds a limit of 20 orphan UTXOs per transaction (
MaxOrphanUTXOs).Enable
wallet.sweep-orphan-utxosby default: Changed the default fromfalsetotrue. This ensures tombstone and burn outputs are automatically swept during onchain transactions.