Skip to content

feat(core): self-heal registration if node falls out of core_validators - #309

Merged
raymondjacobson merged 3 commits into
mainfrom
claude/registration-watchdog
May 22, 2026
Merged

feat(core): self-heal registration if node falls out of core_validators#309
raymondjacobson merged 3 commits into
mainfrom
claude/registration-watchdog

Conversation

@raymondjacobson

Copy link
Copy Markdown
Contributor

Summary

The initial registration loop in startRegistryBridge exits after one successful RegisterSelf and never retries. So if a validator gets deregistered for any reason after that — most commonly another peer's startValidatorWarden acting on a transiently stale eth-registry cache — the node has no mechanism to notice or re-register. It sits in event-listener mode indefinitely, absent from core_validators, until someone restarts the process.

Add startRegistrationWatchdog as a managed routine that runs hourly, checks isSelfAlreadyRegistered, and re-runs RegisterSelf if not. Self-heal within ~1h instead of requiring manual operator intervention.

Motivating incident

validator.stuffisup.com updated its delegate wallet on Ethereum. The node successfully self-registered at 2026-05-20T23:20:15. Six minutes later val019.open-audio-validator.com's warden saw the new wallet missing from val019's local eth-registry cache (val019's websocket subscription had dropped and not yet caught up), fired a dereg, got quorum from 8 similarly-stale peers, and the row was deleted. Stuffisup's bridge had already exited its registration loop and never noticed — so the node has been absent from core_validators for >24h despite being healthy, synced, and registered on Ethereum with the correct wallet.

This watchdog would have re-inserted the row within an hour.

Why an hour

Long enough to not spam the chain, short enough that the recovery window is bounded. The warden runs every 60 min on each peer, so the watchdog interval matches the cadence at which the hostile event could recur.

Why no test

The watchdog is a time.Hour ticker wrapping two already-tested functions (isSelfAlreadyRegistered reads from the DB, RegisterSelf is exercised by the full bootstrap integration path). There's no novel logic here to assert in isolation — a unit test would just be ceremony.

Test plan

  • go build ./...
  • go test ./pkg/core/server/...
  • Verify post-merge on stuffisup: after upgrade and restart, the initial RegisterSelf should land the row; if any peer's stale warden dereg's it again, the watchdog should restore it within an hour. Watch validator_history for the pattern.

🤖 Generated with Claude Code

raymondjacobson and others added 2 commits May 21, 2026 18:54
The initial registration loop in startRegistryBridge exits after one
successful RegisterSelf and never retries. So if a validator gets
deregistered for any reason after that — e.g. another peer's warden
acting on a transiently stale eth-registry cache — the node has no
mechanism to notice or re-register. It sits in event-listener mode
forever, absent from core_validators, until someone restarts the
process.

Add startRegistrationWatchdog as a managed routine that runs hourly,
checks isSelfAlreadyRegistered, and re-runs RegisterSelf if not. This
makes the node self-heal from spurious deregs within ~1h instead of
requiring manual intervention.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
isSelfAlreadyRegistered returns false for both missing and jailed rows,
and RegisterSelf treats a jailed row as a cue to re-attest and unjail.
So as originally written, the watchdog would unjail any jailed node
every hour — defeating jailing as a sticky operator signal and creating
a flap loop with the warden, which re-jails underperforming nodes on
the next SLA cycle.

Scope the watchdog to the missing-row case via a direct pgx.ErrNoRows
check. Jailed rows are left alone for the operator to inspect and
manually unjail via restart.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@raymondjacobson

Copy link
Copy Markdown
Contributor Author

Pushed a fix in ada02df for the jail interaction Ray flagged.

Bug: as originally written, the watchdog used isSelfAlreadyRegistered, which returns false for both missing-row and jailed-row cases. RegisterSelf treats a jailed row as a cue to re-attest and unjail. So the watchdog would unjail any jailed node every hour — and since jailed nodes can't propose blocks (excluded from the active set), the warden's ShouldPurgeValidatorForUnderperformance would re-jail them at the next 60-min cycle. Flap loop forever, no operator awareness.

Fix: scope the watchdog to the missing-row case with a direct pgx.ErrNoRows check. Jailed rows are explicitly left alone — operator restart remains the only path to unjail.

@RolfAris

Copy link
Copy Markdown
Contributor

Two small observations, none super critical but still worth briefly considering I suspect:

1. Watchdog vs warden cadence. The "Why an hour" rationale ties the watchdog to the warden cycle, but the warden cycle is per-peer. With K stale rendezvous attestors, successful deregs arrive at ~60/K min (K=8 in your incident → ~7.5min), while recovery arrives once per W min. Modeling availability as Δ/(Δ + W/2):

Stale attestors K W=60min W=15min
1 50% 80%
4 33% 67%
8 20% 50%

A 15m interval is a one-line change with no normal-operation cost (the watchdog only contacts eth/peers when the row is actually missing) and 2–3× better availability during the exact scenario the PR addresses.

2. CI gap on the no-auto-unjail invariant. Empirically: swapping the pgx.ErrNoRows check for isSelfAlreadyRegistered (the refactor your ada02df commit message warns against) passes go test ./pkg/core/server/... cleanly. Today only the comments enforce the invariant. A minimal helper extraction + targeted test, mirroring the TestIsSelfAlreadyRegistered pattern already in validator_state_test.go, locks it in CI:

func (s *Server) watchdogShouldReregister(ctx context.Context) bool {
    _, err := s.db.GetNodeByEndpoint(ctx, s.config.NodeEndpoint)
    return errors.Is(err, pgx.ErrNoRows)
}
func TestWatchdogShouldReregister(t *testing.T) {
    pool := setupValidatorTestDB(t)
    ctx := context.Background()
    makeServer := func() *Server {
        return &Server{db: db.New(pool), config: &config.Config{NodeEndpoint: testNode.Endpoint}, logger: zap.NewNop()}
    }
    t.Run("absent row returns true", func(t *testing.T) {
        truncateValidators(t, pool)
        assert.True(t, makeServer().watchdogShouldReregister(ctx))
    })
    t.Run("active row returns false", func(t *testing.T) {
        truncateValidators(t, pool)
        require.NoError(t, db.New(pool).InsertRegisteredNode(ctx, testNode))
        assert.False(t, makeServer().watchdogShouldReregister(ctx))
    })
    t.Run("jailed row returns false", func(t *testing.T) {
        truncateValidators(t, pool)
        q := db.New(pool)
        q.InsertRegisteredNode(ctx, testNode)
        q.JailRegisteredNode(ctx, testNode.CometAddress)
        assert.False(t, makeServer().watchdogShouldReregister(ctx))
    })
}

Verified locally — the test fails on the regression refactor (the jailed row subtest catches the unjail-flap; active row catches a bonus wallet-mismatch case).

Neither is blocking; the PR fixes indefinite stranding either way.

Drop the missing-row-vs-jailed branching from ada02df. RegisterSelf is
already idempotent — it early-returns when the node is registered and
unjailed — so the watchdog can just call it unconditionally on a
schedule.

To bound flap when the node genuinely can't recover (e.g. underlying
breakage causes the warden to keep re-jailing), grow the interval
linearly each tick (1h, 2h, 3h, …, 24h cap). Restart resets to 1h,
so a deliberate operator restart speeds recovery — exactly the mental
model we want.

Add a unit test for the schedule function. The goroutine itself is now
small enough that the schedule is the only piece worth asserting.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@raymondjacobson

Copy link
Copy Markdown
Contributor Author

Thanks @RolfAris — pushed 899e7ae reworking this in response.

On cadence (your #1): sticking with 1h, but the backoff schedule changes — see below. Faster intervals would shorten downtime per cycle but multiply the number of register/dereg pairs in validator_history during a sustained cascade. Operationally we'd rather see fewer events to chase, even if each one lasts longer.

On the test (your #2): good call, kept the spirit. The watchdog logic shifted from a should-reregister boolean to a backoff schedule, so the testable pure-function is now nextWatchdogInterval. Subtests cover normal growth, the cap boundary, and post-cap clamp.

On the bigger change. Ray pointed out my ada02df was wrong to make jailed a no-op — operators want jailed nodes to attempt self-recovery, not require manual restart for every transient flap. The new design:

  • Watchdog calls RegisterSelf blindly on each tick — no state checks, no branching on row state. RegisterSelf is already idempotent (early-returns when registered & unjailed), so this is cheap in steady state and self-healing under failure.
  • Interval grows 1h, 2h, 3h, …, capped at 24h.
  • Restart resets to 1h.

Mental model: "your node always tries to come back; restarting makes it try harder." Bounds the flap loop with the warden naturally — by ~12 days of sustained re-jailing the watchdog is only firing once per 24h.

@raymondjacobson
raymondjacobson merged commit 7c33778 into main May 22, 2026
5 checks passed
@raymondjacobson
raymondjacobson deleted the claude/registration-watchdog branch May 22, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants