diff --git a/.github/workflows/go-ci.yml b/.github/workflows/go-ci.yml index b519db7763..e6c4e5881b 100644 --- a/.github/workflows/go-ci.yml +++ b/.github/workflows/go-ci.yml @@ -23,7 +23,7 @@ jobs: go-version: ${{ env.GO_VERSION }} - name: golangci-lint - uses: golangci/golangci-lint-action@v3.4.0 + uses: golangci/golangci-lint-action@v3.6.0 with: version: v1.52.2 diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 314f2fcb14..bed2b3352c 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -12,7 +12,7 @@ jobs: label: runs-on: ubuntu-latest steps: - - uses: mheap/github-action-required-labels@v4 + - uses: mheap/github-action-required-labels@v5 with: mode: minimum count: 1 diff --git a/api/docgen/examples.go b/api/docgen/examples.go index b19b06a7f8..80a8c64d93 100644 --- a/api/docgen/examples.go +++ b/api/docgen/examples.go @@ -83,6 +83,8 @@ func init() { } addToExampleValues(valAddr) + addToExampleValues(state.Address{Address: addr}) + var txResponse *state.TxResponse err = json.Unmarshal([]byte(exampleTxResponse), &txResponse) if err != nil { diff --git a/api/gateway/das.go b/api/gateway/das.go index 565cbd7460..88dc97927c 100644 --- a/api/gateway/das.go +++ b/api/gateway/das.go @@ -10,6 +10,7 @@ const ( ) func (h *Handler) handleDASStateRequest(w http.ResponseWriter, r *http.Request) { + logDeprecation(dasStateEndpoint, "das.SamplingStats") stats, err := h.das.SamplingStats(r.Context()) if err != nil { writeError(w, http.StatusInternalServerError, dasStateEndpoint, err) diff --git a/api/gateway/endpoints.go b/api/gateway/endpoints.go index dfcb96bd06..0ae93b112c 100644 --- a/api/gateway/endpoints.go +++ b/api/gateway/endpoints.go @@ -5,21 +5,32 @@ import ( "net/http" ) -func (h *Handler) RegisterEndpoints(rpc *Server) { +func (h *Handler) RegisterEndpoints(rpc *Server, deprecatedEndpointsEnabled bool) { + if deprecatedEndpointsEnabled { + log.Warn("Deprecated endpoints will be removed from the gateway in the next release. Use the RPC instead.") + // state endpoints + rpc.RegisterHandlerFunc(balanceEndpoint, h.handleBalanceRequest, http.MethodGet) + rpc.RegisterHandlerFunc(submitPFBEndpoint, h.handleSubmitPFB, http.MethodPost) + + // staking queries + rpc.RegisterHandlerFunc(fmt.Sprintf("%s/{%s}", queryDelegationEndpoint, addrKey), h.handleQueryDelegation, + http.MethodGet) + rpc.RegisterHandlerFunc(fmt.Sprintf("%s/{%s}", queryUnbondingEndpoint, addrKey), h.handleQueryUnbonding, + http.MethodGet) + rpc.RegisterHandlerFunc(queryRedelegationsEndpoint, h.handleQueryRedelegations, + http.MethodPost) + + // DASer endpoints + // only register if DASer service is available + if h.das != nil { + rpc.RegisterHandlerFunc(dasStateEndpoint, h.handleDASStateRequest, http.MethodGet) + } + } + // state endpoints - rpc.RegisterHandlerFunc(balanceEndpoint, h.handleBalanceRequest, http.MethodGet) rpc.RegisterHandlerFunc(fmt.Sprintf("%s/{%s}", balanceEndpoint, addrKey), h.handleBalanceRequest, http.MethodGet) rpc.RegisterHandlerFunc(submitTxEndpoint, h.handleSubmitTx, http.MethodPost) - rpc.RegisterHandlerFunc(submitPFBEndpoint, h.handleSubmitPFB, http.MethodPost) - - // staking queries - rpc.RegisterHandlerFunc(fmt.Sprintf("%s/{%s}", queryDelegationEndpoint, addrKey), h.handleQueryDelegation, - http.MethodGet) - rpc.RegisterHandlerFunc(fmt.Sprintf("%s/{%s}", queryUnbondingEndpoint, addrKey), h.handleQueryUnbonding, - http.MethodGet) - rpc.RegisterHandlerFunc(queryRedelegationsEndpoint, h.handleQueryRedelegations, - http.MethodPost) // share endpoints rpc.RegisterHandlerFunc(fmt.Sprintf("%s/{%s}/height/{%s}", namespacedSharesEndpoint, nIDKey, heightKey), @@ -39,10 +50,4 @@ func (h *Handler) RegisterEndpoints(rpc *Server) { rpc.RegisterHandlerFunc(fmt.Sprintf("%s/{%s}", headerByHeightEndpoint, heightKey), h.handleHeaderRequest, http.MethodGet) rpc.RegisterHandlerFunc(headEndpoint, h.handleHeadRequest, http.MethodGet) - - // DASer endpoints - // only register if DASer service is available - if h.das != nil { - rpc.RegisterHandlerFunc(dasStateEndpoint, h.handleDASStateRequest, http.MethodGet) - } } diff --git a/api/gateway/state.go b/api/gateway/state.go index 63ada8ebb2..b584b00d36 100644 --- a/api/gateway/state.go +++ b/api/gateway/state.go @@ -72,8 +72,9 @@ func (h *Handler) handleBalanceRequest(w http.ResponseWriter, r *http.Request) { } addr = valAddr.Bytes() } - bal, err = h.state.BalanceForAddress(r.Context(), addr) + bal, err = h.state.BalanceForAddress(r.Context(), state.Address{Address: addr}) } else { + logDeprecation(balanceEndpoint, "state.Balance") bal, err = h.state.Balance(r.Context()) } if err != nil { @@ -122,6 +123,7 @@ func (h *Handler) handleSubmitTx(w http.ResponseWriter, r *http.Request) { } func (h *Handler) handleSubmitPFB(w http.ResponseWriter, r *http.Request) { + logDeprecation(submitPFBEndpoint, "blob.Submit or state.SubmitPayForBlob") // decode request var req submitPFBRequest err := json.NewDecoder(r.Body).Decode(&req) @@ -171,6 +173,7 @@ func (h *Handler) handleSubmitPFB(w http.ResponseWriter, r *http.Request) { } func (h *Handler) handleQueryDelegation(w http.ResponseWriter, r *http.Request) { + logDeprecation(queryDelegationEndpoint, "state.QueryDelegation") // read and parse request vars := mux.Vars(r) addrStr, exists := vars[addrKey] @@ -202,6 +205,7 @@ func (h *Handler) handleQueryDelegation(w http.ResponseWriter, r *http.Request) } func (h *Handler) handleQueryUnbonding(w http.ResponseWriter, r *http.Request) { + logDeprecation(queryUnbondingEndpoint, "state.QueryUnbonding") // read and parse request vars := mux.Vars(r) addrStr, exists := vars[addrKey] @@ -233,6 +237,7 @@ func (h *Handler) handleQueryUnbonding(w http.ResponseWriter, r *http.Request) { } func (h *Handler) handleQueryRedelegations(w http.ResponseWriter, r *http.Request) { + logDeprecation(queryRedelegationsEndpoint, "state.QueryRedelegations") var req queryRedelegationsRequest err := json.NewDecoder(r.Body).Decode(&req) if err != nil { @@ -264,3 +269,8 @@ func (h *Handler) handleQueryRedelegations(w http.ResponseWriter, r *http.Reques log.Errorw("writing response", "endpoint", queryRedelegationsEndpoint, "err", err) } } + +func logDeprecation(endpoint string, alternative string) { + log.Warn("The " + endpoint + " endpoint is deprecated and will be removed in the next release. Please " + + "use " + alternative + " from the RPC instead.") +} diff --git a/api/rpc/client/client.go b/api/rpc/client/client.go index 9e7e577074..7ac8a55b3d 100644 --- a/api/rpc/client/client.go +++ b/api/rpc/client/client.go @@ -18,18 +18,11 @@ import ( "github.com/celestiaorg/celestia-node/nodebuilder/state" ) -// TODO: this duplication of strings many times across the codebase can be avoided with issue #1176 -var client Client -var Modules = map[string]interface{}{ - "share": &client.Share.Internal, - "state": &client.State.Internal, - "header": &client.Header.Internal, - "fraud": &client.Fraud.Internal, - "das": &client.DAS.Internal, - "p2p": &client.P2P.Internal, - "node": &client.Node.Internal, - "blob": &client.Blob.Internal, -} +var ( + // staticClient is used for generating the OpenRPC spec. + staticClient Client + Modules = moduleMap(&staticClient) +) type Client struct { Fraud fraud.API @@ -61,7 +54,7 @@ func (m *multiClientCloser) closeAll() { } } -// Close closes the connections to all namespaces registered on the client. +// Close closes the connections to all namespaces registered on the staticClient. func (c *Client) Close() { c.closer.closeAll() } @@ -80,7 +73,8 @@ func NewClient(ctx context.Context, addr string, token string) (*Client, error) func newClient(ctx context.Context, addr string, authHeader http.Header) (*Client, error) { var multiCloser multiClientCloser - for name, module := range Modules { + var client Client + for name, module := range moduleMap(&client) { closer, err := jsonrpc.NewClient(ctx, addr, name, module, authHeader) if err != nil { return nil, err @@ -90,3 +84,17 @@ func newClient(ctx context.Context, addr string, authHeader http.Header) (*Clien return &client, nil } + +func moduleMap(client *Client) map[string]interface{} { + // TODO: this duplication of strings many times across the codebase can be avoided with issue #1176 + return map[string]interface{}{ + "share": &client.Share.Internal, + "state": &client.State.Internal, + "header": &client.Header.Internal, + "fraud": &client.Fraud.Internal, + "das": &client.DAS.Internal, + "p2p": &client.P2P.Internal, + "node": &client.Node.Internal, + "blob": &client.Blob.Internal, + } +} diff --git a/cmd/celestia/rpc.go b/cmd/celestia/rpc.go index de8c55e3f4..767fca872c 100644 --- a/cmd/celestia/rpc.go +++ b/cmd/celestia/rpc.go @@ -5,7 +5,6 @@ import ( "encoding/base64" "encoding/hex" "encoding/json" - "errors" "fmt" "io" "log" @@ -15,7 +14,6 @@ import ( "strconv" "strings" - "github.com/cosmos/cosmos-sdk/types" "github.com/spf13/cobra" "github.com/celestiaorg/nmt/namespace" @@ -392,18 +390,12 @@ func sendJSONRPCRequest(namespace, method string, params []interface{}) { } func parseAddressFromString(addrStr string) (state.Address, error) { - var addr state.AccAddress - addr, err := types.AccAddressFromBech32(addrStr) + var address state.Address + err := address.UnmarshalJSON([]byte(addrStr)) if err != nil { - // first check if it is a validator address and can be converted - valAddr, err := types.ValAddressFromBech32(addrStr) - if err != nil { - return nil, errors.New("address must be a valid account or validator address ") - } - return valAddr, nil + return address, err } - - return addr, nil + return address, nil } func parseSignatureForHelpstring(methodSig reflect.StructField) string { diff --git a/go.mod b/go.mod index af31a7a61c..5f5fe104a0 100644 --- a/go.mod +++ b/go.mod @@ -29,7 +29,7 @@ require ( github.com/gorilla/mux v1.8.0 github.com/hashicorp/go-retryablehttp v0.7.2 github.com/hashicorp/golang-lru v0.5.5-0.20210104140557-80c98217689d - github.com/imdario/mergo v0.3.15 + github.com/imdario/mergo v0.3.16 github.com/ipfs/go-blockservice v0.5.0 github.com/ipfs/go-cid v0.4.1 github.com/ipfs/go-datastore v0.6.0 @@ -57,7 +57,7 @@ require ( github.com/multiformats/go-multihash v0.2.2 github.com/open-rpc/meta-schema v0.0.0-20201029221707-1b72ef2ea333 github.com/prometheus/client_golang v1.14.0 - github.com/pyroscope-io/client v0.7.0 + github.com/pyroscope-io/client v0.7.1 github.com/pyroscope-io/otel-profiling-go v0.4.0 github.com/spf13/cobra v1.6.1 github.com/spf13/pflag v1.0.5 @@ -74,6 +74,7 @@ require ( go.uber.org/fx v1.19.3 go.uber.org/zap v1.24.0 golang.org/x/crypto v0.9.0 + golang.org/x/exp v0.0.0-20230321023759-10a507213a29 golang.org/x/sync v0.2.0 golang.org/x/text v0.9.0 google.golang.org/grpc v1.53.0 @@ -309,7 +310,6 @@ require ( go.uber.org/atomic v1.11.0 // indirect go.uber.org/dig v1.17.0 // indirect go.uber.org/multierr v1.11.0 // indirect - golang.org/x/exp v0.0.0-20230321023759-10a507213a29 // indirect golang.org/x/mod v0.10.0 // indirect golang.org/x/net v0.10.0 // indirect golang.org/x/oauth2 v0.4.0 // indirect diff --git a/go.sum b/go.sum index e2ef619a8d..e548acff6a 100644 --- a/go.sum +++ b/go.sum @@ -933,8 +933,8 @@ github.com/iancoleman/orderedmap v0.1.0/go.mod h1:N0Wam8K1arqPXNWjMo21EXnBPOPp36 github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/icrowley/fake v0.0.0-20180203215853-4178557ae428/go.mod h1:uhpZMVGznybq1itEKXj6RYw9I71qK4kH+OGMjRC4KEo= -github.com/imdario/mergo v0.3.15 h1:M8XP7IuFNsqUx6VPK2P9OSmsYsI/YFaGil0uD21V3dM= -github.com/imdario/mergo v0.3.15/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY= +github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4= +github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY= github.com/improbable-eng/grpc-web v0.15.0 h1:BN+7z6uNXZ1tQGcNAuaU1YjsLTApzkjt2tzCixLaUPQ= github.com/improbable-eng/grpc-web v0.15.0/go.mod h1:1sy9HKV4Jt9aEs9JSnkWlRJPuPtwNr0l57L4f878wP8= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= @@ -1778,8 +1778,8 @@ github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1 github.com/prometheus/procfs v0.9.0 h1:wzCHvIvM5SxWqYvwgVL7yJY8Lz3PKn49KQtpgMYJfhI= github.com/prometheus/procfs v0.9.0/go.mod h1:+pB4zwohETzFnmlpe6yd2lSc+0/46IYZRB/chUwxUZY= github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU= -github.com/pyroscope-io/client v0.7.0 h1:LWuuqPQ1oa6x7BnmUOuo/aGwdX85QGhWZUBYWWW3zdk= -github.com/pyroscope-io/client v0.7.0/go.mod h1:4h21iOU4pUOq0prKyDlvYRL+SCKsBc5wKiEtV+rJGqU= +github.com/pyroscope-io/client v0.7.1 h1:yFRhj3vbgjBxehvxQmedmUWJQ4CAfCHhn+itPsuWsHw= +github.com/pyroscope-io/client v0.7.1/go.mod h1:4h21iOU4pUOq0prKyDlvYRL+SCKsBc5wKiEtV+rJGqU= github.com/pyroscope-io/godeltaprof v0.1.0 h1:UBqtjt0yZi4jTxqZmLAs34XG6ycS3vUTlhEUSq4NHLE= github.com/pyroscope-io/godeltaprof v0.1.0/go.mod h1:psMITXp90+8pFenXkKIpNhrfmI9saQnPbba27VIaiQE= github.com/pyroscope-io/otel-profiling-go v0.4.0 h1:Hk/rbUqOWoByoWy1tt4r5BX5xoKAvs5drr0511Ki8ic= diff --git a/nodebuilder/config.go b/nodebuilder/config.go index 3607aa593e..670bbf9bbd 100644 --- a/nodebuilder/config.go +++ b/nodebuilder/config.go @@ -25,6 +25,7 @@ type ConfigLoader func() (*Config, error) // Config is main configuration structure for a Node. // It combines configuration units for all Node subsystems. type Config struct { + Node node.Config Core core.Config State state.Config P2P p2p.Config @@ -39,6 +40,7 @@ type Config struct { // NOTE: Currently, configs are identical, but this will change. func DefaultConfig(tp node.Type) *Config { commonConfig := &Config{ + Node: node.DefaultConfig(tp), Core: core.DefaultConfig(), State: state.DefaultConfig(), P2P: p2p.DefaultConfig(tp), diff --git a/nodebuilder/gateway/config.go b/nodebuilder/gateway/config.go index 903a27489a..f85f207ceb 100644 --- a/nodebuilder/gateway/config.go +++ b/nodebuilder/gateway/config.go @@ -8,9 +8,10 @@ import ( ) type Config struct { - Address string - Port string - Enabled bool + Address string + Port string + Enabled bool + deprecatedEndpoints bool } func DefaultConfig() Config { diff --git a/nodebuilder/gateway/constructors.go b/nodebuilder/gateway/constructors.go index c28153b0a5..c771c12023 100644 --- a/nodebuilder/gateway/constructors.go +++ b/nodebuilder/gateway/constructors.go @@ -10,6 +10,7 @@ import ( // Handler constructs a new RPC Handler from the given services. func Handler( + cfg *Config, state state.Module, share share.Module, header header.Module, @@ -17,7 +18,7 @@ func Handler( serv *gateway.Server, ) { handler := gateway.NewHandler(state, share, header, daser) - handler.RegisterEndpoints(serv) + handler.RegisterEndpoints(serv, cfg.deprecatedEndpoints) handler.RegisterMiddleware(serv) } diff --git a/nodebuilder/gateway/flags.go b/nodebuilder/gateway/flags.go index cd13e47162..4d72a278e5 100644 --- a/nodebuilder/gateway/flags.go +++ b/nodebuilder/gateway/flags.go @@ -6,9 +6,10 @@ import ( ) var ( - enabledFlag = "gateway" - addrFlag = "gateway.addr" - portFlag = "gateway.port" + enabledFlag = "gateway" + addrFlag = "gateway.addr" + portFlag = "gateway.port" + deprecatedEndpoints = "gateway.deprecated-endpoints" ) // Flags gives a set of hardcoded node/gateway package flags. @@ -20,6 +21,11 @@ func Flags() *flag.FlagSet { false, "Enables the REST gateway", ) + flags.Bool( + deprecatedEndpoints, + false, + "Enables deprecated endpoints on the gateway. These will be removed in the next release.", + ) flags.String( addrFlag, "", @@ -40,6 +46,10 @@ func ParseFlags(cmd *cobra.Command, cfg *Config) { if cmd.Flags().Changed(enabledFlag) && err == nil { cfg.Enabled = enabled } + deprecatedEndpointsEnabled, err := cmd.Flags().GetBool(deprecatedEndpoints) + if cmd.Flags().Changed(deprecatedEndpoints) && err == nil { + cfg.deprecatedEndpoints = deprecatedEndpointsEnabled + } addr, port := cmd.Flag(addrFlag), cmd.Flag(portFlag) if !cfg.Enabled && (addr.Changed || port.Changed) { log.Warn("custom address or port provided without enabling gateway, setting config values") diff --git a/nodebuilder/gateway/module.go b/nodebuilder/gateway/module.go index b3070e01a6..b727f4c04d 100644 --- a/nodebuilder/gateway/module.go +++ b/nodebuilder/gateway/module.go @@ -21,8 +21,6 @@ func ConstructModule(tp node.Type, cfg *Config) fx.Option { if !cfg.Enabled { return fx.Options() } - // NOTE @distractedm1nd @renaynay: Remove whenever/if we decide to add auth to gateway - log.Warn("Gateway is enabled, however gateway endpoints are not authenticated. Use with caution!") baseComponents := fx.Options( fx.Supply(cfg), @@ -50,12 +48,13 @@ func ConstructModule(tp node.Type, cfg *Config) fx.Option { "gateway", baseComponents, fx.Invoke(func( + cfg *Config, state stateServ.Module, share shareServ.Module, header headerServ.Module, serv *gateway.Server, ) { - Handler(state, share, header, nil, serv) + Handler(cfg, state, share, header, nil, serv) }), ) default: diff --git a/nodebuilder/header/service.go b/nodebuilder/header/service.go index e6d7d46b8f..f410c04f04 100644 --- a/nodebuilder/header/service.go +++ b/nodebuilder/header/service.go @@ -37,7 +37,8 @@ func newHeaderService( sub libhead.Subscriber[*header.ExtendedHeader], p2pServer *p2p.ExchangeServer[*header.ExtendedHeader], ex libhead.Exchange[*header.ExtendedHeader], - store libhead.Store[*header.ExtendedHeader]) Module { + store libhead.Store[*header.ExtendedHeader], +) Module { return &Service{ syncer: syncer, sub: sub, @@ -66,7 +67,7 @@ func (s *Service) GetByHeight(ctx context.Context, height uint64) (*header.Exten return nil, err case uint64(head.Height()) == height: return head, nil - case uint64(head.Height()) < height: + case uint64(head.Height())+1 < height: return nil, fmt.Errorf("header: given height is from the future: "+ "networkHeight: %d, requestedHeight: %d", head.Height(), height) } diff --git a/nodebuilder/node.go b/nodebuilder/node.go index a6e3a3c3cc..19760831cf 100644 --- a/nodebuilder/node.go +++ b/nodebuilder/node.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "strings" - "time" "github.com/cristalhq/jwt" "github.com/ipfs/go-blockservice" @@ -95,7 +94,7 @@ func NewWithConfig(tp node.Type, network p2p.Network, store Store, cfg *Config, // Start launches the Node and all its components and services. func (n *Node) Start(ctx context.Context) error { - to := timeout(n.Type) + to := n.Config.Node.StartupTimeout ctx, cancel := context.WithTimeout(ctx, to) defer cancel() @@ -141,7 +140,7 @@ func (n *Node) Run(ctx context.Context) error { // Canceling the given context earlier 'ctx' unblocks the Stop and aborts graceful shutdown forcing // remaining Modules/Services to close immediately. func (n *Node) Stop(ctx context.Context) error { - to := timeout(n.Type) + to := n.Config.Node.ShutdownTimeout ctx, cancel := context.WithTimeout(ctx, to) defer cancel() @@ -183,14 +182,3 @@ func newNode(opts ...fx.Option) (*Node, error) { // lifecycleFunc defines a type for common lifecycle funcs. type lifecycleFunc func(context.Context) error - -var DefaultLifecycleTimeout = time.Minute * 2 - -func timeout(tp node.Type) time.Duration { - switch tp { - case node.Light: - return time.Second * 20 - default: - return DefaultLifecycleTimeout - } -} diff --git a/nodebuilder/node/config.go b/nodebuilder/node/config.go new file mode 100644 index 0000000000..e44fe2f014 --- /dev/null +++ b/nodebuilder/node/config.go @@ -0,0 +1,38 @@ +package node + +import ( + "fmt" + "time" +) + +var defaultLifecycleTimeout = time.Minute * 2 + +type Config struct { + StartupTimeout time.Duration + ShutdownTimeout time.Duration +} + +// DefaultConfig returns the default node configuration for a given node type. +func DefaultConfig(tp Type) Config { + var timeout time.Duration + switch tp { + case Light: + timeout = time.Second * 20 + default: + timeout = defaultLifecycleTimeout + } + return Config{ + StartupTimeout: timeout, + ShutdownTimeout: timeout, + } +} + +func (c *Config) Validate() error { + if c.StartupTimeout == 0 { + return fmt.Errorf("invalid startup timeout: %v", c.StartupTimeout) + } + if c.ShutdownTimeout == 0 { + return fmt.Errorf("invalid shutdown timeout: %v", c.ShutdownTimeout) + } + return nil +} diff --git a/nodebuilder/state/mocks/api.go b/nodebuilder/state/mocks/api.go index 814b9a0113..dbd1d5dabe 100644 --- a/nodebuilder/state/mocks/api.go +++ b/nodebuilder/state/mocks/api.go @@ -10,6 +10,7 @@ import ( math "cosmossdk.io/math" blob "github.com/celestiaorg/celestia-node/blob" + state "github.com/celestiaorg/celestia-node/state" types "github.com/cosmos/cosmos-sdk/types" types0 "github.com/cosmos/cosmos-sdk/x/staking/types" gomock "github.com/golang/mock/gomock" @@ -40,10 +41,10 @@ func (m *MockModule) EXPECT() *MockModuleMockRecorder { } // AccountAddress mocks base method. -func (m *MockModule) AccountAddress(arg0 context.Context) (types.Address, error) { +func (m *MockModule) AccountAddress(arg0 context.Context) (state.Address, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AccountAddress", arg0) - ret0, _ := ret[0].(types.Address) + ret0, _ := ret[0].(state.Address) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -70,7 +71,7 @@ func (mr *MockModuleMockRecorder) Balance(arg0 interface{}) *gomock.Call { } // BalanceForAddress mocks base method. -func (m *MockModule) BalanceForAddress(arg0 context.Context, arg1 types.Address) (*types.Coin, error) { +func (m *MockModule) BalanceForAddress(arg0 context.Context, arg1 state.Address) (*types.Coin, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "BalanceForAddress", arg0, arg1) ret0, _ := ret[0].(*types.Coin) diff --git a/nodebuilder/tests/api_test.go b/nodebuilder/tests/api_test.go index f740dc12fa..13cf083eee 100644 --- a/nodebuilder/tests/api_test.go +++ b/nodebuilder/tests/api_test.go @@ -2,8 +2,8 @@ package tests import ( "context" - "fmt" "testing" + "time" "github.com/filecoin-project/go-jsonrpc/auth" "github.com/libp2p/go-libp2p/core/host" @@ -19,6 +19,41 @@ import ( "github.com/celestiaorg/celestia-node/nodebuilder/tests/swamp" ) +func TestGetByHeight(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), swamp.DefaultTestTimeout) + t.Cleanup(cancel) + + sw := swamp.NewSwamp(t, swamp.WithBlockTime(time.Second)) + + // start a bridge node + bridge := sw.NewBridgeNode() + err := bridge.Start(ctx) + require.NoError(t, err) + + signer := bridge.AdminSigner + bridgeAddr := "http://" + bridge.RPCServer.ListenAddr() + + jwt, err := authtoken.NewSignedJWT(signer, []auth.Permission{"public", "read", "write", "admin"}) + require.NoError(t, err) + + client, err := client.NewClient(ctx, bridgeAddr, jwt) + require.NoError(t, err) + + // let a few blocks be produced + _, err = client.Header.WaitForHeight(ctx, 3) + require.NoError(t, err) + + networkHead, err := client.Header.NetworkHead(ctx) + require.NoError(t, err) + _, err = client.Header.GetByHeight(ctx, uint64(networkHead.Height()+1)) + require.Nil(t, err, "Requesting syncer.Head()+1 shouldn't return an error") + + networkHead, err = client.Header.NetworkHead(ctx) + require.NoError(t, err) + _, err = client.Header.GetByHeight(ctx, uint64(networkHead.Height()+2)) + require.ErrorContains(t, err, "given height is from the future") +} + // TestBlobRPC ensures that blobs can be submited via rpc func TestBlobRPC(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), swamp.DefaultTestTimeout) @@ -33,7 +68,6 @@ func TestBlobRPC(t *testing.T) { signer := bridge.AdminSigner bridgeAddr := "http://" + bridge.RPCServer.ListenAddr() - fmt.Println("bridgeAddr: ", bridgeAddr) jwt, err := authtoken.NewSignedJWT(signer, []auth.Permission{"public", "read", "write", "admin"}) require.NoError(t, err) diff --git a/nodebuilder/tests/fraud_test.go b/nodebuilder/tests/fraud_test.go index 2022107cb7..c6876cb624 100644 --- a/nodebuilder/tests/fraud_test.go +++ b/nodebuilder/tests/fraud_test.go @@ -84,8 +84,7 @@ func TestFraudProofBroadcasting(t *testing.T) { require.ErrorIs(t, err, context.DeadlineExceeded) syncCancel() - require.NoError(t, full.Stop(ctx)) - require.NoError(t, sw.RemoveNode(full, node.Full)) + sw.StopNode(ctx, full) full = sw.NewNodeWithStore(node.Full, store) diff --git a/nodebuilder/tests/swamp/swamp.go b/nodebuilder/tests/swamp/swamp.go index 5b99b577b1..1f21ac3bfa 100644 --- a/nodebuilder/tests/swamp/swamp.go +++ b/nodebuilder/tests/swamp/swamp.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "fmt" "net" + "sync" "testing" "time" @@ -16,6 +17,7 @@ import ( ma "github.com/multiformats/go-multiaddr" "github.com/stretchr/testify/require" "go.uber.org/fx" + "golang.org/x/exp/maps" "github.com/celestiaorg/celestia-app/test/util/testnode" apptypes "github.com/celestiaorg/celestia-app/x/blob/types" @@ -45,16 +47,16 @@ const DefaultTestTimeout = time.Minute * 5 // - Slices of created Bridge/Full/Light Nodes // - trustedHash taken from the CoreClient and shared between nodes type Swamp struct { - t *testing.T - Network mocknet.Mocknet - BridgeNodes []*nodebuilder.Node - FullNodes []*nodebuilder.Node - LightNodes []*nodebuilder.Node - comps *Config + t *testing.T + cfg *Config + Network mocknet.Mocknet ClientContext testnode.Context Accounts []string + nodesMu sync.Mutex + nodes map[*nodebuilder.Node]struct{} + genesis *header.ExtendedHeader } @@ -69,37 +71,38 @@ func NewSwamp(t *testing.T, options ...Option) *Swamp { option(ic) } - ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) - // Now, we are making an assumption that consensus mechanism is already tested out // so, we are not creating bridge nodes with each one containing its own core client // instead we are assigning all created BNs to 1 Core from the swamp cctx := core.StartTestNodeWithConfig(t, ic.TestConfig) swp := &Swamp{ t: t, + cfg: ic, Network: mocknet.New(), ClientContext: cctx, - comps: ic, Accounts: ic.Accounts, + nodes: map[*nodebuilder.Node]struct{}{}, } - swp.t.Cleanup(func() { - swp.stopAllNodes(ctx, swp.BridgeNodes, swp.FullNodes, swp.LightNodes) - }) - - swp.setupGenesis(ctx) + swp.t.Cleanup(swp.cleanup) + swp.setupGenesis() return swp } -// stopAllNodes goes through all received slices of Nodes and stops one-by-one -// this eliminates a manual clean-up in the test-cases itself in the end -func (s *Swamp) stopAllNodes(ctx context.Context, allNodes ...[]*nodebuilder.Node) { - for _, nodes := range allNodes { - for _, node := range nodes { - require.NoError(s.t, node.Stop(ctx)) - } - } +// cleanup frees up all the resources +// including stop of all created nodes +func (s *Swamp) cleanup() { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + require.NoError(s.t, s.Network.Close()) + + s.nodesMu.Lock() + defer s.nodesMu.Unlock() + maps.DeleteFunc(s.nodes, func(nd *nodebuilder.Node, _ struct{}) bool { + require.NoError(s.t, nd.Stop(ctx)) + return true + }) } // GetCoreBlockHashByHeight returns a tendermint block's hash by provided height @@ -158,7 +161,10 @@ func (s *Swamp) createPeer(ks keystore.Keystore) host.Host { // setupGenesis sets up genesis Header. // This is required to initialize and start correctly. -func (s *Swamp) setupGenesis(ctx context.Context) { +func (s *Swamp) setupGenesis() { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + // ensure core has surpassed genesis block s.WaitTillHeight(ctx, 2) @@ -180,7 +186,7 @@ func (s *Swamp) setupGenesis(ctx context.Context) { func (s *Swamp) DefaultTestConfig(tp node.Type) *nodebuilder.Config { cfg := nodebuilder.DefaultConfig(tp) - ip, port, err := net.SplitHostPort(s.comps.App.GRPC.Address) + ip, port, err := net.SplitHostPort(s.cfg.App.GRPC.Address) require.NoError(s.t, err) cfg.Core.IP = ip @@ -228,36 +234,29 @@ func (s *Swamp) NewNodeWithConfig(nodeType node.Type, cfg *nodebuilder.Config, o } // NewNodeWithStore creates a new instance of Node with predefined Store. -// Afterwards, the instance is stored in the swamp's Nodes' slice according to the -// node's type provided from the user. func (s *Swamp) NewNodeWithStore( - t node.Type, + tp node.Type, store nodebuilder.Store, options ...fx.Option, ) *nodebuilder.Node { - var n *nodebuilder.Node - signer := apptypes.NewKeyringSigner(s.ClientContext.Keyring, s.Accounts[0], s.ClientContext.ChainID) options = append(options, state.WithKeyringSigner(signer), ) - switch t { + switch tp { case node.Bridge: options = append(options, coremodule.WithClient(s.ClientContext.Client), ) - n = s.newNode(node.Bridge, store, options...) - s.BridgeNodes = append(s.BridgeNodes, n) - case node.Full: - n = s.newNode(node.Full, store, options...) - s.FullNodes = append(s.FullNodes, n) - case node.Light: - n = s.newNode(node.Light, store, options...) - s.LightNodes = append(s.LightNodes, n) + default: } - return n + nd := s.newNode(tp, store, options...) + s.nodesMu.Lock() + s.nodes[nd] = struct{}{} + s.nodesMu.Unlock() + return nd } func (s *Swamp) newNode(t node.Type, store nodebuilder.Store, options ...fx.Option) *nodebuilder.Node { @@ -284,43 +283,13 @@ func (s *Swamp) newNode(t node.Type, store nodebuilder.Store, options ...fx.Opti return node } -// RemoveNode removes a node from the swamp's node slice -// this allows reusage of the same var in the test scenario -// if the user needs to stop and start the same node -func (s *Swamp) RemoveNode(n *nodebuilder.Node, t node.Type) error { - var err error - switch t { - case node.Light: - s.LightNodes, err = s.remove(n, s.LightNodes) - return err - case node.Bridge: - s.BridgeNodes, err = s.remove(n, s.BridgeNodes) - return err - case node.Full: - s.FullNodes, err = s.remove(n, s.FullNodes) - return err - default: - return fmt.Errorf("no such type or node") - } -} - -func (s *Swamp) remove(rn *nodebuilder.Node, sn []*nodebuilder.Node) ([]*nodebuilder.Node, error) { - if len(sn) == 1 { - return nil, nil - } - - initSize := len(sn) - for i := 0; i < len(sn); i++ { - if sn[i] == rn { - sn = append(sn[:i], sn[i+1:]...) - i-- - } - } - - if initSize <= len(sn) { - return sn, fmt.Errorf("cannot delete the node") - } - return sn, nil +// StopNode stops the node and removes from Swamp. +// TODO(@Wondertan): For clean and symmetrical API, we may want to add StartNode. +func (s *Swamp) StopNode(ctx context.Context, nd *nodebuilder.Node) { + s.nodesMu.Lock() + delete(s.nodes, nd) + s.nodesMu.Unlock() + require.NoError(s.t, nd.Stop(ctx)) } // Connect allows to connect peers after hard disconnection. diff --git a/nodebuilder/tests/sync_test.go b/nodebuilder/tests/sync_test.go index 68f33bc61b..af311ba45c 100644 --- a/nodebuilder/tests/sync_test.go +++ b/nodebuilder/tests/sync_test.go @@ -121,8 +121,7 @@ func TestSyncStartStopLightWithBridge(t *testing.T) { require.EqualValues(t, h.Commit.BlockID.Hash, sw.GetCoreBlockHashByHeight(ctx, 30)) - require.NoError(t, light.Stop(ctx)) - require.NoError(t, sw.RemoveNode(light, node.Light)) + sw.StopNode(ctx, light) cfg = nodebuilder.DefaultConfig(node.Light) cfg.Header.TrustedPeers = append(cfg.Header.TrustedPeers, addrs[0].String()) diff --git a/share/eds/store.go b/share/eds/store.go index f01e96a24b..f0d02a1141 100644 --- a/share/eds/store.go +++ b/share/eds/store.go @@ -119,6 +119,10 @@ func (s *Store) Start(ctx context.Context) error { // start Store only if DagStore succeeds ctx, cancel := context.WithCancel(context.Background()) s.cancel = cancel + // initialize empty gc result to avoid panic on access + s.lastGCResult.Store(&dagstore.GCResult{ + Shards: make(map[shard.Key]error), + }) go s.gc(ctx) return nil } @@ -132,10 +136,6 @@ func (s *Store) Stop(context.Context) error { // gc periodically removes all inactive or errored shards. func (s *Store) gc(ctx context.Context) { ticker := time.NewTicker(s.gcInterval) - // initialize empty gc result to avoid panic on access - s.lastGCResult.Store(&dagstore.GCResult{ - Shards: make(map[shard.Key]error), - }) for { select { case <-ctx.Done(): diff --git a/state/address_test.go b/state/address_test.go new file mode 100644 index 0000000000..d701b38aa8 --- /dev/null +++ b/state/address_test.go @@ -0,0 +1,57 @@ +package state + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAddressMarshalling(t *testing.T) { + testCases := []struct { + name string + addressString string + addressFromStr func(string) (interface{}, error) + marshalJSON func(interface{}) ([]byte, error) + unmarshalJSON func([]byte) (interface{}, error) + }{ + { + name: "Account Address", + addressString: "celestia1377k5an3f94v6wyaceu0cf4nq6gk2jtpc46g7h", + addressFromStr: func(s string) (interface{}, error) { return sdk.AccAddressFromBech32(s) }, + marshalJSON: func(addr interface{}) ([]byte, error) { return addr.(sdk.AccAddress).MarshalJSON() }, + unmarshalJSON: func(b []byte) (interface{}, error) { + var addr sdk.AccAddress + err := addr.UnmarshalJSON(b) + return addr, err + }, + }, + { + name: "Validator Address", + addressString: "celestiavaloper1q3v5cugc8cdpud87u4zwy0a74uxkk6u4q4gx4p", + addressFromStr: func(s string) (interface{}, error) { return sdk.ValAddressFromBech32(s) }, + marshalJSON: func(addr interface{}) ([]byte, error) { return addr.(sdk.ValAddress).MarshalJSON() }, + unmarshalJSON: func(b []byte) (interface{}, error) { + var addr sdk.ValAddress + err := addr.UnmarshalJSON(b) + return addr, err + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + addr, err := tc.addressFromStr(tc.addressString) + require.NoError(t, err) + + addrBytes, err := tc.marshalJSON(addr) + assert.NoError(t, err) + assert.Equal(t, []byte("\""+tc.addressString+"\""), addrBytes) + + addrUnmarshalled, err := tc.unmarshalJSON(addrBytes) + assert.NoError(t, err) + assert.Equal(t, addr, addrUnmarshalled) + }) + } +} diff --git a/state/core_access.go b/state/core_access.go index ca0947166d..7b59f3e714 100644 --- a/state/core_access.go +++ b/state/core_access.go @@ -194,9 +194,9 @@ func (ca *CoreAccessor) SubmitPayForBlob( func (ca *CoreAccessor) AccountAddress(context.Context) (Address, error) { addr, err := ca.signer.GetSignerInfo().GetAddress() if err != nil { - return nil, err + return Address{nil}, err } - return addr, nil + return Address{addr}, nil } func (ca *CoreAccessor) Balance(ctx context.Context) (*Balance, error) { @@ -204,7 +204,7 @@ func (ca *CoreAccessor) Balance(ctx context.Context) (*Balance, error) { if err != nil { return nil, err } - return ca.BalanceForAddress(ctx, addr) + return ca.BalanceForAddress(ctx, Address{addr}) } func (ca *CoreAccessor) BalanceForAddress(ctx context.Context, addr Address) (*Balance, error) { diff --git a/state/integration_test.go b/state/integration_test.go index e7d2496397..8862de1bf8 100644 --- a/state/integration_test.go +++ b/state/integration_test.go @@ -110,7 +110,7 @@ func (s *IntegrationTestSuite) TestGetBalance() { require := s.Require() expectedBal := sdk.NewCoin(app.BondDenom, sdk.NewInt(int64(99999999999999999))) for _, acc := range s.accounts { - bal, err := s.accessor.BalanceForAddress(context.Background(), s.getAddress(acc)) + bal, err := s.accessor.BalanceForAddress(context.Background(), Address{s.getAddress(acc)}) require.NoError(err) require.Equal(&expectedBal, bal) } diff --git a/state/state.go b/state/state.go index 987a783239..d55bb6901c 100644 --- a/state/state.go +++ b/state/state.go @@ -1,6 +1,9 @@ package state import ( + "fmt" + "strings" + "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" coretypes "github.com/tendermint/tendermint/types" @@ -15,8 +18,11 @@ type Tx = coretypes.Tx // TxResponse is an alias to the TxResponse type from Cosmos-SDK. type TxResponse = sdk.TxResponse -// Address is an alias to the Address type from Cosmos-SDK. -type Address = sdk.Address +// Address is an alias to the Address type from Cosmos-SDK. It is embedded into a struct to provide +// a non-interface type for JSON serialization. +type Address struct { + sdk.Address +} // ValAddress is an alias to the ValAddress type from Cosmos-SDK. type ValAddress = sdk.ValAddress @@ -26,3 +32,27 @@ type AccAddress = sdk.AccAddress // Int is an alias to the Int type from Cosmos-SDK. type Int = math.Int + +func (a *Address) UnmarshalJSON(data []byte) error { + // To convert the string back to a concrete type, we have to determine the correct implementation + var addr AccAddress + addrString := strings.Trim(string(data), "\"") + addr, err := sdk.AccAddressFromBech32(addrString) + if err != nil { + // first check if it is a validator address and can be converted + valAddr, err := sdk.ValAddressFromBech32(addrString) + if err != nil { + return fmt.Errorf("address must be a valid account or validator address: %w", err) + } + a.Address = valAddr + return nil + } + + a.Address = addr + return nil +} + +func (a Address) MarshalJSON() ([]byte, error) { + // The address is marshaled into a simple string value + return []byte("\"" + a.Address.String() + "\""), nil +}