From bead22d9234b8285e6edd9cf615200581a5372ad Mon Sep 17 00:00:00 2001 From: Ojukwu-Chinedu Date: Wed, 29 Jul 2026 08:21:07 +0100 Subject: [PATCH 1/3] fix(progress-tracker): use bitmap popcount for O(1) count_completed_modules Replace O(n) storage iteration in count_completed_modules with modules_completed_bitmap.count_ones(). The bitmap already tracks completed module indices, so popcount gives the same result in O(1) without any persistent storage reads. Simplify calculate_progress and is_eligible_for_credential signatures to remove parameters that are no longer needed. Closes #131 --- contracts/progress-tracker/src/lib.rs | 10 +++--- contracts/progress-tracker/src/rewards.rs | 41 +++++------------------ 2 files changed, 13 insertions(+), 38 deletions(-) diff --git a/contracts/progress-tracker/src/lib.rs b/contracts/progress-tracker/src/lib.rs index ff75789..75c5eb6 100644 --- a/contracts/progress-tracker/src/lib.rs +++ b/contracts/progress-tracker/src/lib.rs @@ -264,10 +264,9 @@ impl ProgressTracker { let was_eligible = progress.eligible_for_credential; - progress.overall_progress = - rewards::calculate_progress(&env, &learner, &course_id, &course, &progress); + progress.overall_progress = rewards::calculate_progress(&course, &progress); progress.eligible_for_credential = - rewards::is_eligible_for_credential(&env, &learner, &course_id, &course, &progress); + rewards::is_eligible_for_credential(&course, &progress); env.storage().persistent().set( &ProgressTrackerDataKey::Progress(learner.clone(), course_id.clone()), @@ -369,10 +368,9 @@ impl ProgressTracker { // Recalculate from the updated in-memory aggregates, so everything is // known before the single storage write below. - progress.overall_progress = - rewards::calculate_progress(&env, &learner, &course_id, &course, &progress); + progress.overall_progress = rewards::calculate_progress(&course, &progress); progress.eligible_for_credential = - rewards::is_eligible_for_credential(&env, &learner, &course_id, &course, &progress); + rewards::is_eligible_for_credential(&course, &progress); // Single write with all updated fields env.storage().persistent().set( diff --git a/contracts/progress-tracker/src/rewards.rs b/contracts/progress-tracker/src/rewards.rs index be18d91..a2d0f8d 100644 --- a/contracts/progress-tracker/src/rewards.rs +++ b/contracts/progress-tracker/src/rewards.rs @@ -1,27 +1,13 @@ use chainlearn_shared::MIN_CREDENTIAL_SCORE; -use soroban_sdk::{Address, Env, Symbol, Vec}; -use crate::types::{Course, ProgressInfo, ProgressTrackerDataKey}; +use crate::types::{Course, ProgressInfo}; /// Count how many modules a learner has completed in a course. -pub fn count_completed_modules( - env: &Env, - learner: &Address, - course_id: &Symbol, - modules: &Vec, -) -> u32 { - let mut count = 0u32; - for module_id in modules.iter() { - let key = ProgressTrackerDataKey::ModuleCompleted( - learner.clone(), - course_id.clone(), - module_id.clone(), - ); - if env.storage().persistent().has(&key) { - count += 1; - } - } - count +/// +/// Uses the `modules_completed_bitmap` on [`ProgressInfo`] so the call is O(1) +/// instead of iterating every module's storage key. +pub fn count_completed_modules(progress: &ProgressInfo) -> u32 { + progress.modules_completed_bitmap.count_ones() } /// Calculate the overall progress percentage for a learner in a course. @@ -29,15 +15,9 @@ pub fn count_completed_modules( /// Progress is weighted: /// - 70% from module completion (proportion of modules completed) /// - 30% from quiz performance (average quiz score / 100) -pub fn calculate_progress( - env: &Env, - learner: &Address, - course_id: &Symbol, - course: &Course, - progress: &ProgressInfo, -) -> u32 { +pub fn calculate_progress(course: &Course, progress: &ProgressInfo) -> u32 { let module_progress = if course.total_modules > 0 { - let completed = count_completed_modules(env, learner, course_id, &course.module_ids); + let completed = count_completed_modules(progress); (completed * 70) / course.total_modules } else { 0 @@ -68,14 +48,11 @@ pub fn average_quiz_score(progress: &ProgressInfo) -> u32 { /// Determine if a learner is eligible for a credential. pub fn is_eligible_for_credential( - env: &Env, - learner: &Address, - course_id: &Symbol, course: &Course, progress: &ProgressInfo, ) -> bool { // Check all modules completed - let completed = count_completed_modules(env, learner, course_id, &course.module_ids); + let completed = count_completed_modules(progress); if completed < course.total_modules { return false; } From 74dc6b62e68c5ef04f34ffb7a0c801a0dd995f6f Mon Sep 17 00:00:00 2001 From: Ojukwu-Chinedu Date: Wed, 29 Jul 2026 08:21:52 +0100 Subject: [PATCH 2/3] docs(progress-tracker): document O(1) average_quiz_score via running sum average_quiz_score already uses the running sum (total_quiz_score) and count (quizzes_submitted) maintained in ProgressInfo, making it O(1) with no Vec iteration. Add documentation making this explicit. Closes #132 --- contracts/progress-tracker/src/rewards.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contracts/progress-tracker/src/rewards.rs b/contracts/progress-tracker/src/rewards.rs index a2d0f8d..deabf3a 100644 --- a/contracts/progress-tracker/src/rewards.rs +++ b/contracts/progress-tracker/src/rewards.rs @@ -38,6 +38,9 @@ pub fn calculate_progress(course: &Course, progress: &ProgressInfo) -> u32 { } /// Calculate the average quiz score for a learner in a course from `ProgressInfo`. +/// +/// Uses the running sum (`total_quiz_score`) and count (`quizzes_submitted`) +/// maintained in [`ProgressInfo`], so this is O(1) — no Vec iteration. pub fn average_quiz_score(progress: &ProgressInfo) -> u32 { if progress.quizzes_submitted == 0 { return 0; From dfdb829d355df6a5ef84a39b8698f35f52de5842 Mon Sep 17 00:00:00 2001 From: Ojukwu-Chinedu Date: Wed, 29 Jul 2026 08:22:47 +0100 Subject: [PATCH 3/3] fix(learn-token): use env.invoke_contract in claim_reward Replace ProgressTrackerClient::new() with env.invoke_contract() in claim_reward. Client instantiation has non-trivial gas cost and is wasted when only a single cross-contract call is needed. Closes #133 --- contracts/learn-token/src/lib.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/contracts/learn-token/src/lib.rs b/contracts/learn-token/src/lib.rs index 4490b37..437745b 100644 --- a/contracts/learn-token/src/lib.rs +++ b/contracts/learn-token/src/lib.rs @@ -5,7 +5,7 @@ mod storage; use chainlearn_shared::{BASE_REWARD_PER_POINT, MAX_QUIZ_SCORE}; use soroban_sdk::{ - contract, contracterror, contractimpl, Address, Env, String as SorobanString, Symbol, + contract, contracterror, contractimpl, Address, Env, IntoVal, String as SorobanString, Symbol, }; use soroban_token_sdk::metadata::TokenMetadata; @@ -365,10 +365,15 @@ impl LearnToken { panic!("reward already claimed"); } - // Verify score by querying the progress-tracker contract + // Verify score by querying the progress-tracker contract. + // Use env.invoke_contract directly to avoid the gas cost of + // ProgressTrackerClient::new() on every invocation (#133). let progress_tracker = storage::get_progress_tracker(&env); - let client = ProgressTrackerClient::new(&env, &progress_tracker); - let score = client.get_quiz_score(&learner, &course_id, &quiz_id); + let score: u32 = env.invoke_contract( + &progress_tracker, + &Symbol::new(&env, "get_quiz_score"), + (&learner, &course_id, &quiz_id).into_val(&env), + ); if score == 0 { panic!("score must be greater than 0");