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 10 copy edit #440

Merged
merged 6 commits into from
Aug 7, 2018
Merged

BOLT 10 copy edit #440

merged 6 commits into from
Aug 7, 2018

Conversation

toadlyBroodle
Copy link
Collaborator

Copy edit BOLT10 to bring in line with stylesheet guidelines

@toadlyBroodle toadlyBroodle changed the title Bolt10 copy edit BOLT 10 copy edit Jun 8, 2018
Copy link
Collaborator

@shannona shannona left a comment

Choose a reason for hiding this comment

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

More good stuff. Some minor suggestions for you.

- `a`: address types
- used to specify what address types should be returned for `SRV` queries
- a bitfield that uses the types from [BOLT #7](07-routing-gossip.md) as bit
index
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think swapping bullet points 1 and 2 will result in a clearer read.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(And it's more consistent with the l description.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good call

- default value: 6 (i.e. `2 || 4`, since bit 1 and bit 2 are set for IPv4 and
IPv6, respectively)
- `l`: `node_id`
- bech32-encoded `node_id` of a specific node
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefacing this with "a" will make it more consistent with the r description

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

- MUST evaluate the conditions from the _seed root domain_ by
'going up-the-tree', i.e. evaluating right-to-left in a fully qualified domain
name.
- E.g. evaluate the above case: first `n10`, then `a2`, and finally `r0`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps: "to evaluate the above case: first evaluate n10, then a2, and finally r0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

better, thanks!

@rustyrussell rustyrussell self-requested a review June 28, 2018 02:09
Should no entries match all the conditions then an empty reply MUST be returned.
The DNS seed:
- MAY additionally return the corresponding `A` and `AAAA` records that
indicate the IP address for the `SRV` entries in the Extra section [FIXME: not sure what this refers to, is 'Extra section' defined elsewhere?] of the reply.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Me too.... @cdecker ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The additional section in RFC 1035 allows us to return results whose type does not match the query type. This is commonly used to piggyback the A record together with the reply for an MX record. Otherwise the client would have to issue two queries (a) "give me the name of the server", and (b) "now tell me where to find that name". In our case SRV returns only names, not the matching addresses, so we just add them in the additional section.

Copy link
Collaborator

Choose a reason for hiding this comment

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

And yes, I misnamed the section in the original draft :-)

@rustyrussell rustyrussell requested a review from cdecker June 28, 2018 02:19
@cdecker
Copy link
Collaborator

cdecker commented Jul 1, 2018

Excellent changes, if you fix my misnaming of the "additional section" I'm more than happy to merge this asap.

@toadlyBroodle
Copy link
Collaborator Author

@cdecker done, thanks.

@rustyrussell rustyrussell merged commit 76b1ce9 into lightning:master Aug 7, 2018
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.

4 participants