diff --git a/libs/utils/address.go b/libs/utils/address.go index c20d11ad06..716c113bed 100644 --- a/libs/utils/address.go +++ b/libs/utils/address.go @@ -37,9 +37,10 @@ func ValidateAddr(addr string) (string, error) { return addr, nil } - resolved, err := net.ResolveIPAddr("ip4", addr) + _, err = net.LookupHost(addr) if err != nil { - return addr, err + return "", err } - return resolved.String(), nil + + return addr, nil } diff --git a/libs/utils/address_test.go b/libs/utils/address_test.go index b18ead91a1..539ac6f6c3 100644 --- a/libs/utils/address_test.go +++ b/libs/utils/address_test.go @@ -1,7 +1,6 @@ package utils import ( - "net" "testing" "github.com/stretchr/testify/require" @@ -60,11 +59,6 @@ func TestValidateAddr(t *testing.T) { got, err := ValidateAddr(tt.addr) require.NoError(t, err) - // validate that returned value is ip - if ip := net.ParseIP(got); ip == nil { - t.Fatalf("empty ip") - } - if tt.want.unresolved { // unresolved addr has no addr to compare with return diff --git a/nodebuilder/core/config.go b/nodebuilder/core/config.go index bb5eea5b83..4ac8de0d34 100644 --- a/nodebuilder/core/config.go +++ b/nodebuilder/core/config.go @@ -1,29 +1,91 @@ package core import ( + "errors" "fmt" "strconv" "github.com/celestiaorg/celestia-node/libs/utils" ) -var MetricsEnabled bool +var ( + MetricsEnabled bool -// Config combines all configuration fields for managing the relationship with a Core node. + ErrMultipleHostsConfigured = errors.New("multiple hosts configured") +) + +const ( + DefaultRPCScheme = "http" + DefaultRPCPort = "26657" + DefaultGRPCPort = "9090" +) + +// Config combines all configuration fields for +// managing the relationship with a Core node. type Config struct { - IP string - RPCPort string - GRPCPort string + IP string + RPC RPCConfig + GRPC GRPCConfig +} + +type RPCConfig struct { + Scheme string + Host string + Port string +} + +type GRPCConfig struct { + Host string + Port string + // leaving separate to account for TLS + // and secure connections later } // DefaultConfig returns default configuration for managing the // node's connection to a Celestia-Core endpoint. func DefaultConfig() Config { return Config{ - IP: "", - RPCPort: "26657", - GRPCPort: "9090", + IP: "", + RPC: RPCConfig{ + Scheme: DefaultRPCScheme, + Port: DefaultRPCPort, + }, + GRPC: GRPCConfig{ + Port: DefaultGRPCPort, + }, + } +} + +func (cfg *Config) RPCHost() string { + if cfg.RPC.Host != "" { + return cfg.RPC.Host + } + return cfg.IP +} + +func (cfg *Config) GRPCHost() string { + if cfg.GRPC.Host != "" { + return cfg.GRPC.Host } + return cfg.IP +} + +func (cfg *Config) multipleHostsConfigured() error { + if cfg.IP != "" && cfg.RPC.Host != "" { + return fmt.Errorf( + "%w: core.ip overridden by core.rpc.host", + ErrMultipleHostsConfigured, + ) + } + + if cfg.IP != "" && cfg.GRPC.Host != "" { + return fmt.Errorf( + "%w: core.ip overridden by core.grpc.host", + ErrMultipleHostsConfigured, + ) + } + + return nil } // Validate performs basic validation of the config. @@ -32,24 +94,43 @@ func (cfg *Config) Validate() error { return nil } - ip, err := utils.ValidateAddr(cfg.IP) - if err != nil { + if err := cfg.multipleHostsConfigured(); err != nil { return err } - cfg.IP = ip - _, err = strconv.Atoi(cfg.RPCPort) + + if cfg.RPC.Scheme == "" { + cfg.RPC.Scheme = DefaultRPCScheme + } + + rpcHost, err := utils.ValidateAddr(cfg.RPCHost()) + if err != nil { + return fmt.Errorf("nodebuilder/core: invalid rpc host: %s", err.Error()) + } + + cfg.RPC.Host = rpcHost + + grpcHost, err := utils.ValidateAddr(cfg.GRPCHost()) + if err != nil { + return fmt.Errorf("nodebuilder/core: invalid grpc host: %s", err.Error()) + } + + cfg.GRPC.Host = grpcHost + + _, err = strconv.Atoi(cfg.RPC.Port) if err != nil { return fmt.Errorf("nodebuilder/core: invalid rpc port: %s", err.Error()) } - _, err = strconv.Atoi(cfg.GRPCPort) + + _, err = strconv.Atoi(cfg.GRPC.Port) if err != nil { return fmt.Errorf("nodebuilder/core: invalid grpc port: %s", err.Error()) } + return nil } // IsEndpointConfigured returns whether a core endpoint has been set // on the config (true if set). func (cfg *Config) IsEndpointConfigured() bool { - return cfg.IP != "" + return cfg.RPCHost() != "" && cfg.GRPCHost() != "" } diff --git a/nodebuilder/core/config_test.go b/nodebuilder/core/config_test.go new file mode 100644 index 0000000000..91284ccda3 --- /dev/null +++ b/nodebuilder/core/config_test.go @@ -0,0 +1,156 @@ +package core + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDefaultValues(t *testing.T) { + expectedRPCScheme := "http" + expectedRPCPort := "26657" + expectedGRPCPort := "9090" + + assert.Equal(t, expectedRPCScheme, DefaultRPCScheme, "DefaultRPCScheme is incorrect") + assert.Equal(t, expectedRPCPort, DefaultRPCPort, "DefaultRPCPort is incorrect") + assert.Equal(t, expectedGRPCPort, DefaultGRPCPort, "DefaultGRPCPort is incorrect") +} + +func TestGRPCHost(t *testing.T) { + testCases := []struct { + name string + cfg *Config + expected string + }{ + { + name: "Fallback to IP when GRPC Host is not set", + cfg: &Config{ + GRPC: GRPCConfig{ + Host: "", + }, + IP: "127.0.0.1", + }, + expected: "127.0.0.1", + }, + { + name: "Use GRPC Host when set", + cfg: &Config{ + GRPC: GRPCConfig{ + Host: "0.0.0.0", + }, + IP: "127.0.0.1", + }, + expected: "0.0.0.0", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := tc.cfg.GRPCHost() + assert.Equal(t, tc.expected, actual) + }) + } +} + +func TestRPCHost(t *testing.T) { + testCases := []struct { + name string + cfg *Config + expected string + }{ + { + name: "Fallback to IP when GRPC Host is not set", + cfg: &Config{ + RPC: RPCConfig{ + Host: "", + }, + IP: "127.0.0.1", + }, + expected: "127.0.0.1", + }, + { + name: "Use GRPC Host when set", + cfg: &Config{ + RPC: RPCConfig{ + Host: "0.0.0.0", + }, + IP: "127.0.0.1", + }, + expected: "0.0.0.0", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := tc.cfg.RPCHost() + assert.Equal(t, tc.expected, actual) + }) + } +} + +func TestMultipleHostsConfigured(t *testing.T) { + testCases := []struct { + name string + cfg *Config + expected error + }{ + { + name: "IP and RPC Host both configured", + cfg: &Config{ + IP: "127.0.0.1", + RPC: RPCConfig{ + Host: "localhost", + }, + }, + expected: fmt.Errorf("multiple hosts configured: core.ip overridden by core.rpc.host"), + }, + { + name: "IP and gRPC Host both configured", + cfg: &Config{ + IP: "127.0.0.1", + GRPC: GRPCConfig{ + Host: "localhost", + }, + }, + expected: fmt.Errorf("multiple hosts configured: core.ip overridden by core.grpc.host"), + }, + { + name: "Only IP configured", + cfg: &Config{ + IP: "127.0.0.1", + }, + expected: nil, + }, + { + name: "Only RPC Host configured", + cfg: &Config{ + RPC: RPCConfig{ + Host: "localhost", + }, + }, + expected: nil, + }, + { + name: "Only gRPC Host configured", + cfg: &Config{ + GRPC: GRPCConfig{ + Host: "localhost", + }, + }, + expected: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := tc.cfg.multipleHostsConfigured() + if tc.expected == nil { + assert.Nil(t, actual) + } else { + assert.Error(t, actual) + assert.EqualError(t, actual, tc.expected.Error()) + } + }) + } +} diff --git a/nodebuilder/core/constructors.go b/nodebuilder/core/constructors.go index 53c914a041..ed036466d9 100644 --- a/nodebuilder/core/constructors.go +++ b/nodebuilder/core/constructors.go @@ -5,5 +5,5 @@ import ( ) func remote(cfg Config) (core.Client, error) { - return core.NewRemote(cfg.IP, cfg.RPCPort) + return core.NewRemote(cfg.RPCHost(), cfg.RPC.Port) } diff --git a/nodebuilder/core/flags.go b/nodebuilder/core/flags.go index 9cbed9b277..7b7dc5b81c 100644 --- a/nodebuilder/core/flags.go +++ b/nodebuilder/core/flags.go @@ -1,60 +1,84 @@ package core import ( - "fmt" - "github.com/spf13/cobra" flag "github.com/spf13/pflag" ) var ( - coreFlag = "core.ip" - coreRPCFlag = "core.rpc.port" - coreGRPCFlag = "core.grpc.port" + coreIPFlag = "core.ip" // TODO: @ramin - we should deprecate this + + rpcHostFlag = "core.rpc.host" + rpcPortFlag = "core.rpc.port" + rpcUseHTTPS = "core.rpc.https" + + grpcHostFlag = "core.grpc.host" + grpcPortFlag = "core.grpc.port" ) // Flags gives a set of hardcoded Core flags. +// +//nolint:goconst func Flags() *flag.FlagSet { flags := &flag.FlagSet{} flags.String( - coreFlag, + coreIPFlag, "", - "Indicates node to connect to the given core node. "+ + "Indicates node to connect to the given core node's RPC and gRPC. "+ + "NOTE: If this flag is set, the core.rpc.ip and core.grpc.ip flags cannot be set. "+ "Example: , 127.0.0.1. , subdomain.domain.tld "+ "Assumes RPC port 26657 and gRPC port 9090 as default unless otherwise specified.", ) + // RPC configuration flags flags.String( - coreRPCFlag, - "26657", - "Set a custom RPC port for the core node connection. The --core.ip flag must also be provided.", + rpcHostFlag, + "", + "Indicates node to connect to the given core node's RPC. "+ + "NOTE: If this flag is set, the core.ip flag cannot be set. "+ + "Example: , 127.0.0.1. , subdomain.domain.tld "+ + "Assumes RPC port 26657 as default unless otherwise specified.", ) flags.String( - coreGRPCFlag, - "9090", - "Set a custom gRPC port for the core node connection. The --core.ip flag must also be provided.", + rpcPortFlag, + DefaultRPCPort, + "Set a custom RPC port for the core node connection.", + ) + flags.Bool( + rpcUseHTTPS, + false, + "Use https for RPC connection. Default is false.", + ) + // gRPC configuration flags + flags.String( + grpcHostFlag, + "", + "Indicates node to connect to the given core node's gRPC. "+ + "NOTE: If this flag is set, the core.ip flag cannot be set. "+ + "Example: , 127.0.0.1. , subdomain.domain.tld "+ + "Assumes RPC gRPC port 9090 as default unless otherwise specified.", + ) + flags.String( + grpcPortFlag, + DefaultGRPCPort, + "Set a custom gRPC port for the core node connection.", ) return flags } // ParseFlags parses Core flags from the given cmd and saves them to the passed config. -func ParseFlags( - cmd *cobra.Command, - cfg *Config, -) error { - coreIP := cmd.Flag(coreFlag).Value.String() - if coreIP == "" { - if cmd.Flag(coreGRPCFlag).Changed || cmd.Flag(coreRPCFlag).Changed { - return fmt.Errorf("cannot specify RPC/gRPC ports without specifying an IP address for --core.ip") - } - return nil +func ParseFlags(cmd *cobra.Command, cfg *Config) error { + cfg.IP = cmd.Flag(coreIPFlag).Value.String() + cfg.RPC.Host = cmd.Flag(rpcHostFlag).Value.String() + cfg.RPC.Port = cmd.Flag(rpcPortFlag).Value.String() + + // if the flag is present, use HTTPS + if cmd.Flag(rpcUseHTTPS).Changed { + cfg.RPC.Scheme = "https" } - rpc := cmd.Flag(coreRPCFlag).Value.String() - grpc := cmd.Flag(coreGRPCFlag).Value.String() + cfg.GRPC.Host = cmd.Flag(grpcHostFlag).Value.String() + cfg.GRPC.Port = cmd.Flag(grpcPortFlag).Value.String() - cfg.IP = coreIP - cfg.RPCPort = rpc - cfg.GRPCPort = grpc return cfg.Validate() } diff --git a/nodebuilder/core/flags_test.go b/nodebuilder/core/flags_test.go new file mode 100644 index 0000000000..a78f372b5f --- /dev/null +++ b/nodebuilder/core/flags_test.go @@ -0,0 +1,92 @@ +package core + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" +) + +func TestParseFlags(t *testing.T) { + testCases := []struct { + name string + args []string + expected Config + }{ + { + name: "Test core.ip flag sets RPC and gRPC hosts", + args: []string{ + "--core.ip=192.168.1.1", + }, + expected: Config{ + IP: "192.168.1.1", + RPC: RPCConfig{ + Host: "192.168.1.1", + Port: DefaultRPCPort, + Scheme: DefaultRPCScheme, + }, + GRPC: GRPCConfig{ + Host: "192.168.1.1", + Port: DefaultGRPCPort, + }, + }, + }, + { + name: "Test RPC and gRPC flags", + args: []string{ + "--core.rpc.host=127.0.0.1", + "--core.rpc.port=26658", + "--core.grpc.host=127.0.0.1", + "--core.grpc.port=9091", + }, + expected: Config{ + RPC: RPCConfig{ + Host: "127.0.0.1", + Port: "26658", + Scheme: "http", + }, + GRPC: GRPCConfig{ + Host: "127.0.0.1", + Port: "9091", + }, + }, + }, + { + name: "Test HTTPS flag", + args: []string{"--core.rpc.host=127.0.0.1", "--core.rpc.https=true"}, + expected: Config{ + RPC: RPCConfig{ + Host: "127.0.0.1", + Port: DefaultRPCPort, // Assuming DefaultRPCPort is defined elsewhere + Scheme: "https", + }, + GRPC: GRPCConfig{ + Host: "", + Port: DefaultGRPCPort, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Setup Cobra command + cmd := &cobra.Command{ + Use: "testCommand", + } + cmd.Flags().AddFlagSet(Flags()) + + // Apply flags from test case + err := cmd.ParseFlags(tc.args) + assert.NoError(t, err) + + // Parse flags into config + var cfg Config + err = ParseFlags(cmd, &cfg) + assert.NoError(t, err) + + // Assert the expected configuration + assert.Equal(t, tc.expected, cfg) + }) + } +} diff --git a/nodebuilder/state/core.go b/nodebuilder/state/core.go index 591529ecbd..ec9637a5cf 100644 --- a/nodebuilder/state/core.go +++ b/nodebuilder/state/core.go @@ -28,8 +28,17 @@ func coreAccessor( *modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader], error, ) { - ca, err := state.NewCoreAccessor(keyring, string(keyname), sync, corecfg.IP, corecfg.RPCPort, corecfg.GRPCPort, - opts...) + ca, err := state.NewCoreAccessor( + keyring, + string(keyname), + sync, + corecfg.RPC.Scheme, + corecfg.RPCHost(), + corecfg.RPC.Port, + corecfg.GRPCHost(), + corecfg.GRPC.Port, + opts..., + ) sBreaker := &modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader]{ Service: ca, diff --git a/nodebuilder/tests/swamp/swamp.go b/nodebuilder/tests/swamp/swamp.go index 93879c4425..05798436e1 100644 --- a/nodebuilder/tests/swamp/swamp.go +++ b/nodebuilder/tests/swamp/swamp.go @@ -195,8 +195,9 @@ func (s *Swamp) DefaultTestConfig(tp node.Type) *nodebuilder.Config { ip, port, err := net.SplitHostPort(s.cfg.AppConfig.GRPC.Address) require.NoError(s.t, err) - cfg.Core.IP = ip - cfg.Core.GRPCPort = port + cfg.Core.RPC.Host = ip + cfg.Core.GRPC.Host = ip + cfg.Core.GRPC.Port = port return cfg } diff --git a/state/core_access.go b/state/core_access.go index cc5baab740..a5f63dd178 100644 --- a/state/core_access.go +++ b/state/core_access.go @@ -85,9 +85,12 @@ type CoreAccessor struct { prt *merkle.ProofRuntime coreConn *grpc.ClientConn - coreIP string - rpcPort string - grpcPort string + + rpcScheme string + rpcHost string + rpcPort string + grpcHost string + grpcPort string // these fields are mutatable and thus need to be protected by a mutex lock sync.Mutex @@ -112,8 +115,10 @@ func NewCoreAccessor( keyring keyring.Keyring, keyname string, getter libhead.Head[*header.ExtendedHeader], - coreIP, - rpcPort string, + rpcScheme, + rpcHost, + rpcPort, + grpcHost, grpcPort string, options ...Option, ) (*CoreAccessor, error) { @@ -132,18 +137,21 @@ func NewCoreAccessor( } ca := &CoreAccessor{ - keyring: keyring, - addr: addr, - getter: getter, - coreIP: coreIP, - rpcPort: rpcPort, - grpcPort: grpcPort, - prt: prt, + keyring: keyring, + addr: addr, + getter: getter, + rpcScheme: rpcScheme, + rpcHost: rpcHost, + rpcPort: rpcPort, + grpcHost: grpcHost, + grpcPort: grpcPort, + prt: prt, } for _, opt := range options { opt(ca) } + return ca, nil } @@ -154,7 +162,8 @@ func (ca *CoreAccessor) Start(ctx context.Context) error { ca.ctx, ca.cancel = context.WithCancel(context.Background()) // dial given celestia-core endpoint - endpoint := fmt.Sprintf("%s:%s", ca.coreIP, ca.grpcPort) + endpoint := fmt.Sprintf("%s:%s", ca.grpcHost, ca.grpcPort) + client, err := grpc.DialContext( ctx, endpoint, @@ -170,7 +179,7 @@ func (ca *CoreAccessor) Start(ctx context.Context) error { ca.stakingCli = stakingCli ca.feeGrantCli = feegrant.NewQueryClient(ca.coreConn) // create ABCI query client - cli, err := http.New(fmt.Sprintf("http://%s:%s", ca.coreIP, ca.rpcPort), "/websocket") + cli, err := http.New(fmt.Sprintf("%s://%s:%s", ca.rpcScheme, ca.rpcHost, ca.rpcPort), "/websocket") if err != nil { return err } diff --git a/state/core_access_test.go b/state/core_access_test.go index 0f9b0f5318..51abb3a0fd 100644 --- a/state/core_access_test.go +++ b/state/core_access_test.go @@ -36,9 +36,19 @@ func TestSubmitPayForBlob(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - ca, err := NewCoreAccessor(cctx.Keyring, accounts[0], nil, "127.0.0.1", extractPort(rpcAddr), - extractPort(grpcAddr)) + coreIP := "127.0.0.1" + ca, err := NewCoreAccessor( + cctx.Keyring, + accounts[0], + nil, + "http", + coreIP, + extractPort(rpcAddr), + coreIP, + extractPort(grpcAddr), + ) require.NoError(t, err) + // start the accessor err = ca.Start(ctx) require.NoError(t, err) diff --git a/state/integration_test.go b/state/integration_test.go index 6d2a38c8a7..66abd60ff5 100644 --- a/state/integration_test.go +++ b/state/integration_test.go @@ -48,7 +48,7 @@ func (s *IntegrationTestSuite) SetupSuite() { s.cctx = core.StartTestNodeWithConfig(s.T(), cfg) s.accounts = cfg.Accounts - accessor, err := NewCoreAccessor(s.cctx.Keyring, s.accounts[0], localHeader{s.cctx.Client}, "", "", "") + accessor, err := NewCoreAccessor(s.cctx.Keyring, s.accounts[0], localHeader{s.cctx.Client}, "", "", "", "", "") require.NoError(s.T(), err) setClients(accessor, s.cctx.GRPCClient, s.cctx.Client) s.accessor = accessor