diff --git a/instantout/reservation/manager.go b/instantout/reservation/manager.go index 0a902c05..930dff7b 100644 --- a/instantout/reservation/manager.go +++ b/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", diff --git a/instantout/reservation/manager_test.go b/instantout/reservation/manager_test.go index 4b01fcfa..0437955e 100644 --- a/instantout/reservation/manager_test.go +++ b/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 { diff --git a/instantout/reservation/store.go b/instantout/reservation/store.go index 117d02c6..72613f9c 100644 --- a/instantout/reservation/store.go +++ b/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 }