From 8dee76a1b84ebac84b2aa5fe20d8c2864ca262f6 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 29 May 2024 19:57:47 +0200 Subject: [PATCH] peer: decorate delivery addr w/ internal key In this commit, we move to add the internal key to the delivery addr. This way, we give the aux chan closer the extra information it may need to properly augment the normal co-op close process. --- lntest/mock/walletcontroller.go | 5 +- lnwallet/chancloser/chancloser.go | 25 +++++- lnwallet/chancloser/chancloser_test.go | 10 ++- peer/brontide.go | 108 +++++++++++++++++++++++-- 4 files changed, 131 insertions(+), 17 deletions(-) diff --git a/lntest/mock/walletcontroller.go b/lntest/mock/walletcontroller.go index cb7e387010..fd4a433486 100644 --- a/lntest/mock/walletcontroller.go +++ b/lntest/mock/walletcontroller.go @@ -78,9 +78,8 @@ func (w *WalletController) ConfirmedBalance(int32, string) (btcutil.Amount, func (w *WalletController) NewAddress(lnwallet.AddressType, bool, string) (btcutil.Address, error) { - addr, _ := btcutil.NewAddressPubKey( - w.RootKey.PubKey().SerializeCompressed(), &chaincfg.MainNetParams, - ) + pkh := btcutil.Hash160(w.RootKey.PubKey().SerializeCompressed()) + addr, _ := btcutil.NewAddressPubKeyHash(pkh, &chaincfg.MainNetParams) return addr, nil } diff --git a/lnwallet/chancloser/chancloser.go b/lnwallet/chancloser/chancloser.go index 5c73e56fdf..662a38aa76 100644 --- a/lnwallet/chancloser/chancloser.go +++ b/lnwallet/chancloser/chancloser.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" + "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcec/v2/schnorr/musig2" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" @@ -106,6 +107,18 @@ const ( defaultMaxFeeMultiplier = 3 ) +// DeliveryAddrWithKey wraps a normal delivery addr, but also includes the +// internal key for the delivery addr if known. +type DeliveryAddrWithKey struct { + // DeliveryAddress is the raw, serialized pkScript of the delivery + // address. + lnwire.DeliveryAddress + + // InternalKey is the Taproot internal key of the delivery address, if + // the address is a P2TR output. + InternalKey fn.Option[btcec.PublicKey] +} + // ChanCloseCfg holds all the items that a ChanCloser requires to carry out its // duties. type ChanCloseCfg struct { @@ -208,6 +221,10 @@ type ChanCloser struct { // funds to. localDeliveryScript []byte + // localInternalKey is the local delivery address Taproot internal key, + // if the local delivery script is a P2TR output. + localInternalKey fn.Option[btcec.PublicKey] + // remoteDeliveryScript is the script that we'll send the remote party's // settled channel funds to. remoteDeliveryScript []byte @@ -284,7 +301,7 @@ func (d *SimpleCoopFeeEstimator) EstimateFee(chanType channeldb.ChannelType, // NewChanCloser creates a new instance of the channel closure given the passed // configuration, and delivery+fee preference. The final argument should only // be populated iff, we're the initiator of this closing request. -func NewChanCloser(cfg ChanCloseCfg, deliveryScript []byte, +func NewChanCloser(cfg ChanCloseCfg, deliveryScript DeliveryAddrWithKey, idealFeePerKw chainfee.SatPerKWeight, negotiationHeight uint32, closeReq *htlcswitch.ChanClose, closer lntypes.ChannelParty) *ChanCloser { @@ -299,7 +316,8 @@ func NewChanCloser(cfg ChanCloseCfg, deliveryScript []byte, cfg: cfg, negotiationHeight: negotiationHeight, idealFeeRate: idealFeePerKw, - localDeliveryScript: deliveryScript, + localInternalKey: deliveryScript.InternalKey, + localDeliveryScript: deliveryScript.DeliveryAddress, priorFeeOffers: make( map[btcutil.Amount]*lnwire.ClosingSigned, ), @@ -362,6 +380,7 @@ func (c *ChanCloser) initChanShutdown() (*lnwire.Shutdown, error) { ChanPoint: c.chanPoint, ShortChanID: c.cfg.Channel.ShortChanID(), Initiator: c.cfg.Channel.IsInitiator(), + InternalKey: c.localInternalKey, CommitBlob: c.cfg.Channel.LocalCommitmentBlob(), FundingBlob: c.cfg.Channel.FundingBlob(), }) @@ -966,6 +985,7 @@ func (c *ChanCloser) ReceiveClosingSigned( //nolint:funlen req := AuxShutdownReq{ ChanPoint: c.chanPoint, ShortChanID: c.cfg.Channel.ShortChanID(), + InternalKey: c.localInternalKey, Initiator: channel.IsInitiator(), CommitBlob: channel.LocalCommitmentBlob(), FundingBlob: channel.FundingBlob(), @@ -1042,6 +1062,7 @@ func (c *ChanCloser) auxCloseOutputs( req := AuxShutdownReq{ ChanPoint: c.chanPoint, ShortChanID: c.cfg.Channel.ShortChanID(), + InternalKey: c.localInternalKey, Initiator: c.cfg.Channel.IsInitiator(), CommitBlob: c.cfg.Channel.LocalCommitmentBlob(), FundingBlob: c.cfg.Channel.FundingBlob(), diff --git a/lnwallet/chancloser/chancloser_test.go b/lnwallet/chancloser/chancloser_test.go index b32ce5ead9..28709fd5f8 100644 --- a/lnwallet/chancloser/chancloser_test.go +++ b/lnwallet/chancloser/chancloser_test.go @@ -361,7 +361,8 @@ func TestMaxFeeClamp(t *testing.T) { Channel: &channel, MaxFee: test.inputMaxFee, FeeEstimator: &SimpleCoopFeeEstimator{}, - }, nil, test.idealFee, 0, nil, lntypes.Remote, + }, DeliveryAddrWithKey{}, test.idealFee, 0, nil, + lntypes.Remote, ) // We'll call initFeeBaseline early here since we need @@ -402,7 +403,8 @@ func TestMaxFeeBailOut(t *testing.T) { MaxFee: idealFee * 2, } chanCloser := NewChanCloser( - closeCfg, nil, idealFee, 0, nil, lntypes.Remote, + closeCfg, DeliveryAddrWithKey{}, idealFee, 0, + nil, lntypes.Remote, ) // We'll now force the channel state into the @@ -526,7 +528,7 @@ func TestTaprootFastClose(t *testing.T) { DisableChannel: func(wire.OutPoint) error { return nil }, - }, nil, idealFee, 0, nil, lntypes.Local, + }, DeliveryAddrWithKey{}, idealFee, 0, nil, lntypes.Local, ) aliceCloser.initFeeBaseline() @@ -543,7 +545,7 @@ func TestTaprootFastClose(t *testing.T) { DisableChannel: func(wire.OutPoint) error { return nil }, - }, nil, idealFee, 0, nil, lntypes.Remote, + }, DeliveryAddrWithKey{}, idealFee, 0, nil, lntypes.Remote, ) bobCloser.initFeeBaseline() diff --git a/peer/brontide.go b/peer/brontide.go index ffd435a933..5cda8c5525 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -13,11 +13,13 @@ import ( "time" "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/connmgr" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btclog" + "github.com/btcsuite/btcwallet/waddrmgr" "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/buffer" "github.com/lightningnetwork/lnd/build" @@ -885,6 +887,73 @@ func (p *Brontide) QuitSignal() <-chan struct{} { return p.quit } +// internalKeyForAddr returns the internal key associated with a taproot +// address. +func internalKeyForAddr(wallet *lnwallet.LightningWallet, + deliveryScript []byte) (fn.Option[btcec.PublicKey], error) { + + none := fn.None[btcec.PublicKey]() + + pkScript, err := txscript.ParsePkScript(deliveryScript) + if err != nil { + return none, err + } + addr, err := pkScript.Address(&wallet.Cfg.NetParams) + if err != nil { + return none, err + } + + // If it's not a taproot address, we don't require to know the internal + // key in the first place. So we don't return an error here, but also no + // internal key. + _, isTaproot := addr.(*btcutil.AddressTaproot) + if !isTaproot { + return none, nil + } + + walletAddr, err := wallet.AddressInfo(addr) + if err != nil { + return none, err + } + + // If the address isn't known to the wallet, we can't determine the + // internal key. + if walletAddr == nil { + return none, nil + } + + pubKeyAddr, ok := walletAddr.(waddrmgr.ManagedPubKeyAddress) + if !ok { + return none, fmt.Errorf("expected pubkey addr, got %T", + pubKeyAddr) + } + + return fn.Some(*pubKeyAddr.PubKey()), nil +} + +// addrWithInternalKey takes a delivery script, then attempts to supplement it +// with information related to the internal key for the addr, but only if it's +// a taproot addr. +func (p *Brontide) addrWithInternalKey( + deliveryScript []byte) fn.Result[chancloser.DeliveryAddrWithKey] { + + // TODO(roasbeef): not compatible with external shutdown addr? + // Currently, custom channels cannot be created with external upfront + // shutdown addresses, so this shouldn't be an issue. We only require + // the internal key for taproot addresses to be able to provide a non + // inclusion proof of any scripts. + + internalKey, err := internalKeyForAddr(p.cfg.Wallet, deliveryScript) + if err != nil { + return fn.Err[chancloser.DeliveryAddrWithKey](err) + } + + return fn.Ok(chancloser.DeliveryAddrWithKey{ + DeliveryAddress: deliveryScript, + InternalKey: internalKey, + }) +} + // loadActiveChannels creates indexes within the peer for tracking all active // channels returned by the database. It returns a slice of channel reestablish // messages that should be sent to the peer immediately, in case we have borked @@ -1125,9 +1194,16 @@ func (p *Brontide) loadActiveChannels(chans []*channeldb.OpenChannel) ( return } + addr, err := p.addrWithInternalKey( + info.DeliveryScript.Val, + ).Unpack() + if err != nil { + shutdownInfoErr = fmt.Errorf("unable to make "+ + "delivery addr: %w", err) + return + } chanCloser, err := p.createChanCloser( - lnChan, info.DeliveryScript.Val, feePerKw, nil, - info.Closer(), + lnChan, addr, feePerKw, nil, info.Closer(), ) if err != nil { shutdownInfoErr = fmt.Errorf("unable to "+ @@ -2886,8 +2962,12 @@ func (p *Brontide) fetchActiveChanCloser(chanID lnwire.ChannelID) ( return nil, fmt.Errorf("unable to estimate fee") } + addr, err := p.addrWithInternalKey(deliveryScript).Unpack() + if err != nil { + return nil, fmt.Errorf("unable to parse addr: %w", err) + } chanCloser, err = p.createChanCloser( - channel, deliveryScript, feePerKw, nil, lntypes.Remote, + channel, addr, feePerKw, nil, lntypes.Remote, ) if err != nil { p.log.Errorf("unable to create chan closer: %v", err) @@ -3129,8 +3209,12 @@ func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( closingParty = lntypes.Local } + addr, err := p.addrWithInternalKey(deliveryScript).Unpack() + if err != nil { + return nil, fmt.Errorf("unable to parse addr: %w", err) + } chanCloser, err := p.createChanCloser( - lnChan, deliveryScript, feePerKw, nil, closingParty, + lnChan, addr, feePerKw, nil, closingParty, ) if err != nil { p.log.Errorf("unable to create chan closer: %v", err) @@ -3157,8 +3241,8 @@ func (p *Brontide) restartCoopClose(lnChan *lnwallet.LightningChannel) ( // createChanCloser constructs a ChanCloser from the passed parameters and is // used to de-duplicate code. func (p *Brontide) createChanCloser(channel *lnwallet.LightningChannel, - deliveryScript lnwire.DeliveryAddress, fee chainfee.SatPerKWeight, - req *htlcswitch.ChanClose, + deliveryScript chancloser.DeliveryAddrWithKey, + fee chainfee.SatPerKWeight, req *htlcswitch.ChanClose, closer lntypes.ChannelParty) (*chancloser.ChanCloser, error) { _, startingHeight, err := p.cfg.ChainIO.GetBestBlock() @@ -3179,6 +3263,7 @@ func (p *Brontide) createChanCloser(channel *lnwallet.LightningChannel, MusigSession: NewMusigChanCloser(channel), FeeEstimator: &chancloser.SimpleCoopFeeEstimator{}, BroadcastTx: p.cfg.Wallet.PublishTransaction, + AuxCloser: p.cfg.AuxChanCloser, DisableChannel: func(op wire.OutPoint) error { return p.cfg.ChanStatusMgr.RequestDisable( op, false, @@ -3250,10 +3335,17 @@ func (p *Brontide) handleLocalCloseReq(req *htlcswitch.ChanClose) { return } } + addr, err := p.addrWithInternalKey(deliveryScript).Unpack() + if err != nil { + err = fmt.Errorf("unable to parse addr for channel "+ + "%v: %w", req.ChanPoint, err) + p.log.Errorf(err.Error()) + req.Err <- err + return + } chanCloser, err := p.createChanCloser( - channel, deliveryScript, req.TargetFeePerKw, req, - lntypes.Local, + channel, addr, req.TargetFeePerKw, req, lntypes.Local, ) if err != nil { p.log.Errorf(err.Error())