Merge pull request #7216 from ethereum-optimism/aj/cannon-atomic-write

cannon: Use atomic write pattern to avoid leaving partially written files

Makefile

@@ -65,7 +65,7 @@ cannon:
 
 cannon-prestate: op-program cannon
 	./cannon/bin/cannon load-elf --path op-program/bin/op-program-client.elf --out op-program/bin/prestate.json --meta op-program/bin/meta.json
-	./cannon/bin/cannon run --proof-at '=0' --stop-at '=1' --input op-program/bin/prestate.json --meta op-program/bin/meta.json --proof-fmt 'op-program/bin/%d.json' --output /dev/null
+	./cannon/bin/cannon run --proof-at '=0' --stop-at '=1' --input op-program/bin/prestate.json --meta op-program/bin/meta.json --proof-fmt 'op-program/bin/%d.json' --output ""
 	mv op-program/bin/0.json op-program/bin/prestate-proof.json
 
 mod-tidy:

cannon/cmd/json.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path"
 
 	"github.com/ethereum-optimism/optimism/op-service/ioutil"
 )
@@ -27,19 +28,27 @@ func loadJSON[X any](inputPath string) (*X, error) {
 	return &state, nil
 }
 
-func writeJSON[X any](outputPath string, value X, outIfEmpty bool) error {
+func writeJSON[X any](outputPath string, value X) error {
+	if outputPath == "" {
+		return nil
+	}
 	var out io.Writer
-	if outputPath != "" {
-		f, err := ioutil.OpenCompressed(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0755)
+	finish := func() error { return nil }
+	if outputPath != "-" {
+		// Write to a tmp file but reserve the file extension if present
+		tmpPath := outputPath + "-tmp" + path.Ext(outputPath)
+		f, err := ioutil.OpenCompressed(tmpPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0755)
 		if err != nil {
 			return fmt.Errorf("failed to open output file: %w", err)
 		}
 		defer f.Close()
 		out = f
-	} else if outIfEmpty {
-		out = os.Stdout
+		finish = func() error {
+			// Rename the file into place as atomically as the OS will allow
+			return os.Rename(tmpPath, outputPath)
+		}
 	} else {
-		return nil
+		out = os.Stdout
 	}
 	enc := json.NewEncoder(out)
 	if err := enc.Encode(value); err != nil {
@@ -49,5 +58,8 @@ func writeJSON[X any](outputPath string, value X, outIfEmpty bool) error {
 	if err != nil {
 		return fmt.Errorf("failed to append new-line: %w", err)
 	}
+	if err := finish(); err != nil {
+		return fmt.Errorf("failed to finish write: %w", err)
+	}
 	return nil
 }

cannon/cmd/json_test.go

@@ -13,7 +13,7 @@ func TestRoundTripJSON(t *testing.T) {
 	dir := t.TempDir()
 	file := filepath.Join(dir, "test.json")
 	data := &jsonTestData{A: "yay", B: 3}
-	err := writeJSON(file, data, false)
+	err := writeJSON(file, data)
 	require.NoError(t, err)
 
 	// Confirm the file is uncompressed
@@ -32,7 +32,7 @@ func TestRoundTripJSONWithGzip(t *testing.T) {
 	dir := t.TempDir()
 	file := filepath.Join(dir, "test.json.gz")
 	data := &jsonTestData{A: "yay", B: 3}
-	err := writeJSON(file, data, false)
+	err := writeJSON(file, data)
 	require.NoError(t, err)
 
 	// Confirm the file isn't raw JSON

cannon/cmd/load_elf.go

@@ -24,7 +24,7 @@ var (
 	}
 	LoadELFOutFlag = &cli.PathFlag{
 		Name:     "out",
-		Usage:    "Output path to write JSON state to. State is dumped to stdout if set to empty string.",
+		Usage:    "Output path to write JSON state to. State is dumped to stdout if set to -. Not written if empty.",
 		Value:    "state.json",
 		Required: false,
 	}
@@ -66,10 +66,10 @@ func LoadELF(ctx *cli.Context) error {
 	if err != nil {
 		return fmt.Errorf("failed to compute program metadata: %w", err)
 	}
-	if err := writeJSON[*mipsevm.Metadata](ctx.Path(LoadELFMetaFlag.Name), meta, false); err != nil {
+	if err := writeJSON[*mipsevm.Metadata](ctx.Path(LoadELFMetaFlag.Name), meta); err != nil {
 		return fmt.Errorf("failed to output metadata: %w", err)
 	}
-	return writeJSON[*mipsevm.State](ctx.Path(LoadELFOutFlag.Name), state, true)
+	return writeJSON[*mipsevm.State](ctx.Path(LoadELFOutFlag.Name), state)
 }
 
 var LoadELFCommand = &cli.Command{

cannon/cmd/run.go

@@ -28,7 +28,7 @@ var (
 	}
 	RunOutputFlag = &cli.PathFlag{
 		Name:      "output",
-		Usage:     "path of output JSON state. Stdout if left empty.",
+		Usage:     "path of output JSON state. Not written if empty, use - to write to Stdout.",
 		TakesFile: true,
 		Value:     "out.json",
 		Required:  false,
@@ -42,7 +42,7 @@ var (
 	}
 	RunProofFmtFlag = &cli.StringFlag{
 		Name:     "proof-fmt",
-		Usage:    "format for proof data output file names. Proof data is written to stdout if empty.",
+		Usage:    "format for proof data output file names. Proof data is written to stdout if -.",
 		Value:    "proof-%d.json",
 		Required: false,
 	}
@@ -66,7 +66,7 @@ var (
 	}
 	RunMetaFlag = &cli.PathFlag{
 		Name:     "meta",
-		Usage:    "path to metadata file for symbol lookup for enhanced debugging info durign execution.",
+		Usage:    "path to metadata file for symbol lookup for enhanced debugging info during execution.",
 		Value:    "meta.json",
 		Required: false,
 	}
@@ -324,7 +324,7 @@ func Run(ctx *cli.Context) error {
 		}
 
 		if snapshotAt(state) {
-			if err := writeJSON(fmt.Sprintf(snapshotFmt, step), state, false); err != nil {
+			if err := writeJSON(fmt.Sprintf(snapshotFmt, step), state); err != nil {
 				return fmt.Errorf("failed to write state snapshot: %w", err)
 			}
 		}
@@ -360,7 +360,7 @@ func Run(ctx *cli.Context) error {
 				proof.OracleValue = witness.PreimageValue
 				proof.OracleOffset = witness.PreimageOffset
 			}
-			if err := writeJSON(fmt.Sprintf(proofFmt, step), proof, true); err != nil {
+			if err := writeJSON(fmt.Sprintf(proofFmt, step), proof); err != nil {
 				return fmt.Errorf("failed to write proof data: %w", err)
 			}
 		} else {
@@ -371,7 +371,7 @@ func Run(ctx *cli.Context) error {
 		}
 	}
 
-	if err := writeJSON(ctx.Path(RunOutputFlag.Name), state, true); err != nil {
+	if err := writeJSON(ctx.Path(RunOutputFlag.Name), state); err != nil {
 		return fmt.Errorf("failed to write state output: %w", err)
 	}
 	return nil