Repository navigation
chore: ignore captures taken before a market's settle time - #1441
Conversation
Time Submission Status
Submit or update total time with: Add time on top of previous submission with: See available commands to help comply with our Guidelines. |
|
@holdex pr submit-time 5h |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughAttestations now store their capture block timestamp. Settlement and capture-status checks use that timestamp to exclude captures taken before a market’s settle time or without a recorded timestamp. ChangesSettlement Capture Eligibility
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant BlockContext
participant request_attestation
participant attestations
participant settle_market
BlockContext->>request_attestation: provide block timestamp and height
request_attestation->>attestations: store capture with created_timestamp
settle_market->>attestations: select signed capture at or after settle_time
attestations-->>settle_market: return earliest eligible signed capture
Merge Risk: ⚪ Minimal · up to No merge-blocking issue was identified. Run the migration when no market is due, as recommended for deployment. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation Issue
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
e400d95 to
12f094b
Compare
Description
A market resolves against the value that stood at its
settle_time.settle_marketpicks the earliest signed capture of the market's query (#1434), but it had no way to tell a capture taken beforesettle_timefrom one taken after.request_attestationisPUBLIC, a live market'squery_componentsare readable, andattestationsstored heights only. A capture taken beforesettle_time, while the value could still move, would therefore win.This change records the block time on every capture, and settlement now uses only captures taken at or after
settle_time.023-attestation-schema.sqladdsattestations.created_timestamp INT8withALTER TABLE ... ADD COLUMN IF NOT EXISTS, the same way028addeddata_providerandstream_id.024-attestation-actions.sql:request_attestationstores@block_timestampin the new column. The signed payload and the attestation hash do not change, so existing signatures and verifiers are unaffected. I regenerated the.prod.sqloverride withscripts/generate_prod_migrations.py, becauserequest_attestationis one of the fee actions it overrides.032-order-book-actions.sql:settle_marketresolves on the earliest signed capture withcreated_timestamp >= settle_time. A capture taken exactly atsettle_timecounts, which matches the check that lets a market settle at that second. When no capture qualifies, the error now names the case. Either a capture taken in time is waiting for a signature, or every capture was taken too early or has no time, or there is no capture at all.settle_markethas no.prod.sqloverride.tn_settlement: the scheduler's capture check counts only captures taken at or after the market's ownsettle_time. A market whose only capture is early is captured again, instead of waiting on a capture thatsettle_marketrefuses.Rows written before this change
Existing rows get
created_timestamp = NULL. Their block time cannot be recovered fromattestations, so I did not backfill them. Settlement treats a capture without a time like an early one and skips it.Only captures of markets that have not settled are ever read again. Settled markets are untouched. A market that was captured but not yet settled when the migration lands is captured again on the scheduler's next poll and settles on the new capture.
Release order
The binary works with or without the column. If
attestationshas nocreated_timestampyet (Postgres error42703), the scheduler counts every capture and logs this once. Thesettle_marketdeployed at that point also takes any signed capture, so the two agree. The usual order therefore works: deploy the binary first, then runmigrate.sh.The
ALTERtakes anAccessExclusiveLockonattestationsfor a moment on everymigrate.shrun, as028already does. Since #1400, settlement's poll reads run with a 3 secondlock_timeout, so a read that hits the lock fails and retries on the next poll instead of holding up block execution.A market captured but not yet settled when the migration lands is captured again, and the books of one ladder that straddle the migration would settle on captures from different blocks. Running the migration when no market is due avoids both.
Related Problem
resolves: #1435
How Has This Been Tested?
The integration tests run against real Postgres in the test containers, with
-tags kwiltest.New unit tests in
extensions/tn_settlement:created_timestamp >= settle_timeand passes the market's settle time.New integration tests in
TestSettlementIntegration:settle_time(value TRUE) does not count. The capture check reports nothing captured, andsettle_marketrefuses with the "taken before settle_time" error. A second capture exactly atsettle_time(value FALSE) then settles the market FALSE, although the early capture has the lower height.Updated tests: the existing tests took their captures at block time 0, before their markets' settle times. They now take them at or after the settle time, so each test still checks what it checked before.
TestSettlement,TestSettlementPayouts,TestOrderEvents,TestIndexChangeSettlement,TestIndexChangePayout,TestSettlementIntegrationand thetn_settlementunit tests pass.Mutation checks: I applied each mutant below, ran the tests, then restored the files and compared them byte for byte. Every mutant fails at least one test.
>instead of>=; a fallback on any Postgres error; no fallback; the scheduler passing 0 instead of the market's settle time.settle_market;>instead of>=; NULL counted as in time;request_attestationstoring no time; no time filter in the "waiting for a signature" check.Summary by CodeRabbit