Skip to content

BOLT-8 Edits#311

Merged
rustyrussell merged 4 commits into
lightning:masterfrom
shannona:bolt8-edit
Dec 10, 2017
Merged

BOLT-8 Edits#311
rustyrussell merged 4 commits into
lightning:masterfrom
shannona:bolt8-edit

Conversation

@shannona
Copy link
Copy Markdown
Collaborator

@shannona shannona commented Dec 8, 2017

More clarity and copyediting. I also removed quite a few `s that didn't seem to match general usage for "code".

More clarity and copyediting. I also removed quite a few `s that didn't seem to match general usage for "code".
Comment thread 08-transport.md
the responder. This provides a degree of identity hiding for the
responder, as its public key is _never_ transmitted during the handshake. Instead,
authentication is achieved implicitly via a series of Elliptic-Curve
Diffie-Hellman (ECDH) operations followed by a MAC check.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The term "pre-message" is taken from the Noise spec, https://noiseprotocol.org/noise.html#handshake-patterns.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Changed back to "As a pre-message" (and it reads better now with the changes to the rest of the sentence).

Comment thread 08-transport.md
The initiator should produce the given output when fed this input.
The comments reflect internal state for debugging.

```
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Technically indentation serves the same purpose as triple-quote in markdown, but there's nothing wrong with both AFAICT.

Copy link
Copy Markdown
Collaborator

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Nice, much more consistent with rest of spec formatting (@Roasbeef wrote this really early on when we were still shaking that stuff loose).

Copy link
Copy Markdown
Collaborator

@toadlyBroodle toadlyBroodle left a comment

Choose a reason for hiding this comment

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

Nice one :)

Comment thread 08-transport.md Outdated
encryption keys (`ck` the chaining key and `k` the encryption key), and finally
an `AEAD` payload with a zero length cipher text is sent. As this payload is
of length zero, only a `MAC` is sent across. The mixing of `ECDH` outputs into
steps. During each "act" of the handshake: some (possibly encrypted) keying
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

original comma was correct, as the first clause is a prepositional phrase, not a complete sentence

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I prefer the clarity of leading off semi-colon separated lists with a colon, so I've changed the first clause into a sentence to allow this.

Comment thread 08-transport.md Outdated
which act is being executed, with the result mixed into the current set of
encryption keys (`ck` the chaining key and `k` the encryption key); and
an AEAD payload with a zero-length cipher text is sent. As this payload is
length zero, only a MAC is sent across. The mixing of ECDH outputs into
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

...payload has no length, only...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Better sounding. Changed.

Comment thread 08-transport.md Outdated
new ephemeral key with strong cryptographic randomness.

* `s`: A party's **static public key** (`ls` for local, `rs` for remote)
* `s`: A party's **static public key** (`ls` for local, `rs` for remote).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

stylesheet rule is to only use periods at end of list items if they are complete sentences, so original is consistent with stylesheet

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks; must have intended that for the previous item!

Comment thread 08-transport.md Outdated
* where `m[0]` is the _first_ byte of `m`, `m[1:33]` are the next `33`
bytes of `m` and `m[34:]` is the last 16 bytes of `m`
* Read _exactly_ 50 bytes from the network buffer.
* Parse out the read message (`m`) into `v = m[0]`, `re = m[1:33]` and `c = m[34:]`.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

...re = m[1:33], and...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sent to Oxford.

Comment thread 08-transport.md

the responder
* The final encryption keys to be used for sending and
receiving messages for the duration of the session are generated.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the final...generated

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's a complete sentence, not in a requirements list, so it should be capitalized and have a period, yes?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oops, sorry you're right.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Great! I was momentarily confused!

Comment thread 08-transport.md Outdated
* Read _exactly_ `66-bytes` from the network buffer.


* Read _exactly_ 66-bytes from the network buffer.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

66 bytes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread 08-transport.md Outdated

the initiator
* The final encryption keys to be used for sending and
receiving messages for the duration of the session are generated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

period is appropriate here

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread 08-transport.md Outdated
* let `l = len(m)`
* where `len` obtains the length in bytes of the Lightning message
* Serialize `l` into 2 bytes encoded as a big-endian integer.
* Encrypt `l` using `ChaChaPoly-1305`, `sn`, and `sk`, to obtain `lc`
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

...1 (using...sk), to...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Changed three instances of this.

Comment thread 08-transport.md Outdated


* Decrypt `c` using `ChaCha20-Poly1305`, `rn`, and `rk` to obtain decrypted
* Decrypt `c` using `ChaCha20-Poly1305`, `rn`, and `rk`, to obtain decrypted
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

...obtain the decrypted

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Changed three instances of this.

Comment thread 08-transport.md Outdated
Changing keys regularly and forgetting the previous key is useful for
preventing decryption of old messages in the case of later key leakage (ie.
Changing keys regularly and forgetting previous keys is useful to
prevent the decryption of old messages in the case of later key leakage (i.e.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

...messages, in...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Changed.

(1) addressed review items from @rustyrussell and @toadlyBroodle ; and (2) added table of contents courtesy of @bcongdon in #310
@shannona
Copy link
Copy Markdown
Collaborator Author

shannona commented Dec 8, 2017

Responses to reviews now in place, as well as table of contents courtesy of @bcongdon in #310

Added fix for duplicate word courtesy of @dimitris-t in #306.
@shannona shannona mentioned this pull request Dec 8, 2017
Comment thread 08-transport.md Outdated
new ephemeral key with strong cryptographic randomness.

* `s`: A party's **static public key** (`ls` for local, `rs` for remote).
* `s`: A party's **static public key** (`ls` for local, `rs` for remote)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

...a party's...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Got it, thanks.

@rustyrussell rustyrussell merged commit 0d74fd7 into lightning:master Dec 10, 2017
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.

3 participants