From 676a1b140754b63998d75ee9fc61cb557eddc71b Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 20 Nov 2018 15:09:45 +0100 Subject: [PATCH] lnwallet+link: make ChanSyncMsg take channel state as arg This lets us get the channel reestablish message without creating the LightningChannel struct first. --- htlcswitch/link.go | 2 +- lnwallet/channel.go | 17 +++++++++------- lnwallet/channel_test.go | 42 ++++++++++++++++++++-------------------- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 71563592..3ee614c7 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -516,7 +516,7 @@ func (l *channelLink) syncChanStates() error { // First, we'll generate our ChanSync message to send to the other // side. Based on this message, the remote party will decide if they // need to retransmit any data or not. - localChanSyncMsg, err := l.channel.ChanSyncMsg() + localChanSyncMsg, err := lnwallet.ChanSyncMsg(l.channel.State()) if err != nil { return fmt.Errorf("unable to generate chan sync message for "+ "ChannelPoint(%v)", l.channel.ChannelPoint()) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 34e815e7..d45ba296 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -3446,19 +3446,22 @@ func (lc *LightningChannel) ProcessChanSyncMsg( // it. // 3. We didn't get the last RevokeAndAck message they sent, so they'll // re-send it. -func (lc *LightningChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) { +func ChanSyncMsg(c *channeldb.OpenChannel) (*lnwire.ChannelReestablish, error) { + c.Lock() + defer c.Unlock() + // The remote commitment height that we'll send in the // ChannelReestablish message is our current commitment height plus // one. If the receiver thinks that our commitment height is actually // *equal* to this value, then they'll re-send the last commitment that // they sent but we never fully processed. - localHeight := lc.localCommitChain.tip().height + localHeight := c.LocalCommitment.CommitHeight nextLocalCommitHeight := localHeight + 1 // The second value we'll send is the height of the remote commitment // from our PoV. If the receiver thinks that their height is actually // *one plus* this value, then they'll re-send their last revocation. - remoteChainTipHeight := lc.remoteCommitChain.tail().height + remoteChainTipHeight := c.RemoteCommitment.CommitHeight // If this channel has undergone a commitment update, then in order to // prove to the remote party our knowledge of their prior commitment @@ -3466,7 +3469,7 @@ func (lc *LightningChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) { // remote party sent. var lastCommitSecret [32]byte if remoteChainTipHeight != 0 { - remoteSecret, err := lc.channelState.RevocationStore.LookUp( + remoteSecret, err := c.RevocationStore.LookUp( remoteChainTipHeight - 1, ) if err != nil { @@ -3477,7 +3480,7 @@ func (lc *LightningChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) { // Additionally, we'll send over the current unrevoked commitment on // our local commitment transaction. - currentCommitSecret, err := lc.channelState.RevocationProducer.AtIndex( + currentCommitSecret, err := c.RevocationProducer.AtIndex( localHeight, ) if err != nil { @@ -3486,7 +3489,7 @@ func (lc *LightningChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) { return &lnwire.ChannelReestablish{ ChanID: lnwire.NewChanIDFromOutPoint( - &lc.channelState.FundingOutpoint, + &c.FundingOutpoint, ), NextLocalCommitHeight: nextLocalCommitHeight, RemoteCommitTailHeight: remoteChainTipHeight, @@ -6188,7 +6191,7 @@ func (lc *LightningChannel) IsPending() bool { return lc.channelState.IsPending } -// State provides access to the channel's internal state for testing. +// State provides access to the channel's internal state. func (lc *LightningChannel) State() *channeldb.OpenChannel { return lc.channelState } diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 4586d75e..24701f28 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -2433,7 +2433,7 @@ func assertNoChanSyncNeeded(t *testing.T, aliceChannel *LightningChannel, _, _, line, _ := runtime.Caller(1) - aliceChanSyncMsg, err := aliceChannel.ChanSyncMsg() + aliceChanSyncMsg, err := ChanSyncMsg(aliceChannel.channelState) if err != nil { t.Fatalf("line #%v: unable to produce chan sync msg: %v", line, err) @@ -2448,7 +2448,7 @@ func assertNoChanSyncNeeded(t *testing.T, aliceChannel *LightningChannel, "instead wants to send: %v", line, spew.Sdump(bobMsgsToSend)) } - bobChanSyncMsg, err := bobChannel.ChanSyncMsg() + bobChanSyncMsg, err := ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("line #%v: unable to produce chan sync msg: %v", line, err) @@ -2681,11 +2681,11 @@ func TestChanSyncOweCommitment(t *testing.T) { // 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() + aliceSyncMsg, err := ChanSyncMsg(aliceChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.ChanSyncMsg() + bobSyncMsg, err := ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -2995,11 +2995,11 @@ func TestChanSyncOweRevocation(t *testing.T) { // If we fetch the channel sync messages at this state, then Alice // should report that she owes Bob a revocation message, while Bob // thinks they're fully in sync. - aliceSyncMsg, err := aliceChannel.ChanSyncMsg() + aliceSyncMsg, err := ChanSyncMsg(aliceChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.ChanSyncMsg() + bobSyncMsg, err := ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3164,11 +3164,11 @@ func TestChanSyncOweRevocationAndCommit(t *testing.T) { // If we now attempt to resync, then Alice should conclude that she // doesn't need any further updates, while Bob concludes that he needs // to re-send both his revocation and commit sig message. - aliceSyncMsg, err := aliceChannel.ChanSyncMsg() + aliceSyncMsg, err := ChanSyncMsg(aliceChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.ChanSyncMsg() + bobSyncMsg, err := ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3374,11 +3374,11 @@ func TestChanSyncOweRevocationAndCommitForceTransition(t *testing.T) { // Now if we attempt to synchronize states at this point, Alice should // detect that she owes nothing, while Bob should re-send both his // RevokeAndAck as well as his commitment message. - aliceSyncMsg, err := aliceChannel.ChanSyncMsg() + aliceSyncMsg, err := ChanSyncMsg(aliceChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.ChanSyncMsg() + bobSyncMsg, err := ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3573,11 +3573,11 @@ func TestChanSyncFailure(t *testing.T) { // assertLocalDataLoss checks that aliceOld and bobChannel detects that // Alice has lost state during sync. assertLocalDataLoss := func(aliceOld *LightningChannel) { - aliceSyncMsg, err := aliceOld.ChanSyncMsg() + aliceSyncMsg, err := ChanSyncMsg(aliceOld.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.ChanSyncMsg() + bobSyncMsg, err := ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3629,7 +3629,7 @@ func TestChanSyncFailure(t *testing.T) { // If we remove the recovery options from Bob's message, Alice cannot // tell if she lost state, since Bob might be lying. She still should // be able to detect that chains cannot be synced. - bobSyncMsg, err := bobChannel.ChanSyncMsg() + bobSyncMsg, err := ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3643,7 +3643,7 @@ func TestChanSyncFailure(t *testing.T) { // If Bob lies about the NextLocalCommitHeight, making it greater than // what Alice expect, she cannot tell for sure whether she lost state, // but should detect the desync. - bobSyncMsg, err = bobChannel.ChanSyncMsg() + bobSyncMsg, err = ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3656,7 +3656,7 @@ func TestChanSyncFailure(t *testing.T) { // If Bob's NextLocalCommitHeight is lower than what Alice expects, Bob // probably lost state. - bobSyncMsg, err = bobChannel.ChanSyncMsg() + bobSyncMsg, err = ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3669,7 +3669,7 @@ func TestChanSyncFailure(t *testing.T) { // If Alice and Bob's states are in sync, but Bob is sending the wrong // LocalUnrevokedCommitPoint, Alice should detect this. - bobSyncMsg, err = bobChannel.ChanSyncMsg() + bobSyncMsg, err = ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3695,7 +3695,7 @@ func TestChanSyncFailure(t *testing.T) { // when there's a pending remote commit. halfAdvance() - bobSyncMsg, err = bobChannel.ChanSyncMsg() + bobSyncMsg, err = ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3785,11 +3785,11 @@ func TestChannelRetransmissionFeeUpdate(t *testing.T) { // 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() + aliceSyncMsg, err := ChanSyncMsg(aliceChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.ChanSyncMsg() + bobSyncMsg, err := ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -4015,11 +4015,11 @@ func TestChanSyncInvalidLastSecret(t *testing.T) { } // Next, we'll produce the ChanSync messages for both parties. - aliceChanSync, err := aliceChannel.ChanSyncMsg() + aliceChanSync, err := ChanSyncMsg(aliceChannel.channelState) if err != nil { t.Fatalf("unable to generate chan sync msg: %v", err) } - bobChanSync, err := bobChannel.ChanSyncMsg() + bobChanSync, err := ChanSyncMsg(bobChannel.channelState) if err != nil { t.Fatalf("unable to generate chan sync msg: %v", err) }