Repository: celestiaorg/celestia-app

Issue:
Stop polluting `/tmp` with multiple copies of the v3.x binary
## Context

https://github.com/celestiaorg/celestia-app/issues/4849

## Problem

Currently the multiplexer uncompresses the v3.x binary into a temp directory on every `celestia-appd` invocation which pollutes the machine with multiple copies of the v3.x binary.

## Proposal

Stop uncompressing the v3.x binary to a new temp directory. Instead uncompress to a deterministic location based on the v3.x version (e.g. `~/.celestia-app/binaries/v3.10.0-arabica`). If a binary already exists at that location then use it and do not repeat the uncompressing step.

Discussion:

Solution patch:
diff --git a/Makefile b/Makefile
index 459ea1e4..68020ba6 100644
--- a/Makefile
+++ b/Makefile
@@ -22,6 +22,7 @@ ldflags = -X github.com/cosmos/cosmos-sdk/version.Name=celestia-app \
 BUILD_FLAGS := -tags "ledger" -ldflags '$(ldflags)'
 BUILD_FLAGS_MULTIPLEXER := -tags "ledger multiplexer" -ldflags '$(ldflags)'
 
+# NOTE: This version must be updated at the same time as the version in internal/embedding/data.go and .goreleaser.yaml
 CELESTIA_V3_VERSION := v3.10.0-arabica
 
 ## help: Get more info on make commands.
diff --git a/cmd/celestia-appd/cmd/modify_root_command_multiplexer.go b/cmd/celestia-appd/cmd/modify_root_command_multiplexer.go
index a6147c07..38910405 100644
--- a/cmd/celestia-appd/cmd/modify_root_command_multiplexer.go
+++ b/cmd/celestia-appd/cmd/modify_root_command_multiplexer.go
@@ -23,12 +23,12 @@ var v2UpgradeHeight = ""
 
 // modifyRootCommand enhances the root command with the pass through and multiplexer.
 func modifyRootCommand(rootCommand *cobra.Command) {
-	v3AppBinary, err := embedding.CelestiaAppV3()
+	version, compressedBinary, err := embedding.CelestiaAppV3()
 	if err != nil {
 		panic(err)
 	}
 
-	v3, err := appd.New("v3", v3AppBinary)
+	appdV3, err := appd.New(version, compressedBinary)
 	if err != nil {
 		panic(err)
 	}
@@ -39,7 +39,7 @@ func modifyRootCommand(rootCommand *cobra.Command) {
 	}
 
 	versions, err := abci.NewVersions(abci.Version{
-		Appd:        v3,
+		Appd:        appdV3,
 		ABCIVersion: abci.ABCIClientVersion1,
 		AppVersion:  3,
 		StartArgs: append([]string{
diff --git a/internal/embedding/data.go b/internal/embedding/data.go
index 2ae84abc..833b29ca 100644
--- a/internal/embedding/data.go
+++ b/internal/embedding/data.go
@@ -7,14 +7,19 @@ import (
 	"runtime"
 )
 
-// CelestiaAppV3 returns the compressed platform specific Celestia binary.
-func CelestiaAppV3() ([]byte, error) {
+// NOTE: This version must be updated at the same time as the version in the
+// Makefile.
+const v3Version = "v3.10.0-arabica"
+
+// CelestiaAppV3 returns the compressed platform specific Celestia binary and
+// the version.
+func CelestiaAppV3() (version string, compressedBinary []byte, err error) {
 	// Check if we actually have binary data
 	if len(v3binaryCompressed) == 0 {
-		return nil, fmt.Errorf("no binary data available for platform %s", platform())
+		return "", nil, fmt.Errorf("no binary data available for platform %s", platform())
 	}
 
-	return v3binaryCompressed, nil
+	return v3Version, v3binaryCompressed, nil
 }
 
 // platform returns a string representing the current operating system and architecture
diff --git a/multiplexer/appd/init.go b/multiplexer/appd/init.go
new file mode 100644
index 00000000..eab7da5b
--- /dev/null
+++ b/multiplexer/appd/init.go
@@ -0,0 +1,32 @@
+package appd
+
+import (
+	"fmt"
+
+	clienthelpers "cosmossdk.io/client/v2/helpers"
+)
+
+// envPrefix is the environment variable prefix for celestia-appd.
+// Environment variables that Cobra reads must be prefixed with this value.
+const envPrefix = "CELESTIA_APP"
+
+// appDirectory is the name of the application directory. This directory is used
+// to store configs, data, keyrings, and binaries.
+const appDirectory = ".celestia-app"
+
+// nodeHome is the home directory for the application daemon. By default this is
+// HOME/.celestia-app but it can be overridden by the user via --home flag.
+//
+// This gets set as a side-effect of the init() function.
+var nodeHome string
+
+func init() {
+	clienthelpers.EnvPrefix = envPrefix
+
+	var err error
+	nodeHome, err = clienthelpers.GetNodeHomeDirectory(appDirectory)
+	if err != nil {
+		// The userHome is not set in Vercel's Go runtime so log a warning but don't panic.
+		fmt.Printf("Warning userHome err: %s\n", err)
+	}
+}
diff --git a/multiplexer/appd/option.go b/multiplexer/appd/option.go
deleted file mode 100644
index 02285913..00000000
--- a/multiplexer/appd/option.go
+++ /dev/null
@@ -1,29 +0,0 @@
-package appd
-
-import "io"
-
-type CfgOption func(*Appd) *Appd
-
-// WithStdOut configures the standard output of the app.
-func WithStdOut(stdout io.Writer) CfgOption {
-	return func(a *Appd) *Appd {
-		a.stdout = stdout
-		return a
-	}
-}
-
-// WithStdErr configures the standard error of the app.
-func WithStdErr(stderr io.Writer) CfgOption {
-	return func(a *Appd) *Appd {
-		a.stderr = stderr
-		return a
-	}
-}
-
-// WithStdIn configures the standard input of the app.
-func WithStdIn(stdin io.Reader) CfgOption {
-	return func(a *Appd) *Appd {
-		a.stdin = stdin
-		return a
-	}
-}
diff --git a/multiplexer/appd/run.go b/multiplexer/appd/run.go
index f4667021..148da850 100644
--- a/multiplexer/appd/run.go
+++ b/multiplexer/appd/run.go
@@ -12,123 +12,52 @@ import (
 	"path/filepath"
 )
 
-const AppdStopped = -1
+const (
+	// AppdStopped is the process ID of the celestia-appd binary when it is not running.
+	AppdStopped = -1
+)
 
-// Appd represents the appd binary
+// Appd represents a celestia-appd binary.
 type Appd struct {
-	pid            int
-	path           string
-	stdin          io.Reader
-	stderr, stdout io.Writer
-	cleanup        func()
+	// version is the version of the celestia-appd binary.
+	// Example: "v3.10.0-arabica"
+	version string
+	// pid is the process ID of the celestia-appd binary.
+	pid int
+	// path is the path to the celestia-appd binary.
+	path   string
+	stdin  io.Reader
+	stderr io.Writer
+	stdout io.Writer
 }
 
-// New takes a binary and untar it in a temporary directory.
-func New(name string, bin []byte, cfg ...CfgOption) (*Appd, error) {
-	if len(bin) == 0 {
-		return nil, fmt.Errorf("no binary data available: ensure `bin` is not empty")
+// New returns a new Appd instance.
+func New(version string, compressedBinary []byte) (*Appd, error) {
+	if len(compressedBinary) == 0 {
+		return nil, fmt.Errorf("no compressed binary available for version %s", version)
 	}
 
-	// untar the binary.
-	gzr, err := gzip.NewReader(bytes.NewReader(bin))
-	if err != nil {
-		return nil, fmt.Errorf("failed to read binary data for %s: %w", name, err)
+	if err := ensureBinaryDecompressed(version, compressedBinary); err != nil {
+		return nil, fmt.Errorf("failed to decompress binary: %w", err)
 	}
-	defer gzr.Close()
 
-	// create a temporary directory for extraction
-	tmpDir, err := os.MkdirTemp("", fmt.Sprintf("appd-%s-", name))
+	pathToBinary, err := getPathToBinary(version)
 	if err != nil {
-		return nil, fmt.Errorf("failed to create temp directory: %w", err)
-	}
-
-	// cleanup function to remove the extracted binary.
-	cleanup := func() {
-		_ = os.RemoveAll(tmpDir)
+		return nil, fmt.Errorf("failed to get path to binary: %w", err)
 	}
 
-	// extract all files from the tar archive to the temp directory
-	tr := tar.NewReader(gzr)
-	for {
-		header, err := tr.Next()
-		if err == io.EOF {
-			break // End of archive
-		}
-		if err != nil {
-			return nil, fmt.Errorf("failed to read tar header: %w", err)
-		}
-
-		if header.FileInfo().IsDir() {
-			// Create directory
-			dirPath := filepath.Join(tmpDir, header.Name)
-			if err := os.MkdirAll(dirPath, 0o755); err != nil {
-				return nil, fmt.Errorf("failed to create directory %s: %w", dirPath, err)
-			}
-			continue
-		}
-
-		// Create file path
-		filePath := filepath.Join(tmpDir, header.Name)
-
-		// Create parent directory if it doesn't exist
-		if err := os.MkdirAll(filepath.Dir(filePath), 0o755); err != nil {
-			return nil, fmt.Errorf("failed to create parent directory for %s: %w", filePath, err)
-		}
-
-		// Create file
-		f, err := os.OpenFile(filePath, os.O_CREATE|os.O_RDWR, header.FileInfo().Mode())
-		if err != nil {
-			return nil, fmt.Errorf("failed to create file %s: %w", filePath, err)
-		}
-
-		if _, err := io.Copy(f, tr); err != nil {
-			f.Close()
-			return nil, fmt.Errorf("failed to copy file contents to %s: %w", filePath, err)
-		}
-		f.Close()
-	}
-
-	// look for the executable binary in the extracted files
-	var binaryPath string
-	err = filepath.Walk(tmpDir, func(path string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-
-		if !info.IsDir() && info.Mode()&0o111 != 0 {
-			binaryPath = path
-			return filepath.SkipAll // Found it, stop searching
-		}
-		return nil
-	})
-
-	if err != nil {
-		return nil, fmt.Errorf("failed to find executable binary in the archive: %w", err)
-	} else if binaryPath == "" {
-		return nil, fmt.Errorf("no executable binary found in the archive for %s", name)
+	if err = verifyBinaryIsExecutable(pathToBinary); err != nil {
+		return nil, fmt.Errorf("failed to verify binary is executable: %w", err)
 	}
 
 	appd := &Appd{
-		path:    binaryPath,
-		cleanup: cleanup,
-		pid:     AppdStopped, // initialize with stopped state
+		version: version,
+		pid:     AppdStopped,
+		path:    pathToBinary,
 		stdin:   os.Stdin,
 		stdout:  os.Stdout,
 		stderr:  os.Stderr,
 	}
-
-	for _, opt := range cfg {
-		opt(appd)
-	}
-
-	// verify the binary is executable for the current arch
-	testCmd := exec.Command(binaryPath, "--help")
-	testOutput, err := testCmd.CombinedOutput()
-	if err != nil {
-		return nil, fmt.Errorf("binary validation failed (%s): %w\nOutput: %s",
-			binaryPath, err, string(testOutput))
-	}
-
 	return appd, nil
 }
 
@@ -158,37 +87,28 @@ func (a *Appd) Start(args ...string) error {
 	return nil
 }
 
-// CreateExecCommand creates an exec.Cmd for the appd binary.
-func (a *Appd) CreateExecCommand(args ...string) *exec.Cmd {
-	cmd := exec.Command(a.path, args...)
-	cmd.Stdin = a.stdin
-	cmd.Stdout = a.stdout
-	cmd.Stderr = a.stderr
-	return cmd
-}
-
 // Stop terminates the running appd process if it exists.
 func (a *Appd) Stop() error {
 	if a.pid == AppdStopped {
 		return nil
 	}
 
-	proc, err := os.FindProcess(a.pid)
+	process, err := os.FindProcess(a.pid)
 	if err != nil {
 		return fmt.Errorf("failed to find process with PID %d: %w", a.pid, err)
 	}
 
 	// send SIGTERM for graceful shutdown
-	if err := proc.Signal(os.Interrupt); err != nil {
+	if err := process.Signal(os.Interrupt); err != nil {
 		log.Printf("Failed to send interrupt signal, attempting to kill: %v", err)
 		// if interrupt fails, try harder with Kill
-		if err := proc.Kill(); err != nil {
+		if err := process.Kill(); err != nil {
 			return fmt.Errorf("failed to kill process with PID %d: %w", a.pid, err)
 		}
 	}
 
 	// Wait for the process to exit
-	_, err = proc.Wait()
+	_, err = process.Wait()
 	if err != nil {
 		log.Printf("Error waiting for process to exit: %v", err)
 	}
@@ -197,7 +117,134 @@ func (a *Appd) Stop() error {
 	return nil
 }
 
-// Pid returns the pid of the appd process.
+// Pid returns the process ID of the appd process.
 func (a *Appd) Pid() int {
 	return a.pid
 }
+
+// CreateExecCommand creates an exec.Cmd for the appd binary.
+func (a *Appd) CreateExecCommand(args ...string) *exec.Cmd {
+	cmd := exec.Command(a.path, args...)
+	cmd.Stdin = a.stdin
+	cmd.Stdout = a.stdout
+	cmd.Stderr = a.stderr
+	return cmd
+}
+
+// getPathToBinary returns the path to the celestia-appd binary for the given version.
+func getPathToBinary(version string) (string, error) {
+	var pathToBinary string
+	baseDirectory := getDirectoryForVersion(version)
+
+	// look for the executable binary in the extracted files
+	err := filepath.Walk(baseDirectory, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if !info.IsDir() && info.Mode()&0o111 != 0 {
+			pathToBinary = path
+			return filepath.SkipAll // Found it, stop searching
+		}
+		return nil
+	})
+	if err != nil {
+		return "", fmt.Errorf("failed to find executable binary in the archive: %w", err)
+	}
+	if pathToBinary == "" {
+		return "", fmt.Errorf("no executable binary found in the archive for %s", version)
+	}
+	return pathToBinary, nil
+}
+
+// ensureBinaryDecompressed decompresses the binary for the given version if it
+// is not already decompressed.
+func ensureBinaryDecompressed(version string, binary []byte) error {
+	if isBinaryDecompressed(version) {
+		return nil
+	}
+
+	// untar the binary.
+	gzipReader, err := gzip.NewReader(bytes.NewReader(binary))
+	if err != nil {
+		return fmt.Errorf("failed to read binary data for %s: %w", version, err)
+	}
+	defer gzipReader.Close()
+
+	targetDirectory := getDirectoryForVersion(version)
+	if err := os.MkdirAll(targetDirectory, 0o755); err != nil {
+		return fmt.Errorf("failed to create directory: %w", err)
+	}
+
+	// extract all files from the tar archive to the directory
+	tarReader := tar.NewReader(gzipReader)
+	for {
+		header, err := tarReader.Next()
+		if err == io.EOF {
+			break // End of archive
+		}
+		if err != nil {
+			return fmt.Errorf("failed to read tar header: %w", err)
+		}
+
+		if header.FileInfo().IsDir() {
+			// Create directory
+			dirPath := filepath.Join(targetDirectory, header.Name)
+			if err := os.MkdirAll(dirPath, 0o755); err != nil {
+				return fmt.Errorf("failed to create directory %s: %w", dirPath, err)
+			}
+			continue
+		}
+
+		// Create file path
+		filePath := filepath.Join(targetDirectory, header.Name)
+
+		// Create parent directory if it doesn't exist
+		if err := os.MkdirAll(filepath.Dir(filePath), 0o755); err != nil {
+			return fmt.Errorf("failed to create parent directory for %s: %w", filePath, err)
+		}
+
+		// Create file
+		f, err := os.OpenFile(filePath, os.O_CREATE|os.O_RDWR, header.FileInfo().Mode())
+		if err != nil {
+			return fmt.Errorf("failed to create file %s: %w", filePath, err)
+		}
+
+		if _, err := io.Copy(f, tarReader); err != nil {
+			f.Close()
+			return fmt.Errorf("failed to copy file contents to %s: %w", filePath, err)
+		}
+		f.Close()
+	}
+
+	return nil
+}
+
+// isBinaryDecompressed returns true if the binary for the given version
+// has already been decompressed.
+func isBinaryDecompressed(version string) bool {
+	dir := getDirectoryForVersion(version)
+	_, err := os.Stat(dir)
+	return err == nil
+}
+
+func verifyBinaryIsExecutable(pathToBinary string) error {
+	testCmd := exec.Command(pathToBinary, "--help")
+	testOutput, err := testCmd.CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("binary validation failed (%s): %w\nOutput: %s", pathToBinary, err, string(testOutput))
+	}
+	return nil
+}
+
+// getDirectoryForCelestiaAppBinaries returns the directory where all
+// decompressed celestia-app binaries are stored. One directory exists per
+// version.
+func getDirectoryForCelestiaAppBinaries() string {
+	return filepath.Join(nodeHome, "bin")
+}
+
+// getDirectoryForVersion returns the directory for a particular version.
+func getDirectoryForVersion(version string) string {
+	return filepath.Join(getDirectoryForCelestiaAppBinaries(), version)
+}



Test patch:
diff --git a/internal/embedding/data_test.go b/internal/embedding/data_test.go
index 9372768d..3f514b56 100644
--- a/internal/embedding/data_test.go
+++ b/internal/embedding/data_test.go
@@ -6,27 +6,28 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/assert"
 )
 
-func TestCelestiaApp(t *testing.T) {
+func TestCelestiaAppV3(t *testing.T) {
 	// prevent messing with other tests by modifying this.
 	realData := v3binaryCompressed
 	defer func() {
 		v3binaryCompressed = realData
 	}()
 
-	tests := []struct {
-		name          string
-		modifyFn      func()
-		expectedError error
+	testCases := []struct {
+		name            string
+		modifyFn        func()
+		expectedVersion string
+		expectedError   error
 	}{
 		{
 			name: "valid binary data",
 			modifyFn: func() {
 				v3binaryCompressed = realData
 			},
-			expectedError: nil,
+			expectedVersion: v3Version,
 		},
 		{
 			name: "nil binaryCompressed",
@@ -37,20 +38,20 @@ func TestCelestiaApp(t *testing.T) {
 		},
 	}
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			tt.modifyFn()
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			tc.modifyFn()
+			version, binary, err := CelestiaAppV3()
 
-			data, err := CelestiaAppV3()
-
-			if tt.expectedError != nil {
-				require.Error(t, err)
-				require.Equal(t, err, tt.expectedError)
-				require.Nil(t, data)
+			if tc.expectedError != nil {
+				assert.Error(t, err)
+				assert.Equal(t, tc.expectedError, err)
+				assert.Empty(t, version)
+				assert.Nil(t, binary)
 			} else {
-				require.NoError(t, err)
-				require.NotNil(t, data)
-				require.NotEmpty(t, data)
+				assert.NoError(t, err)
+				assert.Equal(t, tc.expectedVersion, version)
+				assert.NotEmpty(t, binary)
 			}
 		})
 	}
diff --git a/multiplexer/appd/option_test.go b/multiplexer/appd/option_test.go
deleted file mode 100644
index 20ff2d4e..00000000
--- a/multiplexer/appd/option_test.go
+++ /dev/null
@@ -1,53 +0,0 @@
-package appd
-
-import (
-	"bytes"
-	"testing"
-
-	"github.com/stretchr/testify/require"
-)
-
-func TestCfgOptions(t *testing.T) {
-	tests := []struct {
-		name     string
-		option   CfgOption
-		expected func(*Appd) bool
-	}{
-		{
-			name:   "WithStdOut sets stdout correctly",
-			option: WithStdOut(&bytes.Buffer{}),
-			expected: func(a *Appd) bool {
-				_, ok := a.stdout.(*bytes.Buffer)
-				return ok
-			},
-		},
-		{
-			name:   "WithStdErr sets stderr correctly",
-			option: WithStdErr(&bytes.Buffer{}),
-			expected: func(a *Appd) bool {
-				_, ok := a.stderr.(*bytes.Buffer)
-				return ok
-			},
-		},
-		{
-			name:   "WithStdIn sets stdin correctly",
-			option: WithStdIn(&bytes.Buffer{}),
-			expected: func(a *Appd) bool {
-				_, ok := a.stdin.(*bytes.Buffer)
-				return ok
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			appdInstance := &Appd{}
-
-			// Apply the config option
-			appdInstance = tt.option(appdInstance)
-
-			require.NotNil(t, appdInstance, "Appd instance should not be nil")
-			require.True(t, tt.expected(appdInstance), "Expected configuration was not applied")
-		})
-	}
-}
diff --git a/multiplexer/appd/run_test.go b/multiplexer/appd/run_test.go
index 9c812f9b..2b69fd88 100644
--- a/multiplexer/appd/run_test.go
+++ b/multiplexer/appd/run_test.go
@@ -4,11 +4,13 @@ package appd
 
 import (
 	"bytes"
-	"github.com/celestiaorg/celestia-app/v4/internal/embedding"
 	"os"
 	"strings"
 	"testing"
 
+	"github.com/celestiaorg/celestia-app/v4/internal/embedding"
+
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -18,10 +20,10 @@ func TestCreateExecCommand(t *testing.T) {
 		t.Skip("skipping test which expects an embedded binary")
 	}
 
-	bin, err := embedding.CelestiaAppV3()
+	version, compressedBinary, err := embedding.CelestiaAppV3()
 	require.NoError(t, err)
 
-	appdInstance, err := New("test-app", bin)
+	appdInstance, err := New(version, compressedBinary)
 	require.NoError(t, err)
 	require.NotNil(t, appdInstance)
 
@@ -33,13 +35,10 @@ func TestCreateExecCommand(t *testing.T) {
 
 	require.NoError(t, cmd.Run())
 
-	output := outputBuffer.String()
-	require.NotEmpty(t, output)
-	require.True(t, strings.HasPrefix(output, "3"), "major version should be 3 of embedded binary. Got: %s", output)
-
-	appdInstance.cleanup()
-	require.Error(t, cmd.Run(), "expected error when running command after cleanup")
-
+	want := strings.TrimPrefix(version, "v")
+	got := outputBuffer.String()
+	require.NotEmpty(t, got)
+	assert.Contains(t, got, want)
 }
 
 // TestStart_Success ensures that the provided executable is launched and provided a pid.
@@ -53,6 +52,7 @@ func TestStart_Success(t *testing.T) {
 		stdin:  os.Stdin,
 		stdout: os.Stdout,
 		stderr: os.Stderr,
+		pid:    AppdStopped,
 	}
 
 	// Start the process