diff --git a/app/app.go b/app/app.go index d5fbf8136..2967cbfba 100644 --- a/app/app.go +++ b/app/app.go @@ -76,6 +76,7 @@ type Config struct { PrivKeyFile string PrivKeyLocking bool MonitoringAddr string + DebugAddr string ValidatorAPIAddr string BeaconNodeAddrs []string JaegerAddr string @@ -259,7 +260,7 @@ func Run(ctx context.Context, conf Config) (err error) { return err } - wireMonitoringAPI(ctx, life, conf.MonitoringAddr, tcpNode, eth2Cl, peerIDs, + wireMonitoringAPI(ctx, life, conf.MonitoringAddr, conf.DebugAddr, tcpNode, eth2Cl, peerIDs, promRegistry, qbftDebug, pubkeys, seenPubkeys, vapiCalls) err = wireCoreWorkflow(ctx, life, conf, cluster, nodeIdx, tcpNode, p2pKey, eth2Cl, diff --git a/app/lifecycle/order.go b/app/lifecycle/order.go index cfa17cca4..19ec71cdd 100644 --- a/app/lifecycle/order.go +++ b/app/lifecycle/order.go @@ -18,6 +18,7 @@ const ( StartAggSigDB StartRelay StartMonitoringAPI + StartDebugAPI StartValidatorAPI StartP2PPing StartP2PRouters @@ -42,5 +43,6 @@ const ( StopP2PPeerDB StopP2PTCPNode StopP2PUDPNode + StopDebugAPI StopMonitoringAPI ) diff --git a/app/lifecycle/orderstart_string.go b/app/lifecycle/orderstart_string.go index 85cb90ddd..7cb254816 100644 --- a/app/lifecycle/orderstart_string.go +++ b/app/lifecycle/orderstart_string.go @@ -15,21 +15,22 @@ func _() { _ = x[StartAggSigDB-2] _ = x[StartRelay-3] _ = x[StartMonitoringAPI-4] - _ = x[StartValidatorAPI-5] - _ = x[StartP2PPing-6] - _ = x[StartP2PRouters-7] - _ = x[StartForceDirectConns-8] - _ = x[StartP2PConsensus-9] - _ = x[StartSimulator-10] - _ = x[StartScheduler-11] - _ = x[StartP2PEventCollector-12] - _ = x[StartPeerInfo-13] - _ = x[StartParSigDB-14] + _ = x[StartDebugAPI-5] + _ = x[StartValidatorAPI-6] + _ = x[StartP2PPing-7] + _ = x[StartP2PRouters-8] + _ = x[StartForceDirectConns-9] + _ = x[StartP2PConsensus-10] + _ = x[StartSimulator-11] + _ = x[StartScheduler-12] + _ = x[StartP2PEventCollector-13] + _ = x[StartPeerInfo-14] + _ = x[StartParSigDB-15] } -const _OrderStart_name = "TrackerPrivkeyLockAggSigDBRelayMonitoringAPIValidatorAPIP2PPingP2PRoutersForceDirectConnsP2PConsensusSimulatorSchedulerP2PEventCollectorPeerInfoParSigDB" +const _OrderStart_name = "TrackerPrivkeyLockAggSigDBRelayMonitoringAPIDebugAPIValidatorAPIP2PPingP2PRoutersForceDirectConnsP2PConsensusSimulatorSchedulerP2PEventCollectorPeerInfoParSigDB" -var _OrderStart_index = [...]uint8{0, 7, 18, 26, 31, 44, 56, 63, 73, 89, 101, 110, 119, 136, 144, 152} +var _OrderStart_index = [...]uint8{0, 7, 18, 26, 31, 44, 52, 64, 71, 81, 97, 109, 118, 127, 144, 152, 160} func (i OrderStart) String() string { if i < 0 || i >= OrderStart(len(_OrderStart_index)-1) { diff --git a/app/lifecycle/orderstop_string.go b/app/lifecycle/orderstop_string.go index 718353193..19c4cee86 100644 --- a/app/lifecycle/orderstop_string.go +++ b/app/lifecycle/orderstop_string.go @@ -20,12 +20,13 @@ func _() { _ = x[StopP2PPeerDB-7] _ = x[StopP2PTCPNode-8] _ = x[StopP2PUDPNode-9] - _ = x[StopMonitoringAPI-10] + _ = x[StopDebugAPI-10] + _ = x[StopMonitoringAPI-11] } -const _OrderStop_name = "SchedulerPrivkeyLockRetryerDutyDBBeaconMockValidatorAPITracingP2PPeerDBP2PTCPNodeP2PUDPNodeMonitoringAPI" +const _OrderStop_name = "SchedulerPrivkeyLockRetryerDutyDBBeaconMockValidatorAPITracingP2PPeerDBP2PTCPNodeP2PUDPNodeDebugAPIMonitoringAPI" -var _OrderStop_index = [...]uint8{0, 9, 20, 27, 33, 43, 55, 62, 71, 81, 91, 104} +var _OrderStop_index = [...]uint8{0, 9, 20, 27, 33, 43, 55, 62, 71, 81, 91, 99, 112} func (i OrderStop) String() string { if i < 0 || i >= OrderStop(len(_OrderStop_index)-1) { diff --git a/app/monitoringapi.go b/app/monitoringapi.go index 6890674fd..4a49873a7 100644 --- a/app/monitoringapi.go +++ b/app/monitoringapi.go @@ -44,7 +44,7 @@ var ( // wireMonitoringAPI constructs the monitoring API and registers it with the life cycle manager. // It serves prometheus metrics, pprof profiling and the runtime enr. -func wireMonitoringAPI(ctx context.Context, life *lifecycle.Manager, addr string, +func wireMonitoringAPI(ctx context.Context, life *lifecycle.Manager, promAddr, debugAddr string, tcpNode host.Host, eth2Cl eth2wrap.Client, peerIDs []peer.ID, registry *prometheus.Registry, qbftDebug http.Handler, pubkeys []core.PubKey, seenPubkeys <-chan core.PubKey, vapiCalls <-chan struct{}, @@ -76,18 +76,8 @@ func wireMonitoringAPI(ctx context.Context, life *lifecycle.Manager, addr string writeResponse(w, http.StatusOK, "ok") }) - // Serve sniffed qbft instances messages in gzipped protobuf format. - mux.Handle("/debug/qbft", qbftDebug) - - // Copied from net/http/pprof/pprof.go - mux.HandleFunc("/debug/pprof/", pprof.Index) - mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) - mux.HandleFunc("/debug/pprof/profile", pprof.Profile) - mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) - mux.HandleFunc("/debug/pprof/trace", pprof.Trace) - server := &http.Server{ - Addr: addr, + Addr: promAddr, Handler: mux, ReadHeaderTimeout: time.Second, } @@ -99,6 +89,29 @@ func wireMonitoringAPI(ctx context.Context, life *lifecycle.Manager, addr string QuorumPeers: cluster.Threshold(len(peerIDs)), }, registry) + if debugAddr != "" { + debugMux := http.NewServeMux() + + // Serve sniffed qbft instances messages in gzipped protobuf format. + debugMux.Handle("/debug/qbft", qbftDebug) + + // Copied from net/http/pprof/pprof.go + debugMux.HandleFunc("/debug/pprof/", pprof.Index) + debugMux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) + debugMux.HandleFunc("/debug/pprof/profile", pprof.Profile) + debugMux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) + debugMux.HandleFunc("/debug/pprof/trace", pprof.Trace) + + debugServer := &http.Server{ + Addr: debugAddr, + Handler: debugMux, + ReadHeaderTimeout: time.Second, + } + + life.RegisterStart(lifecycle.AsyncBackground, lifecycle.StartDebugAPI, httpServeHook(debugServer.ListenAndServe)) + life.RegisterStop(lifecycle.StopDebugAPI, lifecycle.HookFunc(debugServer.Shutdown)) + } + life.RegisterStart(lifecycle.AsyncBackground, lifecycle.StartMonitoringAPI, httpServeHook(server.ListenAndServe)) life.RegisterStart(lifecycle.AsyncBackground, lifecycle.StartMonitoringAPI, lifecycle.HookFuncCtx(checker.Run)) life.RegisterStop(lifecycle.StopMonitoringAPI, lifecycle.HookFunc(server.Shutdown)) diff --git a/cmd/cmd_internal_test.go b/cmd/cmd_internal_test.go index e855c8b2a..a5dbe7a7a 100644 --- a/cmd/cmd_internal_test.go +++ b/cmd/cmd_internal_test.go @@ -61,10 +61,8 @@ func TestCmdFlags(t *testing.T) { LokiService: "charon", }, P2P: p2p.Config{ - Relays: []string{"https://0.relay.obol.tech"}, - TCPAddrs: nil, - Allowlist: "", - Denylist: "", + Relays: []string{"https://0.relay.obol.tech", "https://1.relay.obol.tech"}, + TCPAddrs: nil, }, Feature: featureset.Config{ MinStatus: "stable", @@ -89,10 +87,8 @@ func TestCmdFlags(t *testing.T) { Args: slice("create", "enr"), Datadir: ".charon", P2PConfig: &p2p.Config{ - Relays: []string{"https://0.relay.obol.tech"}, - TCPAddrs: nil, - Allowlist: "", - Denylist: "", + Relays: []string{"https://0.relay.obol.tech"}, + TCPAddrs: nil, }, }, { @@ -114,10 +110,8 @@ func TestCmdFlags(t *testing.T) { LokiService: "charon", }, P2P: p2p.Config{ - Relays: []string{"https://0.relay.obol.tech"}, - TCPAddrs: nil, - Allowlist: "", - Denylist: "", + Relays: []string{"https://0.relay.obol.tech", "https://1.relay.obol.tech"}, + TCPAddrs: nil, }, Feature: featureset.Config{ MinStatus: "stable", diff --git a/cmd/createcluster.go b/cmd/createcluster.go index 3171f9a40..01bcb1c2c 100644 --- a/cmd/createcluster.go +++ b/cmd/createcluster.go @@ -132,7 +132,7 @@ func runCreateCluster(ctx context.Context, w io.Writer, conf clusterConfig) erro conf.Network = eth2util.Goerli.Name } - if err = validateCreateConfig(conf); err != nil { + if err = validateCreateConfig(ctx, conf); err != nil { return err } @@ -301,7 +301,7 @@ func runCreateCluster(ctx context.Context, w io.Writer, conf clusterConfig) erro } // validateCreateConfig returns an error if any of the provided config parameters are invalid. -func validateCreateConfig(conf clusterConfig) error { +func validateCreateConfig(ctx context.Context, conf clusterConfig) error { if conf.NumNodes == 0 && conf.DefFile == "" { // if there's a definition file, infer this value from it later return errors.New("missing --nodes flag") } @@ -320,6 +320,17 @@ func validateCreateConfig(conf clusterConfig) error { return errors.New("number of --keymanager-addresses do not match --keymanager-auth-tokens. Please fix configuration flags") } + for _, addr := range conf.KeymanagerAddrs { + keymanagerURL, err := url.Parse(addr) + if err != nil { + return errors.Wrap(err, "failed to parse keymanager addr", z.Str("addr", addr)) + } + + if keymanagerURL.Scheme != "https" { + log.Warn(ctx, "Keymanager URL does not use https protocol", nil, z.Str("addr", addr)) + } + } + if conf.SplitKeys { if conf.NumDVs != 0 { return errors.New("can't specify --num-validators with --split-existing-keys. Please fix configuration flags") diff --git a/cmd/debug_tools.go b/cmd/debug_tools.go new file mode 100644 index 000000000..84109673e --- /dev/null +++ b/cmd/debug_tools.go @@ -0,0 +1,13 @@ +// Copyright © 2022-2023 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package cmd + +import ( + "github.com/spf13/cobra" +) + +// bindDebugMonitoringFlags binds Prometheus monitoring and debug address CLI flags. The debug address defaults to an empty address. +func bindDebugMonitoringFlags(cmd *cobra.Command, monitorAddr, debugAddr *string, defaultMonitorAddr string) { + cmd.Flags().StringVar(monitorAddr, "monitoring-address", defaultMonitorAddr, "Listening address (ip and port) for the monitoring API (prometheus).") + cmd.Flags().StringVar(debugAddr, "debug-address", "", "Listening address (ip and port) for the pprof and QBFT debug API. It is not enabled by default.") +} diff --git a/cmd/debug_tools_internal_test.go b/cmd/debug_tools_internal_test.go new file mode 100644 index 000000000..d214828d4 --- /dev/null +++ b/cmd/debug_tools_internal_test.go @@ -0,0 +1,110 @@ +// Copyright © 2022-2023 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package cmd + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" + + "github.com/obolnetwork/charon/app" +) + +func genTestCmd(t *testing.T, f func(config app.Config)) *cobra.Command { + t.Helper() + + var conf app.Config + + cmd := &cobra.Command{ + Use: "test", + Short: "test", + } + + cmd.Run = func(cmd *cobra.Command, args []string) { + f(conf) + } + + bindDebugMonitoringFlags(cmd, &conf.MonitoringAddr, &conf.DebugAddr, "") + + return cmd +} + +func Test_bindDebugMonitoringFlags(t *testing.T) { + cmd := &cobra.Command{ + Use: "testcmd", + } + + t.Run("both present", func(t *testing.T) { + var ( + mAddr = "127.0.0.1:9999" + dAddr = "127.0.0.1:8888" + ) + + cmd.ResetCommands() + + testCmd := genTestCmd(t, func(config app.Config) { + require.Equal(t, mAddr, config.MonitoringAddr) + require.Equal(t, dAddr, config.DebugAddr) + }) + + cmd.AddCommand(testCmd) + + cmd.SetArgs([]string{ + "test", + "--monitoring-address", + mAddr, + "--debug-address", + dAddr, + }) + + require.NoError(t, cmd.Execute()) + }) + + t.Run("only monitor", func(t *testing.T) { + var ( + mAddr = "127.0.0.1:9999" + dAddr = "" + ) + cmd.ResetCommands() + + testCmd := genTestCmd(t, func(config app.Config) { + require.Equal(t, mAddr, config.MonitoringAddr) + require.Equal(t, dAddr, config.DebugAddr) + }) + + cmd.AddCommand(testCmd) + + cmd.SetArgs([]string{ + "test", + "--monitoring-address", + mAddr, + }) + + require.NoError(t, cmd.Execute()) + }) + + t.Run("only debug", func(t *testing.T) { + var ( + mAddr = "" + dAddr = "127.0.0.1:8888" + ) + + cmd.ResetCommands() + + testCmd := genTestCmd(t, func(config app.Config) { + require.Equal(t, mAddr, config.MonitoringAddr) + require.Equal(t, dAddr, config.DebugAddr) + }) + + cmd.AddCommand(testCmd) + + cmd.SetArgs([]string{ + "test", + "--debug-address", + dAddr, + }) + + require.NoError(t, cmd.Execute()) + }) +} diff --git a/cmd/relay.go b/cmd/relay.go index de90682b9..e86d6e393 100644 --- a/cmd/relay.go +++ b/cmd/relay.go @@ -34,6 +34,7 @@ func newRelayCmd(runFunc func(context.Context, relay.Config) error) *cobra.Comma bindDataDirFlag(cmd.Flags(), &config.DataDir) bindRelayFlag(cmd, &config) + bindDebugMonitoringFlags(cmd, &config.MonitoringAddr, &config.DebugAddr, "") bindP2PFlags(cmd, &config.P2PConfig) bindLogFlags(cmd.Flags(), &config.LogConfig) bindLokiFlags(cmd.Flags(), &config.LogConfig) @@ -43,7 +44,6 @@ func newRelayCmd(runFunc func(context.Context, relay.Config) error) *cobra.Comma func bindRelayFlag(cmd *cobra.Command, config *relay.Config) { cmd.Flags().StringVar(&config.HTTPAddr, "http-address", "127.0.0.1:3640", "Listening address (ip and port) for the relay http server serving runtime ENR.") - cmd.Flags().StringVar(&config.MonitoringAddr, "monitoring-address", "127.0.0.1:3620", "Listening address (ip and port) for the prometheus and pprof monitoring http server.") cmd.Flags().BoolVar(&config.AutoP2PKey, "auto-p2pkey", true, "Automatically create a p2pkey (secp256k1 private key used for p2p authentication and ENR) if none found in data directory.") cmd.Flags().StringVar(&config.RelayLogLevel, "p2p-relay-loglevel", "", "Libp2p circuit relay log level. E.g., debug, info, warn, error.") diff --git a/cmd/relay/relay.go b/cmd/relay/relay.go index 4b8d16b81..d9f702464 100644 --- a/cmd/relay/relay.go +++ b/cmd/relay/relay.go @@ -34,6 +34,7 @@ type Config struct { DataDir string HTTPAddr string MonitoringAddr string + DebugAddr string P2PConfig p2p.Config LogConfig log.Config AutoP2PKey bool @@ -86,7 +87,7 @@ func Run(ctx context.Context, config Config) error { } // Start serving HTTP: ENR and monitoring. - serverErr := make(chan error, 2) // Buffer for 2 servers. + serverErr := make(chan error, 3) // Buffer for 3 servers. go func() { if config.HTTPAddr == "" { return @@ -99,27 +100,37 @@ func Run(ctx context.Context, config Config) error { serverErr <- server.ListenAndServe() }() - go func() { - if config.MonitoringAddr == "" { - return - } + if config.MonitoringAddr != "" { + go func() { + // Serve prometheus metrics wrapped with relay identifiers. + mux := http.NewServeMux() + mux.Handle("/metrics", promhttp.InstrumentMetricHandler( + promRegistry, promhttp.HandlerFor(promRegistry, promhttp.HandlerOpts{}), + )) + + log.Info(ctx, "Monitoring server started", z.Str("address", config.MonitoringAddr)) + server := http.Server{Addr: config.MonitoringAddr, Handler: mux, ReadHeaderTimeout: time.Second} + serverErr <- server.ListenAndServe() + }() + } - // Serve prometheus metrics wrapped with relay identifiers. - mux := http.NewServeMux() - mux.Handle("/metrics", promhttp.InstrumentMetricHandler( - promRegistry, promhttp.HandlerFor(promRegistry, promhttp.HandlerOpts{}), - )) - - // Copied from net/http/pprof/pprof.go - mux.HandleFunc("/debug/pprof/", pprof.Index) - mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) - mux.HandleFunc("/debug/pprof/profile", pprof.Profile) - mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) - mux.HandleFunc("/debug/pprof/trace", pprof.Trace) - - server := http.Server{Addr: config.MonitoringAddr, Handler: mux, ReadHeaderTimeout: time.Second} - serverErr <- server.ListenAndServe() - }() + if config.DebugAddr != "" { + go func() { + debugMux := http.NewServeMux() + + // Copied from net/http/pprof/pprof.go + debugMux.HandleFunc("/debug/pprof/", pprof.Index) + debugMux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) + debugMux.HandleFunc("/debug/pprof/profile", pprof.Profile) + debugMux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) + debugMux.HandleFunc("/debug/pprof/trace", pprof.Trace) + + log.Info(ctx, "Debug server started", z.Str("address", config.DebugAddr)) + + server := http.Server{Addr: config.DebugAddr, Handler: debugMux, ReadHeaderTimeout: time.Second} + serverErr <- server.ListenAndServe() + }() + } log.Info(ctx, "Relay started", z.Str("peer_name", p2p.PeerName(tcpNode.ID())), diff --git a/cmd/run.go b/cmd/run.go index a61c470c2..2383346f4 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -44,6 +44,7 @@ func newRunCmd(runFunc func(context.Context, app.Config) error, unsafe bool) *co bindPrivKeyFlag(cmd, &conf.PrivKeyFile, &conf.PrivKeyLocking) bindRunFlags(cmd, &conf) + bindDebugMonitoringFlags(cmd, &conf.MonitoringAddr, &conf.DebugAddr, "127.0.0.1:3620") bindNoVerifyFlag(cmd.Flags(), &conf.NoVerify) bindP2PFlags(cmd, &conf.P2P) bindLogFlags(cmd.Flags(), &conf.Log) @@ -70,7 +71,6 @@ func bindRunFlags(cmd *cobra.Command, config *app.Config) { cmd.Flags().StringVar(&config.ManifestFile, "manifest-file", ".charon/cluster-manifest.pb", "The path to the cluster manifest file. If both cluster manifest and cluster lock files are provided, the cluster manifest file takes precedence.") cmd.Flags().StringSliceVar(&config.BeaconNodeAddrs, "beacon-node-endpoints", nil, "Comma separated list of one or more beacon node endpoint URLs.") cmd.Flags().StringVar(&config.ValidatorAPIAddr, "validator-api-address", "127.0.0.1:3600", "Listening address (ip and port) for validator-facing traffic proxying the beacon-node API.") - cmd.Flags().StringVar(&config.MonitoringAddr, "monitoring-address", "127.0.0.1:3620", "Listening address (ip and port) for the monitoring API (prometheus, pprof).") cmd.Flags().StringVar(&config.JaegerAddr, "jaeger-address", "", "Listening address for jaeger tracing.") cmd.Flags().StringVar(&config.JaegerService, "jaeger-service", "charon", "Service name used for jaeger tracing.") cmd.Flags().BoolVar(&config.SimnetBMock, "simnet-beacon-mock", false, "Enables an internal mock beacon node for running a simnet.") @@ -112,12 +112,10 @@ func bindLogFlags(flags *pflag.FlagSet, config *log.Config) { } func bindP2PFlags(cmd *cobra.Command, config *p2p.Config) { - cmd.Flags().StringSliceVar(&config.Relays, "p2p-relays", []string{"https://0.relay.obol.tech"}, "Comma-separated list of libp2p relay URLs or multiaddrs.") + cmd.Flags().StringSliceVar(&config.Relays, "p2p-relays", []string{"https://0.relay.obol.tech", "https://1.relay.obol.tech"}, "Comma-separated list of libp2p relay URLs or multiaddrs.") cmd.Flags().StringVar(&config.ExternalIP, "p2p-external-ip", "", "The IP address advertised by libp2p. This may be used to advertise an external IP.") cmd.Flags().StringVar(&config.ExternalHost, "p2p-external-hostname", "", "The DNS hostname advertised by libp2p. This may be used to advertise an external DNS.") cmd.Flags().StringSliceVar(&config.TCPAddrs, "p2p-tcp-address", nil, "Comma-separated list of listening TCP addresses (ip and port) for libP2P traffic. Empty default doesn't bind to local port therefore only supports outgoing connections.") - cmd.Flags().StringVar(&config.Allowlist, "p2p-allowlist", "", "Comma-separated list of CIDR subnets for allowing only certain peer connections. Example: 192.168.0.0/16 would permit connections to peers on your local network only. The default is to accept all connections.") - cmd.Flags().StringVar(&config.Denylist, "p2p-denylist", "", "Comma-separated list of CIDR subnets for disallowing certain peer connections. Example: 192.168.0.0/16 would disallow connections to peers on your local network. The default is to accept all connections.") cmd.Flags().BoolVar(&config.DisableReuseport, "p2p-disable-reuseport", false, "Disables TCP port reuse for outgoing libp2p connections.") wrapPreRunE(cmd, func(cmd *cobra.Command, args []string) error { diff --git a/core/dutydb/memory.go b/core/dutydb/memory.go index f7f78b37b..cc9905f7b 100644 --- a/core/dutydb/memory.go +++ b/core/dutydb/memory.go @@ -127,8 +127,8 @@ func (db *MemDB) Store(_ context.Context, duty core.Duty, unsignedSet core.Unsig if err != nil { return err } - db.resolveContribQueriesUnsafe() } + db.resolveContribQueriesUnsafe() default: return errors.New("unsupported duty type", z.Str("type", duty.Type.String())) } diff --git a/core/proto.go b/core/proto.go index 1f2d19dd6..f4d533bd1 100644 --- a/core/proto.go +++ b/core/proto.go @@ -5,6 +5,7 @@ package core import ( "bytes" "encoding/json" + "fmt" "testing" ssz "github.com/ferranbt/fastssz" @@ -44,10 +45,15 @@ func DutyFromProto(duty *pbv1.Duty) Duty { } // ParSignedDataFromProto returns the data from a protobuf. -func ParSignedDataFromProto(typ DutyType, data *pbv1.ParSignedData) (ParSignedData, error) { - // TODO(corver): This can panic due to json unmarshalling unexpected data. - // For now, it is a good way to catch compatibility issues. But we should - // recover panics and return an error before launching mainnet. +func ParSignedDataFromProto(typ DutyType, data *pbv1.ParSignedData) (_ ParSignedData, oerr error) { + defer func() { + // This is to respect the technical possibility of unmarshalling to panic. + // However, our protobuf generated types do not have custom marshallers that may panic. + if r := recover(); r != nil { + rowStr := fmt.Sprintf("%v", r) + oerr = errors.Wrap(errors.New(rowStr), "panic recovered") + } + }() if err := protonil.Check(data); err != nil { return ParSignedData{}, errors.Wrap(err, "invalid partial signed proto") @@ -212,7 +218,7 @@ func UnsignedDataSetFromProto(typ DutyType, set *pbv1.UnsignedDataSet) (Unsigned resp := make(UnsignedDataSet) for pubkey, data := range set.Set { var err error - resp[PubKey(pubkey)], err = UnmarshalUnsignedData(typ, data) + resp[PubKey(pubkey)], err = unmarshalUnsignedData(typ, data) if err != nil { return nil, err } diff --git a/core/tracing.go b/core/tracing.go index dbabb0490..fbf713a05 100644 --- a/core/tracing.go +++ b/core/tracing.go @@ -6,7 +6,7 @@ import ( "context" "fmt" "hash/fnv" - "math" + "strconv" "strings" eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0" @@ -30,7 +30,8 @@ func StartDutyTrace(ctx context.Context, duty Duty, spanName string, opts ...tra ctx, outerSpan = tracer.Start(tracer.RootedCtx(ctx, traceID), fmt.Sprintf("core/duty.%s", strings.Title(duty.Type.String()))) ctx, innerSpan = tracer.Start(ctx, spanName, opts...) - outerSpan.SetAttributes(attribute.Int64("slot", safeInt64(duty.Slot))) + slotStr := strconv.FormatUint(duty.Slot, 10) + outerSpan.SetAttributes(attribute.String("slot", slotStr)) return ctx, withEndSpan{ Span: innerSpan, @@ -134,13 +135,3 @@ func WithTracing() WireOption { } } } - -// safeInt64 converts the provided uint64 value to an int64 integer. -// It panics if the provided value can't fit in an int64. -func safeInt64(value uint64) int64 { - if value <= math.MaxInt64 { - return int64(value) - } - - panic("integer overflow") -} diff --git a/core/unsigneddata.go b/core/unsigneddata.go index fdb995038..9c7459dfe 100644 --- a/core/unsigneddata.go +++ b/core/unsigneddata.go @@ -420,9 +420,8 @@ func (s *SyncContribution) UnmarshalSSZ(b []byte) error { return s.SyncCommitteeContribution.UnmarshalSSZ(b) } -// UnmarshalUnsignedData returns an instantiated unsigned data based on the duty type. -// TODO(corver): Unexport once leadercast is removed or uses protobufs. -func UnmarshalUnsignedData(typ DutyType, data []byte) (UnsignedData, error) { +// unmarshalUnsignedData returns an instantiated unsigned data based on the duty type. +func unmarshalUnsignedData(typ DutyType, data []byte) (UnsignedData, error) { switch typ { case DutyAttester: var resp AttestationData diff --git a/core/validatorapi/router_internal_test.go b/core/validatorapi/router_internal_test.go index 4923aa991..4cd5af415 100644 --- a/core/validatorapi/router_internal_test.go +++ b/core/validatorapi/router_internal_test.go @@ -855,8 +855,6 @@ func TestRouter(t *testing.T) { } res, err := cl.Proposal(ctx, opts) require.Error(t, err) - // TODO(xenowits): Fix this test prior to merging. Debug why res is nil. - // require.ErrorContains(t, err, "not implemented") require.Nil(t, res) } @@ -882,7 +880,6 @@ func TestRouter(t *testing.T) { } res, err := cl.BlindedProposal(ctx, opts) require.Error(t, err) - // TODO(xenowits): Fix this test prior to merging. Debug why res is nil. require.Nil(t, res) } diff --git a/dkg/bcast/client.go b/dkg/bcast/client.go index 1a3230c62..b27ba4b91 100644 --- a/dkg/bcast/client.go +++ b/dkg/bcast/client.go @@ -60,8 +60,8 @@ func (c *client) Broadcast(ctx context.Context, msgID string, msg proto.Message) // Send hash to all peers to sign. sigReq := &pb.BCastSigRequest{ - Id: msgID, - Hash: hash, + Id: msgID, + Message: anyMsg, } fork, join, cancel := forkjoin.New(ctx, func(ctx context.Context, pID peer.ID) (*pb.BCastSigResponse, error) { @@ -79,7 +79,7 @@ func (c *client) Broadcast(ctx context.Context, msgID string, msg proto.Message) for i, pID := range c.peers { if c.tcpNode.ID() == pID { // Sign self locally. - sig, err := c.signFunc(msgID, sigReq.Hash) + sig, err := c.signFunc(msgID, hash) if err != nil { return errors.Wrap(err, "sign hash") } diff --git a/dkg/bcast/helpers.go b/dkg/bcast/helpers.go index 6c08a5b1f..de0b15ae2 100644 --- a/dkg/bcast/helpers.go +++ b/dkg/bcast/helpers.go @@ -25,6 +25,9 @@ type hashFunc func(*anypb.Any) ([]byte, error) // Callback is a function that is called when a reliably-broadcast message was successfully received. type Callback func(ctx context.Context, peerID peer.ID, msgID string, msg proto.Message) error +// CheckMessage is a function that ensures that msg is of the type that a given message ID should handle. +type CheckMessage func(ctx context.Context, peerID peer.ID, msgAny *anypb.Any) error + // signFunc is a function that signs a hash. type signFunc func(msgID string, hash []byte) ([]byte, error) diff --git a/dkg/bcast/impl.go b/dkg/bcast/impl.go index 5bf44b39e..e130cdbec 100644 --- a/dkg/bcast/impl.go +++ b/dkg/bcast/impl.go @@ -30,13 +30,13 @@ type Component struct { broadcastFunc BroadcastFunc } -// RegisterCallback adds a callback for msgID. -func (c *Component) RegisterCallback(msgID string, callback Callback) { +// RegisterMessageIDFuncs adds a callback and a check message function for msgID. +func (c *Component) RegisterMessageIDFuncs(msgID string, callback Callback, checkMessage CheckMessage) { c.allowedMsgIDsMutex.Lock() defer c.allowedMsgIDsMutex.Unlock() c.allowedMsgIDs[msgID] = struct{}{} - c.srv.registerCallback(msgID, callback) + c.srv.registerMessageIDFuncs(msgID, callback, checkMessage) } // msgIDAllowed returns true if msgID is an allowed message id. @@ -68,7 +68,7 @@ func New(tcpNode host.Host, peers []peer.ID, secret *k1.PrivateKey) *Component { cl := newClient(tcpNode, peers, p2p.SendReceive, p2p.Send, hashAny, signFunc, verifyFunc) c.broadcastFunc = cl.Broadcast - c.srv = newServer(tcpNode, signFunc, verifyFunc) + c.srv = newServer(tcpNode, signFunc, hashAny, verifyFunc) return &c } diff --git a/dkg/bcast/impl_test.go b/dkg/bcast/impl_test.go index c1dba09de..b0ade23e2 100644 --- a/dkg/bcast/impl_test.go +++ b/dkg/bcast/impl_test.go @@ -12,8 +12,10 @@ import ( "github.com/libp2p/go-libp2p/core/peerstore" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/timestamppb" + "github.com/obolnetwork/charon/app/errors" "github.com/obolnetwork/charon/dkg/bcast" "github.com/obolnetwork/charon/testutil" ) @@ -65,15 +67,25 @@ func TestBCast(t *testing.T) { // Create broadcasters for i := 0; i < n; i++ { i := i - callback := func(ctx context.Context, peerID peer.ID, msgID string, msg proto.Message) error { + callback := func(_ context.Context, peerID peer.ID, msgID string, msg proto.Message) error { results <- result{Source: peerID, MsgID: msgID, Msg: msg, Target: peers[i]} return nil } + checkMessage := func(_ context.Context, _ peer.ID, msgAny *anypb.Any) error { + var ts timestamppb.Timestamp + err := msgAny.UnmarshalTo(&ts) + if err != nil { + return errors.Wrap(err, "anypb error") + } + + return nil + } + bcastFunc := bcast.New(tcpNodes[i], peers, secrets[i]) - bcastFunc.RegisterCallback(msgID1, callback) - bcastFunc.RegisterCallback(msgID2, callback) + bcastFunc.RegisterMessageIDFuncs(msgID1, callback, checkMessage) + bcastFunc.RegisterMessageIDFuncs(msgID2, callback, checkMessage) bcasts = append(bcasts, bcastFunc.Broadcast) } diff --git a/dkg/bcast/server.go b/dkg/bcast/server.go index 6837442e4..79cbd9fca 100644 --- a/dkg/bcast/server.go +++ b/dkg/bcast/server.go @@ -18,11 +18,12 @@ import ( ) // newServer creates a new reliable-broadcast server. -func newServer(tcpNode host.Host, signFunc signFunc, verifyFunc verifyFunc) *server { +func newServer(tcpNode host.Host, signFunc signFunc, hashFunc hashFunc, verifyFunc verifyFunc) *server { s := &server{ - callbacks: map[string]Callback{}, + msgIDFuncs: map[string]messageIDFuncs{}, signFunc: signFunc, verifyFunc: verifyFunc, + hashFunc: hashFunc, dedup: make(map[dedupKey][]byte), } @@ -48,11 +49,17 @@ type dedupKey struct { MsgID string } +type messageIDFuncs struct { + callback Callback + checkMessage CheckMessage +} + // server is a reliable-broadcast server. type server struct { - callbacksMutex sync.Mutex - callbacks map[string]Callback + msgIDFuncsMutex sync.Mutex + msgIDFuncs map[string]messageIDFuncs + hashFunc hashFunc signFunc signFunc verifyFunc verifyFunc @@ -60,20 +67,23 @@ type server struct { dedup map[dedupKey][]byte // map[dedupKey]hash } -func (s *server) getCallback(msgID string) (Callback, bool) { - s.callbacksMutex.Lock() - defer s.callbacksMutex.Unlock() +func (s *server) getMessageIDFunc(msgID string) (messageIDFuncs, bool) { + s.msgIDFuncsMutex.Lock() + defer s.msgIDFuncsMutex.Unlock() - fn, found := s.callbacks[msgID] + fn, found := s.msgIDFuncs[msgID] return fn, found } -func (s *server) registerCallback(msgID string, cb Callback) { - s.callbacksMutex.Lock() - defer s.callbacksMutex.Unlock() +func (s *server) registerMessageIDFuncs(msgID string, cb Callback, cm CheckMessage) { + s.msgIDFuncsMutex.Lock() + defer s.msgIDFuncsMutex.Unlock() - s.callbacks[msgID] = cb + s.msgIDFuncs[msgID] = messageIDFuncs{ + callback: cb, + checkMessage: cm, + } } func (s *server) dedupHash(pID peer.ID, msgID string, hash []byte) error { @@ -92,18 +102,32 @@ func (s *server) dedupHash(pID peer.ID, msgID string, hash []byte) error { return nil } -func (s *server) handleSigRequest(_ context.Context, pID peer.ID, m proto.Message) (proto.Message, bool, error) { +func (s *server) handleSigRequest(ctx context.Context, pID peer.ID, m proto.Message) (proto.Message, bool, error) { req, ok := m.(*pb.BCastSigRequest) if !ok { return nil, false, errors.New("invalid message type") } + fn, found := s.getMessageIDFunc(req.Id) + if !found { + return nil, false, errors.New("unknown message id", z.Str("message_id", req.Id)) + } + + if err := fn.checkMessage(ctx, pID, req.Message); err != nil { + return nil, false, errors.Wrap(err, "signature request message check") + } + + reqMessageHash, err := s.hashFunc(req.Message) + if err != nil { + return nil, false, errors.Wrap(err, "hash any") + } + // Only sign once per peer and message ID. - if err := s.dedupHash(pID, req.Id, req.Hash); err != nil { + if err := s.dedupHash(pID, req.Id, reqMessageHash); err != nil { return nil, false, errors.Wrap(err, "dedup") } - sig, err := s.signFunc(req.Id, req.Hash) + sig, err := s.signFunc(req.Id, reqMessageHash) if err != nil { return nil, false, errors.Wrap(err, "sign hash") } @@ -126,12 +150,12 @@ func (s *server) handleMessage(ctx context.Context, pID peer.ID, m proto.Message return nil, false, errors.Wrap(err, "unmarshal any") } - fn, found := s.getCallback(msg.Id) + fn, found := s.getMessageIDFunc(msg.Id) if !found { return nil, false, errors.New("unknown message id", z.Str("message_id", msg.Id)) } - if err := fn(ctx, pID, msg.Id, inner); err != nil { + if err := fn.callback(ctx, pID, msg.Id, inner); err != nil { return nil, false, errors.Wrap(err, "callback") } diff --git a/dkg/disk.go b/dkg/disk.go index 532478eb6..ffa7d655a 100644 --- a/dkg/disk.go +++ b/dkg/disk.go @@ -13,6 +13,7 @@ import ( "os" "path" "path/filepath" + "strings" eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0" @@ -34,8 +35,14 @@ func loadDefinition(ctx context.Context, conf Config) (cluster.Definition, error // Fetch definition from URI or disk + parsedURL, err := url.ParseRequestURI(conf.DefFile) + var def cluster.Definition - if validURI(conf.DefFile) { + if err == nil && parsedURL.Host != "" { + if !strings.HasPrefix(parsedURL.Scheme, "https") { + log.Warn(ctx, "Definition file URL does not use https protocol", nil, z.Str("addr", conf.DefFile)) + } + var err error def, err = cluster.FetchDefinition(ctx, conf.DefFile) if err != nil { @@ -247,13 +254,6 @@ func checkWrites(dataDir string) error { return nil } -// validURI returns true if the input string is a valid HTTP/HTTPS URI. -func validURI(str string) bool { - u, err := url.Parse(str) - - return err == nil && (u.Scheme == "http" || u.Scheme == "https") && u.Host != "" -} - // randomHex64 returns a random 64 character hex string. It uses crypto/rand. func randomHex64() (string, error) { b := make([]byte, 32) diff --git a/dkg/dkg.go b/dkg/dkg.go index 53c715d95..fa8dbdcd7 100644 --- a/dkg/dkg.go +++ b/dkg/dkg.go @@ -7,6 +7,7 @@ import ( "context" "encoding/hex" "fmt" + "net/url" "time" eth2api "github.com/attestantio/go-eth2-client/api" @@ -112,7 +113,7 @@ func Run(ctx context.Context, conf Config) (err error) { return errors.New("only v1.6.0 and v1.7.0 cluster definition version supported") } - if err := validateKeymanagerFlags(conf.KeymanagerAddr, conf.KeymanagerAuthToken); err != nil { + if err := validateKeymanagerFlags(ctx, conf.KeymanagerAddr, conf.KeymanagerAuthToken); err != nil { return err } @@ -203,7 +204,10 @@ func Run(ctx context.Context, conf Config) (err error) { caster := bcast.New(tcpNode, peerIds, key) // register bcast callbacks for frostp2p - tp := newFrostP2P(tcpNode, peerMap, caster, def.Threshold, def.NumValidators) + tp, err := newFrostP2P(tcpNode, peerMap, caster, def.Threshold, def.NumValidators) + if err != nil { + return errors.Wrap(err, "frost error") + } // register bcast callbacks for lock hash k1 signature handler nodeSigCaster := newNodeSigBcast(peers, nodeIdx, caster) @@ -477,8 +481,9 @@ func startSyncProtocol(ctx context.Context, tcpNode host.Host, key *k1.PrivateKe break } - // Sleep for 100ms to let clients connect with each other. - time.Sleep(time.Millisecond * 100) + // Sleep for 250ms to let clients connect with each other. + // Must be at least two times greater than the sync messages period specified in client.go NewClient(). + time.Sleep(time.Millisecond * 250) } // Disable reconnecting clients to other peer's server once all clients are connected. @@ -1039,7 +1044,7 @@ func writeLockToAPI(ctx context.Context, publishAddr string, lock cluster.Lock) } // validateKeymanagerFlags returns an error if one keymanager flag is present but the other is not. -func validateKeymanagerFlags(addr, authToken string) error { +func validateKeymanagerFlags(ctx context.Context, addr, authToken string) error { if addr != "" && authToken == "" { return errors.New("--keymanager-address provided but --keymanager-auth-token absent. Please fix configuration flags") } @@ -1047,6 +1052,15 @@ func validateKeymanagerFlags(addr, authToken string) error { return errors.New("--keymanager-auth-token provided but --keymanager-address absent. Please fix configuration flags") } + keymanagerURL, err := url.Parse(addr) + if err != nil { + return errors.Wrap(err, "failed to parse keymanager addr", z.Str("addr", addr)) + } + + if keymanagerURL.Scheme != "https" { + log.Warn(ctx, "Keymanager URL does not use https protocol", nil, z.Str("addr", addr)) + } + return nil } diff --git a/dkg/dkg_internal_test.go b/dkg/dkg_internal_test.go index 2fb68c639..8f17a83a4 100644 --- a/dkg/dkg_internal_test.go +++ b/dkg/dkg_internal_test.go @@ -3,6 +3,7 @@ package dkg import ( + "context" "testing" eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0" @@ -179,13 +180,19 @@ func TestValidateKeymanagerFlags(t *testing.T) { authToken: "keymanager-auth-token", errMsg: "--keymanager-auth-token provided but --keymanager-address absent. Please fix configuration flags", }, + { + name: "Malformed address provided", + addr: "https://keymanager@example.com:-80", + authToken: "keymanager-auth-token", + errMsg: "failed to parse keymanager addr", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateKeymanagerFlags(tt.addr, tt.authToken) + err := validateKeymanagerFlags(context.Background(), tt.addr, tt.authToken) if tt.errMsg != "" { - require.Equal(t, err.Error(), tt.errMsg) + require.ErrorContains(t, err, tt.errMsg) } }) } diff --git a/dkg/dkgpb/v1/bcast.pb.go b/dkg/dkgpb/v1/bcast.pb.go index 32e699e77..61b4bbab3 100644 --- a/dkg/dkgpb/v1/bcast.pb.go +++ b/dkg/dkgpb/v1/bcast.pb.go @@ -26,8 +26,8 @@ type BCastSigRequest struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` - Hash []byte `protobuf:"bytes,2,opt,name=hash,proto3" json:"hash,omitempty"` + Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` + Message *anypb.Any `protobuf:"bytes,3,opt,name=message,proto3" json:"message,omitempty"` } func (x *BCastSigRequest) Reset() { @@ -69,9 +69,9 @@ func (x *BCastSigRequest) GetId() string { return "" } -func (x *BCastSigRequest) GetHash() []byte { +func (x *BCastSigRequest) GetMessage() *anypb.Any { if x != nil { - return x.Hash + return x.Message } return nil } @@ -201,25 +201,27 @@ var file_dkg_dkgpb_v1_bcast_proto_rawDesc = []byte{ 0x63, 0x61, 0x73, 0x74, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x0c, 0x64, 0x6b, 0x67, 0x2e, 0x64, 0x6b, 0x67, 0x70, 0x62, 0x2e, 0x76, 0x31, 0x1a, 0x19, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2f, 0x61, 0x6e, 0x79, 0x2e, 0x70, 0x72, - 0x6f, 0x74, 0x6f, 0x22, 0x35, 0x0a, 0x0f, 0x42, 0x43, 0x61, 0x73, 0x74, 0x53, 0x69, 0x67, 0x52, + 0x6f, 0x74, 0x6f, 0x22, 0x57, 0x0a, 0x0f, 0x42, 0x43, 0x61, 0x73, 0x74, 0x53, 0x69, 0x67, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x0e, 0x0a, 0x02, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, - 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, 0x12, 0x12, 0x0a, 0x04, 0x68, 0x61, 0x73, 0x68, 0x18, 0x02, - 0x20, 0x01, 0x28, 0x0c, 0x52, 0x04, 0x68, 0x61, 0x73, 0x68, 0x22, 0x40, 0x0a, 0x10, 0x42, 0x43, - 0x61, 0x73, 0x74, 0x53, 0x69, 0x67, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x0e, - 0x0a, 0x02, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, 0x12, 0x1c, - 0x0a, 0x09, 0x73, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75, 0x72, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, - 0x0c, 0x52, 0x09, 0x73, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75, 0x72, 0x65, 0x22, 0x6e, 0x0a, 0x0c, - 0x42, 0x43, 0x61, 0x73, 0x74, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x0e, 0x0a, 0x02, - 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, 0x12, 0x2e, 0x0a, 0x07, - 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x14, 0x2e, - 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, - 0x41, 0x6e, 0x79, 0x52, 0x07, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x1e, 0x0a, 0x0a, - 0x73, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75, 0x72, 0x65, 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, 0x0c, - 0x52, 0x0a, 0x73, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75, 0x72, 0x65, 0x73, 0x42, 0x2c, 0x5a, 0x2a, - 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x6f, 0x62, 0x6f, 0x6c, 0x6e, - 0x65, 0x74, 0x77, 0x6f, 0x72, 0x6b, 0x2f, 0x63, 0x68, 0x61, 0x72, 0x6f, 0x6e, 0x2f, 0x64, 0x6b, - 0x67, 0x2f, 0x64, 0x6b, 0x67, 0x70, 0x62, 0x2f, 0x76, 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, - 0x6f, 0x33, + 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, 0x12, 0x2e, 0x0a, 0x07, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, + 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x14, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, + 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x41, 0x6e, 0x79, 0x52, 0x07, 0x6d, + 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x4a, 0x04, 0x08, 0x02, 0x10, 0x03, 0x22, 0x40, 0x0a, 0x10, + 0x42, 0x43, 0x61, 0x73, 0x74, 0x53, 0x69, 0x67, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, + 0x12, 0x0e, 0x0a, 0x02, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, + 0x12, 0x1c, 0x0a, 0x09, 0x73, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75, 0x72, 0x65, 0x18, 0x02, 0x20, + 0x01, 0x28, 0x0c, 0x52, 0x09, 0x73, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75, 0x72, 0x65, 0x22, 0x6e, + 0x0a, 0x0c, 0x42, 0x43, 0x61, 0x73, 0x74, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x0e, + 0x0a, 0x02, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, 0x12, 0x2e, + 0x0a, 0x07, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, + 0x14, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, + 0x66, 0x2e, 0x41, 0x6e, 0x79, 0x52, 0x07, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x1e, + 0x0a, 0x0a, 0x73, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75, 0x72, 0x65, 0x73, 0x18, 0x03, 0x20, 0x03, + 0x28, 0x0c, 0x52, 0x0a, 0x73, 0x69, 0x67, 0x6e, 0x61, 0x74, 0x75, 0x72, 0x65, 0x73, 0x42, 0x2c, + 0x5a, 0x2a, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x6f, 0x62, 0x6f, + 0x6c, 0x6e, 0x65, 0x74, 0x77, 0x6f, 0x72, 0x6b, 0x2f, 0x63, 0x68, 0x61, 0x72, 0x6f, 0x6e, 0x2f, + 0x64, 0x6b, 0x67, 0x2f, 0x64, 0x6b, 0x67, 0x70, 0x62, 0x2f, 0x76, 0x31, 0x62, 0x06, 0x70, 0x72, + 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -242,12 +244,13 @@ var file_dkg_dkgpb_v1_bcast_proto_goTypes = []interface{}{ (*anypb.Any)(nil), // 3: google.protobuf.Any } var file_dkg_dkgpb_v1_bcast_proto_depIdxs = []int32{ - 3, // 0: dkg.dkgpb.v1.BCastMessage.message:type_name -> google.protobuf.Any - 1, // [1:1] is the sub-list for method output_type - 1, // [1:1] is the sub-list for method input_type - 1, // [1:1] is the sub-list for extension type_name - 1, // [1:1] is the sub-list for extension extendee - 0, // [0:1] is the sub-list for field type_name + 3, // 0: dkg.dkgpb.v1.BCastSigRequest.message:type_name -> google.protobuf.Any + 3, // 1: dkg.dkgpb.v1.BCastMessage.message:type_name -> google.protobuf.Any + 2, // [2:2] is the sub-list for method output_type + 2, // [2:2] is the sub-list for method input_type + 2, // [2:2] is the sub-list for extension type_name + 2, // [2:2] is the sub-list for extension extendee + 0, // [0:2] is the sub-list for field type_name } func init() { file_dkg_dkgpb_v1_bcast_proto_init() } diff --git a/dkg/dkgpb/v1/bcast.proto b/dkg/dkgpb/v1/bcast.proto index 513314fe9..eeffa1c9b 100644 --- a/dkg/dkgpb/v1/bcast.proto +++ b/dkg/dkgpb/v1/bcast.proto @@ -7,8 +7,9 @@ import "google/protobuf/any.proto"; option go_package = "github.com/obolnetwork/charon/dkg/dkgpb/v1"; message BCastSigRequest { + reserved 2; string id = 1; - bytes hash = 2; + google.protobuf.Any message = 3; } message BCastSigResponse { diff --git a/dkg/frostp2p.go b/dkg/frostp2p.go index 60e74a416..e88aa71ec 100644 --- a/dkg/frostp2p.go +++ b/dkg/frostp2p.go @@ -14,6 +14,7 @@ import ( "github.com/libp2p/go-libp2p/core/peer" "github.com/libp2p/go-libp2p/core/protocol" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/anypb" "github.com/obolnetwork/charon/app/errors" "github.com/obolnetwork/charon/app/log" @@ -37,7 +38,7 @@ func frostMessageIDs() []string { // newFrostP2P returns a p2p frost transport implementation. // It registers bcast handlers on bcastComp. -func newFrostP2P(tcpNode host.Host, peers map[peer.ID]cluster.NodeIdx, bcastComp *bcast.Component, threshold, numVals int) *frostP2P { +func newFrostP2P(tcpNode host.Host, peers map[peer.ID]cluster.NodeIdx, bcastComp *bcast.Component, threshold, numVals int) (*frostP2P, error) { var ( round1CastsRecv = make(chan *pb.FrostRound1Casts, len(peers)) round1P2PRecv = make(chan *pb.FrostRound1P2P, len(peers)) @@ -60,7 +61,12 @@ func newFrostP2P(tcpNode host.Host, peers map[peer.ID]cluster.NodeIdx, bcastComp bcastCallback := newBcastCallback(peers, round1CastsRecv, round2CastsRecv, threshold, numVals) for _, frostMsgID := range frostMessageIDs() { - bcastComp.RegisterCallback(frostMsgID, bcastCallback) + checkMsg, err := newCheckMsg(frostMsgID) + if err != nil { + return nil, err + } + + bcastComp.RegisterMessageIDFuncs(frostMsgID, bcastCallback, checkMsg) } return &frostP2P{ @@ -70,7 +76,46 @@ func newFrostP2P(tcpNode host.Host, peers map[peer.ID]cluster.NodeIdx, bcastComp round1CastsRecv: round1CastsRecv, round1P2PRecv: round1P2PRecv, round2CastsRecv: round2CastsRecv, + }, nil +} + +// newCheckMsg returns a bcast.CheckMessage function for round 1 and 2. +func newCheckMsg(messageID string) (bcast.CheckMessage, error) { + found := false + for _, mID := range frostMessageIDs() { + if mID == messageID { + found = true + break + } + } + + if !found { + return nil, errors.New("frost message id unsupported", z.Str("message_id", messageID)) } + + return func(ctx context.Context, peerID peer.ID, msgAny *anypb.Any) error { + targetFn := func(messageID string) proto.Message { + switch messageID { + case round1CastID: + r := pb.FrostRound1Casts{} + return &r + case round2CastID: + r := pb.FrostRound2Casts{} + return &r + default: + return nil // Note: impossible due to the above check, but needed because compilers. + } + } + + target := targetFn(messageID) + + err := msgAny.UnmarshalTo(target) + if err != nil { + return errors.Wrap(err, "frost check message fail") + } + + return nil + }, nil } // newBcastCallback returns a callback for broadcast in round 1 and round 2 of frost protocol. diff --git a/dkg/nodesigs.go b/dkg/nodesigs.go index 432d6084d..5fb23c972 100644 --- a/dkg/nodesigs.go +++ b/dkg/nodesigs.go @@ -10,10 +10,12 @@ import ( k1 "github.com/decred/dcrd/dcrec/secp256k1/v4" "github.com/libp2p/go-libp2p/core/peer" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/anypb" "github.com/obolnetwork/charon/app/errors" "github.com/obolnetwork/charon/app/k1util" "github.com/obolnetwork/charon/app/log" + "github.com/obolnetwork/charon/app/z" "github.com/obolnetwork/charon/cluster" "github.com/obolnetwork/charon/dkg/bcast" dkgpb "github.com/obolnetwork/charon/dkg/dkgpb/v1" @@ -56,7 +58,7 @@ func newNodeSigBcast( } for _, k1Sig := range nodeSigMsgIDs() { - bcastComp.RegisterCallback(k1Sig, ret.broadcastCallback) + bcastComp.RegisterMessageIDFuncs(k1Sig, ret.broadcastCallback, ret.checkMessage) } return ret @@ -144,6 +146,17 @@ func (n *nodeSigBcast) broadcastCallback(ctx context.Context, _ peer.ID, _ strin return nil } +// checkMessage is the default bcast.CheckMessage for nodeSigBcast. +func (*nodeSigBcast) checkMessage(_ context.Context, peerID peer.ID, msgAny *anypb.Any) error { + var msg dkgpb.MsgNodeSig + err := msgAny.UnmarshalTo(&msg) + if err != nil { + return errors.Wrap(err, "node signature request malformed", z.Str("peer_id", peerID.String())) + } + + return nil +} + // exchange exchanges K1 signatures over lock file hashes with the peers pointed by lh.bcastFunc. func (n *nodeSigBcast) exchange( ctx context.Context, diff --git a/dkg/sync/client.go b/dkg/sync/client.go index d810a5dc9..2507f76e2 100644 --- a/dkg/sync/client.go +++ b/dkg/sync/client.go @@ -38,7 +38,7 @@ func NewClient(tcpNode host.Host, peer peer.ID, hashSig []byte, version version. done: make(chan struct{}), reconnect: true, version: version, - period: 250 * time.Millisecond, + period: 100 * time.Millisecond, // Must be at least two times lower than the sync timeout (dkg.go, startSyncProtocol) } for _, opt := range opts { @@ -53,7 +53,7 @@ func NewClient(tcpNode host.Host, peer peer.ID, hashSig []byte, version version. // supports reestablishing on relay circuit recycling, and supports soft shutdown. type Client struct { // Mutable state - mu sync.Mutex + mu sync.RWMutex connected bool reconnect bool step int @@ -113,16 +113,16 @@ func (c *Client) SetStep(step int) { // getStep returns the current step. func (c *Client) getStep() int { - c.mu.Lock() - defer c.mu.Unlock() + c.mu.RLock() + defer c.mu.RUnlock() return c.step } // IsConnected returns if client is connected to the server or not. func (c *Client) IsConnected() bool { - c.mu.Lock() - defer c.mu.Unlock() + c.mu.RLock() + defer c.mu.RUnlock() return c.connected } @@ -255,8 +255,8 @@ func (c *Client) DisableReconnect() { // shouldReconnect returns true if clients should re-attempt connecting to peers. func (c *Client) shouldReconnect() bool { - c.mu.Lock() - defer c.mu.Unlock() + c.mu.RLock() + defer c.mu.RUnlock() return c.reconnect } diff --git a/dkg/sync/server.go b/dkg/sync/server.go index 7496ade84..5341ca3df 100644 --- a/dkg/sync/server.go +++ b/dkg/sync/server.go @@ -57,7 +57,7 @@ type Server struct { allCount int // Excluding self // Mutable state - mu sync.Mutex + mu sync.RWMutex shutdown map[peer.ID]struct{} connected map[peer.ID]struct{} steps map[peer.ID]int @@ -95,8 +95,8 @@ func (s *Server) setErr(err error) { // Err returns the shared error state for the server. func (s *Server) Err() error { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + defer s.mu.RUnlock() return s.err } @@ -144,8 +144,8 @@ func (s *Server) AwaitAllAtStep(ctx context.Context, step int) error { // isConnected returns the shared connected state for the peer. func (s *Server) isConnected(pID peer.ID) bool { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + defer s.mu.RUnlock() _, ok := s.connected[pID] @@ -170,26 +170,43 @@ func (s *Server) setShutdown(pID peer.ID) { s.shutdown[pID] = struct{}{} } -// setStep sets the peer's reported step. -func (s *Server) setStep(pID peer.ID, step int) { +// updateStep updates the peer's step from the reported value. +// Returns error if the reported step is not the same or monotonically increased. +func (s *Server) updateStep(pID peer.ID, step int) error { s.mu.Lock() defer s.mu.Unlock() + currentPeerStep, hasCurrentPeerStep := s.steps[pID] + + if hasCurrentPeerStep && step < currentPeerStep { + return errors.New("peer reported step is behind the last known step", z.Int("peer_step", step), z.Int("last_step", currentPeerStep)) + } + + if hasCurrentPeerStep && step > currentPeerStep+1 { + return errors.New("peer reported step is ahead the last known step", z.Int("peer_step", step), z.Int("last_step", currentPeerStep)) + } + + if !hasCurrentPeerStep && (step < 0 || step > 1) { + return errors.New("peer reported abnormal initial step, expected 0 or 1", z.Int("peer_step", step)) + } + s.steps[pID] = step + + return nil } // isAllConnected returns if all expected peers are connected. func (s *Server) isAllConnected() bool { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + defer s.mu.RUnlock() return len(s.connected) == s.allCount } // isAllShutdown returns if all expected peers are shutdown. func (s *Server) isAllShutdown() bool { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + defer s.mu.RUnlock() return len(s.shutdown) == s.allCount } @@ -199,8 +216,8 @@ func (s *Server) isAllShutdown() bool { // so one peer will always increment first putting it ahead of the others. At least we know all peers // are or were at the given step. func (s *Server) isAllAtStep(step int) (bool, error) { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + defer s.mu.RUnlock() if len(s.steps) != s.allCount { return false, nil @@ -262,7 +279,9 @@ func (s *Server) handleStream(ctx context.Context, stream network.Stream) error log.Info(ctx, fmt.Sprintf("Connected to peer %d of %d", count, s.allCount)) } - s.setStep(pID, int(msg.Step)) + if err := s.updateStep(pID, int(msg.Step)); err != nil { + return err + } // Write response message if err := writeSizedProto(stream, resp); err != nil { diff --git a/dkg/sync/server_internal_test.go b/dkg/sync/server_internal_test.go new file mode 100644 index 000000000..abddfa3e4 --- /dev/null +++ b/dkg/sync/server_internal_test.go @@ -0,0 +1,60 @@ +// Copyright © 2022-2023 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package sync + +import ( + "testing" + + "github.com/libp2p/go-libp2p/core/peer" + "github.com/stretchr/testify/require" + + "github.com/obolnetwork/charon/app/version" + "github.com/obolnetwork/charon/testutil" +) + +func TestUpdateStep(t *testing.T) { + sv, err := version.Parse("v0.1") + require.NoError(t, err) + + server := &Server{ + defHash: testutil.RandomBytes32(), + tcpNode: nil, + allCount: 1, + shutdown: make(map[peer.ID]struct{}), + connected: make(map[peer.ID]struct{}), + steps: make(map[peer.ID]int), + version: sv, + } + + t.Run("wrong initial step", func(t *testing.T) { + err = server.updateStep("alpha", 100) + require.ErrorContains(t, err, "peer reported abnormal initial step, expected 0 or 1") + }) + + t.Run("valid peer step update", func(t *testing.T) { + err = server.updateStep("bravo", 1) + require.NoError(t, err) + + err = server.updateStep("bravo", 1) + require.NoError(t, err) // same step is allowed + + err = server.updateStep("bravo", 2) + require.NoError(t, err) // next step is allowed + }) + + t.Run("peer step is behind", func(t *testing.T) { + err = server.updateStep("behind", 1) + require.NoError(t, err) + + err = server.updateStep("behind", 0) + require.ErrorContains(t, err, "peer reported step is behind the last known step") + }) + + t.Run("peer step is ahead", func(t *testing.T) { + err = server.updateStep("ahead", 1) + require.NoError(t, err) + + err = server.updateStep("ahead", 3) + require.ErrorContains(t, err, "peer reported step is ahead the last known step") + }) +} diff --git a/docs/configuration.md b/docs/configuration.md index 5aa85bc74..20d12ed86 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -139,6 +139,7 @@ Usage: Flags: --beacon-node-endpoints strings Comma separated list of one or more beacon node endpoint URLs. --builder-api Enables the builder api. Will only produce builder blocks. Builder API must also be enabled on the validator client. Beacon node must be connected to a builder-relay to access the builder network. + --debug-address string Listening address (ip and port) for the pprof and QBFT debug API. It is not enabled by default. --feature-set string Minimum feature set to enable by default: alpha, beta, or stable. Warning: modify at own risk. (default "stable") --feature-set-disable strings Comma-separated list of features to disable, overriding the default minimum feature set. --feature-set-enable strings Comma-separated list of features to enable, overriding the default minimum feature set. @@ -153,14 +154,12 @@ Flags: --loki-addresses strings Enables sending of logfmt structured logs to these Loki log aggregation server addresses. This is in addition to normal stderr logs. --loki-service string Service label sent with logs to Loki. (default "charon") --manifest-file string The path to the cluster manifest file. If both cluster manifest and cluster lock files are provided, the cluster manifest file takes precedence. (default ".charon/cluster-manifest.pb") - --monitoring-address string Listening address (ip and port) for the monitoring API (prometheus, pprof). (default "127.0.0.1:3620") + --monitoring-address string Listening address (ip and port) for the monitoring API (prometheus). (default "127.0.0.1:3620") --no-verify Disables cluster definition and lock file verification. - --p2p-allowlist string Comma-separated list of CIDR subnets for allowing only certain peer connections. Example: 192.168.0.0/16 would permit connections to peers on your local network only. The default is to accept all connections. - --p2p-denylist string Comma-separated list of CIDR subnets for disallowing certain peer connections. Example: 192.168.0.0/16 would disallow connections to peers on your local network. The default is to accept all connections. --p2p-disable-reuseport Disables TCP port reuse for outgoing libp2p connections. --p2p-external-hostname string The DNS hostname advertised by libp2p. This may be used to advertise an external DNS. --p2p-external-ip string The IP address advertised by libp2p. This may be used to advertise an external IP. - --p2p-relays strings Comma-separated list of libp2p relay URLs or multiaddrs. (default [https://0.relay.obol.tech]) + --p2p-relays strings Comma-separated list of libp2p relay URLs or multiaddrs. (default [https://0.relay.obol.tech,https://1.relay.obol.tech]) --p2p-tcp-address strings Comma-separated list of listening TCP addresses (ip and port) for libP2P traffic. Empty default doesn't bind to local port therefore only supports outgoing connections. --private-key-file string The path to the charon enr private key file. (default ".charon/charon-enr-private-key") --private-key-file-lock Enables private key locking to prevent multiple instances using the same key. diff --git a/docs/structure.md b/docs/structure.md index 6919a5f4f..c7ab166d2 100644 --- a/docs/structure.md +++ b/docs/structure.md @@ -32,7 +32,6 @@ charon/ # project root │ │ # core workflow component implementations │ ├─ scheduler/ # scheduler │ ├─ fetcher/ # fetcher -│ ├─ leadercast/ # consensus implementation (will add qbft later) │ ├─ dutydb/ # dutydb │ ├─ validatorapi/ # validatorapi │ ├─ parsigdb/ # parsigdb diff --git a/eth2util/keymanager/keymanager.go b/eth2util/keymanager/keymanager.go index bf418e808..81c2d037d 100644 --- a/eth2util/keymanager/keymanager.go +++ b/eth2util/keymanager/keymanager.go @@ -41,36 +41,33 @@ func (c Client) ImportKeystores(ctx context.Context, keystores []keystore.Keysto z.Int("keystores", len(keystores)), z.Int("passwords", len(passwords))) } - addr, err := url.JoinPath(c.baseURL, "/eth/v1/keystores") + keymanagerURL, err := url.Parse(c.baseURL) if err != nil { - return errors.Wrap(err, "invalid base url", z.Str("base_url", c.baseURL)) + return errors.Wrap(err, "parse address", z.Str("addr", c.baseURL)) } - req, err := newReq(keystores, passwords) - if err != nil { - return err - } + keystoresURL := keymanagerURL.JoinPath("/eth/v1/keystores") - err = postKeys(ctx, addr, c.authToken, req) + req, err := newReq(keystores, passwords) if err != nil { return err } - return nil + return postKeys(ctx, keystoresURL.String(), c.authToken, req) } // VerifyConnection returns an error if the provided keymanager address is not reachable. func (c Client) VerifyConnection(ctx context.Context) error { - u, err := url.Parse(c.baseURL) + keymanagerURL, err := url.Parse(c.baseURL) if err != nil { - return errors.Wrap(err, "parse address") + return errors.Wrap(err, "parse address", z.Str("addr", c.baseURL)) } var d net.Dialer ctx, cancel := context.WithTimeout(ctx, 2*time.Second) defer cancel() - conn, err := d.DialContext(ctx, "tcp", u.Host) + conn, err := d.DialContext(ctx, "tcp", keymanagerURL.Host) if err != nil { return errors.Wrap(err, "cannot ping address", z.Str("addr", c.baseURL)) } diff --git a/eth2util/keymanager/keymanager_test.go b/eth2util/keymanager/keymanager_test.go index 63b444c52..4173b726a 100644 --- a/eth2util/keymanager/keymanager_test.go +++ b/eth2util/keymanager/keymanager_test.go @@ -114,6 +114,12 @@ func TestImportKeystores(t *testing.T) { err := cl.ImportKeystores(ctx, keystores, []string{}) require.ErrorContains(t, err, "lengths of keystores and passwords don't match") }) + + t.Run("malformed keymanager base URL", func(t *testing.T) { + cl := keymanager.New("https://1.1.1.1:-1234", testAuthToken) + err := cl.ImportKeystores(ctx, keystores, passwords) + require.ErrorContains(t, err, "parse address") + }) } func TestVerifyConnection(t *testing.T) { diff --git a/eth2util/rlp/rlp.go b/eth2util/rlp/rlp.go index f76892855..a08b0a86f 100644 --- a/eth2util/rlp/rlp.go +++ b/eth2util/rlp/rlp.go @@ -101,10 +101,9 @@ func decodeLength(item []byte) (offset int, length int, err error) { } if prefix < 0xc0 { - length = int(prefix - 0xb7) // length of the string in bytes in binary form - if length > 8 || length <= 0 { // This is impossible based on outer if else checks - panic("length not in expected range [1,8]") - } + // Due to above checks, prefix will be >= 0xb8 and < 0xc0, + // therefore the length must be in [1,8] + length = int(prefix - 0xb7) // length of the string in bytes in binary form offset := 1 + length @@ -124,10 +123,9 @@ func decodeLength(item []byte) (offset int, length int, err error) { return 1, int(prefix - 0xc0), nil } + // Due to above checks, prefix will be >= 0xf8 and <= 0xff + // therefore the length must be in [1,8] length = int(prefix - 0xf7) - if length > 8 || length <= 0 { // This is impossible based on outer if else checks - panic("length not in expected range [1,8]") - } offset = 1 + length diff --git a/eth2util/types.go b/eth2util/types.go index 6d971b98f..09d63571d 100644 --- a/eth2util/types.go +++ b/eth2util/types.go @@ -4,7 +4,6 @@ package eth2util import ( "encoding/json" - "strings" eth2spec "github.com/attestantio/go-eth2-client/spec" eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0" @@ -268,31 +267,15 @@ func (s SignedEpoch) MarshalJSON() ([]byte, error) { return b, nil } -// UnmarshalJSON unmarshalls both legacy []byte as well as 0xhex signatures. -// Remove support for legacy []byte in v0.19. +// UnmarshalJSON unmarshalls 0xhex signatures. func (s *SignedEpoch) UnmarshalJSON(b []byte) error { - var resp legacySignedEpochJSON + var resp signedEpochJSON if err := json.Unmarshal(b, &resp); err != nil { return errors.Wrap(err, "unmarshal signed epoch") } - s.Epoch = resp.Epoch - - if strings.HasPrefix(string(resp.Signature), "\"0x") { - if err := json.Unmarshal(resp.Signature, &s.Signature); err != nil { - return errors.Wrap(err, "unmarshal signed epoch signature") - } - return nil - } - - var sig []byte - if err := json.Unmarshal(resp.Signature, &sig); err != nil { - return errors.Wrap(err, "unmarshal legacy signed epoch signature") - } else if len(sig) != 96 { - return errors.New("invalid legacy signed epoch signature length") - } - - s.Signature = eth2p0.BLSSignature(sig) + s.Epoch = resp.Epoch + s.Signature = resp.Signature return nil } @@ -302,11 +285,3 @@ type signedEpochJSON struct { Epoch eth2p0.Epoch `json:"epoch"` Signature eth2p0.BLSSignature `json:"signature"` } - -// legacySignedEpochJSON supports both legacy []byte and 0xhex signatures -// -// TODO(corver): Remove in v0.19. -type legacySignedEpochJSON struct { - Epoch eth2p0.Epoch `json:"epoch"` - Signature json.RawMessage `json:"signature"` -} diff --git a/eth2util/types_test.go b/eth2util/types_test.go index 63b5e687e..baa23647b 100644 --- a/eth2util/types_test.go +++ b/eth2util/types_test.go @@ -47,12 +47,6 @@ func TestUnmarshallingSignedEpoch(t *testing.T) { testutil.RequireNoError(t, err) require.Equal(t, string(b), string(b2)) - type legacySig [96]byte - sigB, err := json.Marshal(legacySig(sig)) - require.NoError(t, err) - oldTmpl := `{"epoch": %d,"signature": %s}` - b = []byte(fmt.Sprintf(oldTmpl, epoch, sigB)) - var e2 eth2util.SignedEpoch err = e2.UnmarshalJSON(b) testutil.RequireNoError(t, err) diff --git a/p2p/bootnode.go b/p2p/bootnode.go index 3ce45cf5e..36f39a1a6 100644 --- a/p2p/bootnode.go +++ b/p2p/bootnode.go @@ -29,6 +29,9 @@ func NewRelays(ctx context.Context, relayAddrs []string, lockHashHex string, var resp []*MutablePeer for _, relayAddr := range relayAddrs { if strings.HasPrefix(relayAddr, "http") { + if !strings.HasPrefix(relayAddr, "https") { + log.Warn(ctx, "Relay URL does not use https protocol", nil, z.Str("addr", relayAddr)) + } mutable := new(MutablePeer) go resolveRelay(ctx, relayAddr, lockHashHex, mutable.Set) resp = append(resp, mutable) diff --git a/p2p/config.go b/p2p/config.go index 0a32f290b..6a0d18508 100644 --- a/p2p/config.go +++ b/p2p/config.go @@ -20,10 +20,6 @@ type Config struct { ExternalHost string // TCPAddrs defines the lib-p2p tcp listen addresses. TCPAddrs []string - // Allowlist defines csv CIDR blocks for lib-p2p allowed connections. - Allowlist string - // Allowlist defines csv CIDR blocks for lib-p2p denied connections. - Denylist string // DisableReuseport disables TCP port reuse for libp2p. DisableReuseport bool }