From 8b068174822b1c1520003951d7faeda967205092 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 29 Apr 2018 15:40:59 -0700 Subject: [PATCH] lnwallet: modify NewUnilateralCloseSummary to be aware of pending remote commits In this commit, we modify the NewUnilateralCloseSummary to be able to distinguish between a unilateral closure using the lowest+highest commitment the remote party possesses. Before this commit, if the remote party broadcast their highest commitment, when they have a lower unrevoked commitment, then this function would fail to find the proper output, leaving funds on the chain. To fix this, it's now the duty of the caller to pass remotePendingCommit with the proper value. The caller should use the lowest unrevoked commitment, and the height hint of the broadcast commitment to discern if this is a pending commitment or not. --- lnwallet/channel.go | 30 ++++++++++++++++++++---------- lnwallet/channel_test.go | 18 +++++++++--------- lnwallet/transactions_test.go | 2 +- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 34039db0..2cf85c1e 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1234,10 +1234,10 @@ func compactLogs(ourLog, theirLog *updateLog, // // See the individual comments within the above methods for further details. type LightningChannel struct { - // signer is the main signer instances that will be responsible for + // Signer is the main signer instances that will be responsible for // signing any HTLC and commitment transaction generated by the state // machine. - signer Signer + Signer Signer // signDesc is the primary sign descriptor that is capable of signing // the commitment transaction that spends the multi-sig output. @@ -1350,7 +1350,7 @@ func NewLightningChannel(signer Signer, pCache PreimageCache, lc := &LightningChannel{ // TODO(roasbeef): tune num sig workers? sigPool: newSigPool(runtime.NumCPU(), signer), - signer: signer, + Signer: signer, pCache: pCache, currentHeight: localCommit.CommitHeight, remoteCommitChain: newCommitmentChain(remoteCommit.CommitHeight), @@ -2920,7 +2920,7 @@ func (lc *LightningChannel) SignNextCommitment() (lnwire.Sig, []lnwire.Sig, erro // the new commitment transaction while we're waiting for the rest of // the HTLC signatures to be processed. lc.signDesc.SigHashes = txscript.NewTxSigHashes(newCommitView.txn) - rawSig, err := lc.signer.SignOutputRaw(newCommitView.txn, lc.signDesc) + rawSig, err := lc.Signer.SignOutputRaw(newCommitView.txn, lc.signDesc) if err != nil { close(cancelChan) return sig, htlcSigs, err @@ -4601,7 +4601,7 @@ func (lc *LightningChannel) getSignedCommitTx() (*wire.MsgTx, error) { // With this, we then generate the full witness so the caller can // broadcast a fully signed transaction. lc.signDesc.SigHashes = txscript.NewTxSigHashes(commitTx) - ourSigRaw, err := lc.signer.SignOutputRaw(commitTx, lc.signDesc) + ourSigRaw, err := lc.Signer.SignOutputRaw(commitTx, lc.signDesc) if err != nil { return nil, err } @@ -4678,14 +4678,24 @@ type UnilateralCloseSummary struct { // NewUnilateralCloseSummary creates a new summary that provides the caller // with all the information required to claim all funds on chain in the event -// that the remote party broadcasts their commitment. +// that the remote party broadcasts their commitment. If the +// remotePendingCommit value is set to true, then we'll use the next (second) +// unrevoked commitment point to construct the summary. Otherwise, we assume +// that the remote party broadcast the lower of their two possible commits. func NewUnilateralCloseSummary(chanState *channeldb.OpenChannel, signer Signer, pCache PreimageCache, commitSpend *chainntnfs.SpendDetail, - remoteCommit channeldb.ChannelCommitment) (*UnilateralCloseSummary, error) { + remoteCommit channeldb.ChannelCommitment, + remotePendingCommit bool) (*UnilateralCloseSummary, error) { // First, we'll generate the commitment point and the revocation point - // so we can re-construct the HTLC state and also our payment key. + // so we can re-construct the HTLC state and also our payment key. If + // this is the pending remote commitment, then we'll use the second + // unrevoked commit point in order to properly reconstruct the scripts + // we need to locate. commitPoint := chanState.RemoteCurrentRevocation + if remotePendingCommit { + commitPoint = chanState.RemoteNextRevocation + } keyRing := deriveCommitmentKeys( commitPoint, false, &chanState.LocalChanCfg, &chanState.RemoteChanCfg, @@ -5258,7 +5268,7 @@ func (lc *LightningChannel) ForceClose() (*LocalForceCloseSummary, error) { localCommitment := lc.channelState.LocalCommitment summary, err := NewLocalForceCloseSummary(lc.channelState, - lc.signer, lc.pCache, commitTx, localCommitment) + lc.Signer, lc.pCache, commitTx, localCommitment) if err != nil { return nil, err } @@ -5427,7 +5437,7 @@ func (lc *LightningChannel) CreateCloseProposal(proposedFee btcutil.Amount, // using the generated txid to be notified once the closure transaction // has been confirmed. lc.signDesc.SigHashes = txscript.NewTxSigHashes(closeTx) - sig, err := lc.signer.SignOutputRaw(closeTx, lc.signDesc) + sig, err := lc.Signer.SignOutputRaw(closeTx, lc.signDesc) if err != nil { return nil, nil, 0, err } diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index e788532b..becdbfec 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -1443,13 +1443,13 @@ func TestStateUpdatePersistence(t *testing.T) { t.Fatalf("unable to fetch channel: %v", err) } aliceChannelNew, err := NewLightningChannel( - aliceChannel.signer, nil, aliceChannels[0], + aliceChannel.Signer, nil, aliceChannels[0], ) if err != nil { t.Fatalf("unable to create new channel: %v", err) } bobChannelNew, err := NewLightningChannel( - bobChannel.signer, nil, bobChannels[0], + bobChannel.Signer, nil, bobChannels[0], ) if err != nil { t.Fatalf("unable to create new channel: %v", err) @@ -2543,14 +2543,14 @@ func TestChanSyncFullySynced(t *testing.T) { t.Fatalf("unable to fetch channel: %v", err) } aliceChannelNew, err := NewLightningChannel( - aliceChannel.signer, nil, aliceChannels[0], + aliceChannel.Signer, nil, aliceChannels[0], ) if err != nil { t.Fatalf("unable to create new channel: %v", err) } defer aliceChannelNew.Stop() bobChannelNew, err := NewLightningChannel( - bobChannel.signer, nil, bobChannels[0], + bobChannel.Signer, nil, bobChannels[0], ) if err != nil { t.Fatalf("unable to create new channel: %v", err) @@ -2573,7 +2573,7 @@ func restartChannel(channelOld *LightningChannel) (*LightningChannel, error) { } channelNew, err := NewLightningChannel( - channelOld.signer, channelOld.pCache, nodeChannels[0], + channelOld.Signer, channelOld.pCache, nodeChannels[0], ) if err != nil { return nil, err @@ -4173,8 +4173,8 @@ func TestChannelUnilateralCloseHtlcResolution(t *testing.T) { SpenderTxHash: &commitTxHash, } aliceCloseSummary, err := NewUnilateralCloseSummary( - aliceChannel.channelState, aliceChannel.signer, aliceChannel.pCache, - spendDetail, aliceChannel.channelState.RemoteCommitment, + aliceChannel.channelState, aliceChannel.Signer, aliceChannel.pCache, + spendDetail, aliceChannel.channelState.RemoteCommitment, false, ) if err != nil { t.Fatalf("unable to create alice close summary: %v", err) @@ -4216,7 +4216,7 @@ func TestChannelUnilateralCloseHtlcResolution(t *testing.T) { // With the transaction constructed, we'll generate a witness that // should be valid for it, and verify using an instance of Script. sweepTx.TxIn[0].Witness, err = receiverHtlcSpendTimeout( - aliceChannel.signer, &outHtlcResolution.SweepSignDesc, + aliceChannel.Signer, &outHtlcResolution.SweepSignDesc, sweepTx, int32(outHtlcResolution.Expiry), ) if err != nil { @@ -4250,7 +4250,7 @@ func TestChannelUnilateralCloseHtlcResolution(t *testing.T) { sweepTx, ) sweepTx.TxIn[0].Witness, err = SenderHtlcSpendRedeem( - aliceChannel.signer, &inHtlcResolution.SweepSignDesc, + aliceChannel.Signer, &inHtlcResolution.SweepSignDesc, sweepTx, preimageBob[:], ) if err != nil { diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index 42d5859c..3ab47f2d 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -419,7 +419,7 @@ func TestCommitmentAndHTLCTransactions(t *testing.T) { // of the dependencies. channel := LightningChannel{ channelState: &channelState, - signer: signer, + Signer: signer, localChanCfg: &channelState.LocalChanCfg, remoteChanCfg: &channelState.RemoteChanCfg, }