From 2a7a34ae10e318a198a33a66b2b8249164140391 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 18 Nov 2020 22:45:35 +0100 Subject: [PATCH] contractcourt+lnwallet: use state num instead of commit height when outdated local state This commit fixes a bug that would cause us to not sweep our local output in case we force closed, then lost state or attempted recovery. The reason being that we would use or local commit height when deriving our scripts, which would be incorrect. Instead we use the extracted state number to derive the correct scripts, allowing us to sweep the output. Allthough being an unlikely scenario, we would leave money on chain in this case without any warning (since we would just end up with an empty delay script) and forget about the spend. --- contractcourt/chain_watcher.go | 15 +++++++++------ lnwallet/channel.go | 33 +++++++++++++++++++-------------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/contractcourt/chain_watcher.go b/contractcourt/chain_watcher.go index 51d9c6f2..97aa7852 100644 --- a/contractcourt/chain_watcher.go +++ b/contractcourt/chain_watcher.go @@ -411,7 +411,7 @@ func (c *chainWatcher) handleUnknownLocalState( // though we won't be able to sweep HTLCs. chainSet.commitSet.ConfCommitKey = &LocalHtlcSet if err := c.dispatchLocalForceClose( - commitSpend, chainSet.localCommit, chainSet.commitSet, + commitSpend, broadcastStateNum, chainSet.commitSet, ); err != nil { return false, fmt.Errorf("unable to handle local"+ "close for chan_point=%v: %v", @@ -564,7 +564,9 @@ func (c *chainWatcher) closeObserver(spendNtfn *chainntnfs.SpendEvent) { // We'll go on to check whether it could be our own commitment // that was published and know is confirmed. - ok, err = c.handleKnownLocalState(commitSpend, chainSet) + ok, err = c.handleKnownLocalState( + commitSpend, broadcastStateNum, chainSet, + ) if err != nil { log.Errorf("Unable to handle known local state: %v", err) @@ -657,7 +659,8 @@ func (c *chainWatcher) closeObserver(spendNtfn *chainntnfs.SpendEvent) { // is known to us (the current state). If so we will act on this state using // the passed chainSet. If this is not a known local state, false is returned. func (c *chainWatcher) handleKnownLocalState( - commitSpend *chainntnfs.SpendDetail, chainSet *chainSet) (bool, error) { + commitSpend *chainntnfs.SpendDetail, broadcastStateNum uint64, + chainSet *chainSet) (bool, error) { // If the channel is recovered, we won't have a local commit to check // against, so immediately return. @@ -675,7 +678,7 @@ func (c *chainWatcher) handleKnownLocalState( chainSet.commitSet.ConfCommitKey = &LocalHtlcSet if err := c.dispatchLocalForceClose( - commitSpend, chainSet.localCommit, chainSet.commitSet, + commitSpend, broadcastStateNum, chainSet.commitSet, ); err != nil { return false, fmt.Errorf("unable to handle local"+ "close for chan_point=%v: %v", @@ -945,14 +948,14 @@ func (c *chainWatcher) dispatchCooperativeClose(commitSpend *chainntnfs.SpendDet // dispatchLocalForceClose processes a unilateral close by us being confirmed. func (c *chainWatcher) dispatchLocalForceClose( commitSpend *chainntnfs.SpendDetail, - localCommit channeldb.ChannelCommitment, commitSet CommitSet) error { + stateNum uint64, commitSet CommitSet) error { log.Infof("Local unilateral close of ChannelPoint(%v) "+ "detected", c.cfg.chanState.FundingOutpoint) forceClose, err := lnwallet.NewLocalForceCloseSummary( c.cfg.chanState, c.cfg.signer, - commitSpend.SpendingTx, localCommit, + commitSpend.SpendingTx, stateNum, ) if err != nil { return err diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 090f8f00..4ab00e28 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -6038,7 +6038,7 @@ func (lc *LightningChannel) ForceClose() (*LocalForceCloseSummary, error) { localCommitment := lc.channelState.LocalCommitment summary, err := NewLocalForceCloseSummary( lc.channelState, lc.Signer, commitTx, - localCommitment, + localCommitment.CommitHeight, ) if err != nil { return nil, err @@ -6054,8 +6054,8 @@ func (lc *LightningChannel) ForceClose() (*LocalForceCloseSummary, error) { // NewLocalForceCloseSummary generates a LocalForceCloseSummary from the given // channel state. The passed commitTx must be a fully signed commitment // transaction corresponding to localCommit. -func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Signer, - commitTx *wire.MsgTx, localCommit channeldb.ChannelCommitment) ( +func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, + signer input.Signer, commitTx *wire.MsgTx, stateNum uint64) ( *LocalForceCloseSummary, error) { // Re-derive the original pkScript for to-self output within the @@ -6063,9 +6063,11 @@ func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Si // output in the commitment transaction and potentially for creating // the sign descriptor. csvTimeout := uint32(chanState.LocalChanCfg.CsvDelay) - revocation, err := chanState.RevocationProducer.AtIndex( - localCommit.CommitHeight, - ) + + // We use the passed state num to derive our scripts, since in case + // this is after recovery, our latest channels state might not be up to + // date. + revocation, err := chanState.RevocationProducer.AtIndex(stateNum) if err != nil { return nil, err } @@ -6090,8 +6092,8 @@ func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Si // We'll return the details of this output to the caller so they can // sweep it once it's mature. var ( - delayIndex uint32 - delayScript []byte + delayIndex uint32 + delayOut *wire.TxOut ) for i, txOut := range commitTx.TxOut { if !bytes.Equal(payToUsScriptHash, txOut.PkScript) { @@ -6099,7 +6101,7 @@ func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Si } delayIndex = uint32(i) - delayScript = txOut.PkScript + delayOut = txOut break } @@ -6110,8 +6112,8 @@ func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Si // If the output is non-existent (dust), have the sign descriptor be // nil. var commitResolution *CommitOutputResolution - if len(delayScript) != 0 { - localBalance := localCommit.LocalBalance + if delayOut != nil { + localBalance := delayOut.Value commitResolution = &CommitOutputResolution{ SelfOutPoint: wire.OutPoint{ Hash: commitTx.TxHash(), @@ -6122,8 +6124,8 @@ func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Si SingleTweak: keyRing.LocalCommitKeyTweak, WitnessScript: selfScript, Output: &wire.TxOut{ - PkScript: delayScript, - Value: int64(localBalance.ToSatoshis()), + PkScript: delayOut.PkScript, + Value: localBalance, }, HashType: txscript.SigHashAll, }, @@ -6133,8 +6135,11 @@ func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Si // Once the delay output has been found (if it exists), then we'll also // need to create a series of sign descriptors for any lingering - // outgoing HTLC's that we'll need to claim as well. + // outgoing HTLC's that we'll need to claim as well. If this is after + // recovery there is not much we can do with HTLCs, so we'll always + // use what we have in our latest state when extracting resolutions. txHash := commitTx.TxHash() + localCommit := chanState.LocalCommitment htlcResolutions, err := extractHtlcResolutions( chainfee.SatPerKWeight(localCommit.FeePerKw), true, signer, localCommit.Htlcs, keyRing, &chanState.LocalChanCfg,