lnrpc+lnwallet+funding: implement new RPCs to enable external channel funding#3722
Conversation
Roasbeef
requested review from
guggero and
wpaulino
and removed request for
cfromknecht and
halseth
Roasbeef
added
funding
|
Oh also only the last two commits are new. |
guggero
left a comment
guggero
left a comment
There was a problem hiding this comment.
Very nice to see that an involved change like this has such a small API surface.
Roasbeef
force-pushed
the
external-funding-rpc
branch
from
4e1c104 to
dc92f9f
Compare
|
Updated to be based off of master, won't build yet though until the next push. |
Roasbeef
force-pushed
the
external-funding-rpc
branch
from
dc92f9f to
4a01028
Compare
|
Will add an itest, then this should be ready for final review. |
guggero
left a comment
guggero
left a comment
There was a problem hiding this comment.
Looks almost ready to me, great work! I think an itest would be very valuable indeed.
The RPC API looks good to me, it can easily be extended (with the oneof in place) but is quite minimal for this first iteration.
The requested changes are around input validation only.
Roasbeef
force-pushed
the
external-funding-rpc
branch
3 times, most recently
from
2600450 to
d25e32f
Compare
|
New integration tests pushed! Along the way, I had to copy over the |
guggero
left a comment
guggero
left a comment
There was a problem hiding this comment.
Nice itest!
The linter complains but apart from that, the PR looks good to me. The other comments are just nits.
Roasbeef
force-pushed
the
external-funding-rpc
branch
2 times, most recently
from
09c95d7 to
b04ee32
Compare
halseth
left a comment
halseth
left a comment
There was a problem hiding this comment.
Looks pretty good to me! Only main comment is about having separate RPCs for the funding state step instead of dealing with oneofs.
There was a problem hiding this comment.
a more realistic test would be to publish the funding tx first after the commitment was properly signed
There was a problem hiding this comment.
Sure but we don't have the capabilities to do something like that easily using the current RPC system for both lnd and the miner. There's an internal memWallet in the main btcd harness we can expose for this, but this set gives us a full run through either way. This method only broadcasts the transaction, it's still mined as normal in the regular basic funding tests.
There was a problem hiding this comment.
What's the rational behind having a oneof here, instead of just two RPC endpoints? Seems simpler not having to deal with the oneonf, and the switch in the implementation.
There was a problem hiding this comment.
That would mean a new RPC method for each new funding variation we think of in the future. This instead presents a set of distinct triggers similar to how we do things in the contract court.
Roasbeef
force-pushed
the
external-funding-rpc
branch
from
b04ee32 to
34ef876
Compare
Roasbeef
force-pushed
the
external-funding-rpc
branch
2 times, most recently
from
5a85a64 to
8bffcc7
Compare
…ng modifiers In this commit, we start to expose some of the new external funding functionality over the RPC interface. First, we add a new `funding_shim` field to the regular `OpenChannel` method. This can be used by a caller to express that certain parameters of the funding flow have already been negotiated outside the protocol, and should be used instead. For example, a shim can be provided to use a particular key for the commitment key (ideally cold) rather than use one this is generated by the wallet as normal, or signal that signing will be carried out in an interactive manner (PSBT based). Next, we add a brand new method: `FundingStateStep`. FundingStateStep is an advanced funding related call that allows the caller to either execute some preparatory steps for a funding workflow, or manually progress a funding workflow. The primary way a funding flow is identified is via its pending channel ID. As an example, this method can be used to specify that we're expecting a funding flow for a particular pending channel ID, for which we need to use specific parameters. Alternatively, this can be used to interactively drive PSBT signing for funding for partially complete funding transactions. The new transition methods (funding state machine modifiers) in this commit allow a party to register a funding intent that should be used for a specified incoming pending channel ID. The "responder" to the external channel flow should use this to prep lnd to be able to handle the channel flow properly.
In this commit, we update the `OpenChannel` method to observe the new `funding_shim` field in the main open channel request. If this is specified, and is a channel point shim, then we'll create a custom `chanfunding.Assembler` for the wallet to use in place of the regular funding workflow. With this commit, the "initiator" of an external funding flow can now delegate the remainder of the channel funding workflow to lnd.
In this commit, we implement the currently defined transition methods for the new `FundingStateStep` method. At this point, we're now able to serve the "responder" of the externally initiated channel funding flow by being able to register and cancel a funding flow according to its expected pending channel ID.
Roasbeef
force-pushed
the
external-funding-rpc
branch
from
8bffcc7 to
3de3ec8
Compare
halseth
left a comment
halseth
left a comment
There was a problem hiding this comment.
This PR LGTM now 💯 Excited about the possibilities this opens up 💪
6 participants
In this commit, we start to expose some of the new external funding
functionality over the RPC interface.
First, we add a new
funding_shimfield to the regularOpenChannelmethod. This can be used by a caller to express that certain parameters
of the funding flow have already been negotiated outside the protocol,
and should be used instead. For example, a shim can be provided to use a
particular key for the commitment key (ideally cold) rather than use one
this is generated by the wallet as normal, or signal that signing will
be carried out in an interactive manner (PSBT based).
Next, we add a brand new method:
FundingStateStep. FundingStateStep isan advanced funding related call that allows the caller to either
execute some preparatory steps for a funding workflow, or manually
progress a funding workflow. The primary way a funding flow is
identified is via its pending channel ID. As an example, this method can
be used to specify that we're expecting a funding flow for a particular
pending channel ID, for which we need to use specific parameters.
Alternatively, this can be used to interactively drive PSBT signing for
funding for partially complete funding transactions.
The new transition methods (funding state machine modifiers) in this
commit allow a party to register a funding intent that should be used
for a specified incoming pending channel ID. The "responder" to the
external channel flow should use this to prep lnd to be able to handle
the channel flow properly.
NOTE: this is an early version of the PR without additional tests in order to get early feedback on the proposed cut outs to enable a class of external channel funding useful in channel factory like contexts.
This builds on #3659.