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

[iduhtheman]

Summary

Originally scoped to #227, #228, #229, #230. While preparing this PR, issue number 245, 244, and 243 were found to be literal duplicates of #230, #229, and #228 respectively (identical titles), and another contributor's PR (issue-worker-linked PR #376) already implemented and merged that work first. This PR now covers #227 only — the one issue with no overlapping upstream work — plus several pre-existing, unrelated bugs discovered and fixed while getting this branch to actually build and test against the current main.

closes #227

Re-verified independently while rebasing onto current main (2026-08-31): confirmed via gh issue view that #228, #229, #230 are still OPEN and still assigned to iduhtheman, and that #243, #244, #245 are CLOSED with byte-identical titles ("Add credential metadata update" / "Add credential display properties" / "Add course versioning"). Confirmed PR #376 is MERGED and its body explicitly states closes #246/#245/#244/#243 with a description matching each issue's scope (Course struct version field + update_course_version, CredentialDisplay/CredentialVerification/set_credential_display/get_credential_display, update_credential_metadata), all of which are present on main today. The "duplicate" claim in this PR body is true — #228/#229/#230 are content-duplicates of #243/#244/#245 and are already fully implemented on main via #376. They remain open only because #376 closed them by their own (different) issue numbers rather than #228/#229/#230's; that's a bookkeeping cleanup for a maintainer to do (closing #228/#229/#230 as duplicates), not new code for this PR. No code changes were made for #228/#229/#230.

The one feature in this PR

#227 — Credential transfer rejection with reason: transfer previously panicked with a plain string. It now returns Result<(), ContractError> and rejects with a new Soulbound variant instead, so callers can match on a typed error rather than parsing a panic message.

Update while rebasing onto current main: #227 turned out to itself be a content-duplicate of #242 (identical title/body to #227), which was independently implemented and merged first via #374. Upstream's version returns the same typed Soulbound error but deliberately omits from.require_auth() — the rejection is unconditional and reads/writes no storage, so (per #374's own design notes) there is nothing to authorize. Equivalent test coverage already exists in tests/unit/credential_tests.rs (test_transfer_always_returns_soulbound_error, test_transfer_rejects_even_without_auth_or_existing_credential — which explicitly asserts the call succeeds with no mocked auth at all — and test_transfer_does_not_mutate_credential_state). Kept upstream's implementation and doc comment as-is and added a cross-reference note to the #227 duplicate; did not reintroduce the auth requirement or duplicate the existing test coverage.

Pre-existing bugs fixed as prerequisites (not part of #227's own scope)

Getting this branch to build and test against current main required fixing six separate, unrelated pre-existing breaks — each is its own isolated commit, none folded into the #227 feature commit:

1. learn-token/storage.rs — a duplicate closing brace after append_claim_record broke parsing for the entire learn-token crate (and therefore the whole workspace, since root-level integration tests depend on every workspace member). From learn-token: storage size tracking, plus edge-case coverage for per-address minting, claim history, and pause #373.
2. progress-tracker/lib.rs — complete_module_in_place and submit_quiz_score_in_place both take progress: &mut ProgressInfo, but the single storage write in each passed &progress (a reference to the reference) instead of progress directly, failing to compile. Each also recomputed overall_progress/eligible_for_credential a second time immediately after the write, which was silently discarded — dead code left over from a merge that inserted a new write in front of, instead of in place of, the original recompute-then-write pattern. From feat(progress-tracker): versioning, batch operations, and progress delegation #372 merging awkwardly against feat: add course versioning, credential display properties, and metadata update (#246, #245, #244, #243) #376.
3. credential-nft's CredentialVerification.display — declared as Option<CredentialDisplay>, but 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. This 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, confirmed directly against this SDK version — a real, previously-undetected break in the already-merged . Add credential display properties #244/feat: add course versioning, credential display properties, and metadata update (#246, #245, #244, #243) #376 code, since nothing exercised verify_credential_with_display before now. Fixed by using a Vec<CredentialDisplay> holding 0 or 1 elements instead (every field inside CredentialDisplay stays a true Option<Symbol>, which does work). Added 2 tests that were previously entirely missing for this function.
4. tests/unit/progress_tests.rs — a manually-constructed Course test fixture was missing the version: u32 field . Add course versioning #245 added, failing to compile.
5. tests/unit/credential_tests.rs — test_renew_credential_extends_expiry referenced a .expiry field that doesn't exist on CredentialInfo (the real field is expires_at), failing to compile.
6. tests/unit/token_tests.rs — two adjacent test functions (test_security_batch_claim_reward_supply_overflow_skips_without_panicking and test_governance_proposal_lifecycle) were merged together without a closing brace between them, an unclosed-delimiter error blocking the entire token_tests binary. From test(learn-token): add security tests for overflow/underflow protection #379.
7. progress-tracker/types.rs (found while rebasing onto current main) — Soulbound transfer rejection, delayed admin transfer, init check, and storage-size tracking #374's merge of storage-size tracking (. Add storage size tracking #239) botched the surrounding hunk: it closed ProgressTrackerDataKey's enum body right after the new StorageSize variant, stranding the pre-existing Achievements/AchievementEarned variants as dangling tokens after write_entry's function body (outside any enum) and leaving write_entry itself unclosed — a parse error blocking cargo check --workspace on plain main. Restored the two variants to the enum body and closed write_entry properly. Fixing the parse error surfaced two further pre-existing compile errors in the same code path (also unchanged on main): complete_module_in_place called an undefined get_learner_stats_internal instead of the real public get_learner_stats, and earn_achievement's event publish passed &AchievementType by reference, which soroban-sdk 21.7.7 can't convert (only owned custom-enum values are supported). Fixed both minimally in place.

Test plan

• cargo check --workspace — passes cleanly.
• cargo test -p progress-tracker --lib (79 passed), cargo test -p credential-nft --lib (37 passed).
• cargo test --test credential_tests --test progress_tests --test token_tests --test credential_flow --test full_flow --test batch_operations --test admin_role_flow — all pass (27 + 97 + 50 + 6 + 1 + 3 + 3).
• cargo test --test security_arithmetic_tests --test security_reentrancy_tests --test token_flow --test security_auth_tests --test xcontract_call_tests — all pass.
• cargo fmt run on every file this PR touches.
• cargo clippy -p progress-tracker -p credential-nft — no errors, only pre-existing needless-borrow style warnings on lines this PR doesn't touch.

Pre-existing, unrelated bugs found and deliberately left unfixed (disclosed, not fixed — out of scope for this PR)

• learn-token's own test_storage_size_starts_at_zero fails (expects 0, gets 1) — once the crate can actually compile (fix Implement unit tests for learn-token contract (tests/unit/token_tests.rs) #1 above unblocks it for the first time), this test genuinely fails. This is a logic bug in learn-token: storage size tracking, plus edge-case coverage for per-address minting, claim history, and pause #373's own storage-size-tracking feature (initialize() appears to create one tracked entry the test doesn't expect), unrelated to . Add credential transfer rejection with reason #227 or any of the compile-blocking fixes above.
• tests/integration/upgrade_tests.rs::test_contract_upgrade_preserves_state_and_updates_version fails at runtime with HostError: Error(Storage, MissingValue) inside learn_token's upgrade call. This file is byte-for-byte identical to plain main, and this branch makes no changes to learn-token's upgrade mechanism at all — confirmed pre-existing and unrelated.
• tests/integration/emergency_pause_flow.rs, tests/unit/event_emission_tests.rs, tests/integration/content_hash_flow.rs, tests/integration/multi_admin_flow.rs, tests/integration/security_double_spending_tests.rs, tests/integration/token_vesting_flow.rs all fail to compile on plain main (missing use soroban_sdk::IntoVal; / use soroban_sdk::testutils::Ledger; imports, and a Vec<Val>/Val type mismatch in event_emission_tests.rs). All six files are byte-for-byte identical to main — none of this branch's commits touch them. Left unfixed as out of scope; flagging since they block a full cargo test --workspace from ever succeeding on main today, regardless of this PR.

[drips-wave]

@iduhtheman Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits