From adddc1442e9fd9c26ef49feb8116bc43d187612b Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 6 May 2021 19:53:11 +0800 Subject: [PATCH] multi: refactor NewAnchorResolutions to return fixed values This commit adds a new struct AnchorResolutions which wraps the anchor resolutions for local/remote/pending remote commitment transactions. It is then returned from NewAnchorResolutions. Thus the caller knows how to retrieve a certain anchor resolution. --- contractcourt/chain_arbitrator.go | 2 +- contractcourt/channel_arbitrator.go | 10 ++++-- contractcourt/channel_arbitrator_test.go | 13 ++++---- lnwallet/channel.go | 40 +++++++++++++++--------- lnwallet/channel_test.go | 13 +++++--- 5 files changed, 49 insertions(+), 29 deletions(-) diff --git a/contractcourt/chain_arbitrator.go b/contractcourt/chain_arbitrator.go index a8be6a42..453f292b 100644 --- a/contractcourt/chain_arbitrator.go +++ b/contractcourt/chain_arbitrator.go @@ -241,7 +241,7 @@ type arbChannel struct { // commitment transactions. // // NOTE: Part of the ArbChannel interface. -func (a *arbChannel) NewAnchorResolutions() ([]*lnwallet.AnchorResolution, +func (a *arbChannel) NewAnchorResolutions() (*lnwallet.AnchorResolutions, error) { // Get a fresh copy of the database state to base the anchor resolutions diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index 6d58ef20..0a8de786 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -91,7 +91,7 @@ type ArbChannel interface { // NewAnchorResolutions returns the anchor resolutions for currently // valid commitment transactions. - NewAnchorResolutions() ([]*lnwallet.AnchorResolution, error) + NewAnchorResolutions() (*lnwallet.AnchorResolutions, error) } // ChannelArbitratorConfig contains all the functionality that the @@ -1087,14 +1087,18 @@ func (c *ChannelArbitrator) stateStep( // sweepAnchors offers all given anchor resolutions to the sweeper. It requests // sweeping at the minimum fee rate. This fee rate can be upped manually by the // user via the BumpFee rpc. -func (c *ChannelArbitrator) sweepAnchors(anchors []*lnwallet.AnchorResolution, +func (c *ChannelArbitrator) sweepAnchors(anchors *lnwallet.AnchorResolutions, heightHint uint32) error { // Use the chan id as the exclusive group. This prevents any of the // anchors from being batched together. exclusiveGroup := c.cfg.ShortChanID.ToUint64() - for _, anchor := range anchors { + // TODO: refactor this function in next commit. + for _, anchor := range []*lnwallet.AnchorResolution{ + anchors.Local, anchors.Remote, anchors.RemotePending, + } { + log.Debugf("ChannelArbitrator(%v): pre-confirmation sweep of "+ "anchor of tx %v", c.cfg.ChanPoint, anchor.CommitAnchor) diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index fa3f04ef..d5a2ac1a 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/contractcourt/channel_arbitrator_test.go @@ -2119,9 +2119,7 @@ func TestChannelArbitratorAnchors(t *testing.T) { // Setup two pre-confirmation anchor resolutions on the mock channel. chanArb.cfg.Channel.(*mockChannel).anchorResolutions = - []*lnwallet.AnchorResolution{ - {}, {}, - } + &lnwallet.AnchorResolutions{} if err := chanArb.Start(nil); err != nil { t.Fatalf("unable to start ChannelArbitrator: %v", err) @@ -2286,13 +2284,16 @@ func assertResolverReport(t *testing.T, reports chan *channeldb.ResolverReport, } type mockChannel struct { - anchorResolutions []*lnwallet.AnchorResolution + anchorResolutions *lnwallet.AnchorResolutions } -func (m *mockChannel) NewAnchorResolutions() ([]*lnwallet.AnchorResolution, +func (m *mockChannel) NewAnchorResolutions() (*lnwallet.AnchorResolutions, error) { + if m.anchorResolutions != nil { + return m.anchorResolutions, nil + } - return m.anchorResolutions, nil + return &lnwallet.AnchorResolutions{}, nil } func (m *mockChannel) ForceCloseChan() (*lnwallet.LocalForceCloseSummary, error) { diff --git a/lnwallet/channel.go b/lnwallet/channel.go index c19f58b4..1301d83f 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -6404,16 +6404,33 @@ func (lc *LightningChannel) CompleteCooperativeClose( return closeTx, ourBalance, nil } -// NewAnchorResolutions returns the anchor resolutions for all currently valid -// commitment transactions. Because we have no view on the mempool, we can only -// blindly anchor all of these txes down. -func (lc *LightningChannel) NewAnchorResolutions() ([]*AnchorResolution, +// AnchorResolutions is a set of anchor resolutions that's being used when +// sweeping anchors during local channel force close. +type AnchorResolutions struct { + // Local is the anchor resolution for the local commitment tx. + Local *AnchorResolution + + // Remote is the anchor resolution for the remote commitment tx. + Remote *AnchorResolution + + // RemotePending is the anchor resolution for the remote pending + // commitment tx. The value will be non-nil iff we've created a new + // commitment tx for the remote party which they haven't ACKed yet. + RemotePending *AnchorResolution +} + +// NewAnchorResolutions returns a set of anchor resolutions wrapped in the +// struct AnchorResolutions. Because we have no view on the mempool, we can +// only blindly anchor all of these txes down. Caller needs to check the +// returned values against nil to decide whether there exists an anchor +// resolution for local/remote/pending remote commitment txes. +func (lc *LightningChannel) NewAnchorResolutions() (*AnchorResolutions, error) { lc.Lock() defer lc.Unlock() - var resolutions []*AnchorResolution + resolutions := &AnchorResolutions{} // Add anchor for local commitment tx, if any. localRes, err := NewAnchorResolution( @@ -6422,9 +6439,7 @@ func (lc *LightningChannel) NewAnchorResolutions() ([]*AnchorResolution, if err != nil { return nil, err } - if localRes != nil { - resolutions = append(resolutions, localRes) - } + resolutions.Local = localRes // Add anchor for remote commitment tx, if any. remoteRes, err := NewAnchorResolution( @@ -6433,9 +6448,7 @@ func (lc *LightningChannel) NewAnchorResolutions() ([]*AnchorResolution, if err != nil { return nil, err } - if remoteRes != nil { - resolutions = append(resolutions, remoteRes) - } + resolutions.Remote = remoteRes // Add anchor for remote pending commitment tx, if any. remotePendingCommit, err := lc.channelState.RemoteCommitChainTip() @@ -6451,10 +6464,7 @@ func (lc *LightningChannel) NewAnchorResolutions() ([]*AnchorResolution, if err != nil { return nil, err } - - if remotePendingRes != nil { - resolutions = append(resolutions, remotePendingRes) - } + resolutions.RemotePending = remotePendingRes } return resolutions, nil diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 85ccc305..6de8b2ac 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -917,14 +917,19 @@ func testForceClose(t *testing.T, testCase *forceCloseTestCase) { } // Check the pre-confirmation resolutions. - resList, err := aliceChannel.NewAnchorResolutions() + res, err := aliceChannel.NewAnchorResolutions() if err != nil { t.Fatalf("pre-confirmation resolution error: %v", err) } - if len(resList) != 2 { - t.Fatal("expected two resolutions") - } + // Check we have the expected anchor resolutions. + require.NotNil(t, res.Local, "expected local anchor resolution") + require.NotNil(t, + res.Remote, "expected remote anchor resolution", + ) + require.Nil(t, + res.RemotePending, "expected no anchor resolution", + ) } // The SelfOutputSignDesc should be non-nil since the output to-self is