chore(docs): amend port guidance to enable QUIC support by zilayo · Pull Request #5029 · sigp/lighthouse

zilayo · 2023-12-28T15:03:02Z

Issue Addressed

Following release v4.5.0 Lighthouse will attempt to make QUIC connections.

This isn't immediately obvious unless reading through the beacon-node CLI options and/or release notes as it isn't mentioned in the advanced networking section. The guidance in FAQs will also lead to port conflict errors due to still suggesting port 9001 (default QUIC port) for setting up a secondary node:

lighthouse/book/src/faq.md

Line 479 in 441fc16

    
           Use the flag ```--port <PORT>``` in the beacon node. This flag can be useful when you are running two beacon nodes at the same time. You can leave one beacon node as the default port 9000, and configure the second beacon node to listen on, e.g., ```--port 9001```.

The Docker instructions also don't forward port 9001, so there is likely a large number of nodes running without QUIC which will hinder the ongoing monitoring for latency/bandwidth improvements:

lighthouse/book/src/docker.md

Lines 114 to 116 in 441fc16

    
           ```bash 
        
           docker run -p 9000:9000/tcp -p 9000:9000/udp -p 127.0.0.1:5052:5052 -v $HOME/.lighthouse:/root/.lighthouse sigp/lighthouse lighthouse --network mainnet beacon --http --http-address 0.0.0.0 
        
           ```

Proposed Changes

Additional guidance in https://github.com/sigp/lighthouse/blob/stable/book/src/docker.md to encourage Docker users to run Lighthouse with QUIC enabled by default.
Added information about QUIC and default QUIC ports in https://github.com/sigp/lighthouse/blob/stable/book/src/advanced_networking.md
Added information about QUIC in https://github.com/sigp/lighthouse/blob/stable/book/src/faq.md

https://github.com/zilayo/lighthouse/blob/8c13142449930b9114618545a7f11c4140c465bc/book/src/faq.md?plain=1#L103
https://github.com/zilayo/lighthouse/blob/8c13142449930b9114618545a7f11c4140c465bc/book/src/faq.md?plain=1#L429
https://github.com/zilayo/lighthouse/blob/8c13142449930b9114618545a7f11c4140c465bc/book/src/faq.md?plain=1#L103

Additional Info

Please note - there are some formatting diffs due to the settings in https://github.com/sigp/lighthouse/blob/stable/.editorconfig (particularly a lot in faq.md).

I would suggest a new workflow job is added for book push and/or PRs to format all .md files with prettier using the .editorconfig settings to avoid unnecessary diffs in future PRs.

CLAassistant · 2023-12-28T15:03:08Z

All committers have signed the CLA.

chong-he

Thanks for the update, just a minor suggestion here.

Looping @AgeManning or @jmcph4 to have a look at the QUIC networking modifications, particularly there is a plan to change the default QUIC port to move away from port + 1 to a big number port to avoid using conflicting ports.

chong-he · 2024-01-07T23:15:47Z

book/src/faq.md

+
+Use the flag `--port <PORT>` in the beacon node. This flag can be useful when you are running two beacon nodes at the same time. You can leave one beacon node as the default port 9000, and configure the second beacon node to listen on, e.g., `--port 9100`.
+Since V4.5.0, Lighthouse supports QUIC and by default will use the value of `--port` + 1 to listen via UDP (default `9001`).
+This can be configured by using the flag `--quic-port`.


A suggestion here to link back to the Advanced Networking page for more information.

Suggested change

This can be configured by using the flag `--quic-port`.

This can be configured by using the flag `--quic-port`. Refer to [Advanced Networking](./advanced_networking.md#nat-traversal-port-forwarding) for more information.

Added this to the most recent commit

lighthouse/book/src/faq.md

Line 481 in a6cd18a

This can be configured by using the flag `--quic-port`. Refer to [Advanced Networking](./advanced_networking.md#nat-traversal-port-forwarding) for more information.

AgeManning · 2024-01-09T01:52:56Z

Yeah. I think the default port of 9001 may have been a bad choice.

We probably will need to make the shift to something better in a new release

AgeManning

I did a quick skim. This looks good to me.

Thanks for the addition :)

michaelsproul · 2024-01-10T04:32:14Z

book/src/docker.md


-* `-unstable` for the `unstable` branch
-* empty for a tagged release or the `stable` branch
+-   `-unstable` for the `unstable` branch


I don't see how our editorconfig is causing these changes. As far as I can tell we haven't said "prefer - to *". Similarly with indentation, why are the bodies of the bullet points getting indented?

Do you mind letting us know which editor you're using and which markdown linter it is using? I agree with the idea of linting the book markdown so it's consistent, but would prefer to do that in a separate PR and keep the changes in this PR to a minimum (you could git cherry-pick -p to select just the relevant lines).

Seems to be due to vscode doing format on save with prettier whenever a configuration file is present (by default prettier will look for either a .prettierrc file, or otherwise an .editorconfig file).

Would have been avoided if I didn't have a global installation of prettier / auto format on save.

Prettier uses - instead of * for unordered lists (introduced in prettier/prettier#4440) and the indentation seems to be an ongoing issue (prettier/prettier#5019).

I've rebased onto the most recent merged unstable commit and applied only the relevant changes to this PR. Definitely makes the diff a lot cleaner!

dapplion · 2024-01-29T10:03:40Z

@zilayo please undo all unrelated formating changes

zilayo · 2024-02-04T16:11:53Z

@zilayo please undo all unrelated formating changes

https://github.com/sigp/lighthouse/pull/5029/files#diff-4c0cde4e3c0cafbbcfe5b611cd748df240868938b4ec4e6312c3d0a9961cee81L80-L85

https://github.com/sigp/lighthouse/pull/5029/files#diff-4c0cde4e3c0cafbbcfe5b611cd748df240868938b4ec4e6312c3d0a9961cee81L124-R152

https://github.com/sigp/lighthouse/pull/5029/files#diff-7f971b63805bee219d632810978639b80a70c4dd2752efc0d2f7f58fb989dd61L269-R281

@dapplion Most formatting related changes have been removed, however there's multiple trailing whitespaces deletions in faq.md due to the .editorconfig settings.

lighthouse/.editorconfig

Lines 2 to 7 in 8fb6989

    
           [*] 
        
           indent_style=space 
        
           indent_size=4 
        
           end_of_line=lf 
        
           charset=utf-8 
        
           trim_trailing_whitespace=true

I think it's best to keep these deletions in the PR? Otherwise anytime someone updates faq.md it'll keep reintroducing the whitespace deletions.

Have also squashed everything into a single commit.

dapplion

Copy looks good to me now, thanks!

AgeManning · 2024-02-08T01:43:31Z

@Mergifyio queue

mergify · 2024-02-08T01:43:42Z

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at e470596

* chore(docs): amend port guidance to enable QUIC support

chong-he added ready-for-review The code is ready for review docs Documentation labels Jan 3, 2024

chong-he reviewed Jan 7, 2024

View reviewed changes

AgeManning reviewed Jan 9, 2024

View reviewed changes

michaelsproul reviewed Jan 10, 2024

View reviewed changes

michaelsproul mentioned this pull request Jan 10, 2024

Lint book markdown #5053

Closed

zilayo force-pushed the quic-docs-update branch from 6aa11db to 2184400 Compare January 11, 2024 13:46

dapplion added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jan 29, 2024

chore(docs): amend port guidance to enable QUIC support

7599a80

zilayo force-pushed the quic-docs-update branch from a6cd18a to 7599a80 Compare February 4, 2024 16:01

dapplion approved these changes Feb 6, 2024

View reviewed changes

chong-he added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Feb 6, 2024

mergify bot added a commit that referenced this pull request Feb 8, 2024

Merge of #5029

5d5bd6d

This was referenced Feb 8, 2024

merge queue: embarking unstable (675a231) and [#5177 + #5192 + #4870 + #5029] together #5217

Closed

Fix backfill stalling #5192

Merged

improve libp2p connected peer metrics #4870

Merged

Improve network parameters #5177

Merged

mergify bot merged commit e470596 into sigp:unstable Feb 8, 2024

danielrachi1 pushed a commit to danielrachi1/lighthouse that referenced this pull request Feb 14, 2024

chore(docs): amend port guidance to enable QUIC support (sigp#5029)

4f3c0c5

* chore(docs): amend port guidance to enable QUIC support

	```bash
	docker run -p 9000:9000/tcp -p 9000:9000/udp -p 127.0.0.1:5052:5052 -v $HOME/.lighthouse:/root/.lighthouse sigp/lighthouse lighthouse --network mainnet beacon --http --http-address 0.0.0.0
	```

	This can be configured by using the flag `--quic-port`.
	This can be configured by using the flag `--quic-port`. Refer to [Advanced Networking](./advanced_networking.md#nat-traversal-port-forwarding) for more information.

Conversation

zilayo commented Dec 28, 2023

Issue Addressed

Proposed Changes

Additional Info

Uh oh!

CLAassistant commented Dec 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chong-he left a comment

Choose a reason for hiding this comment

Uh oh!

chong-he Jan 7, 2024

Choose a reason for hiding this comment

Uh oh!

zilayo Jan 11, 2024

Choose a reason for hiding this comment

Uh oh!

AgeManning commented Jan 9, 2024

Uh oh!

AgeManning left a comment

Choose a reason for hiding this comment

Uh oh!

michaelsproul Jan 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zilayo Jan 11, 2024

Choose a reason for hiding this comment

Uh oh!

dapplion commented Jan 29, 2024

Uh oh!

zilayo commented Feb 4, 2024

Uh oh!

dapplion left a comment

Choose a reason for hiding this comment

Uh oh!

AgeManning commented Feb 8, 2024

Uh oh!

mergify bot commented Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ The pull request has been merged automatically

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

CLAassistant commented Dec 28, 2023 •

edited

Loading

michaelsproul Jan 10, 2024 •

edited

Loading

mergify bot commented Feb 8, 2024 •

edited

Loading