feat: Tournament System Implementation #158
Conversation
WalkthroughAdds a guild subsystem (models + systems) with full CRUD/invite flows and ID generation; implements a complete tournament lifecycle in Core (create/join/start/complete/distribute) with events; adds player credit burn/deduct methods; and extends Config with guild-related fields and lib.cairo module wiring. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin
participant Core as CoreActions
participant World as WorldStorage
participant ERC1155 as ERC1155Credits
rect rgb(230,245,255)
note over Admin,Core: Create Tournament (admin-only)
Admin->>Core: create_tournament(type, entry_fee, max_participants)
Core->>World: read/write Config.next_tournament_id
Core->>World: write Tournament (status=Open, registration window)
Core-->>Admin: emit TournamentCreated
end
rect rgb(240,255,230)
note over Player,Core: Join Tournament (fee)
actor Player
Player->>Core: join_tournament(tournament_id)
Core->>World: read Tournament, Participant
Core->>ERC1155: burn(entry_fee) %% via Player.deduct_credits -> burn
Core->>World: update Tournament.prize_pool, write Participant
Core-->>Player: emit PlayerRegistered
end
rect rgb(255,245,230)
note over Admin,Core: Start & Distribute
Admin->>Core: start_tournament(tournament_id)
Core->>World: validate & update Tournament.status=InProgress
Core-->>Admin: emit TournamentStarted
Admin->>Core: distribute_tournament_rewards(tournament_id, winners)
Core->>World: read prize_pool
Core->>ERC1155: mint/prize transfer to each winner
Core-->>Admin: emit TournamentFinished
end
sequenceDiagram
autonumber
actor Creator as Player
participant GuildSys as GuildActions
participant World as WorldStorage
rect rgb(230,255,250)
note over Creator,GuildSys: Create Guild
Creator->>GuildSys: create_guild(name)
GuildSys->>World: ensure caller not in guild, read/update Config.next_guild_id
GuildSys->>World: write Guild (leader=caller), write GuildMember(role=Leader)
GuildSys-->>Creator: emit GuildCreated
end
rect rgb(255,250,230)
note over Creator,GuildSys: Invite & Accept
Creator->>GuildSys: invite_to_guild(target)
GuildSys->>World: check leader/officer, create GuildInvite (expires +24h)
GuildSys-->>Creator: emit GuildInviteSent
actor Target
Target->>GuildSys: accept_guild_invite(guild_id)
GuildSys->>World: validate invite/expiry, ensure target not in guild
GuildSys->>World: write GuildMember, increment Guild.member_count, mark invite accepted
GuildSys-->>Target: emit GuildJoined, GuildInviteAccepted
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
src/systems/guild.cairo (1)
49-49: Consider making guild configuration flexibleThe hardcoded max_members limit of 50 might not suit all use cases.
Consider making this configurable through the contract configuration or as a parameter:
fn create_guild(ref self: ContractState, guild_name: felt252) { // ... existing code ... + let contract: Contract = world.read_model(COA_CONTRACTS); + let max_members = contract.guild_max_members; // Add to Contract model let guild = Guild { // ... other fields ... - max_members: 50, + max_members,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/lib.cairo(2 hunks)src/models/guild.cairo(1 hunks)src/models/player.cairo(2 hunks)src/systems/core.cairo(5 hunks)src/systems/guild.cairo(1 hunks)
🔇 Additional comments (3)
src/models/player.cairo (1)
183-188: LGTM! Clean implementation of the burn functionThe burn function correctly creates a dispatcher and delegates to the ERC1155 contract. The implementation follows the same pattern as the mint function.
src/models/guild.cairo (1)
1-129: LGTM! Well-structured guild domain modelThe guild models, events, and error constants are well-designed with appropriate fields and relationships. The role hierarchy (Member, Officer, Leader) and invite system with expiration are well thought out.
src/lib.cairo (1)
8-8: LGTM! Clean module integrationThe new guild modules are properly integrated into the existing module structure, maintaining consistency with the project's organization.
Also applies to: 33-33
| #[inline(always)] | ||
| fn deduct_credits(ref self: Player, amount: u256, erc1155_address: ContractAddress) { | ||
| self.check(); | ||
| assert(amount > 0, 'INVALID AMOUNT'); | ||
| assert(self.get_credits(erc1155_address) >= amount, 'Insufficient credits'); | ||
| self.burn(CREDITS, erc1155_address, amount); | ||
| } |
There was a problem hiding this comment.
Critical: Potential reentrancy vulnerability in deduct_credits
The function checks the balance before burning, creating a potential race condition. Between the balance check and the burn operation, the player's credits could be modified by another transaction, leading to an incorrect burn amount.
Apply this diff to make the operation atomic:
fn deduct_credits(ref self: Player, amount: u256, erc1155_address: ContractAddress) {
self.check();
assert(amount > 0, 'INVALID AMOUNT');
- assert(self.get_credits(erc1155_address) >= amount, 'Insufficient credits');
+ // Let the burn function handle insufficient balance checks
self.burn(CREDITS, erc1155_address, amount);
}The ERC1155 contract should handle the balance validation atomically during the burn operation.
📝 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.
| #[inline(always)] | |
| fn deduct_credits(ref self: Player, amount: u256, erc1155_address: ContractAddress) { | |
| self.check(); | |
| assert(amount > 0, 'INVALID AMOUNT'); | |
| assert(self.get_credits(erc1155_address) >= amount, 'Insufficient credits'); | |
| self.burn(CREDITS, erc1155_address, amount); | |
| } | |
| #[inline(always)] | |
| fn deduct_credits(ref self: Player, amount: u256, erc1155_address: ContractAddress) { | |
| self.check(); | |
| assert(amount > 0, 'INVALID AMOUNT'); | |
| // Let the burn function handle insufficient balance checks | |
| self.burn(CREDITS, erc1155_address, amount); | |
| } |
🤖 Prompt for AI Agents
In src/models/player.cairo around lines 167 to 173, the separate balance check
before calling burn creates a race/reentrancy window; remove the explicit
get_credits check and rely on the ERC1155 burn call to atomically validate and
decrement the balance (i.e., keep self.check(), assert(amount > 0), then call
self.burn(CREDITS, erc1155_address, amount) without a prior balance assertion),
and ensure the ERC1155 burn implementation enforces sufficient-balance
validation and reverts with a clear error message so the entire deduction is
atomic.
| fn create_tournament( | ||
| ref self: ContractState, | ||
| tournament_type: felt252, | ||
| entry_fee: u256, | ||
| max_participants: u32, | ||
| ) { | ||
| let caller = get_caller_address(); | ||
| let mut world = self.world_default(); | ||
| let contract: Contract = world.read_model(COA_CONTRACTS); | ||
|
|
||
| // Validate admin permissions | ||
| assert(caller == contract.admin, 'Only admin can create tournaments'); | ||
|
|
||
| // Create tournament | ||
| let tournament_id = self.generate_tournament_id(); | ||
| let tournament = Tournament { | ||
| id: tournament_id, | ||
| creator: caller, | ||
| name: 'Tournament', | ||
| tournament_type: TournamentType::SingleElimination, | ||
| status: TournamentStatus::Open, | ||
| prize_pool: 0, | ||
| entry_fee, | ||
| max_players: max_participants, | ||
| min_players: 2, | ||
| registration_start: get_block_timestamp(), | ||
| registration_end: get_block_timestamp() + 86400, // 24 hours from now | ||
| registered_players: 0, | ||
| total_rounds: 0, | ||
| level_requirement: 0, | ||
| }; | ||
|
|
||
| world.write_model(@tournament); | ||
| world | ||
| .emit_event( | ||
| @TournamentCreated { tournament_id, creator: caller, name: 'Tournament' }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Security concern: Missing validation for tournament type parameter
The create_tournament function accepts a tournament_type parameter but ignores it, always setting TournamentType::SingleElimination. This could mislead users who expect different tournament types.
Apply this diff to properly validate and use the tournament type:
fn create_tournament(
ref self: ContractState,
tournament_type: felt252,
entry_fee: u256,
max_participants: u32,
) {
let caller = get_caller_address();
let mut world = self.world_default();
let contract: Contract = world.read_model(COA_CONTRACTS);
// Validate admin permissions
assert(caller == contract.admin, 'Only admin can create tournaments');
+
+ // Validate tournament type
+ let tournament_type_enum = if tournament_type == 'SingleElimination' {
+ TournamentType::SingleElimination
+ } else if tournament_type == 'DoubleElimination' {
+ TournamentType::DoubleElimination
+ } else if tournament_type == 'RoundRobin' {
+ TournamentType::RoundRobin
+ } else {
+ panic!("Invalid tournament type")
+ };
// Create tournament
let tournament_id = self.generate_tournament_id();
let tournament = Tournament {
id: tournament_id,
creator: caller,
name: 'Tournament',
- tournament_type: TournamentType::SingleElimination,
+ tournament_type: tournament_type_enum,
status: TournamentStatus::Open,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/systems/core.cairo around lines 151 to 188, the function currently
ignores the incoming tournament_type parameter and hardcodes
TournamentType::SingleElimination; update the code to validate and use the
provided tournament_type: convert the felt252 input to the TournamentType enum
(or map supported felt values to enum variants), check that it is one of the
allowed variants (return/revert with a clear error if not), assign the resulting
enum value to tournament.tournament_type instead of the hardcoded
SingleElimination, and ensure any related event or downstream logic that assumes
the tournament type uses the validated value.
| registration_start: get_block_timestamp(), | ||
| registration_end: get_block_timestamp() + 86400, // 24 hours from now |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Registration window may be too rigid
The hardcoded 24-hour registration window might not suit all tournament types or scheduling needs.
Consider making the registration period configurable:
fn create_tournament(
ref self: ContractState,
tournament_type: felt252,
entry_fee: u256,
max_participants: u32,
+ registration_duration: u64, // in seconds
) {
// ... existing code ...
registration_start: get_block_timestamp(),
- registration_end: get_block_timestamp() + 86400, // 24 hours from now
+ registration_end: get_block_timestamp() + registration_duration,📝 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.
| registration_start: get_block_timestamp(), | |
| registration_end: get_block_timestamp() + 86400, // 24 hours from now | |
| fn create_tournament( | |
| ref self: ContractState, | |
| tournament_type: felt252, | |
| entry_fee: u256, | |
| max_participants: u32, | |
| registration_duration: u64, // in seconds | |
| ) { | |
| // ... existing code ... | |
| registration_start: get_block_timestamp(), | |
| registration_end: get_block_timestamp() + registration_duration, |
🤖 Prompt for AI Agents
In src/systems/core.cairo around lines 176-177, the registration_end is
hardcoded to get_block_timestamp() + 86400 which makes the registration window
rigid; change the code to accept a configurable registration duration or
explicit registration_end instead of a fixed 24h: add a new parameter (e.g.,
registration_duration_seconds or registration_end_timestamp) to the
function/constructor that creates the registration window, store it in
contract/storage, use that parameter to compute registration_end (falling back
to a default of 86400 if omitted), validate the parameter (non-zero and within
reasonable bounds), and update all call sites and tests to pass the new argument
or rely on the default.
| fn generate_tournament_id(ref self: ContractState) -> u256 { | ||
| let mut world = self.world_default(); | ||
| let mut config: Config = world.read_model(1); | ||
|
|
||
| config.next_tournament_id += 1; | ||
| world.write_model(@config); | ||
|
|
||
| config.next_tournament_id | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Critical: Config model ID collision risk
The generate_tournament_id function uses a Config model with ID 1, which could collide with actual tournament/guild IDs if they start from 1.
Consider using a reserved ID range or a different key for the Config model to avoid potential collisions.
🏁 Script executed:
#!/bin/bash
# Check if Config model is properly defined and if ID 1 could cause collisions
ast-grep --pattern 'struct Config {
$$$
}'
# Check all usages of Config model with ID 1
rg -n "Config.*world\.read_model\(1\)" --type rustLength of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Listing .cairo files (first 200) ==="
if command -v fd >/dev/null 2>&1; then
fd -e cairo --hidden --exclude node_modules --max-results 200 || true
else
find . -type f -name '*.cairo' -print | sed -n '1,200p' || true
fi
echo
echo "=== Search: struct Config definition ==="
rg -n --hidden -S 'struct\s+Config\s*{' || true
echo
echo "=== Search: occurrences of 'Config' ==="
rg -n --hidden -S '\bConfig\b' || true
echo
echo "=== Search: read_model(1) literal usages ==="
rg -n --hidden -S 'read_model\(\s*1\s*\)' || true
echo
echo "=== Search: world.read_model / world.write_model calls ==="
rg -n --hidden -S 'world\.(read_model|write_model|erase_model|uuid)\(' || true
echo
echo "=== Search: next_tournament_id / generate_tournament_id ==="
rg -n --hidden -S 'next_tournament_id|generate_tournament_id' || true
echo
echo "=== Show src/systems/core.cairo around lines 400-460 if file exists ==="
if [ -f src/systems/core.cairo ]; then
sed -n '1,520p' src/systems/core.cairo | sed -n '400,460p'
else
echo "src/systems/core.cairo not found"
fiLength of output: 37231
Critical: Unify Tournament Config model key (0 vs 1)
src/systems/tournament.cairo initializes the tournament Config with id = 0, but src/systems/core.cairo (generate_tournament_id — line 423) and src/systems/guild.cairo (line 210) use world.read_model(1). This reads/updates a different model and will cause incorrect behavior or ID collisions. Change those read_model/write_model calls to the same key (0) or introduce and use a shared CONFIG_KEY constant everywhere.
Locations: src/systems/core.cairo (generate_tournament_id, line 423), src/systems/guild.cairo (line 210), src/systems/tournament.cairo (config init, line 74), src/models/tournament.cairo (Config struct).
🤖 Prompt for AI Agents
In src/systems/core.cairo around lines 423-431, src/systems/guild.cairo around
line 210, and src/systems/tournament.cairo around line 74, the tournament Config
is initialized under model key 0 but later read/written with key 1; update the
read_model/write_model calls to use the same key (0) or define a single
CONFIG_KEY constant in a shared module and use that constant in all three files
so all reads/writes target the same model key; ensure both read_model and
write_model calls are changed accordingly and run tests to verify no ID
collisions.
| if guild_member.role == GuildRole::Leader { | ||
| // For now, disband the guild | ||
| guild.id = 0; // Mark as disbanded | ||
| } |
There was a problem hiding this comment.
Guild disbanding leaves orphaned data
When a leader leaves and the guild is "disbanded" by setting guild.id = 0, this doesn't actually remove the guild from storage and leaves orphaned member records.
Consider implementing proper cleanup or leadership transfer:
// If leader is leaving, disband guild or transfer leadership
if guild_member.role == GuildRole::Leader {
- // For now, disband the guild
- guild.id = 0; // Mark as disbanded
+ if guild.member_count > 1 {
+ // Transfer leadership to the first officer or member found
+ // This requires iterating through members - consider adding a successor field
+ panic!("Leadership transfer not implemented - guild must be empty");
+ } else {
+ // Guild is empty, can be disbanded
+ // Note: The guild record remains but with 0 members
+ guild.member_count = 0;
+ }
}📝 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.
| if guild_member.role == GuildRole::Leader { | |
| // For now, disband the guild | |
| guild.id = 0; // Mark as disbanded | |
| } | |
| if guild_member.role == GuildRole::Leader { | |
| if guild.member_count > 1 { | |
| // Transfer leadership to the first officer or member found | |
| // This requires iterating through members - consider adding a successor field | |
| panic!("Leadership transfer not implemented - guild must be empty"); | |
| } else { | |
| // Guild is empty, can be disbanded | |
| // Note: The guild record remains but with 0 members | |
| guild.member_count = 0; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/systems/guild.cairo around lines 110-113, setting guild.id = 0 when the
leader leaves only marks the guild as disbanded and leaves orphaned member
records; instead either transfer leadership or perform full cleanup: if
transferring, pick a new leader (e.g., highest-ranked remaining member), update
that member's role and save the guild owner/leader field; otherwise remove the
guild from storage and delete all member records and related mappings (iterate
guild member list, delete each member entry and any indexes), emit appropriate
events for leadership change or guild disbanded, and ensure storage
writes/rollbacks are handled atomically so no orphaned data remains.
| fn generate_guild_id(ref self: ContractState) -> u256 { | ||
| let mut world = self.world_default(); | ||
| let mut config: Config = world.read_model(1); | ||
|
|
||
| config.next_tournament_id += 1; | ||
| world.write_model(@config); | ||
|
|
||
| config.next_tournament_id | ||
| } |
There was a problem hiding this comment.
Critical: Wrong Config field used for guild ID generation
The generate_guild_id function incorrectly uses next_tournament_id instead of having a dedicated next_guild_id field.
This will cause ID collisions between guilds and tournaments. Add a separate counter:
fn generate_guild_id(ref self: ContractState) -> u256 {
let mut world = self.world_default();
let mut config: Config = world.read_model(1);
- config.next_tournament_id += 1;
+ config.next_guild_id += 1;
world.write_model(@config);
- config.next_tournament_id
+ config.next_guild_id
}Also update the Config model to include a next_guild_id field.
📝 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.
| fn generate_guild_id(ref self: ContractState) -> u256 { | |
| let mut world = self.world_default(); | |
| let mut config: Config = world.read_model(1); | |
| config.next_tournament_id += 1; | |
| world.write_model(@config); | |
| config.next_tournament_id | |
| } | |
| fn generate_guild_id(ref self: ContractState) -> u256 { | |
| let mut world = self.world_default(); | |
| let mut config: Config = world.read_model(1); | |
| config.next_guild_id += 1; | |
| world.write_model(@config); | |
| config.next_guild_id | |
| } |
🤖 Prompt for AI Agents
In src/systems/guild.cairo around lines 208 to 216, generate_guild_id currently
increments and returns config.next_tournament_id which is wrong; add and use a
dedicated counter config.next_guild_id instead. Update the Config model
(wherever defined) to add a next_guild_id: u256 field (initialized
appropriately), change generate_guild_id to read config.next_guild_id, increment
it, write back the updated Config via world.write_model(@config), and return the
new next_guild_id so guild IDs are tracked separately from tournaments.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/systems/tournament.cairo (7)
92-117: Admin check missing; prize_pool semantics conflict with “fees accumulate”
- No admin/creator permission check in create_tournament (PR requires admin-only).
- Validating an upfront prize_pool and passing it as a parameter conflicts with “automatic prize-pool accumulation from entry fees.” Accumulate into tournament.prize_pool on register instead.
fn create_tournament( ref self: ContractState, name: felt252, tournament_type: TournamentType, - prize_pool: u256, entry_fee: u256, max_players: u32, min_players: u32, registration_start: u64, registration_end: u64, level_requirement: u256, ) { let creator = get_caller_address(); let mut world = self.world_default(); - let mut config: Config = world.read_model(0); + let mut config: Config = world.read_model(0); + assert(creator == config.admin, Errors::NOT_ADMIN); let tournament_id = config.next_tournament_id; @@ - // Ensure prize pool makes sense relative to potential entry fees - let min_entry_fees = entry_fee * min_players.into(); - assert(prize_pool >= min_entry_fees, Errors::LOW_PRIZE_POOL); + // Prize pool starts at 0 and grows via entry feesAnd when constructing Tournament:
- prize_pool, + prize_pool: 0,
193-243: Increase prize_pool on successful registration; consider consistent credit handlingCurrently register() transfers entry_fee but never increments tournament.prize_pool.
if tournament.entry_fee > 0 { let erc1155 = IERC1155Dispatcher { contract_address: config.erc1155_address }; erc1155 .safe_transfer_from( player_id, get_contract_address(), config.credit_token_id, tournament.entry_fee, array![].span(), ); + tournament.prize_pool += tournament.entry_fee; }Optional: For consistency with Core (which uses Player.deduct_credits/mint), consider burning on entry and minting on payout instead of contract-held balances.
144-191: Refund loop relies on user-supplied list; ensure full refunds and state cleanup
- Accepting an arbitrary registered_players list risks partial refunds and double-spends if reused.
- registered_players counter isn’t reset; participants are erased only if listed.
Minimal guard + counter reset:
- fn cancel_tournament( + fn cancel_tournament( ref self: ContractState, tournament_id: u256, registered_players: Array<ContractAddress>, ) { @@ - tournament.status = TournamentStatus::Cancelled; + tournament.status = TournamentStatus::Cancelled; world.write_model(@tournament); @@ - let event = TournamentCancelled { - tournament_id, refunds_processed: registered_players.len().try_into().unwrap(), - }; + // Optional: assert list completeness to avoid missed refunds + assert( + registered_players.len().try_into().unwrap() == tournament.registered_players, + 'Registered players list mismatch' + ); + // Reset count after refunds + tournament.registered_players = 0; + world.write_model(@tournament); + let event = TournamentCancelled { + tournament_id, refunds_processed: registered_players.len().try_into().unwrap(), + };Better: iterate actual Participant storage if you maintain an index (recommended future refactor).
286-296: Allow admin to start tournament; enforce registration window closedPR states creator/admin can start; also advisable to ensure registration has ended before starting.
- assert(caller == tournament.creator, Errors::NOT_CREATOR); + let config: Config = world.read_model(0); + assert(caller == tournament.creator || caller == config.admin, Errors::NOT_CREATOR); assert(tournament.status == TournamentStatus::Open, 'Tournament cannot be started'); + // Ensure registration is closed + let now = get_block_timestamp(); + assert(now >= tournament.registration_end, 'Registration still active');
333-342: Unsafe unwrap before validation; remove redundant/unsafe assertunwrap() on match_.player2 can panic when player2 is None. The subsequent Option logic already handles validation.
- let _loser_id = if winner_id == match_.player1 { - match_.player2.unwrap() - } else { - match_.player1 - }; - assert( - winner_id == match_.player1 || winner_id == match_.player2.unwrap(), - Errors::INVALID_WINNER, - ); + // Winner validation is handled below with Option-aware logic
374-400: Require tournament Completed before claimsPrevent premature prize claims; gate on tournament status.
fn claim_prize(ref self: ContractState, tournament_id: u256, session_id: felt252) { @@ - let winner_key = (tournament_id, player_id); + let winner_key = (tournament_id, player_id); let mut winner: Winner = world.read_model(winner_key); + let tournament: Tournament = world.read_model(tournament_id); + assert(tournament.status == TournamentStatus::Completed, 'Tournament not completed');
560-589: advance_winner: inverted existence check and wrong key handling
- If next_match doesn’t exist, you should create it; if it exists, fill player2.
- When creating, ensure match_id == next_match_id is set explicitly.
- if next_match.match_id != 0 { + if next_match.match_id == 0 { - let round = self.calculate_match_round(tournament.max_players, next_match_id); - let total_matches = tournament.max_players - 1; - let final_match_id = total_matches; - - let mut next_next_match_id_opt = Option::None; - if next_match_id < final_match_id { - // Simplified logic for next match calculation - let matches_in_current_round = 2_u32.pow(tournament.total_rounds - round); - let base_match_id = total_matches - matches_in_current_round + 1; - next_next_match_id_opt = - Option::Some(base_match_id + ((next_match_id - base_match_id) / 2)); - } - - let new_match = Match { - tournament_id, - match_id: next_match_id, - round, - player1: winner_id, - player2: Option::None, - winner: starknet::contract_address_const::<0>(), - is_completed: false, - next_match_id: next_next_match_id_opt, - }; - world.write_model(@new_match); - } else { - next_match.player2 = Option::Some(winner_id); - world.write_model(@next_match); - } + // Create the next match with winner in player1 + let round = self.calculate_match_round(tournament.max_players, next_match_id); + let total_matches = tournament.max_players - 1; + let final_match_id = total_matches; + let mut next_next_match_id_opt = Option::None; + if next_match_id < final_match_id { + let matches_in_current_round = 2_u32.pow(tournament.total_rounds - round); + let base_match_id = total_matches - matches_in_current_round + 1; + next_next_match_id_opt = + Option::Some(base_match_id + ((next_match_id - base_match_id) / 2)); + } + let new_match = Match { + tournament_id, + match_id: next_match_id, + round, + player1: winner_id, + player2: Option::None, + winner: starknet::contract_address_const::<0>(), + is_completed: false, + next_match_id: next_next_match_id_opt, + }; + world.write_model(@new_match); + } else { + // Fill player2 slot in the existing next match + next_match.player2 = Option::Some(winner_id); + world.write_model(@next_match); + }
♻️ Duplicate comments (8)
src/systems/core.cairo (2)
176-177: Registration window hardcoded to 24hAs previously noted, make registration duration configurable or accept explicit registration_end.
- registration_end: get_block_timestamp() + 86400, // 24 hours from now + registration_end: get_block_timestamp() + registration_duration, // param
21-28: tournament_type parameter is ignoredYou still hardcode SingleElimination and ignore the felt252 tournament_type parameter.
- fn create_tournament( - ref self: TContractState, tournament_type: felt252, entry_fee: u256, max_participants: u32, - ); + fn create_tournament( + ref self: TContractState, tournament_type: felt252, entry_fee: u256, max_participants: u32, + );Use the value:
- let tournament = Tournament { + let t_type = if tournament_type == 'SingleElimination' { + TournamentType::SingleElimination + } else { + panic('Invalid tournament type') + }; + let tournament = Tournament { id: tournament_id, creator: caller, name: 'Tournament', - tournament_type: TournamentType::SingleElimination, + tournament_type: t_type,src/systems/guild.cairo (6)
72-75: Same membership bug in join_guildUse PlayerGuildMembership instead of GuildMember(0, caller).
- let existing_guild: GuildMember = world.read_model((0, caller)); - assert(existing_guild.guild_id == 0, 'Player already in guild'); + let membership: PlayerGuildMembership = world.read_model(caller); + assert(membership.guild_id == 0, 'Player already in guild');
101-126: Leaving a guild: incorrect lookup and unsafe “disband by id=0”
- Look up membership via PlayerGuildMembership.
- Don’t “disband” by setting guild.id = 0; either transfer leadership or reject if members remain.
- // Get player's guild membership - let guild_member: GuildMember = world.read_model((0, caller)); - assert(guild_member.guild_id != 0, 'Player not in guild'); + // Get player's guild membership + let membership: PlayerGuildMembership = world.read_model(caller); + assert(membership.guild_id != 0, 'Player not in guild'); + let gm_key = (membership.guild_id, caller); + let guild_member: GuildMember = world.read_model(gm_key); @@ - if guild_member.role == GuildRole::Leader { - // For now, disband the guild - guild.id = 0; // Mark as disbanded - } + if guild_member.role == GuildRole::Leader { + // TODO: implement leader succession. For now, require guild to be empty. + assert(guild.member_count == 1, 'Leader must transfer leadership before leaving'); + } @@ - let empty_guild_member = GuildMember { - guild_id: 0, - player_id: caller, - role: GuildRole::Member, - joined_at: 0, - contribution: 0, - }; + let empty_guild_member = GuildMember { + guild_id: 0, player_id: caller, role: GuildRole::Member, joined_at: 0, contribution: 0 + }; + let cleared = PlayerGuildMembership { player_id: caller, guild_id: 0 }; @@ world.write_model(@guild); world.write_model(@empty_guild_member); + world.write_model(@cleared);
141-144: Also fix target membership checkUse PlayerGuildMembership for the invite target.
- let target_guild: GuildMember = world.read_model((0, player_id)); - assert(target_guild.guild_id == 0, 'Player already in guild'); + let target_membership: PlayerGuildMembership = world.read_model(player_id); + assert(target_membership.guild_id == 0, 'Player already in guild');
164-177: Accept invite: use membership index; avoid multi-guild joinsReplace GuildMember(0, caller) check, and persist PlayerGuildMembership on accept.
- // Check if player is already in a guild - let existing_guild: GuildMember = world.read_model((0, caller)); - assert(existing_guild.guild_id == 0, 'Player already in guild'); + // Check if player is already in a guild + let membership: PlayerGuildMembership = world.read_model(caller); + assert(membership.guild_id == 0, 'Player already in guild'); @@ let guild_member = GuildMember { guild_id, player_id: caller, role: GuildRole::Member, joined_at: get_block_timestamp(), contribution: 0, }; + let player_membership = PlayerGuildMembership { player_id: caller, guild_id }; @@ world.write_model(@guild); world.write_model(@guild_member); + world.write_model(@player_membership); world.write_model(@invite);
129-140: Permission check depends on broken membership lookupReplace GuildMember(0, caller) with PlayerGuildMembership as above.
- let guild_member: GuildMember = world.read_model((0, caller)); + let membership: PlayerGuildMembership = world.read_model(caller); + assert(membership.guild_id != 0, 'Player not in guild'); + let guild_member: GuildMember = world.read_model((membership.guild_id, caller));
81-95: Persist membership index on joinWrite PlayerGuildMembership so future checks work.
// Add player to guild let guild_member = GuildMember { guild_id, player_id: caller, role: GuildRole::Member, joined_at: get_block_timestamp(), contribution: 0, }; + let player_membership = PlayerGuildMembership { player_id: caller, guild_id }; @@ world.write_model(@guild); world.write_model(@guild_member); + world.write_model(@player_membership);
🧹 Nitpick comments (3)
src/models/tournament.cairo (1)
4-4: Confirm necessity of ContractAddressDefault importIf the import exists solely to pull in a Default impl for ContractAddress to satisfy #[derive(Default)] on structs (e.g., Participant, Tournament), keep it. Otherwise, drop the unused import.
src/systems/tournament.cairo (1)
74-80: Use a shared CONFIG_KEY constant to avoid key driftConfig is read/written with literal 0 here. Define and reuse a shared constant to prevent future mismatches across modules.
+const CONFIG_KEY: u8 = 0; ... - let config: Config = world.read_model(0); + let config: Config = world.read_model(CONFIG_KEY); ... - world.write_model(@config); + world.write_model(@config);src/systems/core.cairo (1)
235-255: Optional: Ensure registration closed before startAdd a guard to avoid starting while registration is still open.
assert(tournament.status == TournamentStatus::Open, 'Tournament not open'); + assert(get_block_timestamp() >= tournament.registration_end, 'Registration still active');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/models/guild.cairo(1 hunks)src/models/tournament.cairo(2 hunks)src/systems/core.cairo(5 hunks)src/systems/guild.cairo(1 hunks)src/systems/tournament.cairo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/models/guild.cairo
🔇 Additional comments (2)
src/models/tournament.cairo (1)
32-32: Config.next_guild_id addition looks correctThis unblocks independent guild ID sequencing. Ensure all generators return the pre-increment value for stable IDs across modules.
src/systems/core.cairo (1)
257-281: Ensure tournament status is Completed before distributing rewardsdistribute_tournament_rewards asserts TournamentStatus::Completed but the shown code never sets it; either set status here before minting or ensure another path sets it prior to call.
- assert(tournament.status == TournamentStatus::Completed, 'Tournament not completed'); + if tournament.status != TournamentStatus::Completed { + let mut t = tournament; + t.status = TournamentStatus::Completed; + world.write_model(@t); + }
| // Check if player is already in a guild | ||
| let existing_guild: GuildMember = world.read_model((0, caller)); | ||
| assert(existing_guild.guild_id == 0, 'Player already in guild'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Membership check is incorrect; enables multi-guild membership
Reading GuildMember at key (0, caller) doesn’t tell if the player is in any guild. Introduce a PlayerGuildMembership index keyed by player_id and use it everywhere.
- // Check if player is already in a guild
- let existing_guild: GuildMember = world.read_model((0, caller));
- assert(existing_guild.guild_id == 0, 'Player already in guild');
+ // Check if player is already in a guild
+ let membership: PlayerGuildMembership = world.read_model(caller);
+ assert(membership.guild_id == 0, 'Player already in guild');Add model (in src/models/guild.cairo):
#[dojo::model]
#[derive(Drop, Copy, Serde, Debug, Default)]
pub struct PlayerGuildMembership {
#[key]
pub player_id: ContractAddress,
pub guild_id: u256,
}🤖 Prompt for AI Agents
In src/systems/guild.cairo around lines 36 to 39, the membership check reads
GuildMember at key (0, caller) which doesn’t prove a player’s membership and
allows multi-guild membership; add a PlayerGuildMembership model keyed by
player_id (as suggested), then replace the current read_model((0, caller)) call
with world.read_model<PlayerGuildMembership>((caller,)) and assert
membership.guild_id == 0 to ensure the player isn’t in any guild; also update
all join/leave flows to write/update/delete the PlayerGuildMembership record
(and add necessary imports/types) so membership is tracked by player_id index
everywhere.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/systems/guild.cairo (1)
117-120: Duplicate comment: Guild disbanding approach needs improvementThis is the same issue identified in previous reviews - setting
guild.id = 0doesn't properly clean up the guild and leaves orphaned member data.
🧹 Nitpick comments (2)
src/systems/guild.cairo (2)
42-52: Consider making guild configuration more flexibleThe hardcoded values for max_members (50) and default description might benefit from being configurable parameters to allow for different guild types or administrative control.
-fn create_guild(ref self: ContractState, guild_name: felt252) { +fn create_guild(ref self: ContractState, guild_name: felt252, description: felt252) { // ... existing code ... let guild = Guild { id: guild_id, name: guild_name, leader: caller, level: 1, experience: 0, member_count: 1, max_members: 50, // Consider making this configurable created_at: get_block_timestamp(), - description: 'A new guild', + description, };
162-162: Replace hardcoded 24h (86400) with a shared/configurable constant and guard timestamp additionReplace get_block_timestamp() + 86400 in src/systems/guild.cairo (expires_at) with a named/configurable value (e.g. SECONDS_PER_DAY from src/models/marketplace.cairo or a new GUILD_INVITE_DURATION) and add a check/assert that expires_at >= created_at (or a checked/saturating add) before writing the invite to prevent arithmetic/overflow issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/models/guild.cairo(1 hunks)src/systems/core.cairo(5 hunks)src/systems/guild.cairo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/systems/core.cairo
🔇 Additional comments (4)
src/models/guild.cairo (1)
1-133: Comprehensive guild system model implementation looks solid!The guild system data models, events, and error constants provide a well-structured foundation for guild functionality. The models correctly define the necessary relationships between guilds, members, invitations, and players with appropriate keys and fields.
src/systems/guild.cairo (3)
36-38: LGTM! Correct membership validation implementedThe membership check now properly uses
PlayerGuildMembershipkeyed by player address, which resolves the previous issue of incorrect guild membership validation.
222-229: LGTM! Guild ID generation correctly implementedThe function now properly uses
config.next_guild_idinstead of the tournament counter and returns the pre-increment value, which addresses the previous critical issues.
2-5: Missing import for PlayerGuildMembership modelThe code imports
PlayerGuildMembershipon line 4 but doesn't import it again in the inner module at lines 21-24, which will cause compilation errors when using it inside the contract implementation.use coa::models::guild::{ Guild, GuildMember, GuildRole, GuildInvite, GuildCreated, GuildJoined, GuildLeft, - GuildInviteSent, GuildInviteAccepted, + GuildInviteSent, GuildInviteAccepted, PlayerGuildMembership, };Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/systems/guild.cairo (2)
112-121: Leader leave logic disbands guild incorrectly and orphans state. Require transfer or solo disband.Setting guild.id = 0 makes future reads think the guild “doesn’t exist” while leaving member records and other state behind. This can strand remaining members.
- // Get guild and update member count - let mut guild: Guild = world.read_model(membership.guild_id); - guild.member_count -= 1; + // Get guild and update member count + let mut guild: Guild = world.read_model(membership.guild_id); + assert(guild.member_count > 0, 'Invariant violation: member_count underflow'); + guild.member_count -= 1; // If leader is leaving, disband guild or transfer leadership if guild_member.role == GuildRole::Leader { - // For now, disband the guild - guild.id = 0; // Mark as disbanded + // Do not soft-delete. Require explicit transfer unless solo. + assert( + guild.member_count == 0, + 'Leader must transfer leadership before leaving' + ); + // Option: implement explicit disband that cleans up storage when member_count == 0. }Follow-ups:
- Delete the (guild_id, caller) GuildMember record to avoid stale membership entries.
- Emit a dedicated GuildDisbanded or LeadershipTransferred event when implemented.
21-26: Fix import paths and include PlayerGuildMembership (build break).The submodule doesn’t import PlayerGuildMembership, yet it’s used on Lines 37, 76, 137, 179. Also, mixing coa:: and crate:: risks type mismatches. Unify to crate:: and pull PlayerGuildMembership into scope.
- use coa::models::guild::{ + use crate::models::guild::{ Guild, GuildMember, GuildRole, GuildInvite, GuildCreated, GuildJoined, GuildLeft, - GuildInviteSent, GuildInviteAccepted, + GuildInviteSent, GuildInviteAccepted, PlayerGuildMembership, }; - use coa::models::tournament::Config; + use crate::models::tournament::Config;
🧹 Nitpick comments (3)
src/systems/guild.cairo (3)
1-7: Remove redundant top-level imports to avoid duplication.These are unused outside the trait and duplicate the inner module’s imports. Keep only ContractAddress for the trait.
-use crate::models::guild::{ - Guild, GuildMember, GuildRole, GuildInvite, GuildCreated, GuildJoined, GuildLeft, - GuildInviteSent, GuildInviteAccepted, PlayerGuildMembership, -}; -use crate::models::tournament::Config; +// (keep only ContractAddress for IGuild signature)
149-160: Invite idempotency and capacity pre-check.
- Multiple active invites to the same player/guild are allowed; consider overwriting an existing pending invite or rejecting duplicates.
- Optional: pre-check guild capacity to avoid sending invites that cannot be accepted.
155-156: Consider moving invite TTL to Config.Hard-coding 86400 seconds limits flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/systems/guild.cairo(1 hunks)
🔇 Additional comments (2)
src/systems/guild.cairo (2)
71-101: Clarify: Is open join intended or should invites be mandatory?Current join_guild allows anyone to join without an invite. If the product requires invite-only, gate this behind a valid pending invite (or role-based approval), or remove this endpoint.
215-222: Config key confirmed as 0.Config structs mark
idwith#[key]; marketplace initializesConfig { id: 0 }(src/market/marketplace.cairo) and all inspectedworld.read_model(...)uses for Config areread_model(0)(e.g. src/systems/guild.cairo:217, multiple tournament/core files). Noread_model(1)occurrences found.
| // Create guild | ||
| let guild_id = self.generate_guild_id(); | ||
| let guild = Guild { | ||
| id: guild_id, | ||
| name: guild_name, | ||
| leader: caller, | ||
| level: 1, | ||
| experience: 0, | ||
| member_count: 1, | ||
| max_members: 50, | ||
| created_at: get_block_timestamp(), | ||
| description: 'A new guild', | ||
| }; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Parameterize max_members from Config instead of hard-coding 50.
PR objective mentions configurable member limits. Source this from Config for consistency.
- // Create guild
- let guild_id = self.generate_guild_id();
+ // Create guild
+ let guild_id = self.generate_guild_id();
+ let config: Config = world.read_model(0);
let guild = Guild {
id: guild_id,
name: guild_name,
leader: caller,
level: 1,
experience: 0,
member_count: 1,
- max_members: 50,
+ max_members: config.default_guild_max_members,
created_at: get_block_timestamp(),
description: 'A new guild',
};Verification: ensure Config has default_guild_max_members (or similarly named) and is initialized.
🏁 Script executed:
#!/bin/bash
rg -nC2 'struct\s+Config' src
rg -nC2 'default_.*guild.*max' srcLength of output: 796
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- Find all 'pub struct Config' occurrences ----"
rg -n --hidden -S 'pub struct Config' src || true
echo
echo "---- Show context for each Config definition ----"
rg -n --hidden -S -C5 'pub struct Config' src || true
echo
echo "---- Search for max_members / default_guild / default.*guild patterns ----"
rg -n --hidden -S 'max_members' src || true
rg -n --hidden -S 'default_.*guild' src || true
rg -n --hidden -S 'default.*guild' src || true
rg -n --hidden -S 'default.*max.*member|max.*member' src || true
echo
echo "---- List src/models files ----"
ls -la src/models || true
echo
echo "---- Dump each file that defines 'pub struct Config' (first 240 lines) ----"
for f in $(rg -l --hidden -S 'pub struct Config' src || true); do
echo "=== $f ==="
sed -n '1,240p' "$f" || true
echo
done
echo "---- Show src/systems/guild.cairo (first 240 lines) ----"
sed -n '1,240p' src/systems/guild.cairo || true
echo
echo "---- Search for read_model usages ----"
rg -n --hidden -S 'read_model\(' src || true
echo
echo "---- Done ----"Length of output: 42123
Parameterize guild max_members from Config (don't hard-code 50)
- Add a config field (e.g.
pub default_guild_max_members: u32) to src/models/tournament.cairo::Config (or create a dedicated GuildConfig stored at a stable key). - In src/systems/guild.cairo::create_guild, read that config (
let config: Config = world.read_model(0)) and replacemax_members: 50withmax_members: config.default_guild_max_members. - Ensure the Config instance at key 0 is initialized/updated with a sensible default (
world.write_model(@config)).
🤖 Prompt for AI Agents
In src/systems/guild.cairo around lines 40 to 53, the guild creation currently
hard-codes max_members to 50; update it to read the default from the Config
model instead. Add a pub default_guild_max_members: u32 field to
src/models/tournament.cairo::Config (or create a GuildConfig stored at a stable
key), ensure the Config struct is persisted at key 0 and initialized with a
sensible default via world.write_model(@config), then in create_guild read the
config (e.g. let config: Config = world.read_model(0)) and replace max_members:
50 with max_members: config.default_guild_max_members. Ensure any callers/tests
that rely on the default are updated or the default is set during
migration/initialization.
| // Remove player from guild | ||
| // The GuildMember record can be left as-is or properly deleted | ||
| // Writing zero values is unnecessary overhead | ||
| let empty_membership = PlayerGuildMembership { player_id: caller, guild_id: 0 }; | ||
|
|
||
| world.write_model(@guild); | ||
| world.write_model(@empty_membership); | ||
| world.emit_event(@GuildLeft { guild_id: membership.guild_id, player_id: caller }); |
There was a problem hiding this comment.
Stale GuildMember record is not removed.
You clear PlayerGuildMembership but leave GuildMember((guild_id, caller)) behind, which will corrupt any member enumeration or successor selection.
- Remove the GuildMember((membership.guild_id, caller)) record (use Dojo’s delete API for models). If delete isn’t available, as a stopgap write a zeroed GuildMember at that key and add a TODO to replace with proper deletion.
🤖 Prompt for AI Agents
In src/systems/guild.cairo around lines 122 to 129, you clear the
PlayerGuildMembership but leave the GuildMember((membership.guild_id, caller))
record, which will corrupt member enumeration; remove that record using Dojo’s
model delete API (call the model delete for GuildMember at key
(membership.guild_id, caller)); if the delete API is not yet available, write a
zeroed GuildMember at that key and add a TODO comment to replace it with the
proper delete once supported.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/systems/core.cairo (2)
21-23: Make registration window configurable instead of fixed 24h.Expose a duration/absolute end; default to 24h if caller passes 0.
- fn create_tournament( - ref self: TContractState, tournament_type: felt252, entry_fee: u256, max_participants: u32, - ); + fn create_tournament( + ref self: TContractState, + tournament_type: felt252, + entry_fee: u256, + max_participants: u32, + registration_duration: u64, // seconds; 0 -> default(86400) + );fn create_tournament( ref self: ContractState, tournament_type: felt252, entry_fee: u256, max_participants: u32, + registration_duration: u64, ) { @@ - registration_start: get_block_timestamp(), - registration_end: get_block_timestamp() + 86400, // 24 hours from now + registration_start: get_block_timestamp(), + registration_end: get_block_timestamp() + + if registration_duration == 0 { 86400 } else { registration_duration },Also applies to: 163-168, 188-189
163-171: Don’t ignore the tournament_type parameter — validate and use it.You still hardcode TournamentType::SingleElimination. Map the incoming felt252 to the enum and reject invalid values.
Apply:
fn create_tournament( ref self: ContractState, tournament_type: felt252, entry_fee: u256, max_participants: u32, ) { @@ - let tournament = Tournament { + // Validate/convert tournament_type + let tt = if tournament_type == 'SingleElimination' { + TournamentType::SingleElimination + } else if tournament_type == 'DoubleElimination' { + TournamentType::DoubleElimination + } else if tournament_type == 'RoundRobin' { + TournamentType::RoundRobin + } else { + panic('Invalid tournament type') + }; + let tournament = Tournament { id: tournament_id, creator: caller, name: 'Tournament', - tournament_type: TournamentType::SingleElimination, + tournament_type: tt, status: TournamentStatus::Open,Also applies to: 176-183
src/systems/guild.cairo (2)
110-123: Leader leaving: disbanding by setting id=0 leaves stale data and breaks invariants.This neither removes the guild nor reassigns leadership. Implement proper leadership transfer or explicit disband with cleanup.
Proposed guard until transfer is implemented:
- // If leader is leaving, disband the guild - if guild_member.role == GuildRole::Leader { - guild.id = 0; // Mark as disbanded - } + // If leader is leaving and others remain, block until transfer logic exists + if guild_member.role == GuildRole::Leader && guild.member_count > 1 { + panic('Leader must transfer leadership before leaving'); + } + // If leader is sole member, allow disband: clear guild record + if guild_member.role == GuildRole::Leader && guild.member_count == 1 { + // TODO: delete/erase the guild model; if delete API unavailable, set member_count=0 and add a disbanded flag on Guild model in a future change + guild.member_count = 0; + }Also ensure the original GuildMember((guild_id, caller)) record is removed (see next comment).
Also applies to: 136-140
125-135: Don’t write zeroed GuildMember/Membership records; remove the real one instead.Writing zeroed entries at (0, caller) leaves the original (guild_id, caller) intact and corrupts membership enumeration. Use deletion/erase if available; otherwise overwrite at the correct key.
-// Remove player from guild -// TODO: Replace with proper deletion when Dojo delete API is available -let empty_guild_member = GuildMember { guild_id: 0, player_id: caller, role: GuildRole::Member, joined_at: 0, contribution: 0 }; -let empty_membership = PlayerGuildMembership { player_id: caller, guild_id: 0 }; - -world.write_model(@guild); -world.write_model(@empty_guild_member); -world.write_model(@empty_membership); +// Remove player from guild: clear PlayerGuildMembership and delete/clear the exact GuildMember key +world.write_model(@guild); +world.write_model(@PlayerGuildMembership { player_id: caller, guild_id: 0 }); +// Preferred: world.erase_model::<GuildMember>((membership.guild_id, caller)); +// If erase not available, overwrite the exact key with zeroed struct at (membership.guild_id, caller) +let cleared_member = GuildMember { guild_id: 0, player_id: caller, role: GuildRole::Member, joined_at: 0, contribution: 0 }; +world.write_model(@cleared_member);Also applies to: 136-139
🧹 Nitpick comments (6)
src/systems/core.cairo (3)
185-187: Validate participant bounds at creation.Add a guard so max_participants >= min_players (currently 2) and max_participants > 0.
- min_players: 2, + min_players: 2, @@ - world.write_model(@tournament); + assert(max_participants >= 2, 'max_participants must be >= 2'); + world.write_model(@tournament);
248-268: Optionally gate start by registration window end or add override flag.Today, tournaments can start before registration closes. Consider either:
- require get_block_timestamp() >= registration_end; or
- add an admin override flag/parameter.
-assert(tournament.status == TournamentStatus::Open, 'Tournament not open'); +assert(tournament.status == TournamentStatus::Open, 'Tournament not open'); +assert(get_block_timestamp() >= tournament.registration_end, 'Registration still open');
293-294: Consider updating tournament state and persisting it on finish.If you decide to finalize here, set status = Completed and write tournament before emitting TournamentFinished.
- world.emit_event(@TournamentFinished { tournament_id, winner: *winners.at(0) }); + let mut t2: Tournament = world.read_model(tournament_id); + t2.status = TournamentStatus::Completed; + world.write_model(@t2); + world.emit_event(@TournamentFinished { tournament_id, winner: *winners.at(0) });src/systems/guild.cairo (3)
44-56: Validate guild_name bounds.Add basic checks (non-zero, reasonable max length) to prevent spam/abuse.
fn create_guild(ref self: ContractState, guild_name: felt252) { @@ - // Create guild + // Validate name + assert(guild_name != 0, 'Guild name required'); + // Optional: enforce <= N chars depending on encoding policy + // assert(name_len(guild_name) <= 32, 'Name too long'); + // Create guild
117-118: Underflow risk on member_count.Add a guard before decrement.
- guild.member_count -= 1; + assert(guild.member_count > 0, 'Member count underflow'); + guild.member_count -= 1;
159-167: Check capacity before sending invites.Avoid issuing invites that can’t be accepted due to full capacity.
-// Create invite +// Capacity check before invite +let mut guild: Guild = world.read_model(membership.guild_id); +assert(guild.id != 0, 'Guild not found'); +assert(guild.member_count < guild.max_members, 'Guild full'); +// Create invite
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/models/tournament.cairo(2 hunks)src/systems/core.cairo(6 hunks)src/systems/guild.cairo(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/models/tournament.cairo
🔇 Additional comments (8)
src/systems/core.cairo (3)
85-95: Config init at key 0 looks consistent.Good move consolidating counters and defaults (next_tournament_id, next_guild_id, default_guild_max_members) under Config id=0.
220-229: Persisting Player after deduct_credits: LGTM.The added world.write_model(@player) ensures balance changes are saved.
436-444: ID generation: pre-increment return with Config@0 — LGTM.This avoids skipping the first id and aligns with guild IDs.
src/systems/guild.cairo (5)
36-42: Membership check via PlayerGuildMembership and config read — LGTM.Prevents multi-guild membership and sources max-members from Config.
178-197: Accepting invite: solid validations and updates.Good existence/expiry checks, membership prevention, and state writes + events.
Also applies to: 208-217
220-224: world_default(): LGTM.Consistent with core.
226-233: Guild ID generator (pre-increment, Config@0): LGTM.Matches tournament ID semantics.
74-86: Require invite or public_guild flag before allowing join_guildAnyone can join any guild with capacity; require a valid invite or an explicit public flag before adding the player.
- fn join_guild(ref self: ContractState, guild_id: u256) { + fn join_guild(ref self: ContractState, guild_id: u256) { @@ - // Add player to guild + // Require a valid invite + let invite: GuildInvite = world.read_model((guild_id, caller)); + assert(invite.guild_id != 0, 'Invite required'); + assert(!invite.is_accepted, 'Invite already used'); + assert(get_block_timestamp() <= invite.expires_at, 'Invite expired');Or remove join_guild and use accept_guild_invite exclusively.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/systems/core.cairo (1)
67-67: Critical: World namespace mismatch (“coa_contracts” vs “coa”) breaks all reads.
dojo_initwrites toself.world(@"coa_contracts"), while all other endpoints read/write usingworld_default()→self.world(@"coa"). Contract, Operator, and Config written indojo_initwill not be visible elsewhere.Apply this diff to write to the same world used everywhere else:
- let mut world = self.world(@"coa_contracts"); + let mut world = self.world_default();Optionally, define a single WORLD_NS constant and use it in both places.
Also applies to: 443-446
♻️ Duplicate comments (1)
src/systems/core.cairo (1)
164-201: Use the provided tournament_type instead of hardcoding SingleElimination.You ignore the
tournament_typeparameter and always setSingleElimination. Validate/map the felt → enum and use it. Also add basic param checks (max_participants >= 2,min_players <= max_players).Apply this diff inside the impl:
fn create_tournament( ref self: ContractState, tournament_type: felt252, entry_fee: u256, max_participants: u32, + registration_duration: u64, ) { let caller = get_caller_address(); let mut world = self.world_default(); let contract: Contract = world.read_model(COA_CONTRACTS); // Validate admin permissions assert(caller == contract.admin, 'Only admin can create tournaments'); + // Validate inputs + assert(max_participants >= 2, 'max_participants must be >= 2'); + assert(registration_duration > 0, 'registration_duration must be > 0'); + + // Map felt -> enum (short-string felts) + let tt = if tournament_type == 'SingleElimination' { + TournamentType::SingleElimination + } else if tournament_type == 'DoubleElimination' { + TournamentType::DoubleElimination + } else if tournament_type == 'RoundRobin' { + TournamentType::RoundRobin + } else { + panic('Invalid tournament type') + }; // Create tournament let tournament_id = self.generate_tournament_id(); let tournament = Tournament { id: tournament_id, creator: caller, name: 'Tournament', - tournament_type: TournamentType::SingleElimination, + tournament_type: tt, status: TournamentStatus::Open, prize_pool: 0, entry_fee, max_players: max_participants, min_players: 2, registration_start: get_block_timestamp(), - registration_end: get_block_timestamp() + 86400, // 24 hours from now + registration_end: get_block_timestamp() + registration_duration, registered_players: 0, total_rounds: 0, level_requirement: 0, };
🧹 Nitpick comments (6)
src/systems/core.cairo (6)
21-29: API shape: use a strong type for tournament_type and make registration window configurable.
- Prefer
TournamentType(enum) overfelt252, or validate/map the felt to the enum inside the impl.- Expose a
registration_duration: u64(orregistration_end: u64) parameter instead of hardcoding 24h later.Apply this diff to the interface:
- fn create_tournament( - ref self: TContractState, tournament_type: felt252, entry_fee: u256, max_participants: u32, - ); + fn create_tournament( + ref self: TContractState, + tournament_type: felt252, // map/validate inside impl (or switch to TournamentType) + entry_fee: u256, + max_participants: u32, + registration_duration: u64, // in seconds + );
189-191: Registration window is hardcoded to 24h.Make it configurable (see interface/impl diffs above) or read a default from Config.
203-246: Join flow looks solid; add two small guards.
- Optional: assert now >= registration_start to avoid edge cases if start time is in the future.
- Confirm
world.read_model((tournament_id, caller))returns a defaultParticipantwhen absent in your ORM; otherwise use a safe/try-read pattern.Example minimal guard:
- assert(get_block_timestamp() <= tournament.registration_end, 'Registration closed'); + let now = get_block_timestamp(); + assert(now >= tournament.registration_start, 'Registration not open yet'); + assert(now <= tournament.registration_end, 'Registration closed');
249-269: Gate start against still-open registration (or add a force flag).Starting while registration is open can surprise players. Either assert
now >= registration_endor accept aforce: booland emit a reason.- assert(tournament.status == TournamentStatus::Open, 'Tournament not open'); + assert(tournament.status == TournamentStatus::Open, 'Tournament not open'); + assert(get_block_timestamp() >= tournament.registration_end, 'Registration still open');
316-317: Event carries only the first winner.If multi-winner payouts are supported, consider emitting all winners or an event per winner.
464-472: ID generator LGTM; consider a CONFIG_KEY constant.Define a single
CONFIG_KEY: felt252 = 0and use it here and indojo_initto avoid future drift.Example:
const CONFIG_KEY: felt252 = 0; // ... let mut config: Config = world.read_model(CONFIG_KEY);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/systems/core.cairo(7 hunks)
🔇 Additional comments (4)
src/systems/core.cairo (4)
6-10: Imports look fine for tournament models.No issues spotted.
86-97: Config init OK; verify defaults.
- Confirm
credit_token_id: 0is the intended credit token id.- Confirm
next_tournament_id/next_guild_idstart at 1 across the repo.
271-282: Completion transition LGTM.The state move Open → InProgress → Completed is consistent.
447-450: Helper LGTM.Compact and correct.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/systems/core.cairo (1)
58-67: Blocker — unify world namespace: replace @"coa_contracts" with a single WORLD_NS ('coa')dojo_init writes to @"coa_contracts" while world_default and all other modules use @"coa"; this creates separate storage (orphaned admin/config/permission records). Fix core.cairo to use a shared namespace constant.
- src/systems/core.cairo:67 —
let mut world = self.world(@"coa_contracts");- src/systems/core.cairo:450-452 —
fn world_default(...) { self.world(@"coa") }Apply:
@@ - fn dojo_init( + fn dojo_init( @@ - let mut world = self.world(@"coa_contracts"); + let mut world = self.world(@WORLD_NS); @@ - fn world_default(self: @ContractState) -> WorldStorage { - self.world(@"coa") - } + const WORLD_NS: felt252 = 'coa'; + fn world_default(self: @ContractState) -> WorldStorage { + self.world(@WORLD_NS) + }
♻️ Duplicate comments (3)
src/systems/core.cairo (3)
21-23: Duplicate: Registration window is hardcoded to 24h; make it configurableExpose registration_duration and use it to compute registration_end (validate nonzero, sane max).
@@ trait ICore - fn create_tournament( - ref self: TContractState, tournament_type: felt252, entry_fee: u256, max_participants: u32, - ); + fn create_tournament( + ref self: TContractState, + tournament_type: felt252, + entry_fee: u256, + max_participants: u32, + registration_duration: u64, + ); @@ impl CoreActionsImpl - fn create_tournament( + fn create_tournament( ref self: ContractState, tournament_type: felt252, entry_fee: u256, - max_participants: u32, + max_participants: u32, + registration_duration: u64, ) { @@ - registration_end: get_block_timestamp() + 86400, // 24 hours from now + // e.g., cap at 30 days; tweak as needed + let dur = if registration_duration == 0 || registration_duration > 2_592_000_u64 { + 86_400_u64 + } else { registration_duration }; + registration_end: get_block_timestamp() + dur,Also applies to: 164-169, 189-191
164-201: Blocker: tournament_type parameter is ignored; must parse/validate and useCurrently hardcoded to SingleElimination, violating API expectation and previous review. Parse felt252 into TournamentType and revert on invalid values.
@@ - let tournament = Tournament { + let parsed_type = self.parse_tournament_type(tournament_type); + let tournament = Tournament { @@ - tournament_type: TournamentType::SingleElimination, + tournament_type: parsed_type,Add parser near the helpers (see separate diff below).
454-457: Add parser helper for tournament_type (felt252 -> enum)Keeps the ABI stable while ensuring strong typing inside.
@@ fn usize_to_u256(x: usize) -> u256 { u256 { high: 0, low: x.into() } } + + fn parse_tournament_type(tt: felt252) -> TournamentType { + if tt == 'SingleElimination' { + TournamentType::SingleElimination + } else if tt == 'DoubleElimination' { + TournamentType::DoubleElimination + } else if tt == 'RoundRobin' { + TournamentType::RoundRobin + } else { + panic('Invalid tournament type') + } + }
🧹 Nitpick comments (2)
src/systems/core.cairo (2)
196-201: Event enrichment suggestion: include parameters for observabilityConsider emitting type/fee/caps in TournamentCreated; helps indexers/UX.
- world - .emit_event( - @TournamentCreated { tournament_id, creator: caller, name: 'Tournament' }, - ); + world.emit_event(@TournamentCreated { + tournament_id, + creator: caller, + name: 'Tournament', + entry_fee, + max_participants, + tournament_type: parsed_type.into(), // if your event expects a felt + });
284-324: LGTM: Reward distribution hardening looks solidHandled: non-empty winners, type casting, remainder, participant validation, one-way finalization (prize_pool=0). Nice.
Minor: TournamentFinished only includes first winner; if multi-winner payouts are common, consider a separate TournamentRewardsDistributed event with winners + payouts arrays.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/systems/core.cairo(7 hunks)
🔇 Additional comments (4)
src/systems/core.cairo (4)
86-97: LGTM: Global Config initialization is coherentGood: single config root (id=0), seeded counters, defaults (guild cap = 50). Aligns with later reads in this file.
203-246: LGTM: Registration flow is guarded and persists mutated PlayerGood fixes: balance check, deduct_credits, write back Player, capacity/time/status checks, and participant persistence + event.
271-282: LGTM: Completion transition addedState moves InProgress -> Completed; persisted.
471-479: LGTM: ID generation returns pre-incremented value and uses Config id=0Matches dojo_init and avoids off-by-one gaps.
| // Validate admin permissions | ||
| assert(caller == contract.admin, 'Only admin can create tournaments'); | ||
|
|
||
| // Create tournament |
There was a problem hiding this comment.
Add input validation: max_participants and entry_fee bounds
Reject degenerate tournaments early.
@@
- // Validate admin permissions
+ // Validate admin permissions
assert(caller == contract.admin, 'Only admin can create tournaments');
+ assert(max_participants >= 2_u32, 'max_participants must be >= 2');
+ // Optional: enforce an upper bound to limit gas/state growth
+ assert(max_participants <= 1024_u32, 'max_participants too large');
+ // If zero-fee tournaments are disallowed, uncomment:
+ // assert(entry_fee > 0, 'entry_fee must be > 0');📝 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.
| // Validate admin permissions | |
| assert(caller == contract.admin, 'Only admin can create tournaments'); | |
| // Create tournament | |
| // Validate admin permissions | |
| assert(caller == contract.admin, 'Only admin can create tournaments'); | |
| assert(max_participants >= 2_u32, 'max_participants must be >= 2'); | |
| // Optional: enforce an upper bound to limit gas/state growth | |
| assert(max_participants <= 1024_u32, 'max_participants too large'); | |
| // If zero-fee tournaments are disallowed, uncomment: | |
| // assert(entry_fee > 0, 'entry_fee must be > 0'); | |
| // Create tournament |
🤖 Prompt for AI Agents
In src/systems/core.cairo around lines 174-177, add input validation before
creating the tournament: assert max_participants > 1 and <= a sensible
MAX_PARTICIPANTS constant (or define one if missing), and assert entry_fee >=
MIN_ENTRY_FEE and <= MAX_ENTRY_FEE (define these constants if not present); use
clear revert/assert messages like "Invalid max_participants" and "Invalid
entry_fee" so degenerate tournaments are rejected early; place these checks
immediately after the admin permission check and before any state mutation or
storage writes.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/systems/core.cairo (1)
164-206: Use the tournament_type parameter; currently ignoredThe field is hardcoded to SingleElimination, ignoring the input. Validate and map felt -> enum, then store it. Also make registration_end use a passed duration if you adopt it.
Apply this diff:
fn create_tournament( ref self: ContractState, tournament_type: felt252, entry_fee: u256, max_participants: u32, + // if you adopt configurable window: + // registration_duration: u64, ) { @@ - let tournament = Tournament { + // Parse/validate type + let parsed_type = self.parse_tournament_type(tournament_type); + let tournament = Tournament { id: tournament_id, creator: caller, name: 'Tournament', - tournament_type: TournamentType::SingleElimination, + tournament_type: parsed_type, status: TournamentStatus::Open, prize_pool: 0, entry_fee, max_players: max_participants, min_players: 2, registration_start: get_block_timestamp(), - registration_end: get_block_timestamp() + 86400, // 24 hours from now + // If configurable: + // registration_end: get_block_timestamp() + registration_duration, + // Else keep default: + registration_end: get_block_timestamp() + 86400, registered_players: 0, total_rounds: 0, level_requirement: 0, };Support helper (place in CoreInternalImpl):
fn parse_tournament_type(x: felt252) -> TournamentType { if x == 'SingleElimination' { TournamentType::SingleElimination } else if x == 'DoubleElimination' { TournamentType::DoubleElimination } else if x == 'RoundRobin' { TournamentType::RoundRobin } else { panic_with_felt252('Invalid tournament type') } }
🧹 Nitpick comments (4)
src/systems/core.cairo (4)
21-23: Strengthen API typing for tournament params; consider configurable registration window
- Prefer TournamentType (or validated felt -> enum) over raw felt to prevent invalid values.
- Optionally accept a registration_duration to avoid the hardcoded 24h.
Apply this diff to the trait to make duration configurable (optional):
fn create_tournament( - ref self: TContractState, tournament_type: felt252, entry_fee: u256, max_participants: u32, + ref self: TContractState, + tournament_type: felt252, + entry_fee: u256, + max_participants: u32, + registration_duration: u64, // seconds );Also applies to: 25-29
279-290: Consider emitting a completion eventOptional but useful for observability/indexing. Emit a TournamentCompleted event here (distinct from TournamentFinished).
292-332: Rewards distribution guards look solidNon-empty winners, Completed status, single-shot via prize_pool=0, participant validation, u256-safe split with remainder. Good.
Minor: clarify that winners[0] is the champion in docs or event schema, and consider de-duping winners to avoid double payout.
462-466: Helper OK; centralize to avoid duplicationusize_to_u256 is fine. Consider moving to a shared utils module if used elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/systems/core.cairo(7 hunks)
🔇 Additional comments (4)
src/systems/core.cairo (4)
6-10: LGTM: tournament/gear model importsImports look correct and scoped.
208-251: LGTM: registration flow and balance persistenceOpen/ capacity/ window checks, duplicate guard, credit deduction, prize pool and participant persistence all look correct.
47-47: Remove unused import; mixed-root concern outdated
- Remove unused import core::num::traits::Zero (src/systems/core.cairo:51).
- No crate:: imports found; file consistently uses coa::... roots — ignore the mixed-root suggestion.
Likely an incorrect or invalid review comment.
479-487: LGTM — confirm world namespace & Config key usageID generation is correct (returns pre-increment, reads Config at key 0).
- Inconsistency found: most modules use self.world(@"coa") (e.g. src/systems/tournament.cairo:406, src/systems/guild.cairo:223), but src/systems/core.cairo uses self.world(@"coa_contracts") at line 67 (core.cairo also uses self.world(@"coa") at 459). Align namespaces if unintended.
- Verification needed: initial search did not confirm other reads of Config at key 0 or occurrences of next_tournament_id/next_guild_id — confirm other modules read Config at key 0 (run rg for read_model(0) and next_* or verify manually).
| // Initialize global config | ||
| let config = Config { | ||
| id: 0, | ||
| admin, | ||
| next_tournament_id: 1, | ||
| next_guild_id: 1, | ||
| erc1155_address: erc1155, | ||
| credit_token_id: 0, | ||
| default_guild_max_members: 50, | ||
| }; | ||
| world.write_model(@config); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Critical: World namespace mismatch will orphan Config/Contract writes
dojo_init writes to self.world(@"coa_contracts") (Line 67), while all runtime reads use world_default() -> self.world(@"coa") (Line 459). Contract/Config written during init won’t be readable later.
Apply this diff to write into the same world used elsewhere:
- let mut world = self.world(@"coa_contracts");
+ let mut world = self.world_default();Verify all world namespaces are consistent:
🏁 Script executed:
#!/bin/bash
rg -nP "self\.world\(@\"[^\"]+\"\)" -S
rg -n "world_default\(" -n src/systems/core.cairo -nLength of output: 1603
Critical: dojo_init writes to a different world namespace — change to world_default
src/systems/core.cairo:67 uses self.world(@"coa_contracts"); world_default() returns self.world(@"coa") at src/systems/core.cairo:459, so init writes will be unreadable at runtime.
- let mut world = self.world(@"coa_contracts");
+ let mut world = self.world_default();📝 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.
| // Initialize global config | |
| let config = Config { | |
| id: 0, | |
| admin, | |
| next_tournament_id: 1, | |
| next_guild_id: 1, | |
| erc1155_address: erc1155, | |
| credit_token_id: 0, | |
| default_guild_max_members: 50, | |
| }; | |
| world.write_model(@config); | |
| } | |
| let mut world = self.world_default(); | |
| // Initialize global config | |
| let config = Config { | |
| id: 0, | |
| admin, | |
| next_tournament_id: 1, | |
| next_guild_id: 1, | |
| erc1155_address: erc1155, | |
| credit_token_id: 0, | |
| default_guild_max_members: 50, | |
| }; | |
| world.write_model(@config); | |
| } |
🤖 Prompt for AI Agents
In src/systems/core.cairo around lines 86 to 97, the init writes use the wrong
world namespace (world(@"coa_contracts")); change the write to use the default
world namespace by calling the world_default accessor instead of world so the
Config is written to the 'coa' namespace at runtime; update the write_model
invocation to use world_default() (or self.world_default() if required by scope)
so subsequent reads find the initialized Config.
| fn start_tournament(ref self: ContractState, tournament_id: u256) { | ||
| let caller = get_caller_address(); | ||
| let mut world = self.world_default(); | ||
|
|
||
| let mut tournament: Tournament = world.read_model(tournament_id); | ||
| assert(tournament.creator == caller, 'Only creator can start tournament'); | ||
| assert(tournament.status == TournamentStatus::Open, 'Tournament not open'); | ||
| assert( | ||
| get_block_timestamp() >= tournament.registration_end, 'Registration still open', | ||
| ); | ||
| assert( | ||
| tournament.registered_players >= tournament.min_players, 'Not enough participants', | ||
| ); | ||
|
|
||
| tournament.status = TournamentStatus::InProgress; | ||
|
|
||
| world.write_model(@tournament); | ||
| world | ||
| .emit_event( | ||
| @TournamentStarted { | ||
| tournament_id, initial_matches: tournament.registered_players / 2, | ||
| }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Allow admin to start tournaments too (per objectives)
Spec requires creator/admin authorization. Add admin fallback.
Apply this diff:
fn start_tournament(ref self: ContractState, tournament_id: u256) {
let caller = get_caller_address();
let mut world = self.world_default();
- let mut tournament: Tournament = world.read_model(tournament_id);
- assert(tournament.creator == caller, 'Only creator can start tournament');
+ let mut tournament: Tournament = world.read_model(tournament_id);
+ let contract: Contract = world.read_model(COA_CONTRACTS);
+ assert(
+ caller == tournament.creator || caller == contract.admin,
+ 'Only creator or admin can start tournament'
+ );
assert(tournament.status == TournamentStatus::Open, 'Tournament not open');
assert(
get_block_timestamp() >= tournament.registration_end, 'Registration still open',
);
assert(
tournament.registered_players >= tournament.min_players, 'Not enough participants',
);📝 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.
| fn start_tournament(ref self: ContractState, tournament_id: u256) { | |
| let caller = get_caller_address(); | |
| let mut world = self.world_default(); | |
| let mut tournament: Tournament = world.read_model(tournament_id); | |
| assert(tournament.creator == caller, 'Only creator can start tournament'); | |
| assert(tournament.status == TournamentStatus::Open, 'Tournament not open'); | |
| assert( | |
| get_block_timestamp() >= tournament.registration_end, 'Registration still open', | |
| ); | |
| assert( | |
| tournament.registered_players >= tournament.min_players, 'Not enough participants', | |
| ); | |
| tournament.status = TournamentStatus::InProgress; | |
| world.write_model(@tournament); | |
| world | |
| .emit_event( | |
| @TournamentStarted { | |
| tournament_id, initial_matches: tournament.registered_players / 2, | |
| }, | |
| ); | |
| } | |
| fn start_tournament(ref self: ContractState, tournament_id: u256) { | |
| let caller = get_caller_address(); | |
| let mut world = self.world_default(); | |
| let mut tournament: Tournament = world.read_model(tournament_id); | |
| let contract: Contract = world.read_model(COA_CONTRACTS); | |
| assert( | |
| caller == tournament.creator || caller == contract.admin, | |
| 'Only creator or admin can start tournament' | |
| ); | |
| - assert(tournament.creator == caller, 'Only creator can start tournament'); | |
| assert(tournament.status == TournamentStatus::Open, 'Tournament not open'); | |
| assert( | |
| get_block_timestamp() >= tournament.registration_end, 'Registration still open', | |
| ); | |
| assert( | |
| tournament.registered_players >= tournament.min_players, 'Not enough participants', | |
| ); | |
| tournament.status = TournamentStatus::InProgress; | |
| world.write_model(@tournament); | |
| world | |
| .emit_event( | |
| @TournamentStarted { | |
| tournament_id, initial_matches: tournament.registered_players / 2, | |
| }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/systems/core.cairo around lines 254-277, the start_tournament
authorization currently only allows the creator to start the tournament; change
the check to allow either the creator or an admin: replace the single
creator-only assert with a combined condition that passes when caller ==
tournament.creator OR is_admin(caller) (e.g., assert(caller ==
tournament.creator || is_admin(caller), 'Only creator or admin can start
tournament')), leaving the other assertions and subsequent state updates/events
unchanged.
2 participants
🏰 Add Guild System and Tournament Management
Overview
This PR introduces a comprehensive guild system and enhances tournament functionality, enabling players to form communities and participate in organized competitions.
Closes #144
✨ New Features
Guild System
Tournament Enhancements
🔧 Technical Implementation
New Models
Guild: Core guild data structure with progression trackingGuildMember: Player membership with roles and contribution trackingGuildInvite: Time-based invitation system with expirationNew Systems
GuildActions: Complete guild management systemCoreActions: Tournament creation and managementEnhanced Player Features
deduct_credits(): Support for spending credits on guild/tournament activitiesburn(): Token burning functionality for economy management🎯 Key Features
Guild Management
Tournament System
🛡️ Safety & Validation
Guild System
Tournament System
📊 Events & Observability
Guild Events
GuildCreated: Guild formation trackingGuildJoined/GuildLeft: Membership changesGuildInviteSent/GuildInviteAccepted: Invitation flowGuildLevelUp: Guild progressionTournament Events
TournamentCreated: Tournament initializationPlayerRegistered: Registration trackingTournamentStarted/TournamentFinished: Lifecycle events🔄 Breaking Changes
🧪 Testing Considerations
📈 Future Enhancements
Files Changed:
src/lib.cairo: Added guild and session modulessrc/models/guild.cairo: New guild data models and eventssrc/models/player.cairo: Enhanced wSummary by CodeRabbit
New Features
Chores