Repository navigation
feat(predictify-hybrid): make the per-market outcome cap runtime-configurable (closes #009) - #337
Merged
dultimateade merged 1 commit intoOct 24, 2026
Conversation
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
DataKey::MaxOutcomesPerMarket(persistent,MARKET_TTL_LEDGERS)get_max_outcomes_per_market() -> u32, defaults toDEFAULT_MAX_OUTCOMES_PER_MARKET = 10set_max_outcomes_per_market(admin, max_outcomes), primary admin only, range2..=20create_marketandupdate_event_outcomesmxoutc(max_outcomes_per_market_set) +event_topic_catalogentryRoot cause
Two separate problems, and fixing only the first would have shipped a cap that does not actually bind.
The cap was a constant.
metadata_limits::MAX_OUTCOMES_COUNTwas compiled in, so the only way to change 10 was a redeploy. The issue is about making that value governance-controlled.Enforcement was not in a place that survives. On
mainthe historical check lives incrate::metadata_limits::validate_outcomes_count, and that module does not exist — it was among the ~56 modules deleted frommainin July. Yetmarket_types.rs:499still calls it (Market::validate), which is one of the reasons the crate does not compile today. So onmainthere is no enforcement at all on the real creation path:PredictifyHybrid::create_marketcallsvalidate_unique_outcomesand then a validator from the also-missingcrate::validationmodule.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 intoMarketValidator::validate_market_paramswould mean writing it into a seam the contract entrypoint never calls.MarketValidator::validate_market_paramsis only reached fromMarketCreator::create_market(the internal helper), not fromPredictifyHybrid::create_market.A second path could bypass any cap.
update_event_outcomesrewritesmarket.outcomesafter creation and validated onlylen() < 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, pushingresolve_market/claim_winningstoward the instruction budget.The fix, and why this shape
OutcomeCapManager(markets.rs) owns the parameter:get_max_outcomes(env)readsDataKey::MaxOutcomesPerMarket, falling back toDEFAULT_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 outsideMIN_OUTCOMES_PER_MARKET (2) ..= MAX_OUTCOMES_PER_MARKET_CEILING (20)withInvalidInput, persists with aMARKET_TTL_LEDGERSTTL (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 existingError::TooManyOutcomes(variant 422, already inerr.rswith a description and string code, so no error-enum growth).outcomesand 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.create_marketchecks immediately aftervalidate_unique_outcomes, before any storage write or fee processing, so a rejected market costs nothing and leaves no partial state.update_event_outcomesnow calls the same predicate in place of its barelen() < 2panic — same behaviour for the old case (InvalidOutcomes), plusTooManyOutcomesfor the previously unbounded case.EventEmitter::store_event, which persists the payload and publishes it under the topic; it does not publish a second time (the existingemit_max_bet_cap_setdoes, which is why an isolated harness of this emitter first saw two events for one call — see below). Topicmxoutcis short-symbol safe (≤ 9 chars) and is added toevent_topic_catalog.rsso indexers can discover it.DataKeygained a variant, sodocs/storage.mdgained a row andstorage_docs_tests.rs::DATA_KEY_VARIANTSgained the matching entry — that regression test fails if the two ever diverge.src/tests/max_outcomes_cap_tests.rs, registered insrc/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 onemxoutcevent;create_marketrejects 11 outcomes end-to-end through the generated client;update_event_outcomescannot bypass the cap.I deliberately did not touch
metadata_limits(absent) orvalidation(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 testcannot run for this crate.mainat230856ffails to compile, independently of this PR:What I verified, and how:
No new failures.
cargo check -p predictify-hybridreports 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.The new logic was executed, in an isolated harness. Because the crate cannot compile, I copied
OutcomeCapManager(and the emitter) verbatim into a temporarysoroban-sdk 22crate, with stand-ins only for the three things that live in files I cannot build: theDataKeyvariant, a two-variant-localError, and anEventEmitterwith the realstore_event/get_and_increment_noncebodies. Results:This exercised real Soroban storage (
persistent().set/get/extend_ttlon 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 withcreate_market/update_event_outcomes, which needs the real crate to build.Two defects were found by that harness and fixed before committing: the emitter double-published (2 events per call) because it both called
store_eventand published again; andassert_cap_eventhelper 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 theTryIntoValimport. Both are corrected in the pushed code.Parsing/formatting.
rustfmt --edition 2021 --checkparses every touched file without syntax errors; the new test file is rustfmt-clean.Cross-references checked on
main:Error::TooManyOutcomes = 422exists with a description and string code;MARKET_TTL_LEDGERSis365 * LEDGERS_PER_DAY;MarketValidator::validate_market_paramsis only called fromMarketCreator::create_market(hence the entrypoint-level check);MarketStateManager::update_marketfor 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
maindoes not compile (missing modules + merge corruption invoting.rs/disputes.rs). This blocks every PR's verification, including this one.#[contracterror] Errorhas 132 variants against an XDR cap of 50, so the macro panics during expansion. This PR deliberately reusesTooManyOutcomesinstead of adding variants; splitting the enum is its own task.create_markettakes 13 arguments (Soroban's limit is 10). It needs a params struct; until then the entrypoint cannot be exported.metadata_limitsis restored,validate_outcomes_countshould delegate toOutcomeCapManager::validate_outcome_countrather than reintroducing a constant, soMarket::validateand the entrypoints share one source of truth. The same applies to the creation validator invalidation.PerMarketCoolOff) would let a specific large market be approved without raising the global ceiling.market_createdpayload can contain; consumers that hard-coded a maximum outcomes array should be told.Checklist
create_market,update_event_outcomes)docs/storage.mdCHANGELOG.mdupdatedcargo test/ CI — blocked by the pre-existing non-compilingmain(documented above)Closes #9