Skip to content

Loop In#34

Merged
Roasbeef merged 11 commits into
lightninglabs:masterfrom
joostjager:loopin-merge
Apr 5, 2019
Merged

Loop In#34
Roasbeef merged 11 commits into
lightninglabs:masterfrom
joostjager:loopin-merge

Conversation

@joostjager
Copy link
Copy Markdown
Contributor

@joostjager joostjager commented Apr 1, 2019

This PR adds loop in functionality to the Lightning Loop client.

Loop in allows a lightning channel to be 'refilled' with on-chain coins. It allows a channel to remain open and offers an alternative over closing and reopening the channel.

High level flow

  1. User generates a secret preimage.
  2. User publishes an on-chain htlc tied to the hash of the secret preimage.
  3. Server picks up confirmation of the on-chain htlc.
  4. Server sends an off-chain lightning payment to the user.
  5. User settles the payment by revealing the secret preimage.
  6. Server learns the preimage and uses this to sweep the on-chain htlc.

Usage

Similar to Loop Out. A new command has been added to the loop command line tool:

loop in <amt>

Also the --external flag can be supplied. In that case step 2 of the flow above is skipped and the user is given the opportunity to publish the htlc manually. An example use of this is to withdraw from an exchange into a lightning channel by looping in.

There is also the --channel flag that allows selection of the channel to loop in to. This functionality is not implemented yet.

Testnet only

Loop in is currently only supported on testnet. There are still several changes required to lnd for it to be used on mainnet.

This commit adds the required code to persist loop in swaps. It also
introduces the file loop.go to which shared code is moved.

Sharing of contract serialization/deserialization code has been
reverted. The prepay fields do not apply to loop in, but were part of
the shared contract struct. Without also adding a migration, it wouldn't
be possible to keep the shared code.

In general it is probably more flexible to keep the contract
serialization code separated between in and out swaps.
Allow user to specify that htlc will be published by an external source.
In that case no internal htlc tx will be published.

To facilitate sending to the htlc address, the swap initiation response
is extended with that address.
Copy link
Copy Markdown
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excited to eventually roll this out on mainnet! Comments from the prior earlier review look to be addressed. After running a few more Loops I noticed two things:

  • Loop In quotes still show "loop out"
  • We mark the Loop In as complete after the HTLC is swept, rather than as soon as the invoice has been settled

Comment thread cmd/loop/loopin.go Outdated
Comment thread cmd/loop/main.go
Comment thread loopin.go Outdated
// TODO: Add miner fee of timeout tx to swap cost balance.
}

// Wait for a final state of the swap invoice. It should either be
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should move this to be above waiting for the HTLC spend, or be concurrent even. I tried another Loop in on testnet now and realized that things are shown as pending until the HTLC is swept on chain. However, from the PoV of the client, the Loop In process is complete once its invoice is settled, as it now has more outbound capacity.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everywhere in loop swaps are only marked complete when all information needed for accounting is there. For loop in on the client, this means the htlc sweep tx needs to be confirmed and the swap invoice either canceled or settled. Also after a restart, a swap that is completed from the user PoV, still needs to be resumed to complete the accounting info.

An intermediate state that the swap transitions to when the invoice is paid could be a solution here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a commit that adds this intermediate state.

@Roasbeef
Copy link
Copy Markdown
Member

Roasbeef commented Apr 2, 2019

The README should also be updated to instruct users to build with the invoicerpc tag as well.

@joostjager joostjager force-pushed the loopin-merge branch 3 times, most recently from b2e7bf4 to db39b37 Compare April 4, 2019 12:08
@joostjager joostjager force-pushed the loopin-merge branch 2 times, most recently from a25b081 to 907a118 Compare April 4, 2019 13:40
@joostjager
Copy link
Copy Markdown
Contributor Author

@Roasbeef ptal

@Roasbeef
Copy link
Copy Markdown
Member

Roasbeef commented Apr 5, 2019

Panic on loop monitor w/ the latest attempting a Loop In:

oroutine 68 [running]:
main.(*swapClientServer).marshallSwap(0xc0001c2290, 0xc00024f530, 0xc0006fe420, 0xb51840, 0xc000343dc0)
        /root/loop/cmd/loopd/swapclient_server.go:122 +0x134
main.(*swapClientServer).Monitor.func1(0x6d5a1c0, 0xed42e0803, 0x14cc500, 0xb680a30070e9430c, 0xaa124c441abd7ade, 0xc887a080307fdaae, 0xd00a71501b250270, 0x102, 0x511c5b616b2f239f, 0x7ef9a4a62c974031, ...)
        /root/loop/cmd/loopd/swapclient_server.go:134 +0x4a
main.(*swapClientServer).Monitor(0xc0001c2290, 0xc000343d20, 0xef9960, 0xc000174a30, 0x0, 0x0)
        /root/loop/cmd/loopd/swapclient_server.go:197 +0x877
github.com/lightninglabs/loop/looprpc._SwapClient_Monitor_Handler(0xc3b860, 0xc0001c2290, 0xef84c0, 0xc0001049c0, 0x14eaa10, 0xc000102c00)
        /root/loop/looprpc/client.pb.go:975 +0x109
google.golang.org/grpc.(*Server).processStreamingRPC(0xc000200180, 0xefc2a0, 0xc00024b800, 0xc000102c00, 0xc0001bf0b0, 0x14b49e0, 0x0, 0x0, 0x0)
        /root/gocode/pkg/mod/google.golang.org/grpc@v1.19.0/server.go:1175 +0xb12
google.golang.org/grpc.(*Server).handleStream(0xc000200180, 0xefc2a0, 0xc00024b800, 0xc000102c00, 0x0)
        /root/gocode/pkg/mod/google.golang.org/grpc@v1.19.0/server.go:1254 +0xcbe
google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc000332030, 0xc000200180, 0xefc2a0, 0xc00024b800, 0xc000102c00)
        /root/gocode/pkg/mod/google.golang.org/grpc@v1.19.0/server.go:690 +0x9f
created by google.golang.org/grpc.(*Server).serveStreams.func1
        /root/gocode/pkg/mod/google.golang.org/grpc@v1.19.0/server.go:688 +0xa1

@Roasbeef
Copy link
Copy Markdown
Member

Roasbeef commented Apr 5, 2019

Nvm, it was due to me having issued a swap using the old DB format.

Comment thread loopin.go
return s.sendUpdate(ctx)
}

// setState updates the swap state and last update timestamp.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also roll in persistState into this method as well. Otherwise, I see a few instances above, where we modify the swap state, but don't persist it to the database.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scratch that, I see it's persisted for the first thing that causes it to break out of the loop.

Comment thread cmd/loopd/view.go
return nil
}

func viewOut(swapClient *loop.Client, chainParams *chaincfg.Params) error {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should instead make this an command for the loop binary, but it isn't blocking and can be done in a follow up.

Copy link
Copy Markdown
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🐝

@Roasbeef Roasbeef merged commit e8005d0 into lightninglabs:master Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants