Skip funding confirmation #4424

roeierez · 2020-06-28T18:59:18Z

This PR introduces the ability to skip a channel funding transaction confirmation, making the channel fully operational before its on-chain confirmation (aka a zero-conf channel).
Till confirmation, this channel requires trust between its two parties and in the case of a remote initiator, it puts the received funds of the local party at risk.
Nevertheless, there are cases where it makes sense to support this behavior. For example, in cases both parties decide to trust each other. Or, in cases where trust between the parties already exists (buying a pre-loaded channel from a service like Bitrefill).

We at Breez want this capability to support the following:

Immediate on-boarding:
Currently, new users need to wait for channel confirmation before being able to receive. This PR, together with #4018, enables us to provide a far better user experience as follows:
Let's say Alice wants to pay a new Breez user, Bob.

Bob is connected to the Breez node and issues an invoice with a routing hint that points to a fake channel between Bob and the Breez node.
When Alice pays Bob's invoice, the Breez node intercepts the HTLC and holds it.
Then, the Breez node does the following:
- Opens a channel to Bob.
- Uses the fake short channel id in the routing hints to skip funding confirmation.
- Waits for the channel to become active.
- Pays Bob the original Alices' tx (potentially, minus a service fee Let invoice.Encode receive a function which hashes itself the message #4423)
- Settles backword.
When Bob has a new pending channel it is his choice whether to call SkipFundingConfirmation or use the default behavior that requires confirmation.
In case Bob decided to skip confirmation all received payments are held, giving him control whether to settle additional payments and add to existing trust between the parties.

Behavior:
In order to skip confirmation, the funding locked message should be sent and received on both sides.
Both parties need to agree on a “fake” short channel id and because the protocol doesn't define any kind of message that supports it, a simplified approach was taken where both parties use the "temporary_channel_id" 58 first bits to create a "fake" short channel id. This leaves 18 bits for the block height which ensures it will be less than 2^18 = 262144.

This PR is a work in progress on which I would like to get early feedback regarding the overall approach.

cfromknecht · 2020-07-06T22:01:54Z

@roeierez thanks for the PR, good to get this discussion going! This has been a highly requested feature, and I do think we should look to support it.

IMO the main thing we have to consider is that currently there is critical forwarding info that is indexed by short channel id, and the logic assumes that the channel is only ever used after the short channel id is final, see forwarding packages. In practice, things will break if the short channel id is changed after using the channel as is done in this PR. Further, various parts of the codebase assume that UpdateShortChannelID is only called once—it's been two+ years since that code has been touched, so we want to make sure we aren't missing anything critical.

I think there is a preliminary step before this proposal is possible, i.e. migrating the forwarding packages to use a key that is static for the lifetime of the channel, which probably means either channel id or channel point. In theory this would allow us to mutate the short channel id on the fly, though i admit i have not thought about all the possible edge cases in the switch if this mapping is modified.

roeierez · 2020-07-07T12:28:22Z

@cfromknecht Appreciate you jumped, your input is very helpful and is what I was hoping to get here. The WIP prefix will only be removed once I feel it is ready and currently there is some way to go.
I will definitely look into the forwarding_package and then we can continue the discussion. As for the switch, since the code there already had the mechanism to refresh the short channel id I am more comfortable with the small change there but I'll be more confident only after I will implement the required tests.

roeierez · 2020-07-12T21:18:55Z

I think there is a preliminary step before this proposal is possible, i.e. migrating the forwarding packages to use a key that is static for the lifetime of the channel, which probably means either channel id or channel point. In theory this would allow us to mutate the short channel id on the fly, though i admit i have not thought about all the possible edge cases in the switch if this mapping is modified.

@cfromknecht I looked into it and thought it is better to break the preliminary steps into separate PRs. Started with this #4459. If this looks reasonable to you I will continue to add the missing migration that will enable us to move forward.

roeierez · 2020-07-19T10:54:22Z

I did some major changes to the initial PR by favoring some conventions over flexibility.
In general, I've adopted the keysend model: the risk in an unconfirmed channel is not in the channel itself, rather in actually accepting a payment while the channel is unconfirmed. In some cases, one would want to accept such payments (and the risk involved) on their amount. Others may use a different criteria that are not known at the time of opening.
Based on that, this approach works as follows:

The non-funding node (fundee) decides whether to skip confirmation by sending min_depth=0 in accept_channel message.
The funder then skips confirmation and sends funding_locked (as the funder is not at risk).
Both nodes uses a convention for the short channel id by applying a simple transformation on the random pendingID. This convention is simple and doesn't require additional RPC layer as suggested in the previous version.
All received payments on an unconfirmed channel are held and require the user to explicitly settle them. This mechanism makes it safe to skip confirmation and allows users to selectively approve payments and control their risk

Roasbeef · 2020-07-20T23:36:08Z

I think it may be better to first start a conversation on the spec level w.r.t how zero-conf channels are negotiated and implemented. Otherwise, we may end up shipping with a version that only works with lnd and not the other implementations (all of which I think have some form of 0-conf channels being used in the wild?).

roeierez · 2021-07-20T13:30:10Z

Rebased

Roasbeef

Thanks for reviving this old PR! Completed an initial pass to update my mental model of the desired protocol flow. There're a few things that are unclear, and some improvements I think can be made w.r.t information hiding and abstractions, you'll find those comment in line.

Roasbeef · 2021-08-06T00:30:37Z

lncfg/chain.go

+	FeeRate                 lnwire.MilliSatoshi `long:"feerate" description:"The fee rate used when forwarding payments on our channels. The total fee charged is basefee + (amount * feerate / 1000000), where amount is the forwarded amount."`
+	TimeLockDelta           uint32              `long:"timelockdelta" description:"The CLTV delta we will subtract from a forwarded HTLC's timelock value"`
+	DNSSeeds                []string            `long:"dnsseed" description:"The seed DNS server(s) to use for initial peer discovery. Must be specified as a '<primary_dns>[,<soa_primary_dns>]' tuple where the SOA address is needed for DNS resolution through Tor but is optional for clearnet users. Multiple tuples can be specified, will overwrite the default seed servers."`
+	SkipChannelConfirmation bool                `long:"skip-channel-confirmation" description:"Whether or not to wait for a channel to confirm on-chain before making it operational"`


As this is a protocol level feature/toggle, we should group it with the rest of them (anchors, etc), which is in lncfg/protocl.go.

Roasbeef · 2021-08-06T00:32:16Z

funding/manager.go

@@ -595,6 +602,12 @@ func (f *Manager) start() error {
 		return err
 	}

+	if f.fakeIDSManager, err = newFakeIDsManager(allChannels,
+		f.cfg.Wallet.Cfg.Database.ChannelGraph(),


Do we need to actually pass in both the channel graph and database? Thinking instead we can have a slimmer interface, so we only need to depend on what's absolutely necessary to manga teh set of fake chan IDs. This'll likely alos be useful with the new replicated DB backends like postgres, since in theory, we'd be able to share this set of chan IDs across nodes.

We can do only with padding the channeldb.DB, I agree.

Roasbeef · 2021-08-06T00:32:38Z

funding/manager.go

@@ -958,7 +971,33 @@ func (f *Manager) stateStep(channel *channeldb.OpenChannel,
 	// fundingLocked was sent to peer, but the channel was not added to the
 	// router graph and the channel announcement was not sent.
 	case fundingLockedSent:
-		err := f.addToRouterGraph(channel, shortChanID)
+		// in case we skipped confirmation, now that the channel is fully operational


Style nit: should start with a capital letter.

Roasbeef · 2021-08-06T00:33:20Z

funding/manager.go

+				return fmt.Errorf("failed to release fake id %v", err)
+			}
+		}
+		err = f.addToRouterGraph(channel, shortChanID)


I think this should return early (the above statement), as otherwise, we end up calling addToRouterGraph twice.

I think this should return early (the above statement), as otherwise, we end up calling addToRouterGraph twice.

We want to call 'addToRouterGraph' twice actually once for the fake channel and the second time after the short channel id is changed and channel is confirmed.

Roasbeef · 2021-08-06T00:34:34Z

funding/manager.go

+			// Adding the fake channel to the router graph
+			graph := f.cfg.Wallet.Cfg.Database.ChannelGraph()
+			fakeID := shortChanID
+			if err = f.addToRouterGraph(channel, shortChanID); err != nil {


Alternatively, we can delete this call? Given it looks liek we'll add the channel to teh graph, then delete it right afterwards.

Protocol flow wise, is the intent to instantly send FundingLocked or actually wait until eventual channel confirmation?

Alternatively, we can delete this call? Given it looks liek we'll add the channel to teh graph, then delete it right afterwards.

If we delete this call the fake channel won't exist in the graph and outgoing payments attempts will fail with no route.
The intent here is that once FundingLocked is received for the unconfirmed channel, we add the fake channel to the routing graph, then we wait at this step for the funding confirmation which afterwards we update the new short channel id. After we updated the confirmed channel id we need to add the confirmed channel to the routing graph (hence the second call).

Protocol flow wise, is the intent to instantly send FundingLocked or actually wait until eventual channel confirmation?

The intent here is to send FundingLocked instantly for the unconfirmed channel, then both peers should change the short channel id independently upon confirmation.

Roasbeef · 2021-08-06T01:10:47Z

routing/router.go

-		if r.cfg.AssumeChannelValid {
+		// We can also skip fetching the channel point if we created the
+		// announcement for an unconfirmed channel.
+		ourKey := r.selfNode.PubKeyBytes[:]


We could possibly pass things along as an additional attribute, this way we don't need to leak the details of the "fake" short chan IDs into the router itself.

Roasbeef · 2021-08-06T01:11:42Z

lnwire/short_channel_id.go

+// height is between 2^17 = 131072 and 2^17 + 2^18 - 1 = 393215
+func NewFakeShortChanIDFromInt(chanID uint64) ShortChannelID {
+	c := NewShortChanIDFromInt(chanID)
+	c.BlockHeight = (c.BlockHeight >> 6) + (1 << 17)


What about the TxIndex and output? As otherwise, any zero conf channel accepted at the same block height may end up having the same scid?

The chanID parameter is misleading here. This method actually gets a pending channel id (which is the random number that is generated for pending channels) and generate a new fake channel id from it. Since the pending is already a big random number we shouldn't have a collision in practice.
I will change the naming to be more clear.

Roasbeef · 2021-08-06T01:12:57Z

invoices/update.go

 	htlcSet := inv.HTLCSet(setID, channeldb.HtlcStateAccepted)

+	// Check whether this invoice is hold invoice or the channel is fake.
+	hodlInvoice := inv.HodlInvoice || ctx.circuitKey.ChanID.IsFake()


Similar comment here re leaking in details where it may not be necessary. Rather than force the invoice registry to understand zero conf channels, we can instead have the caller (the link in this case) pass along that information by just setting HoldInvoice to true.

The link doesn't pass the invoice, just the hash. Maybe just add Hold field to this function called by the link?
https://github.com/lightningnetwork/lnd/blob/master/invoices/invoiceregistry.go#L872

Roasbeef · 2021-08-06T01:13:40Z

discovery/gossiper.go

@@ -1622,7 +1622,7 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(
 		// If the advertised inclusionary block is beyond our knowledge
 		// of the chain tip, then we'll ignore for it now.
 		d.Lock()
-		if nMsg.isRemote && isPremature(msg.ShortChannelID, 0) {
+		if nMsg.isRemote && !msg.ShortChannelID.IsFake() && isPremature(msg.ShortChannelID, 0) {


Why would we get a chan ann for a zero conf channel?

It seems that these gossiper announcements are triggered from the funding manager addToRouterGraph method.
It is not the announcement for six confirmations.

Roasbeef · 2021-08-06T01:14:43Z

funding/manager.go

@@ -1398,11 +1398,16 @@ func (f *Manager) handleFundingOpen(peer lnpeer.Peer,
 	// the amount of the channel, and also if any funds are being pushed to
 	// us. If a depth value was set by our channel acceptor, we will use
 	// that value instead.
-	numConfsReq := f.cfg.NumRequiredConfs(msg.FundingAmount, msg.PushAmount)
+	remoteFeatures := peer.RemoteFeatures()
+	skipConfirmationOptional := remoteFeatures.HasFeature(


Style nit: doesn't use typical line folding style.

Can you please elaborate on what's wrong here? I had to cut the line so it won't exceed 80 characters.

Roasbeef · 2021-08-06T18:45:30Z

Thinking about this a bit more, what if we just made the zero conf chan funding into a new channel type? This would address the config issues (always accepting to zero conf), as it would effectively let the initiator attempt to propose a zero conf channel.

roeierez · 2021-08-08T12:28:41Z

Thinking about this a bit more, what if we just made the zero conf chan funding into a new channel type? This would address the config issues (always accepting to zero conf), as it would effectively let the initiator attempt to propose a zero conf channel.

Sounds interesting. I will check this direction.

roeierez · 2021-08-10T11:36:06Z

Thinking about this a bit more, what if we just made the zero conf chan funding into a new channel type? This would address the config issues (always accepting to zero conf), as it would effectively let the initiator attempt to propose a zero conf channel.

Went over the code again and I think we can add another ChanType flag for zero conf. I think the main advantage would be to stop relying on the fake id (isFake method) to determine funding flow and instead use the channel type.

The config option is for the responder to determine if the accept_channel message may include min_depth=0 on the case the remote peer supports it. The acceptor still have the opportunity to override this decision using the ChannelAcceptor hook and return a positive min_depth.

As for the initiator, if he provisions the SkipFundingConfirmationOptional feature currently it means he will always accept min_depth=0 as a response for chan_accept message (the risk is not on his side as he is the funder). We can limit this behaviour by adding another field for the open channel request (allowSkipConfirmation) and consider it when handling the chan_accept message. @Roasbeef is that what you meant?

TheBlueMatt · 2021-08-11T16:51:05Z

Is there a BOLT or blip writeup of the spec changes to this? This is something that is going to really want cross-implementation compatibility and I don't see a writeup to provide feedback against. anywhere

ryanthegentry · 2021-08-12T17:06:36Z

Is there a BOLT or blip writeup of the spec changes to this? This is something that is going to really want cross-implementation compatibility and I don't see a writeup to provide feedback against. anywhere

hi @TheBlueMatt yes, @roeierez wrote up their approach on the mailing list almost a full year ago. If bLIPs (or something similar) were a thing I think it would've received more feedback from other implementations over the past 12 months. Hopefully we can tweak the process to ensure the next feature like this doesn't suffer the same fate!

Crypt-iQ · 2021-09-02T20:04:51Z

funding/manager.go

+		if err != nil {
+			return nil, fmt.Errorf("unable to validate channel: %v", err)
+		}
+		if err := confirmedChan.MarkAsOpen(conf.shortChanID, true); err != nil {


This will create another Packager under a different short channel id. I think this can break the forwarding package logic if there is an existing payment that needs to be fulfilled during this upgrade

Following @cfromknecht feedback (#4424 (comment)) I set this PR as depends on #4459.
That PR converts the packager to use static keys that don't change upon confirmation.

Crypt-iQ · 2021-09-02T20:06:06Z

htlcswitch/link.go

@@ -2934,7 +2934,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,

 	event, err := l.cfg.Registry.NotifyExitHopHtlc(
 		invoiceHash, pd.Amount, pd.Timeout, int32(heightNow),
-		circuitKey, l.hodlQueue.ChanIn(), payload,
+		circuitKey, l.hodlQueue.ChanIn(), payload, l.channel.State().ChanType.IsZeroConf(),


This logic is only called for the exit-hop case, forwarding is also unsafe for the responder in a 0-conf channel flow. I don't think you can assume that they have no other channels

@Crypt-iQ zero-conf inherently means there is trust until the channel is confirmed and the user decides to accept the risk during channel opening. In the case of received payments we allowed the option to mitigate the risk per payment by not settling automatically (holding) and in the case of forward one can mitigate it by using an interceptor.
That being said, I am opened to remove the current hold logic for the sake of consistency.

I was mostly commenting because I was confused that there wasn't consistency. You're right that zero-conf implies trust, but if it requires trust, why the holding of payments?

Originally we thought it would be beneficial for users to have control on the amount received in zero-conf channel but I now agree that it is not necessary (definitely for the initial version of this feature).
I will remove it.

orijbot · 2021-09-13T12:54:32Z

Visit https://dashboard.github.orijtech.com?back=0&pr=4424&remote=true&repo=breez%2Flnd to see benchmark details.

This commit exposes a configuration flag for the channel receiver to skip the funding confirmation of a pending channel. Specifically it advances the funding workflow and makes the channel fully operational by sending and receiving funding locked message. Since there is no risk at the initiator side, when receiving zero amount of required confirmation from the reciever it advances the workflow as well. During the time where the channel is operational but not confirmed both parties use a convention where the first 58 bits of the random pending id serves as the short channel id as long as it does not conflict with existing ids. This convention ensures that the block height of the generated id is less than 50000 and doesn't require any extra RPC layer.

BitcoinErrorLog · 2021-10-07T00:43:13Z

Hey everyone! Just checking in on this and the overall status for 0conf support in LND. Last I heard, there was an intention to get support into the next release. Where do things stand now, and is there anything any of us can do to facilitate? Thanks!

Roasbeef · 2021-10-15T10:06:45Z

Hey everyone! Just checking in on this and the overall status for 0conf support in LND. Last I heard, there was an intention to get support into the next release. Where do things stand now, and is there anything any of us can do to facilitate? Thanks!

Hi, we're still working through some possible edge cases that exist in this PR as well as the portion scheduled to land in the spec, which now includes support for arbitrary channel aliases, which improves privacy as you no longer leak the location of your funding transaction on chain when creating an invoice with hop hints.

hsjoberg · 2021-10-15T22:18:38Z

Yeah I didn't check the new code yet but I hoped for compatibility with Rusty's BOLT spec proposal.
Awesome to hear. 💯

lightninglabs-deploy · 2022-01-27T17:40:31Z

@cfromknecht: review reminder
@roeierez, remember to re-request review from reviewers when ready

Roasbeef · 2022-02-02T01:10:57Z

Has been replaced by #5955

roeierez requested review from halseth and Roasbeef as code owners June 28, 2020 18:59

roeierez force-pushed the skip-funding-confirmation branch 2 times, most recently from 1d4eb25 to d522b98 Compare July 1, 2020 14:15

roeierez requested a review from cfromknecht as a code owner July 1, 2020 14:15

roeierez force-pushed the skip-funding-confirmation branch from d522b98 to ac999ac Compare July 2, 2020 14:45

roeierez requested a review from joostjager as a code owner July 2, 2020 14:45

roeierez force-pushed the skip-funding-confirmation branch 3 times, most recently from 8416260 to b19e80a Compare July 6, 2020 13:54

yaslama mentioned this pull request Jul 9, 2020

PSBT: add no_publish flag for safe batch channel opens v2 #4455

Merged

roeierez mentioned this pull request Jul 12, 2020

Use static key for ChannelPackager #4459

Closed

roeierez force-pushed the skip-funding-confirmation branch from b19e80a to ce53f1a Compare July 19, 2020 10:53

roeierez force-pushed the skip-funding-confirmation branch 5 times, most recently from f3c3447 to 9c4acb3 Compare July 20, 2020 08:39

halseth mentioned this pull request Jul 20, 2020

0-conf channel opening #4136

Closed

roeierez force-pushed the skip-funding-confirmation branch from 9c4acb3 to 91360a4 Compare July 20, 2020 12:36

Roasbeef added enhancement Improvements to existing features / behaviour funding Related to the opening of new channels with funding transactions on the blockchain spec labels Jul 20, 2020

halseth removed their request for review July 21, 2020 07:57

hsjoberg mentioned this pull request Jul 30, 2020

TurboChannel support hsjoberg/blixt-wallet#203

Open

roeierez force-pushed the skip-funding-confirmation branch from 2dd079f to 8c0167a Compare July 18, 2021 14:54

roeierez changed the title ~~Skip funding confirmation [WIP]~~ Skip funding confirmation Jul 19, 2021

roeierez force-pushed the skip-funding-confirmation branch 2 times, most recently from 3ae547d to 379b218 Compare July 19, 2021 17:25

Roasbeef requested changes Aug 6, 2021

View reviewed changes

roeierez force-pushed the skip-funding-confirmation branch from 379b218 to 8d5c511 Compare August 9, 2021 12:01

roeierez force-pushed the skip-funding-confirmation branch from 8d5c511 to 65221aa Compare August 11, 2021 13:04

Crypt-iQ reviewed Sep 2, 2021

View reviewed changes

roeierez force-pushed the skip-funding-confirmation branch from 65221aa to 96b7a28 Compare September 13, 2021 12:54

roeierez added 3 commits September 14, 2021 10:25

don't block premature annoncement for fake channel.

88e5921

Advertise zero-conf support with feature bit.

0a04040

roeierez force-pushed the skip-funding-confirmation branch from 96b7a28 to 0a04040 Compare September 14, 2021 07:29

Roasbeef modified the milestones: v0.14.0, v0.15.0 Oct 15, 2021

Roasbeef closed this Feb 2, 2022

Skip funding confirmation #4424

Skip funding confirmation #4424

Conversation

roeierez commented Jun 28, 2020 • edited Loading

cfromknecht commented Jul 6, 2020

roeierez commented Jul 7, 2020 • edited Loading

roeierez commented Jul 12, 2020

roeierez commented Jul 19, 2020

Roasbeef commented Jul 20, 2020

roeierez commented Jul 20, 2021

Roasbeef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef commented Aug 6, 2021

roeierez commented Aug 8, 2021

roeierez commented Aug 10, 2021

TheBlueMatt commented Aug 11, 2021

ryanthegentry commented Aug 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Crypt-iQ Sep 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orijbot commented Sep 13, 2021

BitcoinErrorLog commented Oct 7, 2021

Roasbeef commented Oct 15, 2021

hsjoberg commented Oct 15, 2021

lightninglabs-deploy commented Jan 27, 2022

Roasbeef commented Feb 2, 2022

roeierez commented Jun 28, 2020 •

edited

Loading

roeierez commented Jul 7, 2020 •

edited

Loading

Crypt-iQ Sep 9, 2021 •

edited

Loading