fix(e2e tests): missing new eigenda-client required config fields (#196)

* fix(e2e tests): missing new eigenda-client required config fields - ethrpc and svcmanageraddr

* Revert "ci: give holesky-test workflow access to secrets via pull_request_target (#153)"

This reverts commit 15b10fd.
The commit was doing things very wrong. I hadn't understood how pull_request_target works.
Was causing the workflow to run against main branch head commit instead of PR commit.
We will need to find another solution to the problem of letting external contributors run this workflow.

* ide: update vscode settings.json with env vars to run holesky testnet e2e tests

* docs: shorten svcManagerAddr holesky testnet comment

* tests: add panic when both INTEGRATION and TESTNET env vars are set

This forces test runner to be fully aware of which test suite he is running (otherwise it implicitly runs the TESTNET suite)

* style: make handleGetShared returned error print hex encoded commitment

Previously it was printing as byte array, which is unreadable and clutters logs

* docs: better comments in metrics middleware

* deps: upgrade eigenda dep to regression fix commit

TODO: will need to update this after that PR is merged

* deps: update eigenda dependency to master head

Just merged Layr-Labs/eigenda#849 which will fix our ci bug

.github/workflows/holesky-test.yml

@@ -3,11 +3,7 @@ name: holesky-test
 on:
   push:
     branches: [ "main" ]
-  # pull_request_target is needed so that external contributors that create PRs from a forked repo
-  # have access to the secrets needed to run the tests. There are security implications to this,
-  # see https://stackoverflow.com/questions/74957218/what-is-the-difference-between-pull-request-and-pull-request-target-event-in-git
-  # MAKE SURE TO ONLY ALLOW RUNNING THIS WORKFLOW AFTER REVIEWING THE PR!
-  pull_request_target:
+  pull_request:
     branches: [ "main" ]
 
 jobs:

.vscode/settings.json

@@ -1,6 +1,11 @@
 {
   "go.testEnvVars": {
-    "INTEGRATION": "true"
+    // Only one of INTEGRATION or TESTNET should be true.
+    "INTEGRATION": "true",
+    "TESTNET": "false",
+    // To test against TESTNET, set SIGNER_PRIVATE_KEY and ETHEREUM_RPC.
+    "SIGNER_PRIVATE_KEY": "",
+    "ETHEREUM_RPC": "https://ethereum-holesky-rpc.publicnode.com"
   },
   "go.testFlags": [
     "-test.parallel",

e2e/main_test.go

@@ -30,6 +30,9 @@ var (
 func ParseEnv() {
 	runIntegrationTests = os.Getenv("INTEGRATION") == "true" || os.Getenv("INTEGRATION") == "1"
 	runTestnetIntegrationTests = os.Getenv("TESTNET") == "true" || os.Getenv("TESTNET") == "1"
+	if runIntegrationTests && runTestnetIntegrationTests {
+		panic("only one of INTEGRATION=true or TESTNET=true env var can be set")
+	}
 }
 
 // TestMain ... run main controller

e2e/setup.go

@@ -183,18 +183,21 @@ func TestSuiteConfig(testCfg *Cfg) server.CLIConfig {
 		panic(err)
 	}
 
+	svcManagerAddr := "0xD4A7E1Bd8015057293f0D0A557088c286942e84b" // holesky testnet
 	eigendaCfg := server.Config{
 		EdaClientConfig: clients.EigenDAClientConfig{
 			RPC:                      holeskyDA,
 			StatusQueryTimeout:       time.Minute * 45,
 			StatusQueryRetryInterval: pollInterval,
 			DisableTLS:               false,
 			SignerPrivateKeyHex:      pk,
+			EthRpcUrl:                ethRPC,
+			SvcManagerAddr:           svcManagerAddr,
 		},
 		VerifierConfig: verify.Config{
 			VerifyCerts:          false,
 			RPCURL:               ethRPC,
-			SvcManagerAddr:       "0xD4A7E1Bd8015057293f0D0A557088c286942e84b", // incompatible with non holeskly networks
+			SvcManagerAddr:       svcManagerAddr,
 			EthConfirmationDepth: 0,
 			KzgConfig: &kzg.KzgConfig{
 				G1Path:          "../resources/g1.point",

go.mod

@@ -5,7 +5,7 @@ go 1.22
 toolchain go1.22.0
 
 require (
-	github.com/Layr-Labs/eigenda v0.8.5-0.20241028201743-5fe3e910a22d
+	github.com/Layr-Labs/eigenda v0.8.5-0.20241031144746-e2ead56a306d
 	github.com/consensys/gnark-crypto v0.12.1
 	github.com/ethereum-optimism/optimism v1.9.4-0.20240927020138-a9c7f349d10b
 	github.com/ethereum/go-ethereum v1.14.11

go.sum

@@ -22,8 +22,8 @@ github.com/BurntSushi/toml v1.4.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2
 github.com/DataDog/datadog-go v3.2.0+incompatible/go.mod h1:LButxg5PwREeZtORoXG3tL4fMGNddJ+vMq1mwgfaqoQ=
 github.com/DataDog/zstd v1.5.6-0.20230824185856-869dae002e5e h1:ZIWapoIRN1VqT8GR8jAwb1Ie9GyehWjVcGh32Y2MznE=
 github.com/DataDog/zstd v1.5.6-0.20230824185856-869dae002e5e/go.mod h1:g4AWEaM3yOg3HYfnJ3YIawPnVdXJh9QME85blwSAmyw=
-github.com/Layr-Labs/eigenda v0.8.5-0.20241028201743-5fe3e910a22d h1:s5U1TaWhC1J2rwc9IQdU/COGvuXALCKMd9GbONUZMxc=
-github.com/Layr-Labs/eigenda v0.8.5-0.20241028201743-5fe3e910a22d/go.mod h1:sqUNf9Ak+EfAX82jDxrb4QbT/g3DViWD3b7YIk36skk=
+github.com/Layr-Labs/eigenda v0.8.5-0.20241031144746-e2ead56a306d h1:2JtVArkLjW61kilkvvLyFHXBMp0ClF8PYCAxWqRnoDQ=
+github.com/Layr-Labs/eigenda v0.8.5-0.20241031144746-e2ead56a306d/go.mod h1:sqUNf9Ak+EfAX82jDxrb4QbT/g3DViWD3b7YIk36skk=
 github.com/Layr-Labs/eigensdk-go v0.1.7-0.20240507215523-7e4891d5099a h1:L/UsJFw9M31FD/WgXTPFB0oxbq9Cu4Urea1xWPMQS7Y=
 github.com/Layr-Labs/eigensdk-go v0.1.7-0.20240507215523-7e4891d5099a/go.mod h1:OF9lmS/57MKxS0xpSpX0qHZl0SKkDRpvJIvsGvMN1y8=
 github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=

server/handlers.go

@@ -95,11 +95,12 @@ func (svr *Server) handleGetOPGenericCommitment(w http.ResponseWriter, r *http.R
 }
 
 func (svr *Server) handleGetShared(ctx context.Context, w http.ResponseWriter, comm []byte, meta commitments.CommitmentMeta) error {
-	svr.log.Info("Processing GET request", "commitment", hex.EncodeToString(comm), "commitmentMeta", meta)
+	commitmentHex := hex.EncodeToString(comm)
+	svr.log.Info("Processing GET request", "commitment", commitmentHex, "commitmentMeta", meta)
 	input, err := svr.sm.Get(ctx, comm, meta.Mode)
 	if err != nil {
 		err = MetaError{
-			Err:  fmt.Errorf("get request failed with commitment %v: %w", comm, err),
+			Err:  fmt.Errorf("get request failed with commitment %v: %w", commitmentHex, err),
 			Meta: meta,
 		}
 		if errors.Is(err, ErrNotFound) {

server/middleware.go

@@ -12,6 +12,7 @@ import (
 )
 
 // withMetrics is a middleware that records metrics for the route path.
+// It does not write anything to the response, that is the job of the handlers.
 func withMetrics(
 	handleFn func(http.ResponseWriter, *http.Request) error,
 	m metrics.Metricer,
@@ -30,9 +31,10 @@ func withMetrics(
 			}
 			return err
 		}
-		// we assume that every route will set the status header
+		// TODO: some routes don't have a version byte, such as /put simple commitments
 		versionByte, err := parseVersionByte(w, r)
 		if err != nil {
+			// we assume that every route will set the status header
 			recordDur(w.Header().Get("status"), string(mode), string(versionByte))
 			return fmt.Errorf("metrics middleware: error parsing version byte: %w", err)
 		}