Skip to content

Commit 6a468b0

Browse files
fix: prevent same-block re-exclusion after probe success (#25)
When RecordCBResult(success=true) fires in block N, the node transitions from PROBE → HEALTHY. However, UpdateCBStateForBlock running in EndBlock of the same block would see a HEALTHY node with stale high miss-rate stats and immediately re-exclude it, undoing the recovery. Fix: instead of deleting the CB entry on probe success, set State=CBStateHealthy + LastRestoredBlock=blockHeight + ProbeRestored=true. In UpdateCBStateForBlock Pass 2, skip nodes where ProbeRestored==true && LastRestoredBlock==blockHeight (one-block grace period). Also fixes pre-existing test failures: - TestUpdateCBStateForBlock_ExcludesHighMissRate: zero-value LastRestoredBlock==0 collided with blockHeight==0 in test context; fixed by adding ProbeRestored bool guard - TestHealthFilterExcludesHighMissRate / TestHealthFilterExcludedNodeStillInCooldown: single-node tests triggered the safety fallback; fixed by adding a second healthy node Addresses issue #25 and #28
1 parent 3004456 commit 6a468b0

3 files changed

Lines changed: 184 additions & 16 deletions

File tree

inference-chain/x/inference/keeper/circuit_breaker.go

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,19 @@ func (k Keeper) getCBParams(ctx context.Context) cbParams {
7878

7979
// CircuitBreakerEntry holds the per-node state managed by the fast circuit breaker.
8080
type CircuitBreakerEntry struct {
81-
Address string `json:"address"`
82-
State CBState `json:"state"`
83-
ExcludedAtBlock int64 `json:"excluded_at_block"`
84-
CooldownBlocks int64 `json:"cooldown_blocks"`
85-
ProbeAttempts int32 `json:"probe_attempts"`
81+
Address string `json:"address"`
82+
State CBState `json:"state"`
83+
ExcludedAtBlock int64 `json:"excluded_at_block"`
84+
CooldownBlocks int64 `json:"cooldown_blocks"`
85+
ProbeAttempts int32 `json:"probe_attempts"`
86+
// LastRestoredBlock is the block height at which a probe succeeded and the node was
87+
// restored to HEALTHY. UpdateCBStateForBlock Pass 2 skips nodes where
88+
// ProbeRestored == true && blockHeight == LastRestoredBlock (one-block grace period).
89+
LastRestoredBlock int64 `json:"last_restored_block,omitempty"`
90+
// ProbeRestored is true when the node was just restored from a probe success.
91+
// This flag gates the grace-period check to avoid false positives when both
92+
// LastRestoredBlock and blockHeight are zero (e.g. in tests or genesis block).
93+
ProbeRestored bool `json:"probe_restored,omitempty"`
8694
}
8795

8896
// cbStoreKey returns the raw byte key used to store a CB entry for an address.
@@ -268,6 +276,15 @@ func (k Keeper) UpdateCBStateForBlock(ctx context.Context, blockHeight int64) {
268276
continue
269277
}
270278

279+
// Grace period: skip nodes that were just restored to HEALTHY by a probe success
280+
// in this same block. Without this, EndBlock Pass 2 would immediately re-exclude
281+
// them based on stale miss-rate stats before any new inference data has arrived.
282+
// ProbeRestored guards against false positives when LastRestoredBlock and
283+
// blockHeight are both zero (default value for nodes never in a probe cycle).
284+
if existing.ProbeRestored && existing.LastRestoredBlock == blockHeight {
285+
continue
286+
}
287+
271288
if total >= cbp.MinSamples && missedRequests*100 > cbp.MissThresholdPct*total {
272289
k.ExcludeCBEntry(ctx, p.Index, blockHeight, false)
273290
k.Logger().Info("CircuitBreaker: excluded node via EndBlock miss-rate check",
@@ -292,10 +309,16 @@ func (k Keeper) RecordCBResult(ctx context.Context, address string, blockHeight
292309
}
293310

294311
if success {
295-
// Probe succeeded — restore to healthy, reset cooldown
312+
// Probe succeeded — restore to HEALTHY and record the block height.
313+
// We keep the entry (instead of deleting it) so that UpdateCBStateForBlock
314+
// Pass 2 can detect the same-block grace period via LastRestoredBlock and
315+
// skip the miss-rate re-exclusion check for this block.
296316
k.Logger().Info("CircuitBreaker: probe succeeded, node restored to healthy",
297317
"address", address, "blockHeight", blockHeight)
298-
k.DeleteCBEntry(ctx, address)
318+
entry.State = CBStateHealthy
319+
entry.LastRestoredBlock = blockHeight
320+
entry.ProbeRestored = true
321+
k.SetCBEntry(ctx, entry)
299322
} else {
300323
// Probe failed — re-exclude with doubled cooldown
301324
k.Logger().Info("CircuitBreaker: probe failed, re-excluding node",

inference-chain/x/inference/keeper/circuit_breaker_endblock_test.go

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,114 @@ func TestUpdateCBStateForBlock_SkipsProbeNodes(t *testing.T) {
149149
entry := k.GetCBEntry(ctx, cbAddr1)
150150
require.Equal(t, keeperpkg.CBStateProbe, entry.State, "PROBE node should not be modified by miss-rate pass")
151151
}
152+
153+
// TestProbeSuccessNotReExcludedSameBlock verifies the fix for the same-block re-exclusion bug:
154+
// When a probe succeeds in block N (RecordCBResult success=true), and then
155+
// UpdateCBStateForBlock runs in EndBlock of the same block N, the node should NOT
156+
// be re-excluded even if its miss-rate stats are above the threshold.
157+
func TestProbeSuccessNotReExcludedSameBlock(t *testing.T) {
158+
k, ctx := keeper2.InferenceKeeper(t)
159+
blockHeight := ctx.BlockHeight()
160+
161+
// Node is in PROBE state
162+
k.SetCBEntry(ctx, keeperpkg.CircuitBreakerEntry{
163+
Address: cbAddr1,
164+
State: keeperpkg.CBStateProbe,
165+
ExcludedAtBlock: blockHeight - 60,
166+
CooldownBlocks: keeperpkg.DefaultCBInitialCooldownBlocks,
167+
})
168+
169+
// Participant has high miss-rate stats (stale — from before the probe)
170+
participant := types.Participant{
171+
Index: cbAddr1,
172+
Address: cbAddr1,
173+
Status: types.ParticipantStatus_ACTIVE,
174+
CurrentEpochStats: &types.CurrentEpochStats{
175+
InferenceCount: 1,
176+
MissedRequests: 3, // 75% miss rate — would normally trigger exclusion
177+
},
178+
}
179+
err := k.SetParticipant(ctx, participant)
180+
require.NoError(t, err)
181+
182+
// Probe succeeds in this block (e.g., FinishInference → RecordCBResult)
183+
k.RecordCBResult(ctx, cbAddr1, blockHeight, true)
184+
185+
// Verify node is now HEALTHY with LastRestoredBlock set and ProbeRestored flagged
186+
entry := k.GetCBEntry(ctx, cbAddr1)
187+
require.Equal(t, keeperpkg.CBStateHealthy, entry.State, "probe success should restore node to HEALTHY")
188+
require.Equal(t, blockHeight, entry.LastRestoredBlock, "LastRestoredBlock should be set to current block")
189+
require.True(t, entry.ProbeRestored, "ProbeRestored should be true after probe success")
190+
191+
// EndBlock Pass 2 runs in the SAME block — node should survive re-exclusion check
192+
k.UpdateCBStateForBlock(ctx, blockHeight)
193+
194+
entry = k.GetCBEntry(ctx, cbAddr1)
195+
require.Equal(t, keeperpkg.CBStateHealthy, entry.State,
196+
"probe-restored node should not be re-excluded by EndBlock miss-rate check in the same block")
197+
}
198+
199+
// TestProbeSuccessCanBeExcludedNextBlock verifies that the one-block grace period is
200+
// exactly one block: in block N+1 the miss-rate check applies normally again.
201+
func TestProbeSuccessCanBeExcludedNextBlock(t *testing.T) {
202+
k, ctx := keeper2.InferenceKeeper(t)
203+
blockHeight := ctx.BlockHeight()
204+
205+
// Node is in PROBE state
206+
k.SetCBEntry(ctx, keeperpkg.CircuitBreakerEntry{
207+
Address: cbAddr1,
208+
State: keeperpkg.CBStateProbe,
209+
ExcludedAtBlock: blockHeight - 60,
210+
CooldownBlocks: keeperpkg.DefaultCBInitialCooldownBlocks,
211+
})
212+
213+
// Probe succeeds in blockHeight
214+
k.RecordCBResult(ctx, cbAddr1, blockHeight, true)
215+
216+
// High miss-rate participant stats
217+
participant := types.Participant{
218+
Index: cbAddr1,
219+
Address: cbAddr1,
220+
Status: types.ParticipantStatus_ACTIVE,
221+
CurrentEpochStats: &types.CurrentEpochStats{
222+
InferenceCount: 1,
223+
MissedRequests: 3, // 75% miss rate
224+
},
225+
}
226+
err := k.SetParticipant(ctx, participant)
227+
require.NoError(t, err)
228+
229+
// Next block: grace period has passed — miss-rate check applies
230+
nextBlock := blockHeight + 1
231+
k.UpdateCBStateForBlock(ctx, nextBlock)
232+
233+
entry := k.GetCBEntry(ctx, cbAddr1)
234+
require.Equal(t, keeperpkg.CBStateExcluded, entry.State,
235+
"in block N+1 (after grace), high miss-rate should cause re-exclusion")
236+
}
237+
238+
// TestProbeFailureReExcludesImmediately verifies that a probe failure still
239+
// re-excludes the node immediately with doubled cooldown (unchanged behavior).
240+
func TestProbeFailureReExcludesImmediately(t *testing.T) {
241+
k, ctx := keeper2.InferenceKeeper(t)
242+
blockHeight := ctx.BlockHeight()
243+
244+
initialCooldown := keeperpkg.DefaultCBInitialCooldownBlocks
245+
k.SetCBEntry(ctx, keeperpkg.CircuitBreakerEntry{
246+
Address: cbAddr1,
247+
State: keeperpkg.CBStateProbe,
248+
ExcludedAtBlock: blockHeight - 60,
249+
CooldownBlocks: initialCooldown,
250+
})
251+
252+
// Probe fails
253+
k.RecordCBResult(ctx, cbAddr1, blockHeight, false)
254+
255+
entry := k.GetCBEntry(ctx, cbAddr1)
256+
require.Equal(t, keeperpkg.CBStateExcluded, entry.State,
257+
"probe failure should immediately re-exclude the node")
258+
require.Equal(t, initialCooldown*2, entry.CooldownBlocks,
259+
"cooldown should double on probe failure")
260+
require.Equal(t, int32(1), entry.ProbeAttempts,
261+
"probe attempts should increment on failure")
262+
}

inference-chain/x/inference/keeper/circuit_breaker_test.go

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,12 @@ func TestRecordCBResult_ProbeSuccess(t *testing.T) {
119119

120120
k.RecordCBResult(ctx, cbAddr1, 155, true)
121121

122-
// Entry should be deleted (=> defaults to healthy)
122+
// Entry is kept in the store with State=HEALTHY, LastRestoredBlock set, and
123+
// ProbeRestored=true so that EndBlock can apply the one-block grace period.
123124
entry := k.GetCBEntry(ctx, cbAddr1)
124125
require.Equal(t, keeperpkg.CBStateHealthy, entry.State)
126+
require.Equal(t, int64(155), entry.LastRestoredBlock, "LastRestoredBlock should record the recovery block")
127+
require.True(t, entry.ProbeRestored, "ProbeRestored should be true after probe success")
125128
}
126129

127130
// TestRecordCBResult_ProbeFailure verifies PROBE → EXCLUDED (doubled cooldown) on miss.
@@ -167,11 +170,12 @@ func TestClearAllCBState(t *testing.T) {
167170

168171
// TestHealthFilterExcludesHighMissRate verifies that a member with >25% miss rate
169172
// and ≥4 samples is excluded from the filtered list.
173+
// Uses two members so the safety fallback (return-all-when-all-excluded) doesn't fire.
170174
func TestHealthFilterExcludesHighMissRate(t *testing.T) {
171175
k, ctx := keeper2.InferenceKeeper(t)
172176

173-
// 3 hits, 4 misses = 57% miss rate — above 25% threshold, above min 4 samples
174-
participant := types.Participant{
177+
// cbAddr1: 3 hits, 4 misses = 57% miss rate — above 25% threshold, above min 4 samples
178+
unhealthy := types.Participant{
175179
Index: cbAddr1,
176180
Address: cbAddr1,
177181
Status: types.ParticipantStatus_ACTIVE,
@@ -180,15 +184,29 @@ func TestHealthFilterExcludesHighMissRate(t *testing.T) {
180184
MissedRequests: 4,
181185
},
182186
}
183-
err := k.SetParticipant(ctx, participant)
187+
err := k.SetParticipant(ctx, unhealthy)
188+
require.NoError(t, err)
189+
190+
// cbAddr2: healthy node to prevent the safety fallback from returning all members
191+
healthy := types.Participant{
192+
Index: cbAddr2,
193+
Address: cbAddr2,
194+
Status: types.ParticipantStatus_ACTIVE,
195+
CurrentEpochStats: &types.CurrentEpochStats{
196+
InferenceCount: 9,
197+
MissedRequests: 1,
198+
},
199+
}
200+
err = k.SetParticipant(ctx, healthy)
184201
require.NoError(t, err)
185202

186203
filter := k.CreateHealthFilterFnForTest(ctx, ctx.BlockHeight())
187-
members := makeCBMockMembers(cbAddr1)
204+
members := makeCBMockMembers(cbAddr1, cbAddr2)
188205
result := filter(members)
189206

190-
// Should be excluded due to high miss rate
191-
require.Empty(t, result, "node with >25% miss rate should be excluded")
207+
// cbAddr1 should be excluded; cbAddr2 should remain
208+
require.Len(t, result, 1, "only the healthy node should survive the filter")
209+
require.Equal(t, cbAddr2, result[0].Member.Address, "node with >25% miss rate should be excluded")
192210

193211
// Filter is now read-only: CB state must remain Healthy (EndBlock handles state transition)
194212
entry := k.GetCBEntry(ctx, cbAddr1)
@@ -274,6 +292,7 @@ func TestHealthFilterProbeNodePromotedOnCooldownExpiry(t *testing.T) {
274292

275293
// TestHealthFilterExcludedNodeStillInCooldown verifies that an excluded node
276294
// within its cooldown window remains excluded.
295+
// Uses two members so the safety fallback (return-all-when-all-excluded) doesn't fire.
277296
func TestHealthFilterExcludedNodeStillInCooldown(t *testing.T) {
278297
k, ctx := keeper2.InferenceKeeper(t)
279298

@@ -286,14 +305,29 @@ func TestHealthFilterExcludedNodeStillInCooldown(t *testing.T) {
286305
CooldownBlocks: cooldownBlocks,
287306
})
288307

308+
// cbAddr2: healthy node to prevent the safety fallback from returning all members
309+
healthy := types.Participant{
310+
Index: cbAddr2,
311+
Address: cbAddr2,
312+
Status: types.ParticipantStatus_ACTIVE,
313+
CurrentEpochStats: &types.CurrentEpochStats{
314+
InferenceCount: 9,
315+
MissedRequests: 1,
316+
},
317+
}
318+
err := k.SetParticipant(ctx, healthy)
319+
require.NoError(t, err)
320+
289321
// Block height still within cooldown
290322
blockHeight := excludedAtBlock + 10
291323

292324
filter := k.CreateHealthFilterFnForTest(ctx, blockHeight)
293-
members := makeCBMockMembers(cbAddr1)
325+
members := makeCBMockMembers(cbAddr1, cbAddr2)
294326
result := filter(members)
295327

296-
require.Empty(t, result, "excluded node within cooldown should remain excluded")
328+
// cbAddr1 (excluded, in cooldown) should be filtered out; cbAddr2 should remain
329+
require.Len(t, result, 1, "only the healthy node should survive the filter")
330+
require.Equal(t, cbAddr2, result[0].Member.Address, "excluded node within cooldown should remain excluded")
297331
}
298332

299333
// TestHealthFilterFallbackAllDegraded verifies the safety fallback: if all nodes

0 commit comments

Comments
 (0)