Skip to content
Open
Changes from all commits
Commits
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
90 changes: 90 additions & 0 deletions chainstate/store/json_persister.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package store

import (
"context"
"fmt"
"os"
"time"

"github.com/Layr-Labs/eigensdk-go/logging"
)

// JSONPersister handles periodic persistence of store state to a JSON file.
type JSONPersister struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't a JSON persister just a snapshot persister?

store Store
path string
logger logging.Logger
}

// NewJSONPersister creates a new JSON persister for the given store.
func NewJSONPersister(store Store, path string, logger logging.Logger) *JSONPersister {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we call os.MkdirAll on the path here? Otherwise I think the WriteFile command will fail if the parent directories don't already exist

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively this is a problem on the infra side to make sure directories are setup properly. I'm okay with either way.

return &JSONPersister{
store: store,
path: path,
logger: logger,
}
}

// Save persists the current store state to the configured JSON file.
// It uses atomic file operations (write to temp, then rename) to ensure consistency.
func (p *JSONPersister) Save(ctx context.Context) error {
data, err := p.store.Snapshot()
if err != nil {
return fmt.Errorf("failed to create snapshot: %w", err)
}

tmpPath := p.path + ".tmp"
if err := os.WriteFile(tmpPath, data, 0644); err != nil {
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The file permissions 0644 allow any user on the system to read the chainstate data. If this data contains sensitive information about operators or the network state, consider using more restrictive permissions like 0600 (read/write only for the owner) to limit access to the file owner only.

Suggested change
if err := os.WriteFile(tmpPath, data, 0644); err != nil {
if err := os.WriteFile(tmpPath, data, 0600); err != nil {

Copilot uses AI. Check for mistakes.
return fmt.Errorf("failed to write temp file: %w", err)
}

if err := os.Rename(tmpPath, p.path); err != nil {
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

If the temporary file write succeeds but the rename operation fails, the temporary file will be left behind. Consider adding cleanup logic to remove the temporary file on error, or document that orphaned .tmp files may need to be cleaned up after failures.

Suggested change
if err := os.Rename(tmpPath, p.path); err != nil {
if err := os.Rename(tmpPath, p.path); err != nil {
// Best-effort cleanup of the temporary file if rename fails.
if removeErr := os.Remove(tmpPath); removeErr != nil && !os.IsNotExist(removeErr) {
p.logger.Error("Failed to remove temp state file after rename error", "path", tmpPath, "error", removeErr)
}

Copilot uses AI. Check for mistakes.
return fmt.Errorf("failed to rename temp file: %w", err)
}
Comment on lines +36 to +43
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The codebase has an established AtomicWrite utility function at litt/util/file_utils.go that provides better atomic file writing guarantees than the current implementation. It handles fsync of both the file and parent directory, and uses proper cleanup of temporary files. Consider using this utility instead of manually implementing atomic writes to maintain consistency across the codebase and improve durability guarantees.

Copilot uses AI. Check for mistakes.

p.logger.Info("State persisted", "path", p.path, "size_bytes", len(data))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can we make this debug log and emit an info log on final save?

return nil
Comment on lines +30 to +46
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The context parameter passed to Save is not used. If the intention is to respect context cancellation during the save operation, consider checking ctx.Done() before performing the save, or remove the parameter if it's not needed. The final save in StartPeriodicSave uses context.Background(), which bypasses cancellation that might be in progress.

Copilot uses AI. Check for mistakes.
}

// Load restores the store state from the configured JSON file.
// If the file doesn't exist, it returns without error (fresh start).
func (p *JSONPersister) Load(ctx context.Context) error {
data, err := os.ReadFile(p.path)
if err != nil {
if os.IsNotExist(err) {
p.logger.Info("No existing state file, starting fresh", "path", p.path)
return nil
}
return fmt.Errorf("failed to read state file: %w", err)
}

if err := p.store.Restore(data); err != nil {
return fmt.Errorf("failed to restore state: %w", err)
}

p.logger.Info("State restored", "path", p.path, "size_bytes", len(data))
return nil
}
Comment on lines +51 to +67
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The context parameter passed to Load is not used within the function. Consider either using it to respect cancellation (e.g., checking ctx.Done() before the restore operation) or removing it if context support isn't needed for this operation.

Copilot uses AI. Check for mistakes.

// StartPeriodicSave starts a background goroutine that periodically saves the store state.
// It also performs a final save when the context is cancelled.
func (p *JSONPersister) StartPeriodicSave(ctx context.Context, interval time.Duration) {
ticker := time.NewTicker(interval)
defer ticker.Stop()

for {
select {
case <-ticker.C:
if err := p.Save(ctx); err != nil {
p.logger.Error("Failed to persist state", "error", err)
}
case <-ctx.Done():
// Perform final save before shutdown
p.logger.Info("Context cancelled, performing final state save")
if err := p.Save(context.Background()); err != nil {
p.logger.Error("Failed final state save", "error", err)
}
return
}
}
Comment on lines +71 to +89
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

StartPeriodicSave returns immediately without providing a way for the caller to know when the background goroutine has finished or to wait for the final save to complete. Consider returning a channel or using a wait group to allow the caller to synchronize with the completion of the final save operation before shutdown completes.

Suggested change
func (p *JSONPersister) StartPeriodicSave(ctx context.Context, interval time.Duration) {
ticker := time.NewTicker(interval)
defer ticker.Stop()
for {
select {
case <-ticker.C:
if err := p.Save(ctx); err != nil {
p.logger.Error("Failed to persist state", "error", err)
}
case <-ctx.Done():
// Perform final save before shutdown
p.logger.Info("Context cancelled, performing final state save")
if err := p.Save(context.Background()); err != nil {
p.logger.Error("Failed final state save", "error", err)
}
return
}
}
// The returned channel is closed when the background goroutine has finished,
// allowing callers to wait for the final save to complete before shutdown.
func (p *JSONPersister) StartPeriodicSave(ctx context.Context, interval time.Duration) <-chan struct{} {
done := make(chan struct{})
go func() {
defer close(done)
ticker := time.NewTicker(interval)
defer ticker.Stop()
for {
select {
case <-ticker.C:
if err := p.Save(ctx); err != nil {
p.logger.Error("Failed to persist state", "error", err)
}
case <-ctx.Done():
// Perform final save before shutdown
p.logger.Info("Context cancelled, performing final state save")
if err := p.Save(context.Background()); err != nil {
p.logger.Error("Failed final state save", "error", err)
}
return
}
}
}()
return done

Copilot uses AI. Check for mistakes.
}
Comment on lines +1 to +90
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

There are no tests for the JSONPersister implementation. The repository has comprehensive test coverage across other modules. Consider adding tests to cover: successful Save/Load operations, handling of non-existent files in Load, atomic write behavior, StartPeriodicSave functionality with context cancellation, and error cases like write failures or corrupt JSON data during restoration.

Copilot uses AI. Check for mistakes.
Loading