fix: reject credential transfer with a typed error, plus several build-blocking bug fixes

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.
 }

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<CredentialDisplay>` (#227 fix): `soroban-sdk` 21.7.7's
+/// `#[contracttype]` derive does not implement the `ScVal` (client/spec)
+/// conversion for `Option<T>` where `T` is a custom struct -- only for SDK
+/// built-ins like `Symbol`. `Option<CredentialDisplay>` 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<CredentialDisplay>> 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<Symbol>`, 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<CredentialDisplay>,
-}
+    /// Display properties, if any were set. Empty when none were set;
+    /// otherwise holds exactly one element.
+    pub display: Vec<CredentialDisplay>,
+}
+
+/// Build the empty `display` value for a [`CredentialVerification`] with no
+/// display data set.
+pub fn no_display(env: &Env) -> Vec<CredentialDisplay> {
+    Vec::new(env)
+}
+
+/// Wrap a single [`CredentialDisplay`] as the `display` value for a
+/// [`CredentialVerification`].
+pub fn one_display(env: &Env, display: CredentialDisplay) -> Vec<CredentialDisplay> {
+    let mut v = Vec::new(env);
+    v.push_back(display);
+    v
+}

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<u64> {
@@ -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<CredentialDisplay> = env
+    let stored: Option<CredentialDisplay> = 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 }
 }
 

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),
         );
     }
 

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),
 }

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]

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(

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();