Skip to content

fix: persist snapshot once for avoiding badger vlog#3153

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.

Sign up for GitHub

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

Merged
julienrbrt merged 6 commits into from
Mar 10, 2026
Merged

fix: persist snapshot once for avoiding badger vlog #3153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6729abd
fix: persist snapshot once for avoiding badger vlog
julienrbrt Mar 10, 2026
d1e578a
cl
julienrbrt Mar 10, 2026
3fbdfae
Merge branch 'main' into julien/cache
julienrbrt Mar 10, 2026
28ffadf
use latest
julienrbrt Mar 10, 2026
0eefee8
Revert "build(deps): Bump docker/login-action from 3 to 4 (#3149)"
julienrbrt Mar 10, 2026
662c81e
fix lint
julienrbrt Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/docker-build-push.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,7 +34,7 @@ jobs:
uses: docker/setup-buildx-action@v4

- name: Log in to GHCR
uses: docker/login-action@v4
uses: docker/login-action@v3
with:
registry: ghcr.io
username: ${{ github.actor }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release-apps.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -81,7 +81,7 @@
uses: docker/setup-buildx-action@v4

- name: Log in to GHCR
uses: docker/login-action@v4
uses: docker/login-action@v3

Check warning

Code scanning / CodeQL

Unpinned tag for a non-immutable Action in workflow or composite action Medium

Unpinned 3rd party Action 'Apps release' step
Uses Step
Loading
uses 'docker/login-action' with ref 'v3', not a pinned commit hash
with:
registry: ghcr.io
username: ${{ github.actor }}
Expand Down
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## v1.0.0

Identical to v1.0.0-rc.5.
### Fixed

- Persist cache snapshot only once at shutdown to avoid Badger vlog increase. [#3153](https://github.com/evstack/ev-node/pull/3153)

## v1.0.0-rc.5

Expand Down
8 changes: 4 additions & 4 deletions apps/evm/go.mod
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,10 +2,10 @@ module github.com/evstack/ev-node/apps/evm

go 1.25.6

//replace (
// github.com/evstack/ev-node => ../../
// github.com/evstack/ev-node/execution/evm => ../../execution/evm
//)
replace (
github.com/evstack/ev-node => ../../
github.com/evstack/ev-node/execution/evm => ../../execution/evm
)
Comment on lines +5 to +8

@coderabbitaicoderabbitaiBotMar 10, 2026

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
python - <<'PY'from pathlib import Pathmod = Path("apps/evm/go.mod")module_dir = mod.parent.resolve()for raw in mod.read_text().splitlines(): line = raw.strip() if "=>" not in line or line.startswith("//"): continue left, right = [part.strip() for part in line.split("=>", 1)] if left.startswith("replace "): target = right.split()[0] elif left.startswith("github.com/"): target = right.split()[0] else: continue if target.startswith("."): resolved = (module_dir / target).resolve() escapes = module_dir not in (resolved, *resolved.parents) print(f"{mod}: {target} -> {resolved}") print(f"escapes module dir: {escapes}")PY

Repository: evstack/ev-node

Length of output: 227

🏁 Script executed:

# Check if go.work exists in repo root
ls -la go.work 2>/dev/null ||echo"go.work not found"# Check the actual module path in apps/evm/go.mod
head -5 apps/evm/go.mod
# Check Go version requirement
head -3 go.mod

Repository: evstack/ev-node

Length of output: 186

🏁 Script executed:

# Search for any references to go.work or workspace setup
rg "go.work" --type md --type txt --type sh .# Check if there are any build/CI configurations that might show intended usage
fd -t f -e yml -e yaml -e Makefile | head -20
# Look for any documentation about building apps/evm standalone
rg "apps/evm" --type md . -A 3

Repository: evstack/ev-node

Length of output: 1538

These local replace directives break standalone releases of apps/evm.

The RELEASE.md documents apps/evm as a standalone module released via go get github.com/evstack/ev-node/apps/evm@v0.3.0. When users obtain this module, the ../../ and ../../execution/evm paths will not exist, making the module unusable.

Move these replacements to a go.work file in the repository root to manage local monorepo development without shipping broken module files. Go 1.25.6 fully supports workspaces.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/evm/go.mod` around lines 5 - 8, The apps/evm module contains local
replace directives ("replace github.com/evstack/ev-node => ../../" and "replace
github.com/evstack/ev-node/execution/evm => ../../execution/evm") that break
standalone releases; remove these replace lines from the apps/evm go.mod and
instead add equivalent replace entries into a repo-root go.work workspace file
(using Go workspaces supported in Go 1.25.6) so local monorepo development works
but published modules remain usable via go get.


require (
github.com/ethereum/go-ethereum v1.17.1
Expand Down
4 changes: 0 additions & 4 deletions apps/evm/go.sum
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -407,12 +407,8 @@ github.com/ethereum/go-bigmodexpfix v0.0.0-20250911101455-f9e208c548ab h1:rvv6MJ
github.com/ethereum/go-bigmodexpfix v0.0.0-20250911101455-f9e208c548ab/go.mod h1:IuLm4IsPipXKF7CW5Lzf68PIbZ5yl7FFd74l/E0o9A8=
github.com/ethereum/go-ethereum v1.17.1 h1:IjlQDjgxg2uL+GzPRkygGULPMLzcYWncEI7wbaizvho=
github.com/ethereum/go-ethereum v1.17.1/go.mod h1:7UWOVHL7K3b8RfVRea022btnzLCaanwHtBuH1jUCH/I=
github.com/evstack/ev-node v1.0.0-rc.5 h1:3qd3KWuwiLr5RKt1j8v2h5X3XZvwyYGbMUDskM+cAeI=
github.com/evstack/ev-node v1.0.0-rc.5/go.mod h1:VAaN0tbN1+SSm7sLzMhHZNUn9sgpf1eok5gu3YVbT38=
github.com/evstack/ev-node/core v1.0.0 h1:s0Tx0uWHme7SJn/ZNEtee4qNM8UO6PIxXnHhPbbKTz8=
github.com/evstack/ev-node/core v1.0.0/go.mod h1:n2w/LhYQTPsi48m6lMj16YiIqsaQw6gxwjyJvR+B3sY=
github.com/evstack/ev-node/execution/evm v1.0.0-rc.4 h1:9IKEVD8QRe6drXJFxnydM5ao6Gw5BPgXXIVCY3516WM=
github.com/evstack/ev-node/execution/evm v1.0.0-rc.4/go.mod h1:aDwQRKH79c49eBgGAkkTPJKSicRxGtxteSqmj0oqAQ4=
github.com/fatih/color v1.10.0/go.mod h1:ELkj/draVOlAH/xkhN6mQ50Qd0MPOk5AAr3maGEBuJM=
github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
github.com/fatih/color v1.15.0/go.mod h1:0h5ZqXfHYED7Bhv2ZJamyIOUej9KtShiJESRwBDUSsw=
Expand Down
2 changes: 1 addition & 1 deletion apps/testapp/go.mod
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,7 +2,7 @@ module github.com/evstack/ev-node/apps/testapp

go 1.25.6

//replace github.com/evstack/ev-node => ../../.
replace github.com/evstack/ev-node => ../../.

@coderabbitaicoderabbitaiBotMar 10, 2026

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bashset -euo pipefail
python - <<'PY'from pathlib import Pathmod = Path("apps/testapp/go.mod")module_dir = mod.parent.resolve()for raw in mod.read_text().splitlines(): line = raw.strip() if "=>" not in line or line.startswith("//"): continue left, right = [part.strip() for part in line.split("=>", 1)] if left.startswith("replace "): target = right.split()[0] elif left.startswith("github.com/"): target = right.split()[0] else: continue if target.startswith("."): resolved = (module_dir / target).resolve() escapes = module_dir not in (resolved, *resolved.parents) print(f"{mod}: {target} -> {resolved}") print(f"escapes module dir: {escapes}")PY

Repository: evstack/ev-node

Length of output: 134

Do not commit a relative replace in this nested module.

apps/testapp is its own module. A versioned checkout of github.com/evstack/ev-node/apps/testapp will not contain the target at ../../., breaking standalone builds of this module outside a full monorepo checkout. Use go.work for local wiring instead of shipping relative replace directives in published go.mod files.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/testapp/go.mod` at line 5, The go.mod in apps/testapp contains a
relative replace directive ("replace github.com/evstack/ev-node => ../../.")
which must be removed because apps/testapp is a standalone module; delete that
replace line from the module's go.mod and instead wire local module replacements
via a workspace file (go.work) at the monorepo root when developing locally so
published/checked-out versions of github.com/evstack/ev-node/apps/testapp do not
depend on a relative path.


require (
github.com/evstack/ev-node v1.0.0-rc.5
Expand Down
2 changes: 0 additions & 2 deletions apps/testapp/go.sum
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -367,8 +367,6 @@ github.com/envoyproxy/protoc-gen-validate v0.10.0/go.mod h1:DRjgyB0I43LtJapqN6Ni
github.com/envoyproxy/protoc-gen-validate v0.10.1/go.mod h1:DRjgyB0I43LtJapqN6NiRwroiAU2PaFuvk/vjgh61ss=
github.com/envoyproxy/protoc-gen-validate v1.0.1/go.mod h1:0vj8bNkYbSTNS2PIyH87KZaeN4x9zpL9Qt8fQC7d+vs=
github.com/envoyproxy/protoc-gen-validate v1.0.2/go.mod h1:GpiZQP3dDbg4JouG/NNS7QWXpgx6x8QiMKdmN72jogE=
github.com/evstack/ev-node v1.0.0-rc.5 h1:3qd3KWuwiLr5RKt1j8v2h5X3XZvwyYGbMUDskM+cAeI=
github.com/evstack/ev-node v1.0.0-rc.5/go.mod h1:VAaN0tbN1+SSm7sLzMhHZNUn9sgpf1eok5gu3YVbT38=
github.com/evstack/ev-node/core v1.0.0 h1:s0Tx0uWHme7SJn/ZNEtee4qNM8UO6PIxXnHhPbbKTz8=
github.com/evstack/ev-node/core v1.0.0/go.mod h1:n2w/LhYQTPsi48m6lMj16YiIqsaQw6gxwjyJvR+B3sY=
github.com/fatih/color v1.10.0/go.mod h1:ELkj/draVOlAH/xkhN6mQ50Qd0MPOk5AAr3maGEBuJM=
Expand Down
16 changes: 5 additions & 11 deletions block/internal/cache/generic_cache.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,7 +141,7 @@ func (c *Cache[T]) getDAIncludedByHeight(blockHeight uint64) (uint64, bool) {
return c.getDAIncluded(hash)
}

// setDAIncluded records DA inclusion and persists the snapshot.
// setDAIncluded records DA inclusion in memory.
// If a previous entry already exists at blockHeight (e.g. a placeholder from
// RestoreFromStore), it is evicted from daIncluded to avoid orphan leaks.
func (c *Cache[T]) setDAIncluded(hash string, daHeight uint64, blockHeight uint64) {
Expand All@@ -151,14 +151,11 @@ func (c *Cache[T]) setDAIncluded(hash string, daHeight uint64, blockHeight uint6
c.daIncluded.Add(hash, daHeight)
c.hashByHeight.Add(blockHeight, hash)
c.setMaxDAHeight(daHeight)
_ = c.persistSnapshot(context.Background())
}

// removeDAIncluded removes the DA-included status of the hash from the cache
// and rewrites the window snapshot.
// removeDAIncluded removes the DA-included status of the hash from the cache.
func (c *Cache[T]) removeDAIncluded(hash string) {
c.daIncluded.Remove(hash)
_ = c.persistSnapshot(context.Background())
}

// daHeight returns the maximum DA height from all DA-included items.
Expand All@@ -185,7 +182,7 @@ func (c *Cache[T]) removeSeen(hash string) {
}

// deleteAllForHeight removes all items and their associated data from the
// cache at the given height and rewrites the window snapshot.
// cache at the given height.
func (c *Cache[T]) deleteAllForHeight(height uint64) {
c.itemsByHeight.Remove(height)

Expand All@@ -200,13 +197,10 @@ func (c *Cache[T]) deleteAllForHeight(height uint64) {
c.hashes.Remove(hash)
c.daIncluded.Remove(hash)
}

_ = c.persistSnapshot(context.Background())
}

// persistSnapshot writes all current in-flight [blockHeight, daHeight] pairs
// to the store under a single key. Called on every mutation; payload is tiny
// (typically <10 entries × 16 bytes).
// persistSnapshot writes all current in-flight [blockHeight, daHeight] pairs to the store under a single key.
// Only called explicitly via SaveToStore. NEVER CALL IT ON HOT-PATH TO AVOID BAGER WRITE AMPLIFICATION.
Comment on lines +202 to +203

@coderabbitaicoderabbitaiBotMar 10, 2026

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.

⚠️ Potential issue | 🟠 Major

Don't leave the new shutdown flush as best-effort.

These lines make SaveToStore the only durable snapshot write, but the production shutdown path in block/components.go:120-135 still just logs that error and returns success. A failed flush now silently loses the only restart checkpoint. Please propagate the shutdown error (or retry it) instead of treating it as informational.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@block/internal/cache/generic_cache.go` around lines 202 - 203, SaveToStore
(which calls persistSnapshot) is now the only durable checkpoint but the
shutdown path that invokes SaveToStore treats failures as informational; change
that shutdown caller to return (or propagate) the error instead of just logging
it, or implement a bounded retry/backoff loop before giving up, so a failed
SaveToStore is not silently ignored. Locate the shutdown/stop routine that calls
SaveToStore and replace the log-only handling with proper error propagation
(return the error) or a retry mechanism, ensuring callers handle the returned
error up the stack.

func (c *Cache[T]) persistSnapshot(ctx context.Context) error {
if c.store == nil || c.storeKeyPrefix == "" {
return nil
Expand Down
18 changes: 11 additions & 7 deletions block/internal/cache/generic_cache_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -178,19 +178,20 @@ func TestCache_RestoreFromStore_PlaceholderOverwrittenByRealHash(t *testing.T) {
assert.Equal(t, uint64(99), daH)
}

// TestCache_RestoreFromStore_RoundTrip verifies that setDAIncluded persists a
// TestCache_RestoreFromStore_RoundTrip verifies that SaveToStore persists a
// snapshot that a freshly-constructed cache can fully recover.
func TestCache_RestoreFromStore_RoundTrip(t *testing.T) {
st := testMemStore(t)
ctx := context.Background()

// First cache instance: write some in-flight entries.
// First cache instance: write some in-flight entries, then flush (shutdown).
c1 := NewCache[testItem](st, "rt/")
c1.setDAIncluded("hashA", 10, 1)
c1.setDAIncluded("hashB", 20, 2)
c1.setDAIncluded("hashC", 30, 3)
// Remove one entry to confirm deletions are also snapshotted.
c1.removeDAIncluded("hashB")
require.NoError(t, c1.SaveToStore(ctx))

// Second cache instance on same store: should recover {hashA→10, hashC→30}.
c2 := NewCache[testItem](st, "rt/")
Expand DownExpand Up@@ -324,6 +325,7 @@ func TestCache_ClearFromStore(t *testing.T) {
c := NewCache[testItem](st, "clear-test/")
c.setDAIncluded("hash1", 100, 1)
c.setDAIncluded("hash2", 200, 2)
require.NoError(t, c.SaveToStore(ctx))

// Verify the snapshot key was written before clearing.
_, err := st.GetMetadata(ctx, "clear-test/__snap")
Expand DownExpand Up@@ -381,9 +383,10 @@ func TestCache_NoPlaceholderLeakAfterRefire(t *testing.T) {
st := testMemStore(t)
ctx := context.Background()

// Step 1: initial run — write a real hash for height 3.
// Step 1: initial run — write a real hash for height 3, then flush (shutdown).
c1 := NewCache[testItem](st, "pfx/")
c1.setDAIncluded("realHash3", 99, 3)
require.NoError(t, c1.SaveToStore(ctx))
// snapshot now contains [{blockHeight:3, daHeight:99}]

// Step 2: restart — placeholder installed for height 3.
Expand DownExpand Up@@ -424,9 +427,10 @@ func TestCache_RestartIdempotent(t *testing.T) {
const blockH = uint64(5)
const daH = uint64(42)

// ── Run 1: normal operation, height 5 in-flight ──────────────────────────
// ── Run 1: normal operation, height 5 in-flight; flush at shutdown ───────
c1 := NewCache[testItem](st, "pfx/")
c1.setDAIncluded(realHash, daH, blockH)
require.NoError(t, c1.SaveToStore(ctx))
// snapshot: [{5, 42}]

for restart := 1; restart <= 3; restart++ {
Expand All@@ -442,16 +446,16 @@ func TestCache_RestartIdempotent(t *testing.T) {
require.True(t, ok, "restart %d: height-based lookup must succeed", restart)
assert.Equal(t, daH, gotDAH, "restart %d: height-based DA height correct", restart)

// ── DA retriever re-fires with the real hash
// ── DA retriever re-fires with the real hash, then flushes (shutdown).
cR.setDAIncluded(realHash, daH, blockH)
require.NoError(t, cR.SaveToStore(ctx), "restart %d: SaveToStore", restart)

// After re-fire: real hash present, no orphan, snapshot updated.
_, realPresent := cR.daIncluded.Get(realHash)
assert.True(t, realPresent, "restart %d: real hash present after re-fire", restart)
assert.Equal(t, 1, cR.daIncluded.Len(), "restart %d: no orphan after re-fire", restart)

// The snapshot rewritten by re-fire must still encode the right data
// The snapshot written by SaveToStore must still encode the right data
// so the next restart can load it correctly.
// (persistSnapshot fires inside setDAIncluded, so the store is up to date.)
}
}
4 changes: 2 additions & 2 deletions block/internal/submitting/submitter_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -700,10 +700,10 @@ func TestSubmitter_setNodeHeightToDAHeight_AfterRestart(t *testing.T) {
require.NoError(t, batch.SetHeight(3))
require.NoError(t, batch.Commit())

// Mark height 3 as DA-included in the pre-restart cache.
// persistSnapshot fires here and writes the snapshot key to st1.
// Mark height 3 as DA-included in the pre-restart cache, then flush (shutdown).
cm1.SetHeaderDAIncluded(h3.Hash().String(), 12, 3)
cm1.SetDataDAIncluded(d3.DACommitment().String(), 12, 3)
require.NoError(t, cm1.SaveToStore())

// Persist DAIncludedHeight = 2 (height 3 is in-flight, not yet finalized).
daIncBz := make([]byte, 8)
Expand Down
2 changes: 2 additions & 0 deletions node/execution_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -96,10 +96,12 @@ func getExecutorFromNode(t *testing.T, node *FullNode) coreexecutor.Executor {
state := sle.state()
if state == nil {
t.Fatal("failoverState is nil")
return nil
}
bc := state.bc
if bc == nil {
t.Fatal("blockComponents is nil")
return nil
}
if bc.Executor != nil {
return bc.Executor.GetCoreExecutor()
Expand Down
2 changes: 2 additions & 0 deletions pkg/da/types/namespace_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -38,6 +38,7 @@ func TestNamespaceV0Creation(t *testing.T) {
}
if ns == nil {
t.Fatal("expected non-nil namespace but got nil")
return
}

if ns.Version != NamespaceVersionZero {
Expand DownExpand Up@@ -119,6 +120,7 @@ func TestNamespaceFromBytes(t *testing.T) {
}
if ns == nil {
t.Fatal("expected non-nil namespace but got nil")
return
}
if !bytes.Equal(tt.input, ns.Bytes()) {
t.Errorf("Should round-trip correctly, expected %v, got %v", tt.input, ns.Bytes())
Expand Down
Loading