Skip to content

staticaddr: channel funding with deposits#1040

Open
hieblmi wants to merge 17 commits intolightninglabs:masterfrom
hieblmi:static-open-channel
Open

staticaddr: channel funding with deposits#1040
hieblmi wants to merge 17 commits intolightninglabs:masterfrom
hieblmi:static-open-channel

Conversation

@hieblmi
Copy link
Collaborator

@hieblmi hieblmi commented Nov 11, 2025

This PR introduces a openchannel subcommand to static addresses.

It provides the same experience as lncli openchannel.
Exmples:

Open a channel with all available deposits:

loop static openchannel --fundmax --sat_per_vbyte XXX --private ...

Open a channel with specified local funding amount, coin-selected from available deposits under fee and dust considerations.

loop static openchannel --local_amt XXX --sat_per_vbyte YYY --private ...

Open a channel from two deposits AAA and BBB while taking their combined value(fundmax) as funding amount and considering fees and dust limit.

loop static openchannel --utxo AAA --utxo BBB --fundmax --sat_per_vbyte YYY --private ...

Open a channel from two deposits AAA and BBB while taking a specified amount(local_amt) as funding amount and considering fees and dust limit.

loop static openchannel --utxo AAA --utxo BBB --local_amt --sat_per_vbyte YYY --private ...

TODOs:

  • Integration tests

@gemini-code-assist
Copy link

Summary of Changes

Hello @hieblmi, 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 extends the functionality of static addresses by enabling them to be used as a source for funding Lightning channels. It introduces a new openchannel command that integrates a robust PSBT-based funding flow with lnd, offering flexible options for selecting deposits. This enhancement allows users to seamlessly convert their static address funds into channel liquidity, thereby increasing the utility and versatility of static addresses within the Lightning ecosystem.

Highlights

  • New openchannel subcommand: Introduced a new openchannel subcommand under loop static which allows users to fund Lightning channels directly from their static address deposits.
  • Flexible Channel Funding Options: The new command supports various funding options, including fundmax to use all available deposits, local_amt for a specified amount, and explicit UTXO selection, mirroring the experience of lncli openchannel.
  • PSBT-based Funding Flow: The channel funding process leverages a Partially Signed Bitcoin Transaction (PSBT) flow, enabling secure and collaborative transaction construction with lnd.
  • New Deposit States: Added new deposit states, opening_channel and channel_published, to track the lifecycle of deposits used for channel funding.
  • Code Refactoring and Reusability: Common utility functions related to PSBTs, Musig2 sessions, and outpoint handling have been refactored into a new staticutil package, improving code organization and reusability across static address features.
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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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. 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

  1. 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.

Copy link

@gemini-code-assist gemini-code-assist bot left a 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 a new openchannel subcommand for static addresses, allowing users to fund channels from their deposits. The implementation is comprehensive, covering the command-line interface, RPC definitions, daemon logic, and documentation.

My review focuses on the new cmd/loop/openchannel.go file, where I've identified a few issues related to context handling, input validation, and error wrapping. These are important for ensuring the robustness and maintainability of the new command.

The refactoring to move utility functions into a staticutil package is a great improvement for code organization and reuse. The protocol changes to use PSBT for withdrawals are also a solid enhancement.

Overall, this is a well-structured PR that adds significant functionality. Addressing the feedback will further improve its quality.

@hieblmi hieblmi force-pushed the static-open-channel branch 4 times, most recently from f9b49dc to 9d37fca Compare November 12, 2025 10:24
@hieblmi hieblmi self-assigned this Nov 17, 2025
@hieblmi hieblmi force-pushed the static-open-channel branch 9 times, most recently from 94f45cc to 375cf47 Compare November 19, 2025 07:55
@hieblmi hieblmi force-pushed the static-open-channel branch 9 times, most recently from 2b9378a to 5b6631d Compare December 9, 2025 15:05
@hieblmi
Copy link
Collaborator Author

hieblmi commented Dec 12, 2025

@gemini-code-assist review

@hieblmi hieblmi requested a review from sputn1ck February 9, 2026 20:41
@hieblmi hieblmi force-pushed the static-open-channel branch 5 times, most recently from d2d2a88 to ffa9e9e Compare February 13, 2026 09:30
@lightninglabs-deploy
Copy link

@sputn1ck: review reminder
@hieblmi, remember to re-request review from reviewers when ready

Make our own type StaticAddressLoopInRequest which has a single field of type
lnrpc.OpenChannelRequest. Removed copy-pasted lnrpc types which are needed for
OpenChannelRequest.
Block-based deposit fetching from the internal lnd wallet was
susceptible to wallet syncing issues. Replace it with interval-based
polling. Reconciliation errors are now logged instead of being fatal,
improving resilience during transient failures.
@hieblmi hieblmi force-pushed the static-open-channel branch from ffa9e9e to 75a263b Compare February 18, 2026 08:41
@hieblmi
Copy link
Collaborator Author

hieblmi commented Feb 18, 2026

Hi @sputn1ck, I addressed some more findings, namely:

  • Added crash recovery for OpeningChannel deposits.
  • Upgraded permissions to swap:execute for both StaticOpenChannel and StaticAddressLoopIn.
  • Aligned channel-open fee/value calculation paths by threading commitment type into tx construction.
  • Hardcoded forwarded SpendUnconfirmed to false.
  • Added unit tests for openchannel recovery logic.

err.Error(), cancelErr,
) {

shimPending = false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data race in shimPending variable. It is accessed from multiple goroutines without synchronization.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a mutex to gate access.

Comment on lines 684 to 690
hash, err := chainhash.NewHash(
update.ChanPending.Txid,
)
if err != nil {
log.Infof("Error creating hash for channel "+
"open tx: %v", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should return error in this if. We use the hash variable below as if it is valid, but it could be nil and we just log the error and crash later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed.

Comment on lines +696 to +703
err = m.cfg.DepositManager.TransitionDeposits(
ctx, deposits, deposit.OnChannelPublished,
deposit.ChannelPublished,
)
if err != nil {
log.Errorf("error transitioning deposits to "+
"ChannelPublished: %v", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TransitionDeposits errors are logged but the function still returns success, leaving deposits stuck in OpeningChannel while the channel is actually pending. This should be returned to the caller or retried.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current approach is to just log the transition error and return the chanOutpoint since the transition error might be a recurring error that isn't solved without intervention.
So the approach is to reconcile these OpeningChannel states after a restart in recoverOpeningChannelDeposits.

Comment on lines 390 to 400
OpeningChannel: fsm.State{
Transitions: fsm.Transitions{
fsm.OnError: Deposited,
OnChannelPublished: ChannelPublished,
OnRecover: OpeningChannel,
},
Action: fsm.NoOpAction,
},
ChannelPublished: fsm.State{
Action: f.FinalizeDepositAction,
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two states lack OnExpiry transitions from them. OnExpiry may be delivered to each of them.

  1. OpeningChannel has Action: fsm.NoOpAction. This means it stops current FSM execution and unlocks the FSM mutex, letting potential SendEvent calls sneak in.
  2. FinalizeDepositAction also returns fsm.NoOp and triggers further things via a channel (f.finalizedDepositChan) so there is still a window when SendEvent can sneak in.

handleBlockNotification fires OnExpiry on every new block once the deposit is expired (staticaddr/deposit/fsm.go), regardless of state.

So there is a possibility that OnExpiry is delivered to each of these new states. We have to add state self-transitions to each of them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a great catch, thank you!
Fixed it.

log.Infof("Recovering %d deposits in OpeningChannel state",
len(openingDeposits))

utxos, err := m.cfg.WalletKit.ListUnspent(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the deposit is spent, but not confirmed? Shouldn't we still track it? Maybe it is evicted from mempool. Or even confirmed first and then reorged.

require.ErrorContains(t, err, tc.expectedErrSubstr)
})
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to add test coverage for edge cases:

  • reorg
  • daemon restart during channel opening
  • eviction from mempool
  • rejection from mempool
  • stream errors
  • PSBT finalize then stream abort
  • duplicate outpoints in request

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added!


// Run runs the open channel manager.
func (m *Manager) Run(ctx context.Context) error {
err := m.recoverOpeningChannelDeposits(ctx)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we recover deposits during daemon running as well, not only during startup time?

If the PSBT finalize succeeds but the stream fails before ChanPending, deposits remain in OpeningChannel until restart.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. We are checking if there's an error but the PSBT was finalized and sent, then we recover remaining deposits.

// selects a local channel amount in which case we coin-select deposits
// to cover for it. 3.) The user selects the fundmax flag, in which case
// we select all deposits to fund the channel.
if len(req.Outpoints) > 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if req.Outpoints has duplicates? This leads to fee miscalculation and invalid PSBT building (having the same input twice). I propose to return an error early if we detect duplicates in req.Outpoints and add test coverage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added logic to detect dupes and return in that case.

Comment on lines 263 to 265
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dead code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

/*
The transaction hash of the channel opening transaction.
*/
string channel_open_tx_hash = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also add the output index (always 0 in the current implementation)? Just in case we implement batch open or something in the future. The code which uses this thing shouldn't guess what is the index, it should take it from the API.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively change channel_open_tx_hash to channel_open_outpoint and put "txid:index" there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the second suggestion, added.

Protect the shimPending variable with a sync.Mutex since it is accessed
from multiple goroutines: the main loop goroutine and the server error
handling goroutine. Without synchronization this is a data race.
Return an error instead of just logging when chainhash.NewHash fails in
the ChanPending handler. The hash variable would be nil and crash on the
subsequent String() call.
…ublished

Both OpeningChannel and ChannelPublished lacked OnExpiry transitions.
handleBlockNotification fires OnExpiry on every new block once the
deposit is expired, regardless of the current state. Since both states
use NoOpAction or FinalizeDepositAction which release the FSM mutex
briefly, an OnExpiry SendEvent can sneak in. Add self-transitions so
the event is safely absorbed.
Duplicate outpoints in the request lead to fee miscalculation and an
invalid PSBT with the same input listed twice. Validate early and return
a clear error message.
Remove unreachable error check after filterNewDeposits which does not
return an error. The err variable was already handled from the
ListUnspent call above and could never be non-nil at this point.
If the PSBT finalize step succeeds but the stream fails before
ChanPending, deposits would remain stuck in OpeningChannel until the
next daemon restart. Run the recovery logic immediately so deposits are
resolved without requiring a restart.
Add tests for the following edge cases requested in review:

- Reorg: channel tx reorged, UTXOs reappear as unspent, deposits
  return to Deposited state.
- Daemon restart during channel opening: deposits in OpeningChannel
  recovered based on UTXO status (spent → ChannelPublished, unspent →
  Deposited).
- Mempool eviction: tx evicted, UTXOs unspent, deposits return to
  Deposited.
- Mempool rejection: tx never accepted, same recovery as eviction.
- Stream errors: lnd stream fails before PSBT finalize, error returned
  without errPsbtFinalized so deposits can be safely rolled back.
- PSBT finalize then stream abort: finalize succeeds but stream dies
  before ChanPending, error wrapped with errPsbtFinalized so caller
  triggers recovery instead of blind rollback.
- Duplicate outpoints: already covered by TestOpenChannelDuplicateOutpoints.
Change StaticOpenChannelResponse to return channel_open_outpoint in
"txid:index" format instead of just the tx hash. This includes the
output index so callers don't have to guess it, which is important for
potential future batch opens.
@hieblmi hieblmi force-pushed the static-open-channel branch from 75a263b to f345933 Compare February 19, 2026 10:42
@hieblmi hieblmi requested a review from starius February 19, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments