From 59f32682a5162d71db0c3c6bce67e81e13a06578 Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 28 Oct 2024 12:46:31 +0100 Subject: [PATCH] contractcourt: introduce option for commitKey. --- contractcourt/briefcase.go | 29 +++++++++++++++--------- contractcourt/briefcase_test.go | 3 ++- contractcourt/chain_watcher.go | 16 ++++++------- contractcourt/channel_arbitrator.go | 20 ++++++++++++---- contractcourt/channel_arbitrator_test.go | 18 +++++++++------ 5 files changed, 55 insertions(+), 31 deletions(-) diff --git a/contractcourt/briefcase.go b/contractcourt/briefcase.go index 26df50b30..acc49be44 100644 --- a/contractcourt/briefcase.go +++ b/contractcourt/briefcase.go @@ -1475,16 +1475,23 @@ func decodeBreachResolution(r io.Reader, b *BreachResolution) error { return binary.Read(r, endian, &b.FundingOutPoint.Index) } -func encodeHtlcSetKey(w io.Writer, h *HtlcSetKey) error { - err := binary.Write(w, endian, h.IsRemote) +func encodeHtlcSetKey(w io.Writer, htlcSetKey HtlcSetKey) error { + err := binary.Write(w, endian, htlcSetKey.IsRemote) if err != nil { return err } - return binary.Write(w, endian, h.IsPending) + + return binary.Write(w, endian, htlcSetKey.IsPending) } func encodeCommitSet(w io.Writer, c *CommitSet) error { - if err := encodeHtlcSetKey(w, c.ConfCommitKey); err != nil { + confCommitKey, err := c.ConfCommitKey.UnwrapOrErr( + fmt.Errorf("HtlcSetKey is not set"), + ) + if err != nil { + return err + } + if err := encodeHtlcSetKey(w, confCommitKey); err != nil { return err } @@ -1494,8 +1501,7 @@ func encodeCommitSet(w io.Writer, c *CommitSet) error { } for htlcSetKey, htlcs := range c.HtlcSets { - htlcSetKey := htlcSetKey - if err := encodeHtlcSetKey(w, &htlcSetKey); err != nil { + if err := encodeHtlcSetKey(w, htlcSetKey); err != nil { return err } @@ -1517,13 +1523,14 @@ func decodeHtlcSetKey(r io.Reader, h *HtlcSetKey) error { } func decodeCommitSet(r io.Reader) (*CommitSet, error) { - c := &CommitSet{ - ConfCommitKey: &HtlcSetKey{}, - HtlcSets: make(map[HtlcSetKey][]channeldb.HTLC), + confCommitKey := HtlcSetKey{} + if err := decodeHtlcSetKey(r, &confCommitKey); err != nil { + return nil, err } - if err := decodeHtlcSetKey(r, c.ConfCommitKey); err != nil { - return nil, err + c := &CommitSet{ + ConfCommitKey: fn.Some(confCommitKey), + HtlcSets: make(map[HtlcSetKey][]channeldb.HTLC), } var numSets uint8 diff --git a/contractcourt/briefcase_test.go b/contractcourt/briefcase_test.go index 89e017fd7..0f44db2ab 100644 --- a/contractcourt/briefcase_test.go +++ b/contractcourt/briefcase_test.go @@ -14,6 +14,7 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lnmock" @@ -753,7 +754,7 @@ func TestCommitSetStorage(t *testing.T) { for _, pendingRemote := range []bool{true, false} { for _, confType := range confTypes { commitSet := &CommitSet{ - ConfCommitKey: &confType, + ConfCommitKey: fn.Some(confType), HtlcSets: make(map[HtlcSetKey][]channeldb.HTLC), } commitSet.HtlcSets[LocalHtlcSet] = activeHTLCs diff --git a/contractcourt/chain_watcher.go b/contractcourt/chain_watcher.go index ef4a4e200..f25ca7e0a 100644 --- a/contractcourt/chain_watcher.go +++ b/contractcourt/chain_watcher.go @@ -93,9 +93,9 @@ type BreachCloseInfo struct { // HTLCs to determine if any additional actions need to be made based on the // remote party's commitments. type CommitSet struct { - // ConfCommitKey if non-nil, identifies the commitment that was + // When the ConfCommitKey is set, it signals that the commitment tx was // confirmed in the chain. - ConfCommitKey *HtlcSetKey + ConfCommitKey fn.Option[HtlcSetKey] // HtlcSets stores the set of all known active HTLC for each active // commitment at the time of channel closure. @@ -509,7 +509,7 @@ func (c *chainWatcher) handleUnknownLocalState( // If this is our commitment transaction, then we try to act even // though we won't be able to sweep HTLCs. - chainSet.commitSet.ConfCommitKey = &LocalHtlcSet + chainSet.commitSet.ConfCommitKey = fn.Some(LocalHtlcSet) if err := c.dispatchLocalForceClose( commitSpend, broadcastStateNum, chainSet.commitSet, ); err != nil { @@ -806,7 +806,7 @@ func (c *chainWatcher) handleKnownLocalState( return false, nil } - chainSet.commitSet.ConfCommitKey = &LocalHtlcSet + chainSet.commitSet.ConfCommitKey = fn.Some(LocalHtlcSet) if err := c.dispatchLocalForceClose( commitSpend, broadcastStateNum, chainSet.commitSet, ); err != nil { @@ -844,7 +844,7 @@ func (c *chainWatcher) handleKnownRemoteState( log.Infof("Remote party broadcast base set, "+ "commit_num=%v", chainSet.remoteStateNum) - chainSet.commitSet.ConfCommitKey = &RemoteHtlcSet + chainSet.commitSet.ConfCommitKey = fn.Some(RemoteHtlcSet) err := c.dispatchRemoteForceClose( commitSpend, chainSet.remoteCommit, chainSet.commitSet, @@ -869,7 +869,7 @@ func (c *chainWatcher) handleKnownRemoteState( log.Infof("Remote party broadcast pending set, "+ "commit_num=%v", chainSet.remoteStateNum+1) - chainSet.commitSet.ConfCommitKey = &RemotePendingHtlcSet + chainSet.commitSet.ConfCommitKey = fn.Some(RemotePendingHtlcSet) err := c.dispatchRemoteForceClose( commitSpend, *chainSet.remotePendingCommit, chainSet.commitSet, @@ -936,7 +936,7 @@ func (c *chainWatcher) handlePossibleBreach(commitSpend *chainntnfs.SpendDetail, // only used to ensure a nil-pointer-dereference doesn't occur and is // not used otherwise. The HTLC's may not exist for the // RemotePendingHtlcSet. - chainSet.commitSet.ConfCommitKey = &RemoteHtlcSet + chainSet.commitSet.ConfCommitKey = fn.Some(RemoteHtlcSet) // THEY'RE ATTEMPTING TO VIOLATE THE CONTRACT LAID OUT WITHIN THE // PAYMENT CHANNEL. Therefore we close the signal indicating a revoked @@ -997,7 +997,7 @@ func (c *chainWatcher) handleUnknownRemoteState( // means we won't be able to recover any HTLC funds. // // TODO(halseth): can we try to recover some HTLCs? - chainSet.commitSet.ConfCommitKey = &RemoteHtlcSet + chainSet.commitSet.ConfCommitKey = fn.Some(RemoteHtlcSet) err := c.dispatchRemoteForceClose( commitSpend, channeldb.ChannelCommitment{}, chainSet.commitSet, commitPoint, diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index a76fcd330..cc1ee6958 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -682,8 +682,14 @@ func (c *ChannelArbitrator) relaunchResolvers(commitSet *CommitSet, // chain actions may exclude some information, but we cannot recover it // for these older nodes at the moment. var confirmedHTLCs []channeldb.HTLC - if commitSet != nil && commitSet.ConfCommitKey != nil { - confirmedHTLCs = commitSet.HtlcSets[*commitSet.ConfCommitKey] + if commitSet != nil && commitSet.ConfCommitKey.IsSome() { + confCommitKey, err := commitSet.ConfCommitKey.UnwrapOrErr( + fmt.Errorf("no commitKey available"), + ) + if err != nil { + return err + } + confirmedHTLCs = commitSet.HtlcSets[confCommitKey] } else { chainActions, err := c.log.FetchChainActions() if err != nil { @@ -2223,15 +2229,21 @@ func (c *ChannelArbitrator) constructChainActions(confCommitSet *CommitSet, // then this is an older node that had a pending close channel before // the CommitSet was introduced. In this case, we'll just return the // existing ChainActionMap they had on disk. - if confCommitSet == nil || confCommitSet.ConfCommitKey == nil { + if confCommitSet == nil || confCommitSet.ConfCommitKey.IsNone() { return c.log.FetchChainActions() } // Otherwise, we have the full commitment set written to disk, and can // proceed as normal. htlcSets := confCommitSet.toActiveHTLCSets() - switch *confCommitSet.ConfCommitKey { + confCommitKey, err := confCommitSet.ConfCommitKey.UnwrapOrErr( + fmt.Errorf("no commitKey available"), + ) + if err != nil { + return nil, err + } + switch confCommitKey { // If the local commitment transaction confirmed, then we'll examine // that as well as their commitments to the set of chain actions. case LocalHtlcSet: diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index 441c36907..1353770d8 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/contractcourt/channel_arbitrator_test.go @@ -590,7 +590,7 @@ func TestChannelArbitratorRemoteForceClose(t *testing.T) { chanArb.cfg.ChainEvents.RemoteUnilateralClosure <- &RemoteUnilateralCloseInfo{ UnilateralCloseSummary: uniClose, CommitSet: CommitSet{ - ConfCommitKey: &RemoteHtlcSet, + ConfCommitKey: fn.Some(RemoteHtlcSet), HtlcSets: make(map[HtlcSetKey][]channeldb.HTLC), }, } @@ -777,7 +777,7 @@ func TestChannelArbitratorBreachClose(t *testing.T) { }, AnchorResolution: anchorRes, CommitSet: CommitSet{ - ConfCommitKey: &RemoteHtlcSet, + ConfCommitKey: fn.Some(RemoteHtlcSet), HtlcSets: map[HtlcSetKey][]channeldb.HTLC{ RemoteHtlcSet: {htlc1, htlc2}, }, @@ -999,7 +999,7 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { }, ChannelCloseSummary: &channeldb.ChannelCloseSummary{}, CommitSet: CommitSet{ - ConfCommitKey: &LocalHtlcSet, + ConfCommitKey: fn.Some(LocalHtlcSet), HtlcSets: map[HtlcSetKey][]channeldb.HTLC{ LocalHtlcSet: htlcSet, }, @@ -1527,7 +1527,7 @@ func TestChannelArbitratorForceCloseBreachedChannel(t *testing.T) { }, } log.commitSet = &CommitSet{ - ConfCommitKey: &RemoteHtlcSet, + ConfCommitKey: fn.Some(RemoteHtlcSet), HtlcSets: map[HtlcSetKey][]channeldb.HTLC{ RemoteHtlcSet: {}, }, @@ -1954,8 +1954,12 @@ func TestChannelArbitratorDanglingCommitForceClose(t *testing.T) { }, ChannelCloseSummary: &channeldb.ChannelCloseSummary{}, CommitSet: CommitSet{ - ConfCommitKey: &testCase.confCommit, - HtlcSets: make(map[HtlcSetKey][]channeldb.HTLC), + ConfCommitKey: fn.Some( + testCase.confCommit, + ), + HtlcSets: make( + map[HtlcSetKey][]channeldb.HTLC, + ), }, } @@ -2875,7 +2879,7 @@ func TestChannelArbitratorAnchors(t *testing.T) { }, ChannelCloseSummary: &channeldb.ChannelCloseSummary{}, CommitSet: CommitSet{ - ConfCommitKey: &LocalHtlcSet, + ConfCommitKey: fn.Some(LocalHtlcSet), HtlcSets: map[HtlcSetKey][]channeldb.HTLC{}, }, }