From 4e6b1cda494514060ba7e7f23686a9c2737b495b Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Fri, 2 Feb 2024 11:40:22 -0800 Subject: [PATCH 1/4] lnwallet: change channelDispute to channelClosed so errors are handled --- lnwallet/channel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 4d2f798df..6d7ca722c 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -7595,7 +7595,7 @@ func (lc *LightningChannel) ForceClose() (*LocalForceCloseSummary, error) { // Set the channel state to indicate that the channel is now in a // contested state. - lc.status = channelDispute + lc.status = channelClosed return summary, nil } From 17e67348a8b7dd9389d5cf414feacbee938dcba1 Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Tue, 30 Jan 2024 20:49:03 -0500 Subject: [PATCH 2/4] lnwallet: remove unused channelPending channelState lnwallet: remove unused channelPendingPayment channelState Since this state is never set nor read, we remove it completely. lnwallet: remove redundant channelDispute channelState In this case, even though we do set this value, it is never read. Further, the times we read the field at all from LightningChannel we want the situation of force-closure to block any other concurrent closure attempts, so we change the sites where we set channelDispute to channelClosed. lnwallet: remove redundant channelClosing channelStatus This value is never used to impact control flow so we need not set it. We also need not have it. --- lnwallet/channel.go | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 6d7ca722c..1df8c2362 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -159,30 +159,14 @@ func (e *ErrCommitSyncLocalDataLoss) Error() string { type channelState uint8 const ( - // channelPending indicates this channel is still going through the - // funding workflow, and isn't yet open. - channelPending channelState = iota // nolint: unused - // channelOpen represents an open, active channel capable of // sending/receiving HTLCs. - channelOpen - - // channelClosing represents a channel which is in the process of being - // closed. - channelClosing + channelOpen channelState = iota // channelClosed represents a channel which has been fully closed. Note // that before a channel can be closed, ALL pending HTLCs must be // settled/removed. channelClosed - - // channelDispute indicates that an un-cooperative closure has been - // detected within the channel. - channelDispute - - // channelPendingPayment indicates that there a currently outstanding - // HTLCs within the channel. - channelPendingPayment // nolint:unused ) // PaymentHash represents the sha256 of a random value. This hash is used to @@ -7853,10 +7837,6 @@ func (lc *LightningChannel) CreateCloseProposal(proposedFee btcutil.Amount, } } - // As everything checks out, indicate in the channel status that a - // channel closure has been initiated. - lc.status = channelClosing - closeTXID := closeTx.TxHash() return sig, &closeTXID, ourBalance, nil } From 606f8e79d1c48ff17b4aaddb2fd58e9637bdb7b6 Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Tue, 30 Jan 2024 21:06:49 -0500 Subject: [PATCH 3/4] lnwallet: rewrite channelState to bool for clarity Over the last few commits we have systematically eliminated all but two states. This allows us to replace it with a boolean to encode the two remaining states. We would like to be able to eliminate this field entirely, but doing so requires being able to prove that the concurrent request block is necessary. This is more difficult and will be left to future commits. --- lnwallet/channel.go | 35 +++++++++-------------------------- lnwallet/channel_test.go | 4 ++-- 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 1df8c2362..028b06e46 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -153,22 +153,6 @@ func (e *ErrCommitSyncLocalDataLoss) Error() string { e.CommitPoint.SerializeCompressed()) } -// channelState is an enum like type which represents the current state of a -// particular channel. -// TODO(roasbeef): actually update state -type channelState uint8 - -const ( - // channelOpen represents an open, active channel capable of - // sending/receiving HTLCs. - channelOpen channelState = iota - - // channelClosed represents a channel which has been fully closed. Note - // that before a channel can be closed, ALL pending HTLCs must be - // settled/removed. - channelClosed -) - // PaymentHash represents the sha256 of a random value. This hash is used to // uniquely track incoming/outgoing payments within this channel, as well as // payments requested by the wallet/daemon. @@ -1267,7 +1251,7 @@ type LightningChannel struct { // the commitment transaction that spends the multi-sig output. signDesc *input.SignDescriptor - status channelState + isClosed bool // ChanPoint is the funding outpoint of this channel. ChanPoint *wire.OutPoint @@ -1522,7 +1506,7 @@ func (lc *LightningChannel) createSignDesc() error { // events do so properly. func (lc *LightningChannel) ResetState() { lc.Lock() - lc.status = channelOpen + lc.isClosed = false lc.Unlock() } @@ -7577,9 +7561,8 @@ func (lc *LightningChannel) ForceClose() (*LocalForceCloseSummary, error) { "summary: %w", err) } - // Set the channel state to indicate that the channel is now in a - // contested state. - lc.status = channelClosed + // Mark the channel as closed to block future closure requests. + lc.isClosed = true return summary, nil } @@ -7773,8 +7756,8 @@ func (lc *LightningChannel) CreateCloseProposal(proposedFee btcutil.Amount, lc.Lock() defer lc.Unlock() - // If we've already closed the channel, then ignore this request. - if lc.status == channelClosed { + // If we're already closing the channel, then ignore this request. + if lc.isClosed { // TODO(roasbeef): check to ensure no pending payments return nil, nil, 0, ErrChanClosing } @@ -7857,8 +7840,8 @@ func (lc *LightningChannel) CompleteCooperativeClose( lc.Lock() defer lc.Unlock() - // If the channel is already closed, then ignore this request. - if lc.status == channelClosed { + // If the channel is already closing, then ignore this request. + if lc.isClosed { // TODO(roasbeef): check to ensure no pending payments return nil, 0, ErrChanClosing } @@ -7962,7 +7945,7 @@ func (lc *LightningChannel) CompleteCooperativeClose( // As the transaction is sane, and the scripts are valid we'll mark the // channel now as closed as the closure transaction should get into the // chain in a timely manner and possibly be re-broadcast by the wallet. - lc.status = channelClosed + lc.isClosed = true return closeTx, ourBalance, nil } diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index db5d0f468..5c0c972ac 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -2147,8 +2147,8 @@ func TestCooperativeCloseDustAdherence(t *testing.T) { } resetChannelState := func() { - aliceChannel.status = channelOpen - bobChannel.status = channelOpen + aliceChannel.isClosed = false + bobChannel.isClosed = false } setBalances := func(aliceBalance, bobBalance lnwire.MilliSatoshi) { From 3a02f2ba659404b5d3f79be233a4e354bd87915e Mon Sep 17 00:00:00 2001 From: Keagan McClelland Date: Thu, 1 Feb 2024 08:08:00 -0800 Subject: [PATCH 4/4] lnwallet: use ResetState instead of tweaking private rows --- lnwallet/channel_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 5c0c972ac..32a00e769 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -2147,8 +2147,8 @@ func TestCooperativeCloseDustAdherence(t *testing.T) { } resetChannelState := func() { - aliceChannel.isClosed = false - bobChannel.isClosed = false + aliceChannel.ResetState() + bobChannel.ResetState() } setBalances := func(aliceBalance, bobBalance lnwire.MilliSatoshi) {