From 3aab11e8a4df0e1514b518e98aef1a8d23a97d6a Mon Sep 17 00:00:00 2001 From: knightqueen-sketch <321944052+knightqueen-sketch@users.noreply.github.com> Date: Sat, 17 Oct 2026 20:12:44 +0000 Subject: [PATCH] simple: batch fix 4 issues Co-authored-by: Simple Agent --- contracts/escrow/src/tests/mod.rs | 1 + contracts/escrow/src/tests/player_freeze.rs | 208 ++++++++++++++++++ contracts/escrow/src/tests/protocol_config.rs | 60 +++++ contracts/oracle/src/lib.rs | 55 +++++ contracts/oracle/src/tests.rs | 41 ++++ .../src/components/match/CreateMatchForm.tsx | 15 +- frontend/src/test/match.test.tsx | 29 +++ 7 files changed, 407 insertions(+), 2 deletions(-) diff --git a/contracts/escrow/src/tests/mod.rs b/contracts/escrow/src/tests/mod.rs index 3699f74f..03663f53 100644 --- a/contracts/escrow/src/tests/mod.rs +++ b/contracts/escrow/src/tests/mod.rs @@ -33,6 +33,7 @@ mod oracle_validation; mod pagination; mod player_balance_history; mod player_freeze; +mod player_freeze_permissions; mod player_many_matches; mod referral; mod security; diff --git a/contracts/escrow/src/tests/player_freeze.rs b/contracts/escrow/src/tests/player_freeze.rs index 96d49816..fb704851 100644 --- a/contracts/escrow/src/tests/player_freeze.rs +++ b/contracts/escrow/src/tests/player_freeze.rs @@ -282,3 +282,211 @@ fn test_create_match_tournament_rejects_frozen_player2() { ); assert_eq!(result, Err(Ok(Error::ContractPaused))); } + +// ── create_match frozen-player guard ───────────────────────────────────────── + +#[test] +fn test_create_match_rejects_frozen_player1() { + let (env, contract_id, _oracle, player1, player2, token, _admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + client.admin_freeze_player(&player1, &reason(&env, "cheating")); + + let result = client.try_create_match( + &player1, + &player2, + &100, + &token, + &String::from_str(&env, "match"), + &Platform::Lichess, + ); + assert_eq!( + result, + Err(Ok(Error::ContractPaused)), + "frozen player1 must not be able to create a match" + ); +} + +#[test] +fn test_create_match_rejects_frozen_player2() { + let (env, contract_id, _oracle, player1, player2, token, _admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + client.admin_freeze_player(&player2, &reason(&env, "cheating")); + + let result = client.try_create_match( + &player1, + &player2, + &100, + &token, + &String::from_str(&env, "match"), + &Platform::Lichess, + ); + assert_eq!( + result, + Err(Ok(Error::ContractPaused)), + "a frozen player must not be named as opponent" + ); +} + +// ── deposit frozen-player guard ────────────────────────────────────────────── + +#[test] +fn test_deposit_rejects_frozen_player() { + let (env, contract_id, _oracle, player1, player2, token, _admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + let match_id = client.create_match( + &player1, + &player2, + &100, + &token, + &String::from_str(&env, "match"), + &Platform::Lichess, + ); + + client.admin_freeze_player(&player1, &reason(&env, "cheating")); + + let result = client.try_deposit(&match_id, &player1); + assert_eq!( + result, + Err(Ok(Error::ContractPaused)), + "frozen player must not be able to deposit" + ); +} + +// ── cancel_match allowed for frozen player ─────────────────────────────────── + +#[test] +fn test_cancel_match_allowed_for_frozen_player() { + let (env, contract_id, _oracle, player1, player2, token, _admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + let match_id = client.create_match( + &player1, + &player2, + &100, + &token, + &String::from_str(&env, "match"), + &Platform::Lichess, + ); + + client.admin_freeze_player(&player1, &reason(&env, "cheating")); + + client.cancel_match(&match_id, &player1); + + let m = client.get_match(&match_id); + assert_eq!( + m.state, + MatchState::Cancelled, + "frozen player must still be able to cancel their own pending match" + ); +} + +// ── claim_vested_payout allowed for frozen player ──────────────────────────── + +#[test] +fn test_claim_vested_payout_allowed_for_frozen_player() { + let (env, contract_id, _oracle, player1, player2, token, _admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + let match_id = client.create_match( + &player1, + &player2, + &100, + &token, + &String::from_str(&env, "match"), + &Platform::Lichess, + ); + + client.deposit(&match_id, &player1); + client.deposit(&match_id, &player2); + + client.admin_freeze_player(&player1, &reason(&env, "cheating")); + + // Freezing must not block a player from claiming an already-vested payout. + client.claim_vested_payout(&match_id, &player1); +} + +// ── admin_unfreeze_player restores all permissions ─────────────────────────── + +#[test] +fn test_admin_unfreeze_player_restores_create_match() { + let (env, contract_id, _oracle, player1, player2, token, _admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + client.admin_freeze_player(&player1, &reason(&env, "temp")); + client.admin_unfreeze_player(&player1); + + let result = client.try_create_match( + &player1, + &player2, + &100, + &token, + &String::from_str(&env, "match"), + &Platform::Lichess, + ); + assert!( + result.is_ok(), + "unfrozen player must be able to create a match again" + ); +} + +#[test] +fn test_admin_unfreeze_player_restores_deposit() { + let (env, contract_id, _oracle, player1, player2, token, _admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + let match_id = client.create_match( + &player1, + &player2, + &100, + &token, + &String::from_str(&env, "match"), + &Platform::Lichess, + ); + + client.admin_freeze_player(&player1, &reason(&env, "temp")); + client.admin_unfreeze_player(&player1); + + let result = client.try_deposit(&match_id, &player1); + assert!( + result.is_ok(), + "unfrozen player must be able to deposit again" + ); +} + +#[test] +fn test_admin_unfreeze_player_restores_cancel_and_claim() { + let (env, contract_id, _oracle, player1, player2, token, _admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + // Cancel path: freeze then unfreeze, then cancel must succeed. + let cancel_id = client.create_match( + &player1, + &player2, + &100, + &token, + &String::from_str(&env, "match"), + &Platform::Lichess, + ); + client.admin_freeze_player(&player1, &reason(&env, "temp")); + client.admin_unfreeze_player(&player1); + client.cancel_match(&cancel_id, &player1); + assert_eq!(client.get_match(&cancel_id).state, MatchState::Cancelled); + + // Claim path: freeze then unfreeze, then claim must succeed. + let claim_id = client.create_match( + &player1, + &player2, + &100, + &token, + &String::from_str(&env, "match"), + &Platform::Lichess, + ); + client.deposit(&claim_id, &player1); + client.deposit(&claim_id, &player2); + client.admin_freeze_player(&player1, &reason(&env, "temp")); + client.admin_unfreeze_player(&player1); + client.claim_vested_payout(&claim_id, &player1); +} diff --git a/contracts/escrow/src/tests/protocol_config.rs b/contracts/escrow/src/tests/protocol_config.rs index 53044d60..c5705716 100644 --- a/contracts/escrow/src/tests/protocol_config.rs +++ b/contracts/escrow/src/tests/protocol_config.rs @@ -179,3 +179,63 @@ fn test_set_protocol_config_rejected_for_non_admin() { "non-admin must not be able to set ProtocolConfig" ); } + +/// `set_protocol_config` must reject a `cancellation_fee_basis_points` above +/// 10_000 (more than 100% of the stake) with `Error::InvalidAmount`, and the +/// previously stored config must remain unchanged. +#[test] +fn test_set_protocol_config_rejects_cancellation_fee_above_max() { + let (env, contract_id, _oracle, _p1, _p2, _token, admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + let cfg_before = client.get_protocol_config(); + + let mut cfg = custom_config(&env, &admin); + cfg.cancellation_fee_basis_points = 10_001; + + let result = client.try_set_protocol_config(&cfg); + assert_eq!( + result, + Err(Ok(Error::InvalidAmount)), + "cancellation_fee_basis_points > 10_000 must be rejected" + ); + + let cfg_after = client.get_protocol_config(); + assert_eq!( + cfg_before, cfg_after, + "rejected config must not be stored" + ); +} + +/// `set_protocol_config` must accept `cancellation_fee_basis_points` exactly +/// at the 10_000 boundary. +#[test] +fn test_set_protocol_config_accepts_cancellation_fee_at_max() { + let (env, contract_id, _oracle, _p1, _p2, _token, admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + let mut cfg = custom_config(&env, &admin); + cfg.cancellation_fee_basis_points = 10_000; + + client.set_protocol_config(&cfg); + + let actual = client.get_protocol_config(); + assert_eq!(actual, cfg); +} + +/// `set_protocol_config` must accept a valid `cancellation_fee_basis_points` +/// below the bound and `get_protocol_config` must reflect it. +#[test] +fn test_set_protocol_config_accepts_valid_cancellation_fee() { + let (env, contract_id, _oracle, _p1, _p2, _token, admin) = setup(); + let client = EscrowContractClient::new(&env, &contract_id); + + let mut cfg = custom_config(&env, &admin); + cfg.cancellation_fee_basis_points = 250; + + client.set_protocol_config(&cfg); + + let actual = client.get_protocol_config(); + assert_eq!(actual.cancellation_fee_basis_points, 250); + assert_eq!(actual, cfg); +} diff --git a/contracts/oracle/src/lib.rs b/contracts/oracle/src/lib.rs index 3eb783a6..6fa614e1 100644 --- a/contracts/oracle/src/lib.rs +++ b/contracts/oracle/src/lib.rs @@ -1239,6 +1239,23 @@ impl OracleContract { page } + /// Return the list of all currently approved oracle addresses. + /// + /// This is the authoritative on-chain view of the approved-oracle set, + /// maintained alongside the per-oracle approval records by + /// `add_approved_oracle` / `remove_approved_oracle`. Monitoring tools can + /// read it directly instead of reconstructing the set from + /// `admin/ora_add` and `admin/ora_rm` events. + /// + /// Returns an empty `Vec` when no oracle has been approved. + pub fn get_approved_oracles(env: Env) -> Vec
{ + extend_instance_ttl(&env); + env.storage() + .instance() + .get(&DataKey::ApprovedOracles) + .unwrap_or(Vec::new(&env)) + } + /// Returns performance metrics and SLA status for a registered oracle. pub fn get_oracle_metrics(env: Env, oracle_address: Address) -> OracleMetrics { env.storage() @@ -1522,6 +1539,44 @@ impl OracleContract { .set(&DataKey::OracleEligibilityVersion, &version); } + /// Append `oracle` to the approved-oracle list if not already present. + /// Keeps `DataKey::ApprovedOracles` in sync with the per-oracle approval + /// records maintained by `add_approved_oracle`. + fn add_to_approved_list(env: &Env, oracle: &Address) { + let mut list: Vec
= env + .storage() + .instance() + .get(&DataKey::ApprovedOracles) + .unwrap_or(Vec::new(env)); + if !list.contains(oracle) { + list.push_back(oracle.clone()); + env.storage() + .instance() + .set(&DataKey::ApprovedOracles, &list); + } + } + + /// Remove `oracle` from the approved-oracle list, if present. Keeps + /// `DataKey::ApprovedOracles` in sync with the per-oracle approval + /// records maintained by `remove_approved_oracle`. + fn remove_from_approved_list(env: &Env, oracle: &Address) { + let list: Vec
= env + .storage() + .instance() + .get(&DataKey::ApprovedOracles) + .unwrap_or(Vec::new(env)); + let mut updated = Vec::new(env); + for i in 0..list.len() { + let addr = list.get(i).unwrap(); + if addr != *oracle { + updated.push_back(addr); + } + } + env.storage() + .instance() + .set(&DataKey::ApprovedOracles, &updated); + } + /// Slash `bps` basis points of `oracle`'s remaining stake, transferring /// the slashed amount to the admin (treasury). No-op if the oracle is not /// registered or has no remaining stake. Returns the amount slashed. diff --git a/contracts/oracle/src/tests.rs b/contracts/oracle/src/tests.rs index d0ea1e22..19bc8d16 100644 --- a/contracts/oracle/src/tests.rs +++ b/contracts/oracle/src/tests.rs @@ -3766,3 +3766,44 @@ fn test_pause_unpause_cycle_succeeds() { client.unpause(); assert_eq!(client.try_unpause(), Err(Ok(Error::InvalidPauseState))); } + +// #1728 — get_approved_oracles exposes the approved oracle set so monitoring +// tools do not have to reconstruct it from admin/ora_add and admin/ora_rm events. +#[test] +fn test_get_approved_oracles_tracks_additions_and_removals() { + let (env, contract_id, ..) = setup(); + let client = OracleContractClient::new(&env, &contract_id); + + // Initially empty. + assert_eq!(client.get_approved_oracles(), soroban_sdk::vec![&env]); + + let oracle_a = Address::generate(&env); + let oracle_b = Address::generate(&env); + + client.add_approved_oracle(&oracle_a); + client.add_approved_oracle(&oracle_b); + + // Both listed in insertion order. + assert_eq!( + client.get_approved_oracles(), + soroban_sdk::vec![&env, oracle_a.clone(), oracle_b.clone()] + ); + + // Re-adding an already-approved oracle must not duplicate it. + client.add_approved_oracle(&oracle_a); + assert_eq!( + client.get_approved_oracles(), + soroban_sdk::vec![&env, oracle_a.clone(), oracle_b.clone()] + ); + + // Removing one leaves only the other. + client.remove_approved_oracle(&oracle_a); + assert_eq!( + client.get_approved_oracles(), + soroban_sdk::vec![&env, oracle_b.clone()] + ); + + // Removing the last one leaves an empty list. + client.remove_approved_oracle(&oracle_b); + assert_eq!(client.get_approved_oracles(), soroban_sdk::vec![&env]); +} diff --git a/frontend/src/components/match/CreateMatchForm.tsx b/frontend/src/components/match/CreateMatchForm.tsx index 3ac8daf0..96a9b8e5 100644 --- a/frontend/src/components/match/CreateMatchForm.tsx +++ b/frontend/src/components/match/CreateMatchForm.tsx @@ -55,13 +55,24 @@ export function CreateMatchForm({ onSubmit }: CreateMatchFormProps) { function handleSubmit(ev: React.FormEvent) { ev.preventDefault(); - if (validate()) onSubmit(form); + if (validate()) { + onSubmit({ + ...form, + player2: form.player2.trim(), + stakeAmount: form.stakeAmount.trim(), + token: form.token.trim(), + gameId: form.gameId.trim(), + }); + } } const set = (field: keyof CreateMatchData) => (ev: React.ChangeEvent) => setForm(f => ({ ...f, [field]: ev.target.value })); + const setGameId = (ev: React.ChangeEvent) => + setForm(f => ({ ...f, gameId: ev.target.value.trim() })); + return (
@@ -86,7 +97,7 @@ export function CreateMatchForm({ onSubmit }: CreateMatchFormProps) {
- + {errors.gameId && {errors.gameId}}
{form.platform === 'lichess' && ( diff --git a/frontend/src/test/match.test.tsx b/frontend/src/test/match.test.tsx index 8362e4f8..90b580a5 100644 --- a/frontend/src/test/match.test.tsx +++ b/frontend/src/test/match.test.tsx @@ -71,6 +71,35 @@ describe('CreateMatchForm', () => { expect(screen.getAllByRole('alert').length).toBeGreaterThan(0); }); + it('shows a required error for a whitespace-only game ID and blocks submit', () => { + const onSubmit = vi.fn(); + render(); + fireEvent.change(screen.getByLabelText(/player 2/i), { target: { value: 'GBOB' } }); + fireEvent.change(screen.getByLabelText(/stake amount/i), { target: { value: '10' } }); + fireEvent.change(screen.getByLabelText(/token address/i), { target: { value: 'GTOK' } }); + fireEvent.change(screen.getByLabelText(/game id/i), { target: { value: ' ' } }); + fireEvent.click(screen.getByRole('button', { name: /create match/i })); + expect(onSubmit).not.toHaveBeenCalled(); + expect(screen.getByText(/required/i)).toBeInTheDocument(); + }); + + it('trims surrounding whitespace from the game ID before submitting', () => { + const onSubmit = vi.fn(); + render(); + fireEvent.change(screen.getByLabelText(/player 2/i), { target: { value: 'GBOB' } }); + fireEvent.change(screen.getByLabelText(/stake amount/i), { target: { value: '10' } }); + fireEvent.change(screen.getByLabelText(/token address/i), { target: { value: 'GTOK' } }); + fireEvent.change(screen.getByLabelText(/game id/i), { target: { value: ' abc12345 ' } }); + fireEvent.click(screen.getByRole('button', { name: /create match/i })); + expect(onSubmit).toHaveBeenCalledWith({ + player2: 'GBOB', + stakeAmount: '10', + token: 'GTOK', + gameId: 'abc12345', + platform: 'lichess', + }); + }); + it('shows error for invalid lichess game ID and blocks submit', () => { const onSubmit = vi.fn(); render();