Closed ellemouton closed 1 week ago
[!IMPORTANT]
Review skipped
Auto reviews are limited to specific labels.
Labels to auto review (1)
* llm-reviewPlease check the settings in the CodeRabbit UI or the
.coderabbit.yaml
file in this repository. To trigger a single review, invoke the@coderabbitai review
command.You can disable this status message by setting the
reviews.review_status
tofalse
in the CodeRabbit configuration file.
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?
hmmm ok from this comment on the original issue, it sounds like we dont actually need to go and change this to a zero conf channel. We can just allow them to send a min-depth of 0 (even though this is not a zero conf channel) and then we just continue as normal (and only send channel_ready when we see fit).
so the diff can actually be way smaller
cc @Crypt-iQ - wdyt?
hmmm ok from this comment on the original issue, it sounds like we dont actually need to go and change this to a zero conf channel. We can just allow them to send a min-depth of 0 (even though this is not a zero conf channel) and then we just continue as normal (and only send channel_ready when we see fit).
so the diff can actually be way smaller
cc @Crypt-iQ - wdyt?
yup that seems fine
Ok I've made the update discussed above.
It's hard to write a test for this case cause we dont really support this between two LND nodes opening a non-zero conf channel cause we want to register for confirmations and this is not allowed with a depth of 0.
So to test this you can apply this patch which allows our channel acceptor to set a MinDepth of 0 & also makes sure we still register for 1 conf if we are on the receiver side of this operation. Then to actually test the flow, you can run the scid_alias_channel_update/public_no-chan-type_update
test which uses the channel acceptor to open a non zero conf channel
@crypt-iq: review reminder
~If our peer indicates to us that they are OK with a zero conf channel (ie, they trust us) by setting min-depth to 0 in accept_channel, then as long as they support SCID aliases, we should just upgrade the channel to ZeroConf and continue the funding flow.~
EDIT: updated to just allow the peer to set min-depth to 0. We dont upgrade the channel type.
Fixes #8783
NOTE: we already allow ourselves to do this when our peer sends us an OpenChannel without a zero-conf channel type: https://github.com/lightningnetwork/lnd/blob/fed760913ff98a89c73414acde6cf29f8728321a/funding/manager.go#L1570-L1583
So all this PR does is allow our peer to do this too :)