Skip to content

Fix: 4 issues (#1726, #1727, #1728, #1729) - #1843

Merged
yahia008 merged 2 commits into
StellarCheckMate:mainfrom
knightqueen-sketch:simple/1726-1727-1728-1729
Oct 19, 2026
Merged

yahia008 merged 2 commits into
StellarCheckMate:mainfrom
knightqueen-sketch:simple/1726-1727-1728-1729

Conversation

@knightqueen-sketch

Copy link
Copy Markdown
Contributor

Closes #1726
Closes #1727
Closes #1728
Closes #1729

Issues

#1726 Test: frozen player cannot create or join a match but can cancel and claim

Category: Escrow Contract - Testing
Priority: Medium
Estimated Time: 1 hour

Description:
The freeze mechanism is described in detail in the README but the test coverage for exact allowed/disallowed operations for frozen players is incomplete.

Tasks:

  • Test: frozen player cannot create_match or be named as opponent
  • Test: frozen player cannot deposit
  • Test: frozen player CAN call cancel_match and claim_vested_payout
  • Test: admin_unfreeze_player restores all permissions

#1727 Frontend: game ID input accepts whitespace-only strings

Category: Frontend - Bug
Priority: Low
Estimated Time: 20 minutes

Description:
The create-match form does not trim or validate the game ID input. A whitespace-only string passes client-side validation and is sent to the contract, which rejects it with an opaque InvalidGameId error.

Tasks:

  • Trim the game ID on input and show a validation error for blank/whitespace values
  • Add a unit test

#1728 Enhancement: oracle contract should expose get_approved_oracles list

Category: Oracle Contract - Enhancement
Priority: Low
Estimated Time: 30 minutes

Description:
There is no function to list all approved oracle addresses. Monitoring tools must reconstruct the set from admin/ora_add and admin/ora_rm events.

Tasks:

  • Add get_approved_oracles() -> Vec<Address> view function
  • Maintain the list in instance storage alongside add_approved_oracle / remove_approved_oracle
  • Add a test

#1729 Fix: set_protocol_config does not validate cancellation_fee_bps upper bound

Category: Escrow Contract - Bug
Priority: Low
Estimated Time: 20 minutes

Description:
protocol_fee_bps is validated against 10_000 but cancellation_fee_bps has no upper-bound check. A value above 10_000 would send more than 100% of the stake as a cancellation fee, draining extra funds from the escrow.

Tasks:

  • Validate cancellation_fee_bps <= 10_000 and return Error::InvalidAmount if exceeded
  • Add a test

Summary

The four issues are independent and can be implemented in parallel. For #1726, the frozen-player rejection reuses Error::ContractPaused (per the require_player_not_frozen helper and the admin_freeze_player doc comment) — assert against that error rather than a dedicated PlayerFrozen variant. For #1729, mirror the existing protocol_fee_bps <= 10_000 check exactly and use Error::InvalidAmount as specified. For #1728, keep the approved-oracle list in instance storage alongside the existing approval records and make add/remove idempotent to avoid duplicates. For #1727, validateGameId already trims and returns 'Required' for blank input, so the main change is trimming on change/submit and adding the tests; ensure the LichessGamePicker onSelect path also trims.

Changes

  • Add tests to contracts/escrow/src/tests/player_freeze.rs covering the full frozen-player permission matrix. Freeze a player via admin_freeze_player, then assert: (a) create_match fails when the frozen player is player1, (b) create_match fails when the frozen player is named as player2 (opponent), (c) deposit fails for a frozen player on an existing Pending match, (d) cancel_match succeeds for a frozen player's own Pending match, (e) claim_vested_payout succeeds for a frozen player on an already-funded/completed match, and (f) after admin_unfreeze_player, create_match, deposit, cancel_match, and claim_vested_payout all succeed again. Use the existing setup()/setup_with_funded_match() fixtures and the existing frozen-player error (Error::ContractPaused, per the require_player_not_frozen helper). (contracts/escrow/src/tests/player_freeze.rs)
  • Ensure the player_freeze test module is registered (it already is via mod player_freeze;); no change needed unless the new tests require additional shared fixtures. If new fixtures are needed, add them here rather than duplicating setup code. (contracts/escrow/src/tests/mod.rs)
  • In set_protocol_config, add an upper-bound check mirroring the existing protocol_fee_bps validation: if cfg.cancellation_fee_basis_points > 10_000, return Err(Error::InvalidAmount). Place the check alongside the other config validations (self-referential treasury/fee_recipient, protocol_fee_bps) so all bounds are enforced before storage. (contracts/escrow/src/lib.rs)
  • Add tests to contracts/escrow/src/tests/protocol_config.rs: (a) set_protocol_config with cancellation_fee_basis_points = 10_001 returns Err(Ok(Error::InvalidAmount)) and the stored config is unchanged; (b) set_protocol_config with cancellation_fee_basis_points = 10_000 (boundary) succeeds; (c) a valid value below the bound is accepted and reflected by get_protocol_config. Use the existing custom_config helper. (contracts/escrow/src/tests/protocol_config.rs)
  • Add an instance-storage key (e.g. DataKey::ApprovedOracles) holding a Vec of approved oracle addresses. Update add_approved_oracle to append the address (avoiding duplicates) and remove_approved_oracle to remove it, keeping the list in sync with the existing per-oracle approval records. Implement pub fn get_approved_oracles(env: Env) -> Vec<Address> returning the stored list (empty vec if unset). (contracts/oracle/src/lib.rs)
  • Add a test in contracts/oracle/src/tests.rs verifying get_approved_oracles: initially empty, after add_approved_oracle for two addresses the list contains both in insertion order, after remove_approved_oracle for one the list contains only the remaining address, and re-adding an already-approved address does not duplicate it. (contracts/oracle/src/tests.rs)
  • In CreateMatchForm.tsx, trim the gameId value on change (store the trimmed value in form state) and ensure validateGameId already rejects blank/whitespace-only values with a 'Required' error (it does via trim). Additionally, in handleSubmit, submit the trimmed form data (trim gameId, player2, token, stakeAmount) so whitespace-only values can never reach onSubmit. Keep the existing validateGameId behavior and error messages. (frontend/src/components/match/CreateMatchForm.tsx)
  • Add a unit test in the CreateMatchForm describe block: fill player2, stakeAmount, token with valid values, set gameId to a whitespace-only string (e.g. ' '), click Create Match, assert onSubmit is not called and a 'Required' validation error is shown for the game ID. Also add a test that a game ID with surrounding whitespace (e.g. ' abc12345 ') is trimmed before being passed to onSubmit. (frontend/src/test/match.test.tsx)

Co-authored-by: Simple Agent <agent@simple.dev>
@drips-wave

drips-wave Bot commented Oct 17, 2026

Copy link
Copy Markdown

@knightqueen-sketch Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@yahia008
yahia008 merged commit 8826ffe into StellarCheckMate:main Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants