-
Notifications
You must be signed in to change notification settings - Fork 2.2k
release: create v0.19.3-rc1 branch #10134
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
In this commit, we make the gossip filter semaphore capacity configurable through a new FilterConcurrency field. This change allows node operators to tune the number of concurrent gossip filter applications based on their node's resources and network position. The previous hard-coded limit of 5 concurrent filter applications could become a bottleneck when multiple peers attempt to synchronize simultaneously. By making this value configurable via the new gossip.filter-concurrency option, operators can increase this limit for better performance on well-resourced nodes or maintain conservative values on resource-constrained systems. We keep the default value at 5 to maintain backward compatibility and avoid unexpected resource usage increases for existing deployments. The sample configuration file is updated to document this new option.
In this commit, we introduce an asynchronous processing queue for GossipTimestampRange messages in the GossipSyncer. This change addresses a critical issue where the gossiper could block indefinitely when processing timestamp range messages during periods of high load. Previously, when a peer sent a GossipTimestampRange message, the gossiper would synchronously call ApplyGossipFilter, which could block on semaphore acquisition, database queries, and rate limiting. This synchronous processing created a bottleneck where the entire peer message processing pipeline would stall, potentially causing timeouts and disconnections. The new design adds a timestampRangeQueue channel with a capacity of 1 message and a dedicated goroutine for processing these messages asynchronously. This follows the established pattern used for other message types in the syncer. When the queue is full, we drop messages and log a warning rather than blocking indefinitely, providing graceful degradation under extreme load conditions.
In this commit, we complete the integration of the asynchronous timestamp range queue by modifying ProcessRemoteAnnouncement to use the new queuing mechanism instead of calling ApplyGossipFilter synchronously. This change ensures that when a peer sends a GossipTimestampRange message, it is queued for asynchronous processing rather than blocking the gossiper's main message processing loop. The modification prevents the peer's readHandler from blocking on potentially slow gossip filter operations, maintaining connection stability during periods of high synchronization activity. If the queue is full when attempting to enqueue a message, we log a warning but return success to prevent peer disconnection. This design choice prioritizes connection stability over guaranteed delivery of every gossip filter request, which is acceptable since peers can always resend timestamp range messages if needed.
In this commit, we complete the integration of the configurable gossip filter concurrency by wiring the new FilterConcurrency configuration through all layers of the application. The changes connect the gossip.filter-concurrency configuration option from the command-line interface through the server initialization code to the gossiper and sync manager. This ensures that operators can actually use the new configuration option to tune their node's concurrent gossip filter processing capacity based on their specific requirements and available resources.
In this commit, we add detailed documentation to help node operators understand and configure the gossip rate limiting system effectively. The new guide addresses a critical knowledge gap that has led to misconfigured nodes experiencing synchronization failures. The documentation covers the token bucket algorithm used for rate limiting, providing clear formulas and examples for calculating appropriate values based on node size and network position. We include specific recommendations ranging from 50 KB/s for small nodes to 1 MB/s for large routing nodes, with detailed calculations showing how these values are derived. The guide explains the relationship between rate limiting and other configuration options like num-restricted-slots and the new filter-concurrency setting. We provide troubleshooting steps for common issues like slow initial sync and peer disconnections, along with debug commands and log patterns to identify problems. Configuration examples are provided for conservative, balanced, and performance-oriented setups, giving operators concrete starting points they can adapt to their specific needs. The documentation emphasizes the importance of not setting rate limits too low, warning that values below 50 KB/s can cause synchronization to fail entirely.
In this commit, we add a new atomic bool to only permit a single gossip backlog goroutine per peer. If we get a new reuqest that needs a backlog while we're still processing the other, then we'll drop that request.
Thus we can mark channels as resolved in an async way to avoid deadlock.
We now make sure to sweep each anchor input in its own sweeping tx, if economically feasible.
We now make sure when removing inputs identified by the exclusive group ID, we only remove the other one, not the one that invoked the removal.
This makes sure there is no general issue with running the pay-join FundPsbt test case in a remote signer setup.
This demonstrates that the "imported" account behaves differently for a wallet that's watch-only vs. normal. The testTaprootImportTapscriptFullKeyFundPsbt test case succeeds in a normal run but fails in a remote-signing setup. For some reason, an imported tapscript address ends up in the "default" account when remote-signing but in the "imported" account for a normal wallet.
Pull reviewers statsStats of the last 30 days for lnd:
|
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.
Summary of Changes
Hello @guggero, 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 is a release candidate branch for v0.19.3-rc1
, bundling several critical bug fixes and performance improvements. My focus was on enhancing the Lightning Network Daemon's (LND) gossip subsystem by significantly increasing default rate limits and introducing configurable concurrency for gossip filter applications. I also addressed a deadlock in the contract resolvers and refined the sweep logic for anchor outputs to prevent transaction pinning. Furthermore, a bug in btcwallet
affecting Tapscript address imports in watch-only setups has been resolved, and a payment retry loop issue for custom HTLCs has been fixed.
Highlights
- Gossip Subsystem Performance Enhancements: The default gossip message rate and burst limits have been increased tenfold (to 1MB/s and 2MB respectively), and a new configuration option for gossip filter concurrency has been introduced. This aims to significantly improve network synchronization performance and prevent resource exhaustion, especially for well-connected nodes.
- Deadlock and Sweep Logic Fixes: Addressed a deadlock in contract resolvers related to channel resolution by refactoring the notification mechanism to be asynchronous. Additionally, anchor output sweeping logic has been improved to prevent transaction pinning by ensuring these outputs are swept independently, even if uneconomical, which is crucial for channel force closures.
- Tapscript Address Import and PSBT Funding Fix: Resolved a bug in
btcwallet
that prevented the proper import of Tapscript addresses in watch-only setups, enhancing the capabilities for remote signing and PSBT funding with these address types. New integration tests were added to cover these scenarios. - Payment Retry Loop Fix for Custom HTLCs: Corrected an issue in the payment lifecycle where payments involving custom HTLCs could get stuck in an infinite retry loop due to incorrect policy validation. The system now correctly skips policy validation for these specific HTLCs.
- Release Candidate Preparation: This pull request updates the version to
v0.19.3-rc1
and includes comprehensive release notes detailing all integrated bug fixes and functional enhancements, preparing for the upcoming release.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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 or fill out our survey to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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 prepares for the v0.19.3-rc1 release by updating the version and incorporating several bug fixes and improvements. The changes include fixing deadlocks in the contract resolver and gossip syncer, improving anchor sweeping logic, and addressing an issue with HTLC amount validation. The code changes are well-structured and include relevant tests. I have one minor suggestion regarding a line length violation in one of the log messages.
Fuzzy match report:
|
Failed CI steps are the unit test flakes fixed in #10052 (which isn't part of this side branch). |
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 🌽
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.
Thank you, LGTM
Includes the following PRs:
ResolveContract
#10108