Use new features of LND 0.21#1156
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 updates the project to leverage new features from LND 0.21, primarily focusing on the integration of the production TAPROOT commitment type. The changes span across the CLI, migration logic, and internal fee/weight estimation modules to ensure compatibility with the new commitment type. Additionally, the PR includes significant cleanup of deprecated API usages and updates to linting configurations to maintain high code quality standards. Highlights
New Features🧠 You can now enable Memory (public preview) 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. 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 the 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 counterproductive. 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for the production TAPROOT commitment type alongside SIMPLE_TAPROOT across CLI commands, channel opening, fee estimation, and withdrawals. Additionally, it bumps the lndclient dependency, optimizes cost migration by omitting hops in payment queries, cleans up deprecated ioutil usage, and updates bbolt error handling. Feedback on the changes points out a potential data race in the mock lightning client when concurrently appending to ListPaymentsRequests during test execution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| req lndclient.ListPaymentsRequest) (*lndclient.ListPaymentsResponse, | ||
| error) { | ||
|
|
||
| h.lnd.ListPaymentsRequests = append(h.lnd.ListPaymentsRequests, req) |
There was a problem hiding this comment.
Appending to h.lnd.ListPaymentsRequests concurrently from multiple goroutines during test execution can lead to a data race, as LndMockServices does not appear to be thread-safe. Consider synchronizing access to this slice, for example by adding a mutex to LndMockServices or using a thread-safe mock implementation.
starius
force-pushed
the
lnd-21-improvements
branch
2 times, most recently
from
68f3569 to
9b66fff
Compare
There was a problem hiding this comment.
Pull request overview
Updates Loop to take advantage of newer LND/lndclient capabilities (notably ListPayments omit_hops and production TAPROOT commitment types), while tightening linting and expanding regression coverage around static address open-channel flows.
Changes:
- Bump
github.com/lightninglabs/lndclienttov0.21.0-2and useListPaymentsRequest.OmitHops=trueto reduce cost-migration payload/processing. - Treat both
SIMPLE_TAPROOTand productionTAPROOTcommitment types as P2TR for static address fee/weight estimation; expose both channel types in the CLI and docs. - Re-enable broader
staticcheckdeprecation checks and update/annotate remaining deprecated API usages; modernize tempdir/file helpers in tests.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
test/lnd_services_mock.go |
Records ListPayments requests and exposes a snapshot helper for assertions. |
test/lightning_client_mock.go |
Captures ListPayments requests and returns a defensive copy of payment slices. |
staticaddr/withdraw/manager.go |
Treats TAPROOT commitment type as P2TR in withdrawal weight estimation. |
staticaddr/withdraw/funding_values_test.go |
Adds parity coverage for TAPROOT commitment type. |
staticaddr/staticutil/utils.go |
Treats TAPROOT commitment type as P2TR in fee estimation. |
staticaddr/staticutil/utils_test.go |
Adds deposit-selection test coverage for TAPROOT commitment type. |
staticaddr/openchannel/manager.go |
Accepts TAPROOT commitment type; adjusts deposit selection comments/flow. |
staticaddr/openchannel/manager_test.go |
Adds resolveCommitmentType test for TAPROOT. |
staticaddr/deposit/manager_test.go |
Documents/annotates deprecated RPC method to satisfy staticcheck. |
staticaddr/address/manager_test.go |
Documents/annotates deprecated RPC method to satisfy staticcheck. |
loopdb/store.go |
Switches timeout matching/wrapping to bbolt/errors.ErrTimeout. |
loopdb/store_test.go |
Updates timeout assertions for bbolt/errors.ErrTimeout; uses t.TempDir(). |
loopdb/migration_04_updates_test.go |
Uses t.TempDir() for legacy-db restore tests. |
loopd/swapclient_server.go |
Adds targeted staticcheck suppressions for deprecated RPC fields kept for compatibility. |
loopd/daemon.go |
Uses bbolt/errors.ErrTimeout and fixes errors.Is argument order for cancellations. |
liquidity/parameters.go |
Preserves legacy budget start field reading with explicit staticcheck suppression. |
go.mod |
Bumps lndclient dependency to v0.21.0-2. |
go.sum |
Updates checksums for the lndclient bump. |
docs/loop.md |
Documents both simple-taproot and taproot for --channel_type. |
docs/loop.1 |
Updates man page to include simple-taproot and taproot. |
cost_migration.go |
Requests ListPayments with OmitHops=true during cost migration. |
cost_migration_test.go |
Asserts cost migration uses OmitHops=true. |
cmd/loop/testdata/sessions/static-openchannel/07_loop-static-openchannel-taproot-success.json |
Adds recorded CLI session for channel_type=taproot success. |
cmd/loop/testdata/sessions/static-openchannel/01_loop-static-openchannel-help.json |
Updates recorded help output for the expanded --channel_type values. |
cmd/loop/openchannel.go |
Adds taproot channel type (production enum) and renames legacy to simple-taproot. |
cmd/loop/main.go |
Replaces deprecated ioutil.ReadFile with os.ReadFile. |
cmd/loop/loopin.go |
Prints swap ID via IdBytes for consistency with other commands. |
.golangci.yml |
Re-enables broad staticcheck checks while excluding selected groups. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return nil, err | ||
| } | ||
|
|
||
| // If fundmax is set, all deposits remain selected for funding. |
There was a problem hiding this comment.
Fixed. The comment now looks like this:
// If a local funding amount is set, coin-select deposits to
// cover it. Otherwise fundmax uses all available deposits.
if req.LocalFundingAmount != 0 {
starius
force-pushed
the
lnd-21-improvements
branch
from
9b66fff to
af3dd46
Compare
hieblmi
left a comment
hieblmi
left a comment
There was a problem hiding this comment.
Thank you for the uplift. This LGTM!
| @@ -273,6 +273,7 @@ func TestSelectDeposits(t *testing.T) { | |||
|
|
|||
| anchors := lnrpc.CommitmentType_ANCHORS | |||
| taproot := lnrpc.CommitmentType_SIMPLE_TAPROOT | |||
There was a problem hiding this comment.
nit: call this simple_taproot
There was a problem hiding this comment.
Fixed. I removed the var block and just inlined lnrpc.CommitmentType_* directly in test cases.
|
|
||
| anchors := lnrpc.CommitmentType_ANCHORS | ||
| taproot := lnrpc.CommitmentType_SIMPLE_TAPROOT | ||
| taprootFinal := lnrpc.CommitmentType_TAPROOT |
There was a problem hiding this comment.
Fixed. I removed the var block and just inlined lnrpc.CommitmentType_* directly in test cases.
| return nil, fmt.Errorf("error selecting "+ | ||
| "deposits: %w", err) | ||
| } | ||
| } else { |
There was a problem hiding this comment.
LoopMinRequiredLndVersion needs another bump. The earlier bump to v0.18.4-beta is correct for the APIs listed in the comment, but this branch now also uses v0.21-era lnd APIs: lnrpc.ListPaymentsRequest.omit_hops and lnrpc.CommitmentType_TAPROOT.
LoopMinRequiredLndVersion = &verrpc.Version{
AppMajor: 0,
AppMinor: 18,
AppPatch: 4,
BuildTags: []string{
"signrpc", "walletrpc", "chainrpc", "invoicesrpc",
},
}
Use the tagged lndclient release that exposes the ListPayments request fields needed by Loop optimizations.
LND v0.21 exposes CommitmentType_TAPROOT as the production taproot channel commitment type, while SIMPLE_TAPROOT remains a legacy taproot enum. Static address channel opens previously rejected TAPROOT and only classified SIMPLE_TAPROOT as a taproot output for fee and weight estimates. Accept TAPROOT in the static address open-channel validator and keep accepting SIMPLE_TAPROOT for compatibility. Treat both taproot commitment enums as P2TR outputs for deposit-selection and withdrawal fee estimates. Callers using the production enum then get the same weight accounting as the legacy taproot enum. This does not change the CLI mapping for user-facing channel_type=taproot. It only makes the static address path compatible with callers that already send LND production taproot commitment type.
The cost cleanup migration pages through LND payments only to build a payment-hash to fee map. It does not inspect HTLC attempts, routes, or per-hop data; pagination still uses the top-level index offsets returned by ListPayments. Setting OmitHops is safe for this migration because LND only strips hop-level route data from HTLC attempts, while preserving the top-level payment fields the migration reads: hash, fee, and response offsets. This reduces response size and query cost for nodes with many or large MPP payments without changing the calculated swap costs. The migration test records the mocked ListPayments requests and asserts that OmitHops is set.
Enable staticcheck's SA1019 check in golangci-lint so deprecated identifiers are caught in CI. Replace deprecated standard library and bbolt APIs with their current equivalents. Keep intentional compatibility reads and writes of deprecated Loop RPC fields behind narrow nolint annotations, because older clients and persisted liquidity parameters still depend on those fields.
starius
force-pushed
the
lnd-21-improvements
branch
from
af3dd46 to
5d2f55c
Compare
LND v0.21 added the production TAPROOT commitment type while SIMPLE_TAPROOT remains available as the legacy enum. The static open-channel CLI previously used "taproot" for SIMPLE_TAPROOT. Keep both choices available by renaming that legacy spelling to "simple-taproot" and mapping "taproot" to TAPROOT. This makes the CLI spelling match the channel type it requests while still leaving an explicit path for users that need SIMPLE_TAPROOT.
starius
force-pushed
the
lnd-21-improvements
branch
from
5d2f55c to
682d454
Compare
I changed it to produce the following error if attempting to open new taproot channel type using LND below 0.21: |
Summary
lndclienttov0.21.0-2to getomit_hopsfields support.ListPaymentswithomit_hops, since the migration only needs payment hashes and fees.TAPROOTcommitment type in static address openchannel handling and treat bothSIMPLE_TAPROOTandTAPROOTas P2TR for fee and withdrawal weight estimation.simple-taprootrequests the legacySIMPLE_TAPROOTenum, whiletaprootrequests the productionTAPROOTenum.channel_type=taproot.golangci-lintand update or annotate the remaining deprecated API use sites so future deprecations are caught earlier.Validation
Pull Request Checklist
release_notes.mdif your PR contains major features, breaking changes or bugfixes