Skip to content

chore: settle on the capture taken for the settle time - #1434

Merged
MicBun merged 1 commit into
mainfrom
settle-a-market-on-one-predictable-capture
Sep 23, 2026
Merged

MicBun merged 1 commit into
mainfrom
settle-a-market-on-one-predictable-capture

Conversation

@MicBun

@MicBun MicBun commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

One gap between this and the requirement, stated up front. #1430 asks for the
value that existed at settle_time. This selects the earliest capture,
which is the same thing only while no capture predates settle_time. Nothing
enforces that — see The pre-capture gap below. It has never happened on
mainnet, and the ordering this replaces is exposed to the same thing in a worse
form, so this is a strict improvement rather than a complete answer.

A market can carry more than one capture. request_attestation freezes the query
result in the block it lands in and a validator signs it in a later one, so a poll
falling between those two blocks sees nothing signed and asks for another. On
mainnet 205 of the 1,073 markets that carry a capture carry two, taken 6 to
220 blocks apart with a mean of 30.

settle_market chose between them with:

ORDER BY signed_height DESC NULLS LAST
LIMIT 1

That is signing order, which has nothing to do with the market. Worse, for 128
of the 205 both rows were signed in the same block
, so the ordering is a tie and
the row the database happens to return first decides the outcome. The two captures
read the query against different chain state, so they are not interchangeable.

Why the earliest

Straight from the requirement: "capture the state of the query as close to
settle_time as possible"
, and a ladder "resolves against the value that existed
at settle_time"
. The settlement path only captures a market once it has fallen
due, so the first capture is the closest it got. Every later one is further away,
and whether it exists at all depends on how long signing took.

Markets already settled under the old ordering are not revisited. #1430 puts
remediation out of scope and asks for it to be tracked separately.

The pre-capture gap

request_attestation is a general primitive: it mentions neither markets nor
settle_time, it is PUBLIC, and a live market's query components are readable
from ob_queries. So a capture can be taken before settle_time, over a
window that has not closed, and under this ordering it would win.

Never observed: 0 of 1,319 captures on mainnet predate their market's creation,
minimum gap +34 blocks. And the ordering being replaced has the same exposure
pointing the other way — capture after watching the value move, win if signed
last — which is the worse of the two, because it can be played with hindsight.

Closing it properly means bounding a capture to the settlement window, which needs
a timestamp on attestations. That table stores heights only, so it is a schema
change and not this PR.

What changed

settle_market now selects ORDER BY created_height ASC among rows that carry a
signature, rather than the most recently signed row of any kind.

The two failure messages are also separated. Selecting only signed rows means a
market with an unsigned capture would otherwise report "attestation not found",
which sends an operator looking for the wrong thing, so the not-found branch now
distinguishes "never captured" from "captured, not yet signed". Both were reachable
before; only the wording changes.

No schema change. settle_market carries no .prod.sql override, confirmed by
re-running scripts/generate_prod_migrations.py.

Tests

One integration test, mutation-checked against real Postgres: two signed captures
of one market, at heights 10 and 20, reading values that give opposite outcomes.
Settlement must resolve TRUE, the earlier one. Reverting the ORDER BY fails it
with exactly that message.

Getting it to mean anything took two corrections worth recording:

  • The first version inserted value = 0 for the earlier capture. insert_records
    drops zeros unless the stream opts in (046-allow-zeros-config.sql), so the
    record was silently discarded and the capture came back with an empty payload.
    The test now calls set_allow_zeros first.
  • Even after that the test passed while proving nothing — both captures read the
    same value, so reverting the ORDER BY still passed. Only the mutation check
    caught it.

Scope

This does not stop a market acquiring two captures in the first place. That is:

It also does not touch ladders. A ladder whose books were captured in different
blocks still settles book by book, and holding one back needs a ladder identity on
ob_queries that does not exist yet — an uncaptured book has no attestations row,
so the grouping cannot be derived at settlement time.

@MicBun MicBun self-assigned this Sep 18, 2026
@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 4a9c25d1-3db8-4dad-ae41-7ce75900894b

📥 Commits

Reviewing files that changed from the base of the PR and between 6c989f1 and 7b3b021.

📒 Files selected for processing (2)
  • extensions/tn_settlement/settlement_integration_test.go
  • internal/migrations/032-order-book-actions.sql

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The settle_market action now selects the earliest signed attestation capture. It reports separate errors for unsigned and missing captures. An integration test verifies settlement with two conflicting captures.

Changes

Settlement Capture Selection

Layer / File(s) Summary
Earliest signed capture lookup
internal/migrations/032-order-book-actions.sql
settle_market selects the signed capture with the lowest created_height. It distinguishes an unsigned capture from a missing attestation.
Earliest capture integration test
extensions/tn_settlement/settlement_integration_test.go
The test creates two conflicting signed captures with the same attestation hash and verifies that settlement selects the earlier capture.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 7b3b0

The change implements the intended earliest-capture settlement behavior without a supported merge-blocking issue.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1 …
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 describes the main change: settlement uses the capture associated with the settlement time. It is concise and related to the change, although it does not explicitly state that the earliest s…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@holdex

holdex Bot commented Sep 18, 2026

Copy link
Copy Markdown

Time Submission Status

Member # Time Running Total Status Last Update
MicBun 4h ✅ Submitted Sep 19, 2026, 3:36 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.

@MicBun
MicBun marked this pull request as ready for review September 19, 2026 03:35
@MicBun

MicBun commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

@holdex pr submit-time 4h

@MicBun
MicBun merged commit e746e12 into main Sep 23, 2026
13 checks passed
@MicBun
MicBun deleted the settle-a-market-on-one-predictable-capture branch September 23, 2026 06:33
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.

1 participant