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

BOLT04: use truncated u64 encoding for next_hop_id #663

Closed

Conversation

@cfromknecht
Copy link
Collaborator

commented Aug 30, 2019

As per discussion in this thread, this PR changes the short_channel_id onion record (type 8) from short_channel_id to tu64, which shaves 8 bytes for the final hop when the short_channel_id is always 0.

@cdecker do you have any thoughts on how do to this mapping? Is that something that should be in this PR?

@t-bast
t-bast approved these changes Aug 30, 2019
Copy link
Collaborator

left a comment

ACK 3d28ace

@Roasbeef
Copy link
Member

left a comment

LGTM

@rustyrussell

This comment has been minimized.

Copy link
Collaborator

commented Sep 2, 2019

Huh? Why would this field appear at all in the final hop?

@t-bast

This comment has been minimized.

Copy link
Collaborator

commented Sep 2, 2019

Doh, rusty's totally right. Currently our implementation simply omits this tlv field entirely for the last hop. And that should settle it :)

@t-bast t-bast moved this from Scheduled to Rejected in Specification Meeting Agenda Sep 2, 2019

@cdecker

This comment has been minimized.

Copy link
Collaborator

commented Sep 2, 2019

During the IRC spec meeting on 2019-09-02 we found out that any current short_channel_id would already require 59 bytes at least to encode, which would result in us always using 8 bytes for any realistic short_channel_id, except for the last hop.

As @rustyrussell points out the short_channel_id MUST be omitted for the last hop. So this is not really a saving after all.

@cfromknecht

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 4, 2019

As @rustyrussell points out the short_channel_id MUST be omitted for the last hop. So this is not really a saving after all.

Ah thanks @rustyrussell! Looks instead like we need to fix this on our end, as we were including it for the final hop.

@cfromknecht cfromknecht closed this Sep 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.