[3/5]: multi: add new AuxFundingController for custom external funding flows #8622
Conversation
Roasbeef
added
funding
|
Important Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Pull reviewers statsStats of the last 30 days for lnd:
|
|
Pushed a few new commits: we'll now include the taproot internal key in the PBST template we return for the PSBT funding flow. |
guggero
force-pushed
the
aux-leaf-fetcher
branch
3 times, most recently
from
a8bfe36
to
e04f50d
Compare
guggero
force-pushed
the
aux-leaf-fetcher
branch
from
e04f50d
to
9f4677d
Compare
guggero
changed the title
guggero
force-pushed
the
aux-leaf-fetcher
branch
from
9f4677d
to
4969b5a
Compare
guggero
force-pushed
the
aux-leaf-fetcher
branch
from
4969b5a
to
5baf204
Compare
guggero
force-pushed
the
aux-leaf-fetcher
branch
from
5baf204
to
11f3626
Compare
guggero
changed the title
guggero
force-pushed
the
aux-funding
branch
2 times, most recently
from
d978f49
to
d43b5f8
Compare
|
Updated this PR to depend on #8660 so we have everything we need in one continuous branch. @ffranr and @GeorgeTsagk please skip the first 6 commits when reviewing this PR! |
In this commit, we add a new abstract message router. Over time, the goal is that this message router replaces the logic we currently have in the readHandler (the giant switch for each message). With this new abstraction, can reduce the responsibilities of the readHandler to *just* reading messages off the wire and handing them off to the msg router. The readHandler no longer needs to know *where* the messages should go, or how they should be dispatched. This will be used in tandem with the new `protofsm` module in an upcoming PR implementing the new rbf-coop close.
Over time with this, we should be able to significantly reduce the size of the peer.Brontide struct as we only need all those deps as the peer needs to recognize and handle each incoming wire message itself.
With this commit, we allow the `MsgRouter` to be available in the `ImplementationCfg`. With this, programs outside of lnd itself are able to now hook into the message processing flow to direct handle custom messages, and even normal wire messages.
This'll be useful for new interface definitions that use the contents of the package.
This lets us get rid of the mutex usage there. We also shift the algo slightly to increment by 1, then use that as the next value, which plays nicer with the atomics.
This struct will house all the information we'll need to do a class of custom channels that relies primarily on adding additional items to the tapscript root of the HTLC/commitment/funding outputs.
With this commit, we'll now populate all the custom channel information within the OpenChannel and ChannelCommitment structs.
| @@ -98,7 +99,6 @@ const ( | |||
| // you and limitless channel size (apart from 21 million cap). | |||
| MaxBtcFundingAmountWumbo = btcutil.Amount(1000000000) | |||
|
|
|||
| // TODO(roasbeef): tune. | |||
There was a problem hiding this comment.
todo removed but value wasn't tuned
There was a problem hiding this comment.
I assume it turned out tuning isn't necessary anymore? @Roasbeef removed those TODOs, so I can just make the assumption they aren't required anymore? Updated the commit message though.
| @@ -3409,6 +3481,7 @@ func (f *Manager) addToRouterGraph(completeChan *channeldb.OpenChannel, | |||
| errChan := f.cfg.SendAnnouncement( | |||
| ann.chanAnn, discovery.ChannelCapacity(completeChan.Capacity), | |||
| discovery.ChannelPoint(completeChan.FundingOutpoint), | |||
| discovery.TapscriptRoot(completeChan.TapscriptRoot), | |||
There was a problem hiding this comment.
this ends up being encoded in the ExtraOpaqueData right?
There was a problem hiding this comment.
We don't have a way to gossip Simple Taproot channels to the wider network yet, so they currently don't land in any p2p message. We just set the field here so that the local gossip validation logic doesn't trip over it and fails to add the channel to the local graph.
| @@ -493,6 +499,10 @@ type Brontide struct { | |||
| // potentially holding lots of un-consumed events. | |||
| channelEventClient *subscribe.Client | |||
|
|
|||
| // msgRouter is an instance of the MsgRouter which is used to send off | |||
| // new wire messages for handing. | |||
| msgRouter fn.Option[protofsm.MsgRouter] | |||
There was a problem hiding this comment.
does this really need to be wrapped as an option?
we're always setting it to be either a custom msgRouter or the default one
There was a problem hiding this comment.
Yeah, I think it minimizes the diff size, otherwise we might need to hook up more to get the unit tests to pass.
| @@ -2513,6 +2523,9 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg, | |||
| cp := *nMsg.optionalMsgFields.channelPoint | |||
| edge.ChannelPoint = cp | |||
| } | |||
|
|
|||
| // Optional tapscript root for custom channels. | |||
| edge.TapscriptRoot = nMsg.optionalMsgFields.tapscriptRoot | |||
There was a problem hiding this comment.
for other nodes in the network what purpose does this field have?
There was a problem hiding this comment.
For being able to verify the pkScript of the channel. Without the tapscript root the validation nodes would assume 2-of-2 MuSig2 with BIP-0086 tweak.
| log.Error(err) | ||
| f.failFundingFlow(peer, cid, err) |
There was a problem hiding this comment.
maybe just log all errors in failFundingFlow
There was a problem hiding this comment.
We already do, but just as a Debugf because there we don't always know if it qualifies as an actual error. So I think it's okay to do so here, we seem to be doing the same in other places too.
funding/manager.go
Outdated
| err = fmt.Errorf("error deriving tapscript root: %w", err) | ||
| msg.Err <- err |
There was a problem hiding this comment.
just msg.Err <- fmt.Errorf(...
There was a problem hiding this comment.
Actually, I forgot to do the logging here, which I think we need. So I kept the separate variable.
In this commit, we make a new `AuxFundingController` interface capable of processing messages off the wire. In addition, we can use it to abstract away details w.r.t how we obtain a `AuxFundingDesc` for a given channel. We'll now use this whenever we get a channel funding request, to make sure we pass along the custom state that a channel may require.
If this is a taproot channel, then we'll return the internal key which'll be useful to callers.
We also add a new assertion to the itests to ensure the field is being properly set.
In this commit, we modify the aux funding work flow slightly. We won't be able to generate the full AuxFundingDesc until both sides has sent+received funding params. So we'll now only attempt to bind the tapscript root as soon as we send+recv the open_channel message. We'll now also make sure that we pass the tapscript root all the way down into the musig2 session creation.
For the initiator, once we get the signal that the PSBT has been finalized, we'll call into the aux funder to get the funding desc. For the responder, once we receive the funding_created message, we'll do the same. We now also have local+remote aux leaves for the commitment transaction. Some old TODO comments that in retrospect aren't required anymore are removed as well.
Depends on #8684.
In this commit, we add the
AuxFundingControllerinterface that can be used to implement a class of custom funding channels that only requires some additional data to be inserted into the tapscript leaves for the funding/HTLC/commitment outputs. This controller also implements theMsgEndpointinterface (see #8520) as well. This allows the controller to intercept certain custom messages related to the funding process (think extra sigs for a channel factory), and even the existing funding messages.We then create a new top level config entry for this interface. This allows an external program to instantiate a
AuxFundingController, make aMsgRouter, register that, then pass both into the lnd top level config.We also pick up where the prior PR in this series left of by using the new abstraction to map a pending chan ID (kinda like the
FundingStateStepRPC) to the aux funding data. This data is then used to populate all the new custom blobs that we'll store for these new channel types, which impacts the way we construct the funding output, and also the commitment/HTLC outputs+scripts.Link to all PRs in the saga: