From d553c304b2530fa7dd6e4fcadd5fa8c45d7f9038 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 12 Sep 2019 13:38:43 +0200 Subject: [PATCH 1/3] cnct: log resolver type on error --- contractcourt/channel_arbitrator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index 93f369258..47004d1dd 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -1871,8 +1871,8 @@ func (c *ChannelArbitrator) resolveContract(currentContract ContractResolver) { } log.Errorf("ChannelArbitrator(%v): unable to "+ - "progress resolver: %v", - c.cfg.ChanPoint, err) + "progress %T: %v", + c.cfg.ChanPoint, currentContract, err) return } From 3131bc4d6487c5850f7b3fb7efa31c8d568de64b Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Fri, 13 Sep 2019 12:29:42 +0200 Subject: [PATCH 2/3] cnct/test: test outgoing dust htlc resolution --- contractcourt/channel_arbitrator_test.go | 40 ++++++++++++++++++++---- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index 65bfde650..c0f78e62f 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/contractcourt/channel_arbitrator_test.go @@ -496,6 +496,8 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { return nil } + chanArb.cfg.PreimageDB = newMockWitnessBeacon() + chanArb.cfg.Registry = &mockRegistry{} if err := chanArb.Start(); err != nil { t.Fatalf("unable to start ChannelArbitrator: %v", err) @@ -512,16 +514,26 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { chanArb.UpdateContractSignals(signals) // Add HTLC to channel arbitrator. - htlcIndex := uint64(99) htlc := channeldb.HTLC{ Incoming: false, Amt: 10000, - HtlcIndex: htlcIndex, + HtlcIndex: 99, + } + + outgoingDustHtlc := channeldb.HTLC{ + Incoming: false, + Amt: 100, + HtlcIndex: 100, + OutputIndex: -1, + } + + htlcSet := []channeldb.HTLC{ + htlc, outgoingDustHtlc, } htlcUpdates <- &ContractUpdate{ HtlcKey: LocalHtlcSet, - Htlcs: []channeldb.HTLC{htlc}, + Htlcs: htlcSet, } errChan := make(chan error, 1) @@ -607,7 +619,7 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { CommitSet: CommitSet{ ConfCommitKey: &LocalHtlcSet, HtlcSets: map[HtlcSetKey][]channeldb.HTLC{ - LocalHtlcSet: {htlc}, + LocalHtlcSet: htlcSet, }, }, } @@ -617,6 +629,22 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { StateWaitingFullResolution, ) + // We expect an immediate resolution message for the outgoing dust htlc. + // It is not resolvable on-chain. + select { + case msgs := <-resolutions: + if len(msgs) != 1 { + t.Fatalf("expected 1 message, instead got %v", len(msgs)) + } + + if msgs[0].HtlcIndex != outgoingDustHtlc.HtlcIndex { + t.Fatalf("wrong htlc index: expected %v, got %v", + outgoingDustHtlc.HtlcIndex, msgs[0].HtlcIndex) + } + case <-time.After(5 * time.Second): + t.Fatalf("resolution msgs not sent") + } + // htlcOutgoingContestResolver is now active and waiting for the HTLC to // expire. It should not yet have passed it on for incubation. select { @@ -649,9 +677,9 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { t.Fatalf("expected 1 message, instead got %v", len(msgs)) } - if msgs[0].HtlcIndex != htlcIndex { + if msgs[0].HtlcIndex != htlc.HtlcIndex { t.Fatalf("wrong htlc index: expected %v, got %v", - htlcIndex, msgs[0].HtlcIndex) + htlc.HtlcIndex, msgs[0].HtlcIndex) } case <-time.After(5 * time.Second): t.Fatalf("resolution msgs not sent") From be7fc9dd3ecb6ecf16716a5743a5599451f693c8 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 12 Sep 2019 15:31:48 +0200 Subject: [PATCH 3/3] cnct: do not create an action for incoming dust htlcs This commit fixes the 'unable to find incoming resolution' error that occured when trying to resolve incoming htlcs below the dust limit that are not actually present on the commitment tx. --- contractcourt/channel_arbitrator.go | 9 +++++++++ contractcourt/channel_arbitrator_test.go | 9 ++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index 47004d1dd..9008894f7 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -1316,6 +1316,15 @@ func (c *ChannelArbitrator) checkCommitChainActions(height uint32, // either learn of it eventually from the outgoing HTLC, or the sender // will timeout the HTLC. for _, htlc := range htlcs.incomingHTLCs { + // If the HTLC is dust, there is no action to be taken. + if htlc.OutputIndex < 0 { + log.Debugf("ChannelArbitrator(%v): no resolution "+ + "needed for incoming dust htlc=%x", + c.cfg.ChanPoint, htlc.RHash[:]) + + continue + } + log.Tracef("ChannelArbitrator(%v): watching chain to decide "+ "action for incoming htlc=%x", c.cfg.ChanPoint, htlc.RHash[:]) diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index c0f78e62f..7b413669f 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/contractcourt/channel_arbitrator_test.go @@ -527,8 +527,15 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { OutputIndex: -1, } + incomingDustHtlc := channeldb.HTLC{ + Incoming: true, + Amt: 105, + HtlcIndex: 101, + OutputIndex: -1, + } + htlcSet := []channeldb.HTLC{ - htlc, outgoingDustHtlc, + htlc, outgoingDustHtlc, incomingDustHtlc, } htlcUpdates <- &ContractUpdate{