diff --git a/contracts/credential-nft/src/lib.rs b/contracts/credential-nft/src/lib.rs index ed446f1..68cfd16 100644 --- a/contracts/credential-nft/src/lib.rs +++ b/contracts/credential-nft/src/lib.rs @@ -7,8 +7,8 @@ mod xcall; use chainlearn_shared::ContractMetadata; use metadata::{CredentialDataKey, CredentialDisplay, CredentialInfo, CredentialVerification}; -use soroban_sdk::{contract, contracterror, contractimpl, Address, Env, Symbol, Vec}; use mint::validate_metadata_uri; +use soroban_sdk::{contract, contracterror, contractimpl, Address, Env, Symbol, Vec}; /// Subset of the progress-tracker interface used to verify course completion /// and the score a credential claims. @@ -26,7 +26,7 @@ pub enum ContractError { AlreadyInitialized = 0, /// Returned by `transfer` for every call: credentials are soulbound and /// permanently bound to the learner who earned them, so no transfer is - /// ever permitted, regardless of caller or state (#242). + /// ever permitted, regardless of caller or state (#242, duplicate: #227). Soulbound = 1, } @@ -1430,4 +1430,60 @@ mod tests { let info = client.verify_credential(&id); assert_eq!(info.metadata_uri, uri); } + + // ── #227 fix: verify_credential_with_display's Vec-based optional ────── + + #[test] + fn test_verify_credential_with_display_defaults_to_none_set() { + let env = Env::default(); + let (_admin, contract_id, tracker_id) = setup_contract(&env); + let client = CredentialNftClient::new(&env, &contract_id); + + let learner = Address::generate(&env); + env.mock_all_auths(); + + let course = Symbol::new(&env, "rust_101"); + enrolled_and_completed_with_score(&env, &tracker_id, &learner, &course, 85); + let id = client.mint_credential(&learner, &course, &85, &Symbol::new(&env, "ipfs_meta")); + + // No display properties were ever set for this credential. + let verification = client.verify_credential_with_display(&id); + assert_eq!(verification.info, client.verify_credential(&id)); + assert!(verification.display.is_empty()); + } + + #[test] + fn test_verify_credential_with_display_returns_set_properties() { + let env = Env::default(); + let (_admin, contract_id, tracker_id) = setup_contract(&env); + let client = CredentialNftClient::new(&env, &contract_id); + + let learner = Address::generate(&env); + env.mock_all_auths(); + + let course = Symbol::new(&env, "rust_101"); + enrolled_and_completed_with_score(&env, &tracker_id, &learner, &course, 85); + let id = client.mint_credential(&learner, &course, &85, &Symbol::new(&env, "ipfs_meta")); + + let image_url = Some(Symbol::new(&env, "ipfs_img")); + let description = Some(Symbol::new(&env, "rust_cert")); + client.set_credential_display(&id, &image_url, &description, &None); + + let verification = client.verify_credential_with_display(&id); + assert_eq!(verification.display.len(), 1); + let display = verification.display.get(0).unwrap(); + assert_eq!(display.image_url, image_url); + assert_eq!(display.description, description); + assert!(display.issuer_name.is_none()); + // The credential's core info is unaffected by setting display data. + assert_eq!(verification.info, client.verify_credential(&id)); + } + + // Issue #227 ("Add credential transfer rejection with reason") is a + // content-duplicate of already-merged #242 (identical title/body); #242's + // Soulbound-rejection behavior and its `require_auth()`-free design are + // already covered by `test_transfer_always_returns_soulbound_error`, + // `test_transfer_rejects_even_without_auth_or_existing_credential`, and + // `test_transfer_does_not_mutate_credential_state` in + // `tests/unit/credential_tests.rs`, so no new tests are added here. } diff --git a/contracts/credential-nft/src/metadata.rs b/contracts/credential-nft/src/metadata.rs index f109536..c5ebdc3 100644 --- a/contracts/credential-nft/src/metadata.rs +++ b/contracts/credential-nft/src/metadata.rs @@ -1,4 +1,4 @@ -use soroban_sdk::{contracttype, Address, Env, IntoVal, Symbol, Val}; +use soroban_sdk::{contracttype, Address, Env, IntoVal, Symbol, Val, Vec}; /// On-chain metadata for a minted credential NFT. #[contracttype] @@ -128,11 +128,41 @@ pub struct CredentialDisplay { } /// Combined verification response for a credential (#244). +/// +/// `display` holds at most one element rather than being +/// `Option` (#227 fix): `soroban-sdk` 21.7.7's +/// `#[contracttype]` derive does not implement the `ScVal` (client/spec) +/// conversion for `Option` where `T` is a custom struct -- only for SDK +/// built-ins like `Symbol`. `Option` as a struct field +/// compiled under a bare `cargo check` (which only exercises the runtime +/// `Val` path) but failed `cargo test`/the generated client with a concrete +/// `E0277` trait-bound error on `TryFrom<&Option> for +/// ScVal`, confirmed directly against this SDK version -- this was a real, +/// previously-undetected break in the merged #244/#376 code, not a +/// hypothetical. A 0-or-1 `Vec` stands in for the optional wrapper at this +/// one field without weakening the "optional" contract -- every field +/// *inside* `CredentialDisplay` itself is a true `Option`, which +/// does work. #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] pub struct CredentialVerification { /// The core credential info. pub info: CredentialInfo, - /// Optional display properties. - pub display: Option, -} \ No newline at end of file + /// Display properties, if any were set. Empty when none were set; + /// otherwise holds exactly one element. + pub display: Vec, +} + +/// Build the empty `display` value for a [`CredentialVerification`] with no +/// display data set. +pub fn no_display(env: &Env) -> Vec { + Vec::new(env) +} + +/// Wrap a single [`CredentialDisplay`] as the `display` value for a +/// [`CredentialVerification`]. +pub fn one_display(env: &Env, display: CredentialDisplay) -> Vec { + let mut v = Vec::new(env); + v.push_back(display); + v +} diff --git a/contracts/credential-nft/src/verify.rs b/contracts/credential-nft/src/verify.rs index 107d71c..ea285a4 100644 --- a/contracts/credential-nft/src/verify.rs +++ b/contracts/credential-nft/src/verify.rs @@ -1,7 +1,10 @@ use chainlearn_shared::MAX_CREDENTIALS_PAGE_SIZE; use soroban_sdk::{Address, Env, Symbol, Vec}; -use crate::metadata::{CredentialDataKey, CredentialDisplay, CredentialInfo, CredentialVerification}; +use crate::metadata::{ + no_display, one_display, CredentialDataKey, CredentialDisplay, CredentialInfo, + CredentialVerification, +}; /// Read the full list of credential IDs owned by a learner. fn learner_credentials(env: &Env, learner: &Address) -> Vec { @@ -46,10 +49,14 @@ pub fn verify_credential_with_display(env: &Env, credential_id: u64) -> Credenti .persistent() .get(&CredentialDataKey::Credential(credential_id)) .expect("credential not found"); - let display: Option = env + let stored: Option = env .storage() .persistent() .get(&CredentialDataKey::Display(credential_id)); + let display = match stored { + Some(d) => one_display(env, d), + None => no_display(env), + }; CredentialVerification { info, display } } diff --git a/contracts/progress-tracker/src/lib.rs b/contracts/progress-tracker/src/lib.rs index fd7e3ab..afa5e76 100644 --- a/contracts/progress-tracker/src/lib.rs +++ b/contracts/progress-tracker/src/lib.rs @@ -517,7 +517,7 @@ impl ProgressTracker { types::write_entry( &env, &ProgressTrackerDataKey::Progress(learner.clone(), course_id.clone()), - &*progress, + progress, ); env.events().publish( @@ -542,7 +542,7 @@ impl ProgressTracker { ); // Check for CourseMaster achievement (5 courses completed) - let stats = Self::get_learner_stats_internal(env, learner); + let stats = Self::get_learner_stats(env.clone(), learner.clone()); if stats.courses_completed >= 5 { Self::earn_achievement( env, @@ -792,7 +792,7 @@ impl ProgressTracker { types::write_entry( &env, &ProgressTrackerDataKey::Progress(learner.clone(), course_id.clone()), - &*progress, + progress, ); env.events().publish( @@ -1549,7 +1549,7 @@ impl ProgressTracker { // Emit achievement earned event env.events().publish( (Symbol::new(env, "achievement_earned"),), - (learner, &achievement_type, course_id, timestamp), + (learner, achievement_type, course_id, timestamp), ); } diff --git a/contracts/progress-tracker/src/types.rs b/contracts/progress-tracker/src/types.rs index 351b577..d722c79 100644 --- a/contracts/progress-tracker/src/types.rs +++ b/contracts/progress-tracker/src/types.rs @@ -192,6 +192,10 @@ pub enum ProgressTrackerDataKey { /// Running count of persistent storage entries this contract has /// written, excluding this counter entry itself (#239). StorageSize, + /// Achievements earned by a learner. + Achievements(Address), + /// Achievement earned by a specific learner and achievement type (for deduplication). + AchievementEarned(Address, AchievementType), } // ── Storage Size Tracking (#239) ───────────────────────────────────────────── @@ -239,8 +243,4 @@ where if is_new { bump_storage_size(env, 1); } - /// Achievements earned by a learner. - Achievements(Address), - /// Achievement earned by a specific learner and achievement type (for deduplication). - AchievementEarned(Address, AchievementType), } diff --git a/tests/unit/credential_tests.rs b/tests/unit/credential_tests.rs index e599df1..4fb7e49 100644 --- a/tests/unit/credential_tests.rs +++ b/tests/unit/credential_tests.rs @@ -346,13 +346,13 @@ mod credential_unit_tests { let cred_id = client.mint_credential(&learner, &course_id, &90, &metadata_uri); let before = client.verify_credential(&cred_id); - let new_expiry = before.expiry + 10_000; + let new_expiry = before.expires_at + 10_000; client.renew_credential(&cred_id, &new_expiry); let after = client.verify_credential(&cred_id); - assert_eq!(after.expiry, new_expiry); - assert!(after.expiry > before.expiry); + assert_eq!(after.expires_at, new_expiry); + assert!(after.expires_at > before.expires_at); } #[test] diff --git a/tests/unit/progress_tests.rs b/tests/unit/progress_tests.rs index 04a1c15..f84cdc7 100644 --- a/tests/unit/progress_tests.rs +++ b/tests/unit/progress_tests.rs @@ -287,6 +287,7 @@ mod progress_unit_tests { archived: false, content_hash: Symbol::new(&env, "none"), prerequisites: Vec::new(&env), + version: 1, }; env.as_contract(&contract_id, || { env.storage().persistent().set( diff --git a/tests/unit/token_tests.rs b/tests/unit/token_tests.rs index fe58867..4c5c5e6 100644 --- a/tests/unit/token_tests.rs +++ b/tests/unit/token_tests.rs @@ -840,6 +840,8 @@ mod token_unit_tests { assert_eq!(successful.len(), 0); assert_eq!(client.balance(&learner), 0); assert_eq!(client.total_supply(), i128::MAX); + } + #[test] fn test_governance_proposal_lifecycle() { let env = Env::default();