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

Siren Docs Upgrade #5979

Merged
merged 21 commits into from
Jul 24, 2024
Merged

Siren Docs Upgrade #5979

merged 21 commits into from
Jul 24, 2024

Conversation

rickimoore
Copy link
Member

Issue Addressed

Which issue # does this PR address?

Proposed Changes

Update siren docs

Additional Info

Added info about sirens new structure

@rickimoore rickimoore changed the title fix: removed unused sections Siren Docs Upgrade Jun 21, 2024
@antondlr antondlr changed the base branch from unstable to release-v5.2.1 June 24, 2024 10:43
@antondlr antondlr added the v5.2.1 Patch release for v5.2.0 label Jun 24, 2024
@michaelsproul
Copy link
Member

This should probably target unstable even if we want it in v5.2.1, because the book is built from unstable, not stable

@michaelsproul michaelsproul changed the base branch from release-v5.2.1 to unstable June 24, 2024 11:45
@michaelsproul michaelsproul added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Jun 26, 2024
Copy link
Member

@chong-he chong-he left a comment

Choose a reason for hiding this comment

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

There are some errors with the markdown linter. Run mdlint from the Lighthouse root depository and it should automatically resolve some of the errors. Some others may need manual editing though

book/src/ui-installation.md Outdated Show resolved Hide resolved
book/src/ui-usage.md Outdated Show resolved Hide resolved
book/src/ui-usage.md Outdated Show resolved Hide resolved
book/src/ui-usage.md Outdated Show resolved Hide resolved
book/src/ui-usage.md Outdated Show resolved Hide resolved
book/src/ui-usage.md Outdated Show resolved Hide resolved
book/src/ui-usage.md Outdated Show resolved Hide resolved
book/src/ui-usage.md Outdated Show resolved Hide resolved
@rickimoore rickimoore requested a review from chong-he July 3, 2024 12:58
Copy link
Member

@chong-he chong-he left a comment

Choose a reason for hiding this comment

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

Looks good now. I just have a comment on the exiting without updating the withdrawal credentials. I wonder if it is worth to open an issue about it? There probably aren't that many users haven't updated their credentials, but there are some that still haven't. It could also be useful in testnet where the withdrawal credentials are of 0x00 type.

@@ -27,7 +27,7 @@ In the absence of a VPN, an alternative approach involves utilizing an SSH tunne

## 6. How do I connect Siren to Lighthouse via a ssh tunnel?

If you would like to access Siren beyond the local network (i.e across the internet), we recommend using an SSH tunnel. This requires a tunnel for 3 ports: `80` (assuming the port is unchanged as per the [installation guide](./ui-installation.md#docker-recommended)), `5052` (for beacon node) and `5062` (for validator client). You can use the command below to perform SSH tunneling:
If you would like to access Siren beyond the local network (i.e across the internet), we recommend using an SSH tunnel. This requires a tunnel for 3 ports: `80` (assuming the port is unchanged as per the [installation guide](./ui-installation.md#building-from-docker-recommended)), `5052` (for beacon node) and `5062` (for validator client). You can use the command below to perform SSH tunneling:
Copy link
Member

Choose a reason for hiding this comment

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

With the new release, do we need to change the port 80? The doc mentions it is on 3443 now

Copy link
Member Author

Choose a reason for hiding this comment

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

@antondlr can you help here?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. I'm gonna scrap the whole ssh tunnel idea, we no longer need that.
Siren is now supposed to run alongside your BN/VC, exposing a webapp that can be accessed from any browser.

Copy link
Member

Choose a reason for hiding this comment

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

Should we remove FAQ no. 6 about ssh tunnel if it is no longer relevant?

book/src/ui-usage.md Outdated Show resolved Hide resolved
book/src/ui-usage.md Outdated Show resolved Hide resolved
book/src/ui-usage.md Outdated Show resolved Hide resolved
@chong-he
Copy link
Member

@antondlr , if you run mdlint from the root depository, it should fix most/all erros in the markdown linter

@rickimoore rickimoore requested a review from chong-he July 17, 2024 10:25
Copy link
Member

@chong-he chong-he left a comment

Choose a reason for hiding this comment

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

Did another testing and this comment is from a user perspective:

How does a user ssh into a node and run Siren? I tested ssh into a node and started Siren with the flag --net=host (got from @antondlr) , Siren started in the terminal, but I can't access SIren via the web browser of the client (I can access via the web browser of the server by just typing localhost)

book/src/ui-installation.md Outdated Show resolved Hide resolved
@rickimoore rickimoore requested a review from antondlr July 17, 2024 14:51
@chong-he chong-he added ready-for-review The code is ready for review docs Documentation and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. v5.2.1 Patch release for v5.2.0 labels Jul 17, 2024
@AgeManning
Copy link
Member

@Mergifyio queue

Copy link

mergify bot commented Jul 21, 2024

queue

🛑 The pull request has been removed from the queue default

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

mergify bot added a commit that referenced this pull request Jul 21, 2024
@AgeManning
Copy link
Member

@Mergifyio refresh

Copy link

mergify bot commented Jul 23, 2024

refresh

✅ Pull request refreshed

@AgeManning
Copy link
Member

@rickimoore - I think you need to merge the latest unstable

@AgeManning
Copy link
Member

@Mergifyio queue

Copy link

mergify bot commented Jul 24, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 9a53f4d

mergify bot added a commit that referenced this pull request Jul 24, 2024
@mergify mergify bot merged commit 9a53f4d into sigp:unstable Jul 24, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation ready-for-review The code is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants