Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions contractcourt/chain_arbitrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ type ChainArbitratorConfig struct {
// AuxResolver is an optional interface that can be used to modify the
// way contracts are resolved.
AuxResolver fn.Option[lnwallet.AuxContractResolver]

// AuxCloser is an optional interface that can be used to finalize
// cooperative channel closes.
AuxCloser fn.Option[AuxChanCloser]
}

// ChainArbitrator is a sub-system that oversees the on-chain resolution of all
Expand Down Expand Up @@ -1138,6 +1142,7 @@ func (c *ChainArbitrator) WatchNewChannel(newChan *channeldb.OpenChannel) error
extractStateNumHint: lnwallet.GetStateNumHint,
auxLeafStore: c.cfg.AuxLeafStore,
auxResolver: c.cfg.AuxResolver,
auxCloser: c.cfg.AuxCloser,
},
)
if err != nil {
Expand Down Expand Up @@ -1315,6 +1320,7 @@ func (c *ChainArbitrator) loadOpenChannels() error {
extractStateNumHint: lnwallet.GetStateNumHint,
auxLeafStore: c.cfg.AuxLeafStore,
auxResolver: c.cfg.AuxResolver,
auxCloser: c.cfg.AuxCloser,
},
)
if err != nil {
Expand Down
156 changes: 156 additions & 0 deletions contractcourt/chain_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -37,6 +38,77 @@ const (
maxCommitPointPollTimeout = 10 * time.Minute
)

Copy link
Collaborator

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/chantypes

This would avoid the duplication and the circular dependency. 🦅 🦅 🪨💨

// 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 {
Copy link
Member

Choose a reason for hiding this comment

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

These types are duplicated from lnwallet/chancloser/aux_closer.go. Why do they need to be moved here?

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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this a todo specifically for roasbeef and not for @GeorgeTsagk?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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]
Copy link
Member

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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) {
Copy link
Member

Choose a reason for hiding this comment

The 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,
}

// Call FinalizeClose on the aux closer.
Copy link
Member

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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()
Expand Down
27 changes: 0 additions & 27 deletions lnwallet/chancloser/chancloser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 ReceiveClosingSigned to the chainwatcher. That also introduces a waiting time of ~10 minutes before the finalization is initiated. Couldn't we keep doing both?

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.
Expand Down
53 changes: 53 additions & 0 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -220,6 +221,53 @@ type peerSlotStatus struct {
state peerAccessStatus
}

Copy link
Collaborator

Choose a reason for hiding this comment

The 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. server.go is so long already.

// 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,
Copy link
Member

Choose a reason for hiding this comment

The 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 chanclose package into the chain watcher?

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
Expand Down Expand Up @@ -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.
Expand Down