lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.63k stars 2.07k forks source link

[spec]: start to set the `dont_forward` bit in `ChannelUpdate` #7416

Open Crypt-iQ opened 1 year ago

Crypt-iQ commented 1 year ago

We need to start setting the bit at index 1 in the ChannelUpdate message flags per https://github.com/lightning/bolts/commit/6fee63fc342736a2eb7f6e856ae0d85807cc50ec. It is only set for updates that we don't want forwarded. See the spec change for more details.

saubyk commented 1 year ago

prioed for 17

oldmonad commented 1 year ago

Hi, a new contributor here. Can I take a stab at this?

saubyk commented 1 year ago

Hi @oldmonad if you're interested, please go through the problem statement and the linked spec. Once your analysis is complete, please provide a concise plan of action on this issue, before attempting a pr. Thanks.

oldmonad commented 1 year ago

Got it

oldmonad commented 1 year ago

Hi @saubyk, I provided a brief overview of the issue and action plan in this document. I would greatly appreciate any feedback.

ellemouton commented 1 year ago

thanks for the outline @oldmonad!

I think we just need to start setting the bit so that other impls dont start rejecting our channel_updates but I dont think we necessarily need to make this one required on our side. Keep to hear what @Crypt-iQ thinks though.

Crypt-iQ commented 1 year ago

We don't need to make the htlc_maximum_msat bit required because that may cause newer LND nodes to disconnect from older nodes. We'll need to set the dont_forward bit in funding/manager.go when sending out a channel update: https://github.com/lightningnetwork/lnd/blob/1052b139bd45d560056c2b6ea4e80c10129092b9/funding/manager.go#L3076

and also in discovery/gossiper.go when sending the update to the remote peer: https://github.com/lightningnetwork/lnd/blob/1052b139bd45d560056c2b6ea4e80c10129092b9/discovery/gossiper.go#L1731

Also, this bit should be set even if aliases aren't used. It would be nice if all newer channels persisted the bit from the start and older channels performed an on-the-fly migration similar to what was done for the htlc-maximum bit here: https://github.com/lightningnetwork/lnd/blob/1052b139bd45d560056c2b6ea4e80c10129092b9/routing/localchans/manager.go#L199-L201

I'll have to check if there are any other callsites that I'm forgetting

ziggie1984 commented 1 year ago

@oldmonad are you still working on this ?

ziggie1984 commented 1 year ago

Also, this bit should be set even if aliases aren't used. It would be nice if all newer channels persisted the bit from the start and older channels performed an on-the-fly migration similar to what was done for the htlc-maximum bit here:

Did I understand it correctly by "even channel which are not using aliases" but they have to be private tho, that's at least what I implemented. Basically you did all the work have not fund another place where we should set this bit ;)

Crypt-iQ commented 1 year ago

Yup they need to be private for the flag to be set