-
Couldn't load subscription status.
- Fork 2.2k
Aux Closer: Move coop-close aux finalization to chain watcher #10289
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ import ( | |
| "github.com/lightningnetwork/lnd/lnutils" | ||
| "github.com/lightningnetwork/lnd/lnwallet" | ||
| "github.com/lightningnetwork/lnd/lnwire" | ||
| "github.com/lightningnetwork/lnd/tlv" | ||
| ) | ||
|
|
||
| const ( | ||
|
|
@@ -37,6 +38,77 @@ const ( | |
| maxCommitPointPollTimeout = 10 * time.Minute | ||
| ) | ||
|
|
||
| // CloseOutput represents an output that should be included in the close | ||
| // transaction. | ||
| type CloseOutput struct { | ||
| // Amt is the amount of the output. | ||
| Amt btcutil.Amount | ||
|
|
||
| // DustLimit is the dust limit for the local node. | ||
| DustLimit btcutil.Amount | ||
|
|
||
| // PkScript is the script that should be used to pay to the output. | ||
| PkScript []byte | ||
|
|
||
| // ShutdownRecords is the set of custom records that may result in | ||
| // extra close outputs being added. | ||
| ShutdownRecords lnwire.CustomRecords | ||
| } | ||
|
|
||
| // AuxShutdownReq is used to request a set of extra custom records to include | ||
| // in the shutdown message. | ||
| type AuxShutdownReq struct { | ||
| // ChanPoint is the channel point of the channel that is being shut | ||
| // down. | ||
| ChanPoint wire.OutPoint | ||
|
|
||
| // ShortChanID is the short channel ID of the channel that is being | ||
| // closed. | ||
| ShortChanID lnwire.ShortChannelID | ||
|
|
||
| // Initiator is true if the local node is the initiator of the channel. | ||
| Initiator bool | ||
|
|
||
| // InternalKey is the internal key for the shutdown addr. This will | ||
| // only be set for taproot shutdown addrs. | ||
| InternalKey fn.Option[btcec.PublicKey] | ||
|
|
||
| // CommitBlob is the blob that was included in the last commitment. | ||
| CommitBlob fn.Option[tlv.Blob] | ||
|
|
||
| // FundingBlob is the blob that was included in the funding state. | ||
| FundingBlob fn.Option[tlv.Blob] | ||
| } | ||
|
|
||
| // AuxCloseDesc is used to describe the channel close that is being performed. | ||
| type AuxCloseDesc struct { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These types are duplicated from |
||
| AuxShutdownReq | ||
|
|
||
| // CloseFee is the closing fee to be paid for this state. | ||
| CloseFee btcutil.Amount | ||
|
|
||
| // CommitFee is the fee that was paid for the last commitment. | ||
| CommitFee btcutil.Amount | ||
|
|
||
| // LocalCloseOutput is the output that the local node should be paid | ||
| // to. This is None if the local party will not have an output on the | ||
| // co-op close transaction. | ||
| LocalCloseOutput fn.Option[CloseOutput] | ||
|
|
||
| // RemoteCloseOutput is the output that the remote node should be paid | ||
| // to. This will be None if the remote party will not have an output on | ||
| // the co-op close transaction. | ||
| RemoteCloseOutput fn.Option[CloseOutput] | ||
| } | ||
|
|
||
| // AuxChanCloser is used to allow an external caller to finalize a cooperative | ||
| // channel close. | ||
| type AuxChanCloser interface { | ||
| // FinalizeClose is called after the close transaction has been agreed | ||
| // upon and confirmed. | ||
| FinalizeClose(desc AuxCloseDesc, closeTx *wire.MsgTx) error | ||
| } | ||
|
|
||
| // LocalUnilateralCloseInfo encapsulates all the information we need to act on | ||
| // a local force close that gets confirmed. | ||
| type LocalUnilateralCloseInfo struct { | ||
|
|
@@ -229,6 +301,9 @@ type chainWatcherConfig struct { | |
|
|
||
| // auxResolver is used to supplement contract resolution. | ||
| auxResolver fn.Option[lnwallet.AuxContractResolver] | ||
|
|
||
| // auxCloser is used to finalize cooperative closes. | ||
| auxCloser fn.Option[AuxChanCloser] | ||
| } | ||
|
|
||
| // chainWatcher is a system that's assigned to every active channel. The duty | ||
|
|
@@ -986,6 +1061,76 @@ func (c *chainWatcher) toSelfAmount(tx *wire.MsgTx) btcutil.Amount { | |
| return btcutil.Amount(fn.Sum(vals)) | ||
| } | ||
|
|
||
| // finalizeCoopClose calls the aux closer to finalize a cooperative close | ||
| // transaction that has been confirmed on-chain. | ||
| func (c *chainWatcher) finalizeCoopClose(aux AuxChanCloser, | ||
| closeTx *wire.MsgTx) error { | ||
|
|
||
| chanState := c.cfg.chanState | ||
|
|
||
| // Get the shutdown info to extract the local delivery script. | ||
| shutdown, err := chanState.ShutdownInfo() | ||
| if err != nil { | ||
| return fmt.Errorf("get shutdown info: %w", err) | ||
| } | ||
|
|
||
| // TODO(roasbeef): Extract the internal key if this is a taproot | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this a todo specifically for roasbeef and not for @GeorgeTsagk? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This todo was meant for @GeorgeTsagk , but realized now it is not needed at all. Will update comments accordingly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? |
||
| // channel. For now, we leave it as None since we don't persist it | ||
| // separately from the delivery script. | ||
| var internalKey fn.Option[btcec.PublicKey] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the existing call site, this is always properly set. |
||
|
|
||
| // Build the AuxShutdownReq. | ||
| req := AuxShutdownReq{ | ||
| ChanPoint: chanState.FundingOutpoint, | ||
| ShortChanID: chanState.ShortChanID(), | ||
| InternalKey: internalKey, | ||
| Initiator: chanState.IsInitiator, | ||
| CommitBlob: chanState.LocalCommitment.CustomBlob, | ||
| FundingBlob: chanState.CustomBlob, | ||
| } | ||
|
|
||
| // Extract close outputs from the transaction. We need to identify | ||
| // which outputs belong to local vs remote parties. | ||
| var localCloseOutput, remoteCloseOutput fn.Option[CloseOutput] | ||
|
|
||
| // Get the delivery scripts for both parties. | ||
| var localDeliveryScript lnwire.DeliveryAddress | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment says both parties, but lonly extracts this script for a single party. |
||
| shutdown.WhenSome(func(s channeldb.ShutdownInfo) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC this must be set at this point. We can encode this invariant by returning an error if it isn't available. |
||
| localDeliveryScript = s.DeliveryScript.Val | ||
| }) | ||
|
|
||
| // Scan through the close transaction outputs to identify local and | ||
| // remote outputs. | ||
| for _, out := range closeTx.TxOut { | ||
| if len(localDeliveryScript) > 0 && | ||
| slices.Equal(out.PkScript, localDeliveryScript) { | ||
|
|
||
| localCloseOutput = fn.Some(CloseOutput{ | ||
| Amt: btcutil.Amount(out.Value), | ||
| PkScript: out.PkScript, | ||
| DustLimit: chanState.LocalChanCfg.DustLimit, | ||
| }) | ||
| } else { | ||
| // This must be the remote output. | ||
| remoteCloseOutput = fn.Some(CloseOutput{ | ||
| Amt: btcutil.Amount(out.Value), | ||
| PkScript: out.PkScript, | ||
| DustLimit: chanState.RemoteChanCfg.DustLimit, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Build the AuxCloseDesc. | ||
| desc := AuxCloseDesc{ | ||
| AuxShutdownReq: req, | ||
| LocalCloseOutput: localCloseOutput, | ||
| RemoteCloseOutput: remoteCloseOutput, | ||
| } | ||
Roasbeef marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Call FinalizeClose on the aux closer. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Superflous comments here and above. |
||
| return aux.FinalizeClose(desc, closeTx) | ||
| } | ||
|
|
||
| // dispatchCooperativeClose processed a detect cooperative channel closure. | ||
| // We'll use the spending transaction to locate our output within the | ||
| // transaction, then clean up the database state. We'll also dispatch a | ||
|
|
@@ -1036,6 +1181,17 @@ func (c *chainWatcher) dispatchCooperativeClose(commitSpend *chainntnfs.SpendDet | |
| ChannelCloseSummary: closeSummary, | ||
| } | ||
|
|
||
| // If we have an aux closer, finalize the cooperative close now that | ||
| // it's confirmed. | ||
| err = fn.MapOptionZ( | ||
| c.cfg.auxCloser, func(aux AuxChanCloser) error { | ||
| return c.finalizeCoopClose(aux, broadcastTx) | ||
| }, | ||
| ) | ||
| if err != nil { | ||
| return fmt.Errorf("finalize coop close: %w", err) | ||
| } | ||
|
|
||
| // With the event processed, we'll now notify all subscribers of the | ||
| // event. | ||
| c.Lock() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -970,33 +970,6 @@ func (c *ChanCloser) ReceiveClosingSigned( //nolint:funlen | |
| } | ||
| c.closingTx = closeTx | ||
|
|
||
| // If there's an aux chan closer, then we'll finalize with it | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: is there value in leaving this code? This PR introduces more robust handling of the aux closer finalization by moving the hook call site from In case our peer stays online this would lead to a faster finalization which in turn maybe is a better user experience. |
||
| // before we write to disk. | ||
| err = fn.MapOptionZ( | ||
| c.cfg.AuxCloser, func(aux AuxChanCloser) error { | ||
| channel := c.cfg.Channel | ||
| //nolint:ll | ||
| req := AuxShutdownReq{ | ||
| ChanPoint: c.chanPoint, | ||
| ShortChanID: c.cfg.Channel.ShortChanID(), | ||
| InternalKey: c.localInternalKey, | ||
| Initiator: channel.IsInitiator(), | ||
| CommitBlob: channel.LocalCommitmentBlob(), | ||
| FundingBlob: channel.FundingBlob(), | ||
| } | ||
| desc := AuxCloseDesc{ | ||
| AuxShutdownReq: req, | ||
| LocalCloseOutput: c.localCloseOutput, | ||
| RemoteCloseOutput: c.remoteCloseOutput, | ||
| } | ||
|
|
||
| return aux.FinalizeClose(desc, closeTx) | ||
| }, | ||
| ) | ||
| if err != nil { | ||
| return noClosing, err | ||
| } | ||
|
|
||
| // Before publishing the closing tx, we persist it to the | ||
| // database, such that it can be republished if something goes | ||
| // wrong. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,7 @@ import ( | |
| "github.com/lightningnetwork/lnd/lnutils" | ||
| "github.com/lightningnetwork/lnd/lnwallet" | ||
| "github.com/lightningnetwork/lnd/lnwallet/chainfee" | ||
| chcl "github.com/lightningnetwork/lnd/lnwallet/chancloser" | ||
| "github.com/lightningnetwork/lnd/lnwallet/chanfunding" | ||
| "github.com/lightningnetwork/lnd/lnwallet/rpcwallet" | ||
| "github.com/lightningnetwork/lnd/lnwire" | ||
|
|
@@ -220,6 +221,53 @@ type peerSlotStatus struct { | |
| state peerAccessStatus | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe consider moving to a dedicated file like contractcourt/aux_closer_adapter.go for better organization. |
||
| // auxCloserAdapter adapts a chancloser.AuxChanCloser to the | ||
| // contractcourt.AuxChanCloser interface. | ||
| type auxCloserAdapter struct { | ||
| chcl.AuxChanCloser | ||
| } | ||
|
|
||
| // FinalizeClose adapts the chancloser types to contractcourt types. | ||
| func (a auxCloserAdapter) FinalizeClose(desc contractcourt.AuxCloseDesc, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this adapter at all? Why can't we pass the existing interface as defined in the |
||
| closeTx *wire.MsgTx) error { | ||
|
|
||
| // Convert contractcourt types to chancloser types. | ||
| chanCloserDesc := chcl.AuxCloseDesc{ | ||
| AuxShutdownReq: chcl.AuxShutdownReq{ | ||
| ChanPoint: desc.ChanPoint, | ||
| ShortChanID: desc.ShortChanID, | ||
| Initiator: desc.Initiator, | ||
| InternalKey: desc.InternalKey, | ||
| CommitBlob: desc.CommitBlob, | ||
| FundingBlob: desc.FundingBlob, | ||
| }, | ||
| CloseFee: desc.CloseFee, | ||
| CommitFee: desc.CommitFee, | ||
| LocalCloseOutput: fn.MapOption( | ||
| func(o contractcourt.CloseOutput) chcl.CloseOutput { | ||
| return chcl.CloseOutput{ | ||
| Amt: o.Amt, | ||
| DustLimit: o.DustLimit, | ||
| PkScript: o.PkScript, | ||
| ShutdownRecords: o.ShutdownRecords, | ||
| } | ||
| }, | ||
| )(desc.LocalCloseOutput), | ||
| RemoteCloseOutput: fn.MapOption( | ||
| func(o contractcourt.CloseOutput) chcl.CloseOutput { | ||
| return chcl.CloseOutput{ | ||
| Amt: o.Amt, | ||
| DustLimit: o.DustLimit, | ||
| PkScript: o.PkScript, | ||
| ShutdownRecords: o.ShutdownRecords, | ||
| } | ||
| }, | ||
| )(desc.RemoteCloseOutput), | ||
| } | ||
|
|
||
| return a.AuxChanCloser.FinalizeClose(chanCloserDesc, closeTx) | ||
| } | ||
|
|
||
| // server is the main server of the Lightning Network Daemon. The server houses | ||
| // global state pertaining to the wallet, database, and the rpcserver. | ||
| // Additionally, the server is also used as a central messaging bus to interact | ||
|
|
@@ -1375,6 +1423,11 @@ func newServer(ctx context.Context, cfg *Config, listenAddrs []net.Addr, | |
| AuxLeafStore: implCfg.AuxLeafStore, | ||
| AuxSigner: implCfg.AuxSigner, | ||
| AuxResolver: implCfg.AuxContractResolver, | ||
| AuxCloser: fn.MapOption( | ||
| func(c chcl.AuxChanCloser) contractcourt.AuxChanCloser { | ||
| return auxCloserAdapter{AuxChanCloser: c} | ||
| }, | ||
| )(implCfg.AuxChanCloser), | ||
| }, dbs.ChanStateDB) | ||
|
|
||
| // Select the configuration and funding parameters for Bitcoin. | ||
|
|
||
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.
Consider refactoring these types into a shared package. e.g:
lnwallet/chantypesThis would avoid the duplication and the circular dependency. 🦅 🦅 🪨💨