cmd: p2p store info

[tac0turtle]

Overview

add store-info command to inspect p2p store to see what is present

[github-actions]

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	Nov 13, 2025, 9:02 PM

[claude]

Claude finished @tac0turtle's task —— View job

---

Code Review for PR #2835: cmd: p2p store info

Note: This PR has been merged. This review is provided for documentation and future reference.

---

Summary

This PR adds a new store-info command (StoreP2PInspectCmd) that allows inspection of go-header P2P stores to display head/tail information for both header and data stores. The implementation is generally solid with good test coverage.

---

1. Code Quality & Architecture

✅ Strengths

• Clean separation of concerns: The command logic is well-separated into focused functions (inspectP2PStore, populateSnapshot, printP2PStoreSnapshot)
• Idiomatic Go: Code follows Go conventions with proper error handling and defer usage
• Type safety: Good use of Go generics with inspectP2PStore[H goheader.Header[H]]
• Proper resource cleanup: Datastore closing is handled with deferred cleanup

⚠️ Areas for Improvement

pkg/cmd/store.go:92-98 - Missing error check on Stop()

deferfunc() {
_=storeImpl.Stop(context.Background())
}()

The Stop() error is silently discarded. Consider logging it or combining with other cleanup errors:

deferfunc() {
ifstopErr:=storeImpl.Stop(context.Background()); stopErr!=nil {
// Consider logging or returning combined error
}
}()

pkg/cmd/store.go:171-182 - Fallback logic could be clearer
The resolveDBName function has complex fallback logic that could benefit from inline comments explaining when each case occurs:

funcresolveDBName(cmd*cobra.Command) string {
// If no command context, use default config filenameifcmd==nil {
returnconfig.ConfigFileName
}
root:=cmd.Root()
// Use root command name as DB name for proper namespacingifroot==nil||root.Name() =="" {
returnconfig.ConfigFileName
}
returnroot.Name()
}

---

2. Error Handling & Resilience

✅ Strengths

• Consistent error wrapping: Good use of fmt.Errorf with %w verb for error context
• Graceful handling of empty stores: Properly distinguishes between ErrEmptyStore, ErrNotFound, and real errors
• Context propagation: Proper use of context.Context for cancellation

⚠️ Areas for Improvement

pkg/cmd/store.go:18-20 - Context handling could be more defensive

ctx:=cmd.Context()
ifctx==nil {
ctx=context.Background()
}

Consider using context.WithTimeout for the inspection operations to prevent indefinite hangs:

ctx:=cmd.Context()
ifctx==nil {
ctx=context.Background()
}
ctx, cancel:=context.WithTimeout(ctx, 30*time.Second)
defercancel()

pkg/cmd/store.go:27-31 - Error handling on Close could be improved

deferfunc() {
ifcloseErr:=rawStore.Close(); closeErr!=nil {
cmd.PrintErrf("warning: failed to close datastore: %v\n", closeErr)
}
}()

While this prints a warning, the error doesn't affect the return value. Consider whether cleanup errors should fail the command or be logged more prominently.

pkg/cmd/store.go:122-128 - Error messages could be more actionable
When head/tail reads fail with unexpected errors, consider providing suggestions for resolution:

default:
returnfmt.Errorf("failed to read %s head (store may be corrupted, consider running integrity check): %w", snapshot.Label, err)

---

3. Security

✅ Strengths

• No credential exposure: The command only displays heights and hashes, not sensitive data
• Safe file operations: Uses filepath.Join for path construction
• Read-only operations: Command only reads from the datastore

⚠️ Considerations

Path traversal protection: While resolveStorePath uses filepath.Join, consider validating that the resolved path is within expected boundaries, especially if configs can be user-supplied.

Data exposure: The command outputs hash values and timestamps. While not sensitive, ensure this aligns with privacy requirements in production environments.

---

4. Performance & Resource Efficiency

✅ Strengths

• Proper resource cleanup: Datastores are properly closed with defers
• Efficient inspection: Only reads head/tail, not entire store
• No memory leaks: Store lifecycle properly managed

⚠️ Areas for Improvement

pkg/cmd/store.go:93 - Context management

_=storeImpl.Stop(context.Background())

Using context.Background() for cleanup may not respect cancellation. Consider:

stopCtx, stopCancel:=context.WithTimeout(context.Background(), 5*time.Second)
deferstopCancel()
_=storeImpl.Stop(stopCtx)

Potential optimization: If multiple stores need inspection, consider parallelizing the reads:

// Could use errgroup for concurrent inspectiong, ctx:=errgroup.WithContext(ctx)
g.Go(func() error { returninspectP2PStore[*types.SignedHeader](...) })
g.Go(func() error { returninspectP2PStore[*types.Data](...) })

Though for a CLI diagnostic command, sequential is probably fine.

---

5. Testing & Reliability

✅ Strengths

• Comprehensive test coverage: TestStoreP2PInspectCmd covers the happy path
• Test isolation: Uses t.TempDir() for clean test environments
• Realistic test data: seedHeaderStore creates actual header entries with proper signing
• Output verification: Tests check for expected strings in output

⚠️ Areas for Improvement

Missing edge case tests in pkg/cmd/store_test.go:

1. Empty store test: While the test checks for "status: empty" in data store, there's no dedicated test for a completely empty database
2. Error scenarios: No tests for:
   • Corrupted datastore
   • Missing/invalid config
   • Permission errors
   • Store that fails to start
3. Large height values: No test for stores with very large height numbers
4. Concurrent access: No test for behavior when store is locked/in-use

Suggested additional tests:

funcTestStoreP2PInspectCmd_EmptyStore(t*testing.T) { /* ... */ }
funcTestStoreP2PInspectCmd_InvalidPath(t*testing.T) { /* ... */ }
funcTestStoreP2PInspectCmd_CorruptedStore(t*testing.T) { /* ... */ }

Test cleanup: The test properly uses defer for cleanup, but consider adding explicit verification that cleanup succeeded:

require.NoError(t, headerStore.Stop(ctx), "Failed to stop header store")
require.NoError(t, rawStore.Close(), "Failed to close raw store")

---

6. Documentation & Developer Experience

✅ Strengths

• Good command descriptions: Both Short and Long descriptions are clear
• Helpful output formatting: Clear section headers and labels
• Informative error messages: Errors include context

⚠️ Areas for Improvement

Missing godoc comments for key functions:

// inspectP2PStore opens a go-header store with the given prefix and returns// a snapshot of its current state including head/tail information.// It handles store lifecycle (start/stop) internally.funcinspectP2PStore[H goheader.Header[H]](...) (p2pStoreSnapshot, error) { /* ... */ }

pkg/cmd/store.go:66-73 - Command documentation could include examples:

Long: `Opens the datastore used by the node's go-header services and reportsthe current height, head, and tail information for both the header and data stores.Examples: # Inspect stores for default config evnode store-info # Inspect stores with custom home directory  evnode store-info --home /custom/path`,

Output formatting: Consider adding a --format flag for JSON output to enable programmatic parsing:

fmt.Fprintln(out, "status: empty (no entries found)")
// Could also support:// {"label": "Header Store", "status": "empty", "height": 0}

pkg/cmd/store.go:164-170 - formatTime helper lacks documentation

// formatTime returns a formatted time string for display, or empty string if time is zero.funcformatTime(t time.Time) string { /* ... */ }

---

7. Additional Observations

Integration with existing code

apps/{testapp,evm,grpc}/single/main.go - Command registration
The command is properly registered in all three apps. Good consistency across the codebase.

Code coverage concern

Codecov reports 72% patch coverage with 28 lines missing. The missing lines are likely:

• Error path branches (corrupted store, missing config, etc.)
• Empty store branches in populateSnapshot
• Some defer cleanup paths

Recommendation: Add error scenario tests to improve coverage of error handling paths.

---

Summary & Recommendations

Critical Issues

None - this is production-ready code.

High Priority Improvements

1. Add timeout context for store operations to prevent hangs
2. Add tests for error scenarios (corrupted store, invalid config, etc.)
3. Consider logging or handling Stop() errors in cleanup

Medium Priority Improvements

1. Add godoc comments for exported and complex internal functions
2. Add usage examples to command documentation
3. Improve error messages with actionable suggestions
4. Consider adding JSON output format option

Low Priority Improvements

1. Add inline comments explaining fallback logic in resolveDBName
2. Consider path traversal validation in resolveStorePath
3. Add explicit cleanup verification in tests

Overall Assessment

This is well-written, production-quality code that follows Go best practices. The implementation is clean, maintainable, and properly tested. The main areas for improvement are:

• More comprehensive error scenario testing
• Better documentation for complex functions
• Defensive timeout handling for long-running operations

The code demonstrates good understanding of:

• Go generics and type constraints
• Resource lifecycle management
• Error handling patterns
• Test isolation and setup

Recommendation: Approved with minor suggestions for future improvements. The code is safe to merge and use in production.

---

[codecov]

Codecov Report

❌ Patch coverage is 72.00000% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.72%. Comparing base (1911431) to head (90f206e).

Files with missing lines	Patch %	Lines
pkg/cmd/store.go	72.00%	16 Missing and 12 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## marko/sync_service_fix #2835 +/- ##
========================================================
Coverage 64.72% 64.72% ========================================================
Files 81 81 Lines 7172 7272 +100 ========================================================
+ Hits 4642 4707 +65 - Misses 1990 2013 +23 - Partials 540 552 +12 

Flag	Coverage Δ
combined	64.72% <72.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.