Skip to content

Add ability to buy reservations for instant outs from server - #883

Open
sputn1ck wants to merge 9 commits into
lightninglabs:masterfrom
sputn1ck:buy_reservations
Open

Add ability to buy reservations for instant outs from server#883
sputn1ck wants to merge 9 commits into
lightninglabs:masterfrom
sputn1ck:buy_reservations

Conversation

@sputn1ck

@sputn1ck sputn1ck commented Feb 3, 2025

Copy link
Copy Markdown
Member

This pull request introduces several new features and improvements related to reservation management and state handling. The most significant changes include adding a new reservation command, updating the reservation state machine to support client-initiated reservations, and enhancing error handling and timeout configurations.

New Features:

  • Added a new reservation command to create reservations with specified values and expiry times in cmd/loop/reservations.go. [1] [2] [3]
  • Introduced client-initiated reservation handling in instantout/reservation/actions.go and instantout/reservation/fsm.go. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]

Enhancements:

  • Added a default RPC timeout configuration in cmd/loop/main.go.
  • Updated error handling and added timeout configurations for prepayment in instantout/reservation/actions.go. [1] [2] [3]

Testing and Mocking:

  • Updated tests and mock implementations to support new reservation commands and client-initiated reservations in instantout/reservation/actions_test.go. [1] [2] [3]

Code Refactoring:

  • Refactored the reservation manager to handle asynchronous events and maintain current blockchain height in instantout/reservation/manager.go. [1] [2] [3] [4]

These changes improve the flexibility and robustness of the reservation system, allowing for better error handling and support for both server-initiated and client-initiated reservations.

@sputn1ck
sputn1ck marked this pull request as ready for review February 3, 2025 16:36
@sputn1ck sputn1ck self-assigned this Feb 3, 2025
@sputn1ck
sputn1ck force-pushed the buy_reservations branch 2 times, most recently from 9d3f8f7 to c8bd673 Compare April 8, 2025 07:41
@sputn1ck
sputn1ck force-pushed the buy_reservations branch 3 times, most recently from a9b6c87 to 16fd7ed Compare April 17, 2025 09:49
@lightninglabs-deploy

Copy link
Copy Markdown

@bhandras: review reminder
@starius: review reminder
@hieblmi: review reminder

hieblmi added a commit to hieblmi/loop that referenced this pull request Aug 11, 2026
SendPaymentAndPollAccepted and BuildHtlc both run after
PollPaymentAcceptedAction has called LockReservation on every
reservation backing the swap. Their OnRecover transitions pointed
directly to Failed, whose action is fsm.NoOpAction -- so on daemon
restart while in either state, the FSM moved to Failed without ever
unlocking the reservations. The local store kept them in the Locked
state until on-chain expiry (typically tens of hours later), making
them unusable for any subsequent swap. For users who pay for
reservations (PR lightninglabs#883's invoice-requested flow) that is a direct
material loss.

Add an intermediate UnlockReservationsOnRecover state whose action
calls handleErrorAndUnlockReservations and then routes to Failed via
the normal OnError edge. SendPaymentAndPollAccepted.OnRecover and
BuildHtlc.OnRecover now point at this state instead of Failed
directly.

Init.OnRecover -> Failed is left alone because at that point the
InstantOut row has not yet been persisted and no reservation locks
have been taken; there is nothing to clean up. Post-PushPreimage
states (PushPreimage.OnRecover -> PushPreimage, etc.) are also left
alone since they self-loop on recovery rather than terminate.

The cleanup helper itself still derives its context from the caller's
context (see existing handleErrorAndUnlockReservations); fixing that
context-cancel hazard is a separate change.
hieblmi added a commit to hieblmi/loop that referenced this pull request Aug 11, 2026
When loopd is started without --experimental the swap client server's
reservationManager and instantOutManager are nil. ListReservations
already returns codes.Unimplemented in that case; the rest of the
instant-out / reservation RPC family didn't, and would dereference a
nil pointer.

Affected handlers (all of which now return the same Unimplemented
status):

  - ReservationRequest (new in PR lightninglabs#883)
  - ReservationQuote   (new in PR lightninglabs#883)
  - InstantOut
  - InstantOutQuote
  - ListInstantOuts

Without this fix an authenticated caller can crash the daemon by
invoking any of these RPCs against a non-experimental loopd. With
default localhost binding the attack surface is small, but loop is
also commonly fronted by lit / LSP wrappers that expose RPCs to other
internal services, so a single packet is enough for a remote DoS.
hieblmi added a commit to hieblmi/loop that referenced this pull request Aug 11, 2026
SendPaymentAndPollAccepted and BuildHtlc both run after
PollPaymentAcceptedAction has called LockReservation on every
reservation backing the swap. Their OnRecover transitions pointed
directly to Failed, whose action is fsm.NoOpAction -- so on daemon
restart while in either state, the FSM moved to Failed without ever
unlocking the reservations. The local store kept them in the Locked
state until on-chain expiry (typically tens of hours later), making
them unusable for any subsequent swap. For users who pay for
reservations (PR lightninglabs#883's invoice-requested flow) that is a direct
material loss.

Add an intermediate UnlockReservationsOnRecover state whose action
calls handleErrorAndUnlockReservations and then routes to Failed via
the normal OnError edge. SendPaymentAndPollAccepted.OnRecover and
BuildHtlc.OnRecover now point at this state instead of Failed
directly.

Init.OnRecover -> Failed is left alone because at that point the
InstantOut row has not yet been persisted and no reservation locks
have been taken; there is nothing to clean up. Post-PushPreimage
states (PushPreimage.OnRecover -> PushPreimage, etc.) are also left
alone since they self-loop on recovery rather than terminate.

The cleanup helper itself still derives its context from the caller's
context (see existing handleErrorAndUnlockReservations); fixing that
context-cancel hazard is a separate change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants