reservation: reject duplicate reservation entries

Check active and persisted reservations before creating a new state
machine, preserving the existing reservation when a duplicate arrives.

instantout/reservation/manager.go

@@ -2,6 +2,7 @@ package reservation
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 	"sync"
@@ -111,13 +112,28 @@ func (m *Manager) newReservation(ctx context.Context, currentHeight uint32,
 		return nil, err
 	}
 
+	_, err = m.cfg.Store.GetReservation(ctx, reservationID)
+	switch {
+	case err == nil:
+		return nil, ErrReservationAlreadyExists
+
+	case !errors.Is(err, ErrReservationNotFound):
+		return nil, err
+	}
+
 	// Create the reservation state machine. We need to pass in the runCtx
 	// of the reservation manager so that the state machine will keep on
 	// running even if the grpc conte
 	reservationFSM := NewFSM(m.cfg)
 
-	// Add the reservation to the active reservations map.
+	// Add the reservation to the active reservations map. Check the map while
+	// holding the lock as concurrent callers may both have completed the store
+	// lookup above.
 	m.Lock()
+	if _, ok := m.activeReservations[reservationID]; ok {
+		m.Unlock()
+		return nil, ErrReservationAlreadyExists
+	}
 	m.activeReservations[reservationID] = reservationFSM
 	m.Unlock()
 
@@ -146,6 +162,12 @@ func (m *Manager) newReservation(ctx context.Context, currentHeight uint32,
 		fsm.WithWaitForStateOption(time.Second),
 	)
 	if err != nil {
+		m.Lock()
+		if m.activeReservations[reservationID] == reservationFSM {
+			delete(m.activeReservations, reservationID)
+		}
+		m.Unlock()
+
 		if reservationFSM.LastActionError != nil {
 			return nil, fmt.Errorf("error waiting for "+
 				"state: %v, last action error: %v",

instantout/reservation/manager_test.go

@@ -144,6 +144,35 @@ func TestManagerContinuesAfterInvalidNotification(t *testing.T) {
 	require.NoError(t, <-errChan)
 }
 
+// TestManagerRejectsDuplicateReservation verifies that a duplicate server
+// notification cannot replace the active FSM for an existing reservation.
+func TestManagerRejectsDuplicateReservation(t *testing.T) {
+	testContext := newManagerTestContext(t)
+	ctx := t.Context()
+	req := &swapserverrpc.ServerReservationNotification{
+		ReservationId: defaultReservationId[:],
+		Value:         uint64(defaultValue),
+		ServerKey:     defaultPubkeyBytes,
+		Expiry: uint32(testContext.mockLnd.Height) +
+			defaultExpiry,
+	}
+
+	firstFSM, err := testContext.manager.newReservation(
+		ctx, uint32(testContext.mockLnd.Height), req,
+	)
+	require.NoError(t, err)
+
+	secondFSM, err := testContext.manager.newReservation(
+		ctx, uint32(testContext.mockLnd.Height), req,
+	)
+	require.ErrorIs(t, err, ErrReservationAlreadyExists)
+	require.Nil(t, secondFSM)
+	require.Same(
+		t, firstFSM,
+		testContext.manager.activeReservations[defaultReservationId],
+	)
+}
+
 // ManagerTestContext is a helper struct that contains all the necessary
 // components to test the reservation manager.
 type ManagerTestContext struct {

instantout/reservation/store.go

@@ -180,6 +180,10 @@ func (r *SQLStore) GetReservation(ctx context.Context,
 			return nil
 		})
 	if err != nil {
+		if errors.Is(err, sql.ErrNoRows) {
+			return nil, ErrReservationNotFound
+		}
+
 		return nil, err
 	}