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

BOLT 3: fix definition of flip(B) in P. #779

Merged
merged 1 commit into from
Jul 7, 2020

Conversation

rustyrussell
Copy link
Collaborator

It turns out everyone does P[B / 8] ^= (1 << (P % 8)), which is not what the spec says to do (it implies you would treat P as a bitstring numbered 255 to 0).

See this stackoverflow question:
https://stackoverflow.com/questions/49928131/lightning-secret-generation-from-seed

Reported-by: Janus Troelsen @ysangkok (on Twitter)
Signed-off-by: Rusty Russell rusty@rustcorp.com.au

@rustyrussell rustyrussell added the clarification substantive change or addition around wording or meaning label May 19, 2020
@t-bast
Copy link
Collaborator

t-bast commented May 19, 2020

Interesting, I hadn't visited that part of our code much to be honest.
This is quite different from what the spec required, did implementations communicate and coordinate to implement the same thing?

@rustyrussell
Copy link
Collaborator Author

The spec says everything should be big-endian, even our bitfields (thought that is, of course, insane). Here we treat the sha as a little-endian bitfield, which is natural and wholesome, but not in line with the rest of the BOLTs.

Copy link
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, just need to fix the spellchecker and we should be good?

Where "flip(B)" alternates the B'th least significant bit in the value P.
Where "flip(B)" alternates the (B mod 8)'th bit of the (B div 8)'th
byte of the value. So, "flip(0) in e3b0..." is "e2b0...", and
"flip(10) in "e3b0..." is "e3b4".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does the last hex string have no "..."?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed!

It turns out everyone does `P[B / 8] ^= (1 << (P % 8))`,
which is not what the spec says to do (it implies you
would treat P as a bitstring numbered 255 to 0).

See this stackoverflow question:
	https://stackoverflow.com/questions/49928131/lightning-secret-generation-from-seed

Reported-by: Janus Troelsen @ysangkok (on Twitter)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
Copy link
Collaborator Author

OK, I felt it a little dangerous to add "th" to the word list, so I removed the "'th" instead.

@t-bast t-bast merged commit 0ac9a6c into lightning:master Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification substantive change or addition around wording or meaning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants