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

docs(CSN-568): Update docs and README #282

Closed
wants to merge 6 commits into from

Conversation

cisterciansis
Copy link
Collaborator

No description provided.

@cisterciansis cisterciansis deleted the docs(CSN-568)-Update-main-README branch February 1, 2025 22:32
@cisterciansis cisterciansis restored the docs(CSN-568)-Update-main-README branch February 3, 2025 14:59
@cisterciansis cisterciansis reopened this Feb 3, 2025
@cisterciansis cisterciansis deleted the docs(CSN-568)-Update-main-README branch February 3, 2025 15:04
@cisterciansis cisterciansis restored the docs(CSN-568)-Update-main-README branch February 3, 2025 15:06
@cisterciansis cisterciansis reopened this Feb 3, 2025
Copy link
Collaborator

@IvanAnishchuk IvanAnishchuk left a comment

Choose a reason for hiding this comment

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

First of all, please correct branch name. That format with (braces) is for commit messages and PR titles, branch names should follow more url/filename-friendly format as follows: docs/CSN-568-Update-main-README while title should in this case read docs(CSN-568): update docs and README (and perhaps a section of README should be contributors guide documenting things like these naming conventions so that it's easy to reference)

@IvanAnishchuk
Copy link
Collaborator

Also please take care of the merge conflicts

Copy link
Collaborator

@IvanAnishchuk IvanAnishchuk left a comment

Choose a reason for hiding this comment

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

Please look into the areas I outlines in the individual comments. We should be be very careful with port opening instructions, that's the most common thing to misconfigure - good wording and examples around those parts should reduce the number of support requests significantly.

README.md Outdated
Comment on lines 295 to 303
1. **Install and configure `ufw`**:
```bash
sudo apt install ufw
sudo ufw allow 4444
sudo ufw allow 22/tcp
sudo ufw enable
sudo ufw status
```
You can open any custom range: `sudo ufw allow XXXX:YYYY/tcp` and use them as your `axon.port`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This example should create working configuration if copy-pasted verbatim. Include example of open axon port (8091 is a good default I think) and change the comment to explain that 8091 can be changed to any port number as long as other configuration below is changed accordingly

and [register the wallet to a netuid](https://github.com/opentensor/docs/blob/main/subnetworks/registration.md).
Once you have done so, you can run the miner and validator with the following commands.
## Registering Your Hotkey
You need **$TAO** tokens in your coldkey to register the hotkey on the chosen netuid.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe there's some CLI to check exactly how much Tao is currently required for registration. It would be cool to see it here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

btcli subnet lock-cost

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, that's for creating a new subnet... The value required to burn for regi is in RECYCLE column in btcli subnet list (it can be optionally filtered with e.g. grep but I'd rather add a python script later to do it properly)

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thats right i forgot. thank you

README.md Outdated

## Troubleshooting
- **No requests received (no ‘Challenge’ or ‘Specs’ events)**:
- Check your open ports (default 4444).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Axon port should be explicitly mentioned here (it's the most common support request, better be thorough with explanations). Also include a command that could be used to run this check (ufw open ports check command and how to get running miner CLI parameters - there should be the same port number which is the essence of "check your open ports").

README.md Outdated
- **Test**: `test`
- Or use a custom endpoint, e.g. `subvortex.info:9944` (recommended)
- **`--wallet.name`** & **`--wallet.hotkey`**: The coldkey/hotkey names you created.
- **`--axon.port`**: A port opened in your custom range as instructed above (e.g., 8091).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the first time this default value is mentioned, but I think it should be corrected above with future reference to this so that new user remembers it going forward.

@IvanAnishchuk
Copy link
Collaborator

Also #278 has a different take on the same instructions. I'd strongly prefer explanations, defaults, and examples were in sync in both documents - we don't want any dysfunction in case someone decides to mix and match and also automated script should produce the same result as copypasting default examples with clear explanations where things diverge.

@IvanAnishchuk IvanAnishchuk changed the title /docs(CSN-568) update docs and README docs(CSN-568): Update docs and README Feb 3, 2025
@cisterciansis cisterciansis deleted the docs(CSN-568)-Update-main-README branch February 3, 2025 17:56
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.

2 participants