Skip to content

Refactor payout distribution logic to require explicit claims from winners #291

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

Merged
greatest0fallt1me merged 1 commit into from
Jan 31, 2026
Merged

Refactor payout distribution logic to require explicit claims from winners #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 3 additions & 6 deletions contracts/predictify-hybrid/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1250,9 +1250,9 @@ impl PredictifyHybrid {
market.state = MarketState::Resolved;
env.storage().persistent().set(&market_id, &market);

// Note: Bet resolution is skipped to avoid segfaults
// Since place_bet syncs votes/stakes, distribute_payouts works via vote-based system
// Individual bet status can be updated separately if needed
// Note: Bet resolution is skipped to avoid segfaults.
// Since place_bet syncs votes/stakes, winners must call claim_winnings(user, market_id)
// to receive their share; no automatic distribution is performed here.

// Emit market resolved event (simplified to avoid segfaults)
let oracle_result_str = market
Expand Down Expand Up @@ -1282,9 +1282,6 @@ impl PredictifyHybrid {
&MarketState::Resolved,
&reason,
);

// Automatically distribute payouts
let _ = Self::distribute_payouts(env.clone(), market_id);
}

/// Fetches oracle result for a market from external oracle contracts.
Expand Down
70 changes: 36 additions & 34 deletions contracts/predictify-hybrid/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1089,11 +1089,17 @@ fn test_automatic_payout_distribution() {
max_entry_ttl: 10000,
});

// Resolve market manually (this also calls distribute_payouts internally)
// Resolve market manually (winners must call claim_winnings explicitly)
test.env.mock_all_auths();
client.resolve_market_manual(&test.admin, &market_id, &String::from_str(&test.env, "yes"));

// Verify market state and that winners were marked as claimed (payouts distributed automatically)
// Winners claim winnings explicitly
test.env.mock_all_auths();
client.claim_winnings(&user1, &market_id);
test.env.mock_all_auths();
client.claim_winnings(&user2, &market_id);

// Verify market state and that winners were marked as claimed
let market_after = test.env.as_contract(&test.contract_id, || {
test.env
.storage()
Expand Down Expand Up @@ -1820,21 +1826,22 @@ fn test_manual_dispute_resolution_triggers_payout() {
max_entry_ttl: 10000,
});

// Manually resolve (this should trigger payout distribution)
// Manually resolve; winner must claim winnings explicitly
test.env.mock_all_auths();
client.resolve_market_manual(&test.admin, &market_id, &String::from_str(&test.env, "yes"));

// Verify payout was distributed (user should be marked as claimed)
test.env.mock_all_auths();
client.claim_winnings(&user1, &market_id);

let market_after = test.env.as_contract(&test.contract_id, || {
test.env
.storage()
.persistent()
.get::<Symbol, Market>(&market_id)
.unwrap()
});
// Note: The automatic payout distribution is called but may not mark votes as claimed
// since votes and bets are separate systems. This test verifies the resolution works.
assert_eq!(market_after.state, MarketState::Resolved);
assert!(market_after.claimed.get(user1.clone()).unwrap_or(false));
}

// ===== PAYOUT DISTRIBUTION TESTS =====
Expand Down Expand Up @@ -1958,9 +1965,9 @@ fn test_claim_winnings_successful() {
test.env.mock_all_auths();
client.resolve_market_manual(&test.admin, &market_id, &String::from_str(&test.env, "yes"));

// 5. Distribute payouts to winners (separate step after resolution)
// 5. Winner claims winnings explicitly
test.env.mock_all_auths();
let _total_distributed = client.distribute_payouts(&market_id);
client.claim_winnings(&test.user, &market_id);

// Verify claimed status
let market = test.env.as_contract(&test.contract_id, || {
Expand All @@ -1970,9 +1977,8 @@ fn test_claim_winnings_successful() {
.get::<Symbol, Market>(&market_id)
.unwrap()
});
// Note: claimed status tracking may vary by implementation
// assert!(market.claimed.get(test.user.clone()).unwrap_or(false));
assert_eq!(market.state, MarketState::Resolved);
assert!(market.claimed.get(test.user.clone()).unwrap_or(false));
}

#[test]
Expand Down Expand Up @@ -2340,7 +2346,10 @@ fn test_market_state_after_claim() {
test.env.mock_all_auths();
client.resolve_market_manual(&test.admin, &market_id, &String::from_str(&test.env, "yes"));

// resolve_market_manual distributes payouts internally; verify claimed flag is set
// Winner must claim winnings explicitly
test.env.mock_all_auths();
client.claim_winnings(&test.user, &market_id);

let market = test.env.as_contract(&test.contract_id, || {
test.env
.storage()
Expand Down Expand Up @@ -2454,7 +2463,13 @@ fn test_integration_full_market_lifecycle_with_payouts() {
test.env.mock_all_auths();
client.resolve_market_manual(&test.admin, &market_id, &String::from_str(&test.env, "yes"));

// Verify market state
// Winners claim winnings explicitly
test.env.mock_all_auths();
client.claim_winnings(&user1, &market_id);
test.env.mock_all_auths();
client.claim_winnings(&user2, &market_id);

// Verify market state and claimed flags
let market = test.env.as_contract(&test.contract_id, || {
test.env
.storage()
Expand All @@ -2467,16 +2482,6 @@ fn test_integration_full_market_lifecycle_with_payouts() {
market.winning_outcome,
Some(String::from_str(&test.env, "yes"))
);

// resolve_market_manual distributes payouts internally; verify market state and claimed flags
let market = test.env.as_contract(&test.contract_id, || {
test.env
.storage()
.persistent()
.get::<Symbol, Market>(&market_id)
.unwrap()
});
assert_eq!(market.state, MarketState::Resolved);
assert!(market.claimed.get(user1.clone()).unwrap_or(false));
assert!(market.claimed.get(user2.clone()).unwrap_or(false));
assert!(!market.claimed.get(user3.clone()).unwrap_or(false)); // Loser hasn't claimed
Expand Down Expand Up @@ -2520,21 +2525,19 @@ fn test_payout_event_emission() {
test.env.mock_all_auths();
client.resolve_market_manual(&test.admin, &market_id, &String::from_str(&test.env, "yes"));

// Distribute payouts to winners (separate step after resolution)
// Winner claims winnings explicitly (emits payout event)
test.env.mock_all_auths();
let _total_distributed = client.distribute_payouts(&market_id);
client.claim_winnings(&test.user, &market_id);

// Events are emitted automatically - we just verify the market state
let market = test.env.as_contract(&test.contract_id, || {
test.env
.storage()
.persistent()
.get::<Symbol, Market>(&market_id)
.unwrap()
});
// Note: Claimed field tracking is implementation-specific
// assert!(market.claimed.get(test.user.clone()).unwrap_or(false));
assert_eq!(market.state, MarketState::Resolved);
assert!(market.claimed.get(test.user.clone()).unwrap_or(false));
}

#[test]
Expand Down Expand Up @@ -2590,21 +2593,19 @@ fn test_reentrancy_protection_claim() {
test.env.mock_all_auths();
client.resolve_market_manual(&test.admin, &market_id, &String::from_str(&test.env, "yes"));

// Distribute payouts to winners (reentrancy protection is in this function)
// Winner claims winnings (reentrancy protection in claim_winnings)
test.env.mock_all_auths();
let _total_distributed = client.distribute_payouts(&market_id);
client.claim_winnings(&test.user, &market_id);

// Verify state was updated (reentrancy protection)
let market = test.env.as_contract(&test.contract_id, || {
test.env
.storage()
.persistent()
.get::<Symbol, Market>(&market_id)
.unwrap()
});
// Note: Claimed field tracking is implementation-specific
// assert!(market.claimed.get(test.user.clone()).unwrap_or(false));
assert_eq!(market.state, MarketState::Resolved);
assert!(market.claimed.get(test.user.clone()).unwrap_or(false));
}

// ===== REENTRANCY GUARD AND SECURITY HARDENING TESTS =====
Expand Down Expand Up @@ -2747,8 +2748,7 @@ fn test_overflow_protection_calculate_payout() {

#[test]
fn test_checks_effects_before_interactions_claim() {
// After resolve_market_manual, distribute_payouts runs and sets claimed for winners
// before any external interaction (checks-effects-interactions). Verify winner is marked claimed.
// claim_winnings sets claimed before external transfer (checks-effects-interactions).
let test = PredictifyTest::setup();
let market_id = test.create_test_market();
let client = PredictifyHybridClient::new(&test.env, &test.contract_id);
Expand Down Expand Up @@ -2778,6 +2778,8 @@ fn test_checks_effects_before_interactions_claim() {
});
test.env.mock_all_auths();
client.resolve_market_manual(&test.admin, &market_id, &String::from_str(&test.env, "yes"));
test.env.mock_all_auths();
client.claim_winnings(&test.user, &market_id);
let market_after = test.env.as_contract(&test.contract_id, || {
test.env
.storage()
Expand Down
Loading