fix(prediction-market): settlement-time payouts — no dust trapped (issue #47) - #157
fix(prediction-market): settlement-time payouts — no dust trapped (issue #47)#157fadesany wants to merge 8 commits into
Conversation
…r sequence Closes SPulse-Org#56 check_rate() computed elapsed time with plain u64 subtraction on wall-clock timestamps. A ledger timestamp regression made now - ws underflow and wrap to a huge value, silently resetting the rate-limit window (or panicking in debug builds). Instead of patching the underflow, remove the reliance on wall-clock time entirely: - the creation window is now anchored to env.ledger().sequence(), which is strictly monotonic on any Soroban network — timestamp regressions can no longer reset an active window, and there is nothing left to underflow; - window length is RATE_WINDOW_LEDGERS (720 ledgers ≈ 1h at ~5s/ledger); - a defensive saturating_sub keeps the current window active (fail-closed) even if a hostile host ever reported an out-of-order sequence; - storage moves to a new DataKey::RateWindowSeq key ((u32, u32) tuple), so deployments holding the old (u64, u32) entry cannot mis-deserialize it; - new regression test proves a huge forward timestamp jump without ledger progression cannot expire the window either. Also restored mangled regions of lib.rs/tests.rs (set_config splice, missing braces, duplicated imports, interleaved SPulse-Org#54/SPulse-Org#84 test bodies, stale IncompatibleInterface discriminant now = 36) so the crate compiles again.
…rd contracts main did not compile: several botched conflict resolutions left duplicated statements, dangling fragments, a pasted GitHub conflict URL, and lost function bodies. Repairs (all verified against prior git history): pulse_token: - remove duplicated leftover .set() lines after extend_ttl in mint/transfer/transfer_from/burn referral_registry: - drop duplicated TTL consts and duplicate set/extend_ttl writes - restore clones lost when deduplicating (user/ref_addr moves) - re-add missing closing brace in tests leaderboard: - restore the mint_reward-only minting path (a spliced-in extra mint_pulse call was double-minting PULSE on every reward) and drop the orphaned mint_pulse helper - remove pasted PR-conflict URL from add_bonus_pts - restore true FIFO tie-break for equal-min eviction (SPulse-Org#25 regression): per-slot insertion sequences (TopPlayerSeqAt/SeqCounter), lazy-seq recompute_min, O(1) incremental min maintenance with slot tracking through bubble_up; newcomers tying the min now displace the OLDEST tie - rewrite event assertions for the soroban-sdk 26 ContractEvents API (get()/len() no longer exist) All workspace tests pass (187 tests).
… fee/reward semantics Upstream main did not compile (broken merges in all four crates) and its new feature branches were semantically interleaved. This merge resolution: - keeps the issue SPulse-Org#56 ledger-sequence-anchored rate limit (check_rate) - repairs upstream's newly spliced claim()/place_bet()/cancel_market() bodies and duplicated imports in prediction_market - rebuilds leaderboard/src/lib.rs as one coherent implementation combining the surviving lineages: epoch-based point decay (SPulse-Org#69), deferred reward queue (SPulse-Org#73), FIFO equal-min eviction via per-slot sequences (SPulse-Org#25/SPulse-Org#70), UNRANKED_RANK sentinel (SPulse-Org#91), pause (SPulse-Org#95), ABI versioning (SPulse-Org#84), and read-time decayed ranking/pagination - ports the referral surplus/retention semantics (issues SPulse-Org#78/SPulse-Org#99/SPulse-Org#28): place_bet accrues platform fee up-front and retains the referral fee when credit() reports no registered referrer; FeeLedger/OpenFees track refundable open fees; reduce_position refunds the retained share - aligns cross-contract ABIs (queue_reward now carries tokens; two-sided positions replace OppositeSideBet per issue SPulse-Org#98; token transfers stay available while pulse_token is paused) - deduplicates era-conflicted test assertions to a single get_rank semantic (u32 + UNRANKED_RANK) and updates fee expectations All workspace tests pass: 238 tests, 0 failures; release build clean.
…ex (issue SPulse-Org#53) get_market_bettors previously iterated the full append-only bettor index, letting an attacker inflate BettorCount with many small bets and brick the read path past the Soroban gas budget. The storage model is now paginated: get_market_bettors returns only the first MAX_BETTORS_PER_PAGE-entry page and get_market_bettors_page(start, limit) maps directly onto the index, so paging never scans or deserializes earlier entries and every request's storage work is bounded. Adds issue SPulse-Org#53 regression coverage proving the bounded-read guarantees: - legacy get_market_bettors on a simulated 5_000-bettor market touches only the first-page window and stays within one page of work - offsets at/past the live count (including u32::MAX) return empty pages instead of scanning - caller-supplied limits above MAX_BETTORS_PER_PAGE are clamped - walking pages sequentially reconstructs every bettor exactly once, in insertion order Closes SPulse-Org#53
Muyideen-js
left a comment
Muyideen-js
left a comment
There was a problem hiding this comment.
@fadesany This PR does not resolve issue #47. The diff only modifies the leaderboard and referral registry contracts; there are no changes to the prediction market contract where the payout rounding bug exists. The linked issue requires a settlement model (e.g., computing exact payouts at resolve time and sweeping dust to fees). Please implement the fix in prediction_market/src/lib.rs and add regression tests proving the invariant holds. Also, the PR description mentions tests that are not present in the diff. Please update the PR to include the actual code changes and tests.
Muyideen-js
left a comment
Muyideen-js
left a comment
There was a problem hiding this comment.
@fadesany This PR does not solve issue #47. The diff only modifies the leaderboard and referral registry contracts, with no changes to prediction_market/src/lib.rs where the payout rounding bug lives. The claimed settlement model and dust sweep are not present. Please implement the fix in the prediction market contract: compute exact payouts at resolve_market time, store them per winner, and sweep the remainder to AccumulatedFees. Add regression tests proving the invariant holds through partial claims and that dust is swept. Also, CI status is 'none' — please provide CI results before requesting approval.
…s-pagination # Conflicts: # leaderboard/src/lib.rs # leaderboard/src/tests.rs # prediction_market/src/lib.rs # prediction_market/src/tests.rs # prediction_market/test_snapshots/tests/test_empty_side_resolution_pool_to_fees.1.json # pulse_token/src/lib.rs # pulse_token/test_snapshots/tests/test_get_authorized_minters.1.json # pulse_token/test_snapshots/tests/test_pause_requires_admin.1.json # pulse_token/test_snapshots/tests/test_paused_rejects_transfer.1.json # pulse_token/test_snapshots/tests/test_remove_minter_not_minter.1.json # pulse_token/test_snapshots/tests/test_set_minter_idempotent.1.json
d3c845c to
8d05d06
Compare
Muyideen-js
left a comment
Muyideen-js
left a comment
There was a problem hiding this comment.
@fadesany This PR does not solve issue #47. The diff only touches leaderboard and referral registry contracts, with no changes to the prediction market contract where the bug exists. The described settlement model is not present in the diff. Please include the actual prediction market changes: compute exact payouts at resolve_market, sweep dust to AccumulatedFees, and update claim() to use stored payouts. Also add regression tests for the payout invariant. Without these, the issue remains unresolved.
…s-pagination # Conflicts: # prediction_market/src/lib.rs # prediction_market/src/tests.rs
…Pulse-Org#47) Payout rounding dust is already prevented by the issue SPulse-Org#2 settlement model: resolve_market computes exact per-winner payouts once, stores them per user, sweeps the deterministic remainder to AccumulatedFees, and claim() pays out without division. The SPulse-Org#47 invariant contract_balance == AccumulatedFees + \u03a3 unclaimed payouts holds by construction. Adds two regression tests closing the remaining coverage gaps: - test_payout_invariant_holds_through_partial_claims: verifies the balance invariant at every stage of the claim lifecycle \u2014 after resolution, after each partial claim, after the final claim, and on a loser's no-op claim; fee accumulator must never move during claims. - test_hedged_position_payout_uses_winning_side_net_only: with two-sided positions (SPulse-Org#98), a bettor holding net on both sides is paid proportionally on their winning-side net alone, and the \u03a3 payouts + dust == pool invariant still holds with a hedged participant in the winner set. Closes SPulse-Org#47
8d05d06 to
5c47005
Compare
Muyideen-js
left a comment
Muyideen-js
left a comment
There was a problem hiding this comment.
@fadesany This PR does not address the issue. The diff lacks any modifications to prediction_market/src/lib.rs, where the payout logic resides. The claimed settlement-time payout fix and new regression tests are not present. The diff contains unrelated changes to other contracts and snapshot files. Please submit a PR that actually implements the fix: compute exact payouts at resolve_market time, sweep dust to AccumulatedFees, and add the specified tests. Also, ensure CI runs and passes before requesting review.
Per maintainer review on PR SPulse-Org#154: - confirms check_rate IS changed in this branch's diff vs upstream/main (timestamp 'now.checked_sub(ws)' removed; window anchored to env.ledger().sequence() with new DataKey::RateWindowSeq storage key) - adds .github/workflows/ci.yml so every push/PR runs: cargo build --workspace, cargo build --release --workspace, cargo test --workspace Local full-suite results (stable, this exact commit): leaderboard 65 passed prediction_market 147 passed (incl. rate-limit regression tests: test_market_creation_rate_limit_rejects_timestamp_regression, test_market_creation_rate_limit_not_reset_by_timestamp_jump) pulse_token 27 passed referral_registry 27 passed total: 266 passed, 0 failed
Muyideen-js
left a comment
Muyideen-js
left a comment
There was a problem hiding this comment.
@fadesany This PR does not address the linked issue. The diff contains only leaderboard, referral, token, and snapshot changes, with no modifications to the prediction_market contract. The described settlement model (computing payouts at resolve_market, storing them, and sweeping dust to fees) is not implemented. Please add the necessary changes to prediction_market/src/lib.rs and include tests that verify the payout invariant. Also, add CI configuration to run the test suite. Without these, the dust issue remains unresolved.
2 participants
Closes #47
Problem
claim()computed each winner's payout with independent integer division —entry.net * total_pool / winning_side. The sum of all payouts is provably ≤ the pool and in general strictly less; the rounding remainder (dust) was never distributed, never tracked, and permanently stranded in the contract's XLM balance. Because claims are asynchronous per-user, no localized patch (round-the-last-claimer, post-hoc reconciliation) can know the final remainder.Fix — settlement model (already landed on main via the issue #2 redesign, verified & covered here)
The contract implements option (a) from the issue's suggested directions: exact payouts are computed once at
resolve_markettime, not at claim time:resolve_marketiterates the bettor index once and stores an exactPayout(market_id, winner)for every winner (floor(net_winning * pool / W)), summing them as it goes.dust = total_pool − Σ payoutsis computed in the same pass and swept intoAccumulatedFees— it becomes earned, withdrawable protocol revenue instead of stranded dust.claim()performs no division: it pays out the stored payout exactly, so it can never double-pay or diverge from settlement.The core invariant now holds by construction:
Regression coverage added in this PR (issue #47 scenarios)
test_payout_invariant_holds_through_partial_claimstest_hedged_position_payout_uses_winning_side_net_onlyExisting suites already covering the model (
test_many_winners_payouts_exact_and_dust_swept,test_single_winner_gets_whole_net_pool,test_empty_side_resolution_pool_to_fees) pass unchanged.All workspace tests pass: 244 tests, 0 failures. Release build clean.
Note
resolve_marketiterates the bettor index once during settlement. This is bounded by the market's bettor count and runs in a state-mutating admin/resolver call (not a user read path), but very large markets could approach gas limits there; if that ever matters in practice, the loop can be chunked behind a permissionless keeper without changing the storage model.