Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Clarification how messages are referred to #941

Closed

Conversation

lightning-developer
Copy link
Contributor

I added a clarification to BOLT 1 that explains that messages are usually referred to with their message_name and not as message_name message.

I was confused while reviewing #932 where @pm47 wrote:

  • SHOULD send an error to request the peer to fail the channel.

I was looking where errors in BOLT 2 where defined and realized that error meant error message of type 17 in BOLT1.

I checked in the text and realized that "message" is often dropped.

However there are also 111 occurrences in the text where "message" is appended after message_name as can be found with:

grep "\` message" *md -c
00-introduction.md:0
01-messaging.md:17
02-peer-protocol.md:45
03-transactions.md:3
04-onion-routing.md:1
05-onchain.md:1
07-routing-gossip.md:37
08-transport.md:0
09-features.md:6
10-dns-bootstrap.md:0
11-payment-encoding.md:1
CONTRIBUTING.md:0

Since there are 111 occurrences of the opposite behavior I am not sure if this should be defined / unified.

I added a clarification to BOLT 1 that explains that messages are usually referred to with their `message_name` and not as `message_name` message.
@rustyrussell
Copy link
Collaborator

OK, so meta things (how to write the spec, vs how to read the spec) don't belong in the spec itself, but in .copy-edit-stylesheet-checklist.md or CONTRIBUTING.md (the latter could use some love!).

But this is more about reading the spec, in which case it's valid, however brevity is wit, so I suggest shortening this significantly?

@lightning-developer
Copy link
Contributor Author

Do you just want me to shorten this here in BOLT1 now or move it to one of the files that you named?

Also should I go through the 111 occurrences where message is being appended to the message_name and see if we can unify this with a bit of clean up? I guess Where message names are being introduced like headings it makes sense to keep it like update_add_htlc message but in other cases the word message could be removed.

@rustyrussell
Copy link
Collaborator

I think just say:

Note: we refer to messages by their message_name, instead of their type (e.g. "The receiving node sends error" instead of "... sends error message" or "... sends message 17").

And yes, a correctional commit following this (or series of commits) would be great!

@rustyrussell rustyrussell added the spelling These changes may be merged without additional sign off from the weekly meeting label Dec 6, 2021
Changed the formulation to the suggestion by @rustyrussel. for more context see: lightning#941 (comment)
Kept one instance where it it was written ...`init` message exchange and the situation where it was talked about `update_` messages
there were situation where there was written `message_name` messages which I have replaced with `message_name`s.
@lightning-developer
Copy link
Contributor Author

I have just removed the usage of message_name message usage from most bolts. Sometimes I had to fix the sentence a bit. There were a few cases where it made sense to leave the word message explicit behind the message_name for example once there was the talk about "init message exchange" where I felt that message semantically belonged to the word exchange.

I made them in separate commits for each BOLT which should make merging or rebasing simpler if that should become necessary

This should be ready for review now.

@@ -325,11 +325,11 @@ The 2-byte `len` field indicates the number of bytes in the immediately followin
The channel is referred to by `channel_id`, unless `channel_id` is 0 (i.e. all bytes are 0), in which case it refers to all channels.

The funding node:
- for all error messages sent before (and including) the `funding_created` message:
Copy link
Contributor

Choose a reason for hiding this comment

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

nack

I don't think this introduces some improvement on spec, and as usual, the free text gives us the power like humans to interpret the free text in a different way. IMO, are two ways to say the same things, and this change can cause another change in the future for other people who preferer the first style.

- MUST use `temporary_channel_id` in lieu of `channel_id`.

The fundee node:
- for all error messages sent before (and not including) the `funding_signed` message:
Copy link
Contributor

Choose a reason for hiding this comment

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

nack

@@ -339,7 +339,7 @@ A sending node:
- SHOULD send `error` with the unknown `channel_id` in reply to messages of type `32`-`255` related to unknown channels.
- MAY send an empty `data` field.
- when failure was caused by an invalid signature check:
- SHOULD include the raw, hex-encoded transaction in reply to a `funding_created`, `funding_signed`, `closing_signed`, or `commitment_signed` message.
- SHOULD include the raw, hex-encoded transaction in reply to a `funding_created`, `funding_signed`, `closing_signed`, or `commitment_signed`.
Copy link
Contributor

Choose a reason for hiding this comment

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

nack

Comment on lines -381 to +385
The `pong` message is to be sent whenever a `ping` message is received. It
`pong` is to be sent whenever a `ping` is received. It
serves as a reply and also serves to keep the connection alive, while
explicitly notifying the other end that the receiver is still active. Within
the received `ping` message, the sender will specify the number of bytes to be
included within the data payload of the `pong` message.
the received `ping`, the sender will specify the number of bytes to be
included within the data payload of the `pong`.
Copy link
Contributor

Choose a reason for hiding this comment

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

nack

Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

nack

IMO these are two ways to say the same things, I save only the clarification made in the first commit.

@t-bast
Copy link
Collaborator

t-bast commented Sep 18, 2024

This has been inactive and I'm not sure there's actionable follow-up, so I'm closing this. Can be re-worked an re-opened if deemed useful.

@t-bast t-bast closed this Sep 18, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
spelling These changes may be merged without additional sign off from the weekly meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants