diff --git a/backend/src/markets/market-settlement-state.util.spec.ts b/backend/src/markets/market-settlement-state.util.spec.ts new file mode 100644 index 00000000..abbc0b28 --- /dev/null +++ b/backend/src/markets/market-settlement-state.util.spec.ts @@ -0,0 +1,83 @@ +import { MarketSettlementState } from './entities/market.entity'; +import { + ALLOWED_SETTLEMENT_TRANSITIONS, + canTransition, + describeIllegalTransition, +} from './market-settlement-state.util'; + +const ALL_STATES = Object.values(MarketSettlementState); + +/** Every transition the lifecycle is meant to permit, listed out by hand so + * the test states the rules rather than re-deriving them from the table. */ +const VALID: ReadonlyArray<[MarketSettlementState, MarketSettlementState]> = [ + [MarketSettlementState.PENDING, MarketSettlementState.PROPOSED], + [MarketSettlementState.PROPOSED, MarketSettlementState.SETTLING], + [MarketSettlementState.PROPOSED, MarketSettlementState.CHALLENGED], + [MarketSettlementState.SETTLING, MarketSettlementState.SETTLING], + [MarketSettlementState.SETTLING, MarketSettlementState.SETTLED], + [MarketSettlementState.CHALLENGED, MarketSettlementState.SETTLED], +]; + +const isValid = (from: MarketSettlementState, to: MarketSettlementState) => + VALID.some(([f, t]) => f === from && t === to); + +describe('market settlement transitions', () => { + it.each(VALID)('allows %s -> %s', (from, to) => { + expect(canTransition(from, to)).toBe(true); + }); + + const INVALID = ALL_STATES.flatMap((from) => + ALL_STATES.filter((to) => !isValid(from, to)).map( + (to) => [from, to] as const, + ), + ); + + it.each(INVALID)('rejects %s -> %s', (from, to) => { + expect(canTransition(from, to)).toBe(false); + }); + + it('covers every pair of states exactly once', () => { + expect(VALID.length + INVALID.length).toBe( + ALL_STATES.length * ALL_STATES.length, + ); + }); + + it('treats settled as terminal', () => { + expect(ALLOWED_SETTLEMENT_TRANSITIONS[MarketSettlementState.SETTLED]).toEqual( + [], + ); + for (const to of ALL_STATES) { + expect(canTransition(MarketSettlementState.SETTLED, to)).toBe(false); + } + }); + + it('has a table entry for every state, so a new state cannot be forgotten', () => { + for (const state of ALL_STATES) { + expect(ALLOWED_SETTLEMENT_TRANSITIONS[state]).toBeDefined(); + } + expect(Object.keys(ALLOWED_SETTLEMENT_TRANSITIONS).sort()).toEqual( + [...ALL_STATES].sort(), + ); + }); + + describe('describeIllegalTransition', () => { + it('names both states and what would have been allowed instead', () => { + const message = describeIllegalTransition( + MarketSettlementState.PENDING, + MarketSettlementState.SETTLED, + ); + expect(message).toContain('"pending"'); + expect(message).toContain('"settled"'); + expect(message).toContain('proposed'); + }); + + it('says so plainly when the source state is terminal', () => { + expect( + describeIllegalTransition( + MarketSettlementState.SETTLED, + MarketSettlementState.PROPOSED, + ), + ).toContain('terminal state'); + }); + }); +}); diff --git a/backend/src/markets/market-settlement-state.util.ts b/backend/src/markets/market-settlement-state.util.ts new file mode 100644 index 00000000..4f6b675e --- /dev/null +++ b/backend/src/markets/market-settlement-state.util.ts @@ -0,0 +1,61 @@ +import { MarketSettlementState } from './entities/market.entity'; + +/** + * The single source of truth for which settlement transitions are legal. + * + * Until now these rules were spelled out three times as ad-hoc `if` checks in + * `markets.service.ts` and once more as an eligibility predicate in + * `market-settlement.scheduler.ts`. Each copy was correct, but nothing tied + * them together, so a new state or a changed rule had to be found in four + * places by hand. + * + * The lifecycle, as implemented today: + * + * PENDING ──propose──▶ PROPOSED ──grace window expires──▶ SETTLING ──▶ SETTLED + * │ + * └──challenge──▶ CHALLENGED ──admin adjudicates──▶ SETTLED + * + * SETTLED is terminal. Cancellation is deliberately absent: it is carried by + * the separate `is_cancelled` flag rather than by this enum, and folding it in + * here would change behaviour rather than describe it. + */ +export const ALLOWED_SETTLEMENT_TRANSITIONS: Readonly< + Record +> = Object.freeze({ + [MarketSettlementState.PENDING]: [MarketSettlementState.PROPOSED], + [MarketSettlementState.PROPOSED]: [ + MarketSettlementState.SETTLING, + MarketSettlementState.CHALLENGED, + ], + // Re-entrant: the scheduler may re-claim a market it already marked + // SETTLING when a previous attempt crashed between claiming and settling. + [MarketSettlementState.SETTLING]: [ + MarketSettlementState.SETTLING, + MarketSettlementState.SETTLED, + ], + [MarketSettlementState.CHALLENGED]: [MarketSettlementState.SETTLED], + [MarketSettlementState.SETTLED]: [], +}); + +export function canTransition( + from: MarketSettlementState, + to: MarketSettlementState, +): boolean { + return ALLOWED_SETTLEMENT_TRANSITIONS[from].includes(to); +} + +/** + * Message for a rejected transition. Names both states and what would have + * been allowed instead, so the caller is told what to do rather than only + * that they were wrong. + */ +export function describeIllegalTransition( + from: MarketSettlementState, + to: MarketSettlementState, +): string { + const allowed = ALLOWED_SETTLEMENT_TRANSITIONS[from]; + const suffix = allowed.length + ? `allowed from "${from}": ${allowed.join(', ')}` + : `"${from}" is a terminal state`; + return `Cannot move market settlement from "${from}" to "${to}" (${suffix})`; +} diff --git a/backend/src/markets/market-settlement.scheduler.ts b/backend/src/markets/market-settlement.scheduler.ts index 01ec679c..38c0e96c 100644 --- a/backend/src/markets/market-settlement.scheduler.ts +++ b/backend/src/markets/market-settlement.scheduler.ts @@ -3,6 +3,7 @@ import { Cron, CronExpression } from '@nestjs/schedule'; import { InjectRepository } from '@nestjs/typeorm'; import { DataSource, QueryRunner, Repository } from 'typeorm'; import { Market, MarketSettlementState } from './entities/market.entity'; +import { canTransition } from './market-settlement-state.util'; import { SettlementAttempt, SettlementAttemptStatus, @@ -229,10 +230,11 @@ export class MarketSettlementScheduler { const fresh = await queryRunner.manager.findOne(Market, { where: { id: market.id }, }); + // Asks the transition table rather than re-listing the states here, so + // this predicate cannot drift from the rules the service enforces. const stillEligible = fresh && - (fresh.settlement_state === MarketSettlementState.PROPOSED || - fresh.settlement_state === MarketSettlementState.SETTLING) && + canTransition(fresh.settlement_state, MarketSettlementState.SETTLING) && !!fresh.proposed_outcome; if (!stillEligible) { diff --git a/backend/src/markets/markets.service.ts b/backend/src/markets/markets.service.ts index 6b1dbbf0..9eb4ef49 100644 --- a/backend/src/markets/markets.service.ts +++ b/backend/src/markets/markets.service.ts @@ -42,6 +42,10 @@ import { Comment } from './entities/comment.entity'; import { moderateCommentContent } from '../common/comment-moderation.util'; import { MarketTemplate } from './entities/market-template.entity'; import { Market, MarketSettlementState } from './entities/market.entity'; +import { + canTransition, + describeIllegalTransition, +} from './market-settlement-state.util'; import { UserBookmark } from './entities/user-bookmark.entity'; import { MarketPriceSnapshot } from './entities/market-price-snapshot.entity'; import { Prediction } from '../predictions/entities/prediction.entity'; @@ -564,9 +568,14 @@ export class MarketsService { throw new ConflictException('Market is already resolved'); } - if (market.settlement_state !== MarketSettlementState.PENDING) { + if ( + !canTransition(market.settlement_state, MarketSettlementState.PROPOSED) + ) { throw new ConflictException( - `Cannot propose a resolution while market is in "${market.settlement_state}" state`, + describeIllegalTransition( + market.settlement_state, + MarketSettlementState.PROPOSED, + ), ); } @@ -616,9 +625,15 @@ export class MarketsService { ): Promise { const market = await this.findByIdOrOnChainId(id); - if (market.settlement_state !== MarketSettlementState.PROPOSED) { + if ( + !canTransition(market.settlement_state, MarketSettlementState.CHALLENGED) + ) { throw new BadRequestException( - 'Market does not have a resolution pending challenge', + 'Market does not have a resolution pending challenge: ' + + describeIllegalTransition( + market.settlement_state, + MarketSettlementState.CHALLENGED, + ), ); } @@ -661,7 +676,11 @@ export class MarketsService { const market = await this.findByIdOrOnChainId(id); if (market.settlement_state !== MarketSettlementState.CHALLENGED) { - throw new BadRequestException('Market does not have an active challenge'); + // Narrower than the transition table on purpose: SETTLING may legally + // reach SETTLED, but only via the scheduler, never via adjudication. + throw new BadRequestException( + `Market does not have an active challenge (state: "${market.settlement_state}")`, + ); } if (!market.outcome_options.includes(dto.outcome)) {