From 9484e15125d43cffedac5f284cfbd7397a5bb59c Mon Sep 17 00:00:00 2001 From: Ryan Date: Mon, 29 Jan 2024 06:58:00 -0600 Subject: [PATCH 1/2] feat(rpc/node): readiness check (#3118) Implements a readiness check on the node module. Question: I have set the permission level to public, but remember that we removed all public methods in favor of `read`. Is this fine or should this method also be `read`? ____________ Comes from a conversation with @joroshiba from Astria --------- Co-authored-by: ramin --- nodebuilder/module.go | 2 +- nodebuilder/node/admin.go | 8 ++++++++ nodebuilder/node/node.go | 8 ++++++++ nodebuilder/tests/api_test.go | 4 ++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/nodebuilder/module.go b/nodebuilder/module.go index 26dc1d9e33..f9948b2011 100644 --- a/nodebuilder/module.go +++ b/nodebuilder/module.go @@ -50,7 +50,6 @@ func ConstructModule(tp node.Type, network p2p.Network, cfg *Config, store Store state.ConstructModule(tp, &cfg.State, &cfg.Core), modhead.ConstructModule[*header.ExtendedHeader](tp, &cfg.Header), share.ConstructModule(tp, &cfg.Share), - rpc.ConstructModule(tp, &cfg.RPC), gateway.ConstructModule(tp, &cfg.Gateway), core.ConstructModule(tp, &cfg.Core), das.ConstructModule(tp, &cfg.DASer), @@ -58,6 +57,7 @@ func ConstructModule(tp node.Type, network p2p.Network, cfg *Config, store Store blob.ConstructModule(), node.ConstructModule(tp), prune.ConstructModule(tp), + rpc.ConstructModule(tp, &cfg.RPC), ) return fx.Module( diff --git a/nodebuilder/node/admin.go b/nodebuilder/node/admin.go index 8063835f1d..f71af66081 100644 --- a/nodebuilder/node/admin.go +++ b/nodebuilder/node/admin.go @@ -38,6 +38,14 @@ func (m *module) Info(context.Context) (Info, error) { }, nil } +func (m *module) Ready(context.Context) (bool, error) { + // Because the node uses FX to provide the RPC last, all services' lifecycles have been started by + // the point this endpoint is available. It is not 100% guaranteed at this point that all services + // are fully ready, but it is very high probability and all endpoints are available at this point + // regardless. + return true, nil +} + func (m *module) LogLevelSet(_ context.Context, name, level string) error { return logging.SetLogLevel(name, level) } diff --git a/nodebuilder/node/node.go b/nodebuilder/node/node.go index 18ce93615b..b2bc7dac31 100644 --- a/nodebuilder/node/node.go +++ b/nodebuilder/node/node.go @@ -14,6 +14,9 @@ type Module interface { // Info returns administrative information about the node. Info(context.Context) (Info, error) + // Ready returns true once the node's RPC is ready to accept requests. + Ready(context.Context) (bool, error) + // LogLevelSet sets the given component log level to the given level. LogLevelSet(ctx context.Context, name, level string) error @@ -28,6 +31,7 @@ var _ Module = (*API)(nil) type API struct { Internal struct { Info func(context.Context) (Info, error) `perm:"admin"` + Ready func(context.Context) (bool, error) `perm:"read"` LogLevelSet func(ctx context.Context, name, level string) error `perm:"admin"` AuthVerify func(ctx context.Context, token string) ([]auth.Permission, error) `perm:"admin"` AuthNew func(ctx context.Context, perms []auth.Permission) (string, error) `perm:"admin"` @@ -38,6 +42,10 @@ func (api *API) Info(ctx context.Context) (Info, error) { return api.Internal.Info(ctx) } +func (api *API) Ready(ctx context.Context) (bool, error) { + return api.Internal.Ready(ctx) +} + func (api *API) LogLevelSet(ctx context.Context, name, level string) error { return api.Internal.LogLevelSet(ctx, name, level) } diff --git a/nodebuilder/tests/api_test.go b/nodebuilder/tests/api_test.go index 960ce38875..751eaa63e5 100644 --- a/nodebuilder/tests/api_test.go +++ b/nodebuilder/tests/api_test.go @@ -48,6 +48,10 @@ func TestNodeModule(t *testing.T) { require.NoError(t, err) require.Equal(t, info.APIVersion, node.APIVersion) + ready, err := client.Node.Ready(ctx) + require.NoError(t, err) + require.True(t, ready) + perms, err := client.Node.AuthVerify(ctx, jwt) require.NoError(t, err) require.Equal(t, perms, adminPerms) From 1bae0600307a64bc0a94279ca50eab08ab9770b6 Mon Sep 17 00:00:00 2001 From: ramin Date: Mon, 29 Jan 2024 14:01:11 +0000 Subject: [PATCH 2/2] chore(spelling): fix all stray typos across comments and tests (#3142) Used a tool to find stray spellings then fix them, hopefully this will prevent future small PRs for little fixes emerging and we can encourage more significant / valuable PRs from external contributors --- libs/pidstore/pidstore_test.go | 2 +- nodebuilder/p2p/cmd/p2p.go | 2 +- nodebuilder/tests/api_test.go | 2 +- nodebuilder/tests/swamp/swamp.go | 4 ++-- share/getters/shrex_test.go | 4 ++-- share/ipld/corrupted_data_test.go | 8 ++++---- share/p2p/peers/manager.go | 2 +- state/core_access.go | 2 +- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/libs/pidstore/pidstore_test.go b/libs/pidstore/pidstore_test.go index 56f8a308ec..4a35783db3 100644 --- a/libs/pidstore/pidstore_test.go +++ b/libs/pidstore/pidstore_test.go @@ -22,7 +22,7 @@ func TestPutLoad(t *testing.T) { ds := sync.MutexWrap(datastore.NewMapDatastore()) - t.Run("unitialized-pidstore", func(t *testing.T) { + t.Run("uninitialized-pidstore", func(t *testing.T) { testPutLoad(ctx, ds, t) }) t.Run("initialized-pidstore", func(t *testing.T) { diff --git a/nodebuilder/p2p/cmd/p2p.go b/nodebuilder/p2p/cmd/p2p.go index 5951595fa4..8b44802947 100644 --- a/nodebuilder/p2p/cmd/p2p.go +++ b/nodebuilder/p2p/cmd/p2p.go @@ -224,7 +224,7 @@ var connectednessCmd = &cobra.Command{ var natStatusCmd = &cobra.Command{ Use: "nat-status", - Short: "Gets the currrent NAT status", + Short: "Gets the current NAT status", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { client, err := cmdnode.ParseClientFromCtx(cmd.Context()) diff --git a/nodebuilder/tests/api_test.go b/nodebuilder/tests/api_test.go index 751eaa63e5..a3b99a750b 100644 --- a/nodebuilder/tests/api_test.go +++ b/nodebuilder/tests/api_test.go @@ -93,7 +93,7 @@ func TestGetByHeight(t *testing.T) { require.ErrorContains(t, err, "given height is from the future") } -// TestBlobRPC ensures that blobs can be submited via rpc +// TestBlobRPC ensures that blobs can be submitted via rpc func TestBlobRPC(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), swamp.DefaultTestTimeout) t.Cleanup(cancel) diff --git a/nodebuilder/tests/swamp/swamp.go b/nodebuilder/tests/swamp/swamp.go index 617fe76151..9faf69744d 100644 --- a/nodebuilder/tests/swamp/swamp.go +++ b/nodebuilder/tests/swamp/swamp.go @@ -39,7 +39,7 @@ import ( var blackholeIP6 = net.ParseIP("100::") -// DefaultTestTimeout should be used as the default timout on all the Swamp tests. +// DefaultTestTimeout should be used as the default timeout on all the Swamp tests. // It's generously set to 5 minutes to give enough time for CI. const DefaultTestTimeout = time.Minute * 5 @@ -331,7 +331,7 @@ func (s *Swamp) Disconnect(t *testing.T, peerA, peerB *nodebuilder.Node) { // SetBootstrapper sets the given bootstrappers as the "bootstrappers" for the // Swamp test suite. Every new full or light node created on the suite afterwards // will automatically add the suite's bootstrappers as trusted peers to their config. -// NOTE: Bridge nodes do not automaatically add the bootstrappers as trusted peers. +// NOTE: Bridge nodes do not automatically add the bootstrappers as trusted peers. // NOTE: Use `NewNodeWithStore` to avoid this automatic configuration. func (s *Swamp) SetBootstrapper(t *testing.T, bootstrappers ...*nodebuilder.Node) { for _, trusted := range bootstrappers { diff --git a/share/getters/shrex_test.go b/share/getters/shrex_test.go index b625bb4c10..075735579b 100644 --- a/share/getters/shrex_test.go +++ b/share/getters/shrex_test.go @@ -121,7 +121,7 @@ func TestShrexGetter(t *testing.T) { t.Cleanup(cancel) // generate test data - eds, dah, maxNamesapce := generateTestEDS(t) + eds, dah, maxNamespace := generateTestEDS(t) eh := headertest.RandExtendedHeaderWithRoot(t, dah) require.NoError(t, edsStore.Put(ctx, dah.Hash(), eds)) peerManager.Validate(ctx, srvHost.ID(), shrexsub.Notification{ @@ -129,7 +129,7 @@ func TestShrexGetter(t *testing.T) { Height: 1, }) - namespace, err := addToNamespace(maxNamesapce, 1) + namespace, err := addToNamespace(maxNamespace, 1) require.NoError(t, err) // check for namespace to be not in root require.Len(t, ipld.FilterRootByNamespace(dah, namespace), 0) diff --git a/share/ipld/corrupted_data_test.go b/share/ipld/corrupted_data_test.go index d1d6e6b4d5..0d0af6dd35 100644 --- a/share/ipld/corrupted_data_test.go +++ b/share/ipld/corrupted_data_test.go @@ -25,7 +25,7 @@ func TestNamespaceHasher_CorruptedData(t *testing.T) { t.Cleanup(cancel) net := availability_test.NewTestDAGNet(ctx, t) - requestor := full.Node(net) + requester := full.Node(net) provider, mockBS := availability_test.MockNode(t, net) provider.Availability = full.TestAvailability(t, getters.NewIPLDGetter(provider.BlockService)) net.ConnectAll() @@ -37,15 +37,15 @@ func TestNamespaceHasher_CorruptedData(t *testing.T) { eh := headertest.RandExtendedHeaderWithRoot(t, root) getCtx, cancelGet := context.WithTimeout(ctx, sharesAvailableTimeout) t.Cleanup(cancelGet) - err := requestor.SharesAvailable(getCtx, eh) + err := requester.SharesAvailable(getCtx, eh) require.NoError(t, err) // clear the storage of the requester so that it must retrieve again, then start attacking // we reinitialize the node to clear the eds store - requestor = full.Node(net) + requester = full.Node(net) mockBS.Attacking = true getCtx, cancelGet = context.WithTimeout(ctx, sharesAvailableTimeout) t.Cleanup(cancelGet) - err = requestor.SharesAvailable(getCtx, eh) + err = requester.SharesAvailable(getCtx, eh) require.ErrorIs(t, err, share.ErrNotAvailable) } diff --git a/share/p2p/peers/manager.go b/share/p2p/peers/manager.go index 23fa18bcb2..964b022249 100644 --- a/share/p2p/peers/manager.go +++ b/share/p2p/peers/manager.go @@ -303,7 +303,7 @@ func (m *Manager) subscribeHeader(ctx context.Context, headerSub libhead.Subscri log.Debugw("stored initial height", "height", h.Height()) } - // update storeFrom if header heigh + // update storeFrom if header height m.storeFrom.Store(uint64(max(0, int(h.Height())-storedPoolsAmount))) log.Debugw("updated lowest stored height", "height", h.Height()) } diff --git a/state/core_access.go b/state/core_access.go index 358457b4f0..c3fbd4836a 100644 --- a/state/core_access.go +++ b/state/core_access.go @@ -181,7 +181,7 @@ func (ca *CoreAccessor) constructSignedTx( } // SubmitPayForBlob builds, signs, and synchronously submits a MsgPayForBlob. It blocks until the -// transaction is committed and returns the TxReponse. If gasLim is set to 0, the method will +// transaction is committed and returns the TxResponse. If gasLim is set to 0, the method will // automatically estimate the gas limit. If the fee is negative, the method will use the nodes min // gas price multiplied by the gas limit. func (ca *CoreAccessor) SubmitPayForBlob(