From 2a6ad6e6343ff56a03fc0c1d27d148aebed854a4 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 11 Sep 2019 11:15:57 +0200 Subject: [PATCH] channeldb+lnwallet: don't pass isRestoredChan to ChanSyncMsg Since we have access to the internal state of the channel, we can instead get it directly instead of passing it in as a parameter. --- channeldb/channel.go | 13 ++++------- contractcourt/chain_watcher.go | 12 +++------- htlcswitch/link.go | 4 +--- lnwallet/channel.go | 4 +--- lnwallet/channel_test.go | 42 +++++++++++++++++----------------- peer.go | 2 +- 6 files changed, 32 insertions(+), 45 deletions(-) diff --git a/channeldb/channel.go b/channeldb/channel.go index 3db55e08..8b5d5253 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -803,13 +803,10 @@ func (c *OpenChannel) MarkBorked() error { // 3. We didn't get the last RevokeAndAck message they sent, so they'll // re-send it. // -// The isRestoredChan bool indicates if we need to craft a chan sync message -// for a channel that's been restored. If this is a restored channel, then -// we'll modify our typical chan sync message to ensure they force close even -// if we're on the very first state. -func (c *OpenChannel) ChanSyncMsg( - isRestoredChan bool) (*lnwire.ChannelReestablish, error) { - +// If this is a restored channel, having status ChanStatusRestored, then we'll +// modify our typical chan sync message to ensure they force close even if +// we're on the very first state. +func (c *OpenChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) { c.Lock() defer c.Unlock() @@ -853,7 +850,7 @@ func (c *OpenChannel) ChanSyncMsg( // If we've restored this channel, then we'll purposefully give them an // invalid LocalUnrevokedCommitPoint so they'll force close the channel // allowing us to sweep our funds. - if isRestoredChan { + if c.hasChanStatus(ChanStatusRestored) { currentCommitSecret[0] ^= 1 } diff --git a/contractcourt/chain_watcher.go b/contractcourt/chain_watcher.go index c7ba2323..3d13c922 100644 --- a/contractcourt/chain_watcher.go +++ b/contractcourt/chain_watcher.go @@ -731,9 +731,7 @@ func (c *chainWatcher) dispatchCooperativeClose(commitSpend *chainntnfs.SpendDet } // Attempt to add a channel sync message to the close summary. - chanSync, err := c.cfg.chanState.ChanSyncMsg( - c.cfg.chanState.HasChanStatus(channeldb.ChanStatusRestored), - ) + chanSync, err := c.cfg.chanState.ChanSyncMsg() if err != nil { log.Errorf("ChannelPoint(%v): unable to create channel sync "+ "message: %v", c.cfg.chanState.FundingOutpoint, err) @@ -810,9 +808,7 @@ func (c *chainWatcher) dispatchLocalForceClose( } // Attempt to add a channel sync message to the close summary. - chanSync, err := c.cfg.chanState.ChanSyncMsg( - c.cfg.chanState.HasChanStatus(channeldb.ChanStatusRestored), - ) + chanSync, err := c.cfg.chanState.ChanSyncMsg() if err != nil { log.Errorf("ChannelPoint(%v): unable to create channel sync "+ "message: %v", c.cfg.chanState.FundingOutpoint, err) @@ -996,9 +992,7 @@ func (c *chainWatcher) dispatchContractBreach(spendEvent *chainntnfs.SpendDetail } // Attempt to add a channel sync message to the close summary. - chanSync, err := c.cfg.chanState.ChanSyncMsg( - c.cfg.chanState.HasChanStatus(channeldb.ChanStatusRestored), - ) + chanSync, err := c.cfg.chanState.ChanSyncMsg() if err != nil { log.Errorf("ChannelPoint(%v): unable to create channel sync "+ "message: %v", c.cfg.chanState.FundingOutpoint, err) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 04cebc8d..fc92a5e9 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -609,9 +609,7 @@ func (l *channelLink) syncChanStates() error { // side. Based on this message, the remote party will decide if they // need to retransmit any data or not. chanState := l.channel.State() - localChanSyncMsg, err := chanState.ChanSyncMsg( - chanState.HasChanStatus(channeldb.ChanStatusRestored), - ) + localChanSyncMsg, err := chanState.ChanSyncMsg() 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 6a0a3e8d..90871498 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -5089,9 +5089,7 @@ func NewUnilateralCloseSummary(chanState *channeldb.OpenChannel, signer input.Si } // Attempt to add a channel sync message to the close summary. - chanSync, err := chanState.ChanSyncMsg( - chanState.HasChanStatus(channeldb.ChanStatusRestored), - ) + chanSync, err := chanState.ChanSyncMsg() if err != nil { walletLog.Errorf("ChannelPoint(%v): unable to create channel sync "+ "message: %v", chanState.FundingOutpoint, err) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 3a0281c5..ca686d34 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -2530,7 +2530,7 @@ func assertNoChanSyncNeeded(t *testing.T, aliceChannel *LightningChannel, _, _, line, _ := runtime.Caller(1) - aliceChanSyncMsg, err := aliceChannel.channelState.ChanSyncMsg(false) + aliceChanSyncMsg, err := aliceChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("line #%v: unable to produce chan sync msg: %v", line, err) @@ -2545,7 +2545,7 @@ func assertNoChanSyncNeeded(t *testing.T, aliceChannel *LightningChannel, "instead wants to send: %v", line, spew.Sdump(bobMsgsToSend)) } - bobChanSyncMsg, err := bobChannel.channelState.ChanSyncMsg(false) + bobChanSyncMsg, err := bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("line #%v: unable to produce chan sync msg: %v", line, err) @@ -2778,11 +2778,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.channelState.ChanSyncMsg(false) + aliceSyncMsg, err := aliceChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg(false) + bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3092,11 +3092,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.channelState.ChanSyncMsg(false) + aliceSyncMsg, err := aliceChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg(false) + bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3261,11 +3261,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.channelState.ChanSyncMsg(false) + aliceSyncMsg, err := aliceChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg(false) + bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3472,11 +3472,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.channelState.ChanSyncMsg(false) + aliceSyncMsg, err := aliceChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg(false) + bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3677,11 +3677,11 @@ func TestChanSyncFailure(t *testing.T) { assertLocalDataLoss := func(aliceOld *LightningChannel) { t.Helper() - aliceSyncMsg, err := aliceOld.channelState.ChanSyncMsg(false) + aliceSyncMsg, err := aliceOld.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg(false) + bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3755,7 +3755,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.channelState.ChanSyncMsg(false) + bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3769,7 +3769,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.channelState.ChanSyncMsg(false) + bobSyncMsg, err = bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3782,7 +3782,7 @@ func TestChanSyncFailure(t *testing.T) { // If Bob's NextLocalCommitHeight is lower than what Alice expects, Bob // probably lost state. - bobSyncMsg, err = bobChannel.channelState.ChanSyncMsg(false) + bobSyncMsg, err = bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3795,7 +3795,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.channelState.ChanSyncMsg(false) + bobSyncMsg, err = bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3824,7 +3824,7 @@ func TestChanSyncFailure(t *testing.T) { // when there's a pending remote commit. halfAdvance() - bobSyncMsg, err = bobChannel.channelState.ChanSyncMsg(false) + bobSyncMsg, err = bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -3912,11 +3912,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.channelState.ChanSyncMsg(false) + aliceSyncMsg, err := aliceChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } - bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg(false) + bobSyncMsg, err := bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to produce chan sync msg: %v", err) } @@ -4361,11 +4361,11 @@ func TestChanSyncInvalidLastSecret(t *testing.T) { } // Next, we'll produce the ChanSync messages for both parties. - aliceChanSync, err := aliceChannel.channelState.ChanSyncMsg(false) + aliceChanSync, err := aliceChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to generate chan sync msg: %v", err) } - bobChanSync, err := bobChannel.channelState.ChanSyncMsg(false) + bobChanSync, err := bobChannel.channelState.ChanSyncMsg() if err != nil { t.Fatalf("unable to generate chan sync msg: %v", err) } diff --git a/peer.go b/peer.go index 33639a9b..962f5fad 100644 --- a/peer.go +++ b/peer.go @@ -454,7 +454,7 @@ func (p *peer) loadActiveChannels(chans []*channeldb.OpenChannel) ( // channel reestablish message sent from the peer, but // that's okay since the assumption is that we did when // marking the channel borked. - chanSync, err := dbChan.ChanSyncMsg(false) + chanSync, err := dbChan.ChanSyncMsg() if err != nil { peerLog.Errorf("Unable to create channel "+ "reestablish message for channel %v: "+