Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
124 changes: 122 additions & 2 deletions contracts/game_contract/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ const MAX_STAKE: Symbol = symbol_short!("MAXSTAKE");
const FEE_BIPS: Symbol = symbol_short!("FEE_BIPS"); // u32 (0–1000, i.e. 0–10 %)
const TREASURY_ADDR: Symbol = symbol_short!("TR_ADDR"); // Address
const CONTRACT_ADMIN: Symbol = symbol_short!("CT_ADMIN"); // Address
const FEE_ADMINS: Symbol = symbol_short!("FEE_ADMS"); // Vec<Address>
const FEE_THRESHOLD: Symbol = symbol_short!("FEE_THR"); // u32

// Dispute resolution system
const DISPUTE_FEE: Symbol = symbol_short!("D_FEE"); // i128 - fee to file a dispute
Expand Down Expand Up @@ -672,17 +674,86 @@ impl GameContract {
env.storage().instance().set(&MAX_STAKE, &new_limit);
}

pub fn configure_fees(env: Env, admin: Address, fee_bips: u32, treasury_address: Address) {
pub fn configure_fee_multisig(
env: Env,
caller: Address,
new_admins: Vec<Address>,
threshold: u32,
) {
let current_admin: Address = env
.storage()
.instance()
.get(&CONTRACT_ADMIN)
.expect("Not initialized");
current_admin.require_auth();

if admin != current_admin {
if caller != current_admin {
panic!("Unauthorized admin address");
}
if threshold == 0 || threshold > new_admins.len() as u32 {
panic!("Invalid threshold");
}

let mut unique_set = Vec::new(&env);
for i in 0..new_admins.len() {
let admin = new_admins.get(i).unwrap();
if unique_set.contains(&admin) {
panic!("Duplicate admins in list");
}
unique_set.push_back(admin);
}

env.storage().instance().set(&FEE_ADMINS, &new_admins);
env.storage().instance().set(&FEE_THRESHOLD, &threshold);
}

pub fn configure_fees(
env: Env,
admins: Vec<Address>,
fee_bips: u32,
treasury_address: Address,
) {
let stored_admins: Vec<Address> =
env.storage()
.instance()
.get(&FEE_ADMINS)
.unwrap_or_else(|| {
let current_admin: Address = env
.storage()
.instance()
.get(&CONTRACT_ADMIN)
.expect("Not initialized");
let mut v = Vec::new(&env);
v.push_back(current_admin);
v
});
let threshold: u32 = env.storage().instance().get(&FEE_THRESHOLD).unwrap_or(1);

if admins.len() < threshold {
panic!("Not enough admin approvals");
}

let mut unique_approvals = 0;
let mut approved_set = Vec::new(&env);

for i in 0..admins.len() {
let admin = admins.get(i).unwrap();
if stored_admins.contains(&admin) {
if approved_set.contains(&admin) {
panic!("Duplicate admin approvals");
}
admin.require_auth();
approved_set.push_back(admin.clone());
unique_approvals += 1;
} else {
panic!("Unauthorized admin address");
}
}

if unique_approvals < threshold {
panic!("Not enough valid admin approvals");
}

if fee_bips > 1000 {
panic!("Fee bips must be between 0 and 1000");
}
Expand Down Expand Up @@ -1069,6 +1140,39 @@ impl GameContract {
.get(dispute.game_id)
.ok_or(ContractError::GameNotFound)?;

// Update dispute status
dispute.status = DisputeStatus::Resolved;
dispute.resolution = Some(resolution.clone());
let game_id = dispute.game_id;
disputes.set(dispute_id, dispute);
env.storage().instance().set(&DISPUTES, &disputes);

// Update game state and process payout based on arbitrator's decision
if let Some(ref winner_addr) = winner {
// Winner takes all
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)?;

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

games.set(game_id, game);
env.storage().instance().set(&GAMES, &games);
} else {
// Draw - refund both players
let mut games: Map<u64, Game> = env
.storage()
.instance()
.get(&GAMES)
.ok_or(ContractError::GameNotFound)?;

let game = games.get(game_id).ok_or(ContractError::GameNotFound)?;
if game.state != GameState::InProgress {
return Err(ContractError::GameAlreadyCompleted);
}
Expand Down Expand Up @@ -1644,6 +1748,7 @@ mod tests {
&0u32,
&treasury_addr,
);
client.configure_dispute_system(&admin, &arbitrator, &50i128);
client.configure_dispute_system(&admin, &arbitrator, &25i128);
Comment on lines +1751 to 1752

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

Duplicate configure_dispute_system calls in three dispute tests.

Each of these tests calls configure_dispute_system twice back-to-back with different dispute_fee values (50 then 25, or 50 then 0). The second call silently overrides the first, so only the second value takes effect — this is clearly a merge artifact and makes the expected balances (875 at line 1767, 900 at line 2124) hard to reason about. Remove one of the two calls in each test and confirm the intended dispute_fee for each scenario.

-        client.configure_dispute_system(&admin, &arbitrator, &50i128);
         client.configure_dispute_system(&admin, &arbitrator, &25i128);

Also applies to: 1801-1802, 2107-2108

🤖 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 1751 - 1752, Tests call
configure_dispute_system twice in a row (configure_dispute_system(&admin,
&arbitrator, &50i128); then configure_dispute_system(&admin, &arbitrator,
&25i128); or 50 then 0), so the second call overrides the first; remove the
unintended duplicate call so only the intended dispute_fee is set (decide
whether the test should use 50, 25, or 0), update the test to call
configure_dispute_system only once (referencing the
configure_dispute_system(...) invocation), and then verify the test assertions
that depend on the fee (expected balances like 875 and 900) match the retained
dispute_fee; apply the same change for the other duplicate occurrences of
configure_dispute_system in the file.

client.set_max_stake(&1_000i128);

Expand Down Expand Up @@ -1693,6 +1798,7 @@ mod tests {
&0u32,
&treasury_addr,
);
client.configure_dispute_system(&admin, &arbitrator, &50i128);
client.configure_dispute_system(&admin, &arbitrator, &0i128);
client.set_max_stake(&1_000i128);

Expand All @@ -1710,6 +1816,19 @@ mod tests {
&resolution,
);

// Arbitrator resolves in favor of player1
let resolution = Bytes::from_slice(&env, b"Player1 wins");
client.resolve_dispute(
&dispute_id,
&arbitrator,
&Some(player1.clone()),
&resolution,
);

// Verify player1 received the payout
assert_eq!(token_client.balance(&player1), 1_050);

// Verify dispute is resolved
let dispute = client.get_dispute(&dispute_id);
assert_eq!(dispute.status, DisputeStatus::Resolved);
assert_eq!(token_client.balance(&player1), 1_100);
Expand Down Expand Up @@ -1985,6 +2104,7 @@ mod tests {
&0u32,
&treasury_addr,
);
client.configure_dispute_system(&admin, &arbitrator, &50i128);
client.configure_dispute_system(&admin, &arbitrator, &25i128);
client.set_max_stake(&1_000i128);

Expand Down
8 changes: 6 additions & 2 deletions contracts/game_contract/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,14 +305,18 @@ fn test_configure_fees_permissioned() {

// Update fees as admin
let new_treasury = Address::generate(&env);
client.configure_fees(&admin, &50, &new_treasury); // 5% fee
let mut admins = Vec::new(&env);
admins.push_back(admin.clone());
client.configure_fees(&admins, &50, &new_treasury); // 5% fee

// Verify update
// (In a real test we'd check storage or run a payout, but here we just ensure it doesn't panic)

// Attempt update as someone else should panic
let stranger = Address::generate(&env);
let res = client.try_configure_fees(&stranger, &100, &new_treasury);
let mut stranger_admins = Vec::new(&env);
stranger_admins.push_back(stranger.clone());
let res = client.try_configure_fees(&stranger_admins, &100, &new_treasury);
assert!(res.is_err());
}

Expand Down
Loading