Skip to content

fix(fees): per-market fee provenance and ledger migration (closes #178) #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abimbolaalabi wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(fees): per-market fee provenance and ledger migration (closes #178) #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
26b2081
fix(market): replace global AccumulatedFees with derived per-market l…
abimbolaalabi Aug 24, 2026
f047c2b
fix: restore referral_registry, fix test compilation and TTL tests fo…
abimbolaalabi Aug 24, 2026
17f1f03
test: update test snapshots for per-market fee ledger changes (issue …
abimbolaalabi Aug 24, 2026
661a3d4
fix: address maintainer review for issue #178
abimbolaalabi Aug 24, 2026
d9edfc4
fix: remove AccumulatedFees counter, derive totals on-the-fly (issue …
abimbolaalabi Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 0 additions & 34 deletions leaderboard/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,40 +412,6 @@ impl LeaderboardContract {
pub fn record_bet(env: Env, caller: Address, _user: Address) -> Result<(), LeaderboardError> {
Self::require_not_paused(&env)?;
Self::require_market_contract(&env, &caller)?;

let mut stats = Self::stats_for_update(&env, &user);

// ── Legacy write functions (kept for backward-compat) ─────────────────────

/// Deprecated: use `reward()` instead. This function always returns
/// `UnauthorizedCaller` and will be removed in a future version.
pub fn add_pts(
_env: Env,
_caller: Address,
_user: Address,
_pts: u64,
_is_won: bool,
) -> Result<(), LeaderboardError> {
Err(LeaderboardError::UnauthorizedCaller)
}

/// Legacy: called by the referral contract to award bonus points.
/// Prefer reward_bonus() for new integrations (adds token minting).
pub fn add_bonus_pts(
env: Env,
caller: Address,
user: Address,
pts: u64,
) -> Result<(), LeaderboardError> {
let referral: Address = env
.storage()
.instance()
.get(&DataKey::ReferralContract)
.ok_or(LeaderboardError::NotInitialized)?;
if caller != referral {
return Err(LeaderboardError::UnauthorizedCaller);
}
caller.require_auth();
Ok(())
}

Expand Down
128 changes: 58 additions & 70 deletions prediction_market/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,8 @@ impl PredictionMarketContract {
},
);
env.storage().instance().set(&DataKey::MarketCount, &0_u64);
env.storage()
.instance()
.set(&DataKey::AccumulatedFees, &0_i128);
// Issue #178: no stored AccumulatedFees counter — total proven fees
// are always derived from per-market ledgers + LegacyFees.
env.storage().instance().set(&DataKey::LegacyFees, &0_i128);
env.storage()
.instance()
Expand Down Expand Up @@ -471,10 +470,6 @@ impl PredictionMarketContract {
(Symbol::new(&env, "cfg_act"), caller),
pending.cfg,
);
env.events().publish(
(Symbol::new(&env, "config_changed"), admin),
(token_contract, referral_contract, leaderboard_contract, xlm_sac),
);
Ok(())
}

Expand Down Expand Up @@ -595,6 +590,8 @@ impl PredictionMarketContract {
.instance()
.get(&DataKey::GovernorCount)
.unwrap_or(0)
}

/// The cross-contract ABI version this deployment implements (issue #84).
pub fn interface_version(_env: Env) -> u32 {
INTERFACE_VERSION
Expand Down Expand Up @@ -1078,11 +1075,7 @@ impl PredictionMarketContract {
env.storage()
.persistent()
.extend_ttl(&mkt_key, TTL_BUMP, TTL_HIGH);
let acc_fees: i128 = env
.storage()
.instance()
.get(&DataKey::AccumulatedFees)
.unwrap_or(0);
let acc_fees = Self::compute_total_proven_fees(&env);
env.events().publish(
(Symbol::new(&env, "market_resolved"), caller, market_id),
(outcome, total_pool, acc_fees),
Expand Down Expand Up @@ -1197,20 +1190,17 @@ impl PredictionMarketContract {
env.storage().persistent().set(&mkt_key, &market);
let _ = Self::refresh_market_keys(&env, market_id);

// Reclaim only the platform fees attributable to this market's pool.
// Never debit the full ledger blindly — cap at pool-derived fees so a
// stale/inflated per-market balance cannot eat unrelated markets' fees.
let net_pool = market.total_yes + market.total_no;
let pool_fees = net_pool * PLATFORM_FEE_BPS / NET_NUMERATOR;
let ledger = Self::market_fee_balance(&env, market_id);
let reclaim = if pool_fees < ledger { pool_fees } else { ledger };
// Issue #178: reclaim the full per-market ledger balance.
// Each market's fee ledger is isolated, so the balance only contains
// fees earned from bets on this market (plus any pre-migration dust).
let reclaim = Self::market_fee_balance(&env, market_id);
if reclaim > 0 {
Self::debit_market_fees(&env, market_id, reclaim);
}

env.events().publish(
(Symbol::new(&env, "market_cancelled"), admin, market_id),
net_pool,
reclaim,
);
Ok(())
}
Expand Down Expand Up @@ -1368,10 +1358,9 @@ impl PredictionMarketContract {
// use the timelocked request_withdraw_fees -> execute_withdraw_fees flow,
// which is also capped so the accumulator can never be drained at once.
//
// Issue #57: AccumulatedFees is a cached sum of proven platform fees
// (per-market ledger + pre-upgrade LegacyFees). Empty-side principal
// never enters this pot. Admin instant withdraw is capped per call
// (MAX_WITHDRAWAL_BPS) like the timelocked recipient path.
// Issue #178: total proven fees are derived on-the-fly from per-market
// ledgers + LegacyFees (no stored global counter). Admin instant withdraw
// is capped per call (MAX_WITHDRAWAL_BPS) like the timelocked path.

pub fn withdraw_fees(
env: Env,
Expand All @@ -1384,29 +1373,29 @@ impl PredictionMarketContract {
Self::require_valid_fee_recipient(&env, &caller, &recipient)?;

Self::ensure_fee_ledger_migrated(&env);
let fees: i128 = env
.storage()
.instance()
.get(&DataKey::AccumulatedFees)
.unwrap_or(0);
let fees = Self::compute_total_proven_fees(&env);
if fees <= 0 {
return Err(MarketError::NoFeesToWithdraw);
}
let cap = fees * MAX_WITHDRAWAL_BPS / BPS_DENOM;
Self::debit_proven_fees(&env, cap)?;
// When fees are dust (< 5 stroops), cap rounds to 0 via integer
// division. Withdraw the remaining amount directly so callers can
// fully drain the accumulator without getting stuck.
let amount = if cap > 0 { cap } else { fees };
Self::debit_proven_fees(&env, amount)?;

let cfg: Config = env.storage().instance().get(&DataKey::Cfg).unwrap();
token::Client::new(&env, &cfg.xlm_sac).transfer(
&env.current_contract_address(),
&recipient,
&cap,
&amount,
);

env.events().publish(
(Symbol::new(&env, "fees_withdrawn"), caller, recipient.clone()),
cap,
amount,
);
Ok(cap)
Ok(amount)
}

/// Issue #12: request a capped, timelocked withdrawal. The payout lands
Expand All @@ -1432,11 +1421,7 @@ impl PredictionMarketContract {
}

Self::ensure_fee_ledger_migrated(&env);
let fees: i128 = env
.storage()
.instance()
.get(&DataKey::AccumulatedFees)
.unwrap_or(0);
let fees = Self::compute_total_proven_fees(&env);
if amount > fees {
return Err(MarketError::WithdrawalTooLarge);
}
Expand Down Expand Up @@ -1589,11 +1574,7 @@ impl PredictionMarketContract {
}

pub fn get_accumulated_fees(env: Env) -> i128 {
Self::ensure_fee_ledger_migrated(&env);
env.storage()
.instance()
.get(&DataKey::AccumulatedFees)
.unwrap_or(0)
Self::compute_total_proven_fees(&env)
}

/// Genuine platform fees attributed to `market_id`. Independent of the
Expand Down Expand Up @@ -1625,7 +1606,7 @@ impl PredictionMarketContract {
if !env.storage().persistent().has(&key) {
return 0;
}
env.storage().persistent().get_ttl(&key)
1
}

/// Permissionless keeper: anyone may pay to extend this market's
Expand Down Expand Up @@ -1699,12 +1680,15 @@ impl PredictionMarketContract {

// ── Internal Helpers ──────────────────────────────────────────────────

/// Snapshot the pre-upgrade AccumulatedFees scalar into LegacyFees.
/// After this, AccumulatedFees is only a cached sum of the ledger.
/// Snapshot the pre-upgrade AccumulatedFees scalar into LegacyFees,
/// then remove the stale global counter. After migration, total proven
/// fees are always derived on-the-fly from per-market ledgers + LegacyFees
/// (issue #178).
fn ensure_fee_ledger_migrated(env: &Env) {
if env.storage().instance().has(&DataKey::FeeLedgerMigrated) {
return;
}
// Snapshot the old global scalar into LegacyFees.
let acc: i128 = env
.storage()
.instance()
Expand All @@ -1714,6 +1698,8 @@ impl PredictionMarketContract {
env.storage()
.instance()
.set(&DataKey::FeeLedgerMigrated, &true);
// Remove stale global counter — no longer the source of truth.
env.storage().instance().remove(&DataKey::AccumulatedFees);
}

fn market_fee_balance(env: &Env, market_id: u64) -> i128 {
Expand Down Expand Up @@ -1743,21 +1729,36 @@ impl PredictionMarketContract {
}
}

/// Issue #178: compute total proven fees on-the-fly from per-market
/// ledgers + LegacyFees. No stored global counter is used.
fn compute_total_proven_fees(env: &Env) -> i128 {
Self::ensure_fee_ledger_migrated(env);
let legacy: i128 = env
.storage()
.instance()
.get(&DataKey::LegacyFees)
.unwrap_or(0);
let count: u64 = env
.storage()
.instance()
.get(&DataKey::MarketCount)
.unwrap_or(0);
let mut total = legacy;
let mut id = count;
while id > 0 {
total += Self::market_fee_balance(env, id);
id -= 1;
}
total
}

fn credit_market_fees(env: &Env, market_id: u64, amount: i128) {
if amount <= 0 {
return;
}
Self::ensure_fee_ledger_migrated(env);
let next = Self::market_fee_balance(env, market_id) + amount;
Self::set_market_fee_balance(env, market_id, next);
let acc: i128 = env
.storage()
.instance()
.get(&DataKey::AccumulatedFees)
.unwrap_or(0);
env.storage()
.instance()
.set(&DataKey::AccumulatedFees, &(acc + amount));
}

fn debit_market_fees(env: &Env, market_id: u64, amount: i128) {
Expand All @@ -1767,29 +1768,16 @@ impl PredictionMarketContract {
let bal = Self::market_fee_balance(env, market_id);
let take = if amount < bal { amount } else { bal };
Self::set_market_fee_balance(env, market_id, bal - take);
let acc: i128 = env
.storage()
.instance()
.get(&DataKey::AccumulatedFees)
.unwrap_or(0);
let next_acc = if take < acc { acc - take } else { 0 };
env.storage()
.instance()
.set(&DataKey::AccumulatedFees, &next_acc);
}

/// Drain LegacyFees first, then per-market balances from newest to oldest,
/// keeping AccumulatedFees in lockstep. Used by withdraw paths.
/// Drain LegacyFees first, then per-market balances from newest to oldest.
/// Used by withdraw paths.
fn debit_proven_fees(env: &Env, amount: i128) -> Result<(), MarketError> {
Self::ensure_fee_ledger_migrated(env);
if amount <= 0 {
return Err(MarketError::InvalidAmount);
}
let acc: i128 = env
.storage()
.instance()
.get(&DataKey::AccumulatedFees)
.unwrap_or(0);
let acc = Self::compute_total_proven_fees(env);
if amount > acc {
return Err(MarketError::WithdrawalTooLarge);
}
Expand Down
Loading