Fix config checker logic (#138)

* remove -v from make test

* refactor: config checker
one rule was broken due to new semantics of confDepth flag, which can't be negative anymore

* fix: config tests

* fix: lint

* feat: add new eigenda-cert-verification-enabled flag

* fix: config tests

* docs: update readme with new cert-verification-enabled flag info

Author: samlaf

Makefile

@@ -49,7 +49,7 @@ clean:
 	rm bin/eigenda-proxy
 
 test:
-	go test -v ./... -parallel 4 
+	go test ./... -parallel 4 
 
 e2e-test: stop-minio stop-redis run-minio run-redis
 	$(E2ETEST) && \

README.md

@@ -30,7 +30,9 @@ In order to disperse to the EigenDA network in production, or at high throughput
 | `--eigenda-custom-quorum-ids` |  | `$EIGENDA_PROXY_CUSTOM_QUORUM_IDS` | Custom quorum IDs for writing blobs. Should not include default quorums 0 or 1. |
 | `--eigenda-disable-point-verification-mode` | `false` | `$EIGENDA_PROXY_DISABLE_POINT_VERIFICATION_MODE` | Disable point verification mode. This mode performs IFFT on data before writing and FFT on data after reading. Disabling requires supplying the entire blob for verification against the KZG commitment. |
 | `--eigenda-disable-tls` | `false` | `$EIGENDA_PROXY_GRPC_DISABLE_TLS` | Disable TLS for gRPC communication with the EigenDA disperser. Default is false. |
+| --eigenda-cert-verification-enabled | `false` | `$EIGENDA_PROXY_CERT_VERIFICATION_ENABLED` | Whether to verify certificates received from EigenDA disperser. |
 | `--eigenda-disperser-rpc` |  | `$EIGENDA_PROXY_EIGENDA_DISPERSER_RPC` | RPC endpoint of the EigenDA disperser. |
+| `--eigenda-svc-manager-addr` |  | `$EIGENDA_PROXY_SERVICE_MANAGER_ADDR` | The deployed EigenDA service manager address. The list can be found here: https://github.com/Layr-Labs/eigenlayer-middleware/?tab=readme-ov-file#current-mainnet-deployment |
 | `--eigenda-eth-confirmation-depth` | `-1` | `$EIGENDA_PROXY_ETH_CONFIRMATION_DEPTH` | The number of Ethereum blocks of confirmation that the DA bridging transaction must have before it is assumed by the proxy to be final. If set negative the proxy will always wait for blob finalization. |
 | `--eigenda-eth-rpc` |  | `$EIGENDA_PROXY_ETH_RPC` | JSON RPC node endpoint for the Ethereum network used for finalizing DA blobs. See available list here: https://docs.eigenlayer.xyz/eigenda/networks/ |
 | `--eigenda-g1-path` | `"resources/g1.point"` | `$EIGENDA_PROXY_TARGET_KZG_G1_PATH` | Directory path to g1.point file. |
@@ -41,7 +43,6 @@ In order to disperse to the EigenDA network in production, or at high throughput
 | `--eigenda-signer-private-key-hex` |  | `$EIGENDA_PROXY_SIGNER_PRIVATE_KEY_HEX` | Hex-encoded signer private key. This key should not be associated with an Ethereum address holding any funds. |
 | `--eigenda-status-query-retry-interval` | `5s` | `$EIGENDA_PROXY_STATUS_QUERY_INTERVAL` | Interval between retries when awaiting network blob finalization. Default is 5 seconds. |
 | `--eigenda-status-query-timeout` | `30m0s` | `$EIGENDA_PROXY_STATUS_QUERY_TIMEOUT` | Duration to wait for a blob to finalize after being sent for dispersal. Default is 30 minutes. |
-| `--eigenda-svc-manager-addr` |  | `$EIGENDA_PROXY_SERVICE_MANAGER_ADDR` | The deployed EigenDA service manager address. The list can be found here: https://github.com/Layr-Labs/eigenlayer-middleware/?tab=readme-ov-file#current-mainnet-deployment |
 | `--log.color` | `false` | `$EIGENDA_PROXY_LOG_COLOR` | Color the log output if in terminal mode. |
 | `--log.format` | `text` | `$EIGENDA_PROXY_LOG_FORMAT` | Format the log output. Supported formats: 'text', 'terminal', 'logfmt', 'json', 'json-pretty'. |
 | `--log.level` | `INFO` | `$EIGENDA_PROXY_LOG_LEVEL` | The lowest log level that will be output. |

server/config.go

@@ -17,9 +17,6 @@ import (
 const (
 	// eigenda client flags
 	EigenDADisperserRPCFlagName          = "eigenda-disperser-rpc"
-	EthRPCFlagName                       = "eigenda-eth-rpc"
-	SvcManagerAddrFlagName               = "eigenda-svc-manager-addr"
-	EthConfirmationDepthFlagName         = "eigenda-eth-confirmation-depth"
 	StatusQueryRetryIntervalFlagName     = "eigenda-status-query-retry-interval"
 	StatusQueryTimeoutFlagName           = "eigenda-status-query-timeout"
 	DisableTLSFlagName                   = "eigenda-disable-tls"
@@ -29,6 +26,12 @@ const (
 	PutBlobEncodingVersionFlagName       = "eigenda-put-blob-encoding-version"
 	DisablePointVerificationModeFlagName = "eigenda-disable-point-verification-mode"
 
+	// cert verification flags
+	CertVerificationEnabledFlagName = "eigenda-cert-verification-enabled"
+	EthRPCFlagName                  = "eigenda-eth-rpc"
+	SvcManagerAddrFlagName          = "eigenda-svc-manager-addr"
+	EthConfirmationDepthFlagName    = "eigenda-eth-confirmation-depth"
+
 	// kzg flags
 	G1PathFlagName        = "eigenda-g1-path"
 	G2TauFlagName         = "eigenda-g2-tau-path"
@@ -79,10 +82,13 @@ type Config struct {
 	// the blob encoding version to use when writing blobs from the high level interface.
 	PutBlobEncodingVersion codecs.BlobEncodingVersion
 
-	// eth vars
-	EthRPC               string
-	SvcManagerAddr       string
-	EthConfirmationDepth int64
+	// eth verification vars
+	// TODO: right now verification and confirmation depth are tightly coupled
+	//       we should decouple them
+	CertVerificationEnabled bool
+	EthRPC                  string
+	SvcManagerAddr          string
+	EthConfirmationDepth    int64
 
 	// kzg vars
 	CacheDir         string
@@ -143,18 +149,11 @@ func (cfg *Config) VerificationCfg() *verify.Config {
 		NumWorker:       uint64(runtime.GOMAXPROCS(0)), // #nosec G115
 	}
 
-	if cfg.EthRPC == "" || cfg.SvcManagerAddr == "" {
-		return &verify.Config{
-			Verify:    false,
-			KzgConfig: kzgCfg,
-		}
-	}
-
 	return &verify.Config{
-		Verify:               true,
+		KzgConfig:            kzgCfg,
+		VerifyCerts:          cfg.CertVerificationEnabled,
 		RPCURL:               cfg.EthRPC,
 		SvcManagerAddr:       cfg.SvcManagerAddr,
-		KzgConfig:            kzgCfg,
 		EthConfirmationDepth: uint64(cfg.EthConfirmationDepth), // #nosec G115
 	}
 }
@@ -189,19 +188,20 @@ func ReadConfig(ctx *cli.Context) Config {
 			PutBlobEncodingVersion:       codecs.BlobEncodingVersion(ctx.Uint(PutBlobEncodingVersionFlagName)),
 			DisablePointVerificationMode: ctx.Bool(DisablePointVerificationModeFlagName),
 		},
-		G1Path:                 ctx.String(G1PathFlagName),
-		G2PowerOfTauPath:       ctx.String(G2TauFlagName),
-		CacheDir:               ctx.String(CachePathFlagName),
-		MaxBlobLength:          ctx.String(MaxBlobLengthFlagName),
-		SvcManagerAddr:         ctx.String(SvcManagerAddrFlagName),
-		EthRPC:                 ctx.String(EthRPCFlagName),
-		EthConfirmationDepth:   ctx.Int64(EthConfirmationDepthFlagName),
-		MemstoreEnabled:        ctx.Bool(MemstoreFlagName),
-		MemstoreBlobExpiration: ctx.Duration(MemstoreExpirationFlagName),
-		MemstoreGetLatency:     ctx.Duration(MemstoreGetLatencyFlagName),
-		MemstorePutLatency:     ctx.Duration(MemstorePutLatencyFlagName),
-		FallbackTargets:        ctx.StringSlice(FallbackTargets),
-		CacheTargets:           ctx.StringSlice(CacheTargets),
+		G1Path:                  ctx.String(G1PathFlagName),
+		G2PowerOfTauPath:        ctx.String(G2TauFlagName),
+		CacheDir:                ctx.String(CachePathFlagName),
+		CertVerificationEnabled: ctx.Bool(CertVerificationEnabledFlagName),
+		MaxBlobLength:           ctx.String(MaxBlobLengthFlagName),
+		SvcManagerAddr:          ctx.String(SvcManagerAddrFlagName),
+		EthRPC:                  ctx.String(EthRPCFlagName),
+		EthConfirmationDepth:    ctx.Int64(EthConfirmationDepthFlagName),
+		MemstoreEnabled:         ctx.Bool(MemstoreFlagName),
+		MemstoreBlobExpiration:  ctx.Duration(MemstoreExpirationFlagName),
+		MemstoreGetLatency:      ctx.Duration(MemstoreGetLatencyFlagName),
+		MemstorePutLatency:      ctx.Duration(MemstorePutLatencyFlagName),
+		FallbackTargets:         ctx.StringSlice(FallbackTargets),
+		CacheTargets:            ctx.StringSlice(CacheTargets),
 	}
 	// the eigenda client can only wait for 0 confirmations or finality
 	// the da-proxy has a more fine-grained notion of confirmation depth
@@ -246,16 +246,22 @@ func (cfg *Config) Check() error {
 		return fmt.Errorf("max blob length is 0")
 	}
 
-	if cfg.SvcManagerAddr != "" && cfg.EthRPC == "" {
-		return fmt.Errorf("svc manager address is set, but Eth RPC is not set")
-	}
-
-	if cfg.EthRPC != "" && cfg.SvcManagerAddr == "" {
-		return fmt.Errorf("eth rpc is set, but svc manager address is not set")
+	if !cfg.MemstoreEnabled {
+		if cfg.ClientConfig.RPC == "" {
+			return fmt.Errorf("using eigenda backend (memstore.enabled=false) but eigenda disperser rpc url is not set")
+		}
 	}
 
-	if cfg.EthConfirmationDepth >= 0 && (cfg.SvcManagerAddr == "" || cfg.EthRPC == "") {
-		return fmt.Errorf("eth confirmation depth is set for certificate verification, but Eth RPC or SvcManagerAddr is not set")
+	if cfg.CertVerificationEnabled {
+		if cfg.MemstoreEnabled {
+			return fmt.Errorf("cannot enable cert verification when memstore is enabled")
+		}
+		if cfg.EthRPC == "" {
+			return fmt.Errorf("cert verification enabled but eth rpc is not set")
+		}
+		if cfg.SvcManagerAddr == "" {
+			return fmt.Errorf("cert verification enabled but svc manager address is not set")
+		}
 	}
 
 	if cfg.S3Config.S3CredentialType == store.S3CredentialUnknown && cfg.S3Config.Endpoint != "" {
@@ -271,10 +277,6 @@ func (cfg *Config) Check() error {
 		return fmt.Errorf("redis password is set, but endpoint is not")
 	}
 
-	if !cfg.MemstoreEnabled && cfg.ClientConfig.RPC == "" {
-		return fmt.Errorf("eigenda disperser rpc url is not set")
-	}
-
 	err = cfg.checkTargets(cfg.FallbackTargets)
 	if err != nil {
 		return err
@@ -453,19 +455,31 @@ func CLIFlags() []cli.Flag {
 			EnvVars: prefixEnvVars("TARGET_CACHE_PATH"),
 			Value:   "resources/SRSTables/",
 		},
+		&cli.BoolFlag{
+			Name:    CertVerificationEnabledFlagName,
+			Usage:   "Whether to verify certificates received from EigenDA disperser.",
+			EnvVars: prefixEnvVars("CERT_VERIFICATION_ENABLED"),
+			// TODO: ideally we'd want this to be turned on by default when eigenda backend is used (memstore.enabled=false)
+			Value: false,
+		},
 		&cli.StringFlag{
-			Name:    EthRPCFlagName,
-			Usage:   "JSON RPC node endpoint for the Ethereum network used for finalizing DA blobs. See available list here: https://docs.eigenlayer.xyz/eigenda/networks/",
+			Name: EthRPCFlagName,
+			Usage: "JSON RPC node endpoint for the Ethereum network used for finalizing DA blobs.\n" +
+				"See available list here: https://docs.eigenlayer.xyz/eigenda/networks/\n" +
+				fmt.Sprintf("Mandatory when %s is true.", CertVerificationEnabledFlagName),
 			EnvVars: prefixEnvVars("ETH_RPC"),
 		},
 		&cli.StringFlag{
-			Name:    SvcManagerAddrFlagName,
-			Usage:   "The deployed EigenDA service manager address. The list can be found here: https://github.com/Layr-Labs/eigenlayer-middleware/?tab=readme-ov-file#current-mainnet-deployment",
+			Name: SvcManagerAddrFlagName,
+			Usage: "The deployed EigenDA service manager address.\n" +
+				"The list can be found here: https://github.com/Layr-Labs/eigenlayer-middleware/?tab=readme-ov-file#current-mainnet-deployment\n" +
+				fmt.Sprintf("Mandatory when %s is true.", CertVerificationEnabledFlagName),
 			EnvVars: prefixEnvVars("SERVICE_MANAGER_ADDR"),
 		},
 		&cli.Int64Flag{
-			Name:    EthConfirmationDepthFlagName,
-			Usage:   "The number of Ethereum blocks to wait before considering a submitted blob's DA batch submission confirmed. `0` means wait for inclusion only. `-1` means wait for finality.",
+			Name: EthConfirmationDepthFlagName,
+			Usage: "The number of Ethereum blocks to wait before considering a submitted blob's DA batch submission confirmed.\n" +
+				"`0` means wait for inclusion only. `-1` means wait for finality.",
 			EnvVars: prefixEnvVars("ETH_CONFIRMATION_DEPTH"),
 			Value:   -1,
 		},

server/config_test.go

@@ -35,15 +35,16 @@ func validCfg() *Config {
 			PutBlobEncodingVersion:       0,
 			DisablePointVerificationMode: false,
 		},
-		G1Path:                 "path/to/g1",
-		G2PowerOfTauPath:       "path/to/g2",
-		CacheDir:               "path/to/cache",
-		MaxBlobLength:          "2MiB",
-		SvcManagerAddr:         "0x1234567890abcdef",
-		EthRPC:                 "http://localhost:8545",
-		EthConfirmationDepth:   12,
-		MemstoreEnabled:        true,
-		MemstoreBlobExpiration: 25 * time.Minute,
+		G1Path:                  "path/to/g1",
+		G2PowerOfTauPath:        "path/to/g2",
+		CacheDir:                "path/to/cache",
+		MaxBlobLength:           "2MiB",
+		CertVerificationEnabled: false,
+		SvcManagerAddr:          "0x1234567890abcdef",
+		EthRPC:                  "http://localhost:8545",
+		EthConfirmationDepth:    12,
+		MemstoreEnabled:         true,
+		MemstoreBlobExpiration:  25 * time.Minute,
 	}
 }
 
@@ -71,37 +72,39 @@ func TestConfigVerification(t *testing.T) {
 		require.Error(t, err)
 	})
 
-	t.Run("MissingSvcManagerAddr", func(t *testing.T) {
-		cfg := validCfg()
-
-		cfg.EthRPC = "http://localhost:6969"
-		cfg.EthConfirmationDepth = 12
-		cfg.SvcManagerAddr = ""
-
-		err := cfg.Check()
-		require.Error(t, err)
-	})
-
-	t.Run("MissingCertVerificationParams", func(t *testing.T) {
-		cfg := validCfg()
-
-		cfg.EthConfirmationDepth = 12
-		cfg.SvcManagerAddr = ""
-		cfg.EthRPC = "http://localhost:6969"
-
-		err := cfg.Check()
-		require.Error(t, err)
-	})
-
-	t.Run("MissingEthRPC", func(t *testing.T) {
-		cfg := validCfg()
-
-		cfg.EthConfirmationDepth = 12
-		cfg.SvcManagerAddr = "0x00000000123"
-		cfg.EthRPC = ""
-
-		err := cfg.Check()
-		require.Error(t, err)
+	t.Run("CertVerificationEnabled", func(t *testing.T) {
+		// when eigenDABackend is enabled (memstore.enabled = false),
+		// some extra fields are required.
+		t.Run("MissingSvcManagerAddr", func(t *testing.T) {
+			cfg := validCfg()
+			// cert verification only makes sense when memstore is disabled (we use eigenda as backend)
+			cfg.MemstoreEnabled = false
+			cfg.CertVerificationEnabled = true
+			cfg.SvcManagerAddr = ""
+
+			err := cfg.Check()
+			require.Error(t, err)
+		})
+
+		t.Run("MissingEthRPC", func(t *testing.T) {
+			cfg := validCfg()
+			// cert verification only makes sense when memstore is disabled (we use eigenda as backend)
+			cfg.MemstoreEnabled = false
+			cfg.CertVerificationEnabled = true
+			cfg.EthRPC = ""
+
+			err := cfg.Check()
+			require.Error(t, err)
+		})
+
+		t.Run("CantDoCertVerificationWhenMemstoreEnabled", func(t *testing.T) {
+			cfg := validCfg()
+			cfg.MemstoreEnabled = true
+			cfg.CertVerificationEnabled = true
+
+			err := cfg.Check()
+			require.Error(t, err)
+		})
 	})
 
 	t.Run("MissingS3AccessKeys", func(t *testing.T) {

server/load_store.go

@@ -49,7 +49,7 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (store.
 		log.Info("Using S3 backend")
 		s3, err = store.NewS3(cfg.S3Config)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("failed to create S3 store: %w", err)
 		}
 	}
 
@@ -58,7 +58,7 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (store.
 		// create Redis backend store
 		redis, err = store.NewRedisStore(&cfg.RedisCfg)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("failed to create Redis store: %w", err)
 		}
 	}
 
@@ -68,10 +68,10 @@ func LoadStoreRouter(ctx context.Context, cfg CLIConfig, log log.Logger) (store.
 
 	verifier, err := verify.NewVerifier(vCfg, log)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to create verifier: %w", err)
 	}
 
-	if vCfg.Verify {
+	if vCfg.VerifyCerts {
 		log.Info("Certificate verification with Ethereum enabled")
 	} else {
 		log.Warn("Verification disabled")

store/memory_test.go

@@ -27,7 +27,7 @@ func getDefaultMemStoreTestConfig() MemStoreConfig {
 
 func getDefaultVerifierTestConfig() *verify.Config {
 	return &verify.Config{
-		Verify: false,
+		VerifyCerts: false,
 		KzgConfig: &kzg.KzgConfig{
 			G1Path:          "../resources/g1.point",
 			G2PowerOf2Path:  "../resources/g2.point.powerOf2",

verify/verifier.go

@@ -17,45 +17,48 @@ import (
 )
 
 type Config struct {
-	Verify               bool
+	KzgConfig   *kzg.KzgConfig
+	VerifyCerts bool
+	// below 3 fields are only required if VerifyCerts is true
 	RPCURL               string
 	SvcManagerAddr       string
-	KzgConfig            *kzg.KzgConfig
 	EthConfirmationDepth uint64
 }
 
 type Verifier struct {
-	verifyCert  bool
+	// kzgVerifier is needed to commit blobs to the memstore
 	kzgVerifier *kzgverifier.Verifier
+	// cert verification is optional, and verifies certs retrieved from eigenDA when turned on
+	verifyCerts bool
 	cv          *CertVerifier
 }
 
 func NewVerifier(cfg *Config, l log.Logger) (*Verifier, error) {
 	var cv *CertVerifier
 	var err error
 
-	if cfg.Verify {
+	if cfg.VerifyCerts {
 		cv, err = NewCertVerifier(cfg, l)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("failed to create cert verifier: %w", err)
 		}
 	}
 
 	kzgVerifier, err := kzgverifier.NewVerifier(cfg.KzgConfig, false)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to create kzg verifier: %w", err)
 	}
 
 	return &Verifier{
-		verifyCert:  cfg.Verify,
 		kzgVerifier: kzgVerifier,
+		verifyCerts: cfg.VerifyCerts,
 		cv:          cv,
 	}, nil
 }
 
 // verifies V0 eigenda certificate type
 func (v *Verifier) VerifyCert(cert *Certificate) error {
-	if !v.verifyCert {
+	if !v.verifyCerts {
 		return nil
 	}