From b784373f7433bf0a8b4e5035e8b5e3fbca5b828e Mon Sep 17 00:00:00 2001 From: Thompson <140930314+Godbrand0@users.noreply.github.com> Date: Wed, 27 May 2026 03:42:19 -0400 Subject: [PATCH] Fix invalid quest ID error handling, closes #211 --- contracts/quest_chain/src/lib.rs | 80 ++++++++++++++------ contracts/quest_chain/src/test.rs | 120 ++++++++++++++++-------------- 2 files changed, 121 insertions(+), 79 deletions(-) diff --git a/contracts/quest_chain/src/lib.rs b/contracts/quest_chain/src/lib.rs index d937bd1..b0825e1 100644 --- a/contracts/quest_chain/src/lib.rs +++ b/contracts/quest_chain/src/lib.rs @@ -1,10 +1,42 @@ #![no_std] use soroban_sdk::{ - contract, contractimpl, contracttype, symbol_short, token, Address, Env, Symbol, Vec, + contract, contractimpl, contracterror, contracttype, symbol_short, token, Address, Env, Symbol, Vec, }; // +// ────────────────────────────────────────────────────────── +// ERRORS +// ────────────────────────────────────────────────────────── +// + +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum QuestChainError { + AlreadyInitialized = 1, + TooFewQuests = 2, + TooManyQuests = 3, + MaxChainsReached = 4, + ChainNotActive = 5, + ChainNotStarted = 6, + ChainExpired = 7, + ChainAlreadyStarted = 8, + QuestNotFound = 9, + QuestAlreadyCompleted = 10, + QuestNotUnlocked = 11, + NoCheckpointAvailable = 12, + NoProgressToReset = 13, + RewardTokenNotConfigured = 14, + NoPendingRewards = 15, + InsufficientRewardPool = 16, + AmountMustBePositive = 17, + AdminOnly = 18, + DuplicateQuestId = 19, + InvalidPrerequisite = 20, + InvalidBranch = 21, +} + // ────────────────────────────────────────────────────────── // DATA STRUCTURES // ────────────────────────────────────────────────────────── @@ -144,7 +176,7 @@ impl QuestChainContract { admin.require_auth(); if env.storage().persistent().has(&DataKey::Config) { - panic!("Already initialized"); + soroban_sdk::panic_with_error!(&env, QuestChainError::AlreadyInitialized); } let config = ChainConfig { @@ -185,10 +217,10 @@ impl QuestChainContract { let config: ChainConfig = env.storage().persistent().get(&DataKey::Config).unwrap(); if (quests.len() as u32) < config.min_quests_per_chain { - panic!("Too few quests"); + soroban_sdk::panic_with_error!(&env, QuestChainError::TooFewQuests); } if (quests.len() as u32) > config.max_quests_per_chain { - panic!("Too many quests"); + soroban_sdk::panic_with_error!(&env, QuestChainError::TooManyQuests); } // Validate quest structure @@ -209,7 +241,7 @@ impl QuestChainContract { counter += 1; if counter > config.max_chains { - panic!("Max chains reached"); + soroban_sdk::panic_with_error!(&env, QuestChainError::MaxChainsReached); } let chain = QuestChain { @@ -262,19 +294,19 @@ impl QuestChainContract { .unwrap(); if !chain.active { - panic!("Chain not active"); + soroban_sdk::panic_with_error!(&env, QuestChainError::ChainNotActive); } // Check time limits let current_time = env.ledger().timestamp(); if let Some(start) = chain.start_time { if current_time < start { - panic!("Chain not started yet"); + soroban_sdk::panic_with_error!(&env, QuestChainError::ChainNotStarted); } } if let Some(end) = chain.end_time { if current_time > end { - panic!("Chain expired"); + soroban_sdk::panic_with_error!(&env, QuestChainError::ChainExpired); } } @@ -284,7 +316,7 @@ impl QuestChainContract { .persistent() .has(&DataKey::PlayerProgress(player.clone(), chain_id)) { - panic!("Chain already started"); + soroban_sdk::panic_with_error!(&env, QuestChainError::ChainAlreadyStarted); } // Initialize progress @@ -321,7 +353,7 @@ impl QuestChainContract { .unwrap(); if !chain.active { - panic!("Chain not active"); + soroban_sdk::panic_with_error!(&env, QuestChainError::ChainNotActive); } let mut progress: PlayerProgress = env @@ -333,13 +365,13 @@ impl QuestChainContract { // Verify quest exists and is unlockable let quest = Self::get_quest_by_id(&chain, quest_id); if quest.is_none() { - panic!("Quest not found"); + soroban_sdk::panic_with_error!(&env, QuestChainError::QuestNotFound); } let quest = quest.unwrap(); // Check if quest is already completed if progress.completed_quests.contains(&quest_id) { - panic!("Quest already completed"); + soroban_sdk::panic_with_error!(&env, QuestChainError::QuestAlreadyCompleted); } // Check if quest is unlocked @@ -351,7 +383,7 @@ impl QuestChainContract { let is_current = progress.current_quest == Some(quest_id); if !prerequisites_met && !branch_unlocked && !is_current { - panic!("Quest not unlocked"); + soroban_sdk::panic_with_error!(&env, QuestChainError::QuestNotUnlocked); } // Mark quest as completed @@ -436,7 +468,7 @@ impl QuestChainContract { .unwrap(); if progress.checkpoint_quest.is_none() { - panic!("No checkpoint available"); + soroban_sdk::panic_with_error!(&env, QuestChainError::NoCheckpointAvailable); } let checkpoint_id = progress.checkpoint_quest.unwrap(); @@ -521,7 +553,7 @@ impl QuestChainContract { .persistent() .has(&DataKey::PlayerProgress(player.clone(), chain_id)) { - panic!("No progress to reset"); + soroban_sdk::panic_with_error!(&env, QuestChainError::NoProgressToReset); } env.storage() @@ -623,7 +655,7 @@ impl QuestChainContract { let config: ChainConfig = env.storage().persistent().get(&DataKey::Config).unwrap(); let reward_token = match config.reward_token { Some(token) => token, - None => panic!("Reward token not configured"), + None => soroban_sdk::panic_with_error!(&env, QuestChainError::RewardTokenNotConfigured), }; let pending: i128 = env @@ -633,7 +665,7 @@ impl QuestChainContract { .unwrap_or(0); if pending <= 0 { - panic!("No pending rewards"); + soroban_sdk::panic_with_error!(&env, QuestChainError::NoPendingRewards); } // Check reward pool has enough @@ -644,7 +676,7 @@ impl QuestChainContract { .unwrap_or(0); if pool < pending { - panic!("Insufficient reward pool"); + soroban_sdk::panic_with_error!(&env, QuestChainError::InsufficientRewardPool); } // Transfer rewards @@ -734,13 +766,13 @@ impl QuestChainContract { Self::assert_admin(&env, &admin); if amount <= 0 { - panic!("Amount must be positive"); + soroban_sdk::panic_with_error!(&env, QuestChainError::AmountMustBePositive); } let config: ChainConfig = env.storage().persistent().get(&DataKey::Config).unwrap(); let reward_token = match config.reward_token { Some(token) => token, - None => panic!("Reward token not configured"), + None => soroban_sdk::panic_with_error!(&env, QuestChainError::RewardTokenNotConfigured), }; // Transfer tokens from admin to contract @@ -768,7 +800,7 @@ impl QuestChainContract { fn assert_admin(env: &Env, user: &Address) { let config: ChainConfig = env.storage().persistent().get(&DataKey::Config).unwrap(); if config.admin != *user { - panic!("Admin only"); + soroban_sdk::panic_with_error!(env, QuestChainError::AdminOnly); } } @@ -777,7 +809,7 @@ impl QuestChainContract { let mut seen_ids = Vec::new(env); for quest in quests.iter() { if seen_ids.contains(&quest.id) { - panic!("Duplicate quest ID"); + soroban_sdk::panic_with_error!(env, QuestChainError::DuplicateQuestId); } seen_ids.push_back(quest.id); } @@ -793,7 +825,7 @@ impl QuestChainContract { } } if !found { - panic!("Invalid prerequisite"); + soroban_sdk::panic_with_error!(env, QuestChainError::InvalidPrerequisite); } } @@ -807,7 +839,7 @@ impl QuestChainContract { } } if !found { - panic!("Invalid branch"); + soroban_sdk::panic_with_error!(env, QuestChainError::InvalidBranch); } } } diff --git a/contracts/quest_chain/src/test.rs b/contracts/quest_chain/src/test.rs index ca90091..b8a1862 100644 --- a/contracts/quest_chain/src/test.rs +++ b/contracts/quest_chain/src/test.rs @@ -113,13 +113,13 @@ fn test_initialization() { } #[test] -#[should_panic(expected = "Already initialized")] +#[should_panic(expected = "Error(Contract, #1)")] fn test_double_initialization() { let env = Env::default(); env.mock_all_auths(); let (client, admin) = setup_contract(&env); - client.initialize(&admin); + client.initialize(&admin, &None); } #[test] @@ -133,8 +133,8 @@ fn test_create_chain() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -144,7 +144,7 @@ fn test_create_chain() { let chain = client.get_chain(&chain_id); assert_eq!(chain.id, chain_id); - assert_eq!(chain.title, Symbol::new(&env, "Test Chain")); + assert_eq!(chain.title, Symbol::new(&env, "TestChain")); assert_eq!(chain.quests.len(), 5); assert_eq!(chain.total_reward, 1000); // 100 + 150 + 200 + 250 + 300 assert!(chain.active); @@ -164,8 +164,8 @@ fn test_create_time_limited_chain() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Time Limited"), - &Symbol::new(&env, "A time-limited chain"), + &Symbol::new(&env, "TimeLimited"), + &Symbol::new(&env, "TimeLimDesc"), &quests, &start_time, &end_time, @@ -177,7 +177,7 @@ fn test_create_time_limited_chain() { } #[test] -#[should_panic(expected = "Too few quests")] +#[should_panic(expected = "Error(Contract, #2)")] fn test_create_chain_too_few_quests() { let env = Env::default(); env.mock_all_auths(); @@ -188,7 +188,7 @@ fn test_create_chain_too_few_quests() { client.create_chain( &admin, &Symbol::new(&env, "Empty"), - &Symbol::new(&env, "Empty chain"), + &Symbol::new(&env, "EmptyDesc"), &empty_quests, &None, &None, @@ -206,8 +206,8 @@ fn test_start_chain() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -225,7 +225,7 @@ fn test_start_chain() { } #[test] -#[should_panic(expected = "Chain already started")] +#[should_panic(expected = "Error(Contract, #8)")] fn test_start_chain_twice() { let env = Env::default(); env.mock_all_auths(); @@ -236,8 +236,8 @@ fn test_start_chain_twice() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -249,7 +249,7 @@ fn test_start_chain_twice() { } #[test] -#[should_panic(expected = "Chain not started yet")] +#[should_panic(expected = "Error(Contract, #6)")] fn test_start_chain_before_start_time() { let env = Env::default(); env.mock_all_auths(); @@ -260,8 +260,8 @@ fn test_start_chain_before_start_time() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &Some(2000u64), &None, @@ -272,7 +272,7 @@ fn test_start_chain_before_start_time() { } #[test] -#[should_panic(expected = "Chain expired")] +#[should_panic(expected = "Error(Contract, #7)")] fn test_start_chain_after_end_time() { let env = Env::default(); env.mock_all_auths(); @@ -283,8 +283,8 @@ fn test_start_chain_after_end_time() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &Some(1000u64), &Some(2000u64), @@ -305,8 +305,8 @@ fn test_sequential_quest_completion() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -325,13 +325,14 @@ fn test_sequential_quest_completion() { // Complete quest 2 client.complete_quest(&player, &chain_id, &2); + client.complete_quest(&player, &chain_id, &3); let progress = client.get_player_progress(&player, &chain_id).unwrap(); assert_eq!(progress.completed_quests.len(), 2); assert_eq!(progress.total_reward_earned, 250); // 100 + 150 } #[test] -#[should_panic(expected = "Prerequisites not met")] +#[should_panic(expected = "Error(Contract, #11)")] fn test_complete_quest_without_prerequisites() { let env = Env::default(); env.mock_all_auths(); @@ -342,8 +343,8 @@ fn test_complete_quest_without_prerequisites() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -354,6 +355,7 @@ fn test_complete_quest_without_prerequisites() { // Try to complete quest 2 without completing quest 1 client.complete_quest(&player, &chain_id, &2); + client.complete_quest(&player, &chain_id, &3); } #[test] @@ -367,8 +369,8 @@ fn test_branching_paths() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -404,8 +406,8 @@ fn test_progress_checkpointing() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -421,6 +423,7 @@ fn test_progress_checkpointing() { // Complete quest 2 (no checkpoint) client.complete_quest(&player, &chain_id, &2); + client.complete_quest(&player, &chain_id, &3); let progress = client.get_player_progress(&player, &chain_id).unwrap(); assert_eq!(progress.checkpoint_quest, Some(1)); // Still at quest 1 @@ -441,8 +444,8 @@ fn test_reset_to_checkpoint() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -455,6 +458,7 @@ fn test_reset_to_checkpoint() { client.complete_quest(&player, &chain_id, &1); // Complete quest 2 client.complete_quest(&player, &chain_id, &2); + client.complete_quest(&player, &chain_id, &3); // Complete quest 3 client.complete_quest(&player, &chain_id, &3); @@ -473,7 +477,7 @@ fn test_reset_to_checkpoint() { } #[test] -#[should_panic(expected = "No checkpoint available")] +#[should_panic(expected = "Error(Contract, #12)")] fn test_reset_to_checkpoint_no_checkpoint() { let env = Env::default(); env.mock_all_auths(); @@ -484,8 +488,8 @@ fn test_reset_to_checkpoint_no_checkpoint() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -509,8 +513,8 @@ fn test_reset_chain() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -520,6 +524,7 @@ fn test_reset_chain() { client.start_chain(&player, &chain_id); client.complete_quest(&player, &chain_id, &1); client.complete_quest(&player, &chain_id, &2); + client.complete_quest(&player, &chain_id, &3); // Reset entire chain client.reset_chain(&player, &chain_id); @@ -539,8 +544,8 @@ fn test_chain_completion() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -552,6 +557,7 @@ fn test_chain_completion() { // Complete all quests sequentially client.complete_quest(&player, &chain_id, &1); client.complete_quest(&player, &chain_id, &2); + client.complete_quest(&player, &chain_id, &3); client.complete_quest(&player, &chain_id, &4); client.complete_quest(&player, &chain_id, &5); @@ -575,8 +581,8 @@ fn test_cumulative_rewards() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -594,6 +600,7 @@ fn test_cumulative_rewards() { assert_eq!(progress.total_reward_earned, total_reward); client.complete_quest(&player, &chain_id, &2); + client.complete_quest(&player, &chain_id, &3); total_reward += 150; let progress = client.get_player_progress(&player, &chain_id).unwrap(); assert_eq!(progress.total_reward_earned, total_reward); @@ -620,8 +627,8 @@ fn test_leaderboard() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -633,6 +640,7 @@ fn test_leaderboard() { env.ledger().set_timestamp(1000); client.complete_quest(&player1, &chain_id, &1); client.complete_quest(&player1, &chain_id, &2); + client.complete_quest(&player1, &chain_id, &3); client.complete_quest(&player1, &chain_id, &4); client.complete_quest(&player1, &chain_id, &5); @@ -651,6 +659,7 @@ fn test_leaderboard() { env.ledger().set_timestamp(3000); client.complete_quest(&player3, &chain_id, &1); client.complete_quest(&player3, &chain_id, &2); + client.complete_quest(&player3, &chain_id, &3); client.complete_quest(&player3, &chain_id, &4); client.complete_quest(&player3, &chain_id, &5); @@ -677,8 +686,8 @@ fn test_multiple_players_same_chain() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -721,8 +730,8 @@ fn test_admin_functions() { let quests = create_test_quests(&env); let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -734,7 +743,7 @@ fn test_admin_functions() { } #[test] -#[should_panic(expected = "Admin only")] +#[should_panic(expected = "Error(Contract, #18)")] fn test_unauthorized_admin_action() { let env = Env::default(); env.mock_all_auths(); @@ -746,7 +755,7 @@ fn test_unauthorized_admin_action() { } #[test] -#[should_panic(expected = "Quest already completed")] +#[should_panic(expected = "Error(Contract, #10)")] fn test_complete_quest_twice() { let env = Env::default(); env.mock_all_auths(); @@ -757,8 +766,8 @@ fn test_complete_quest_twice() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -771,7 +780,7 @@ fn test_complete_quest_twice() { } #[test] -#[should_panic(expected = "Quest not unlocked")] +#[should_panic(expected = "Error(Contract, #11)")] fn test_complete_unlocked_quest() { let env = Env::default(); env.mock_all_auths(); @@ -782,8 +791,8 @@ fn test_complete_unlocked_quest() { let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -824,8 +833,8 @@ fn test_pending_rewards_tracking() { let quests = create_test_quests(&env); let chain_id = client.create_chain( &admin, - &Symbol::new(&env, "Test Chain"), - &Symbol::new(&env, "A test quest chain"), + &Symbol::new(&env, "TestChain"), + &Symbol::new(&env, "TestDesc"), &quests, &None, &None, @@ -843,6 +852,7 @@ fn test_pending_rewards_tracking() { // Complete quest 2 client.complete_quest(&player, &chain_id, &2); + client.complete_quest(&player, &chain_id, &3); let pending = client.get_pending_rewards(&player, &chain_id); assert_eq!(pending, 250); // Quest 1 + Quest 2 rewards }