diff --git a/channeldb/channel.go b/channeldb/channel.go index 27f8887c6..7189e56af 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -2171,7 +2171,7 @@ func (c *OpenChannel) AdvanceCommitChainTail(fwdPkg *FwdPkg) error { // NextLocalHtlcIndex returns the next unallocated local htlc index. To ensure // this always returns the next index that has been not been allocated, this // will first try to examine any pending commitments, before falling back to the -// last locked-in local commitment. +// last locked-in remote commitment. func (c *OpenChannel) NextLocalHtlcIndex() (uint64, error) { // First, load the most recent commit diff that we initiated for the // remote party. If no pending commit is found, this is not treated as @@ -2187,8 +2187,8 @@ func (c *OpenChannel) NextLocalHtlcIndex() (uint64, error) { return pendingRemoteCommit.Commitment.LocalHtlcIndex, nil } - // Otherwise, fallback to using the local htlc index of our commitment. - return c.LocalCommitment.LocalHtlcIndex, nil + // Otherwise, fallback to using the local htlc index of their commitment. + return c.RemoteCommitment.LocalHtlcIndex, nil } // LoadFwdPkgs scans the forwarding log for any packages that haven't been diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 0906141ac..d7f95df16 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -3158,6 +3158,161 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { assertLinkBandwidth(t, alice.link, aliceStartingBandwidth) } +// TestChannelLinkTrimCircuitsRemoteCommit checks that the switch and link +// don't trim circuits if the ADD is locked in on the remote commitment but +// not on our local commitment. +func TestChannelLinkTrimCircuitsRemoteCommit(t *testing.T) { + t.Parallel() + + const ( + chanAmt = btcutil.SatoshiPerBitcoin * 5 + numHtlcs = 2 + ) + + // We'll start by creating a new link with our chanAmt (5 BTC). + aliceLink, bobChan, batchTicker, start, cleanUp, restore, err := + newSingleLinkTestHarness(chanAmt, 0) + if err != nil { + t.Fatalf("unable to create link: %v", err) + } + + if err := start(); err != nil { + t.Fatalf("unable to start test harness: %v", err) + } + defer cleanUp() + + alice := newPersistentLinkHarness( + t, aliceLink, batchTicker, restore, + ) + + // Compute the static fees that will be used to determine the + // correctness of Alice's bandwidth when forwarding HTLCs. + estimator := chainfee.NewStaticEstimator(6000, 0) + feePerKw, err := estimator.EstimateFeePerKW(1) + if err != nil { + t.Fatalf("unable to query fee estimator: %v", err) + } + + defaultCommitFee := alice.channel.StateSnapshot().CommitFee + htlcFee := lnwire.NewMSatFromSatoshis( + feePerKw.FeeForWeight(input.HTLCWeight), + ) + + // The starting bandwidth of the channel should be exactly the amount + // that we created the channel between her and Bob, minus the commitment + // fee and fee of adding an HTLC. + expectedBandwidth := lnwire.NewMSatFromSatoshis( + chanAmt-defaultCommitFee, + ) - htlcFee + assertLinkBandwidth(t, alice.link, expectedBandwidth) + + // Capture Alice's starting bandwidth to perform later, relative + // bandwidth assertions. + aliceStartingBandwidth := alice.link.Bandwidth() + + // Next, we'll create an HTLC worth 1 BTC that will be used as a dummy + // message for the test. + var mockBlob [lnwire.OnionPacketSize]byte + htlcAmt := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) + _, htlc, _, err := generatePayment(htlcAmt, htlcAmt, 5, mockBlob) + if err != nil { + t.Fatalf("unable to create payment: %v", err) + } + + // Create `numHtlc` htlcPackets and payment circuits that will be used + // to drive the test. All of the packets will use the same dummy HTLC. + addPkts, circuits := genAddsAndCircuits(numHtlcs, htlc) + + // To begin the test, start by committing the circuits for our first two + // HTLCs. + fwdActions := alice.commitCircuits(circuits) + + // Both of these circuits should have successfully added, as this is the + // first attempt to send them. + if len(fwdActions.Adds) != numHtlcs { + t.Fatalf("expected %d circuits to be added", numHtlcs) + } + alice.assertNumPendingNumOpenCircuits(2, 0) + + // Since both were committed successfully, we will now deliver them to + // Alice's link. + for _, addPkt := range addPkts { + if err := alice.link.HandleSwitchPacket(addPkt); err != nil { + t.Fatalf("unable to handle switch packet: %v", err) + } + } + + // Wait until Alice's link has sent both HTLCs via the peer. + alice.checkSent(addPkts) + + // Pass both of the htlcs to Bob. + for i, addPkt := range addPkts { + pkt, ok := addPkt.htlc.(*lnwire.UpdateAddHTLC) + if !ok { + t.Fatalf("unable to add packet") + } + + pkt.ID = uint64(i) + + _, err := bobChan.ReceiveHTLC(pkt) + if err != nil { + t.Fatalf("unable to receive htlc: %v", err) + } + } + + // The resulting bandwidth should reflect that Alice is paying both + // htlc amounts, in addition to both htlc fees. + assertLinkBandwidth(t, alice.link, + aliceStartingBandwidth-numHtlcs*(htlcAmt+htlcFee), + ) + + // Now, initiate a state transition by Alice so that the pending HTLCs + // are locked in. + alice.trySignNextCommitment() + alice.assertNumPendingNumOpenCircuits(2, 2) + + select { + case aliceMsg := <-alice.msgs: + // Pass the commitment signature to Bob. + sig, ok := aliceMsg.(*lnwire.CommitSig) + if !ok { + t.Fatalf("alice did not send commitment signature") + } + + err := bobChan.ReceiveNewCommitment(sig.CommitSig, sig.HtlcSigs) + if err != nil { + t.Fatalf("unable to receive new commitment: %v", err) + } + case <-time.After(time.Second): + } + + // Next, revoke Bob's current commitment and send it to Alice so that we + // can test that Alice's circuits aren't trimmed. + rev, _, err := bobChan.RevokeCurrentCommitment() + if err != nil { + t.Fatalf("unable to revoke current commitment: %v", err) + } + + _, _, _, _, err = alice.channel.ReceiveRevocation(rev) + if err != nil { + t.Fatalf("unable to receive revocation: %v", err) + } + + // Restart Alice's link, which simulates a disconnection with the remote + // peer. + cleanUp = alice.restart(false) + defer cleanUp() + + alice.assertNumPendingNumOpenCircuits(2, 2) + + // Restart the link + switch and check that the number of open circuits + // doesn't change. + cleanUp = alice.restart(true) + defer cleanUp() + + alice.assertNumPendingNumOpenCircuits(2, 2) +} + // TestChannelLinkBandwidthChanReserve checks that the bandwidth available // on the channel link reflects the channel reserve that must be kept // at all times.