diff --git a/lnwallet/channel.go b/lnwallet/channel.go index f54889f9..dfa1fca6 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1667,6 +1667,14 @@ func (lc *LightningChannel) restoreStateLogs( // If we did have a dangling commit, then we'll examine which updates // we included in that state and re-insert them into our update log. for _, logUpdate := range pendingRemoteCommitDiff.LogUpdates { + // If the log update is a fee update, then it doesn't need an + // entry within the updateLog, so we'll just apply it and move + // on. + if feeUpdate, ok := logUpdate.UpdateMsg.(*lnwire.UpdateFee); ok { + lc.pendingAckFeeUpdate = &feeUpdate.FeePerKw + continue + } + payDesc, err := lc.logUpdateToPayDesc( &logUpdate, lc.remoteUpdateLog, pendingHeight, pendingCommit.FeePerKw, pendingRemoteKeys, @@ -2310,7 +2318,7 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, // We've created a new commitment for the remote chain that // includes a fee update, and have not received a commitment - // after the fee update has been ACked. + // after the fee update has been ACKed. case !remoteChain && lc.pendingAckFeeUpdate != nil: feePerKw = *lc.pendingAckFeeUpdate } @@ -2810,11 +2818,24 @@ func (lc *LightningChannel) createCommitDiff( // out. chanID := lnwire.NewChanIDFromOutPoint(&lc.channelState.FundingOutpoint) + // If we have a fee update that we're waiting on an ACK of, then we'll + // create an entry so this is properly retransmitted. Note that we can + // only send an UpdateFee message if we're the initiator of the + // channel. + var logUpdates []channeldb.LogUpdate + if lc.channelState.IsInitiator && lc.pendingFeeUpdate != nil { + logUpdates = append(logUpdates, channeldb.LogUpdate{ + UpdateMsg: &lnwire.UpdateFee{ + ChanID: chanID, + FeePerKw: *lc.pendingFeeUpdate, + }, + }) + } + // We'll now run through our local update log to locate the items which // were only just committed within this pending state. This will be the // set of items we need to retransmit if we reconnect and find that // they didn't process this new state fully. - var logUpdates []channeldb.LogUpdate for e := lc.localUpdateLog.Front(); e != nil; e = e.Next() { pd := e.Value.(*PaymentDescriptor) @@ -3040,8 +3061,8 @@ func (lc *LightningChannel) SignNextCommitment() (*btcec.Signature, []*btcec.Sig // If we are the channel initiator then we would have signed any sent // fee update at this point, so mark this update as pending ACK, and // set pendingFeeUpdate to nil. We can do this since we know we won't - // sign any new commitment before receiving a revoke_and_ack, because - // of the revocation window of 1. + // sign any new commitment before receiving a RevokeAndAck, because of + // the revocation window of 1. if lc.channelState.IsInitiator { lc.pendingAckFeeUpdate = lc.pendingFeeUpdate lc.pendingFeeUpdate = nil @@ -3226,7 +3247,7 @@ func (lc *LightningChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) { remoteChainTipHeight := lc.remoteCommitChain.tail().height // If this channel has undergone a commitment update, then in order to - // prove to the remote party our knwoeldge of their prior commitment + // prove to the remote party our knowledge of their prior commitment // state, we'll also send over the last commitment secret that the // remote party sent. var lastCommitSecret [32]byte diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 7732e372..f851e821 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -3376,6 +3376,191 @@ func TestChanSyncOweRevocationAndCommitForceTransition(t *testing.T) { } } +// TestChannelRetransmissionFeeUpdate tests that the initiator will include any +// pending fee updates if it needs to retransmit signatures. +func TestChannelRetransmissionFeeUpdate(t *testing.T) { + t.Parallel() + + // Create a test channel which will be used for the duration of this + // unittest. The channel will be funded evenly with Alice having 5 BTC, + // and Bob having 5 BTC. + aliceChannel, bobChannel, cleanUp, err := createTestChannels(1) + if err != nil { + t.Fatalf("unable to create test channels: %v", err) + } + defer cleanUp() + + // First, we'll fetch the current fee rate present within the + // commitment transactions. + startingFeeRate := aliceChannel.channelState.LocalCommitment.FeePerKw + + // Next, we'll start a commitment update, with Alice sending a new + // update to double the fee rate of the commitment. + newFeeRate := startingFeeRate * 2 + if err := aliceChannel.UpdateFee(newFeeRate); err != nil { + t.Fatalf("unable to update fee for Alice's channel: %v", err) + } + if err := bobChannel.ReceiveUpdateFee(newFeeRate); err != nil { + t.Fatalf("unable to update fee for Bob's channel: %v", err) + } + + // Now, Alice will send a new commitment to Bob, but we'll simulate a + // connection failure, so Bob doesn't get her signature. + aliceSig, aliceHtlcSigs, err := aliceChannel.SignNextCommitment() + if err != nil { + t.Fatalf("unable to sign commitment: %v", err) + } + + // Restart both channels to simulate a connection restart. + aliceChannel, err = restartChannel(aliceChannel) + if err != nil { + t.Fatalf("unable to restart alice: %v", err) + } + defer aliceChannel.Stop() + bobChannel, err = restartChannel(bobChannel) + if err != nil { + t.Fatalf("unable to restart channel: %v", err) + } + defer bobChannel.Stop() + + // Bob doesn't get this message so upon reconnection, they need to + // synchronize. Alice should conclude that she owes Bob a commitment, + // while Bob should think he's properly synchronized. + aliceSyncMsg, err := aliceChannel.ChanSyncMsg() + if err != nil { + t.Fatalf("unable to produce chan sync msg: %v", err) + } + bobSyncMsg, err := bobChannel.ChanSyncMsg() + if err != nil { + t.Fatalf("unable to produce chan sync msg: %v", err) + } + + // Bob should detect that he doesn't need to send anything to Alice. + bobMsgsToSend, err := bobChannel.ProcessChanSyncMsg(aliceSyncMsg) + if err != nil { + t.Fatalf("unable to process chan sync msg: %v", err) + } + if len(bobMsgsToSend) != 0 { + t.Fatalf("expected bob to send %v messages instead "+ + "will send %v: %v", 0, len(bobMsgsToSend), + spew.Sdump(bobMsgsToSend)) + } + + // When Alice processes Bob's chan sync message, she should realize + // that she needs to first send a new UpdateFee message, and also a + // CommitSig. + aliceMsgsToSend, err := aliceChannel.ProcessChanSyncMsg( + bobSyncMsg, + ) + if err != nil { + t.Fatalf("unable to process chan sync msg: %v", err) + } + if len(aliceMsgsToSend) != 2 { + t.Fatalf("expected alice to send %v messages instead "+ + "will send %v: %v", 2, len(aliceMsgsToSend), + spew.Sdump(aliceMsgsToSend)) + } + + // The first message should be an UpdateFee message. + retransFeeMsg, ok := aliceMsgsToSend[0].(*lnwire.UpdateFee) + if !ok { + t.Fatalf("expected UpdateFee message, instead have: %v", + spew.Sdump(aliceMsgsToSend[0])) + } + + // The fee should match exactly the new fee update we applied above. + if retransFeeMsg.FeePerKw != newFeeRate { + t.Fatalf("fee update doesn't match: expected %v, got %v", + newFeeRate, retransFeeMsg) + } + + // The second, should be a CommitSig message, and be identical to the + // sig message she sent prior. + commitSigMsg, ok := aliceMsgsToSend[1].(*lnwire.CommitSig) + if !ok { + t.Fatalf("expected a CommitSig message, instead have %v", + spew.Sdump(aliceMsgsToSend[1])) + } + if !commitSigMsg.CommitSig.IsEqual(aliceSig) { + t.Fatalf("commit sig msgs don't match: expected %x got %x", + aliceSig.Serialize(), commitSigMsg.CommitSig.Serialize()) + } + if len(commitSigMsg.HtlcSigs) != len(aliceHtlcSigs) { + t.Fatalf("wrong number of htlc sigs: expected %v, got %v", + len(aliceHtlcSigs), len(commitSigMsg.HtlcSigs)) + } + for i, htlcSig := range commitSigMsg.HtlcSigs { + if !htlcSig.IsEqual(aliceHtlcSigs[i]) { + t.Fatalf("htlc sig msgs don't match: "+ + "expected %x got %x", + aliceHtlcSigs[i].Serialize(), + htlcSig.Serialize()) + } + } + + // Now, we if re-apply the updates to Bob, we should be able to resume + // the commitment update as normal. + if err := bobChannel.ReceiveUpdateFee(newFeeRate); err != nil { + t.Fatalf("unable to update fee for Bob's channel: %v", err) + } + + err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs) + if err != nil { + t.Fatalf("bob unable to process alice's commitment: %v", err) + } + bobRevocation, err := bobChannel.RevokeCurrentCommitment() + if err != nil { + t.Fatalf("unable to revoke bob commitment: %v", err) + } + bobSig, bobHtlcSigs, err := bobChannel.SignNextCommitment() + if err != nil { + t.Fatalf("bob unable to sign commitment: %v", err) + } + _, err = aliceChannel.ReceiveRevocation(bobRevocation) + if err != nil { + t.Fatalf("alice unable to recv revocation: %v", err) + } + err = aliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs) + if err != nil { + t.Fatalf("alice unable to rev bob's commitment: %v", err) + } + aliceRevocation, err := aliceChannel.RevokeCurrentCommitment() + if err != nil { + t.Fatalf("alice unable to revoke commitment: %v", err) + } + if _, err := bobChannel.ReceiveRevocation(aliceRevocation); err != nil { + t.Fatalf("bob unable to recv revocation: %v", err) + } + + // Both parties should now have the latest fee rate locked-in. + if aliceChannel.channelState.LocalCommitment.FeePerKw != newFeeRate { + t.Fatalf("alice's feePerKw was not locked in") + } + if bobChannel.channelState.LocalCommitment.FeePerKw != newFeeRate { + t.Fatalf("bob's feePerKw was not locked in") + } + + // Finally, we'll add with adding a new HTLC, then forcing a state + // transition. This should also proceed as normal. + var bobPreimage [32]byte + copy(bobPreimage[:], bytes.Repeat([]byte{0xaa}, 32)) + rHash := sha256.Sum256(bobPreimage[:]) + bobHtlc := &lnwire.UpdateAddHTLC{ + PaymentHash: rHash, + Amount: lnwire.NewMSatFromSatoshis(20000), + Expiry: uint32(10), + } + if _, err := bobChannel.AddHTLC(bobHtlc); err != nil { + t.Fatalf("unable to add bob's htlc: %v", err) + } + if _, err := aliceChannel.ReceiveHTLC(bobHtlc); err != nil { + t.Fatalf("unable to recv bob's htlc: %v", err) + } + if err := forceStateTransition(bobChannel, aliceChannel); err != nil { + t.Fatalf("unable to complete bob's state transition: %v", err) + } +} + // TestChanSyncUnableToSync tests that if Alice or Bob receive an invalid // ChannelReestablish messages,then they reject the message and declare the // channel un-continuable by returning ErrCannotSyncCommitChains.