Update structure of docs #806
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
""" WalkthroughThe changes reorganize and expand documentation for the XMTP project, including the README and supporting docs. Updates introduce clearer structure, new sections on deployment, onboarding, and metrics, and shift node onboarding responsibilities to the dev team. No code or technical instructions were altered; all modifications are organizational and additive to documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Node Operator
participant Dev Team
participant Registry
Node Operator->>Dev Team: Provide node public key and address
Dev Team->>Registry: Register node (with public key and address)
Registry-->>Dev Team: Confirmation of registration
Dev Team->>Node Operator: Notify registration complete and share resources
Possibly related PRs
Suggested reviewers
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
doc/metrics_catalog.md (1)
3-3: Correct the project name typo.The document refers to “xmptd” but the project is named xmtpd.
Apply this diff:-This document catalogs the Prometheus metrics instrumentation for xmptd. +This document catalogs the Prometheus metrics instrumentation for xmtpd.README.md (3)
71-71: Add missing comma for clarity.Insert a comma after “metrics” for better readability.
-To enable metrics you need to start the node with `--metrics.enable`. +To enable metrics, you need to start the node with `--metrics.enable`.🧰 Tools
🪛 LanguageTool
[typographical] ~71-~71: It seems that a comma is missing.
Context: ...t.go. ### Monitor the node To enable metrics you need to start the node with--metr...(IN_ORDER_TO_VB_COMMA)
89-89: Make link path consistent.Other top-level docs use a leading slash for
/doc/*. Add it here:-See [Node Registry Migration](doc/node-registry-migration-v2.md). +See [Node Registry Migration](/doc/node-registry-migration-v2.md).
93-93: Make link path consistent.Use a leading slash for the metrics catalog link:
-See [Metrics catalog](doc/metrics_catalog.md). +See [Metrics catalog](/doc/metrics_catalog.md).doc/onboarding.md (1)
11-11: Add missing comma for readability.Insert a comma after “correct” to separate clauses clearly.
-Prompt them to ensure that the public key and address are correct as they are immutable and can't be changed in the future. +Prompt them to ensure that the public key and address are correct, as they are immutable and can't be changed in the future.🧰 Tools
🪛 LanguageTool
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...ure that the public key and address are correct as they are immutable and can't be chan...(AI_HYDRA_LEO_MISSING_COMMA)
doc/deploy.md (2)
44-44: Add missing comma for clarity.Separate the clauses with a comma after “address”.
-Be sure to register nodes with their correct public key and address as they are immutable and cannot be changed in the future. +Be sure to register nodes with their correct public key and address, as they are immutable and cannot be changed in the future.
139-139: Fix typo (“th eprocess” → “the process”).Correct the spacing in the word “the process”.
-If the contract gets deployed correctly, th eprocess will return a few addresses. +If the contract gets deployed correctly, the process will return a few addresses.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(5 hunks)doc/deploy.md(5 hunks)doc/metrics_catalog.md(1 hunks)doc/onboarding.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[typographical] ~71-~71: It seems that a comma is missing.
Context: ...t.go. ### Monitor the node To enable metrics you need to start the node with --metr...
(IN_ORDER_TO_VB_COMMA)
[misspelling] ~79-~79: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...rl http://localhost:8008 ``` ## Deploy an xmtpd node to XMTP testnet See [Deploy...
(EN_A_VS_AN)
[misspelling] ~81-~81: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...xmtpd node to XMTP testnet See [Deploy an xmtpd node to XMTP testnet](/doc/deploy...
(EN_A_VS_AN)
doc/onboarding.md
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...ure that the public key and address are correct as they are immutable and can't be chan...
(AI_HYDRA_LEO_MISSING_COMMA)
doc/deploy.md
[misspelling] ~1-~1: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: # Deploy an xmtpd node to XMTP testnet **
(EN_A_VS_AN)
[uncategorized] ~43-~43: Possible missing comma found.
Context: ...nodes with their correct public key and address as they are immutable and cannot be cha...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
doc/deploy.md
11-11: Bare URL used
null
(MD034, no-bare-urls)
12-12: Bare URL used
null
(MD034, no-bare-urls)
136-136: Bare URL used
null
(MD034, no-bare-urls)
137-137: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd-cli)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd)
🔇 Additional comments (1)
doc/onboarding.md (1)
21-26: Verify the CLI command path.The registration example uses
dev/cli register-node, but other docs (e.g., deploy.md) referencedev/cmd/cli register-node. Please confirm the correct binary path and update for consistency.
Reorganize documentation structure to improve navigation and clarity across XMTP documentation files
📍Where to StartStart with the README.md file as it contains the highest-level structural changes and new content organization that flows into the other documentation files. Changes since #806 opened
Macroscope summarized 2fc4b8e. |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
doc/deploy.md (1)
69-75:⚠️ Potential issueFix inconsistent environment variable usage
The verification script references
$PRIVATE_KEY, but earlier the variable isADMIN_PRIVATE_KEY. Update it for consistency:- dev/cmd/get-all-nodes \ - --admin.private-key=$PRIVATE_KEY + dev/cmd/get-all-nodes \ + --admin.private-key=${ADMIN_PRIVATE_KEY}
♻️ Duplicate comments (1)
doc/deploy.md (1)
23-25: Nested markdown link correctedThe malformed nested link has been replaced with a single well-formed link as suggested in past reviews.
🧹 Nitpick comments (6)
doc/deploy.md (6)
1-4: Consider standardizing the experimental warning as an admonitionThe inline
**⚠️ Experimental:**banner could be more consistent if styled as a blockquote or a unified callout (e.g., using> **⚠️ Experimental:** …). This improves readability and aligns with common Markdown conventions for warnings.🧰 Tools
🪛 LanguageTool
[misspelling] ~1-~1: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: # Deploy an xmtpd node to XMTP testnet **⚠️ Experi...(EN_A_VS_AN)
5-5: Convert bare URL to descriptive linkThe bare URL
https://xmtp-testnet.explorer.alchemy.com/is flagged by markdownlint. Replace it with a descriptive link, for example:The XMTP testnet environment lives in [XMTP Sepolia Chain Explorer](https://xmtp-testnet.explorer.alchemy.com/).
9-13: Hyperlink table URLs for better Markdown complianceThe DNS names in the table are bare URLs and trigger MD034. Consider wrapping them in link syntax for clarity:
| [grpc.testnet.xmtp.network](https://grpc.testnet.xmtp.network) | US-EAST-2 | … | | [grpc2.testnet.xmtp.network](https://grpc2.testnet.xmtp.network) | EU-NORTH-1 | … |🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
11-11: Bare URL used
null(MD034, no-bare-urls)
12-12: Bare URL used
null(MD034, no-bare-urls)
31-34: Format the localhost URL consistentlyInstead of
<http://localhost:7545/>, use code formatting for consistency with other examples:The blockchain starts at `http://localhost:7545/`.
41-44: Add missing comma for clarityInsert a comma after “address” to improve readability:
- Be sure to register nodes with their correct public key and address as they are immutable… + Be sure to register nodes with their correct public key and address, as they are immutable…🧰 Tools
🪛 LanguageTool
[uncategorized] ~43-~43: Possible missing comma found.
Context: ...nodes with their correct public key and address as they are immutable and cannot be cha...(AI_HYDRA_LEO_MISSING_COMMA)
135-138: Convert bare URLs in variable list to descriptive linksReplace the plain URLs in the environment variables list with named links to improve clarity:
- `$VERIFIER_URL`: [Conduit Testnet Staging API](https://explorer-testnet-staging-88dqtxdinc.t.conduit.xyz/api) - `$RPC_URL`: [Conduit Testnet Staging RPC](https://rpc-testnet-staging-88dqtxdinc.t.conduit.xyz/)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
136-136: Bare URL used
null(MD034, no-bare-urls)
137-137: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/deploy.md(5 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/deploy.md
[misspelling] ~1-~1: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: # Deploy an xmtpd node to XMTP testnet **
(EN_A_VS_AN)
[uncategorized] ~43-~43: Possible missing comma found.
Context: ...nodes with their correct public key and address as they are immutable and cannot be cha...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
doc/deploy.md
11-11: Bare URL used
null
(MD034, no-bare-urls)
12-12: Bare URL used
null
(MD034, no-bare-urls)
136-136: Bare URL used
null
(MD034, no-bare-urls)
137-137: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd-cli)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd)
🔇 Additional comments (3)
doc/deploy.md (3)
47-62: Register-node instructions look goodThe shell snippet clearly defines the required environment variables and usage of
dev/cmd/cli register-node.
80-106: JSON example formatting is clearThe JSON response snippet is well-formatted, includes syntax highlighting, and accurately represents expected output.
🧰 Tools
🪛 Gitleaks (8.26.0)
90-90: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
98-98: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.17.2)
82-82: Hard tabs
Column: 1(MD010, no-hard-tabs)
83-83: Hard tabs
Column: 1(MD010, no-hard-tabs)
84-84: Hard tabs
Column: 1(MD010, no-hard-tabs)
85-85: Hard tabs
Column: 1(MD010, no-hard-tabs)
86-86: Hard tabs
Column: 1(MD010, no-hard-tabs)
87-87: Hard tabs
Column: 1(MD010, no-hard-tabs)
88-88: Hard tabs
Column: 1(MD010, no-hard-tabs)
89-89: Hard tabs
Column: 1(MD010, no-hard-tabs)
90-90: Hard tabs
Column: 1(MD010, no-hard-tabs)
91-91: Hard tabs
Column: 1(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1(MD010, no-hard-tabs)
95-95: Hard tabs
Column: 1(MD010, no-hard-tabs)
96-96: Hard tabs
Column: 1(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1(MD010, no-hard-tabs)
99-99: Hard tabs
Column: 1(MD010, no-hard-tabs)
100-100: Hard tabs
Column: 1(MD010, no-hard-tabs)
101-101: Hard tabs
Column: 1(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1(MD010, no-hard-tabs)
103-103: Hard tabs
Column: 1(MD010, no-hard-tabs)
114-121: Health probe examples are accurateThe gRPC health probe commands are correctly shown with expected statuses.
doc/deploy.md
Outdated
| @@ -1,49 +1,48 @@ | |||
| # How to deploy a local environment | |||
| # Deploy an xmtpd node to XMTP testnet | |||
There was a problem hiding this comment.
I updated this title - but I am still not sure I understand that this document is about. =P
It seems like it is about deploying an xmtpd node to testnet - but it also includes info about deploying a local environment. I think what I need to understand is how these two things work together.
My guess is that the main topic of this doc is how to deploy an xmtpd node to testnet - if you are an xmtpd dev and need to do this for an Ephemera-run node.
But this doc is also about how to deploy an xmtpd node if you need to use it with a local dev env -- but the node is not on the testnet?
There was a problem hiding this comment.
It's mixed info now, I agree that it's counterintuitive. Probably we could split this into two? How to deploy a local environment for development, and other document with info around XMTP Sepolia, Ephemera nodes, the nodes status page, etc.
There was a problem hiding this comment.
Thank you @fbac - love it - let me take another pass at it!
doc/deploy.md
Outdated
| ## Deploy a local development environment | ||
|
|
||
| There are two ways of deploying a local environment: | ||
| Use one of the following methods to deploy a local development environment: |
There was a problem hiding this comment.
What is included in a local development environment? What gets deployed?
There was a problem hiding this comment.
Everything needed by xmtpd to run: databases, mls validation service, chain with all the contracts.
doc/deploy.md
Outdated
| This method automatically pre-registers two nodes. | ||
|
|
||
| This method automatically pre-register two nodes. | ||
| ## Register a node |
There was a problem hiding this comment.
Is this here to tell us how to register an Ephemera-run node, as well as how to register a node for use with a local dev env -- which is not on the testnet?
There was a problem hiding this comment.
Ideally this would tell the user how to register nodes manually to use the local development network.
doc/deploy.md
Outdated
| status: SERVING | ||
| ``` | ||
|
|
||
| ## Deploy a new XMTP testnet contract |
There was a problem hiding this comment.
When will an xmtpd need to do this?
There was a problem hiding this comment.
Ideally, never. The local development environment should make use of a chain container image with all the contracts already deployed in deterministic addresses.
There was a problem hiding this comment.
@fbac Based on your answer, I've deleted the "Deploy a new XMTP testnet contract" section from this doc. It sounds like including it might cause confusion? <3
doc/metrics_catalog.md
Outdated
| @@ -1,3 +1,7 @@ | |||
| # Metrics catalog | |||
|
|
|||
| This document catalogs the Prometheus metrics instrumentation for xmptd. | |||
There was a problem hiding this comment.
Are these Prometheus metrics?
There was a problem hiding this comment.
good question. Our goal is to be openmetrics, but we are actually not OM compliant. Currently we are Prometheus.
For clarity, OM is prometheus with a bunch of extra restrictions.
I will check whether we can switch some flags to make us OM. It is the goal.
There was a problem hiding this comment.
- the docs can now safely say it is openmetrics :)
| @@ -1,63 +1,45 @@ | |||
| # How to onboard a new node in testnet | |||
| # Support a new XMTP testnet node operator | |||
There was a problem hiding this comment.
I updated this doc to be the xmtpd dev side of onboarding a new node operator - whereas the Helm doc is the node operator side of the process. WDYT?
| You can provide them with the following links: | ||
|
|
||
| This step might differ for every operator. A good starting point is our [Deployment Guide](deploy.md) | ||
| - Take the next step in deploying your node to the XMTP testnet: [Step 3: Set up dependencies](https://github.com/xmtp/xmtpd-infrastructure/blob/main/helm/README.md#step-3-set-up-dependencies) |
There was a problem hiding this comment.
This is the next step for the node operator, correct?
| ``` | ||
|
|
||
| # Contributing | ||
| ## About the XMTP testnet |
There was a problem hiding this comment.
Pulled this section out of deploy.md and put it here.
| | ---------------------------------- | ---------- | -------------------------------------------------------------------- | | ||
| | https://grpc.testnet.xmtp.network | US-EAST-2 | 0x03e5442c5d1fe2f02b6b9a1a386383a7766860b40a6079a0223994ffa2ce10512c | | ||
| | https://grpc2.testnet.xmtp.network | EU-NORTH-1 | 0x02fc261d43a0153539a4c64c29763cb0e7e377c0eac2910c3d4bedb2235ac70371 | | ||
|
|
There was a problem hiding this comment.
Added the sections below just to surface useful docs in the repo
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
README.md (2)
69-74: Typographical refinement: add comma after “metrics”
For improved readability, insert a comma in the sentence:-To enable metrics you need to start the node with `--metrics.enable`. +To enable metrics, you need to start the node with `--metrics.enable`.🧰 Tools
🪛 LanguageTool
[typographical] ~71-~71: It seems that a comma is missing.
Context: ...t.go. ### Monitor the node To enable metrics you need to start the node with--metr...(IN_ORDER_TO_VB_COMMA)
85-88: Enhance table entries: convert bare URLs to markdown links
Replacing the raw URLs in the DNS column with markdown links will make them clickable and semantically consistent. For example:-| https://grpc.testnet.xmtp.network | US-EAST-2 | … -| https://grpc2.testnet.xmtp.network | EU-NORTH-1 | … +| [grpc.testnet.xmtp.network](https://grpc.testnet.xmtp.network) | US-EAST-2 | … +| [grpc2.testnet.xmtp.network](https://grpc2.testnet.xmtp.network) | EU-NORTH-1 | …🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
87-87: Bare URL used
null(MD034, no-bare-urls)
88-88: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(5 hunks)doc/deploy.md(4 hunks)doc/metrics_catalog.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- doc/metrics_catalog.md
- doc/deploy.md
🧰 Additional context used
🪛 LanguageTool
README.md
[typographical] ~71-~71: It seems that a comma is missing.
Context: ...t.go. ### Monitor the node To enable metrics you need to start the node with --metr...
(IN_ORDER_TO_VB_COMMA)
[misspelling] ~90-~90: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...eac2910c3d4bedb2235ac70371 | ## Deploy an xmtpd node to XMTP testnet See [Deploy...
(EN_A_VS_AN)
[misspelling] ~92-~92: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...xmtpd node to XMTP testnet See [Deploy an xmtpd node to XMTP testnet](/doc/deploy...
(EN_A_VS_AN)
🪛 markdownlint-cli2 (0.17.2)
README.md
87-87: Bare URL used
null
(MD034, no-bare-urls)
88-88: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd-cli)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd)
🔇 Additional comments (5)
README.md (5)
29-31: Approve Quickstart section addition
The new “Quickstart” section with an “Install and build dependencies” subsection provides a clear entry point for new users. It consolidates the initial setup commands (dev/up/dev/down) nicely under a single heading.
51-57: Approve “Start the node” instructions
The new “Start the node” subsection clearly conveys the command (dev/run) needed to launch the xmtpd daemon. This aligns well with the other Quickstart steps.
59-67: Approve “Test the node” instructions
This subsection succinctly shows how to execute the full suite of unit and integration tests (dev/test) and directs readers to explore_test.gofiles. Well scoped and informative.🧰 Tools
🪛 LanguageTool
[uncategorized] ~67-~67: You might be missing the article “the” here.
Context: ...xmtpd` repo to help ensure and maintain correctness of the code over time and to avoid regr...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
98-101: Approve migration section
The new “Migrate to XMTP node registry V2” section is clear, and the relative link (doc/node-registry-migration-v2.md) is correctly formatted.
102-105: Approve metrics catalog link
The “Metrics catalog” section with a link todoc/metrics_catalog.mdis well placed and correctly referenced.
jhaaaa
force-pushed
the
update-doc-structure
branch
from
486bd47 to
2b6ab99
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
doc/deploy.md (1)
18-18: Ensure consistent heading hierarchy.The headings “Use the XMTP Contracts Image” and “Use the dev/up automation” jump from H1 to H3. They should be H2 to maintain proper heading increment (H1 → H2 → H3). For example:
- ### Use the XMTP Contracts Image + ## Use the XMTP Contracts Image - ### Use the dev/up automation + ## Use the dev/up automationAlso applies to: 28-28
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
18-18: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3(MD001, heading-increment)
README.md (3)
71-71: Add missing comma for clarity.The sentence reads more clearly with a comma after “enable metrics”:
- To enable metrics you need to start the node with `--metrics.enable`. + To enable metrics, you need to start the node with `--metrics.enable`.🧰 Tools
🪛 LanguageTool
[typographical] ~71-~71: It seems that a comma is missing.
Context: ...t.go. ### Monitor the node To enable metrics you need to start the node with--metr...(IN_ORDER_TO_VB_COMMA)
92-92: Fix relative link path.Remove the leading slash so the link resolves correctly on GitHub:
- See [Deploy a local environment for developing with xmtpd](/doc/deploy.md). + See [Deploy a local environment for developing with xmtpd](doc/deploy.md).
96-96: Fix relative link path.Remove the leading slash for the onboarding guide link:
- See [Support a new XMTP testnet node operator](/doc/onboarding.md). + See [Support a new XMTP testnet node operator](doc/onboarding.md).doc/onboarding.md (2)
11-11: Add missing comma for clarity.Insert a comma to improve readability:
- Prompt them to ensure that the public key and address are correct as they are immutable and can't be changed in the future. + Prompt them to ensure that the public key and address are correct, as they are immutable and can't be changed in the future.🧰 Tools
🪛 LanguageTool
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...ure that the public key and address are correct as they are immutable and can't be chan...(AI_HYDRA_LEO_MISSING_COMMA)
45-45: Fix relative link path.Remove the leading slash so the observability link works on GitHub:
- [Set up Prometheus service discovery for xmtpd in Kubernetes using Helm](/doc/k8s-prometheus-monitoring.md) + [Set up Prometheus service discovery for xmtpd in Kubernetes using Helm](doc/k8s-prometheus-monitoring.md)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(5 hunks)doc/deploy.md(4 hunks)doc/metrics_catalog.md(1 hunks)doc/onboarding.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- doc/metrics_catalog.md
🧰 Additional context used
🪛 LanguageTool
doc/onboarding.md
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...ure that the public key and address are correct as they are immutable and can't be chan...
(AI_HYDRA_LEO_MISSING_COMMA)
README.md
[typographical] ~71-~71: It seems that a comma is missing.
Context: ...t.go. ### Monitor the node To enable metrics you need to start the node with --metr...
(IN_ORDER_TO_VB_COMMA)
🪛 markdownlint-cli2 (0.17.2)
doc/deploy.md
18-18: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd-cli)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd)
jhaaaa
force-pushed
the
update-doc-structure
branch
from
2b6ab99 to
90a6213
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (3)
doc/deploy.md (2)
60-71:⚠️ Potential issueFix inconsistent environment variable in verification example.
The script exportsADMIN_PRIVATE_KEYbut the invocation uses$PRIVATE_KEY. This will fail. Apply this diff:- dev/cmd/get-all-nodes \ - --admin.private-key=$PRIVATE_KEY + dev/cmd/get-all-nodes \ + --admin.private-key=$ADMIN_PRIVATE_KEY
102-112: 🛠️ Refactor suggestionUpdate health-probe examples to local endpoints.
This section still references testnet addresses; for a local dev environment, use localhost. For example:- grpc-health-probe -tls -addr grpc.testnet.xmtp.network:443 + grpc-health-probe -addr localhost:5050 status: SERVING - grpc-health-probe -tls -addr grpc2.testnet.xmtp.network:443 + grpc-health-probe -addr localhost:5051 status: SERVINGdoc/onboarding.md (1)
42-46:⚠️ Potential issueFix link path to Kubernetes Prometheus guide.
Remove the leading slash so it’s repository-relative:- [Set up Prometheus service discovery for xmtpd in Kubernetes using Helm](/doc/k8s-prometheus-monitoring.md) + [Set up Prometheus service discovery for xmtpd in Kubernetes using Helm](doc/k8s-prometheus-monitoring.md)
🧹 Nitpick comments (4)
README.md (3)
69-74: Add comma for clarity in monitoring instructions.
A comma after “To enable metrics” improves readability.
Apply this diff:- To enable metrics you need to start the node with `--metrics.enable`. + To enable metrics, you need to start the node with `--metrics.enable`.🧰 Tools
🪛 LanguageTool
[typographical] ~71-~71: It seems that a comma is missing.
Context: ...t.go. ### Monitor the node To enable metrics you need to start the node with--metr...(IN_ORDER_TO_VB_COMMA)
85-88: Convert bare URLs in the testnet node table to markdown links.
This avoids lint warnings and improves readability. For example:-| https://grpc.testnet.xmtp.network | US-EAST-2 | 0x03e5442c5d... +| [grpc.testnet.xmtp.network](https://grpc.testnet.xmtp.network) | US-EAST-2 | 0x03e5442c5d... -| https://grpc2.testnet.xmtp.network | EU-NORTH-1 | 0x02fc261d4... +| [grpc2.testnet.xmtp.network](https://grpc2.testnet.xmtp.network) | EU-NORTH-1 | 0x02fc261d4...🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
87-87: Bare URL used
null(MD034, no-bare-urls)
88-88: Bare URL used
null(MD034, no-bare-urls)
98-104: Fix typo in OpenMetrics catalog heading/link text.
Consistently spell “xmtpd” (not “xmptd”) in both the heading and link text:- ## xmtpd OpenMetrics catalog - See [xmptd OpenMetrics catalog](doc/metrics_catalog.md). + ## xmtpd OpenMetrics catalog + See [xmtpd OpenMetrics catalog](doc/metrics_catalog.md).doc/onboarding.md (1)
7-12: Add comma for grammatical clarity.
Insert a comma before “as” in this sentence:- Prompt them to ensure that the public key and address are correct as they are immutable and can't be changed in the future. + Prompt them to ensure that the public key and address are correct, as they are immutable and can't be changed in the future.🧰 Tools
🪛 LanguageTool
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...ure that the public key and address are correct as they are immutable and can't be chan...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(5 hunks)doc/deploy.md(4 hunks)doc/metrics_catalog.md(1 hunks)doc/onboarding.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- doc/metrics_catalog.md
🧰 Additional context used
🪛 LanguageTool
doc/onboarding.md
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...ure that the public key and address are correct as they are immutable and can't be chan...
(AI_HYDRA_LEO_MISSING_COMMA)
README.md
[typographical] ~71-~71: It seems that a comma is missing.
Context: ...t.go. ### Monitor the node To enable metrics you need to start the node with --metr...
(IN_ORDER_TO_VB_COMMA)
🪛 markdownlint-cli2 (0.17.2)
README.md
87-87: Bare URL used
null
(MD034, no-bare-urls)
88-88: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd-cli)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd)
🔇 Additional comments (12)
README.md (5)
29-31: Quickstart section is clear and well-structured.
The new “Quickstart” heading and the “Install and build dependencies” subsection accurately consolidate the setup steps forxmtpd.
51-57: Start instructions look good.
The “Start the node” subsection clearly shows thedev/runcommand.
59-65: Test commands are correct.
The “Test the node” section accurately demonstrates how to run unit and integration tests.
80-82: About the XMTP testnet section is well phrased.
The new section succinctly introduces the Sepolia chain testnet context.
90-96: Deployment and onboarding links are correctly repository-relative.
The links todoc/deploy.mdanddoc/onboarding.mduse the proper paths without leading slashes.doc/deploy.md (5)
1-17: Title and introduction clearly focus on local environment.
The new heading, warning banner, and description outline exactly what this doc covers.
20-27: XMTP Contracts Image section is clear.
The instructions and link to contract-image documentation accurately guide the user.
29-33:dev/upautomation instructions are concise.
The note about automatic pre-registration of two nodes is useful.
34-39: Register a node introduction is well written.
The rationale and scope of manual registration are explained succinctly.
40-57: Register-node code block is accurate.
Environment variables and CLI flags are clearly laid out.doc/onboarding.md (2)
1-6: Onboarding introduction correctly refocused.
The title, link to infrastructure repo, and scope statement align with the PR objectives.
13-26: Verify CLI invocation path.
The doc usesdev/cli register-node, whereas other docs referencedev/cmd/cli. Please confirm the correct binary path in the repository and update this section if needed.
| @@ -1,3 +1,7 @@ | |||
| # xmptd OpenMetrics catalog | |||
There was a problem hiding this comment.
this file is auto-generated so I added your changes here #825
doc/onboarding.md
Outdated
| "address": "0x9419db765e6b469edc028ffa72ba2944f2bad169" | ||
| } | ||
|
|
||
| export XMTPD_CONTRACTS_RPC_URL="https://rpc-testnet-staging-88dqtxdinc.t.conduit.xyz/" |
There was a problem hiding this comment.
this is no longer correct. It needs the alchemy URL which contains an API key
doc/onboarding.md
Outdated
| export XMTPD_CONTRACTS_NODES_ADDRESS=<depends> | ||
| export PRIVATE_KEY=<secret> | ||
|
|
||
| dev/cli register-node \ |
There was a problem hiding this comment.
dev/cli got moved to ./dev/cmd/cli
the flags are:
--admin.private-key, --http-address, --node-owner-address and --node-signing-key-pub
doc/deploy.md
Outdated
| Use the script provided in `dev/up`, which will automatically handle the deployment for you. The blockchain is started at <http://localhost:7545/> | ||
| Use the script provided in `dev/up` to automatically handle the deployment for you. The blockchain starts at <http://localhost:7545/>. | ||
|
|
||
| This method automatically pre-registers two nodes. |
There was a problem hiding this comment.
technically by default just 1 :)
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
doc/deploy.md (1)
59-67:⚠️ Potential issueFix environment variable mismatch in registration verification
The snippet exportsADMIN_PRIVATE_KEYbut uses$PRIVATE_KEYin the CLI command, leading to a broken command. Unify to useADMIN_PRIVATE_KEYconsistently:-export XMTPD_SETTLEMENT_CHAIN_RPC_URL="http://localhost:7545/" -export XMTPD_SETTLEMENT_CHAIN_CHAIN_ID=31337 -export XMTPD_SETTLEMENT_CHAIN_NODE_REGISTRY_ADDRESS="0xDEADBEEF" -export ADMIN_PRIVATE_KEY="0xDEADBEEF" -dev/cmd/get-all-nodes \ - --admin.private-key=$PRIVATE_KEY +export XMTPD_SETTLEMENT_CHAIN_RPC_URL="http://localhost:7545/" +export XMTPD_SETTLEMENT_CHAIN_CHAIN_ID=31337 +export XMTPD_SETTLEMENT_CHAIN_NODE_REGISTRY_ADDRESS="0xDEADBEEF" +export ADMIN_PRIVATE_KEY="0xDEADBEEF" +dev/cmd/get-all-nodes \ + --admin.private-key=$ADMIN_PRIVATE_KEY
♻️ Duplicate comments (1)
doc/deploy.md (1)
20-22: Fix or confirm markdown link formatting
The[documentation](https://github.com/xmtp/smart-contracts/blob/main/doc/xmtp-contracts-image.md#using-the-image)link is correctly formatted, addressing the previous nested-bracket issue.
🧹 Nitpick comments (5)
doc/deploy.md (3)
1-5: Clear scope and experimental note
The header and introductory lines clearly define that this doc is focused on local development. Consider removing or updating the “⚠️ Experimental” warning once the content has stabilized to avoid confusion.
13-16: Clear deployment options
Listing both the XMTP Contracts Image and thedev/upautomation gives users flexibility. Consider adding direct links to each method’s detailed documentation for quicker access.
44-50: Environment variable placeholders are explicit
Exporting the required variables (RPC_URL,CHAIN_ID,NODE_REGISTRY_ADDRESS, etc.) is clear. You may want to call out that dummy values (0xDEADBEEF) should be replaced with actual local addresses/keys.doc/onboarding.md (2)
7-11: Refine registration prompt clarity
Consider adding a comma for readability:“Prompt them to ensure that the public key and address are correct, as they are immutable and can't be changed in the future.”
🧰 Tools
🪛 LanguageTool
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...ure that the public key and address are correct as they are immutable and can't be chan...(AI_HYDRA_LEO_MISSING_COMMA)
44-45: Consider relative link consistency
The link to/doc/k8s-prometheus-monitoring.mduses an absolute path from the repo root. For consistency with other Markdown files, consider using a relative path (e.g.,doc/k8s-prometheus-monitoring.md).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
doc/deploy.md(4 hunks)doc/onboarding.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
doc/deploy.md (1)
Learnt from: fbac
PR: xmtp/xmtpd#827
File: pkg/indexer/app_chain/app_chain.go:56-60
Timestamp: 2025-05-20T14:03:40.445Z
Learning: In the xmtpd repository, clients referenced as pointers should not be closed in error paths if they might be used by upstream callers.
🪛 LanguageTool
doc/onboarding.md
[uncategorized] ~11-~11: Possible missing comma found.
Context: ...ure that the public key and address are correct as they are immutable and can't be chan...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd-cli)
- GitHub Check: Test (Node)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd)
- GitHub Check: Upgrade Tests
🔇 Additional comments (9)
doc/deploy.md (4)
7-11: Good overview of components
The bullet list succinctly captures all required components for a local environment. This clarity will help new contributors understand the setup at a glance.
24-26: Good default environment guidance
Noting thatdev/local.envhas sensible defaults and pointing to the smart-contracts repo helps users get started quickly.
30-32: Verify pre-registered node count
The note thatdev/upnow pre-registers one node (instead of two previously) should be validated. Ensure the script’s README or help text reflects this change.
36-38: Manual node registration step is clear
The instructions to register nodes after deployment are straightforward. Confirm that downstream examples (CLI flags) are in sync with the environment variables below.doc/onboarding.md (5)
1-5: Clear statement of dev-team support role
The header and intro clearly define that this guide is for xmtpd dev-team-assisted onboarding. Good alignment with project structure.
13-13: Team membership requirement is clear
Stating that only@ephemerahq/backendmembers can register nodes sets clear permissions boundaries.
16-19: Verify environment variable naming
The variableXMTPD_CONTRACTS_NODES_ADDRESSmay differ from other docs (NODE_REGISTRY_ADDRESS). Please confirm the correct key name and update for consistency across documentation.
21-25: CLI flags accurately documented
Theregister-nodecommand flags match the expected parameters. Ensure the$PRIVATE_KEYplaceholder is clear for operators.
36-40: Operator notification step is clear
The instructions to notify the node operator and provide links are concise and helpful.
| ### Verify node deployment | ||
|
|
||
| The easiest way to verify node deployment on the local network is to use the [gRPC Health Probe](https://github.com/grpc-ecosystem/grpc-health-probe). | ||
|
|
||
| The easiest way is to use [GRPC Health Probe](https://github.com/grpc-ecosystem/grpc-health-probe) | ||
| For example, you can run: | ||
|
|
||
| ```shell | ||
| grpc-health-probe -tls -addr grpc.testnet.xmtp.network:443 |
There was a problem hiding this comment.
Update health check for local environment
The examples still point to grpc.testnet.xmtp.network and grpc2.testnet.xmtp.network:443, which applies to the testnet, not local development. Replace with the local gRPC address (e.g., localhost:5050 or $NODE_HTTP_ADDRESS):
- grpc-health-probe -tls -addr grpc.testnet.xmtp.network:443
- status: SERVING
+ grpc-health-probe -addr localhost:5050
+ status: SERVINGCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In doc/deploy.md around lines 102 to 109, the health check examples use testnet
addresses like grpc.testnet.xmtp.network and grpc2.testnet.xmtp.network:443,
which are incorrect for local development. Update these examples to use the
local gRPC address such as localhost:5050 or the environment variable
$NODE_HTTP_ADDRESS to reflect the local environment setup.
There was a problem hiding this comment.
Is this valid feedback?
There was a problem hiding this comment.
@jhaaaa Yes, this is valid feedback. The document is specifically about deploying a "local environment for developing with xmtpd," but the health check examples use testnet addresses (grpc.testnet.xmtp.network:443 and grpc2.testnet.xmtp.network:443).
For consistency with the local development focus, the examples should use local addresses like localhost:5050 (which appears in your verification response example) or reference the $NODE_HTTP_ADDRESS environment variable that's defined earlier in the document. The suggested change would also remove the -tls flag since local development typically doesn't require TLS.
| ## Confirm node registration | ||
|
|
||
| Before official testnet launch, only the members of @ephemerahq/backend can register nodes. | ||
| To verify the node appears in the registry, run: | ||
|
|
||
| ## Step 3) Register Node with smart contract | ||
| ```bash | ||
| dev/cmd/get-all-nodes --admin.private-key=$ADMIN_PRIVATE_KEY |
There was a problem hiding this comment.
Fix variable inconsistency in registration confirmation
The snippet sets PRIVATE_KEY but uses $ADMIN_PRIVATE_KEY. Unify the variable usage. For example:
-export PRIVATE_KEY=<secret>
+export ADMIN_PRIVATE_KEY=<secret>
dev/cmd/get-all-nodes --admin.private-key=$ADMIN_PRIVATE_KEYCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In doc/onboarding.md around lines 28 to 33, the environment variable used in the
command snippet is inconsistent; it sets PRIVATE_KEY but references
$ADMIN_PRIVATE_KEY. Update the snippet to use the same variable name
consistently, either change the variable set to ADMIN_PRIVATE_KEY or change the
usage to $PRIVATE_KEY, ensuring both match for clarity and correctness.
There was a problem hiding this comment.
Ah, yes - I see!
Should the variable be $PRIVATE_KEY (on like 24) or $ADMIN_PRIVATE_KEY (on line 33)?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for PRIVATE_KEY usage in the codebase
echo "Searching for PRIVATE_KEY usage:"
rg "PRIVATE_KEY" --type md --type sh --type go -A 2 -B 2
echo -e "\n\nSearching for ADMIN_PRIVATE_KEY usage:"
rg "ADMIN_PRIVATE_KEY" --type md --type sh --type go -A 2 -B 2
# Search specifically for private key CLI flags
echo -e "\n\nSearching for admin.private-key flag usage:"
rg "admin.private-key" --type md --type sh --type go -A 2 -B 2
# Let's also check if there's a reference implementation in the CLI code
echo -e "\n\nSearching for CLI command implementations:"
rg "register-node|get-all-nodes" --type go -A 5 -B 5
Length of output: 11818
@jhaaaa Based on looking through the codebase, ADMIN_PRIVATE_KEY is the standard variable used elsewhere in the documentation (like in deploy.md) for these administrative commands. I recommend updating the variable name at the beginning to match:
-export PRIVATE_KEY=<secret>
+export ADMIN_PRIVATE_KEY=<secret>
./dev/cmd/cli register-node \
--http-address=<node DNS> \
--node-owner-address=<node address> \
- --admin.private-key=$PRIVATE_KEY \
+ --admin.private-key=$ADMIN_PRIVATE_KEY \
--node-signing-key-pub=<node pub-key>This would maintain consistency both within this document and with other documentation files in the codebase.
Reorganize documentation structure to improve navigation and add new deployment guides in xmtpd documentation
Updates documentation across multiple files to improve organization and clarity:
📍Where to Start
Begin with the README.md which provides the entry point to all documentation and shows the new organizational structure.
Macroscope summarized 1fcfefe.
Summary by CodeRabbit