diff --git a/decentralized-api/internal/authzcache/cache.go b/decentralized-api/internal/authzcache/cache.go index 01c8717ee7..58ae5116b0 100644 --- a/decentralized-api/internal/authzcache/cache.go +++ b/decentralized-api/internal/authzcache/cache.go @@ -8,10 +8,17 @@ import ( "time" "github.com/productscience/inference/x/inference/types" + "golang.org/x/sync/singleflight" ) const authzCacheTTL = 2 * time.Minute +// authzQueryTimeout bounds the coalesced signer lookup. The queries run under a +// fresh context, not any single caller's, so one coalesced caller cancelling +// (client disconnect or its own deadline firing) does not fail every other +// caller that shares the singleflight result. +const authzQueryTimeout = 15 * time.Second + // SignerInfo holds address and pubkey for an authorized signer. type SignerInfo struct { Address string @@ -26,8 +33,13 @@ type cachedEntry struct { // AuthzCache caches authorized signers for granter addresses to avoid repeated chain queries. // Keys are cached with TTL since authz grants can change. type AuthzCache struct { - mu sync.RWMutex - cache map[string]*cachedEntry // "granterAddress|msgTypeUrl" -> entry + mu sync.RWMutex + cache map[string]*cachedEntry // "granterAddress|msgTypeUrl" -> entry + // sf coalesces concurrent misses for the same key into a single pair of chain + // queries, so an epoch-boundary/expiry burst does not fan out N identical + // lookups; combined with running the queries outside c.mu it stops one slow + // granter from throttling verification for every other granter. + sf singleflight.Group recorder cosmosclient.CosmosMessageClient } @@ -77,6 +89,7 @@ func (c *AuthzCache) GetPubKeyForSigner(ctx context.Context, granterAddress, sig func (c *AuthzCache) getSigners(ctx context.Context, granterAddress, msgTypeUrl string) ([]SignerInfo, error) { cacheKey := granterAddress + "|" + msgTypeUrl + // Fast path: cache hit under the read lock. c.mu.RLock() if entry, ok := c.cache[cacheKey]; ok && time.Now().Before(entry.expiresAt) { signers := entry.signers @@ -85,56 +98,81 @@ func (c *AuthzCache) getSigners(ctx context.Context, granterAddress, msgTypeUrl } c.mu.RUnlock() - c.mu.Lock() - defer c.mu.Unlock() + // Miss: coalesce concurrent fetches for this key into one pair of RPCs, run + // OUTSIDE c.mu so a slow granter cannot throttle verification for everyone. + // DoChan (not Do) so this caller can still abandon its wait via its own ctx + // without killing the shared query for everyone else. + ch := c.sf.DoChan(cacheKey, func() (interface{}, error) { + // Re-check: another goroutine may have populated the cache while we were + // becoming the singleflight leader. + c.mu.RLock() + if entry, ok := c.cache[cacheKey]; ok && time.Now().Before(entry.expiresAt) { + signers := entry.signers + c.mu.RUnlock() + return signers, nil + } + c.mu.RUnlock() - // Double-check after acquiring write lock - if entry, ok := c.cache[cacheKey]; ok && time.Now().Before(entry.expiresAt) { - return entry.signers, nil - } + logging.Debug("Fetching authz signers", types.Validation, + "granterAddress", granterAddress, "msgTypeUrl", msgTypeUrl) - logging.Debug("Fetching authz signers", types.Validation, - "granterAddress", granterAddress, "msgTypeUrl", msgTypeUrl) + queryClient := c.recorder.NewInferenceQueryClient() - queryClient := c.recorder.NewInferenceQueryClient() + // Run under a fresh, decoupled context so a coalesced caller's + // cancellation cannot fail everyone sharing this singleflight result. + qctx, cancel := context.WithTimeout(context.Background(), authzQueryTimeout) + defer cancel() - // Get grantees (warm keys) for this message type - grantees, err := queryClient.GranteesByMessageType(ctx, &types.QueryGranteesByMessageTypeRequest{ - GranterAddress: granterAddress, - MessageTypeUrl: msgTypeUrl, - }) - if err != nil { - return nil, err - } + // Get grantees (warm keys) for this message type + grantees, err := queryClient.GranteesByMessageType(qctx, &types.QueryGranteesByMessageTypeRequest{ + GranterAddress: granterAddress, + MessageTypeUrl: msgTypeUrl, + }) + if err != nil { + return nil, err + } - // Get granter's own public key - participant, err := queryClient.AccountByAddress(ctx, &types.QueryAccountByAddressRequest{ - Address: granterAddress, - }) - if err != nil { - return nil, err - } + // Get granter's own public key + participant, err := queryClient.AccountByAddress(qctx, &types.QueryAccountByAddressRequest{ + Address: granterAddress, + }) + if err != nil { + return nil, err + } - // Collect all signers: grantees + granter - signers := make([]SignerInfo, 0, len(grantees.Grantees)+1) - for _, grantee := range grantees.Grantees { + // Collect all signers: grantees + granter + signers := make([]SignerInfo, 0, len(grantees.Grantees)+1) + for _, grantee := range grantees.Grantees { + signers = append(signers, SignerInfo{ + Address: grantee.Address, + PubKey: grantee.PubKey, + }) + } signers = append(signers, SignerInfo{ - Address: grantee.Address, - PubKey: grantee.PubKey, + Address: granterAddress, + PubKey: participant.Pubkey, }) - } - signers = append(signers, SignerInfo{ - Address: granterAddress, - PubKey: participant.Pubkey, - }) - c.cache[cacheKey] = &cachedEntry{ - signers: signers, - expiresAt: time.Now().Add(authzCacheTTL), - } + c.mu.Lock() + c.cache[cacheKey] = &cachedEntry{ + signers: signers, + expiresAt: time.Now().Add(authzCacheTTL), + } + c.mu.Unlock() + + logging.Debug("Cached authz signers", types.Validation, + "granterAddress", granterAddress, "count", len(signers)) - logging.Debug("Cached authz signers", types.Validation, - "granterAddress", granterAddress, "count", len(signers)) + return signers, nil + }) - return signers, nil + select { + case res := <-ch: + if res.Err != nil { + return nil, res.Err + } + return res.Val.([]SignerInfo), nil + case <-ctx.Done(): + return nil, ctx.Err() + } } diff --git a/decentralized-api/internal/authzcache/cache_fetch_test.go b/decentralized-api/internal/authzcache/cache_fetch_test.go new file mode 100644 index 0000000000..c21e7d7aa9 --- /dev/null +++ b/decentralized-api/internal/authzcache/cache_fetch_test.go @@ -0,0 +1,146 @@ +package authzcache + +import ( + "context" + "fmt" + "sync" + "sync/atomic" + "testing" + "time" + + "decentralized-api/cosmosclient" + + "github.com/productscience/inference/x/inference/types" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" +) + +// fakeAuthzQueryClient implements only the two query methods getSigners uses; the +// embedded (nil) interface satisfies the rest and panics if anything else runs. +type fakeAuthzQueryClient struct { + types.QueryClient + granteesFn func(ctx context.Context) (*types.QueryGranteesByMessageTypeResponse, error) + accountFn func(ctx context.Context) (*types.QueryAccountByAddressResponse, error) +} + +func (f *fakeAuthzQueryClient) GranteesByMessageType(ctx context.Context, in *types.QueryGranteesByMessageTypeRequest, opts ...grpc.CallOption) (*types.QueryGranteesByMessageTypeResponse, error) { + return f.granteesFn(ctx) +} + +func (f *fakeAuthzQueryClient) AccountByAddress(ctx context.Context, in *types.QueryAccountByAddressRequest, opts ...grpc.CallOption) (*types.QueryAccountByAddressResponse, error) { + return f.accountFn(ctx) +} + +type fakeAuthzRecorder struct { + cosmosclient.CosmosMessageClient + qc types.QueryClient +} + +func (r *fakeAuthzRecorder) NewInferenceQueryClient() types.QueryClient { return r.qc } + +// TestGetSignersSingleflightsAndReleasesLock proves that concurrent misses for +// the same granter coalesce into one pair of chain queries, and that the cache +// mutex is not held across the queries, so a slow granter cannot throttle +// verification for every other granter. +func TestGetSignersSingleflightsAndReleasesLock(t *testing.T) { + var granteesCalls int32 + entered := make(chan struct{}, 1) + release := make(chan struct{}) + + qc := &fakeAuthzQueryClient{ + granteesFn: func(ctx context.Context) (*types.QueryGranteesByMessageTypeResponse, error) { + atomic.AddInt32(&granteesCalls, 1) + select { + case entered <- struct{}{}: + default: + } + <-release + return &types.QueryGranteesByMessageTypeResponse{Grantees: []*types.Grantee{{Address: "g1", PubKey: "pk1"}}}, nil + }, + accountFn: func(ctx context.Context) (*types.QueryAccountByAddressResponse, error) { + return &types.QueryAccountByAddressResponse{Pubkey: "granterpk"}, nil + }, + } + cache := NewAuthzCache(&fakeAuthzRecorder{qc: qc}) + + const N = 8 + var wg sync.WaitGroup + for i := 0; i < N; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _, _ = cache.GetPubKeys(context.Background(), "granter", "msg") + }() + } + + select { + case <-entered: + case <-time.After(2 * time.Second): + t.Fatal("no RPC started") + } + + locked := make(chan struct{}) + go func() { + cache.mu.RLock() + cache.mu.RUnlock() + close(locked) + }() + select { + case <-locked: + case <-time.After(time.Second): + t.Fatal("cache mutex held across the RPC - one slow granter would throttle everyone") + } + + close(release) + wg.Wait() + require.Equal(t, int32(1), atomic.LoadInt32(&granteesCalls), "singleflight must coalesce concurrent misses into one query pair") +} + +// TestGetPubKeyForSignerAndCaching covers correctness of the fetch path and that a +// single fetch populates the cache for all subsequent lookups within the TTL. +func TestGetPubKeyForSignerAndCaching(t *testing.T) { + var calls int32 + qc := &fakeAuthzQueryClient{ + granteesFn: func(ctx context.Context) (*types.QueryGranteesByMessageTypeResponse, error) { + atomic.AddInt32(&calls, 1) + return &types.QueryGranteesByMessageTypeResponse{Grantees: []*types.Grantee{{Address: "grantee1", PubKey: "gpk1"}}}, nil + }, + accountFn: func(ctx context.Context) (*types.QueryAccountByAddressResponse, error) { + return &types.QueryAccountByAddressResponse{Pubkey: "granterpk"}, nil + }, + } + cache := NewAuthzCache(&fakeAuthzRecorder{qc: qc}) + + pk, err := cache.GetPubKeyForSigner(context.Background(), "granter", "grantee1", "msg") + require.NoError(t, err) + require.Equal(t, "gpk1", pk) + + pk, err = cache.GetPubKeyForSigner(context.Background(), "granter", "granter", "msg") + require.NoError(t, err) + require.Equal(t, "granterpk", pk) + + pk, err = cache.GetPubKeyForSigner(context.Background(), "granter", "nobody", "msg") + require.NoError(t, err) + require.Equal(t, "", pk, "unknown signer returns empty, not an error") + + pks, err := cache.GetPubKeys(context.Background(), "granter", "msg") + require.NoError(t, err) + require.ElementsMatch(t, []string{"gpk1", "granterpk"}, pks) + + require.Equal(t, int32(1), atomic.LoadInt32(&calls), "all lookups served from a single cached fetch") +} + +func TestGetSignersPropagatesRPCError(t *testing.T) { + qc := &fakeAuthzQueryClient{ + granteesFn: func(ctx context.Context) (*types.QueryGranteesByMessageTypeResponse, error) { + return nil, fmt.Errorf("chain unavailable") + }, + accountFn: func(ctx context.Context) (*types.QueryAccountByAddressResponse, error) { + return &types.QueryAccountByAddressResponse{}, nil + }, + } + cache := NewAuthzCache(&fakeAuthzRecorder{qc: qc}) + + _, err := cache.GetPubKeys(context.Background(), "granter", "msg") + require.Error(t, err) +} diff --git a/decentralized-api/internal/epoch_group_cache.go b/decentralized-api/internal/epoch_group_cache.go index 247447aeee..a618ce1095 100644 --- a/decentralized-api/internal/epoch_group_cache.go +++ b/decentralized-api/internal/epoch_group_cache.go @@ -4,13 +4,21 @@ import ( "context" "decentralized-api/cosmosclient" "decentralized-api/logging" + "strconv" "sync" + "time" "github.com/productscience/inference/x/inference/types" + "golang.org/x/sync/singleflight" ) const maxCachedEpochs = 2 +// epochGroupQueryTimeout bounds the chain query for the current epoch group data. +// The cometbft/cosmos client sets no default timeout, so without this a stalled +// node would hang the fetch indefinitely. +const epochGroupQueryTimeout = 15 * time.Second + type cachedEpochData struct { data *types.EpochGroupData addressSet map[string]struct{} // O(1) lookup for active participants @@ -26,6 +34,13 @@ type EpochGroupDataCache struct { // Multi-epoch cache for GetEpochGroupData (max 2 epochs) epochCache map[uint64]*cachedEpochData + // Singleflight groups coalesce concurrent cache misses for the same epoch + // into a single chain query. Combined with running the RPC outside c.mu, + // this prevents an epoch-boundary thundering herd and stops one slow query + // from pinning the cache mutex and stalling every caller. + currentGroupSF singleflight.Group + epochGroupSF singleflight.Group + recorder cosmosclient.CosmosMessageClient } @@ -37,90 +52,134 @@ func NewEpochGroupDataCache(recorder cosmosclient.CosmosMessageClient) *EpochGro } func (c *EpochGroupDataCache) GetCurrentEpochGroupData(currentEpochIndex uint64) (*types.EpochGroupData, error) { + // Fast path: cache hit under the read lock. c.mu.RLock() if c.cachedGroupData != nil && c.cachedEpochIndex == currentEpochIndex { - defer c.mu.RUnlock() - return c.cachedGroupData, nil + data := c.cachedGroupData + c.mu.RUnlock() + return data, nil } c.mu.RUnlock() - c.mu.Lock() - defer c.mu.Unlock() + // Miss: coalesce concurrent fetches for this epoch into one RPC, and run the + // RPC WITHOUT holding c.mu so a slow/hung chain query cannot pin the mutex + // and stall every inference-validation caller. + key := strconv.FormatUint(currentEpochIndex, 10) + result, err, _ := c.currentGroupSF.Do(key, func() (interface{}, error) { + // Re-check: another goroutine may have populated the cache while we were + // becoming the singleflight leader. + c.mu.RLock() + prevIndex := c.cachedEpochIndex + if c.cachedGroupData != nil && c.cachedEpochIndex == currentEpochIndex { + data := c.cachedGroupData + c.mu.RUnlock() + return data, nil + } + c.mu.RUnlock() + + logging.Info("Fetching new epoch group data", types.Config, + "cachedEpochIndex", prevIndex, "currentEpochIndex", currentEpochIndex) - if c.cachedGroupData != nil && c.cachedEpochIndex == currentEpochIndex { - return c.cachedGroupData, nil - } + ctx, cancel := context.WithTimeout(context.Background(), epochGroupQueryTimeout) + defer cancel() + queryClient := c.recorder.NewInferenceQueryClient() + resp, err := queryClient.CurrentEpochGroupData(ctx, &types.QueryCurrentEpochGroupDataRequest{}) + if err != nil { + logging.Warn("Failed to query current epoch group data", types.Config, "error", err) + return nil, err + } + + data := &resp.EpochGroupData + c.mu.Lock() + c.cachedEpochIndex = currentEpochIndex + c.cachedGroupData = data + c.mu.Unlock() - logging.Info("Fetching new epoch group data", types.Config, - "cachedEpochIndex", c.cachedEpochIndex, "currentEpochIndex", currentEpochIndex) + logging.Info("Updated epoch group data cache", types.Config, + "epochIndex", currentEpochIndex, + "validationWeights", len(resp.EpochGroupData.ValidationWeights)) - queryClient := c.recorder.NewInferenceQueryClient() - req := &types.QueryCurrentEpochGroupDataRequest{} - resp, err := queryClient.CurrentEpochGroupData(context.Background(), req) + return data, nil + }) if err != nil { - logging.Warn("Failed to query current epoch group data", types.Config, "error", err) return nil, err } - - c.cachedEpochIndex = currentEpochIndex - c.cachedGroupData = &resp.EpochGroupData - - logging.Info("Updated epoch group data cache", types.Config, - "epochIndex", currentEpochIndex, - "validationWeights", len(resp.EpochGroupData.ValidationWeights)) - - return c.cachedGroupData, nil + return result.(*types.EpochGroupData), nil } // GetEpochGroupData returns epoch group data for specific epoch. // Uses cache, queries chain only on cache miss. Keeps max 2 epochs. func (c *EpochGroupDataCache) GetEpochGroupData(ctx context.Context, epochIndex uint64) (*types.EpochGroupData, error) { + // Fast path: cache hit under the read lock. c.mu.RLock() if cached, ok := c.epochCache[epochIndex]; ok { + data := cached.data c.mu.RUnlock() - return cached.data, nil + return data, nil } c.mu.RUnlock() - c.mu.Lock() - defer c.mu.Unlock() + // Miss: coalesce and run the RPC outside c.mu (see GetCurrentEpochGroupData). + // DoChan so this caller can abandon its wait via its own ctx without killing + // the shared query. + key := strconv.FormatUint(epochIndex, 10) + ch := c.epochGroupSF.DoChan(key, func() (interface{}, error) { + c.mu.RLock() + if cached, ok := c.epochCache[epochIndex]; ok { + data := cached.data + c.mu.RUnlock() + return data, nil + } + c.mu.RUnlock() - // Double-check after acquiring write lock - if cached, ok := c.epochCache[epochIndex]; ok { - return cached.data, nil - } + logging.Debug("Fetching epoch group data", types.Config, "epochIndex", epochIndex) + + // Run under a fresh, decoupled context (bounded) so a coalesced caller's + // cancellation cannot fail everyone sharing this singleflight result. + qctx, cancel := context.WithTimeout(context.Background(), epochGroupQueryTimeout) + defer cancel() + queryClient := c.recorder.NewInferenceQueryClient() + resp, err := queryClient.EpochGroupData(qctx, &types.QueryGetEpochGroupDataRequest{ + EpochIndex: epochIndex, + }) + if err != nil { + return nil, err + } - logging.Debug("Fetching epoch group data", types.Config, "epochIndex", epochIndex) + // Build address set for O(1) lookups. + addressSet := make(map[string]struct{}, len(resp.EpochGroupData.ValidationWeights)) + for _, vw := range resp.EpochGroupData.ValidationWeights { + addressSet[vw.MemberAddress] = struct{}{} + } + data := &resp.EpochGroupData - queryClient := c.recorder.NewInferenceQueryClient() - resp, err := queryClient.EpochGroupData(ctx, &types.QueryGetEpochGroupDataRequest{ - EpochIndex: epochIndex, - }) - if err != nil { - return nil, err - } + c.mu.Lock() + // Prune if needed (keep max 2 epochs). + if len(c.epochCache) >= maxCachedEpochs { + c.pruneOldest(epochIndex) + } + c.epochCache[epochIndex] = &cachedEpochData{ + data: data, + addressSet: addressSet, + } + c.mu.Unlock() - // Prune if needed (keep max 2 epochs) - if len(c.epochCache) >= maxCachedEpochs { - c.pruneOldest(epochIndex) - } + logging.Debug("Cached epoch group data", types.Config, + "epochIndex", epochIndex, + "participants", len(addressSet)) - // Build address set for O(1) lookups - addressSet := make(map[string]struct{}, len(resp.EpochGroupData.ValidationWeights)) - for _, vw := range resp.EpochGroupData.ValidationWeights { - addressSet[vw.MemberAddress] = struct{}{} - } + return data, nil + }) - c.epochCache[epochIndex] = &cachedEpochData{ - data: &resp.EpochGroupData, - addressSet: addressSet, + select { + case res := <-ch: + if res.Err != nil { + return nil, res.Err + } + return res.Val.(*types.EpochGroupData), nil + case <-ctx.Done(): + return nil, ctx.Err() } - - logging.Debug("Cached epoch group data", types.Config, - "epochIndex", epochIndex, - "participants", len(addressSet)) - - return &resp.EpochGroupData, nil } // IsActiveParticipant checks if address is active at given epoch. O(1) lookup. diff --git a/decentralized-api/internal/epoch_group_cache_test.go b/decentralized-api/internal/epoch_group_cache_test.go new file mode 100644 index 0000000000..278012f0c1 --- /dev/null +++ b/decentralized-api/internal/epoch_group_cache_test.go @@ -0,0 +1,125 @@ +package internal + +import ( + "context" + "sync" + "sync/atomic" + "testing" + "time" + + "decentralized-api/cosmosclient" + + "github.com/productscience/inference/x/inference/types" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" +) + +// fakeEpochQueryClient implements only the two query methods the cache uses; the +// embedded (nil) interface satisfies the rest and panics if anything else is called. +type fakeEpochQueryClient struct { + types.QueryClient + currentFn func(ctx context.Context) (*types.QueryCurrentEpochGroupDataResponse, error) + epochFn func(ctx context.Context, in *types.QueryGetEpochGroupDataRequest) (*types.QueryGetEpochGroupDataResponse, error) +} + +func (f *fakeEpochQueryClient) CurrentEpochGroupData(ctx context.Context, in *types.QueryCurrentEpochGroupDataRequest, opts ...grpc.CallOption) (*types.QueryCurrentEpochGroupDataResponse, error) { + return f.currentFn(ctx) +} + +func (f *fakeEpochQueryClient) EpochGroupData(ctx context.Context, in *types.QueryGetEpochGroupDataRequest, opts ...grpc.CallOption) (*types.QueryGetEpochGroupDataResponse, error) { + return f.epochFn(ctx, in) +} + +type fakeEpochRecorder struct { + cosmosclient.CosmosMessageClient + qc types.QueryClient +} + +func (r *fakeEpochRecorder) NewInferenceQueryClient() types.QueryClient { return r.qc } + +// TestGetCurrentEpochGroupDataSingleflightsAndReleasesLock proves the epoch-boundary +// fix: concurrent misses coalesce into a single chain query (singleflight), the +// query runs with a bounded context, and the cache mutex is NOT held across it, so +// one slow/hung query cannot stall every inference-validation caller. +func TestGetCurrentEpochGroupDataSingleflightsAndReleasesLock(t *testing.T) { + var calls int32 + var deadlineSeen atomic.Bool + entered := make(chan struct{}, 1) + release := make(chan struct{}) + + qc := &fakeEpochQueryClient{currentFn: func(ctx context.Context) (*types.QueryCurrentEpochGroupDataResponse, error) { + atomic.AddInt32(&calls, 1) + if _, ok := ctx.Deadline(); ok { + deadlineSeen.Store(true) + } + select { + case entered <- struct{}{}: + default: + } + <-release + return &types.QueryCurrentEpochGroupDataResponse{}, nil + }} + cache := NewEpochGroupDataCache(&fakeEpochRecorder{qc: qc}) + + const N = 8 + var wg sync.WaitGroup + errs := make(chan error, N) + for i := 0; i < N; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _, err := cache.GetCurrentEpochGroupData(7) + errs <- err + }() + } + + select { + case <-entered: + case <-time.After(2 * time.Second): + t.Fatal("no RPC started") + } + + // The mutex must not be held across the (still-blocked) RPC. + locked := make(chan struct{}) + go func() { + cache.mu.RLock() + cache.mu.RUnlock() + close(locked) + }() + select { + case <-locked: + case <-time.After(time.Second): + t.Fatal("cache mutex held across the RPC - a hung query would stall all callers") + } + + close(release) + wg.Wait() + close(errs) + for err := range errs { + require.NoError(t, err) + } + require.Equal(t, int32(1), atomic.LoadInt32(&calls), "singleflight must coalesce concurrent misses into one RPC") + require.True(t, deadlineSeen.Load(), "the chain query must run with a bounded (deadline) context") + + // A subsequent call for the same epoch is served from cache (no new RPC). + _, err := cache.GetCurrentEpochGroupData(7) + require.NoError(t, err) + require.Equal(t, int32(1), atomic.LoadInt32(&calls), "cache hit must not issue another RPC") +} + +// TestGetEpochGroupDataCaches confirms the multi-epoch path also queries once and +// then serves from cache. +func TestGetEpochGroupDataCaches(t *testing.T) { + var calls int32 + qc := &fakeEpochQueryClient{epochFn: func(ctx context.Context, in *types.QueryGetEpochGroupDataRequest) (*types.QueryGetEpochGroupDataResponse, error) { + atomic.AddInt32(&calls, 1) + return &types.QueryGetEpochGroupDataResponse{}, nil + }} + cache := NewEpochGroupDataCache(&fakeEpochRecorder{qc: qc}) + + for i := 0; i < 3; i++ { + _, err := cache.GetEpochGroupData(context.Background(), 42) + require.NoError(t, err) + } + require.Equal(t, int32(1), atomic.LoadInt32(&calls), "repeated gets for the same epoch must hit the cache") +} diff --git a/decentralized-api/internal/server/public/post_chat_errors_test.go b/decentralized-api/internal/server/public/post_chat_errors_test.go index 9afe90909e..9736ec2808 100644 --- a/decentralized-api/internal/server/public/post_chat_errors_test.go +++ b/decentralized-api/internal/server/public/post_chat_errors_test.go @@ -21,6 +21,7 @@ import ( "github.com/labstack/echo/v4" "github.com/productscience/inference/x/inference/calculations" "github.com/productscience/inference/x/inference/types" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -427,7 +428,7 @@ func TestHandleTransferRequest_CapacityLimit(t *testing.T) { mockCosmos := &cosmosclient.MockCosmosMessageClient{} mockCosmos.On("NewInferenceQueryClient").Return(types.NewQueryClient(conn)) - mockCosmos.On("Status", context.Background()).Return(status, nil) + mockCosmos.On("Status", mock.Anything).Return(status, nil) s := &Server{ e: e, diff --git a/decentralized-api/internal/server/public/post_chat_handler.go b/decentralized-api/internal/server/public/post_chat_handler.go index 24a2578cbf..d61fb8e0a2 100644 --- a/decentralized-api/internal/server/public/post_chat_handler.go +++ b/decentralized-api/internal/server/public/post_chat_handler.go @@ -324,6 +324,11 @@ func (s *Server) enforceTransferAgentAccess(taAddress string) error { return echo.NewHTTPError(http.StatusForbidden, "Transfer Agent not allowed") } +// statusQueryTimeout bounds the chain Status() lookups on the request path so a +// stalled node cannot hang request goroutines indefinitely (the cometbft client +// has no default timeout of its own). +const statusQueryTimeout = 10 * time.Second + func (s *Server) handleTransferRequest(ctx echo.Context, request *ChatRequest) (err error) { traceCtx, op := observability.Inference.StartTransfer( ctx.Request().Context(), request.OpenAiRequest.Model, request.RequesterAddress) @@ -358,7 +363,9 @@ func (s *Server) handleTransferRequest(ctx echo.Context, request *ChatRequest) ( return err } - status, err := s.recorder.Status(context.Background()) + statusCtx, cancel := context.WithTimeout(ctx.Request().Context(), statusQueryTimeout) + status, err := s.recorder.Status(statusCtx) + cancel() if err != nil { logging.Error("Failed to get status", types.Inferences, "error", err) return err @@ -775,7 +782,9 @@ func (s *Server) validateFullRequest(ctx echo.Context, request *ChatRequest) err } func (s *Server) validateTimestampNonce(request *ChatRequest) error { - status, err := s.recorder.Status(context.Background()) + statusCtx, cancel := context.WithTimeout(context.Background(), statusQueryTimeout) + status, err := s.recorder.Status(statusCtx) + cancel() if err != nil { logging.Error("Failed to get status", types.Inferences, "error", err) return err diff --git a/decentralized-api/internal/server/public/post_chat_status_timeout_test.go b/decentralized-api/internal/server/public/post_chat_status_timeout_test.go new file mode 100644 index 0000000000..78b468f0fa --- /dev/null +++ b/decentralized-api/internal/server/public/post_chat_status_timeout_test.go @@ -0,0 +1,43 @@ +package public + +import ( + "context" + "testing" + "time" + + coretypes "github.com/cometbft/cometbft/rpc/core/types" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "decentralized-api/cosmosclient" +) + +// TestValidateTimestampNoncePassesBoundedContext proves the request-path chain +// Status() lookup is now deadline-bounded (statusQueryTimeout) instead of using +// context.Background(), so a stalled node cannot hang the request goroutine. +func TestValidateTimestampNoncePassesBoundedContext(t *testing.T) { + configManager := newTestConfigManager(t) + status := &coretypes.ResultStatus{ + SyncInfo: coretypes.SyncInfo{ + LatestBlockHeight: 1, + LatestBlockTime: time.Now(), + }, + } + + var gotDeadline bool + mockCosmos := &cosmosclient.MockCosmosMessageClient{} + mockCosmos.On("Status", mock.Anything).Run(func(args mock.Arguments) { + _, gotDeadline = args.Get(0).(context.Context).Deadline() + }).Return(status, nil) + + s := &Server{recorder: mockCosmos, configManager: configManager} + + request := &ChatRequest{ + Timestamp: status.SyncInfo.LatestBlockTime.UnixNano(), + AuthKey: "f6-unique-authkey-validate-timestamp-nonce", + } + + require.NoError(t, s.validateTimestampNonce(request)) + require.True(t, gotDeadline, "Status must be called with a deadline-bounded context, not context.Background()") + mockCosmos.AssertExpectations(t) +} diff --git a/devshard/cmd/devshardctl/escrow_rotator.go b/devshard/cmd/devshardctl/escrow_rotator.go index 1300a419ab..98dee09038 100644 --- a/devshard/cmd/devshardctl/escrow_rotator.go +++ b/devshard/cmd/devshardctl/escrow_rotator.go @@ -25,8 +25,27 @@ var ( gatewayCreateRotationEscrow = (*Gateway).createRotationEscrow gatewayCreateDepletionEscrow func(*Gateway, context.Context, GatewaySettings, EscrowRotationModelSettings, string, uint64) (*CreateDevshardEscrowResult, error) gatewaySettleDevshardOnChain = (*Gateway).settleDevshardOnChain + + // rotationFinalizeTimeout bounds how long the process-wide finalizeMu is held + // across the network Finalize protocol during settlement. finalizeMu serialises + // every finalize (user /v1/finalize included), so without a cap a hung Finalize + // blocks all of them - and the admin settle path passes only the request + // context, which has no server-side deadline. A var so tests can shrink it. + rotationFinalizeTimeout = 2 * time.Minute ) +// finalizeUnderLock runs the session Finalize while holding the process-wide +// finalizeMu (which serialises all finalize operations), but bounds the call - +// and therefore the lock hold - with rotationFinalizeTimeout, so a hung finalize +// protocol cannot block every other /v1/finalize indefinitely. +func (g *Gateway) finalizeUnderLock(ctx context.Context, finalize func(context.Context) error) error { + g.finalizeMu.Lock() + defer g.finalizeMu.Unlock() + fctx, cancel := context.WithTimeout(ctx, rotationFinalizeTimeout) + defer cancel() + return finalize(fctx) +} + func (g *Gateway) startEscrowRotatorIfEnabled() { g.mu.Lock() defer g.mu.Unlock() @@ -412,14 +431,11 @@ func (g *Gateway) settleDevshardOnChain(ctx context.Context, id string, req admi } log.Printf("devshard_settle_key_loaded escrow=%s settler=%s key_env=%q", id, signer.Address(), privateKeyEnv) if rt.proxy.sm.Phase() != types.PhaseSettlement { - g.finalizeMu.Lock() log.Printf("gateway_finalize_lock_acquired escrow=%s path=rotation_settle", id) - if err := rt.session.Finalize(ctx); err != nil { - g.finalizeMu.Unlock() + if err := g.finalizeUnderLock(ctx, rt.session.Finalize); err != nil { log.Printf("devshard_settle_failed escrow=%s stage=finalize error=%q", id, err.Error()) return nil, err } - g.finalizeMu.Unlock() log.Printf("devshard_settle_finalize_completed escrow=%s phase=%s", id, sessionPhaseLabel(rt.proxy.sm.Phase())) } else { log.Printf("devshard_settle_finalize_skipped escrow=%s phase=%s", id, sessionPhaseLabel(rt.proxy.sm.Phase())) diff --git a/devshard/cmd/devshardctl/escrow_tx_concurrency_test.go b/devshard/cmd/devshardctl/escrow_tx_concurrency_test.go new file mode 100644 index 0000000000..1feaae96fa --- /dev/null +++ b/devshard/cmd/devshardctl/escrow_tx_concurrency_test.go @@ -0,0 +1,79 @@ +package main + +import ( + "context" + "fmt" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// TestScheduleDepletedEscrowReplacementBoundsConcurrency proves the escrowTxSem +// semaphore caps how many escrow-create chain transactions run at once. Without +// it, a mass-depletion event fans out one goroutine+tx per escrow simultaneously +// (per-id dedup does not bound the total), hammering the RPC node. We fire more +// jobs than slots and assert the observed concurrency never exceeds the cap. +func TestScheduleDepletedEscrowReplacementBoundsConcurrency(t *testing.T) { + const slots = 2 + const jobs = 6 + + var inFlight int32 + var maxSeen int32 + release := make(chan struct{}) + + oldCreate := gatewayCreateDepletionEscrow + gatewayCreateDepletionEscrow = func(_ *Gateway, _ context.Context, _ GatewaySettings, _ EscrowRotationModelSettings, _ string, _ uint64) (*CreateDevshardEscrowResult, error) { + cur := atomic.AddInt32(&inFlight, 1) + for { + m := atomic.LoadInt32(&maxSeen) + if cur <= m || atomic.CompareAndSwapInt32(&maxSeen, m, cur) { + break + } + } + <-release // hold the slot until the test lets go + atomic.AddInt32(&inFlight, -1) + return nil, fmt.Errorf("stub: hold slot then fail (no deactivate path)") + } + t.Cleanup(func() { gatewayCreateDepletionEscrow = oldCreate }) + + g := &Gateway{ + settings: GatewaySettings{ + EscrowRotation: EscrowRotationSettings{ + Enabled: true, + Models: []EscrowRotationModelSettings{ + {ModelID: "m", Amount: 1, PrivateKeyEnv: "KEY", TargetCount: 1}, + }, + }, + }, + escrowTxSem: make(chan struct{}, slots), + replenishmentInFlight: make(map[string]struct{}), + } + + for i := 0; i < jobs; i++ { + g.scheduleDepletedEscrowReplacement(fmt.Sprintf("id-%d", i), "m", "test") + } + + // The pool fills to exactly `slots`; the rest block on acquireEscrowTxSlot. + require.Eventually(t, func() bool { return atomic.LoadInt32(&inFlight) == slots }, + 2*time.Second, 5*time.Millisecond, "pool should fill to the cap") + time.Sleep(100 * time.Millisecond) // give any would-be extra worker time to (wrongly) start + require.LessOrEqual(t, atomic.LoadInt32(&maxSeen), int32(slots), + "semaphore must cap concurrent escrow-create txs") + + close(release) // drain the remaining jobs + require.Eventually(t, func() bool { return atomic.LoadInt32(&inFlight) == 0 }, + 3*time.Second, 5*time.Millisecond, "all jobs should complete") + require.Equal(t, int32(slots), atomic.LoadInt32(&maxSeen), "concurrency should reach, but not exceed, the cap") +} + +// TestEscrowTxSlotHelpersAreNilSafe guards the tests-and-literals path: a Gateway +// built without NewGateway has a nil escrowTxSem and must run unbounded, never panic. +func TestEscrowTxSlotHelpersAreNilSafe(t *testing.T) { + var g *Gateway + require.NotPanics(t, func() { g.acquireEscrowTxSlot(); g.releaseEscrowTxSlot() }) + + g = &Gateway{} + require.NotPanics(t, func() { g.acquireEscrowTxSlot(); g.releaseEscrowTxSlot() }) +} diff --git a/devshard/cmd/devshardctl/finalize_lock_test.go b/devshard/cmd/devshardctl/finalize_lock_test.go new file mode 100644 index 0000000000..d7638f333d --- /dev/null +++ b/devshard/cmd/devshardctl/finalize_lock_test.go @@ -0,0 +1,57 @@ +package main + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// TestFinalizeUnderLockBoundsHoldAndReleasesMutex proves a hung finalize protocol +// cannot hold the process-wide finalizeMu forever: the call is cut off by +// rotationFinalizeTimeout and the mutex is released, so other /v1/finalize +// requests are not blocked indefinitely. +func TestFinalizeUnderLockBoundsHoldAndReleasesMutex(t *testing.T) { + old := rotationFinalizeTimeout + rotationFinalizeTimeout = 50 * time.Millisecond + t.Cleanup(func() { rotationFinalizeTimeout = old }) + + g := &Gateway{} + + // A finalize that hangs until its context is cancelled (a stuck protocol). + hang := func(ctx context.Context) error { + <-ctx.Done() + return ctx.Err() + } + + start := time.Now() + err := g.finalizeUnderLock(context.Background(), hang) + require.Error(t, err, "a hung finalize must be cut off by the timeout") + require.Less(t, time.Since(start), 2*time.Second, "the lock hold must be bounded, not indefinite") + + // The mutex must have been released: a subsequent finalize proceeds promptly. + done := make(chan error, 1) + go func() { + done <- g.finalizeUnderLock(context.Background(), func(context.Context) error { return nil }) + }() + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(time.Second): + t.Fatal("finalizeMu was not released after a bounded finalize; it would block every /v1/finalize") + } +} + +// TestFinalizeUnderLockPassesBoundedContextAndResult confirms the happy path: +// the finalize receives a deadline-bounded context and its result propagates. +func TestFinalizeUnderLockPassesBoundedContextAndResult(t *testing.T) { + g := &Gateway{} + var gotDeadline bool + err := g.finalizeUnderLock(context.Background(), func(ctx context.Context) error { + _, gotDeadline = ctx.Deadline() + return nil + }) + require.NoError(t, err) + require.True(t, gotDeadline, "finalize must receive a deadline-bounded context") +} diff --git a/devshard/cmd/devshardctl/gateway.go b/devshard/cmd/devshardctl/gateway.go index 61859b58dc..f308be71e4 100644 --- a/devshard/cmd/devshardctl/gateway.go +++ b/devshard/cmd/devshardctl/gateway.go @@ -62,6 +62,7 @@ type Gateway struct { settlementInFlight map[string]struct{} replenishmentMu sync.Mutex replenishmentInFlight map[string]struct{} + escrowTxSem chan struct{} mu sync.Mutex roundRobinSeed atomic.Uint64 } @@ -489,6 +490,7 @@ func NewGateway(runtimes []*devshardRuntime, limiter *GatewayLimiter, defaultMod }, rotationFailures: make(map[string]struct{}), settlementInFlight: make(map[string]struct{}), + escrowTxSem: make(chan struct{}, maxConcurrentEscrowTx), } g.participantLimiter.SetMetrics(g.metrics) g.metrics.AttachGateway(g) @@ -592,6 +594,13 @@ const ( autoSettlementRetryInterval = 10 * time.Second autoSettlementAttemptTimeout = 5 * time.Minute autoSettlementMaxAttempts = 30 + + // maxConcurrentEscrowTx bounds how many escrow-rotation chain transactions + // (depletion replacements + auto-settlements) broadcast at once. Per-id dedup + // alone let a mass-depletion event fan out one chain tx per escrow at the same + // instant, hammering the RPC node into 503s and a self-amplifying crash loop; + // this caps the concurrent broadcasts across both schedulers. + maxConcurrentEscrowTx = 4 ) // checkBalances scans all active runtimes and deactivates any whose @@ -3419,6 +3428,24 @@ func (g *Gateway) retireRotatedDevshard(ctx context.Context, id, reason string, return true, nil } +// acquireEscrowTxSlot blocks until a concurrency slot for escrow-rotation chain +// transactions is free. It is nil-safe so a Gateway built without NewGateway +// (e.g. in tests) simply runs unbounded rather than panicking. +func (g *Gateway) acquireEscrowTxSlot() { + if g == nil || g.escrowTxSem == nil { + return + } + g.escrowTxSem <- struct{}{} +} + +// releaseEscrowTxSlot returns a slot acquired by acquireEscrowTxSlot. +func (g *Gateway) releaseEscrowTxSlot() { + if g == nil || g.escrowTxSem == nil { + return + } + <-g.escrowTxSem +} + func (g *Gateway) scheduleDepletedEscrowReplacement(id, modelID, reason string) { if g == nil { return @@ -3444,6 +3471,9 @@ func (g *Gateway) scheduleDepletedEscrowReplacement(id, modelID, reason string) g.replenishmentMu.Unlock() }() + g.acquireEscrowTxSlot() + defer g.releaseEscrowTxSlot() + ctx, cancel := context.WithTimeout(context.Background(), autoSettlementAttemptTimeout) defer cancel() if err := g.replaceDepletedEscrow(ctx, id, modelID, reason); err != nil { @@ -3524,9 +3554,11 @@ func (g *Gateway) scheduleAutoSettlement(id, reason string) { }() for attempt := 1; attempt <= autoSettlementMaxAttempts; attempt++ { + g.acquireEscrowTxSlot() ctx, cancel := context.WithTimeout(context.Background(), autoSettlementAttemptTimeout) result, err := gatewaySettleDevshardOnChain(g, ctx, id, adminSettleEscrowRequest{}) cancel() + g.releaseEscrowTxSlot() if err == nil { log.Printf("auto_settle_submitted escrow=%s reason=%s tx_hash=%s settler=%s", id, reason, result.TxHash, result.Settler) diff --git a/inference-chain/internal/rpc/client.go b/inference-chain/internal/rpc/client.go index 32f64c27f1..b5b0dd6bac 100644 --- a/inference-chain/internal/rpc/client.go +++ b/inference-chain/internal/rpc/client.go @@ -1,23 +1,65 @@ package rpc import ( + "context" "encoding/json" "errors" "fmt" + "net" "net/http" "strconv" + "time" ) const ( defaultTrustedBlocksPeriod = 1000 ) +// rpcHTTPClient bounds the transport-level phases (connect + wait-for-response +// headers) of every RPC fetch; net/http's default client sets no timeout at all. +// The per-request context deadlines below additionally bound the body read, so a +// node that sends headers and then stalls mid-body cannot hang the caller either. +var rpcHTTPClient = &http.Client{ + Transport: &http.Transport{ + DialContext: (&net.Dialer{Timeout: 10 * time.Second}).DialContext, + ResponseHeaderTimeout: 30 * time.Second, + }, +} + +// Per-request total deadlines (cover the whole request including the body read). +// status/block responses are tiny, so a short cap is safe; genesis can be large, +// so it gets a generous cap that still bounds a stalled transfer. Vars so tests +// can shrink them. +var ( + statusRequestTimeout = 30 * time.Second + genesisRequestTimeout = 5 * time.Minute +) + +// httpGetWithTimeout issues a GET whose context deadline bounds the entire +// request, including reading the body. The caller must close resp.Body and then +// call cancel (defer cancel() after the body is fully read/decoded). +func httpGetWithTimeout(url string, timeout time.Duration) (*http.Response, context.CancelFunc, error) { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + cancel() + return nil, nil, err + } + resp, err := rpcHTTPClient.Do(req) + if err != nil { + cancel() + return nil, nil, err + } + return resp, cancel, nil +} + func getStatus(rpcNode string) (*StatusResponse, error) { url := fmt.Sprintf("%s/status", rpcNode) - resp, err := http.Get(url) + resp, cancel, err := httpGetWithTimeout(url, statusRequestTimeout) if err != nil { return nil, err } + defer cancel() defer resp.Body.Close() if resp.StatusCode != http.StatusOK { @@ -73,10 +115,11 @@ func GetBlockHash(rpcNode string, height uint64) (string, error) { } url := fmt.Sprintf("%s/block?height=%d", rpcNode, height) - resp, err := http.Get(url) + resp, cancel, err := httpGetWithTimeout(url, statusRequestTimeout) if err != nil { return "", err } + defer cancel() defer resp.Body.Close() var block BlockResponse @@ -101,10 +144,11 @@ func GetNodeId(nodeRpcUrl string) (string, error) { func DownloadGenesis(nodeAddress string) (json.RawMessage, error) { url := fmt.Sprintf("%s/genesis", nodeAddress) - resp, err := http.Get(url) + resp, cancel, err := httpGetWithTimeout(url, genesisRequestTimeout) if err != nil { return nil, err } + defer cancel() defer resp.Body.Close() if resp.StatusCode != http.StatusOK { diff --git a/inference-chain/internal/rpc/client_test.go b/inference-chain/internal/rpc/client_test.go new file mode 100644 index 0000000000..0c0e19ce7a --- /dev/null +++ b/inference-chain/internal/rpc/client_test.go @@ -0,0 +1,171 @@ +package rpc + +import ( + "net" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestGetStatusParsesAndFailsOnNon200(t *testing.T) { + ok := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/status", r.URL.Path) + _, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":1,"result":{"node_info":{"id":"node-1"},"sync_info":{"latest_block_height":"1500","earliest_block_height":"1","earliest_block_hash":"E_HASH"}}}`)) + })) + defer ok.Close() + + status, err := getStatus(ok.URL) + require.NoError(t, err) + require.Equal(t, "1500", status.Result.SyncInfo.LatestBlockHeight) + require.Equal(t, "node-1", status.Result.NodeInfo.ID) + + id, err := GetNodeId(ok.URL) + require.NoError(t, err) + require.Equal(t, "node-1", id) + + bad := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer bad.Close() + _, err = getStatus(bad.URL) + require.Error(t, err) +} + +func TestGetBlockHash(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"result":{"block":{"header":{"height":"1500"}},"block_id":{"hash":"BLOCK_HASH"}}}`)) + })) + defer srv.Close() + + hash, err := GetBlockHash(srv.URL, 1500) + require.NoError(t, err) + require.Equal(t, "BLOCK_HASH", hash) + + _, err = GetBlockHash(srv.URL, 0) + require.Error(t, err, "height 0 must be rejected before any request") +} + +func TestGetTrustedBlockUsesEarliestUnderPeriod(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"result":{"sync_info":{"latest_block_height":"500","earliest_block_height":"1","earliest_block_hash":"E_HASH"}}}`)) + })) + defer srv.Close() + + height, hash, err := GetTrustedBlock(srv.URL, 1000) + require.NoError(t, err) + require.EqualValues(t, 1, height) + require.Equal(t, "E_HASH", hash) +} + +func TestGetTrustedBlockUsesOffsetOverPeriod(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/status": + _, _ = w.Write([]byte(`{"result":{"sync_info":{"latest_block_height":"5000","earliest_block_height":"1","earliest_block_hash":"E"}}}`)) + case "/block": + require.Equal(t, "4000", r.URL.Query().Get("height")) + _, _ = w.Write([]byte(`{"result":{"block":{"header":{"height":"4000"}},"block_id":{"hash":"H4000"}}}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + + height, hash, err := GetTrustedBlock(srv.URL, 1000) + require.NoError(t, err) + require.EqualValues(t, 4000, height) + require.Equal(t, "H4000", hash) +} + +func TestDownloadGenesis(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/genesis", r.URL.Path) + _, _ = w.Write([]byte(`{"result":{"genesis":{"chain_id":"gonka-test"}}}`)) + })) + defer srv.Close() + + genesis, err := DownloadGenesis(srv.URL) + require.NoError(t, err) + require.JSONEq(t, `{"chain_id":"gonka-test"}`, string(genesis)) + + bad := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadGateway) + })) + defer bad.Close() + _, err = DownloadGenesis(bad.URL) + require.Error(t, err) +} + +func TestGetBlockHashErrorsOnEmptyHeader(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"result":{"block":{"header":{"height":""}},"block_id":{"hash":""}}}`)) + })) + defer srv.Close() + _, err := GetBlockHash(srv.URL, 10) + require.Error(t, err) +} + +func TestGetTrustedBlockPropagatesStatusError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + _, _, err := GetTrustedBlock(srv.URL, 1000) + require.Error(t, err) +} + +// TestRPCClientBoundsStalledNode proves the shared client bounds a stalled node +// instead of hanging forever: with a short response-header timeout, a server that +// accepts the connection but never responds makes getStatus return an error +// quickly. Uses the real getStatus path with an injected short-timeout client. +func TestRPCClientBoundsStalledNode(t *testing.T) { + stalled := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-r.Context().Done() // never send response headers + })) + defer stalled.Close() + + old := rpcHTTPClient + rpcHTTPClient = &http.Client{Transport: &http.Transport{ + DialContext: (&net.Dialer{Timeout: time.Second}).DialContext, + ResponseHeaderTimeout: 100 * time.Millisecond, + }} + defer func() { rpcHTTPClient = old }() + + start := time.Now() + _, err := getStatus(stalled.URL) + require.Error(t, err) + require.Less(t, time.Since(start), 2*time.Second, "must be bounded by the timeout, not hang") +} + +// TestGetStatusBoundsBodyStall covers the case the response-header timeout misses: +// a node that returns 200 headers and then stalls mid-body. The per-request +// context deadline must still bound the read instead of hanging forever. +func TestGetStatusBoundsBodyStall(t *testing.T) { + old := statusRequestTimeout + statusRequestTimeout = 150 * time.Millisecond + defer func() { statusRequestTimeout = old }() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + if f, ok := w.(http.Flusher); ok { + f.Flush() // deliver headers, then never write the body + } + <-r.Context().Done() + })) + defer srv.Close() + + start := time.Now() + _, err := getStatus(srv.URL) + require.Error(t, err, "a node that sends headers then stalls the body must be bounded") + require.Less(t, time.Since(start), 2*time.Second, "must be cut off by the request deadline") +} + +func TestRPCHTTPClientIsBounded(t *testing.T) { + tr, ok := rpcHTTPClient.Transport.(*http.Transport) + require.True(t, ok, "rpcHTTPClient must use a configured *http.Transport") + require.NotNil(t, tr.DialContext, "connect must be bounded") + require.Greater(t, tr.ResponseHeaderTimeout, time.Duration(0), "response-header wait must be bounded") +} diff --git a/proxy/sidecar/main.go b/proxy/sidecar/main.go index 611046a813..1753304334 100644 --- a/proxy/sidecar/main.go +++ b/proxy/sidecar/main.go @@ -18,6 +18,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "time" ) @@ -1219,6 +1220,12 @@ func syncWhitelist() error { participants := pResp.ActiveParticipants.Participants totalParticipants = int64(len(participants)) + // Count DNS resolution failures (incremented by the workers below). This lets + // the guard tell a transient outage (lookups erroring) apart from a set that + // legitimately yields no public IPs (no URLs, or URLs that resolve to private + // IPs by design) - only the former should preserve state and retry. + var resolutionFailures int64 + // Worker Pool for DNS Resolution concurrency := 20 sem := make(chan struct{}, concurrency) @@ -1261,6 +1268,7 @@ func syncWhitelist() error { defer cancel() resolvedIPs, err := resolver.LookupIPAddr(ctx, host) if err != nil { + atomic.AddInt64(&resolutionFailures, 1) logMsg("Warning - could not resolve %s: %v", host, err) return // skippedResolution } @@ -1295,6 +1303,18 @@ func syncWhitelist() error { logMsg("Found %d unique public IPs to whitelist (Total scanned: %d).", len(allowed), totalParticipants) + // A transient DNS/resolution outage at the once-per-epoch sync moment can + // leave us with zero resolved IPs even though participants exist. Treat that + // as a sync failure (like the 404 path above) rather than overwriting the + // nginx whitelist with an empty set: return an error so the caller preserves + // the existing whitelist and does NOT advance BlockHeightSynced, so the next + // tick retries within the same epoch instead of de-whitelisting every + // validator (dropping their rate-limit exemption and accruing fail2ban bans) + // until the epoch flips. + if failures := atomic.LoadInt64(&resolutionFailures); failures > 0 && len(allowed) == 0 { + return fmt.Errorf("DNS resolution failed for %d participant(s) and no public IPs resolved - preserving existing whitelist state (transient resolution failure)", failures) + } + // Update In-Memory BanManager (so it doesn't ban these IPs) if GlobalBanManager != nil { GlobalBanManager.UpdateWhitelist(allowed) diff --git a/proxy/sidecar/whitelist_sync_test.go b/proxy/sidecar/whitelist_sync_test.go new file mode 100644 index 0000000000..559d22fce3 --- /dev/null +++ b/proxy/sidecar/whitelist_sync_test.go @@ -0,0 +1,63 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// TestSyncWhitelistPreservesStateWhenNoIPsResolve proves that a transient +// resolution outage (participants exist but none resolve to a public IP) makes +// syncWhitelist return an error instead of overwriting the nginx whitelist with +// an empty set. The error keeps the caller from advancing BlockHeightSynced, so +// the sync retries within the same epoch rather than de-whitelisting every +// validator until the epoch flips. +func TestSyncWhitelistPreservesStateWhenNoIPsResolve(t *testing.T) { + // Both hosts use the reserved .invalid TLD (RFC 6761), which never resolves. + const body = `{"active_participants":{"participants":[{"inference_url":"http://sync-fail-1.invalid"},{"inference_url":"http://sync-fail-2.invalid"}]}}` + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/epochs/current/participants" { + t.Errorf("unexpected path %s", r.URL.Path) + } + _, _ = w.Write([]byte(body)) + })) + defer srv.Close() + + oldURL := ApiUrl + ApiUrl = srv.URL + defer func() { ApiUrl = oldURL }() + + err := syncWhitelist() + if err == nil { + t.Fatal("expected syncWhitelist to fail when no participant IPs resolve; a nil error would wipe the whitelist") + } + if !strings.Contains(err.Error(), "DNS resolution failed for 2") { + t.Fatalf("expected a 'DNS resolution failed for 2' preservation error, got: %v", err) + } +} + +// TestSyncWhitelistDoesNotPreserveWhenNoResolutionFailures proves the guard keys +// off DNS failures, not merely "zero public IPs": a participant whose URL is a +// private literal IP resolves without error and is dropped as private, yielding +// zero public IPs but zero resolution failures. The preservation guard must NOT +// fire (that would loop forever on an all-private set); any error here must come +// from the downstream nginx write, not the guard. +func TestSyncWhitelistDoesNotPreserveWhenNoResolutionFailures(t *testing.T) { + const body = `{"active_participants":{"participants":[{"inference_url":"http://10.1.2.3:8080"}]}}` + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(body)) + })) + defer srv.Close() + + oldURL := ApiUrl + ApiUrl = srv.URL + defer func() { ApiUrl = oldURL }() + + err := syncWhitelist() + if err != nil && strings.Contains(err.Error(), "resolution failure") { + t.Fatalf("preservation guard fired on an all-private set (would loop forever): %v", err) + } +}