Skip to content

feat: implement admin game settlement and staking logic #661

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
ScriptedBro wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: implement admin game settlement and staking logic #661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6771814
feat: implement admin game settlement and staking logic
ScriptedBro Apr 23, 2026
1505a04
Merge upstream/main into feature/game-settlement-staking
ScriptedBro Apr 29, 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
37 changes: 37 additions & 0 deletions contracts/game_contract/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,42 @@ impl GameContract {
Ok(())
}

/// Settle the game gracefully by the admin/backend (e.g. checkmate validated off-chain).
/// This sets the game to Completed and automatically processes the payout.
pub fn resolve_game(env: Env, game_id: u64, winner: Address) -> Result<(), ContractError> {
let admin: Address = env
.storage()
.instance()
.get(&CONTRACT_ADMIN)
.expect("Not initialized");
admin.require_auth();

let mut games: Map<u64, Game> = env
.storage()
.instance()
.get(&GAMES)
.ok_or(ContractError::GameNotFound)?;

let mut game = games.get(game_id).ok_or(ContractError::GameNotFound)?;

if game.state != GameState::InProgress {
return Err(ContractError::GameNotInProgress);
}

if game.player1 != winner && Some(winner.clone()) != game.player2 {
return Err(ContractError::NotPlayer);
}

game.state = GameState::Completed;
game.winner = Some(winner.clone());
Self::process_payout(&env, &game, &winner)?;

games.set(game_id, game);
env.storage().instance().set(&GAMES, &games);
Comment on lines +722 to +727

@coderabbitai coderabbitai Bot Apr 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Prevent replay payout after admin resolution.

resolve_game pays immediately, then persists a Completed game with a winner. The public payout path accepts that same state/winner and can call process_payout again, draining other funds if the contract has enough token balance. Add an explicit paid/settled guard, or make process_payout reject when both player escrow deposits are no longer present.

🛡️ One minimal guard in process_payout 
         // Deduct both stakes first (clean state, prevents double-spend)
         let player1_escrow = escrow.get(game.player1.clone()).unwrap_or(0);
+        if player1_escrow < game.wager_amount {
+            return Err(ContractError::InsufficientFunds);
+        }
         escrow.set(game.player1.clone(), player1_escrow - game.wager_amount);
 
         let player2 = game.player2.as_ref().ok_or(ContractError::GameFull)?;
         let player2_escrow = escrow.get(player2.clone()).unwrap_or(0);
+        if player2_escrow < game.wager_amount {
+            return Err(ContractError::InsufficientFunds);
+        }
         escrow.set(player2.clone(), player2_escrow - game.wager_amount);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/game_contract/src/lib.rs` around lines 387 - 392, The admin
resolution flow calls Self::process_payout in resolve_game and then persists the
game as GameState::Completed, but the public payout entrypoint can later call
payout and replay process_payout; add a guard to prevent double payouts by (a)
extending the Game struct with a boolean like paid or settled and set it to true
after process_payout in resolve_game, and (b) at the start of process_payout
(and/or payout) check that game.paid is false and return an error if true;
alternatively, have process_payout validate that both players’ escrow deposits
still exist and return an error when they are already cleared—update
resolve_game to set the paid flag when it performs the payout and persist that
field (references: resolve_game, payout, process_payout, GameState::Completed,
Game struct).


Ok(())
}

pub fn payout_tournament(
env: Env,
game_id: u64,
Expand Down Expand Up @@ -3008,3 +3044,4 @@ mod tests {
assert_eq!(second, Err(Ok(ContractError::AlreadySettled)));
}
}
mod test;

@coderabbitai coderabbitai Bot Apr 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Inspect the unguarded Rust test module and its imports.

fd '^lib\.rs$' | xargs rg -n -C2 '^\s*mod test;\s*$|^\s*#\[cfg\(test\)\]'
fd '^test\.rs$' | xargs sed -n '1,120p'

Repository: NOVUS-X/XLMate

Length of output: 5612

Add #[cfg(test)] guard to the external test module declaration for consistency.

The external test module mod test; (line 1059) is not guarded with #[cfg(test)], unlike the inline mod tests block at line 793. While test.rs itself starts with #![cfg(test)], explicitly guarding the module declaration in lib.rs is clearer and matches the pattern used elsewhere in this file.

Proposed fix 
-mod test;
+#[cfg(test)]
+mod test;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
mod test;
#[cfg(test)]
mod test;
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/game_contract/src/lib.rs` at line 1059, Add an explicit test-only
cfg attribute to the external test module declaration: prefix the existing `mod
test;` declaration in `lib.rs` with `#[cfg(test)]` so it matches the inline `mod
tests` pattern and makes the test module import conditional; update the `mod
test;` declaration (symbol: mod test) to be guarded by `#[cfg(test)]`.

43 changes: 42 additions & 1 deletion contracts/game_contract/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ extern crate std;
use super::*;
use ed25519_dalek::{Signer, SigningKey};
use rand::rngs::OsRng;
use soroban_sdk::token::StellarAssetClient;
use soroban_sdk::token::{StellarAssetClient, TokenClient};
use soroban_sdk::{Address, Bytes, BytesN, Env, Map, Vec, testutils::Address as _};

/// Helper: seed a completed game directly into contract storage, bypassing
Expand Down Expand Up @@ -765,3 +765,44 @@ fn test_submit_move_player1_cannot_move_twice() {
let res = client.try_submit_move(&game_id, &player1, &Vec::from_array(&env, [2u32]));
assert_eq!(res, Err(Ok(ContractError::NotYourTurn)));
}

#[test]
fn test_resolve_game_by_admin() {
let env = Env::default();
env.mock_all_auths();

let contract_id = env.register_contract(None, GameContract);
let client = GameContractClient::new(&env, &contract_id);

let admin = Address::generate(&env);
let issuer = Address::generate(&env);
let player1 = Address::generate(&env);
let player2 = Address::generate(&env);
let treasury_addr = Address::generate(&env);

let stellar_token = env.register_stellar_asset_contract_v2(issuer);
let token_address = stellar_token.address();
let stellar_asset_client = StellarAssetClient::new(&env, &token_address);
let token_client = TokenClient::new(&env, &token_address);

client.initialize_token(&admin, &token_address);

let admin_key = Bytes::from_slice(&env, &[0u8; 32]);
client.initialize_puzzle_rewards(&admin, &admin_key, &0i128, &20u32, &treasury_addr);

let wager = 500;
stellar_asset_client.mint(&player1, &wager);
stellar_asset_client.mint(&player2, &wager);

let game_id = client.create_game(&player1, &wager);
client.join_game(&game_id, &player2);

// Let the admin resolve the game, meaning player1 won
client.resolve_game(&game_id, &player1);

// Check balances
// Pool = 1000. Fee = 1000 * 20 / 1000 = 20. Payout = 980.
assert_eq!(token_client.balance(&player1), 980);
assert_eq!(token_client.balance(&treasury_addr), 20);
assert_eq!(token_client.balance(&player2), 0);
}
Comment on lines +769 to +808

@coderabbitai coderabbitai Bot Apr 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Test does not actually verify admin-only authorization or final game state.

Two gaps in a test that the PR summary claims verifies "authorization scoping" and settlement:

  1. env.mock_all_auths() on line 348 unconditionally approves every require_auth in the transaction. The test never exercises a non-admin caller invoking resolve_game, so the admin-only guard is not actually covered. A companion negative case (e.g. try_resolve_game from a stranger with per-address auth mocks, expecting Err) is needed to back up the PR's "authorization scoping" claim — analogous to how test_configure_fees_permissioned (lines 292–316) pairs a success path with a stranger-rejected path.
  2. Only token balances are asserted. The core contract-side effects of resolve_game — transitioning GameState::Completed and recording winner = Some(player1) — are never read back from storage. If a future refactor accidentally skipped the state/winner update but still produced a correct payout, this test would silently pass.
Suggested additions 
// After resolve_game:
env.as_contract(&contract_id, || {
    let games: Map<u64, Game> = env.storage().instance().get(&GAMES).unwrap();
    let game = games.get(game_id).unwrap();
    assert_eq!(game.state, GameState::Completed);
    assert_eq!(game.winner, Some(player1.clone()));
});

// And a separate test:
#[test]
fn test_resolve_game_rejects_non_admin() {
    // ...setup identical up to create_game/join_game...
    let stranger = Address::generate(&env);
    // Use per-address auth mocks instead of mock_all_auths so the admin
    // requirement is actually checked.
    let res = client
        .mock_auths(&[/* stranger only */])
        .try_resolve_game(&game_id, &player1);
    assert!(res.is_err());
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/game_contract/src/test.rs` around lines 345 - 384, The test uses
env.mock_all_auths() so it never exercises the admin-only guard on resolve_game
and it only asserts token balances, not contract state changes; update the test
to (1) replace env.mock_all_auths() with per-address auths (or add a new test
test_resolve_game_rejects_non_admin) that calls try_resolve_game as a non-admin
and asserts Err to verify authorization, and (2) after
client.resolve_game(&game_id, &player1) read back the contract storage (GAMES
map) and assert the game’s state equals GameState::Completed and game.winner ==
Some(player1) to verify the contract-side settlement effects.

Loading