Skip to content

Comments

BOLT-4 clarifications#1318

Open
w3irdrobot wants to merge 6 commits intolightning:masterfrom
w3irdrobot:bolt4-clarifications
Open

BOLT-4 clarifications#1318
w3irdrobot wants to merge 6 commits intolightning:masterfrom
w3irdrobot:bolt4-clarifications

Conversation

@w3irdrobot
Copy link

Lots of small clarifications in BOLT-4 found while attempting to implement onion routing:

  • The mix header is called a few different things throughout the document. A note was added to explain that
  • When discussing right-shifting the header during creation, hop_payload was used instead of hop_payloads
  • Clarified the description of the location of copying data after right-shifting
  • Lightning uses a mix-header size of 1300. The sample packet creation code used some variables that weren't actually in the source, making it a bit confusing when trying to read it to understand the process. I hardcoded the values with constants as well as fixed some formatting
  • The header creation description makes it clear the header should be generated inside-out, or the last hop first. However, the filler is supposed to be created in regular order but is not mentioned. So I added a small note just to make the clear
  • The last commit might be off, but there's nothing in the BOLTs detailing how the associated data is sent to each hop to ensure they are able to calculate the HMACs correctly. This leads me to believe there is a standard piece of data that is assumed to be used by each implementation. However, I didn't see anything in the BOLTs detailing this. Looking at the LDK source, it seems the payment hash might be that standard. So I added that note in here for clarification. However, if this is wrong, I can remove that from the PR

The sample code in BOLT4 seemed to be copied from a more generic
implementation of SPHINX packet creation. This can lead to confusion to
someone new reading the specification. Hardcoding these values removes
this confusion while simplifying the function signatures.
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

Successfully merging this pull request may close these issues.

1 participant