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

Should we remove the short_channel_id? #301

Closed
pm47 opened this issue Dec 4, 2017 · 4 comments
Closed

Should we remove the short_channel_id? #301

pm47 opened this issue Dec 4, 2017 · 4 comments

Comments

@pm47
Copy link
Collaborator

pm47 commented Dec 4, 2017

I know we are in feature freeze... but this is just itching!

IIRC, when we discussed this short_channel_id back in Milan, main advantages over 32B channel_id were:

  • compact (8B) globally unique identifier
  • reduces channel_update size, which is the most used routing messages
  • reduces onion size
  • also happens to give the channel's coordinates on the blockchain

This comes at a cost:

  • additional negotiation on channel opening, with the need to wait for 6 confirmations to make sure id is final
  • more complexity with two identifiers for a channel

But this was before 956e880, which introduces support for multiple chains. As a result, we had to add a 32B chain_hash to channel_update, which negates most advantages of the short_channel_id, since the GUID is now 32+8=40B, greater than channel_id.

At this point shouldn't we just get rid of it, and use the channel_id in channel_update instead of chain_hash + short_channel_id? The only place where we really need a short identifier is in the onion, but even there why not just use the first 8B of the channel_id? The risk of collisions should be extremely low because that'd be a local id, and could be easily detected and prevented at channel opening anyway. It would also be so much easier to debug routes! We could also just let each channel endpoint advertise the local short_channel_id of their choice in the channel_update.

Another argument for keeping the chain_hash in the channel_update could be to easily filter out announcements for a chain we are not interested in. But nodes have to lookup their channel_announcement table anyway to make sure the announcement is tied to a real channel, and we can get the chain_hash at this time.

@akumaigorodski
Copy link

another silly and irrelevant reason to remove it is bitcoinj wallets can't know tx's number in a block so they have problems with short_channel_id

@rustyrussell
Copy link
Collaborator

It meshes well with compact proofs (give me tx N in block B, with proof). OTOH a full node in practice can produce that by TXID anyway, and eventually we'll probably accompany these messages with a proof and the full tx (at least, as an option for lite nodes).

Changing this for 1.1 would not be super painful though: we'd just have two types of channel_announce, and your chosen short-channel-id would be the current one. After everyone has upgraded, turn off the old announcements, and then your short-channel-id must be the first 8 bytes of the channel_id. Or something...

@rustyrussell
Copy link
Collaborator

OTOH, the chainid in update is discardable (you can compact to a few bits if you support multiple chains). With schnorr we end up with a minimal channel_announce of 1 sig, 3 keys, and short channel id == 64+99+8 == 172 bytes. Maybe smaller, depending on what scheme bitcoin uses for sigs (128-bit keyhash maybe?) and whether we store node_ids separately internally and use a reference (most nodes will have > 1 channel). So we could see it as low as 120 bytes or so, making the addition of 24-32 bytes non-trivial.

This is probably the key space bottleneck to route scaling, so squeezing bytes here is a win. But on the other hand, we could defer that to when we have schnorr anyway.

@t-bast
Copy link
Collaborator

t-bast commented Jun 8, 2022

That ship has sailed a long time ago, closing this old issue 😉.

@t-bast t-bast closed this as completed Jun 8, 2022
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

4 participants