Skip to content

chore(l1): improve release docs#6960

Merged
edg-l merged 1 commit into
mainfrom
improve-docs
Jul 6, 2026
Merged

chore(l1): improve release docs#6960
edg-l merged 1 commit into
mainfrom
improve-docs

Conversation

@edg-l

@edg-l edg-l commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Motivation

Improve the release related docs a bit on findings i found.

Description

Checklist

  • Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR includes breaking changes to the Store requiring a re-sync.

@edg-l edg-l requested a review from a team as a code owner July 3, 2026 13:38
@edg-l edg-l added the docs Improvements or additions to documentation label Jul 3, 2026
@github-actions github-actions Bot added the L1 Ethereum client label Jul 3, 2026
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Kimi Code Review

The changes are documentation-only and generally look correct, but I have specific concerns regarding operational safety and maintainability:

docs/developers/release-process.md

Line 134 (approximate): Hardcoded version

wget -q -O ethrex-linux-x86_64 \
  https://github.com/lambdaclass/ethrex/releases/download/v20.0.0-rc.1/ethrex-linux-x86_64

The version v20.0.0-rc.1 is hardcoded. This will become outdated and potentially confusing. Suggest using a placeholder like <VERSION> or adding a note that this is an example version.

Lines 133-140: Missing integrity verification
The documentation downloads a binary but doesn't verify its checksum. For production mainnet nodes, this is a security risk. Add:

# Download checksum file and verify
wget -q -O checksums.txt https://github.com/lambdaclass/ethrex/releases/download/<VERSION>/checksums.txt
sha256sum -c checksums.txt --ignore-missing

Missing operational safety steps:
Consider adding notes about:

  1. Backup: Backup the database/state before upgrading (critical for consensus safety)
  2. Sync status: Ensure the node is fully synced before stopping
  3. Health check: Verify the new binary starts correctly before detaching from tmux
  4. Rollback: Brief note on how to rollback if the new version fails

Line 142: Signal handling note
The ctrl-c method relies on proper SIGINT handling. Consider adding a note that if graceful shutdown fails, operators may need to wait for database compaction or use SIGTERM.

docs/developers/l2/upgrade-test.md

Line 419: --no-monitor flag
The addition looks correct. Ensure this flag is documented in the CLI help text (outside this PR's scope, but worth verifying).

General Observations

  1. Process completeness: The upgrade procedure assumes the JWT secret and data directory permissions are already correct from the previous run. This is reasonable for documentation, but consider noting that file permissions should be verified if the binary is replaced.

  2. Logging: The tee -a for logging is good practice. Consider mentioning log rotation to prevent disk space issues on long-running mainnet nodes.

The documentation is operationally useful but needs the hardcoded version fixed and should emphasize binary verification for security.


Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@greptile-apps

greptile-apps Bot commented Jul 3, 2026

Copy link
Copy Markdown

Greptile Summary

This PR improves two release-related documentation files with small but practical fixes found during real usage.

  • upgrade-test.md: fixes a broken multi-line shell command by adding the missing \\ continuation character and appending the --no-monitor flag.
  • release-process.md: replaces a <!-- TODO --> placeholder with a real upgrade procedure for the Prysm/Lodestar/Teku/Grandine mainnet nodes, including tmux session management and wget binary download steps.

Confidence Score: 4/5

Documentation-only changes; safe to merge with the minor placeholder inconsistency noted.

Both files contain documentation improvements with no code logic changes. The upgrade-test.md fix is correct and straightforward. The release-process.md addition is useful real content, though the hardcoded version number in the wget URL and --version comment differs from the placeholder convention used elsewhere in the same document, which could cause a release engineer to accidentally use the wrong version on a future release.

docs/developers/release-process.md — the new wget example uses a specific hardcoded version rather than the placeholder pattern the document prescribes.

Important Files Changed

Filename Overview
docs/developers/release-process.md Replaces a TODO with real upgrade instructions for the Prysm/Lodestar/Teku/Grandine mainnet nodes; the bash snippet uses a hardcoded version (v20.0.0-rc.1) instead of the document's established vX.Y.Z-rc.W placeholder.
docs/developers/l2/upgrade-test.md Adds missing \ line continuation after --proof-coordinator.addr 127.0.0.1 and appends the --no-monitor flag to the command block.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SSH into mainnet node] --> B[tmux ls — list sessions]
    B --> C[tmux a -t ethrex — attach]
    C --> D[Ctrl-C — stop ethrex gracefully]
    D --> E[exit tmux]
    E --> F[wget new ethrex-linux-x86_64 binary]
    F --> G[chmod +x binary]
    G --> H[./ethrex-linux-x86_64 --version — verify]
    H --> I[tmux new-session — restart ethrex]
    I --> J[Verify node is syncing]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[SSH into mainnet node] --> B[tmux ls — list sessions]
    B --> C[tmux a -t ethrex — attach]
    C --> D[Ctrl-C — stop ethrex gracefully]
    D --> E[exit tmux]
    E --> F[wget new ethrex-linux-x86_64 binary]
    F --> G[chmod +x binary]
    G --> H[./ethrex-linux-x86_64 --version — verify]
    H --> I[tmux new-session — restart ethrex]
    I --> J[Verify node is syncing]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
docs/developers/release-process.md:135-138
**Hardcoded version in example — should use the `vX.Y.Z-rc.W` placeholder**

The `wget` URL and the `--version` comment both use the literal `v20.0.0-rc.1` / `v20.0.0-` rather than the `vX.Y.Z-rc.W` placeholder that the rest of the document establishes at line 103. A release engineer following this section during a different release would have to remember to swap the URL manually, which is error-prone and inconsistent with the doc's own convention.

Reviews (1): Last reviewed commit: "improve release docs" | Re-trigger Greptile

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

Review: PR 6960 — chore(l1): improve release docs

This is a documentation-only change (no code/logic touched), so the usual EVM/consensus/security concerns don't apply. Two concrete issues in the new release-process content are worth fixing before merge:

Findings

  1. Hardcoded real version breaks the doc's placeholder convention (docs/developers/release-process.md:134-138)
    The rest of the document consistently uses vX.Y.Z-rc.W as a placeholder to be substituted per-release (see lines 108, 111, 154). The new block instead hardcodes a real-looking version:

    wget -q -O ethrex-linux-x86_64 \
      https://github.com/lambdaclass/ethrex/releases/download/v20.0.0-rc.1/ethrex-linux-x86_64
    ...
    ./ethrex-linux-x86_64 --version # ethrex/v20.0.0-

    A future release engineer following this doc verbatim could end up downloading/running an old RC binary instead of substituting the version actually under test. Should be vX.Y.Z-rc.W like the surrounding sections, with the version comment made generic (e.g. # ethrex/vX.Y.Z-).

  2. Missing ssh step, inconsistent with sibling sections (docs/developers/release-process.md:127-146)
    The other host sections explicitly show the SSH login as the first command (e.g. line 108 ssh admin@ethrex-ethdocker-mainnet, line 151 ssh admin@ethrex-multisync-main). The new ethrex-prysm/lodestar/teku/grandine block jumps straight to tmux ls with no ssh admin@ethrex-<host> line, so a reader copy-pasting the block would run these commands on their own machine rather than the target host. Worth adding the missing ssh line for consistency and to avoid operator error on a mainnet-adjacent procedure.

Minor/optional

  • The hardcoded path /home/admin/secrets/jwt.hex (line 145) reveals an internal directory/username convention in a public repo. Low severity, and likely intentional since it mirrors real infra, but flagging in case that's not meant to be public.
  • No binary integrity check (checksum/signature) is mentioned before chmod +x / running the downloaded release binary on production mainnet hosts. Not a blocker, but worth considering for a process that runs untrusted-until-verified binaries on production infra.

Looks fine

  • The --no-monitor flag added in docs/developers/l2/upgrade-test.md:420 is a real, existing CLI flag (cmd/ethrex/l2/options.rs), and its addition to the sequencer restart command is a reasonable, low-risk doc correction.
  • The private keys used in upgrade-test.md are well-known local devnet test keys (not real secrets), consistent with the rest of that local-only upgrade-test procedure.

Automated review by Claude (Anthropic) · sonnet · custom prompt

Comment on lines +135 to +138
https://github.com/lambdaclass/ethrex/releases/download/v20.0.0-rc.1/ethrex-linux-x86_64
chmod +x ethrex-linux-x86_64

./ethrex-linux-x86_64 --version # ethrex/v20.0.0-

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Hardcoded version in example — should use the vX.Y.Z-rc.W placeholder

The wget URL and the --version comment both use the literal v20.0.0-rc.1 / v20.0.0- rather than the vX.Y.Z-rc.W placeholder that the rest of the document establishes at line 103. A release engineer following this section during a different release would have to remember to swap the URL manually, which is error-prone and inconsistent with the doc's own convention.

Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/developers/release-process.md
Line: 135-138

Comment:
**Hardcoded version in example — should use the `vX.Y.Z-rc.W` placeholder**

The `wget` URL and the `--version` comment both use the literal `v20.0.0-rc.1` / `v20.0.0-` rather than the `vX.Y.Z-rc.W` placeholder that the rest of the document establishes at line 103. A release engineer following this section during a different release would have to remember to swap the URL manually, which is error-prone and inconsistent with the doc's own convention.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Codex Code Review

  1. docs/developers/release-process.md: The new host-upgrade recipe hardcodes v20.0.0-rc.1 in both the download URL and the expected --version output (:134-138), but the checklist explicitly says these commands should be reused by replacing vX.Y.Z-rc.W with the version under test (:103). As written, the next release operator can follow the doc verbatim and restart a mainnet node with the old RC binary. This should be parameterized the same way as the rest of the section.

No Rust/EVM/security findings beyond that; the --no-monitor addition in docs/developers/l2/upgrade-test.md matches the current L2 CLI and avoids the monitor TUI during scripted upgrade testing.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@ElFantasma ElFantasma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just one nit inline.

exit

wget -q -O ethrex-linux-x86_64 \
https://github.com/lambdaclass/ethrex/releases/download/v20.0.0-rc.1/ethrex-linux-x86_64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: v20.0.0-rc.1 is baked into the wget URL and the version-check comment on line 138 shows # ethrex/v20.0.0- (looks truncated). Would swapping in a <VERSION> placeholder — or a # replace with the target release tag note above the block — make this section stay copy-pasteable across future upgrades without editing the concrete tag each time? Not blocking.

@github-project-automation github-project-automation Bot moved this to In Review in ethrex_l1 Jul 3, 2026
@edg-l edg-l added this pull request to the merge queue Jul 6, 2026
Merged via the queue into main with commit ae5fe94 Jul 6, 2026
58 checks passed
@edg-l edg-l deleted the improve-docs branch July 6, 2026 06:33
@github-project-automation github-project-automation Bot moved this from In Review to Done in ethrex_l1 Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants