Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wallet: PSBT channel funding #4079

Merged
merged 9 commits into from Mar 31, 2020
Merged

Conversation

@guggero
Copy link
Collaborator

@guggero guggero commented Mar 13, 2020

Fixes #3085 and #3936.

This PR introduces channel funding through PSBTs! With this, it is possible to open one or more channels from lnd without it needing to have a wallet balance.

@guggero guggero added this to the 0.10.0 milestone Mar 17, 2020
@guggero guggero added this to In Progress in v0.10.0-beta via automation Mar 17, 2020
@Roasbeef Roasbeef added the v0.10 label Mar 17, 2020
@Roasbeef Roasbeef self-requested a review Mar 17, 2020
Copy link
Member

@Roasbeef Roasbeef left a comment

Completed an initial pass. Upon first glance, it feel like the new internal flow to resume/suspend a funding flow to wait for PBST information could be made a bit more streamlined. There's a lot of mutation where instead perhaps we can apply our changes directly to a state machine using pre-defined methods. It also isn't clear to me why the user needs to register their shim before hand, compared to just passing it in with OpenChannel.

go.mod Show resolved Hide resolved
lnrpc/rpc.proto Show resolved Hide resolved
lnrpc/rpc.proto Show resolved Hide resolved
lnrpc/rpc.proto Show resolved Hide resolved
lnrpc/rpc.proto Outdated Show resolved Hide resolved
lnwallet/wallet.go Outdated Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
cmd/lncli/psbt.go Outdated Show resolved Hide resolved
cmd/lncli/psbt.go Outdated Show resolved Hide resolved
v0.10.0-beta automation moved this from In Progress to Review In Progress Mar 18, 2020
@guggero guggero force-pushed the guggero:psbt-chanfunding branch 4 times, most recently from 3f67a91 to 7ae75b9 Mar 18, 2020
@guggero guggero changed the title [WIP]: PSBT channel funding wallet: PSBT channel funding Mar 23, 2020
@guggero guggero marked this pull request as ready for review Mar 23, 2020
@guggero guggero requested a review from halseth as a code owner Mar 23, 2020
@guggero guggero requested a review from Roasbeef Mar 23, 2020
@guggero guggero force-pushed the guggero:psbt-chanfunding branch 5 times, most recently from eeef1e5 to fc7e062 Mar 23, 2020
@Roasbeef Roasbeef requested review from wpaulino and removed request for halseth Mar 24, 2020
Copy link
Member

@Roasbeef Roasbeef left a comment

The control flow in this version is much easier to follow IMO! FWIW, it now more closely resembles the napkin sketch I made for a flow like this when I was first designing the chanfunding package. No major outstanding comments, just a few simplifications/fixes here and there. I also really dig the new docs added, will be sure to give that a spin myself before I LGTM the PR as a whole.

lnrpc/rpc.proto Outdated Show resolved Hide resolved
lnrpc/rpc.proto Show resolved Hide resolved
lnwallet/chanfunding/psbt_assembler.go Outdated Show resolved Hide resolved
lnwallet/chanfunding/psbt_assembler.go Show resolved Hide resolved
lnwallet/chanfunding/psbt_assembler.go Show resolved Hide resolved
fundingmanager.go Show resolved Hide resolved
rpcserver.go Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
lntest/itest/psbt.go Show resolved Hide resolved
@guggero guggero force-pushed the guggero:psbt-chanfunding branch 2 times, most recently from 26eeb98 to 9835b11 Mar 25, 2020
@guggero
Copy link
Collaborator Author

@guggero guggero commented Mar 25, 2020

I further cleaned up the logic of the PR, thanks for the suggestions, @Roasbeef.
I also added two new commits that fix problems that I found while doing some manual tests.

@guggero guggero requested a review from Roasbeef Mar 25, 2020
@guggero guggero force-pushed the guggero:psbt-chanfunding branch 2 times, most recently from a8fc623 to befb099 Mar 30, 2020
@guggero
Copy link
Collaborator Author

@guggero guggero commented Mar 30, 2020

About the logs: Yes, that is too much. There was still an error in there that if the remote peer canceled the reservation, we also tried to cancel it back, which resulted in additional errors.
I fixed the error and demoted some messages to debug.

@guggero guggero force-pushed the guggero:psbt-chanfunding branch 2 times, most recently from fc956c7 to c79e221 Mar 30, 2020
@guggero guggero requested review from wpaulino and Roasbeef Mar 30, 2020
Copy link
Collaborator

@wpaulino wpaulino left a comment

Just a few more nits, but this is pretty much good to land by me!

docs/psbt.md Outdated Show resolved Hide resolved
docs/psbt.md Outdated Show resolved Hide resolved
docs/psbt.md Show resolved Hide resolved
docs/psbt.md Outdated Show resolved Hide resolved
cmd/lncli/cmd_open_channel.go Show resolved Hide resolved
@Roasbeef Roasbeef moved this from Review In Progress to Merge Queue in v0.10.0-beta Mar 30, 2020
Copy link
Member

@Roasbeef Roasbeef left a comment

LGTM 🧿

guggero added 7 commits Mar 31, 2020
We add a new funding assembler and intent type that handle channel
funding through the use of a PSBT. The PsbtIntent is in itself a
simple state machine that can be stepped through the process of
assembling the required information for the funding output, verifying
a user supplied PSBT for correctness, accepting a fully signed PSBT
and then assembling the funding wire message.
In case the funding manager detects that a funding flow is requested
to be executed with the help of a PsbtIntent, the normal channel
negotiation with the remote peer is interrupted, as soon as the
accept_channel message was received. With the remote peer's funding
multisig key and our local key, we can derive the funding output
script and its address. This is enough to start the PSBT funding
and signing process which the user will do externally to the daemon.
A PSBT funding flow consists of multiple steps. We add new RPC
messages that can trigger the underlying state machine to transition
to a new state. We also add new response messages that tell the
API user what the current state is.
This is a pure code move!
We add a new flag --psbt to the openchannel command which triggers
an interactive conversation between the command line and the user.
@guggero guggero force-pushed the guggero:psbt-chanfunding branch from c79e221 to 2a03be1 Mar 31, 2020
@guggero
Copy link
Collaborator Author

@guggero guggero commented Mar 31, 2020

Nits fixed, commit dates adjusted and rebased.

guggero added 2 commits Mar 31, 2020
@guggero guggero force-pushed the guggero:psbt-chanfunding branch from 2a03be1 to 2f37155 Mar 31, 2020
@wpaulino wpaulino merged commit f996203 into lightningnetwork:master Mar 31, 2020
1 of 2 checks passed
1 of 2 checks passed
coverage/coveralls Coverage decreased (-0.2%) to 62.805%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
v0.10.0-beta automation moved this from Merge Queue to Done Mar 31, 2020
@guggero guggero deleted the guggero:psbt-chanfunding branch Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v0.10.0-beta
  
Done
Linked issues

Successfully merging this pull request may close these issues.

3 participants