Skip to content

Use zero-sized field for message padding #467

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

Closed
jkczyz opened this issue Jan 29, 2020 · 5 comments
Closed

Use zero-sized field for message padding #467

jkczyz opened this issue Jan 29, 2020 · 5 comments
Labels
good first issue Good for newcomers

Comments

@jkczyz
Copy link
Contributor

jkczyz commented Jan 29, 2020

Could you use something like PhantomData<[u8; 12]> (or our own custom Padding type) for this? It could allow for using zero bytes yet still have a field for reading/writing the data. Then we'd make reading the padding explicit rather than doing so while reading outgoing_cltv_value.

Originally posted by @jkczyz in #434

@jkczyz jkczyz added the good first issue Good for newcomers label Jan 29, 2020
@varunsrin
Copy link

varunsrin commented Jan 30, 2020

Happy to pick this up, unless @TheBlueMatt is already working on it.

@jkczyz To clarify, do we always set amt_to_forward (u64) and outgoing_cltv_value (u32) to 0 values to provide padding for the legacy format? And is the goal to use an explicit type rather than implicit ones?

@jkczyz
Copy link
Contributor Author

jkczyz commented Jan 30, 2020

Happy to pick this up, unless @TheBlueMatt is already working on it.

Go for it!

@jkczyz To clarify, do we always set amt_to_forward (u64) and outgoing_cltv_value (u32) to 0 values to provide padding for the legacy format? And is the goal to use an explicit type rather than implicit ones?

Padding actually predates the format becoming legacy. It is reserved for future use in that format. This is defined in BOLT 4 in v1.0 of the spec.

https://github.com/lightningnetwork/lightning-rfc/blob/v1.0/04-onion-routing.md

The changes in #434 are from the working v1.1 spec, which makes that format legacy.

https://github.com/lightningnetwork/lightning-rfc/blob/master/04-onion-routing.md

@varunsrin
Copy link

Pushed for review: #469

@upjohnc
Copy link
Contributor

upjohnc commented Jun 6, 2023

IIUC: this ticket could be closed. Looks like PR #469 made the code change.

@TheBlueMatt
Copy link
Collaborator

That PR never ended up getting merged, though I'm not sure it's worth doing this now - it wouldn't clean up our serialization and let us switch to a macro and eventually we'll fully remove the pre-TLV onion format.

@TheBlueMatt TheBlueMatt closed this as not planned Won't fix, can't repro, duplicate, stale Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants