Fix s3 panic from keccak commitment put requests (#129)

* refactor: e2e server_test to separate testsuite config creation from testsuite creation this will give flexibility to change the exact config being used to start the testsuite server * test: add e2e test for keccak commitment to expect error from s3 backend not set * fix: panic on nil ptr dereference when s3 backend not set * fix: lint * fix: e2e tests
Layr-Labs · Sep 19, 2024 · 36cfbd9 · 36cfbd9
1 parent 3325042
commit 36cfbd9
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 31 deletions.
diff --git a/e2e/optimism_test.go b/e2e/optimism_test.go
@@ -123,7 +123,9 @@ func TestOptimismKeccak256Commitment(gt *testing.T) {
 
 	testCfg := e2e.TestConfig(useMemory())
 	testCfg.UseKeccak256ModeS3 = true
-	proxyTS, shutDown := e2e.CreateTestSuite(gt, testCfg)
+
+	tsConfig := e2e.TestSuiteConfig(gt, testCfg)
+	proxyTS, shutDown := e2e.CreateTestSuite(gt, tsConfig)
 	defer shutDown()
 
 	t := actions.NewDefaultTesting(gt)
@@ -176,7 +178,8 @@ func TestOptimismAltDACommitment(gt *testing.T) {
 		gt.Skip("Skipping test as INTEGRATION or TESTNET env var not set")
 	}
 
-	proxyTS, shutDown := e2e.CreateTestSuite(gt, e2e.TestConfig(useMemory()))
+	tsConfig := e2e.TestSuiteConfig(gt, e2e.TestConfig(useMemory()))
+	proxyTS, shutDown := e2e.CreateTestSuite(gt, tsConfig)
 	defer shutDown()
 
 	t := actions.NewDefaultTesting(gt)

diff --git a/e2e/server_test.go b/e2e/server_test.go
@@ -26,7 +26,8 @@ func TestOptimismClientWithKeccak256Commitment(t *testing.T) {
 	testCfg := e2e.TestConfig(useMemory())
 	testCfg.UseKeccak256ModeS3 = true
 
-	ts, kill := e2e.CreateTestSuite(t, testCfg)
+	tsConfig := e2e.TestSuiteConfig(t, testCfg)
+	ts, kill := e2e.CreateTestSuite(t, tsConfig)
 	defer kill()
 
 	daClient := op_plasma.NewDAClient(ts.Address(), false, true)
@@ -41,6 +42,30 @@ func TestOptimismClientWithKeccak256Commitment(t *testing.T) {
 	require.Equal(t, testPreimage, preimage)
 }
 
+func TestKeccak256CommitmentRequestErrorsWhenS3NotSet(t *testing.T) {
+	if !runIntegrationTests && !runTestnetIntegrationTests {
+		t.Skip("Skipping test as INTEGRATION or TESTNET env var not set")
+	}
+
+	t.Parallel()
+
+	testCfg := e2e.TestConfig(useMemory())
+	testCfg.UseKeccak256ModeS3 = true
+
+	tsConfig := e2e.TestSuiteConfig(t, testCfg)
+	tsConfig.S3Config.Endpoint = ""
+	ts, kill := e2e.CreateTestSuite(t, tsConfig)
+	defer kill()
+
+	daClient := op_plasma.NewDAClient(ts.Address(), false, true)
+
+	testPreimage := []byte(e2e.RandString(100))
+
+	_, err := daClient.SetInput(ts.Ctx, testPreimage)
+	// TODO: the server currently returns an internal server error. Should it return a 400 instead?
+	require.Error(t, err)
+}
+
 /*
 this test asserts that the data can be posted/read to EigenDA
 with a concurrent S3 backend configured
@@ -53,7 +78,8 @@ func TestOptimismClientWithGenericCommitment(t *testing.T) {
 
 	t.Parallel()
 
-	ts, kill := e2e.CreateTestSuite(t, e2e.TestConfig(useMemory()))
+	tsConfig := e2e.TestSuiteConfig(t, e2e.TestConfig(useMemory()))
+	ts, kill := e2e.CreateTestSuite(t, tsConfig)
 	defer kill()
 
 	daClient := op_plasma.NewDAClient(ts.Address(), false, false)
@@ -77,7 +103,8 @@ func TestProxyClient(t *testing.T) {
 
 	t.Parallel()
 
-	ts, kill := e2e.CreateTestSuite(t, e2e.TestConfig(useMemory()))
+	tsConfig := e2e.TestSuiteConfig(t, e2e.TestConfig(useMemory()))
+	ts, kill := e2e.CreateTestSuite(t, tsConfig)
 	defer kill()
 
 	cfg := &client.Config{
@@ -104,7 +131,8 @@ func TestProxyServerWithLargeBlob(t *testing.T) {
 
 	t.Parallel()
 
-	ts, kill := e2e.CreateTestSuite(t, e2e.TestConfig(useMemory()))
+	tsConfig := e2e.TestSuiteConfig(t, e2e.TestConfig(useMemory()))
+	ts, kill := e2e.CreateTestSuite(t, tsConfig)
 	defer kill()
 
 	cfg := &client.Config{
@@ -131,7 +159,8 @@ func TestProxyServerWithOversizedBlob(t *testing.T) {
 
 	t.Parallel()
 
-	ts, kill := e2e.CreateTestSuite(t, e2e.TestConfig(useMemory()))
+	tsConfig := e2e.TestSuiteConfig(t, e2e.TestConfig(useMemory()))
+	ts, kill := e2e.CreateTestSuite(t, tsConfig)
 	defer kill()
 
 	cfg := &client.Config{
@@ -172,7 +201,8 @@ func TestProxyServerCaching(t *testing.T) {
 	testCfg := e2e.TestConfig(useMemory())
 	testCfg.UseS3Caching = true
 
-	ts, kill := e2e.CreateTestSuite(t, testCfg)
+	tsConfig := e2e.TestSuiteConfig(t, testCfg)
+	ts, kill := e2e.CreateTestSuite(t, tsConfig)
 	defer kill()
 
 	cfg := &client.Config{
@@ -214,7 +244,8 @@ func TestProxyServerCachingWithRedis(t *testing.T) {
 	testCfg := e2e.TestConfig(useMemory())
 	testCfg.UseRedisCaching = true
 
-	ts, kill := e2e.CreateTestSuite(t, testCfg)
+	tsConfig := e2e.TestSuiteConfig(t, testCfg)
+	ts, kill := e2e.CreateTestSuite(t, tsConfig)
 	defer kill()
 
 	cfg := &client.Config{
@@ -266,7 +297,8 @@ func TestProxyServerReadFallback(t *testing.T) {
 	testCfg.UseS3Fallback = true
 	testCfg.Expiration = time.Millisecond * 1
 
-	ts, kill := e2e.CreateTestSuite(t, testCfg)
+	tsConfig := e2e.TestSuiteConfig(t, testCfg)
+	ts, kill := e2e.CreateTestSuite(t, tsConfig)
 	defer kill()
 
 	cfg := &client.Config{

diff --git a/e2e/setup.go b/e2e/setup.go
@@ -32,8 +32,9 @@ const (
 )
 
 type Cfg struct {
-	UseMemory          bool
-	Expiration         time.Duration
+	UseMemory  bool
+	Expiration time.Duration
+	// at most one of the below options should be true
 	UseKeccak256ModeS3 bool
 	UseS3Caching       bool
 	UseRedisCaching    bool
@@ -84,15 +85,7 @@ func createS3Config(eigendaCfg server.Config) server.CLIConfig {
 	}
 }
 
-type TestSuite struct {
-	Ctx    context.Context
-	Log    log.Logger
-	Server *server.Server
-}
-
-func CreateTestSuite(t *testing.T, testCfg *Cfg) (TestSuite, func()) {
-	ctx := context.Background()
-
+func TestSuiteConfig(t *testing.T, testCfg *Cfg) server.CLIConfig {
 	// load signer key from environment
 	pk := os.Getenv(privateKey)
 	if pk == "" && !testCfg.UseMemory {
@@ -112,12 +105,6 @@ func CreateTestSuite(t *testing.T, testCfg *Cfg) (TestSuite, func()) {
 		pollInterval = time.Minute * 1
 	}
 
-	log := oplog.NewLogger(os.Stdout, oplog.CLIConfig{
-		Level:  log.LevelDebug,
-		Format: oplog.FormatLogFmt,
-		Color:  true,
-	}).New("role", svcName)
-
 	eigendaCfg := server.Config{
 		ClientConfig: clients.EigenDAClientConfig{
 			RPC:                      holeskyDA,
@@ -143,7 +130,6 @@ func CreateTestSuite(t *testing.T, testCfg *Cfg) (TestSuite, func()) {
 	}
 
 	var cfg server.CLIConfig
-
 	switch {
 	case testCfg.UseKeccak256ModeS3:
 		cfg = createS3Config(eigendaCfg)
@@ -167,9 +153,26 @@ func CreateTestSuite(t *testing.T, testCfg *Cfg) (TestSuite, func()) {
 		}
 	}
 
+	return cfg
+}
+
+type TestSuite struct {
+	Ctx    context.Context
+	Log    log.Logger
+	Server *server.Server
+}
+
+func CreateTestSuite(t *testing.T, testSuiteCfg server.CLIConfig) (TestSuite, func()) {
+	log := oplog.NewLogger(os.Stdout, oplog.CLIConfig{
+		Level:  log.LevelDebug,
+		Format: oplog.FormatLogFmt,
+		Color:  true,
+	}).New("role", svcName)
+
+	ctx := context.Background()
 	store, err := server.LoadStoreRouter(
 		ctx,
-		cfg,
+		testSuiteCfg,
 		log,
 	)
 	require.NoError(t, err)

diff --git a/server/load_store.go b/server/load_store.go
@@ -11,7 +11,7 @@ import (
 )
 
 // populateTargets ... creates a list of storage backends based on the provided target strings
-func populateTargets(targets []string, s3 *store.S3Store, redis *store.RedStore) []store.PrecomputedKeyStore {
+func populateTargets(targets []string, s3 store.PrecomputedKeyStore, redis *store.RedStore) []store.PrecomputedKeyStore {
 	stores := make([]store.PrecomputedKeyStore, len(targets))
 
 	for i, f := range targets {
@@ -42,7 +42,7 @@ func populateTargets(targets []string, s3 *store.S3Store, redis *store.RedStore)
 func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (store.IRouter, error) {
 	// create S3 backend store (if enabled)
 	var err error
-	var s3 *store.S3Store
+	var s3 store.PrecomputedKeyStore
 	var redis *store.RedStore
 
 	if cfg.S3Config.Bucket != "" && cfg.S3Config.Endpoint != "" {