Skip to content

feat(predictify-hybrid): make the per-market outcome cap runtime-configurable (closes #009) - #337

Merged
dultimateade merged 1 commit into
dultimateade:mainfrom
Whiznificent:feat/issue-009-configurable-max-outcomes
Oct 24, 2026
Merged

dultimateade merged 1 commit into
dultimateade:mainfrom
Whiznificent:feat/issue-009-configurable-max-outcomes

Conversation

@Whiznificent

Copy link
Copy Markdown
Contributor

What this fixes

Closes #9.

The maximum number of outcomes a market can declare was a compile-time constant (MAX_OUTCOMES_COUNT = 10). Changing it — up or down — required a contract upgrade, so governance could not respond to demand for larger multi-outcome markets, nor tighten the cap if unusually large markets turned out to be expensive to resolve.

The cap is now runtime state:

Piece Where
Storage DataKey::MaxOutcomesPerMarket (persistent, MARKET_TTL_LEDGERS)
Read get_max_outcomes_per_market() -> u32, defaults to DEFAULT_MAX_OUTCOMES_PER_MARKET = 10
Write set_max_outcomes_per_market(admin, max_outcomes), primary admin only, range 2..=20
Enforced create_market and update_event_outcomes
Event mxoutc (max_outcomes_per_market_set) + event_topic_catalog entry

Root cause

Two separate problems, and fixing only the first would have shipped a cap that does not actually bind.

  1. The cap was a constant. metadata_limits::MAX_OUTCOMES_COUNT was compiled in, so the only way to change 10 was a redeploy. The issue is about making that value governance-controlled.

  2. Enforcement was not in a place that survives. On main the historical check lives in crate::metadata_limits::validate_outcomes_count, and that module does not exist — it was among the ~56 modules deleted from main in July. Yet market_types.rs:499 still calls it (Market::validate), which is one of the reasons the crate does not compile today. So on main there is no enforcement at all on the real creation path: PredictifyHybrid::create_market calls validate_unique_outcomes and then a validator from the also-missing crate::validation module.

    That matters for the shape of this PR: putting the new runtime cap only into metadata_limits-style code would mean writing a check into a file that does not exist, and putting it only into MarketValidator::validate_market_params would mean writing it into a seam the contract entrypoint never calls. MarketValidator::validate_market_params is only reached from MarketCreator::create_market (the internal helper), not from PredictifyHybrid::create_market.

  3. A second path could bypass any cap. update_event_outcomes rewrites market.outcomes after creation and validated only len() < 2. An admin could take a 2-outcome market and give it 200 outcomes, making resolution iterate a list that no creation-time check ever saw — and, at 200 outcomes, pushing resolve_market / claim_winnings toward the instruction budget.

The fix, and why this shape

  1. OutcomeCapManager (markets.rs) owns the parameter:
    • get_max_outcomes(env) reads DataKey::MaxOutcomesPerMarket, falling back to DEFAULT_MAX_OUTCOMES_PER_MARKET (10). Because "absent" means "10", existing behaviour is unchanged and no migration is needed for deployed contracts.
    • set_max_outcomes(env, caller, max_outcomes) requires auth, rejects anything outside MIN_OUTCOMES_PER_MARKET (2) ..= MAX_OUTCOMES_PER_MARKET_CEILING (20) with InvalidInput, persists with a MARKET_TTL_LEDGERS TTL (the value must outlive the markets it governs) and emits the new event.
    • validate_outcome_count(env, outcomes) is the single predicate used by every writer: < 2 → InvalidOutcomes, > cap → the existing Error::TooManyOutcomes (variant 422, already in err.rs with a description and string code, so no error-enum growth).
  2. Bounds, and why 2..=20. The floor is 2 because a market with fewer than two outcomes cannot be resolved. The ceiling is a deliberate safety rail, not a product decision: the cap bounds a resolution loop over outcomes and over the winning-outcome vector, and it bounds the size of the per-market record. Letting a mistyped or compromised admin key set 500 would create markets that cannot be resolved cheaply — the exact failure the cap exists to prevent. 20 is 2× the historical limit, so real demand can be met without opening that door.
  3. Both write paths enforce it. create_market checks immediately after validate_unique_outcomes, before any storage write or fee processing, so a rejected market costs nothing and leaves no partial state. update_event_outcomes now calls the same predicate in place of its bare len() < 2 panic — same behaviour for the old case (InvalidOutcomes), plus TooManyOutcomes for the previously unbounded case.
  4. Lowering the cap does not retro-invalidate markets. The cap is read at validation time only. If governance lowers it from 10 to 3, existing 5-outcome markets keep their outcomes and remain resolvable; only new markets (and outcome rewrites) are checked against 3. Invalidating live markets on a config change would strand their bets, so this is intentional and covered by a test.
  5. One event, not two. The emitter calls EventEmitter::store_event, which persists the payload and publishes it under the topic; it does not publish a second time (the existing emit_max_bet_cap_set does, which is why an isolated harness of this emitter first saw two events for one call — see below). Topic mxoutc is short-symbol safe (≤ 9 chars) and is added to event_topic_catalog.rs so indexers can discover it.
  6. Docs kept in sync with storage. DataKey gained a variant, so docs/storage.md gained a row and storage_docs_tests.rs::DATA_KEY_VARIANTS gained the matching entry — that regression test fails if the two ever diverge.
  7. Tests (src/tests/max_outcomes_cap_tests.rs, registered in src/tests/mod.rs) — 13 cases: default is 10; boundary accepted; 11 rejected; 1 rejected; raise to 12 then 12 accepted / 13 rejected; lower to 3 leaves an existing 5-outcome market intact while rejecting new 5-outcome markets; non-admin rejected without changing the stored cap; below-min and above-ceiling rejected without changing it; cap change emits exactly one mxoutc event; create_market rejects 11 outcomes end-to-end through the generated client; update_event_outcomes cannot bypass the cap.

I deliberately did not touch metadata_limits (absent) or validation (absent) — re-creating those modules is baseline repair, not this issue, and doing it here would bury a 60-line feature in a 5,000-line resurrection diff.

How it was tested

Honest status: CI cannot be green on this repo, and cargo test cannot run for this crate. main at 230856f fails to compile, independently of this PR:

$ 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

What I verified, and how:

  1. No new failures. cargo check -p predictify-hybrid reports the same 20 parse-stage errors before and after this branch (git stash -u → check → git stash pop). Parsing aborts before type checking, so this proves "adds no new error", not "type-checks" — stated plainly rather than dressed up as a green build.

  2. The new logic was executed, in an isolated harness. Because the crate cannot compile, I copied OutcomeCapManager (and the emitter) verbatim into a temporary soroban-sdk 22 crate, with stand-ins only for the three things that live in files I cannot build: the DataKey variant, a two-variant-local Error, and an EventEmitter with the real store_event / get_and_increment_nonce bodies. Results:

    running 5 tests
    test default_cap_is_ten_and_boundary_holds ... ok            # 10 ok, 11 → TooManyOutcomes, 1 → InvalidOutcomes
    test raised_cap_is_persisted_and_enforced ... ok             # set 12 → get 12, 12 ok, 13 rejected
    test lowered_cap_keeps_stored_value_and_rejects_larger_lists ... ok
    test out_of_range_caps_are_rejected_and_do_not_persist ... ok # 1 and 21 → InvalidInput, cap still 10
    test cap_change_publishes_mxoutc_event ... ok                # exactly 1 event per change
    test result: ok. 5 passed; 0 failed
    

    This exercised real Soroban storage (persistent().set/get/extend_ttl on a registered contract instance) and real event publication. The harness was deleted afterwards; it is not part of the diff. It validates the manager and emitter bodies, not the integration with create_market / update_event_outcomes, which needs the real crate to build.

  3. Two defects were found by that harness and fixed before committing: the emitter double-published (2 events per call) because it both called store_event and published again; and assert_cap_event helper in the test suite was reading the wrong tuple index for topics (env.events().all() yields (contract, topics, data), so topics is .1) and was missing the TryIntoVal import. Both are corrected in the pushed code.

  4. Parsing/formatting. rustfmt --edition 2021 --check parses every touched file without syntax errors; the new test file is rustfmt-clean.

  5. Cross-references checked on main: Error::TooManyOutcomes = 422 exists with a description and string code; MARKET_TTL_LEDGERS is 365 * LEDGERS_PER_DAY; MarketValidator::validate_market_params is only called from MarketCreator::create_market (hence the entrypoint-level check); MarketStateManager::update_market for the tests' market fixture.

To execute the suite once the crate builds: cargo test -p predictify-hybrid -- max_outcomes_cap_tests.

Follow-ups worth filing separately

  1. main does not compile (missing modules + merge corruption in voting.rs/disputes.rs). This blocks every PR's verification, including this one.
  2. #[contracterror] Error has 132 variants against an XDR cap of 50, so the macro panics during expansion. This PR deliberately reuses TooManyOutcomes instead of adding variants; splitting the enum is its own task.
  3. create_market takes 13 arguments (Soroban's limit is 10). It needs a params struct; until then the entrypoint cannot be exported.
  4. When metadata_limits is restored, validate_outcomes_count should delegate to OutcomeCapManager::validate_outcome_count rather than reintroducing a constant, so Market::validate and the entrypoints share one source of truth. The same applies to the creation validator in validation.
  5. Per-market overrides. A global cap is the right first step; a per-market override (like PerMarketCoolOff) would let a specific large market be approved without raising the global ceiling.
  6. Indexer contract. Raising the cap from 10 to 20 changes what a valid market_created payload can contain; consumers that hard-coded a maximum outcomes array should be told.

Checklist

  • Cap is admin-configurable at runtime (set/get entrypoints, default preserves existing behaviour, no migration)
  • Enforced on both paths that can change the outcome list (create_market, update_event_outcomes)
  • Admin-only, bounds-checked, evented, catalogued, documented in docs/storage.md
  • Tests added (13 cases) and registered; storage-docs regression list updated
  • CHANGELOG.md updated
  • No new dependencies, no new error variants, no storage-layout change beyond an additive key
  • cargo test / CI — blocked by the pre-existing non-compiling main (documented above)

Closes #9

…urable

The 10-outcome limit was a compile-time constant, so changing it required a
contract upgrade. The cap now lives in persistent storage
(DataKey::MaxOutcomesPerMarket), defaults to 10 for backwards compatibility,
and is settable by the primary admin within a 2..=20 range.

Enforcement moves to the two paths that can change a market's outcome list:
create_market (previously relied on a hard-coded check) and
update_event_outcomes (previously had no upper bound at all). Lowering the cap
deliberately leaves existing markets untouched so live markets stay
resolvable.

Closes dultimateade#9
@dultimateade
dultimateade merged commit 1bc49fa 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.

[#009][feat] Issue #009

2 participants