From f2100c8f0055c27ac0678ae54e8f8c3eccc377cd Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Thu, 31 Oct 2024 14:27:35 +0000 Subject: [PATCH] fix(disperser-client): make constructor return err when config is wrong --- api/clients/disperser_client.go | 32 +++++++++++++++++++++------- api/clients/disperser_client_test.go | 5 +++-- api/clients/eigenda_client.go | 13 ++++++----- inabox/tests/integration_test.go | 4 ++-- inabox/tests/ratelimit_test.go | 5 +++-- tools/traffic/generator.go | 9 ++++++-- tools/traffic/generator_v2.go | 8 +++++-- 7 files changed, 53 insertions(+), 23 deletions(-) diff --git a/api/clients/disperser_client.go b/api/clients/disperser_client.go index c051fb7c35..c2422ea00c 100644 --- a/api/clients/disperser_client.go +++ b/api/clients/disperser_client.go @@ -102,20 +102,36 @@ var _ DisperserClient = &disperserClient{} // // // Subsequent calls will use the existing connection // status2, requestId2, err := client.DisperseBlob(ctx, otherData, otherQuorums) -func NewDisperserClient(config *Config, signer core.BlobRequestSigner) *disperserClient { - if config == nil { - config = &Config{} - } - if config.MaxRetrieveBlobSizeBytes == 0 { - // Set to 100MiB for forward compatibility. - // Check official documentation for current max blob size on mainnet. - config.MaxRetrieveBlobSizeBytes = 100 * 1024 * 1024 +func NewDisperserClient(config *Config, signer core.BlobRequestSigner) (*disperserClient, error) { + if err := checkConfigAndSetDefaults(config); err != nil { + return nil, fmt.Errorf("invalid config: %w", err) } return &disperserClient{ config: config, signer: signer, // conn and client are initialized lazily + }, nil +} + +func checkConfigAndSetDefaults(c *Config) error { + if c == nil { + return fmt.Errorf("config is nil") + } + if c.Hostname == "" { + return fmt.Errorf("config.Hostname is empty") } + if c.Port == "" { + return fmt.Errorf("config.Port is empty") + } + if c.Timeout == 0 { + return fmt.Errorf("config.Timeout is 0") + } + if c.MaxRetrieveBlobSizeBytes == 0 { + // Set to 100MiB for forward compatibility. + // Check official documentation for current max blob size on mainnet. + c.MaxRetrieveBlobSizeBytes = 100 * 1024 * 1024 + } + return nil } // Close closes the grpc connection to the disperser server. diff --git a/api/clients/disperser_client_test.go b/api/clients/disperser_client_test.go index 28d98dea74..00277657bd 100644 --- a/api/clients/disperser_client_test.go +++ b/api/clients/disperser_client_test.go @@ -14,12 +14,13 @@ import ( func TestPutBlobNoopSigner(t *testing.T) { config := clients.NewConfig("nohost", "noport", time.Second, false) - disperserClient := clients.NewDisperserClient(config, auth.NewLocalNoopSigner()) + disperserClient, err := clients.NewDisperserClient(config, auth.NewLocalNoopSigner()) + assert.NoError(t, err) test := []byte("test") test[0] = 0x00 // make sure the first byte of the requst is always 0 quorums := []uint8{0} - _, _, err := disperserClient.DisperseBlobAuthenticated(context.Background(), test, quorums) + _, _, err = disperserClient.DisperseBlobAuthenticated(context.Background(), test, quorums) st, isGRPCError := status.FromError(err) assert.True(t, isGRPCError) assert.Equal(t, codes.InvalidArgument.String(), st.Code().String()) diff --git a/api/clients/eigenda_client.go b/api/clients/eigenda_client.go index d2569b01d2..81d7716807 100644 --- a/api/clients/eigenda_client.go +++ b/api/clients/eigenda_client.go @@ -85,16 +85,16 @@ func NewEigenDAClient(log log.Logger, config EigenDAClientConfig) (*EigenDAClien var edasmCaller *edasm.ContractEigenDAServiceManagerCaller ethClient, err = ethclient.Dial(config.EthRpcUrl) if err != nil { - return nil, fmt.Errorf("failed to dial ETH RPC node: %w", err) + return nil, fmt.Errorf("dial ETH RPC node: %w", err) } edasmCaller, err = edasm.NewContractEigenDAServiceManagerCaller(common.HexToAddress(config.SvcManagerAddr), ethClient) if err != nil { - return nil, fmt.Errorf("failed to create EigenDAServiceManagerCaller: %w", err) + return nil, fmt.Errorf("new EigenDAServiceManagerCaller: %w", err) } host, port, err := net.SplitHostPort(config.RPC) if err != nil { - return nil, fmt.Errorf("failed to parse EigenDA RPC: %w", err) + return nil, fmt.Errorf("parse EigenDA RPC: %w", err) } var signer core.BlobRequestSigner @@ -108,11 +108,14 @@ func NewEigenDAClient(log log.Logger, config EigenDAClientConfig) (*EigenDAClien } disperserConfig := NewConfig(host, port, config.ResponseTimeout, !config.DisableTLS) - disperserClient := NewDisperserClient(disperserConfig, signer) + disperserClient, err := NewDisperserClient(disperserConfig, signer) + if err != nil { + return nil, fmt.Errorf("new disperser-client: %w", err) + } lowLevelCodec, err := codecs.BlobEncodingVersionToCodec(config.PutBlobEncodingVersion) if err != nil { - return nil, fmt.Errorf("error initializing EigenDA client: %w", err) + return nil, fmt.Errorf("create low level codec: %w", err) } var codec codecs.BlobCodec diff --git a/inabox/tests/integration_test.go b/inabox/tests/integration_test.go index 4bfdfa22f3..d7dd44a7c6 100644 --- a/inabox/tests/integration_test.go +++ b/inabox/tests/integration_test.go @@ -36,12 +36,12 @@ var _ = Describe("Inabox Integration", func() { privateKeyHex := "0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcded" signer := auth.NewLocalBlobRequestSigner(privateKeyHex) - disp := clients.NewDisperserClient(&clients.Config{ + disp, err := clients.NewDisperserClient(&clients.Config{ Hostname: "localhost", Port: "32003", Timeout: 10 * time.Second, }, signer) - + Expect(err).To(BeNil()) Expect(disp).To(Not(BeNil())) data := make([]byte, 1024) diff --git a/inabox/tests/ratelimit_test.go b/inabox/tests/ratelimit_test.go index 47a93decb7..1f72cbe015 100644 --- a/inabox/tests/ratelimit_test.go +++ b/inabox/tests/ratelimit_test.go @@ -107,15 +107,16 @@ func testRatelimit(t *testing.T, testConfig *deploy.Config, c ratelimitTestCase) ctx, cancel := context.WithTimeout(context.Background(), time.Minute) defer cancel() - disp := clients.NewDisperserClient(&clients.Config{ + disp, err := clients.NewDisperserClient(&clients.Config{ Hostname: "localhost", Port: testConfig.Dispersers[0].DISPERSER_SERVER_GRPC_PORT, Timeout: 10 * time.Second, }, nil) + assert.NoError(t, err) assert.NotNil(t, disp) data := make([]byte, c.blobSize) - _, err := rand.Read(data) + _, err = rand.Read(data) assert.NoError(t, err) dispersalTicker := time.NewTicker(c.dispersalInterval) diff --git a/tools/traffic/generator.go b/tools/traffic/generator.go index 2f8731ffd5..5afdf0b564 100644 --- a/tools/traffic/generator.go +++ b/tools/traffic/generator.go @@ -4,6 +4,7 @@ import ( "context" "crypto/rand" "encoding/hex" + "fmt" "os" "os/signal" "sync" @@ -27,12 +28,16 @@ func NewTrafficGenerator(config *Config, signer core.BlobRequestSigner) (*Traffi loggerConfig := common.DefaultLoggerConfig() logger, err := common.NewLogger(loggerConfig) if err != nil { - return nil, err + return nil, fmt.Errorf("new logger: %w", err) } + dispserserClient, err := clients.NewDisperserClient(&config.Config, signer) + if err != nil { + return nil, fmt.Errorf("new disperser-client: %w", err) + } return &TrafficGenerator{ Logger: logger, - DisperserClient: clients.NewDisperserClient(&config.Config, signer), + DisperserClient: dispserserClient, Config: config, }, nil } diff --git a/tools/traffic/generator_v2.go b/tools/traffic/generator_v2.go index 7b2a92159f..61575b9c00 100644 --- a/tools/traffic/generator_v2.go +++ b/tools/traffic/generator_v2.go @@ -88,7 +88,11 @@ func NewTrafficGeneratorV2(config *config.Config) (*Generator, error) { unconfirmedKeyChannel := make(chan *workers.UnconfirmedKey, 100) - disperserClient := clients.NewDisperserClient(config.DisperserClientConfig, signer) + disperserClient, err := clients.NewDisperserClient(config.DisperserClientConfig, signer) + if err != nil { + cancel() + return nil, fmt.Errorf("new disperser-client: %w", err) + } statusVerifier := workers.NewBlobStatusTracker( &ctx, &waitGroup, @@ -134,7 +138,7 @@ func NewTrafficGeneratorV2(config *config.Config) (*Generator, error) { waitGroup: &waitGroup, generatorMetrics: generatorMetrics, logger: &logger, - disperserClient: clients.NewDisperserClient(config.DisperserClientConfig, signer), + disperserClient: disperserClient, eigenDAClient: client, config: config, writers: writers,