Skip to content

Commit

Permalink
Remove scheme id from client CLI and take it from chain info (#840)
Browse files Browse the repository at this point in the history
* remove scheme id from client CLI and take it from chain info
  • Loading branch information
emmanuelm41 committed Oct 21, 2021
1 parent a0e6aa5 commit d0c06af
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 61 deletions.
37 changes: 10 additions & 27 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
"fmt"
"time"

"github.com/drand/drand/common/scheme"

"github.com/drand/drand/chain"
"github.com/drand/drand/log"
"github.com/drand/drand/metrics"
Expand All @@ -20,12 +18,9 @@ const clientStartupTimeoutDefault = time.Second * 5

// New Creates a client with specified configuration.
func New(options ...Option) (Client, error) {
sch, _ := scheme.GetSchemeByIDWithDefault("")

cfg := clientConfig{
cacheSize: 32,
log: log.DefaultLogger(),
scheme: sch,
}

for _, opt := range options {
Expand Down Expand Up @@ -65,6 +60,11 @@ func makeClient(cfg *clientConfig) (Client, error) {
return nil, err
}

// try to populate chain info
if err := cfg.tryPopulateInfo(cfg.clients...); err != nil {
return nil, err
}

// provision watcher client
var wc Client
if cfg.watcher != nil {
Expand All @@ -83,7 +83,8 @@ func makeClient(cfg *clientConfig) (Client, error) {

verifiers := make([]Client, 0, len(cfg.clients))
for _, source := range cfg.clients {
opts := Opts{scheme: cfg.scheme, strict: cfg.fullVerify}
opts := Opts{scheme: cfg.chainInfo.Scheme, strict: cfg.fullVerify}

nv := newVerifyingClient(source, cfg.previousResult, opts)
verifiers = append(verifiers, nv)
if source == wc {
Expand Down Expand Up @@ -133,9 +134,10 @@ func makeOptimizingClient(cfg *clientConfig, verifiers []Client, watcher Client,
}

func makeWatcherClient(cfg *clientConfig, cache Cache) (Client, error) {
if err := cfg.tryPopulateInfo(cfg.clients...); err != nil {
return nil, err
if cfg.chainInfo == nil {
return nil, fmt.Errorf("chain info cannot be nil")
}

w, err := cfg.watcher(cfg.chainInfo, cache)
if err != nil {
return nil, err
Expand All @@ -149,9 +151,6 @@ func attachMetrics(cfg *clientConfig, c Client) (Client, error) {
if err := metrics.RegisterClientMetrics(cfg.prometheus); err != nil {
return nil, err
}
if err := cfg.tryPopulateInfo(c); err != nil {
return nil, err
}
return newWatchLatencyMetricClient(c, cfg.chainInfo), nil
}
return c, nil
Expand All @@ -174,8 +173,6 @@ type clientConfig struct {
fullVerify bool
// insecure indicates the root of trust does not need to be present.
insecure bool
// scheme holds a set of values the client will use to act in specific ways, regarding signature verification, etc
scheme scheme.Scheme
// cache size - how large of a cache to keep locally.
cacheSize int
// customized client log.
Expand Down Expand Up @@ -227,20 +224,6 @@ func Insecurely() Option {
}
}

// WithSchemeID allows user to set a scheme using its ID. The scheme
// is used to customize some behaviors inside the client
func WithSchemeID(schID string) Option {
return func(cfg *clientConfig) error {
sch, err := scheme.GetSchemeByIDWithDefault(schID)
if err != nil {
return fmt.Errorf("scheme is not valid")
}

cfg.scheme = sch
return nil
}
}

// WithCacheSize specifies how large of a cache of randomness values should be
// kept locally. Default 32
func WithCacheSize(size int) Option {
Expand Down
12 changes: 5 additions & 7 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ func TestClientConstraints(t *testing.T) {
t.Fatal("Client needs root of trust unless insecure specified explicitly")
}

if _, e := client.New(client.From(client.MockClientWithResults(0, 5)), client.Insecurely()); e != nil {
c := client.MockClientWithResults(0, 5)
// As we will run is insecurely, we will set chain info so client can fetch it
c.OptionalInfo = fakeChainInfo()

if _, e := client.New(client.From(c), client.Insecurely()); e != nil {
t.Fatal(e)
}
}
Expand All @@ -48,7 +52,6 @@ func TestClientMultiple(t *testing.T) {
var e error
c, e = client.New(
client.From(http.ForURLs([]string{"http://" + addr1, "http://" + addr2}, chainInfo.Hash())...),
client.WithSchemeID(sch.ID),
client.WithChainHash(chainInfo.Hash()))

if e != nil {
Expand Down Expand Up @@ -93,7 +96,6 @@ func TestClientCache(t *testing.T) {
var c client.Client
var e error
c, e = client.New(client.From(http.ForURLs([]string{"http://" + addr1}, chainInfo.Hash())...),
client.WithSchemeID(sch.ID),
client.WithChainHash(chainInfo.Hash()), client.WithCacheSize(1))

if e != nil {
Expand Down Expand Up @@ -125,7 +127,6 @@ func TestClientWithoutCache(t *testing.T) {
var e error
c, e = client.New(
client.From(http.ForURLs([]string{"http://" + addr1}, chainInfo.Hash())...),
client.WithSchemeID(sch.ID),
client.WithChainHash(chainInfo.Hash()),
client.WithCacheSize(0))

Expand Down Expand Up @@ -162,7 +163,6 @@ func TestClientWithWatcher(t *testing.T) {
var err error
c, err = client.New(
client.WithChainInfo(info),
client.WithSchemeID(sch.ID),
client.WithWatcher(watcherCtor),
)

Expand Down Expand Up @@ -250,7 +250,6 @@ func TestClientAutoWatch(t *testing.T) {
client.From(client.MockClientWithInfo(chainInfo)),
client.WithChainHash(chainInfo.Hash()),
client.WithWatcher(watcherCtor),
client.WithSchemeID(sch.ID),
client.WithAutoWatch(),
)

Expand Down Expand Up @@ -314,7 +313,6 @@ func TestClientAutoWatchRetry(t *testing.T) {
client.WithAutoWatch(),
client.WithAutoWatchRetry(time.Second),
client.WithCacheSize(len(results)),
client.WithSchemeID(sch.ID),
)

if err != nil {
Expand Down
10 changes: 7 additions & 3 deletions client/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import (
// MockClient provide a mocked client interface
type MockClient struct {
sync.Mutex
WatchCh chan Result
WatchF func(context.Context) <-chan Result
Results []mock.Result
OptionalInfo *chain.Info
WatchCh chan Result
WatchF func(context.Context) <-chan Result
Results []mock.Result
// Delay causes results to be delivered after this period of time has
// passed. Note that if the context is canceled a result is still consumed
// from Results.
Expand Down Expand Up @@ -83,6 +84,9 @@ func (m *MockClient) Watch(ctx context.Context) <-chan Result {
}

func (m *MockClient) Info(ctx context.Context) (*chain.Info, error) {
if m.OptionalInfo != nil {
return m.OptionalInfo, nil
}
return nil, errors.New("not supported (mock client info)")
}

Expand Down
4 changes: 4 additions & 0 deletions client/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,17 @@ import (
"testing"
"time"

"github.com/drand/drand/common/scheme"

"github.com/drand/drand/chain"
"github.com/drand/drand/test"
)

// fakeChainInfo creates a chain info object for use in tests.
func fakeChainInfo() *chain.Info {
sch := scheme.GetSchemeFromEnv()
return &chain.Info{
Scheme: sch,
Period: time.Second,
GenesisTime: time.Now().Unix(),
PublicKey: test.GenerateIDs(1)[0].Public.Key,
Expand Down
3 changes: 1 addition & 2 deletions client/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func mockClientWithVerifiableResults(n int) (client.Client, []mock.Result, error
sch := scheme.GetSchemeFromEnv()

info, results := mock.VerifiableResults(n, sch)
mc := client.MockClient{Results: results, StrictRounds: true}
mc := client.MockClient{Results: results, StrictRounds: true, OptionalInfo: info}

var c client.Client
var err error
Expand All @@ -24,7 +24,6 @@ func mockClientWithVerifiableResults(n int) (client.Client, []mock.Result, error
client.WithChainInfo(info),
client.WithVerifiedResult(&results[0]),
client.WithFullChainVerification(),
client.WithSchemeID(sch.ID),
)

if err != nil {
Expand Down
17 changes: 0 additions & 17 deletions cmd/client/lib/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (
"path"
"strings"

"github.com/drand/drand/common/scheme"

"github.com/BurntSushi/toml"
"github.com/drand/drand/chain"
"github.com/drand/drand/client"
Expand Down Expand Up @@ -80,13 +78,6 @@ var (
Usage: "Local (host:)port for constructed libp2p host to listen on",
}

// TypeFlag indicates a set of values drand will use to configure the randomness generation process
SchemeFlag = &cli.StringFlag{
Name: "scheme",
Usage: "Indicates a set of values drand will use to configure the randomness generation process",
Value: scheme.DefaultSchemeID,
}

// JsonFlag is the CLI flag for enabling JSON output for logger
JSONFlag = &cli.BoolFlag{
Name: "json",
Expand All @@ -105,7 +96,6 @@ var ClientFlags = []cli.Flag{
RelayFlag,
PortFlag,
JSONFlag,
SchemeFlag,
}

// Create builds a client, and can be invoked from a cli action supplied
Expand Down Expand Up @@ -152,13 +142,6 @@ func Create(c *cli.Context, withInstrumentation bool, opts ...client.Option) (cl
opts = append(opts, client.Insecurely())
}

schemeFound, err := scheme.GetSchemeByIDWithDefault(c.String(SchemeFlag.Name))
if err != nil {
return nil, fmt.Errorf("scheme %s given is invalid", c.String(SchemeFlag.Name))
}

opts = append(opts, client.WithSchemeID(schemeFound.ID))

clients = append(clients, buildHTTPClients(c, &info, hash, withInstrumentation)...)

gopt, err := buildGossipClient(c)
Expand Down
10 changes: 5 additions & 5 deletions cmd/client/lib/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,36 +58,36 @@ func TestClientLib(t *testing.T) {
go grpcLis.Start()
defer grpcLis.Stop(context.Background())

args := []string{"mock-client", "--url", "http://" + addr, "--grpc-connect", grpcLis.Addr(), "--insecure", "--scheme", sch.ID}
args := []string{"mock-client", "--url", "http://" + addr, "--grpc-connect", grpcLis.Addr(), "--insecure"}

fmt.Printf("%+v", args)
err = run(args)
if err != nil {
t.Fatal("GRPC should work", err)
}

args = []string{"mock-client", "--url", "https://" + addr, "--scheme", sch.ID}
args = []string{"mock-client", "--url", "https://" + addr}

err = run(args)
if err == nil {
t.Fatal("http needs insecure or hash", err)
}

args = []string{"mock-client", "--url", "http://" + addr, "--hash", hex.EncodeToString(info.Hash()), "--scheme", sch.ID}
args = []string{"mock-client", "--url", "http://" + addr, "--hash", hex.EncodeToString(info.Hash())}

err = run(args)
if err != nil {
t.Fatal("http should construct", err)
}

args = []string{"mock-client", "--relay", fakeGossipRelayAddr, "--scheme", sch.ID}
args = []string{"mock-client", "--relay", fakeGossipRelayAddr}

err = run(args)
if err == nil {
t.Fatal("relays need URL or hash", err)
}

args = []string{"mock-client", "--relay", fakeGossipRelayAddr, "--hash", hex.EncodeToString(info.Hash()), "--scheme", sch.ID}
args = []string{"mock-client", "--relay", fakeGossipRelayAddr, "--hash", hex.EncodeToString(info.Hash())}

err = run(args)
if err != nil {
Expand Down

0 comments on commit d0c06af

Please sign in to comment.