Fixing all outstanding issues except metrics

The metrics and the WithPrometheus option currently are no-op since this isn't being handled properly anymore

Makefile

@@ -1,3 +1,10 @@
+.PHONY: drand-relay-gossip client-tool build clean
+
+build: drand-relay-gossip client-tool
+
+clean:
+	rm -f ./drand-relay-gossip ./drand-cli
+
 drand-relay-gossip:
 	go build -o drand-relay-gossip ./gossip-relay/main.go
 

client/client.go

@@ -16,37 +16,49 @@ import (
 	"github.com/drand/drand/v2/crypto"
 )
 
-const clientStartupTimeoutDefault = time.Second * 5
+const ClientStartupTimeout = time.Second * 5
 
-// New creates a client with specified configuration.
-func New(ctx context.Context, l log.Logger, options ...Option) (drand.Client, error) {
+// New creates a watcher, verifying, optimizing client with the specified options.
+// It expects to be provided at least 1 valid client, or more using From().
+// If not specified, a default context with a timeout of ClientStartupTimeout
+// will be used when fetching chain information during client setup.
+func New(options ...Option) (drand.Client, error) {
 	cfg := clientConfig{
 		cacheSize: 32,
-		log:       l,
 	}
 
 	for _, opt := range options {
 		if err := opt(&cfg); err != nil {
 			return nil, err
 		}
 	}
-	return makeClient(ctx, l, &cfg)
+	if cfg.log == nil {
+		cfg.log = log.DefaultLogger()
+	}
+	if cfg.setupCtx == nil {
+		ctx, cancel := context.WithTimeout(context.Background(), ClientStartupTimeout)
+		cfg.setupCtx = ctx
+		defer cancel()
+	}
+	return makeClient(&cfg)
 }
 
 // Wrap provides a single entrypoint for wrapping a concrete client
-// implementation with configured aggregation, caching, and retry logic
-func Wrap(ctx context.Context, l log.Logger, clients []drand.Client, options ...Option) (drand.Client, error) {
-	return New(ctx, l, append(options, From(clients...))...)
+// implementation with configured aggregation, caching, and retry logic.
+// It calls New and has the same expectations.
+func Wrap(clients []drand.Client, options ...Option) (drand.Client, error) {
+	return New(append(options, From(clients...))...)
 }
 
-func trySetLog(c drand.Client, l log.Logger) {
+func trySetLog(c any, l log.Logger) {
 	if lc, ok := c.(drand.LoggingClient); ok {
 		lc.SetLog(l)
 	}
 }
 
-// makeClient creates a client from a configuration.
-func makeClient(ctx context.Context, l log.Logger, cfg *clientConfig) (drand.Client, error) {
+// makeClient creates a watching verifying optimizing client from a configuration.
+func makeClient(cfg *clientConfig) (drand.Client, error) {
+	l := cfg.log
 	if !cfg.insecure && cfg.chainHash == nil && cfg.chainInfo == nil {
 		l.Errorw("no root of trust specified")
 		return nil, errors.New("no root of trust specified")
@@ -65,7 +77,7 @@ func makeClient(ctx context.Context, l log.Logger, cfg *clientConfig) (drand.Cli
 	}
 
 	// try to populate chain info
-	if err := cfg.tryPopulateInfo(ctx, cfg.clients...); err != nil {
+	if err := cfg.tryPopulateInfo(cfg.setupCtx, cfg.clients...); err != nil {
 		return nil, err
 	}
 
@@ -146,7 +158,7 @@ func makeWatcherClient(cfg *clientConfig, cache Cache) (drand.Client, error) {
 		return nil, fmt.Errorf("chain info cannot be nil")
 	}
 
-	w, err := cfg.watcher(cfg.chainInfo, cache)
+	w, err := cfg.watcher(cfg.log, cfg.chainInfo, cache)
 	if err != nil {
 		return nil, err
 	}
@@ -179,6 +191,9 @@ type clientConfig struct {
 	// customized client log.
 	log log.Logger
 
+	// only used during setup to try and fetch chain info if chain info is nil
+	setupCtx context.Context
+
 	// autoWatchRetry specifies the time after which the watch channel
 	// created by the autoWatch is re-opened when no context error occurred.
 	autoWatchRetry time.Duration
@@ -188,18 +203,14 @@ type clientConfig struct {
 
 func (c *clientConfig) tryPopulateInfo(ctx context.Context, clients ...drand.Client) (err error) {
 	if c.chainInfo == nil {
-		ctx, cancel := context.WithTimeout(ctx, clientStartupTimeoutDefault)
-		defer cancel()
-
+		var cerr error
 		for _, cli := range clients {
-			c.chainInfo, err = cli.Info(ctx)
-			if err == nil {
+			c.chainInfo, cerr = cli.Info(ctx)
+			if cerr == nil {
 				return
 			}
-
-			if ctx.Err() != nil {
-				return ctx.Err()
-			}
+			// we accumulate errors to try all clients even if the first one fails
+			err = errors.Join(err, cerr, ctx.Err())
 		}
 	}
 	return
@@ -247,7 +258,8 @@ func WithChainHash(chainHash []byte) Option {
 }
 
 // WithChainInfo configures the client to root trust in the given randomness
-// chain information
+// chain information, this prevents the setup of the client from attempting to
+// fetch the chain info through the clients from the remotes.
 func WithChainInfo(chainInfo *chain.Info) Option {
 	return func(cfg *clientConfig) error {
 		if cfg.chainHash != nil && !bytes.Equal(cfg.chainHash, chainInfo.Hash()) {
@@ -258,10 +270,32 @@ func WithChainInfo(chainInfo *chain.Info) Option {
 	}
 }
 
-// WithVerifiedResult provides a checkpoint of randomness verified at a given round.
+// WithLogger overrides the logging options for the client,
+// allowing specification of additional tags, or redirection / configuration
+// of logging level and output. If it is not used to set a specific logger,
+// the client's logger will be used. This only works for clients that satisfy
+// the drand.LoggingClient interface.
+func WithLogger(l log.Logger) Option {
+	return func(cfg *clientConfig) error {
+		cfg.log = l
+		return nil
+	}
+}
+
+// WithSetupCtx allows you to provide a custom setup context that will be used
+// if WithChainInfo isn't used and the client setup has to try and fetch the
+// ChainInfo from the remotes.
+func WithSetupCtx(ctx context.Context) Option {
+	return func(cfg *clientConfig) error {
+		cfg.setupCtx = ctx
+		return nil
+	}
+}
+
+// WithTrustedResult provides a checkpoint of randomness verified at a given round.
 // Used in combination with `VerifyFullChain`, this allows for catching up only on
-// previously not-yet-verified results.
-func WithVerifiedResult(result drand.Result) Option {
+// previously not-yet-verified results. Note that in general this is not something you need.
+func WithTrustedResult(result drand.Result) Option {
 	return func(cfg *clientConfig) error {
 		if cfg.previousResult != nil && cfg.previousResult.GetRound() > result.GetRound() {
 			return errors.New("refusing to override verified result with an earlier result")
@@ -271,12 +305,13 @@ func WithVerifiedResult(result drand.Result) Option {
 	}
 }
 
-// WithFullChainVerification validates random beacons not just as being generated correctly
-// from the group signature, but ensures that the full chain is deterministic by making sure
+// WithFullChainVerification validates random beacons using the chained schemes are
+// not just as being generated correctly from the group signature,
+// but ensures that the full chain is deterministic by making sure
 // each round is derived correctly from the previous one. In cases of compromise where
 // a single party learns sufficient shares to derive the full key, malicious randomness
 // could otherwise be generated that is signed, but not properly derived from previous rounds
-// according to protocol.
+// according to protocol. Note that in general this is not something you need.
 func WithFullChainVerification() Option {
 	return func(cfg *clientConfig) error {
 		cfg.fullVerify = true
@@ -290,7 +325,7 @@ type Watcher interface {
 }
 
 // WatcherCtor creates a Watcher once chain info is known.
-type WatcherCtor func(chainInfo *chain.Info, cache Cache) (Watcher, error)
+type WatcherCtor func(l log.Logger, chainInfo *chain.Info, cache Cache) (Watcher, error)
 
 // WithWatcher specifies a channel that can provide notifications of new
 // randomness bootstrappeed from the chain info.

client/client_test.go

@@ -23,25 +23,23 @@ import (
 )
 
 func TestClientConstraints(t *testing.T) {
-	ctx := context.Background()
-	lg := log.New(nil, log.DebugLevel, true)
-	if _, e := client.New(ctx, lg); e == nil {
+	if _, e := client.New(); e == nil {
 		t.Fatal("client can't be created without root of trust")
 	}
 
-	if _, e := client.New(ctx, lg, client.WithChainHash([]byte{0})); e == nil {
+	if _, e := client.New(client.WithChainHash([]byte{0})); e == nil {
 		t.Fatal("Client needs URLs if only a chain hash is specified")
 	}
 
-	if _, e := client.New(ctx, lg, client.From(clientMock.ClientWithResults(0, 5))); e == nil {
+	if _, e := client.New(client.From(clientMock.ClientWithResults(0, 5))); e == nil {
 		t.Fatal("Client needs root of trust unless insecure specified explicitly")
 	}
 
 	c := clientMock.ClientWithResults(0, 5)
 	// As we will run is insecurely, we will set chain info so client can fetch it
 	c.OptionalInfo = fakeChainInfo(t)
 
-	if _, e := client.New(ctx, lg, client.From(c), client.Insecurely()); e != nil {
+	if _, e := client.New(client.From(c), client.Insecurely()); e != nil {
 		t.Fatal(e)
 	}
 }
@@ -69,10 +67,7 @@ func TestClientMultiple(t *testing.T) {
 		return
 	}
 
-	c, e := client.New(ctx,
-		lg,
-		client.From(httpClients...),
-		client.WithChainHash(chainInfo.Hash()))
+	c, e := client.New(client.From(httpClients...), client.WithChainHash(chainInfo.Hash()))
 
 	if e != nil {
 		t.Fatal(e)
@@ -97,8 +92,7 @@ func TestClientWithChainInfo(t *testing.T) {
 	lg := log.New(nil, log.DebugLevel, true)
 	hc, err := http.NewWithInfo(lg, "http://nxdomain.local/", chainInfo, nil)
 	require.NoError(t, err)
-	c, err := client.New(ctx, lg, client.WithChainInfo(chainInfo),
-		client.From(hc))
+	c, err := client.New(client.WithChainInfo(chainInfo), client.From(hc))
 	if err != nil {
 		t.Fatal("existing group creation shouldn't do additional validaiton.")
 	}
@@ -124,8 +118,11 @@ func TestClientCache(t *testing.T) {
 		return
 	}
 
-	c, e := client.New(ctx, lg, client.From(httpClients...),
-		client.WithChainHash(chainInfo.Hash()), client.WithCacheSize(1))
+	c, e := client.New(
+		client.From(httpClients...),
+		client.WithChainHash(chainInfo.Hash()),
+		client.WithCacheSize(1),
+	)
 
 	if e != nil {
 		t.Fatal(e)
@@ -162,8 +159,7 @@ func TestClientWithoutCache(t *testing.T) {
 		return
 	}
 
-	c, err := client.New(ctx,
-		lg,
+	c, err := client.New(
 		client.From(httpClients...),
 		client.WithChainHash(chainInfo.Hash()),
 		client.WithCacheSize(0))
@@ -192,13 +188,12 @@ func TestClientWithWatcher(t *testing.T) {
 	}
 	close(ch)
 
-	watcherCtor := func(chainInfo *chain.Info, _ client.Cache) (client.Watcher, error) {
+	watcherCtor := func(l log.Logger, chainInfo *chain.Info, _ client.Cache) (client.Watcher, error) {
 		return &clientMock.Client{WatchCh: ch}, nil
 	}
 
 	var c drand.Client
-	c, err = client.New(ctx,
-		lg,
+	c, err = client.New(client.WithLogger(lg),
 		client.WithChainInfo(info),
 		client.WithWatcher(watcherCtor),
 	)
@@ -217,16 +212,13 @@ func TestClientWithWatcher(t *testing.T) {
 }
 
 func TestClientWithWatcherCtorError(t *testing.T) {
-	ctx := context.Background()
-	lg := log.New(nil, log.DebugLevel, true)
 	watcherErr := errors.New("boom")
-	watcherCtor := func(chainInfo *chain.Info, _ client.Cache) (client.Watcher, error) {
+	watcherCtor := func(l log.Logger, chainInfo *chain.Info, _ client.Cache) (client.Watcher, error) {
 		return nil, watcherErr
 	}
 
 	// constructor should return error returned by watcherCtor
-	_, err := client.New(ctx,
-		lg,
+	_, err := client.New(
 		client.WithChainInfo(fakeChainInfo(t)),
 		client.WithWatcher(watcherCtor),
 	)
@@ -236,15 +228,13 @@ func TestClientWithWatcherCtorError(t *testing.T) {
 }
 
 func TestClientChainHashOverrideError(t *testing.T) {
-	ctx := context.Background()
 	lg := log.New(nil, log.DebugLevel, true)
 	chainInfo := fakeChainInfo(t)
 	_, err := client.Wrap(
-		ctx,
-		lg,
 		[]drand.Client{client.EmptyClientWithInfo(chainInfo)},
 		client.WithChainInfo(chainInfo),
 		client.WithChainHash(fakeChainInfo(t).Hash()),
+		client.WithLogger(lg),
 	)
 	if err == nil {
 		t.Fatal("expected error, received no error")
@@ -255,15 +245,13 @@ func TestClientChainHashOverrideError(t *testing.T) {
 }
 
 func TestClientChainInfoOverrideError(t *testing.T) {
-	ctx := context.Background()
 	lg := log.New(nil, log.DebugLevel, true)
 	chainInfo := fakeChainInfo(t)
 	_, err := client.Wrap(
-		ctx,
-		lg,
 		[]drand.Client{client.EmptyClientWithInfo(chainInfo)},
 		client.WithChainHash(chainInfo.Hash()),
 		client.WithChainInfo(fakeChainInfo(t)),
+		client.WithLogger(lg),
 	)
 	if err == nil {
 		t.Fatal("expected error, received no error")
@@ -298,13 +286,12 @@ func TestClientAutoWatch(t *testing.T) {
 	}
 	close(ch)
 
-	watcherCtor := func(chainInfo *chain.Info, _ client.Cache) (client.Watcher, error) {
+	watcherCtor := func(l log.Logger, chainInfo *chain.Info, _ client.Cache) (client.Watcher, error) {
 		return &clientMock.Client{WatchCh: ch}, nil
 	}
 
 	var c drand.Client
-	c, err = client.New(ctx,
-		lg,
+	c, err = client.New(
 		client.From(clientMock.ClientWithInfo(chainInfo)),
 		client.WithChainHash(chainInfo.Hash()),
 		client.WithWatcher(watcherCtor),
@@ -327,7 +314,6 @@ func TestClientAutoWatch(t *testing.T) {
 
 func TestClientAutoWatchRetry(t *testing.T) {
 	ctx := context.Background()
-	lg := log.New(nil, log.DebugLevel, true)
 	sch, err := crypto.GetSchemeFromEnv()
 	require.NoError(t, err)
 
@@ -367,8 +353,7 @@ func TestClientAutoWatchRetry(t *testing.T) {
 	}
 
 	var c drand.Client
-	c, err = client.New(ctx,
-		lg,
+	c, err = client.New(
 		client.From(&failer, clientMock.ClientWithInfo(info)),
 		client.WithChainInfo(info),
 		client.WithAutoWatch(),