eclair icon indicating copy to clipboard operation
eclair copied to clipboard

Add support for Peerswap atomic swap based local liquidity management protocol

Open remyers opened this issue 3 years ago • 4 comments

This PR implements the draft PeerSwap protocol .

We create a SwapRegister actor at setup which will spawn swap sender actors in response to user requests. The SwapRegister will also spawn swap receiver actors in response to a swap requests received from peers. The SwapRegister forwards swap messages based on their swapId field to the corresponding local swap actor or remote peer. When a swap actor has completed it simply stops which triggers the SwapRegister to remove it.

Project TODOs:

  • [X] SwapInSender actor and happy path tests
  • [X] SwapInReceiver actor and happy path tests
  • [x] SwapOutSender actor and happy path tests
  • [x] SwapOutReceiver actor and happy path tests
  • [X] SwapIn sender/receiver cross actor tests
  • [x] SwapOut sender/receiver cross actor tests
  • [x] CLI for SwapInSender
  • [ ] CLI and/or configuration for SwapInReceiver
  • [x] CLI for SwapOutSender
  • [x] CLI and/or configuration for SwapOutReceiver
  • [x] happy path signet testing with CLN plugin (bitcoin only)
  • [ ] persist swap info to a database before committing onchain txs or offchain payments
  • [ ] adjust timeouts and timeout behavior to ensure safe operation of the protocol
  • [ ] convert implementation into a Eclair plug-in

Protocol implementation TODOs:

  • [ ] Only a single swap should be active on a given channel at one time.
  • [ ] Channels should fail updates that would remove liquidity needed for an active swap.

PeerSwap specification suggestions/questions to discuss:

  • [ ] custom byte encoded message rather than json #1
  • [ ] use 32-byte channel ids instead of short channel ids #2
  • [ ] nit: rename “OpeningTxBroadcasted” to grammatically correct “OpeningTxBroadcast” #3
  • [ ] send a signature instead of a private key for cooperative closes #4

remyers avatar Jul 05 '22 09:07 remyers

My latest push is rebased on master (from c1daaf3a), cleans up the commit history and has been tested on signet between two eclair nodes.

remyers avatar Aug 30 '22 14:08 remyers

Codecov Report

Merging #2342 (1f14ed1) into master (fb6eb48) will decrease coverage by 0.59%. The diff coverage is 65.87%.

@@            Coverage Diff             @@
##           master    #2342      +/-   ##
==========================================
- Coverage   84.79%   84.19%   -0.60%     
==========================================
  Files         199      209      +10     
  Lines       15584    16081     +497     
  Branches      662      638      -24     
==========================================
+ Hits        13214    13540     +326     
- Misses       2370     2541     +171     
Impacted Files Coverage Δ
...r-core/src/main/scala/fr/acinq/eclair/Eclair.scala 49.73% <0.00%> (-0.82%) :arrow_down:
...la/fr/acinq/eclair/transactions/Transactions.scala 96.04% <0.00%> (-0.77%) :arrow_down:
...in/scala/fr/acinq/eclair/swap/SwapInReceiver.scala 52.50% <52.50%> (ø)
...ala/fr/acinq/eclair/swap/LocalSwapKeyManager.scala 52.94% <52.94%> (ø)
...main/scala/fr/acinq/eclair/swap/SwapInSender.scala 54.48% <54.48%> (ø)
...-core/src/main/scala/fr/acinq/eclair/io/Peer.scala 88.70% <60.00%> (-0.64%) :arrow_down:
.../main/scala/fr/acinq/eclair/swap/SwapHelpers.scala 71.05% <71.05%> (ø)
...main/scala/fr/acinq/eclair/swap/SwapRegister.scala 75.60% <75.60%> (ø)
...q/eclair/wire/protocol/PeerSwapMessageCodecs.scala 82.50% <82.50%> (ø)
.../scala/fr/acinq/eclair/swap/SwapTransactions.scala 98.03% <98.03%> (ø)
... and 16 more

codecov-commenter avatar Aug 30 '22 14:08 codecov-commenter

Very nice work, I think you clearly understood how to architect such a system: the flow between actors looks good.

The gist of my comments for this first review is that we should really focus this PR to be an MVP, as simple as possible. I think we can remove some nice-to-haves that can come later which would simplify the number of events that need to be handled, and will let us focus on the core of the peer swap protocol. This is important to ensure we don't have a bug in the core protocol as it could lead to loss of funds. I know it's tempting to make things better whenever you see a potential improvement (such as making state checks before accepting swaps), but once we have an MVP we can add those later (just keep a list somewhere of all the improvements you want to add in follow-up PRs). Otherwise the PR will be cluttered with a lot of comments on non-critical parts, and we may miss more important protocol issues.

Thanks for the detailed review so far and helpful advice. I agree that it make sense to strip it down as much as possible to the critical protocol elements. Now that the basic architecture appears correct, I have gone ahead and finished off the protocol by adding the remaining states necessary to support the swap-out workflow. This workflow is the same as swap-in but begins with a Lightning fee payment from the swap initiator instead of a premium paid by the on-chain (maker) actor.

I will now look at incorporating your SwapRegister suggestions as a prelude to persisting the swap states to a DB.

remyers avatar Sep 14 '22 09:09 remyers

I'd also like to rename my two main swap actors as follows:

  • SwapInSender -> SwapMaker
  • SwapInReceiver -> SwapTaker

The maker actor is the one that swaps on-chain UTXO value via the opening tx for off-chain Lightning liquidity. The taker actor always swaps off-chain Lightning liquidity for on-chain UTXO value. This naming would be more consistent with the terminology used in the current PeerSwap specification and is a necessary change now that my SwapIn* actors participate in both swap-in and swap-out workflows.

remyers avatar Sep 14 '22 09:09 remyers

Closing this PR and opening #2496 which moves this feature into an Eclair Plugin with a cleaner commit history.

remyers avatar Nov 21 '22 08:11 remyers