Skip to content

fix: remove datadir before devnet run#379

Merged
MegaRedHand merged 1 commit into
mainfrom
run-devnet-remove-datadir
May 18, 2026
Merged

fix: remove datadir before devnet run#379
MegaRedHand merged 1 commit into
mainfrom
run-devnet-remove-datadir

Conversation

@MegaRedHand
Copy link
Copy Markdown
Collaborator

🗒️ Description / Motivation

Running make run-devnet more than once results in errors due to another database already existing. This can be solved by deleting the whole datadir after a devnet run.

What Changed

A step deleting lean-quickstart/local-devnet/data/ was added to the run-devnet make target.

@github-actions
Copy link
Copy Markdown

🤖 Kimi Code Review

Makefile: Lines 41-43

  1. Portability: Use $(RM) instead of literal rm for better cross-platform compatibility:

    @$(RM) -rf lean-quickstart/local-devnet/data/
  2. Safety concern: The -rf flags combined with a relative path could be dangerous if lean-quickstart is a symlink pointing outside the project directory (symlink attack). Consider verifying it's a regular directory first:

    @if [ -d "lean-quickstart/local-devnet/data" ]; then $(RM) -rf lean-quickstart/local-devnet/data/; fi
  3. User transparency: The @ suppression hides the deletion from the user. For destructive operations, consider removing @ or adding an explicit echo:

    @echo "Cleaning stale devnet data..."
  4. Race condition check: Ensure this runs after the lean-quickstart dependency completes but before the devnet actually starts. The current ordering looks correct.

Otherwise, the workaround for the broken --cleanData flag is reasonable for devnet tooling. The change is low-risk but consider adding the directory existence check to prevent errors if the clone fails or the path changes upstream.


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

@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review

PR #379 Review: fix: remove datadir before devnet run

Overview

Adds 3 lines to the run-devnet Makefile target to rm -rf the devnet data directory before each run, fixing the "database already exists" error on repeated invocations.


Analysis

Correctness

The fix is correct. Cleaning the data dir before starting a fresh devnet is the right approach when stale state causes failures. The placement — after lean-quickstart is cloned but before the devnet starts — is correct.

The --cleanData comment is valuable context. It explains why this workaround exists rather than using the upstream flag. Worth also tracking whether lean-quickstart ever fixes that flag, so this workaround can be removed.

Minor inconsistency in the PR description: the description says "deleting the data dir after a devnet run" but the code deletes it before. Before is the right choice — this is just a wording issue in the description.

Tradeoff: unconditional cleanup

The rm -rf is always run, which means there is no way to resume a devnet from a previous state using this target. That's probably fine for a dev/testing target, but if resumable devnets become a use case, a separate make clean-devnet target (or an opt-in flag) would be worth adding later. Not a blocker for this PR.

Style

Uses @ prefix to suppress echo, consistent with the rest of the target. The comments are clear and follow the inline @# pattern already used elsewhere.


Summary

This is a clean, minimal fix with a correct implementation and good explanatory comments. The only note is the description wording ("after" vs "before"), which is cosmetic. No blocking issues.


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

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 18, 2026

Greptile Summary

This PR fixes a repeated-run failure in make run-devnet by deleting lean-quickstart/local-devnet/data/ before starting the devnet, preventing errors caused by a pre-existing database. The change is minimal and correctly placed after the lean-quickstart dependency step.

Confidence Score: 5/5

The change is safe to merge — it adds a single rm -rf of a well-scoped subdirectory that is guaranteed to exist (or not) after the dependency step resolves.

The addition deletes only the transient database state directory before each devnet run. The -f flag prevents errors on first run when the directory doesn't exist yet, and the path is always valid because the lean-quickstart clone step is a resolved dependency before the recipe body executes. No config files or persistent assets are touched.

No files require special attention.

Important Files Changed

Filename Overview
Makefile Adds rm -rf lean-quickstart/local-devnet/data/ before the devnet run to clear stale database state; the -f flag handles the first-run case gracefully, and the path is always valid because the lean-quickstart dependency is resolved before the recipe executes.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Make as make run-devnet
    participant Docker as docker-build
    participant Git as lean-quickstart (clone)
    participant FS as Filesystem
    participant Devnet as spin-node.sh

    Dev->>Make: make run-devnet
    Make->>Docker: docker build (dependency)
    Docker-->>Make: image built
    Make->>Git: clone (if not present)
    Git-->>Make: repo ready
    Make->>FS: rm -rf lean-quickstart/local-devnet/data/ (NEW)
    FS-->>Make: stale data removed
    Make->>Devnet: ./spin-node.sh --node all --generateGenesis --metrics
    Devnet-->>Dev: devnet running (logs → devnet.log)
Loading

Reviews (1): Last reviewed commit: "fix: remove datadir before devnet run" | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown

🤖 Codex Code Review

  1. Medium - Makefile: rm -rf lean-quickstart/local-devnet/data/ is a destructive footgun because the trailing / makes rm descend into a symlink target rather than removing the link itself. If lean-quickstart/local-devnet/data is ever symlinked outside the repo, make run-devnet will recursively delete that external directory’s contents. This is easy to harden: refuse symlinks before deletion, or resolve the path and verify it stays under lean-quickstart/local-devnet before cleaning.

No other issues stood out in this PR. It only changes a devnet helper target, so there are no fork-choice, attestation, justification/finalization, state-transition, XMSS, or SSZ correctness concerns in the diff itself.


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

@MegaRedHand MegaRedHand merged commit 3e81771 into main May 18, 2026
7 checks passed
@MegaRedHand MegaRedHand deleted the run-devnet-remove-datadir branch May 18, 2026 16:12
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