Skip to content

chore: stop re-broadcasting permanently-failed market settlements#1403

Merged
MicBun merged 2 commits into
mainfrom
chore/settlement-permanent-failure-quarantine
Jul 3, 2026
Merged

chore: stop re-broadcasting permanently-failed market settlements#1403
MicBun merged 2 commits into
mainfrom
chore/settlement-permanent-failure-quarantine

Conversation

@MicBun

@MicBun MicBun commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

What changes

A prediction market whose signed attestation is unparseable — for example an empty, 128-byte binary payload captured when the source data landed after the attestation fired (the market-368 incident) — can never be settled: settle_market reverts deterministically on every attempt. The tn_settlement scheduler previously treated that permanent failure as retryable, so it re-broadcast a settle_market tx that reverted identically every 5-minute poll forever, burning nonces on the settlement signer and committing a failed tx to a mainnet block each time (~36 failed txs/hour during the incident).

This teaches the scheduler to tell a permanent failure from a transient one and stop:

  • Classify (internal/engine_ops.go): a settle_market revert whose error matches an attestation-parse-failure signature is permanent — the attestation is signed and immutable, so re-parsing the same bytes fails forever. settle_market → parse_attestation_boolean routes binary markets (action_id 6–9) to parseBinaryActionResult and numeric markets (action_id 1–5) to parseNumericActionResult, so the signature list covers both: the binary failures (binary action result must be …, abi-encoded bool, the boolean-decode errors), the numeric failures (result payload contains no values, failed to decode ABI result payload, expected 2 arrays …, values must be []*big.Int), and the malformed-canonical / unsupported action_id family. Nonce and network/broadcast errors are explicitly excluded and keep retrying. The list is deliberately narrow: an unfamiliar error stays transient (no regression), so the fix can never wrongly strand a settleable market.
  • Stop the inner retry (BroadcastSettleMarketWithRetry): on a permanent failure it returns immediately with the new ErrPermanentSettleFailure sentinel instead of burning the remaining retry attempts.
  • Quarantine the market (scheduler/scheduler.go): the scheduler skips a permanently-failed market until a 1-hour re-probe cooldown, then probes it once. This bounds the failed-tx rate to at most one per cooldown per stuck market (vs every poll), logs it loudly for manual intervention, and auto-recovers — if an operator re-attests (a fresh 32-byte attestation overrides the bad one), the next probe settles it and clears the quarantine.

The quarantine is in-memory and leader-local (only the leader runs settlement); on restart or leadership change it re-probes once and re-quarantines if still permanent. Nothing on-chain changes — no migration, no precompile output, no consensus behavior. This is defect (C) from the backlog; defects (A) gate empty/non-boolean attestations at signing and (B) soften parse_attestation_boolean are consensus/migration changes left as separate follow-ups. The kept-deployed admin_force_settle_market remains the operator remedy.

Tests

  • internal/engine_ops_test.go: TestIsPermanentSettleError (table: the 368 binary payload, malformed-canonical, boolean-decode, and the numeric empty/malformed/wrong-array-count cases all classify permanent; nonce/network/unrelated-revert stay transient) and TestBroadcastSettleMarketWithRetry_PermanentFailureStopsImmediately (permanent failure attempted exactly once, returns the sentinel).
  • scheduler/scheduler_test.go: TestRunSettlementCycle_PermanentFailureQuarantinesMarket (attempted once, quarantined, then skipped and not re-broadcast) and TestRunSettlementCycle_SuccessClearsQuarantine (a market that settles after the cooldown clears its quarantine).

Both tn_settlement package suites pass; go build ./... and go vet -tags kwiltest ./extensions/tn_settlement/... are clean.

@holdex

holdex Bot commented Jul 3, 2026

Copy link
Copy Markdown

Time Submission Status

Member # Time Running Total Status Last Update
MicBun 4h ✅ Submitted Jul 3, 2026, 11:16 AM

Submit or update total time with:

@holdex pr submit-time 2h

Add time on top of previous submission with:

@holdex pr add-time 1h30m

See available commands to help comply with our Guidelines.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@MicBun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 19 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c7fa737d-9da4-485a-a196-f471cc1c5ac2

📥 Commits

Reviewing files that changed from the base of the PR and between b8c669f and 32226ca.

📒 Files selected for processing (2)
  • extensions/tn_settlement/scheduler/scheduler.go
  • extensions/tn_settlement/scheduler/scheduler_test.go
📝 Walkthrough

Walkthrough

This PR adds permanent-failure detection to BroadcastSettleMarketWithRetry, classifying deterministic on-chain settle_market errors and returning a new ErrPermanentSettleFailure sentinel instead of retrying. The scheduler now maintains a per-market quarantine map, skipping markets flagged with permanent failures for a one-hour cooldown, refactored via a new runSettlementCycle helper.

Changes

Permanent settle failure quarantine

Layer / File(s) Summary
Permanent error detection in retry loop
extensions/tn_settlement/internal/engine_ops.go, extensions/tn_settlement/internal/engine_ops_test.go
Adds exported ErrPermanentSettleFailure, isPermanentSettleError classifier checking error-message substrings while excluding nonce errors, and integrates it into the retry loop to stop immediately on permanent failures; includes new unit tests for classification and retry short-circuit behavior.
Scheduler quarantine state and settlement cycle
extensions/tn_settlement/scheduler/constants.go, extensions/tn_settlement/scheduler/scheduler.go, extensions/tn_settlement/scheduler/scheduler_test.go
Adds PermanentFailureReprobeCooldown constant, a quarantine map with mutex on SettlementScheduler, quarantine check/set/clear helpers, and refactors cron/RunOnce logic into runSettlementCycle that skips quarantined markets, quarantines on permanent failure, and clears quarantine on success; adds mock support and tests for quarantine behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • trufnetwork/node#1272: Modifies the same settlement retry path in BroadcastSettleMarketWithRetry that this PR extends with permanent-failure detection.
  • trufnetwork/node#1283: Both PRs address settle_market failure handling, with #1283 introducing deterministic "blocked" error cases relevant to the classification added here.

Suggested labels: type: fix

Suggested reviewers: pr-time-tracker

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: preventing repeated broadcasts for permanently failed market settlements.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/settlement-permanent-failure-quarantine

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@MicBun

MicBun commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@holdex pr submit-time 4h

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@extensions/tn_settlement/scheduler/scheduler.go`:
- Around line 267-299: The settlement loop in scheduler.go is consuming the DB
limit before applying in-memory quarantine checks, so quarantined markets can
block newer eligible ones from being processed. Update the market selection
logic around s.runSettlementCycle/FindUnsettledMarkets so it fetches or
paginates enough candidates to settle up to maxMarkets non-quarantined markets,
rather than stopping at the first maxMarkets results. Add a regression test
covering maxMarkets=1 with one quarantined older market and one eligible newer
market to verify the newer market is still settled.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cb93293d-2cc9-4a2e-b8f9-5348dc129a68

📥 Commits

Reviewing files that changed from the base of the PR and between 1efd883 and b8c669f.

📒 Files selected for processing (5)
  • extensions/tn_settlement/internal/engine_ops.go
  • extensions/tn_settlement/internal/engine_ops_test.go
  • extensions/tn_settlement/scheduler/constants.go
  • extensions/tn_settlement/scheduler/scheduler.go
  • extensions/tn_settlement/scheduler/scheduler_test.go

Comment thread extensions/tn_settlement/scheduler/scheduler.go Outdated
@MicBun MicBun self-assigned this Jul 3, 2026
@MicBun MicBun merged commit fb06558 into main Jul 3, 2026
7 checks passed
@MicBun MicBun deleted the chore/settlement-permanent-failure-quarantine branch July 3, 2026 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Problem: operators can't stop endless retries of a stuck market

1 participant