Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop 5-conf check for channel announcement validation #3532

Closed
tnull opened this issue Jan 14, 2025 · 4 comments
Closed

Drop 5-conf check for channel announcement validation #3532

tnull opened this issue Jan 14, 2025 · 4 comments

Comments

@tnull
Copy link
Contributor

tnull commented Jan 14, 2025

When validating channel announcements we currently check that the funding transaction has reached at least 5 confirmations (intentionally leaving a buffer of 1 here in case we're lagging behind on chain syncing).

Now lightning/bolts#1215 relaxed the specific 6-conf limit in favor of sending announcement_signatures only after the funding transaction has enough confirmations to ensure that it won't be reorganized., leaving it to the respective participants to decide when this would be the case.

We should hence drop/relax the 5-conf check here:

// If the block doesn't yet have five confirmations, error out.
//
// The BOLT spec requires nodes wait for six confirmations before announcing a
// channel, and we give them one block of headroom in case we're delayed seeing a
// block.
if block_height + 5 > tip_height {
return Err(UtxoLookupError::UnknownTx);
}
}

Question remains if we still need to at least check for 1 or 2-conf (still accounting for a 1-conf buffer), as most will implement this either as broadcasting after 3-conf or 6-conf in practice?

@TheBlueMatt TheBlueMatt modified the milestone: 0.2 Jan 14, 2025
@TheBlueMatt
Copy link
Collaborator

Actually, I'm not sure we can do this? We don't currently handle the case of channels getting reorg'd out once they're in our graph, and I don't really want to add support for that...IMO the spec change should be reverted, but we can follow up there.

@tnull
Copy link
Contributor Author

tnull commented Jan 14, 2025

Actually, I'm not sure we can do this? We don't currently handle the case of channels getting reorg'd out once they're in our graph, and I don't really want to add support for that...IMO the spec change should be reverted, but we can follow up there.

lol, yeah, as discussed we kind of trusting on the judgement of the channel parties what 'enough confirmations' is.

I took your ACK to mean we'd bite the bullet and handle reorgs in the NetworkGraph.

@tnull
Copy link
Contributor Author

tnull commented Jan 16, 2025

I guess we can close this now that lightning/bolts#1220 is going to clarify that we'll keep the 6-conf delay for broadcasting. IIUC, we'll still need to introduce a cache for announcement_signatures though, will open another issue for that.

@tnull tnull closed this as completed Jan 16, 2025
@TheBlueMatt
Copy link
Collaborator

We have one - Channel::announcement_sigs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants