Skip to content

feat(predictify-hybrid): time-gated update_market_question for market creators (closes #004) - #336

Merged
dultimateade merged 2 commits into
dultimateade:mainfrom
Whiznificent:feat/issue-004-update-market-question
Oct 24, 2026
Merged

dultimateade merged 2 commits into
dultimateade:mainfrom
Whiznificent:feat/issue-004-update-market-question

Conversation

@Whiznificent

Copy link
Copy Markdown
Contributor

What this fixes

Closes #4.

Market creators could not correct a typo in a published question. The only edit path on main is update_event_description, which is admin-only, so a creator who mistypes "Wil BTC close above 100k?" has to find the admin, and the admin has to trust that the request is cosmetic. This PR adds a creator-facing, time-gated update_market_question that is valid only while nobody has committed anything to the market.

total_staked == 0 is necessary but not sufficient, so the function also refuses edits when:

  • the caller is not market.admin;
  • the market is not Active (e.g. Ended, Resolved, Cancelled);
  • env.ledger().timestamp() >= market.end_time — a market that has stopped taking bets must stop accepting edits too;
  • votes or stakes are non-empty even if total_staked was never updated (defensive: the gate must not depend on one bookkeeping field staying consistent).

Once the first stake lands, the question is frozen. Anyone who has placed money has placed it against specific text, and resolution criteria must not move afterwards.

Root cause

Not a bug in existing code — a missing capability, plus a design problem in the place people would naturally extend.

PredictifyHybrid::update_event_description (contract entrypoint) and MarketStateManager (state layer) already exist and already do the right thing mechanically: they replace question, call Market::refresh_metadata_commitment, invalidate the read cache, and emit MarketDescriptionUpdatedEvent. But the entrypoint is gated on require_primary_admin, so the only actor who can fix a creator's typo is the admin.

The tempting "fix" — loosening update_event_description to also accept the market creator — was rejected because it would silently widen an admin entrypoint's authority for every existing integration (and for every market, including ones the caller does not own). Instead the creator gets its own entrypoint with its own authorization rule, and the shared state logic is factored into MarketStateManager::update_question so the two paths cannot drift in what they store.

There is a second reason the creator path cannot simply reuse the admin one: the admin path's end_time behaviour is not defined for creators, and a creator editing an expired market would change metadata that a pending oracle/resolution flow may already have read.

The fix, and why this shape

  1. MarketStateManager::update_question (markets.rs) — one place that owns the mutation:
    • validates the new text (InvalidQuestion when empty, QuestionTooLong above MAX_QUESTION_LENGTH = 500, matching the documented 1–500 character API);
    • loads the market (MarketNotFound), then enforces creator (Unauthorized), state (InvalidState), end_time (MarketClosed) and the stake/vote gate (InvalidState);
    • writes question, calls refresh_metadata_commitment, persists through update_market (which also bumps the TTL and invalidates MarketReadCache), then emits MarketDescriptionUpdatedEvent with the old and new text.
  2. PredictifyHybrid::update_market_question (lib.rs) — a thin entrypoint: creator.require_auth() then delegate. Keeping the entrypoint thin matches how the rest of lib.rs is organised (entrypoints authorize + delegate, markets.rs owns state transitions).
  3. Metadata commitment refresh — this is the part that makes the change observable rather than cosmetic. verify_market_metadata(market_id, expected) compares against the stored commitment, so a client holding the old commitment now fails verification instead of silently rendering stale text. A test asserts the stored commitment equals Market::compute_metadata_commitment(question, outcomes, oracle_config) after the edit.
  4. No new error variants. The existing Error::InvalidQuestion, QuestionTooLong, InvalidState, MarketClosed, Unauthorized and MarketNotFound already describe every rejection. Adding variants would have expanded an enum that is already too large for the XDR spec (see follow-ups) for no benefit.
  5. Tests (src/tests/update_market_question_tests.rs, registered in src/tests/mod.rs) — 9 cases: happy path with commitment assertion, exact-length boundary, rejection after a stake, after a vote, by a non-creator, after end_time, on a non-active market, for an empty question, for an over-long question (asserting the stored question was not modified).
  6. CHANGELOG.md — one entry under Unreleased / Added, as required by the repo's changelog check.

How it was tested

Honest status: CI cannot be green on this repo, and I could not execute the tests here. main at 230856f does not compile at all:

$ cargo check -p predictify-hybrid
error[E0583]: file not found for module `utils`
error[E0583]: file not found for module `validation`
error[E0583]: file not found for module `versioning`
...
contracts/predictify-hybrid/src/voting.rs:2235:1: error: unexpected closing delimiter: `}`
error: could not compile `predictify-hybrid` (lib) due to 20 previous errors

This is pre-existing, unrelated to this PR, and bigger than this issue (a July commit deleted ~56 modules; see follow-ups). What I could verify:

  • cargo check -p predictify-hybrid fails with exactly the same 20 parse-stage errors before and after this branch (git stash → check → git stash pop). No new diagnostics are introduced by the change. Because parsing aborts early, type checking is not reached, so this is not proof that the new code type-checks — it is proof it adds no new failure.
  • rustfmt --edition 2021 --check parses every touched file (lib.rs, markets.rs, tests/mod.rs, the new test file) without syntax errors.
  • Every helper the new code calls was re-read on main for signature and semantics: Market::refresh_metadata_commitment, Market::compute_metadata_commitment, MarketStateManager::update_market (persists + bumps TTL + invalidates cache), EventEmitter::emit_market_description_updated(env, market_id, old, new, admin), and the six Error variants used.
  • The tests are written against the generated PredictifyHybridClient, using the same try_* / assert_contract_error idiom as the existing suites, so they run unchanged as soon as the crate builds.

I would rather tell you this than claim a green run. To review the behaviour end to end, cargo test -p predictify-hybrid -- update_market_question_tests must be run after the crate is restored to a compiling state.

Follow-ups worth filing separately

  1. main does not compile. 56+ modules referenced by lib.rs (utils, validation, versioning, gas, config, balances, audit, governance, metadata_limits, audit_trail, rate_limiter, circuit_breaker, …) are missing, and voting.rs / disputes.rs carry orphan-brace corruption from -X theirs merges. This blocks all verification for every PR in the repo.
  2. #[contracterror] Error has 132 variants. The XDR spec caps a contract error enum at 50, so soroban-sdk-macros panics while expanding the enum; the crate cannot compile even after the missing modules are restored. It needs to be split into domain enums (markets/fees/disputes/oracles) plus a thin top-level error.
  3. create_market takes 13 arguments. Soroban caps contract arguments at 10, so the entrypoint cannot be exported as written; it needs a CreateMarketParams struct.
  4. Creator self-service edits on dispute outcomes. If creators can fix question text, the same argument applies to the outcome labels of an un-staked market; update_event_outcomes is admin-only today and could take a creator variant with the same gates.
  5. update_event_description does not check end_time. The admin path checks bets/votes only; adding the same time gate there would make the two edit paths consistent.

Checklist

  • Addresses the issue's stated behaviour (update_market_question, total_staked == 0 gate, creator authorization)
  • Tests added (9 cases) and registered in the test module
  • CHANGELOG.md updated
  • No new dependencies, no new error variants, no storage-layout change
  • cargo test / CI — blocked by the pre-existing non-compiling main (documented above)

Closes #4

Whiznificent and others added 2 commits October 24, 2026 13:46
Market creators can correct a typo in a market question only while no stake
has been committed, the market is still Active, and its end_time has not
passed. The metadata commitment is refreshed so cached metadata fails
verification, and the existing MarketDescriptionUpdated event is emitted.

Closes dultimateade#4
@dultimateade
dultimateade merged commit f1bf286 into dultimateade:main Oct 24, 2026
1 of 2 checks passed
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.

[#004][feat] Issue #004

2 participants