Skip to content

Commit

Permalink
lntest+itest: remove the usage of ht.AssertActiveHtlcs
Browse files Browse the repository at this point in the history 
The method `AssertActiveHtlcs` is now removed due to it's easy to be
misused. To assert a given htlc, use `AssertOutgoingHTLCActive` and
`AssertIncomingHTLCActive` instead for ensuring the HTLC exists in the
right direction. Although often the case `AssertNumActiveHtlcs` would be
enough as it implicitly checks the forwarding behavior for an
intermediate node by asserting there are always num_payment*2 HTLCs.
  • Loading branch information
@yyforyongyu
yyforyongyu committed Dec 3, 2024
1 parent 68fcba8 commit 2970aa7
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 92 deletions.
8 changes: 6 additions & 2 deletions itest/lnd_hold_invoice_force_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,12 @@ func testHoldInvoiceForceClose(ht *lntest.HarnessTest) {

// Once the HTLC has cleared, alice and bob should both have a single
// htlc locked in.
ht.AssertActiveHtlcs(alice, payHash[:])
ht.AssertActiveHtlcs(bob, payHash[:])
//
// Alice should have one outgoing HTLCs on channel Alice -> Bob.
ht.AssertOutgoingHTLCActive(alice, chanPoint, payHash[:])

// Bob should have one incoming HTLC on channel Alice -> Bob.
ht.AssertIncomingHTLCActive(bob, chanPoint, payHash[:])

// Get our htlc expiry height and current block height so that we
// can mine the exact number of blocks required to expire the htlc.
Expand Down
149 changes: 111 additions & 38 deletions itest/lnd_multi-hop_force_close_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,18 @@ func runLocalClaimOutgoingHTLC(ht *lntest.HarnessTest,
}
ht.SendPaymentAssertInflight(alice, req)

// Verify that all nodes in the path now have two HTLC's with the
// proper parameters.
ht.AssertActiveHtlcs(alice, dustPayHash, payHash)
ht.AssertActiveHtlcs(bob, dustPayHash, payHash)
ht.AssertActiveHtlcs(carol, dustPayHash, payHash)
// At this point, all 3 nodes should now have an active channel with
// the created HTLC pending on all of them.
//
// Alice should have two outgoing HTLCs on channel Alice -> Bob.
ht.AssertNumActiveHtlcs(alice, 2)

// Bob should have two incoming HTLCs on channel Alice -> Bob, and two
// outgoing HTLCs on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(bob, 4)

// Carol should have two incoming HTLCs on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(carol, 2)

// We'll now mine enough blocks to trigger Bob's force close the
// channel Bob=>Carol due to the fact that the HTLC is about to
Expand Down Expand Up @@ -364,7 +371,7 @@ func runLocalClaimOutgoingHTLC(ht *lntest.HarnessTest,
// At this point, Bob should have canceled backwards the dust HTLC that
// we sent earlier. This means Alice should now only have a single HTLC
// on her channel.
ht.AssertActiveHtlcs(alice, payHash)
ht.AssertNumActiveHtlcs(alice, 1)

// With the closing transaction confirmed, we should expect Bob's HTLC
// timeout transaction to be offered to the sweeper due to the expiry
Expand Down Expand Up @@ -651,9 +658,18 @@ func runMultiHopReceiverPreimageClaim(ht *lntest.HarnessTest,

// At this point, all 3 nodes should now have an active channel with
// the created HTLC pending on all of them.
ht.AssertActiveHtlcs(alice, payHash[:])
ht.AssertActiveHtlcs(bob, payHash[:])
ht.AssertActiveHtlcs(carol, payHash[:])
// At this point, all 3 nodes should now have an active channel with
// the created HTLCs pending on all of them.
//
// Alice should have one outgoing HTLCs on channel Alice -> Bob.
ht.AssertNumActiveHtlcs(alice, 1)

// Bob should have one incoming HTLC on channel Alice -> Bob, and one
// outgoing HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(bob, 2)

// Carol should have one incoming HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(carol, 1)

// Wait for Carol to mark invoice as accepted. There is a small gap to
// bridge between adding the htlc to the channel and executing the exit
Expand Down Expand Up @@ -1010,11 +1026,20 @@ func runLocalForceCloseBeforeHtlcTimeout(ht *lntest.HarnessTest,
}
ht.SendPaymentAssertInflight(alice, req)

// Once the HTLC has cleared, all channels in our mini network should
// have the it locked in.
ht.AssertActiveHtlcs(alice, payHash)
ht.AssertActiveHtlcs(bob, payHash)
ht.AssertActiveHtlcs(carol, payHash)
// At this point, all 3 nodes should now have an active channel with
// the created HTLC pending on all of them.
// At this point, all 3 nodes should now have an active channel with
// the created HTLCs pending on all of them.
//
// Alice should have one outgoing HTLC on channel Alice -> Bob.
ht.AssertNumActiveHtlcs(alice, 1)

// Bob should have one incoming HTLC on channel Alice -> Bob, and one
// outgoing HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(bob, 2)

// Carol should have one incoming HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(carol, 1)

// Now that all parties have the HTLC locked in, we'll immediately
// force close the Bob -> Carol channel. This should trigger contract
Expand Down Expand Up @@ -1339,11 +1364,18 @@ func runRemoteForceCloseBeforeHtlcTimeout(ht *lntest.HarnessTest,
}
ht.SendPaymentAssertInflight(alice, req)

// Once the HTLC has cleared, all the nodes in our mini network should
// show that the HTLC has been locked in.
ht.AssertActiveHtlcs(alice, payHash[:])
ht.AssertActiveHtlcs(bob, payHash[:])
ht.AssertActiveHtlcs(carol, payHash[:])
// At this point, all 3 nodes should now have an active channel with
// the created HTLCs pending on all of them.
//
// Alice should have one outgoing HTLC on channel Alice -> Bob.
ht.AssertNumActiveHtlcs(alice, 1)

// Bob should have one incoming HTLC on channel Alice -> Bob, and one
// outgoing HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(bob, 2)

// Carol should have one incoming HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(carol, 1)

// At this point, we'll now instruct Carol to force close the tx. This
// will let us exercise that Bob is able to sweep the expired HTLC on
Expand Down Expand Up @@ -1600,9 +1632,18 @@ func runLocalClaimIncomingHTLC(ht *lntest.HarnessTest,

// At this point, all 3 nodes should now have an active channel with
// the created HTLC pending on all of them.
ht.AssertActiveHtlcs(alice, payHash[:])
ht.AssertActiveHtlcs(bob, payHash[:])
ht.AssertActiveHtlcs(carol, payHash[:])
// At this point, all 3 nodes should now have an active channel with
// the created HTLCs pending on all of them.
//
// Alice should have one outgoing HTLC on channel Alice -> Bob.
ht.AssertNumActiveHtlcs(alice, 1)

// Bob should have one incoming HTLC on channel Alice -> Bob, and one
// outgoing HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(bob, 2)

// Carol should have one incoming HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(carol, 1)

// Wait for carol to mark invoice as accepted. There is a small gap to
// bridge between adding the htlc to the channel and executing the exit
Expand Down Expand Up @@ -1903,9 +1944,18 @@ func runLocalClaimIncomingHTLCLeased(ht *lntest.HarnessTest,

// At this point, all 3 nodes should now have an active channel with
// the created HTLC pending on all of them.
ht.AssertActiveHtlcs(alice, payHash[:])
ht.AssertActiveHtlcs(bob, payHash[:])
ht.AssertActiveHtlcs(carol, payHash[:])
// At this point, all 3 nodes should now have an active channel with
// the created HTLCs pending on all of them.
//
// Alice should have one outgoing HTLC on channel Alice -> Bob.
ht.AssertNumActiveHtlcs(alice, 1)

// Bob should have one incoming HTLC on channel Alice -> Bob, and one
// outgoing HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(bob, 2)

// Carol should have one incoming HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(carol, 1)

// Wait for carol to mark invoice as accepted. There is a small gap to
// bridge between adding the htlc to the channel and executing the exit
Expand Down Expand Up @@ -2257,10 +2307,17 @@ func runLocalPreimageClaim(ht *lntest.HarnessTest,
ht.SendPaymentAssertInflight(alice, req)

// At this point, all 3 nodes should now have an active channel with
// the created HTLC pending on all of them.
ht.AssertActiveHtlcs(alice, payHash[:])
ht.AssertActiveHtlcs(bob, payHash[:])
ht.AssertActiveHtlcs(carol, payHash[:])
// the created HTLCs pending on all of them.
//
// Alice should have one outgoing HTLC on channel Alice -> Bob.
ht.AssertNumActiveHtlcs(alice, 1)

// Bob should have one incoming HTLC on channel Alice -> Bob, and one
// outgoing HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(bob, 2)

// Carol should have one incoming HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(carol, 1)

// Wait for carol to mark invoice as accepted. There is a small gap to
// bridge between adding the htlc to the channel and executing the exit
Expand Down Expand Up @@ -2540,10 +2597,17 @@ func runLocalPreimageClaimLeased(ht *lntest.HarnessTest,
ht.SendPaymentAssertInflight(alice, req)

// At this point, all 3 nodes should now have an active channel with
// the created HTLC pending on all of them.
ht.AssertActiveHtlcs(alice, payHash[:])
ht.AssertActiveHtlcs(bob, payHash[:])
ht.AssertActiveHtlcs(carol, payHash[:])
// the created HTLCs pending on all of them.
//
// Alice should have one outgoing HTLC on channel Alice -> Bob.
ht.AssertNumActiveHtlcs(alice, 1)

// Bob should have one incoming HTLC on channel Alice -> Bob, and one
// outgoing HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(bob, 2)

// Carol should have one incoming HTLC on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(carol, 1)

// Wait for carol to mark invoice as accepted. There is a small gap to
// bridge between adding the htlc to the channel and executing the exit
Expand Down Expand Up @@ -2997,11 +3061,20 @@ func runHtlcAggregation(ht *lntest.HarnessTest,
ht.SendPaymentAssertInflight(carol, req)
}

// At this point, all 3 nodes should now the HTLCs active on their
// channels.
ht.AssertActiveHtlcs(alice, payHashes...)
ht.AssertActiveHtlcs(bob, payHashes...)
ht.AssertActiveHtlcs(carol, payHashes...)
// At this point, all 3 nodes should now have an active channel with
// the created HTLCs pending on all of them.
//
// Alice sent numInvoices and received numInvoices payments, she should
// have numInvoices*2 HTLCs.
ht.AssertNumActiveHtlcs(alice, numInvoices*2)

// Bob should have 2*numInvoices HTLCs on channel Alice -> Bob, and
// numInvoices*2 HTLCs on channel Bob -> Carol.
ht.AssertNumActiveHtlcs(bob, numInvoices*4)

// Carol sent numInvoices and received numInvoices payments, she should
// have numInvoices*2 HTLCs.
ht.AssertNumActiveHtlcs(carol, numInvoices*2)

// Wait for Alice and Carol to mark the invoices as accepted. There is
// a small gap to bridge between adding the htlc to the channel and
Expand Down
52 changes: 0 additions & 52 deletions lntest/harness_assertion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1314,58 +1314,6 @@ func (h *HarnessTest) AssertNumActiveHtlcs(hn *node.HarnessNode, num int) {
hn.Name())
}

// AssertActiveHtlcs makes sure the node has the _exact_ HTLCs matching
// payHashes on _all_ their channels.
func (h *HarnessTest) AssertActiveHtlcs(hn *node.HarnessNode,
payHashes ...[]byte) {

err := wait.NoError(func() error {
// We require the RPC call to be succeeded and won't wait for
// it as it's an unexpected behavior.
req := &lnrpc.ListChannelsRequest{}
nodeChans := hn.RPC.ListChannels(req)

for _, ch := range nodeChans.Channels {
// Record all payment hashes active for this channel.
htlcHashes := make(map[string]struct{})

for _, htlc := range ch.PendingHtlcs {
h := hex.EncodeToString(htlc.HashLock)
_, ok := htlcHashes[h]
if ok {
return fmt.Errorf("duplicate HashLock "+
"in PendingHtlcs: %v",
ch.PendingHtlcs)
}
htlcHashes[h] = struct{}{}
}

// Channel should have exactly the payHashes active.
if len(payHashes) != len(htlcHashes) {
return fmt.Errorf("node [%s:%x] had %v "+
"htlcs active, expected %v",
hn.Name(), hn.PubKey[:],
len(htlcHashes), len(payHashes))
}

// Make sure all the payHashes are active.
for _, payHash := range payHashes {
h := hex.EncodeToString(payHash)
if _, ok := htlcHashes[h]; ok {
continue
}

return fmt.Errorf("node [%s:%x] didn't have: "+
"the payHash %v active", hn.Name(),
hn.PubKey[:], h)
}
}

return nil
}, DefaultTimeout)
require.NoError(h, err, "timeout checking active HTLCs")
}

// AssertIncomingHTLCActive asserts the node has a pending incoming HTLC in the
// given channel. Returns the HTLC if found and active.
func (h *HarnessTest) AssertIncomingHTLCActive(hn *node.HarnessNode,
Expand Down

0 comments on commit 2970aa7

Please sign in to comment.