diff --git a/htlcswitch/mailbox.go b/htlcswitch/mailbox.go index dd5b4a64..66398e29 100644 --- a/htlcswitch/mailbox.go +++ b/htlcswitch/mailbox.go @@ -12,9 +12,15 @@ import ( "github.com/lightningnetwork/lnd/lnwire" ) -// ErrMailBoxShuttingDown is returned when the mailbox is interrupted by a -// shutdown request. -var ErrMailBoxShuttingDown = errors.New("mailbox is shutting down") +var ( + // ErrMailBoxShuttingDown is returned when the mailbox is interrupted by + // a shutdown request. + ErrMailBoxShuttingDown = errors.New("mailbox is shutting down") + + // ErrPacketAlreadyExists signals that an attempt to add a packet failed + // because it already exists in the mailbox. + ErrPacketAlreadyExists = errors.New("mailbox already has packet") +) // MailBox is an interface which represents a concurrent-safe, in-order // delivery queue for messages from the network and also from the main switch. @@ -564,7 +570,7 @@ func (m *memoryMailBox) AddPacket(pkt *htlcPacket) error { case *lnwire.UpdateFulfillHTLC, *lnwire.UpdateFailHTLC: if _, ok := m.pktIndex[pkt.inKey()]; ok { m.pktCond.L.Unlock() - return nil + return ErrPacketAlreadyExists } entry := m.htlcPkts.PushBack(pkt) @@ -577,7 +583,7 @@ func (m *memoryMailBox) AddPacket(pkt *htlcPacket) error { case *lnwire.UpdateAddHTLC: if _, ok := m.addIndex[pkt.inKey()]; ok { m.pktCond.L.Unlock() - return nil + return ErrPacketAlreadyExists } entry := m.addPkts.PushBack(&pktWithExpiry{ diff --git a/htlcswitch/mailbox_test.go b/htlcswitch/mailbox_test.go index 999468dc..655caaff 100644 --- a/htlcswitch/mailbox_test.go +++ b/htlcswitch/mailbox_test.go @@ -44,7 +44,10 @@ func TestMailBoxCouriers(t *testing.T) { } sentPackets[i] = pkt - mailBox.AddPacket(pkt) + err := mailBox.AddPacket(pkt) + if err != nil { + t.Fatalf("unable to add packet: %v", err) + } } // Next, we'll do the same, but this time adding wire messages. @@ -495,6 +498,47 @@ func TestMailBoxAddExpiry(t *testing.T) { ctx.checkFails(secondBatch) } +// TestMailBoxDuplicateAddPacket asserts that the mailbox returns an +// ErrPacketAlreadyExists failure when two htlcPackets are added with identical +// incoming circuit keys. +func TestMailBoxDuplicateAddPacket(t *testing.T) { + t.Parallel() + + mailBox := newMemoryMailBox(&mailBoxConfig{ + clock: clock.NewDefaultClock(), + }) + mailBox.Start() + defer mailBox.Stop() + + addTwice := func(t *testing.T, pkt *htlcPacket) { + // The first add should succeed. + err := mailBox.AddPacket(pkt) + if err != nil { + t.Fatalf("unable to add packet: %v", err) + } + + // Adding again with the same incoming circuit key should fail. + err = mailBox.AddPacket(pkt) + if err != ErrPacketAlreadyExists { + t.Fatalf("expected ErrPacketAlreadyExists, got: %v", err) + } + } + + // Assert duplicate AddPacket calls fail for all types of HTLCs. + addTwice(t, &htlcPacket{ + incomingHTLCID: 0, + htlc: &lnwire.UpdateAddHTLC{}, + }) + addTwice(t, &htlcPacket{ + incomingHTLCID: 1, + htlc: &lnwire.UpdateFulfillHTLC{}, + }) + addTwice(t, &htlcPacket{ + incomingHTLCID: 2, + htlc: &lnwire.UpdateFailHTLC{}, + }) +} + // TestMailOrchestrator asserts that the orchestrator properly buffers packets // for channels that haven't been made live, such that they are delivered // immediately after BindLiveShortChanID. It also tests that packets are delivered