From a6235e6f4a42b672b678d71ab1c77039e4362d2d Mon Sep 17 00:00:00 2001 From: bdchatham Date: Wed, 7 Oct 2026 08:38:07 -0700 Subject: [PATCH 1/2] fix(sidecar): confirm an unjail from the jail state when the tx index is off The PLT-1392 harbor e2e unjailed a validator: the tx landed and the validator reads bonded, but the task failed with 'inclusion unverifiable: node transaction indexing is disabled'. An Unjail task always targets a validator, and validators often run with the tx index off, so the task reported failure on a successful unjail. When the node cannot look up the tx, the handler now polls the jail state for up to 30 seconds. A validator that reads not jailed was released, and the task completes; the result keeps the tx hash and the unverifiable inclusion status. A validator that still reads jailed keeps today's terminal unverifiable error. Refs: PLT-1392 Co-Authored-By: Claude Opus 5.5 --- api/v1alpha1/seinodetask_types.go | 6 ++-- sidecar/tasks/unjail.go | 44 +++++++++++++++++++++++++-- sidecar/tasks/unjail_test.go | 50 +++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/api/v1alpha1/seinodetask_types.go b/api/v1alpha1/seinodetask_types.go index 904ce1e0..f377d5b0 100644 --- a/api/v1alpha1/seinodetask_types.go +++ b/api/v1alpha1/seinodetask_types.go @@ -80,8 +80,10 @@ const ( // sidecar keyring. The sidecar refuses before broadcast when the account has // no validator, the validator's self-delegation is missing or below its min // self-delegation, or the validator is not jailed, still in its jail period, - // or tombstoned. NOT chain-idempotent: a second unjail of a released validator - // spends the fee and fails, so do not re-create a Complete task. + // or tombstoned. On a node with the tx index off, as on most validators, the + // sidecar cannot look up the tx, so it confirms the release from the jail + // state instead. NOT chain-idempotent: a second unjail of a released + // validator spends the fee and fails, so do not re-create a Complete task. SeiNodeTaskKindUnjail SeiNodeTaskKind = "Unjail" ) diff --git a/sidecar/tasks/unjail.go b/sidecar/tasks/unjail.go index 74d0095b..1903bac1 100644 --- a/sidecar/tasks/unjail.go +++ b/sidecar/tasks/unjail.go @@ -73,10 +73,22 @@ type Unjailer struct { // and SignAndBroadcast. readJail func(ctx context.Context, cfg engine.ExecutionConfig, chainID string, valAddr sdk.ValAddress) (jailState, error) broadcast func(ctx context.Context, cfg engine.ExecutionConfig, in SignAndBroadcastInput) (*SignAndBroadcastResult, error) + + // confirmWait bounds how long an unjail whose tx the node cannot look up + // waits for the validator to read released; confirmEvery is the poll + // interval. Zero values check once. + confirmWait time.Duration + confirmEvery time.Duration } func NewUnjailer(cfg engine.ExecutionConfig) *Unjailer { - return &Unjailer{cfg: cfg, readJail: chainJailState, broadcast: SignAndBroadcast} + return &Unjailer{ + cfg: cfg, + readJail: chainJailState, + broadcast: SignAndBroadcast, + confirmWait: 30 * time.Second, + confirmEvery: time.Second, + } } // Handler checks the validator's jail state, then delegates to @@ -117,17 +129,45 @@ func (u *Unjailer) Handler() engine.TaskHandler { return nil, err } out, cerr := classifyGovResult(engine.TaskUnjail, result) + releasedByState := result.Unverifiable && u.releasedAfter(ctx, params.ChainID, valAddr) + if releasedByState { + cerr = nil + } unjailLog.Info("unjail broadcast", "taskId", taskID, "chainId", params.ChainID, "validator", valAddr.String(), "txHash", out.TxHash, "height", out.Height, - "inclusionStatus", out.InclusionStatus) + "inclusionStatus", out.InclusionStatus, + "releasedByState", releasedByState) return out, cerr }) } +// releasedAfter settles an unjail whose tx the node cannot look up because its +// tx index is off, as on most validators. The unjail's effect shows in state +// instead: a validator that reads not jailed was released. An unjail from +// elsewhere that lands first reads the same, and the validator is released +// either way. It polls until confirmWait passes, and checks at least once. +func (u *Unjailer) releasedAfter(ctx context.Context, chainID string, valAddr sdk.ValAddress) bool { + deadline := time.Now().Add(u.confirmWait) + for { + st, err := u.readJail(ctx, u.cfg, chainID, valAddr) + if err == nil && !st.CatchingUp && !st.Jailed { + return true + } + if !time.Now().Before(deadline) { + return false + } + select { + case <-ctx.Done(): + return false + case <-time.After(u.confirmEvery): + } + } +} + // checkJailed refuses an unjail that the chain would reject after taking the // fee. CheckTx does not run the message handler, so these cases otherwise // surface only as a committed-but-failed tx. After the catching-up check, the diff --git a/sidecar/tasks/unjail_test.go b/sidecar/tasks/unjail_test.go index 7b272187..ff9a8720 100644 --- a/sidecar/tasks/unjail_test.go +++ b/sidecar/tasks/unjail_test.go @@ -178,6 +178,56 @@ func TestUnjailCommittedFailureIsTerminalAndKeepsTxHash(t *testing.T) { } } +// PLT-1392 harbor e2e: validators often run with the tx index off, so the +// node cannot look up the unjail tx. The task then confirms the release from +// the jail state: Complete once the validator reads released. +func TestUnjailUnverifiableTxConfirmedByJailState(t *testing.T) { + kr, _ := testKeyring(t) + states := []jailState{releasable(), releasable(), with(func(s *jailState) { s.Jailed = false })} + reads := 0 + u := &Unjailer{ + cfg: engine.ExecutionConfig{Keyring: kr, Checkpointer: newFakeCheckpointer(nil)}, + readJail: func(context.Context, engine.ExecutionConfig, string, sdk.ValAddress) (jailState, error) { + st := states[min(reads, len(states)-1)] + reads++ + return st, nil + }, + broadcast: func(context.Context, engine.ExecutionConfig, SignAndBroadcastInput) (*SignAndBroadcastResult, error) { + return &SignAndBroadcastResult{TxHash: "ABCD", Unverifiable: true}, nil + }, + confirmWait: time.Second, + confirmEvery: time.Millisecond, + } + out, err := runUnjail(t, u, "node_admin") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if out == nil || out.TxHash != "ABCD" || out.InclusionStatus != wire.InclusionUnverifiable { + t.Errorf("result = %+v; want the tx hash, still marked unverifiable", out) + } + if reads != 3 { + t.Errorf("read jail state %d times, want 3 (pre-check, then two polls)", reads) + } +} + +// A validator that still reads jailed when the wait ends keeps the +// unverifiable failure: the operator must check the tx through an indexed RPC. +func TestUnjailUnverifiableTxStillJailedStaysUnverifiable(t *testing.T) { + h, _ := newUnjailHarness(t, releasable(), nil, &SignAndBroadcastResult{TxHash: "ABCD", Unverifiable: true}) + h.u.confirmWait = 20 * time.Millisecond + h.u.confirmEvery = 5 * time.Millisecond + out, err := runUnjail(t, h.u, "node_admin") + if !IsTerminal(err) || !strings.Contains(err.Error(), "inclusion unverifiable") { + t.Fatalf("want terminal inclusion-unverifiable error, got %v", err) + } + if out == nil || out.TxHash != "ABCD" { + t.Errorf("result = %+v", out) + } + if h.reads < 2 { + t.Errorf("read jail state %d times, want the pre-check plus at least one poll", h.reads) + } +} + // A rehydrated run must adopt the first run's tx, not re-check the jail: the // first unjail may already have released the validator. func TestUnjailWithTxMarkerSkipsJailCheck(t *testing.T) { From 325962821275ff04334d9b72383cb946ceb19a8a Mon Sep 17 00:00:00 2001 From: bdchatham Date: Wed, 7 Oct 2026 08:51:23 -0700 Subject: [PATCH 2/2] fix(sidecar): bound the unjail confirm wait and document the completed-unverifiable case Review follow-ups on #603: - releasedAfter wraps the poll in a context deadline, so a slow read near the end cannot stretch the 30-second wait, and it logs the last read error when it gives up. - wire.InclusionUnverifiable and classifyGovResult now say that an Unjail task can complete with this status once the validator reads released. Refs: PLT-1392 Co-Authored-By: Claude Opus 5.5 --- sidecar/tasks/gov_result.go | 3 ++- sidecar/tasks/unjail.go | 17 +++++++++++------ sidecarapi/wire/wire.go | 5 ++++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/sidecar/tasks/gov_result.go b/sidecar/tasks/gov_result.go index 41910721..f1cf83a0 100644 --- a/sidecar/tasks/gov_result.go +++ b/sidecar/tasks/gov_result.go @@ -54,7 +54,8 @@ func classifyGovResult(taskType engine.TaskType, r *SignAndBroadcastResult) (*wi case r.Unverifiable: // Broadcast accepted but the node's tx index is off, so the outcome is // unobservable. Terminal (retrying this node is futile) but NOT - // committed_failed — the operator must verify via an indexed RPC. + // committed_failed — the operator must verify via an indexed RPC. The + // Unjail handler overrides this when the jail state shows the release. out.InclusionStatus = wire.InclusionUnverifiable txBroadcastTotal.WithLabelValues(string(taskType), wire.InclusionUnverifiable).Inc() return out, Terminal(fmt.Errorf("tx %s inclusion unverifiable: %w", r.TxHash, errTxIndexingDisabled)) diff --git a/sidecar/tasks/unjail.go b/sidecar/tasks/unjail.go index 1903bac1..90cab3e8 100644 --- a/sidecar/tasks/unjail.go +++ b/sidecar/tasks/unjail.go @@ -75,8 +75,8 @@ type Unjailer struct { broadcast func(ctx context.Context, cfg engine.ExecutionConfig, in SignAndBroadcastInput) (*SignAndBroadcastResult, error) // confirmWait bounds how long an unjail whose tx the node cannot look up - // waits for the validator to read released; confirmEvery is the poll - // interval. Zero values check once. + // waits for the validator to read released, reads included; confirmEvery + // is the poll interval. confirmWait time.Duration confirmEvery time.Duration } @@ -149,19 +149,24 @@ func (u *Unjailer) Handler() engine.TaskHandler { // tx index is off, as on most validators. The unjail's effect shows in state // instead: a validator that reads not jailed was released. An unjail from // elsewhere that lands first reads the same, and the validator is released -// either way. It polls until confirmWait passes, and checks at least once. +// either way. It polls until confirmWait passes; the deadline also bounds each +// read, so a slow read cannot stretch the wait. func (u *Unjailer) releasedAfter(ctx context.Context, chainID string, valAddr sdk.ValAddress) bool { - deadline := time.Now().Add(u.confirmWait) + ctx, cancel := context.WithTimeout(ctx, u.confirmWait) + defer cancel() + var lastErr error for { st, err := u.readJail(ctx, u.cfg, chainID, valAddr) if err == nil && !st.CatchingUp && !st.Jailed { return true } - if !time.Now().Before(deadline) { - return false + if err != nil { + lastErr = err } select { case <-ctx.Done(): + unjailLog.Warn("unjail release not confirmed from the jail state", + "validator", valAddr.String(), "wait", u.confirmWait, "lastReadErr", lastErr) return false case <-time.After(u.confirmEvery): } diff --git a/sidecarapi/wire/wire.go b/sidecarapi/wire/wire.go index 8b3b5f25..63afad48 100644 --- a/sidecarapi/wire/wire.go +++ b/sidecarapi/wire/wire.go @@ -103,7 +103,10 @@ const ( // its on-chain outcome cannot be observed from the target node (its tx // index is disabled), so the sidecar can confirm neither success nor // failure. Terminal — retrying the same node is futile — but distinct from - // committed_failed: the operator must verify via an indexed RPC. + // committed_failed: the operator must verify via an indexed RPC. One + // exception: an unjail confirms its effect from the validator's jail state, + // so an Unjail task can complete with this status once the validator reads + // released. Key completion on the task phase, not on this value alone. InclusionUnverifiable = "unverifiable" )