op-service: remove the requirement for signer.address to be set when …

…using op-service
ethereum-optimism · Oct 7, 2024 · ebeb2a4 · ebeb2a4
1 parent eb5ede9
commit ebeb2a4
Show file tree

Hide file tree

Showing 6 changed files with 8 additions and 18 deletions.
diff --git a/op-node/p2p/gossip_test.go b/op-node/p2p/gossip_test.go
@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	opsigner "github.com/ethereum-optimism/optimism/op-service/signer"
 	"io"
 	"math/big"
 	"testing"
@@ -14,6 +13,7 @@ import (
 	"github.com/ethereum-optimism/optimism/op-node/rollup"
 	"github.com/golang/snappy"
 
+	opsigner "github.com/ethereum-optimism/optimism/op-service/signer"
 	// "github.com/ethereum-optimism/optimism/op-service/eth"
 	"github.com/ethereum-optimism/optimism/op-service/eth"
 	"github.com/ethereum-optimism/optimism/op-service/testutils"

diff --git a/op-node/p2p/signer.go b/op-node/p2p/signer.go
@@ -4,15 +4,15 @@ import (
 	"context"
 	"crypto/ecdsa"
 	"errors"
-	opsigner "github.com/ethereum-optimism/optimism/op-service/signer"
-	"github.com/ethereum/go-ethereum/log"
 	"io"
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/log"
 
 	"github.com/ethereum-optimism/optimism/op-node/rollup"
+	opsigner "github.com/ethereum-optimism/optimism/op-service/signer"
 )
 
 var SigningDomainBlocksV1 = [32]byte{}

diff --git a/op-service/signer/cli.go b/op-service/signer/cli.go
@@ -50,17 +50,14 @@ func (c CLIConfig) Check() error {
 	if err := c.TLSConfig.Check(); err != nil {
 		return err
 	}
-	if !((c.Endpoint == "" && c.Address == "") || (c.Endpoint != "" && c.Address != "")) {
+	if !((c.Endpoint == "" && c.Address == "") || (c.Endpoint != "")) {
 		return errors.New("signer endpoint and address must both be set or not set")
 	}
 	return nil
 }
 
 func (c CLIConfig) Enabled() bool {
-	if c.Endpoint != "" && c.Address != "" {
-		return true
-	}
-	return false
+	return c.Endpoint != ""
 }
 
 func ReadCLIConfig(ctx *cli.Context) CLIConfig {

diff --git a/op-service/signer/cli_test.go b/op-service/signer/cli_test.go
@@ -31,13 +31,6 @@ func TestInvalidConfig(t *testing.T) {
 				config.Address = "0x1234"
 			},
 		},
-		{
-			name:     "MissingAddress",
-			expected: "signer endpoint and address must both be set or not set",
-			configChange: func(config *CLIConfig) {
-				config.Endpoint = "http://localhost"
-			},
-		},
 		{
 			name:     "InvalidTLSConfig",
 			expected: "all tls flags must be set if at least one is set",
@@ -58,7 +51,7 @@ func TestInvalidConfig(t *testing.T) {
 
 func configForArgs(args ...string) CLIConfig {
 	app := cli.NewApp()
-	app.Flags = CLIFlags("TEST_")
+	app.Flags = CLIFlags("TEST_", "")
 	app.Name = "test"
 	var config CLIConfig
 	app.Action = func(ctx *cli.Context) error {

diff --git a/op-service/signer/client.go b/op-service/signer/client.go
@@ -121,7 +121,7 @@ func (s *SignerClient) SignBlockPayload(ctx context.Context, signingHash common.
 	}
 
 	if len(result) < 65 {
-		return [65]byte{}, fmt.Errorf("invalid signature", result)
+		return [65]byte{}, fmt.Errorf("invalid signature: %s", result.String())
 	}
 
 	var signature [65]byte

diff --git a/op-service/tls/cli_test.go b/op-service/tls/cli_test.go
@@ -45,7 +45,7 @@ func TestInvalidConfig(t *testing.T) {
 
 func configForArgs(args ...string) CLIConfig {
 	app := cli.NewApp()
-	app.Flags = CLIFlagsWithFlagPrefix("TEST_", "test")
+	app.Flags = CLIFlagsWithFlagPrefix("TEST_", "test", "")
 	app.Name = "test"
 	var config CLIConfig
 	app.Action = func(ctx *cli.Context) error {