From 56253c47e900e8e09a01bd34b52cf995a8998676 Mon Sep 17 00:00:00 2001 From: Nwokedi Uche Date: Sun, 30 Aug 2026 15:56:19 +0100 Subject: [PATCH 1/4] feat(credential-nft): reject transfer with typed Soulbound error (#242) transfer() now returns Result<(), ContractError> and always rejects with the new ContractError::Soulbound variant instead of panicking, so callers get a documented, typed reason instead of a raw host trap. The rejection is unconditional (no auth check, no storage access), so no state is ever mutated. Also fixes an existing unclosed-brace bug in credential_tests.rs that otherwise fails the whole test binary to compile, and adds test coverage for the new behavior. --- contracts/credential-nft/src/lib.rs | 79 +++++++++++++++++++++------- tests/unit/credential_tests.rs | 81 +++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 18 deletions(-) diff --git a/contracts/credential-nft/src/lib.rs b/contracts/credential-nft/src/lib.rs index 5d80b62..d861e7b 100644 --- a/contracts/credential-nft/src/lib.rs +++ b/contracts/credential-nft/src/lib.rs @@ -24,6 +24,10 @@ pub trait ProgressTrackerInterface { #[repr(u32)] 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). + Soulbound = 1, } /// NFT credential contract for ChainLearn course certificates. @@ -388,7 +392,10 @@ impl CredentialNft { // ── Emergency Pause (#189) ──────────────────────────────────────────── fn is_paused(env: &Env) -> bool { - env.storage().persistent().get(&CredentialDataKey::Paused).unwrap_or(false) + env.storage() + .persistent() + .get(&CredentialDataKey::Paused) + .unwrap_or(false) } fn require_not_paused(env: &Env) { @@ -399,17 +406,29 @@ impl CredentialNft { /// Pause all state-changing operations. Admin only. pub fn emergency_pause(env: Env) { - let admin: Address = env.storage().persistent().get(&CredentialDataKey::Admin).expect("not initialized"); + let admin: Address = env + .storage() + .persistent() + .get(&CredentialDataKey::Admin) + .expect("not initialized"); admin.require_auth(); - env.storage().persistent().set(&CredentialDataKey::Paused, &true); + env.storage() + .persistent() + .set(&CredentialDataKey::Paused, &true); // Event would ideally be emitted here, but we will omit it for simplicity if it wasn't added to events.rs } /// Unpause state-changing operations. Admin only. pub fn unpause(env: Env) { - let admin: Address = env.storage().persistent().get(&CredentialDataKey::Admin).expect("not initialized"); + let admin: Address = env + .storage() + .persistent() + .get(&CredentialDataKey::Admin) + .expect("not initialized"); admin.require_auth(); - env.storage().persistent().set(&CredentialDataKey::Paused, &false); + env.storage() + .persistent() + .set(&CredentialDataKey::Paused, &false); } /// Returns the admin address. @@ -456,19 +475,32 @@ impl CredentialNft { /// Reject transfer of a credential. /// /// Credentials are soulbound (non-transferable) and permanently bound to the - /// learner who earned them. This function enforces that policy by rejecting - /// all transfer attempts. + /// learner who earned them: a credential attests that a specific learner, + /// and no one else, met a course's completion criteria, so allowing it to + /// change hands would let it be sold, gifted, or otherwise separated from + /// the achievement it certifies. This function enforces that policy by + /// explicitly rejecting every transfer attempt with a typed error rather + /// than panicking, so callers get a clear, documented reason instead of a + /// raw host trap, and can handle the rejection programmatically. + /// + /// No storage is read or written: the rejection is unconditional and does + /// not depend on `from`, `to`, `credential_id`, or any on-chain state, so + /// there is nothing to authorize and no state to leave unchanged. /// /// # Arguments - /// * `from` - The current holder (must authorize) - /// * `to` - The intended recipient (not used, transfer rejected) - /// * `credential_id` - The credential being transferred (not used, transfer rejected) + /// * `from` - The current holder (unused; transfer is always rejected) + /// * `to` - The intended recipient (unused; transfer is always rejected) + /// * `credential_id` - The credential being transferred (unused; transfer is always rejected) /// - /// # Panics - /// Always panics with a message explaining credentials are non-transferable. - pub fn transfer(_env: Env, from: Address, _to: Address, _credential_id: u64) { - from.require_auth(); - panic!("credentials are soulbound and non-transferable"); + /// # Returns + /// Always `Err(ContractError::Soulbound)`. Never `Ok`. + pub fn transfer( + _env: Env, + _from: Address, + _to: Address, + _credential_id: u64, + ) -> Result<(), ContractError> { + Err(ContractError::Soulbound) } /// Generate a course completion certificate URI for a learner and course (#223). @@ -512,14 +544,22 @@ impl CredentialNft { // If credential already minted, update metadata_uri in CredentialInfo if let Some(cred_id) = env.storage().persistent().get::<_, u64>(&dup_key) { let cred_key = CredentialDataKey::Credential(cred_id); - if let Some(mut info) = env.storage().persistent().get::<_, CredentialInfo>(&cred_key) { + if let Some(mut info) = env + .storage() + .persistent() + .get::<_, CredentialInfo>(&cred_key) + { info.metadata_uri = cert_uri.clone(); env.storage().persistent().set(&cred_key, &info); } } env.events().publish( - (Symbol::new(&env, "certificate_generated"), learner.clone(), course_id.clone()), + ( + Symbol::new(&env, "certificate_generated"), + learner.clone(), + course_id.clone(), + ), (cert_uri.clone(),), ); @@ -1296,7 +1336,10 @@ mod tests { assert_eq!(client.get_certificate_uri(&learner, &course), None); let cert_uri = client.generate_certificate(&learner, &course); - assert_eq!(client.get_certificate_uri(&learner, &course), Some(cert_uri.clone())); + assert_eq!( + client.get_certificate_uri(&learner, &course), + Some(cert_uri.clone()) + ); // Mint credential and check that metadata_uri gets updated with generated certificate URI let cred_id = client.mint_credential(&learner, &course, &85, &cert_uri); diff --git a/tests/unit/credential_tests.rs b/tests/unit/credential_tests.rs index f6868b8..2dfa99f 100644 --- a/tests/unit/credential_tests.rs +++ b/tests/unit/credential_tests.rs @@ -375,4 +375,85 @@ mod credential_unit_tests { let stored_reason = client.get_revocation_reason(&cred_id); assert_eq!(stored_reason, Some(reason)); } + + // ── Issue #242: transfer is always rejected as Soulbound ──────────────── + + #[test] + fn test_transfer_always_returns_soulbound_error() { + 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); + let other = Address::generate(&env); + env.mock_all_auths(); + + let course_id = Symbol::new(&env, "rust_101"); + let metadata_uri = Symbol::new(&env, "ipfs_Qm123"); + enrolled_and_completed_with_score(&env, &tracker_id, &learner, &course_id, 85); + let cred_id = client.mint_credential(&learner, &course_id, &85, &metadata_uri); + + let result = client.try_transfer(&learner, &other, &cred_id); + assert!(result.is_err(), "transfer must always be rejected"); + let contract_err = result + .err() + .expect("expected an error") + .expect("expected a typed contract error, not a host trap"); + assert_eq!(contract_err, credential_nft::ContractError::Soulbound); + } + + #[test] + fn test_transfer_rejects_even_without_auth_or_existing_credential() { + // The rejection is unconditional: it doesn't depend on auth, on + // `from`/`to` being real accounts, or on `credential_id` existing. + let env = Env::default(); + let (_admin, contract_id, _tracker_id) = setup_contract(&env); + let client = CredentialNftClient::new(&env, &contract_id); + + let from = Address::generate(&env); + let to = Address::generate(&env); + // Intentionally no `env.mock_all_auths()` and no minted credential. + + let result = client.try_transfer(&from, &to, &999); + let contract_err = result + .err() + .expect("expected an error") + .expect("expected a typed contract error, not a host trap"); + assert_eq!(contract_err, credential_nft::ContractError::Soulbound); + } + + #[test] + fn test_transfer_does_not_mutate_credential_state() { + 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); + let other = Address::generate(&env); + env.mock_all_auths(); + + let course_id = Symbol::new(&env, "rust_101"); + let metadata_uri = Symbol::new(&env, "ipfs_Qm123"); + enrolled_and_completed_with_score(&env, &tracker_id, &learner, &course_id, 85); + let cred_id = client.mint_credential(&learner, &course_id, &85, &metadata_uri); + + let before = client.verify_credential(&cred_id); + let learner_credentials_before = client.get_credentials_for(&learner, &0, &50); + let other_credentials_before = client.get_credentials_for(&other, &0, &50); + + let _ = client.try_transfer(&learner, &other, &cred_id); + + let after = client.verify_credential(&cred_id); + assert_eq!(before, after, "credential record must be unchanged"); + assert_eq!( + client.get_credentials_for(&learner, &0, &50), + learner_credentials_before, + "original holder's credential list must be unchanged" + ); + assert_eq!( + client.get_credentials_for(&other, &0, &50), + other_credentials_before, + "intended recipient must not gain the credential" + ); + } } From 164622870b1501dc634856787543506afc0745c4 Mon Sep 17 00:00:00 2001 From: Nwokedi Uche Date: Sun, 30 Aug 2026 16:02:56 +0100 Subject: [PATCH 2/4] feat(learn-token): add configurable delay to admin transfer (#241) transfer_admin(new_admin) no longer updates the admin immediately. It now records a PendingAdminTransfer (new_admin + initiated_at) and emits admin_transfer_initiated; the transfer only takes effect once new_admin calls accept_admin() after admin_transfer_delay() has elapsed (default 48h, admin-configurable via set_admin_transfer_delay). The current admin can call cancel_admin_transfer() at any point before acceptance to abort an unauthorized or mistaken transfer. This closes the window where a single compromised admin key could hand control to an attacker-controlled address in one transaction. Added: - storage::PendingAdminTransfer, TokenDataKey::PendingAdmin/AdminTransferDelay - accept_admin(), cancel_admin_transfer(), pending_admin(), admin_transfer_delay(), set_admin_transfer_delay() - events: admin_transfer_initiated/accepted/cancelled, admin_transfer_delay_updated - unit tests covering delay enforcement, cancellation, configurability, auth requirements, event emission, and re-initiation overwrite semantics --- contracts/learn-token/src/events.rs | 103 ++++++++--- contracts/learn-token/src/lib.rs | 151 ++++++++++++++-- contracts/learn-token/src/storage.rs | 87 +++++++++- tests/unit/token_tests.rs | 246 +++++++++++++++++++++++++++ 4 files changed, 536 insertions(+), 51 deletions(-) diff --git a/contracts/learn-token/src/events.rs b/contracts/learn-token/src/events.rs index 3560d80..3bbdae3 100644 --- a/contracts/learn-token/src/events.rs +++ b/contracts/learn-token/src/events.rs @@ -34,8 +34,13 @@ pub fn reward_claimed( reward_amount: i128, course_id: &Symbol, ) { - let topics = (Symbol::new(env, "reward"), learner.clone(), course_id.clone()); - env.events().publish(topics, (quiz_id, score, reward_amount)); + let topics = ( + Symbol::new(env, "reward"), + learner.clone(), + course_id.clone(), + ); + env.events() + .publish(topics, (quiz_id, score, reward_amount)); } /// Emitted when tokens are transferred directly. @@ -241,10 +246,8 @@ pub fn vesting_created( duration_seconds: u64, ) { let topics = (Symbol::new(env, "vesting_created"), beneficiary.clone()); - env.events().publish( - topics, - (total_amount, cliff_timestamp, duration_seconds), - ); + env.events() + .publish(topics, (total_amount, cliff_timestamp, duration_seconds)); } /// Emitted when vested tokens are claimed (#225). @@ -258,7 +261,8 @@ pub fn vesting_claimed( total_claimed: i128, ) { let topics = (Symbol::new(env, "vesting_claimed"), beneficiary.clone()); - env.events().publish(topics, (claimed_amount, total_claimed)); + env.events() + .publish(topics, (claimed_amount, total_claimed)); } /// Emitted when a governance proposal is created (#226). @@ -267,36 +271,28 @@ pub fn vesting_claimed( /// Data: (proposal_id, start_time, end_time) pub fn proposal_created(env: &Env, proposal_id: u64, start_time: u64, end_time: u64) { let topics = (Symbol::new(env, "proposal_created"),); - env.events().publish(topics, (proposal_id, start_time, end_time)); + env.events() + .publish(topics, (proposal_id, start_time, end_time)); } /// Emitted when a vote is cast on a proposal (#226). /// /// Topics: ["vote_cast", voter] /// Data: (proposal_id, choice, voting_power) -pub fn vote_cast( - env: &Env, - proposal_id: u64, - voter: &Address, - choice: u32, - voting_power: i128, -) { +pub fn vote_cast(env: &Env, proposal_id: u64, voter: &Address, choice: u32, voting_power: i128) { let topics = (Symbol::new(env, "vote_cast"), voter.clone()); - env.events().publish(topics, (proposal_id, choice, voting_power)); + env.events() + .publish(topics, (proposal_id, choice, voting_power)); } /// Emitted when a governance proposal is executed (#226). /// /// Topics: ["proposal_executed"] /// Data: (proposal_id, winning_choice, winning_votes) -pub fn proposal_executed( - env: &Env, - proposal_id: u64, - winning_choice: u32, - winning_votes: i128, -) { +pub fn proposal_executed(env: &Env, proposal_id: u64, winning_choice: u32, winning_votes: i128) { let topics = (Symbol::new(env, "proposal_executed"),); - env.events().publish(topics, (proposal_id, winning_choice, winning_votes)); + env.events() + .publish(topics, (proposal_id, winning_choice, winning_votes)); } /// Emitted when the maximum supply cap is updated. @@ -305,5 +301,64 @@ pub fn proposal_executed( /// Data: (old_max_supply, new_max_supply) pub fn max_supply_updated(env: &Env, old_max_supply: i128, new_max_supply: i128) { let topics = (Symbol::new(env, "max_supply_updated"),); - env.events().publish(topics, (old_max_supply, new_max_supply)); + env.events() + .publish(topics, (old_max_supply, new_max_supply)); +} + +/// Emitted when an admin transfer is initiated (#241). +/// +/// Topics: ["admin_transfer_initiated", new_admin] — indexed by the +/// candidate address so "is address X a pending admin anywhere" is a topic +/// filter instead of a scan. +/// Data: (current_admin, initiated_at, accept_after) +pub fn admin_transfer_initiated( + env: &Env, + current_admin: &Address, + new_admin: &Address, + initiated_at: u64, + accept_after: u64, +) { + let topics = ( + Symbol::new(env, "admin_transfer_initiated"), + new_admin.clone(), + ); + env.events() + .publish(topics, (current_admin.clone(), initiated_at, accept_after)); +} + +/// Emitted when a pending admin transfer is accepted and takes effect (#241). +/// +/// Topics: ["admin_transfer_accepted", new_admin] +/// Data: (previous_admin,) +pub fn admin_transfer_accepted(env: &Env, previous_admin: &Address, new_admin: &Address) { + let topics = ( + Symbol::new(env, "admin_transfer_accepted"), + new_admin.clone(), + ); + env.events().publish(topics, (previous_admin.clone(),)); +} + +/// Emitted when a pending admin transfer is cancelled before acceptance (#241). +/// +/// Topics: ["admin_transfer_cancelled", new_admin] — same topic slot as the +/// other two admin-transfer events, so a client can correlate the lifecycle +/// of one candidate transfer with a single topic filter. +/// Data: (current_admin,) +pub fn admin_transfer_cancelled(env: &Env, current_admin: &Address, new_admin: &Address) { + let topics = ( + Symbol::new(env, "admin_transfer_cancelled"), + new_admin.clone(), + ); + env.events().publish(topics, (current_admin.clone(),)); +} + +/// Emitted when the configurable admin-transfer delay is updated (#241). +/// +/// Topics: ["admin_transfer_delay_updated"] — a rare, admin-only, +/// contract-wide config event; there is no per-address query pattern to index. +/// Data: (old_delay_seconds, new_delay_seconds) +pub fn admin_transfer_delay_updated(env: &Env, old_delay_seconds: u64, new_delay_seconds: u64) { + let topics = (Symbol::new(env, "admin_transfer_delay_updated"),); + env.events() + .publish(topics, (old_delay_seconds, new_delay_seconds)); } diff --git a/contracts/learn-token/src/lib.rs b/contracts/learn-token/src/lib.rs index 2303819..a91f6fc 100644 --- a/contracts/learn-token/src/lib.rs +++ b/contracts/learn-token/src/lib.rs @@ -179,9 +179,7 @@ impl LearnToken { /// Only has an effect when `Cooldown` is active; a no-op otherwise so it /// is safe to call unconditionally after every successful transfer. fn record_transfer_timestamp(env: &Env, from: &Address) { - if let storage::TransferRestriction::Cooldown(_) = - storage::get_transfer_restriction(env) - { + if let storage::TransferRestriction::Cooldown(_) = storage::get_transfer_restriction(env) { storage::set_last_transfer_ledger(env, from, env.ledger().sequence()); } } @@ -830,14 +828,10 @@ impl LearnToken { } } - // ── Emergency Pause (#189) ──────────────────────────────────────────── - - // ── Admin ───────────────────────────────────────────────────────────── - /// Grant an admin role to an address. Admin only. pub fn grant_role(env: Env, caller: Address, address: Address, role: storage::AdminRole) { caller.require_auth(); @@ -1079,17 +1073,41 @@ impl LearnToken { if old_max_supply > 0 && new_max_supply > old_max_supply { let max_allowed = old_max_supply.checked_mul(2).expect("overflow"); if new_max_supply > max_allowed { - panic!("max supply increase exceeds governance limit (maximum 2x increase per update)"); + panic!( + "max supply increase exceeds governance limit (maximum 2x increase per update)" + ); } } storage::set_max_supply(&env, new_max_supply); events::max_supply_updated(&env, old_max_supply, new_max_supply); } - /// Transfer admin rights to a new address. + /// Initiate a delayed transfer of admin rights to a new address (#241). + /// + /// This does **not** change the admin immediately. It records + /// `new_admin` as pending; the transfer only takes effect once + /// `new_admin` calls [`Self::accept_admin`] after + /// [`Self::admin_transfer_delay`] has elapsed. The current admin can call + /// [`Self::cancel_admin_transfer`] any time before acceptance to abort it. + /// + /// # Why a delay + /// An immediate transfer means a single compromised admin key can hand + /// control to an attacker-controlled address in one transaction, with no + /// window to notice or react. Delaying the handoff — and emitting an + /// `admin_transfer_initiated` event when it starts — gives the real admin + /// (or anyone monitoring the contract) time to call + /// `cancel_admin_transfer` before the new address can ever exercise + /// admin rights. + /// + /// Calling this again before a pending transfer is accepted overwrites + /// it with the new candidate and restarts the delay from now. /// /// # Arguments /// * `new_admin` - The new admin address + /// + /// # Panics + /// * If the caller is not the current admin + /// * If `new_admin` is the zero address pub fn transfer_admin(env: Env, new_admin: Address) { let admin = storage::get_admin(&env); admin.require_auth(); @@ -1102,7 +1120,97 @@ impl LearnToken { panic!("cannot transfer admin to zero address"); } - storage::set_admin(&env, &new_admin); + let initiated_at = env.ledger().timestamp(); + let delay = storage::get_admin_transfer_delay(&env); + storage::set_pending_admin( + &env, + &storage::PendingAdminTransfer { + new_admin: new_admin.clone(), + initiated_at, + }, + ); + + events::admin_transfer_initiated( + &env, + &admin, + &new_admin, + initiated_at, + initiated_at.saturating_add(delay), + ); + } + + /// Complete a pending admin transfer once its delay has elapsed (#241). + /// + /// Must be called by the pending `new_admin` address, proving control of + /// that key before it's granted admin rights. Clears the pending + /// transfer and emits `admin_transfer_accepted` on success. + /// + /// # Panics + /// * If there is no pending admin transfer + /// * If the caller is not the pending `new_admin` + /// * If [`Self::admin_transfer_delay`] has not yet elapsed since + /// `transfer_admin` was called + pub fn accept_admin(env: Env) { + let pending = storage::get_pending_admin(&env).expect("no pending admin transfer"); + pending.new_admin.require_auth(); + + let delay = storage::get_admin_transfer_delay(&env); + let ready_at = pending.initiated_at.saturating_add(delay); + if env.ledger().timestamp() < ready_at { + panic!("admin transfer delay has not elapsed"); + } + + let previous_admin = storage::get_admin(&env); + storage::set_admin(&env, &pending.new_admin); + storage::clear_pending_admin(&env); + + events::admin_transfer_accepted(&env, &previous_admin, &pending.new_admin); + } + + /// Cancel a pending admin transfer before it is accepted (#241). + /// + /// Admin only. The primary safeguard against a compromised admin key: + /// the legitimate admin can abort an unauthorized `transfer_admin` call + /// any time before the pending `new_admin` accepts it. + /// + /// # Panics + /// * If the caller is not the current admin + /// * If there is no pending admin transfer + pub fn cancel_admin_transfer(env: Env) { + let admin = storage::get_admin(&env); + admin.require_auth(); + + let pending = storage::get_pending_admin(&env).expect("no pending admin transfer"); + storage::clear_pending_admin(&env); + + events::admin_transfer_cancelled(&env, &admin, &pending.new_admin); + } + + /// Returns the in-flight pending admin transfer, if any (#241). + pub fn pending_admin(env: Env) -> Option { + storage::get_pending_admin(&env) + } + + /// Returns the current admin-transfer delay, in seconds (#241). + pub fn admin_transfer_delay(env: Env) -> u64 { + storage::get_admin_transfer_delay(&env) + } + + /// Set the admin-transfer delay, in seconds. Admin only (#241). + /// + /// Applies to transfers initiated after this call; it does not change + /// the deadline of a transfer already pending. + /// + /// # Panics + /// * If the caller is not the current admin + pub fn set_admin_transfer_delay(env: Env, delay_seconds: u64) { + let admin = storage::get_admin(&env); + admin.require_auth(); + + let old_delay = storage::get_admin_transfer_delay(&env); + storage::set_admin_transfer_delay(&env, delay_seconds); + + events::admin_transfer_delay_updated(&env, old_delay, delay_seconds); } /// Update the progress-tracker contract address. Admin only. @@ -1364,7 +1472,13 @@ impl LearnToken { exhausted: false, }; storage::set_vesting_schedule(&env, &beneficiary, &schedule); - events::vesting_created(&env, &beneficiary, total_amount, cliff_timestamp, duration_seconds); + events::vesting_created( + &env, + &beneficiary, + total_amount, + cliff_timestamp, + duration_seconds, + ); } /// Claim vested tokens. Beneficiary only. @@ -1377,8 +1491,8 @@ impl LearnToken { Self::require_not_paused(&env); beneficiary.require_auth(); - let schedule = storage::get_vesting_schedule(&env, &beneficiary) - .expect("no vesting schedule found"); + let schedule = + storage::get_vesting_schedule(&env, &beneficiary).expect("no vesting schedule found"); if schedule.exhausted { panic!("vesting schedule fully claimed"); @@ -1431,7 +1545,10 @@ impl LearnToken { } /// Return the vesting schedule for a beneficiary (#225). - pub fn get_vesting_schedule(env: Env, beneficiary: Address) -> Option { + pub fn get_vesting_schedule( + env: Env, + beneficiary: Address, + ) -> Option { storage::get_vesting_schedule(&env, &beneficiary) } @@ -1508,8 +1625,7 @@ impl LearnToken { pub fn vote(env: Env, voter: Address, proposal_id: u64, choice: u32) { voter.require_auth(); - let mut proposal = storage::get_proposal(&env, proposal_id) - .expect("proposal not found"); + let mut proposal = storage::get_proposal(&env, proposal_id).expect("proposal not found"); let now = env.ledger().timestamp(); if now < proposal.start_time { @@ -1554,8 +1670,7 @@ impl LearnToken { let admin = storage::get_admin(&env); admin.require_auth(); - let mut proposal = storage::get_proposal(&env, proposal_id) - .expect("proposal not found"); + let mut proposal = storage::get_proposal(&env, proposal_id).expect("proposal not found"); if proposal.executed { panic!("proposal already executed"); diff --git a/contracts/learn-token/src/storage.rs b/contracts/learn-token/src/storage.rs index 90e63ea..a15bfd3 100644 --- a/contracts/learn-token/src/storage.rs +++ b/contracts/learn-token/src/storage.rs @@ -63,6 +63,12 @@ pub enum TokenDataKey { StorageEntryCount, /// List of registered admins and their assigned roles (#212). Admins, + /// In-flight admin transfer awaiting its delay to elapse, if any (#241). + PendingAdmin, + /// Configurable delay (in seconds) a pending admin transfer must wait + /// before it can be accepted (#241). Defaults to + /// `DEFAULT_ADMIN_TRANSFER_DELAY_SECONDS` until overridden. + AdminTransferDelay, } #[contracttype] @@ -87,7 +93,6 @@ pub struct RoleKey { pub role: AdminRole, } - #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] pub enum TransferRestriction { @@ -156,6 +161,23 @@ pub struct VestingSchedule { pub exhausted: bool, } +/// An admin transfer that has been initiated but not yet accepted (#241). +/// +/// `transfer_admin` records one of these instead of updating `Admin` +/// immediately, so a compromised admin key can't hand control to an +/// attacker-controlled address in a single transaction: the current admin +/// (or anyone watching `admin_transfer_initiated` events) has until +/// `initiated_at + delay` to call `cancel_admin_transfer` before `new_admin` +/// can call `accept_admin`. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct PendingAdminTransfer { + /// The address the admin role is being transferred to. + pub new_admin: Address, + /// Ledger timestamp `transfer_admin` was called. + pub initiated_at: u64, +} + /// A governance proposal (#226). #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] @@ -249,6 +271,48 @@ pub fn remove_admin(env: &Env, address: &Address, role: &AdminRole) { revoke_role(env, address, role); } +// ── Admin Transfer Delay (#241) ────────────────────────────────────────────── + +/// Default delay (in seconds) a pending admin transfer must wait before it +/// can be accepted, until the admin sets a different value via +/// `set_admin_transfer_delay`. 172_800s = 48 hours. +pub const DEFAULT_ADMIN_TRANSFER_DELAY_SECONDS: u64 = 172_800; + +/// Store the in-flight pending admin transfer. +pub fn set_pending_admin(env: &Env, pending: &PendingAdminTransfer) { + env.storage() + .persistent() + .set(&TokenDataKey::PendingAdmin, pending); +} + +/// Retrieve the in-flight pending admin transfer, if any. +pub fn get_pending_admin(env: &Env) -> Option { + env.storage().persistent().get(&TokenDataKey::PendingAdmin) +} + +/// Clear the in-flight pending admin transfer (accepted or cancelled). +pub fn clear_pending_admin(env: &Env) { + env.storage() + .persistent() + .remove(&TokenDataKey::PendingAdmin); +} + +/// Set the configurable admin-transfer delay, in seconds. +pub fn set_admin_transfer_delay(env: &Env, delay_seconds: u64) { + env.storage() + .persistent() + .set(&TokenDataKey::AdminTransferDelay, &delay_seconds); +} + +/// Get the configurable admin-transfer delay, in seconds. Falls back to +/// `DEFAULT_ADMIN_TRANSFER_DELAY_SECONDS` until explicitly set. +pub fn get_admin_transfer_delay(env: &Env) -> u64 { + env.storage() + .persistent() + .get(&TokenDataKey::AdminTransferDelay) + .unwrap_or(DEFAULT_ADMIN_TRANSFER_DELAY_SECONDS) +} + // ── Role Management (#190) ─────────────────────────────────────────────────── /// Check if an address has a specific role. @@ -258,7 +322,7 @@ pub fn has_role(env: &Env, address: &Address, role: &AdminRole) -> bool { if address == &admin { return true; } - + // Also, anyone with AdminRole::Admin has all roles if role != &AdminRole::Admin { let admin_key = TokenDataKey::Role(RoleKey { @@ -303,7 +367,6 @@ pub fn revoke_role(env: &Env, address: &Address, role: &AdminRole) { } } - // ── Emergency Pause (#189) ────────────────────────────────────────────────── /// Check if the contract is currently paused. @@ -316,10 +379,11 @@ pub fn is_paused(env: &Env) -> bool { /// Set the paused state. pub fn set_paused(env: &Env, paused: bool) { - env.storage().persistent().set(&TokenDataKey::Paused, &paused); + env.storage() + .persistent() + .set(&TokenDataKey::Paused, &paused); } - /// Get the balance for a given address. pub fn get_balance(env: &Env, address: &Address) -> i128 { env.storage() @@ -424,7 +488,11 @@ pub fn check_allowance_expired(env: &Env, owner: &Address, spender: &Address) -> /// Read-only version of check_allowance_expired that does not perform storage side-effects. #[allow(dead_code)] -pub fn check_allowance_expired_readonly(env: &Env, owner: &Address, spender: &Address) -> (bool, bool, u32) { +pub fn check_allowance_expired_readonly( + env: &Env, + owner: &Address, + spender: &Address, +) -> (bool, bool, u32) { let key = AllowanceKey { owner: owner.clone(), spender: spender.clone(), @@ -714,7 +782,10 @@ pub fn track_allowance_spender(env: &Env, owner: &Address, spender: &Address) { /// spenders whose allowance has since expired or been fully spent). pub fn get_allowance_spenders(env: &Env, owner: &Address) -> Vec
{ let key = TokenDataKey::AllowanceSpenders(owner.clone()); - env.storage().persistent().get(&key).unwrap_or(Vec::new(env)) + env.storage() + .persistent() + .get(&key) + .unwrap_or(Vec::new(env)) } /// Replace `owner`'s tracked-spender list wholesale (used after a cleanup @@ -820,8 +891,6 @@ pub fn append_claim_record(env: &Env, learner: &Address, record: &ClaimRecord) { track_entry_created(env); } } -} - // ── Vesting Schedules (#225) ────────────────────────────────────────────────── diff --git a/tests/unit/token_tests.rs b/tests/unit/token_tests.rs index 62a8f2b..b5d72c8 100644 --- a/tests/unit/token_tests.rs +++ b/tests/unit/token_tests.rs @@ -981,5 +981,251 @@ mod token_unit_tests { // Attempting to claim again after schedule is exhausted must panic client.claim_vested(&beneficiary); } + + // ── Issue #241: admin transfer delay ───────────────────────────────────── + + #[test] + fn test_transfer_admin_does_not_change_admin_immediately() { + let env = Env::default(); + let (admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + let new_admin = Address::generate(&env); + env.mock_all_auths(); + + client.transfer_admin(&new_admin); + + assert_eq!(client.admin(), admin, "admin must not change until accepted"); + let pending = client.pending_admin().expect("pending transfer expected"); + assert_eq!(pending.new_admin, new_admin); + } + + #[test] + fn test_accept_admin_before_delay_elapses_fails() { + let env = Env::default(); + let (_admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + let new_admin = Address::generate(&env); + env.mock_all_auths(); + + client.transfer_admin(&new_admin); + + // No time has passed at all yet. + let result = client.try_accept_admin(); + assert!(result.is_err(), "accept before delay elapses must fail"); + } + + #[test] + fn test_accept_admin_after_delay_elapses_succeeds() { + let env = Env::default(); + let (admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + let new_admin = Address::generate(&env); + env.mock_all_auths(); + + client.transfer_admin(&new_admin); + let delay = client.admin_transfer_delay(); + + env.ledger().with_mut(|l| { + l.timestamp += delay; + }); + + client.accept_admin(); + + assert_eq!(client.admin(), new_admin); + assert_eq!( + client.pending_admin(), + None, + "pending transfer must be cleared after acceptance" + ); + assert_ne!(client.admin(), admin); + } + + #[test] + fn test_accept_admin_requires_new_admin_auth() { + let env = Env::default(); + let (_admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + let new_admin = Address::generate(&env); + + env.mock_all_auths(); + client.transfer_admin(&new_admin); + let delay = client.admin_transfer_delay(); + env.ledger().with_mut(|l| { + l.timestamp += delay; + }); + + // Only new_admin's auth should satisfy accept_admin's require_auth; + // asserting the exact auth tree catches a caller-address check being + // silently dropped or swapped for a different address. + client.accept_admin(); + assert_eq!( + env.auths()[0].0, new_admin, + "accept_admin must require new_admin's auth, not the caller's" + ); + } + + #[test] + fn test_cancel_admin_transfer_aborts_pending_transfer() { + let env = Env::default(); + let (admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + let new_admin = Address::generate(&env); + env.mock_all_auths(); + + client.transfer_admin(&new_admin); + assert!(client.pending_admin().is_some()); + + client.cancel_admin_transfer(); + + assert_eq!(client.pending_admin(), None); + assert_eq!(client.admin(), admin); + + // The delay elapsing afterward must not resurrect the cancelled transfer. + let delay = client.admin_transfer_delay(); + env.ledger().with_mut(|l| { + l.timestamp += delay; + }); + let result = client.try_accept_admin(); + assert!(result.is_err(), "cancelled transfer must not be acceptable"); + assert_eq!(client.admin(), admin); + } + + #[test] + fn test_cancel_admin_transfer_requires_admin_auth() { + let env = Env::default(); + let (admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + let new_admin = Address::generate(&env); + env.mock_all_auths(); + client.transfer_admin(&new_admin); + + client.cancel_admin_transfer(); + assert_eq!( + env.auths()[0].0, admin, + "cancel_admin_transfer must require the current admin's auth" + ); + } + + #[test] + fn test_admin_transfer_delay_is_configurable() { + let env = Env::default(); + let (_admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + env.mock_all_auths(); + + let default_delay = client.admin_transfer_delay(); + let custom_delay = default_delay * 2; + client.set_admin_transfer_delay(&custom_delay); + + assert_eq!(client.admin_transfer_delay(), custom_delay); + + // A transfer initiated after the change is gated by the new delay. + let new_admin = Address::generate(&env); + client.transfer_admin(&new_admin); + + env.ledger().with_mut(|l| { + l.timestamp += default_delay; + }); + // Old (shorter) delay must not be enough anymore. + assert!(client.try_accept_admin().is_err()); + + env.ledger().with_mut(|l| { + l.timestamp += custom_delay - default_delay; + }); + client.accept_admin(); + assert_eq!(client.admin(), new_admin); + } + + #[test] + fn test_transfer_admin_emits_initiated_event() { + let env = Env::default(); + let (_admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + let new_admin = Address::generate(&env); + env.mock_all_auths(); + + client.transfer_admin(&new_admin); + + let all = env.events().all(); + let (_, topics, _) = all.last().expect("no events emitted"); + let event_name: Symbol = topics.get(0).unwrap().into_val(&env); + let new_admin_topic: Address = topics.get(1).unwrap().into_val(&env); + assert_eq!(event_name, Symbol::new(&env, "admin_transfer_initiated")); + assert_eq!(new_admin_topic, new_admin); + } + + #[test] + fn test_accept_admin_emits_accepted_event() { + let env = Env::default(); + let (_admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + let new_admin = Address::generate(&env); + env.mock_all_auths(); + + client.transfer_admin(&new_admin); + let delay = client.admin_transfer_delay(); + env.ledger().with_mut(|l| { + l.timestamp += delay; + }); + client.accept_admin(); + + let all = env.events().all(); + let (_, topics, _) = all.last().expect("no events emitted"); + let event_name: Symbol = topics.get(0).unwrap().into_val(&env); + let new_admin_topic: Address = topics.get(1).unwrap().into_val(&env); + assert_eq!(event_name, Symbol::new(&env, "admin_transfer_accepted")); + assert_eq!(new_admin_topic, new_admin); + } + + #[test] + fn test_cancel_admin_transfer_emits_cancelled_event() { + let env = Env::default(); + let (_admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + let new_admin = Address::generate(&env); + env.mock_all_auths(); + + client.transfer_admin(&new_admin); + client.cancel_admin_transfer(); + + let all = env.events().all(); + let (_, topics, _) = all.last().expect("no events emitted"); + let event_name: Symbol = topics.get(0).unwrap().into_val(&env); + let new_admin_topic: Address = topics.get(1).unwrap().into_val(&env); + assert_eq!(event_name, Symbol::new(&env, "admin_transfer_cancelled")); + assert_eq!(new_admin_topic, new_admin); + } + + #[test] + fn test_re_initiating_transfer_overwrites_previous_pending_admin() { + let env = Env::default(); + let (_admin, contract_id, _) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + let first_candidate = Address::generate(&env); + let second_candidate = Address::generate(&env); + env.mock_all_auths(); + + client.transfer_admin(&first_candidate); + client.transfer_admin(&second_candidate); + + let pending = client.pending_admin().expect("pending transfer expected"); + assert_eq!( + pending.new_admin, second_candidate, + "second transfer_admin call must overwrite the first candidate" + ); + + let delay = client.admin_transfer_delay(); + env.ledger().with_mut(|l| { + l.timestamp += delay; + }); + + client.accept_admin(); + assert_eq!( + client.admin(), + second_candidate, + "only the surviving (second) candidate can complete the transfer" + ); + assert_ne!(client.admin(), first_candidate); + } } From b4c91dad0f77bfef12b20d784dc8333938650967 Mon Sep 17 00:00:00 2001 From: Nwokedi Uche Date: Sun, 30 Aug 2026 16:09:07 +0100 Subject: [PATCH 3/4] feat: add is_initialized() to every contract (#240) Adds a read-only is_initialized() function to learn-token, credential-nft, and progress-tracker, so deployment scripts can check initialization status directly instead of inferring it from some other call panicking with 'not initialized'. Implementation is consistent across all three contracts: each checks for the presence of its own Admin storage key, matching the same sentinel each contract's initialize() already uses to guard against double-initialization. learn-token already had this exact check as a private storage::is_initialized() helper; this just exposes it on the contract. credential-nft and progress-tracker gained the equivalent inline (neither has a separate storage.rs module). Tests added per contract: false before initialize, true after, and a before/after metadata comparison confirming the call is read-only. --- contracts/credential-nft/src/lib.rs | 11 ++++ contracts/learn-token/src/lib.rs | 11 ++++ contracts/progress-tracker/src/lib.rs | 13 +++++ tests/unit/credential_tests.rs | 34 +++++++++++ tests/unit/progress_tests.rs | 84 +++++++++++++++++++++++---- tests/unit/token_tests.rs | 52 +++++++++++++++-- 6 files changed, 190 insertions(+), 15 deletions(-) diff --git a/contracts/credential-nft/src/lib.rs b/contracts/credential-nft/src/lib.rs index d861e7b..24b0436 100644 --- a/contracts/credential-nft/src/lib.rs +++ b/contracts/credential-nft/src/lib.rs @@ -69,6 +69,17 @@ impl CredentialNft { Ok(()) } + /// Returns whether the contract has been initialized (#240). + /// + /// Read-only: performs a single storage existence check and never + /// mutates state. Lets deployment scripts confirm `initialize()` has + /// already run before calling admin-only setup steps, instead of + /// discovering an uninitialized contract only when some other call + /// panics with "not initialized". + pub fn is_initialized(env: Env) -> bool { + env.storage().persistent().has(&CredentialDataKey::Admin) + } + /// Get the contract's on-chain name and version (#107). /// /// Lets external tools (indexers, block explorers, upgrade tooling) diff --git a/contracts/learn-token/src/lib.rs b/contracts/learn-token/src/lib.rs index a91f6fc..dd1bf2d 100644 --- a/contracts/learn-token/src/lib.rs +++ b/contracts/learn-token/src/lib.rs @@ -995,6 +995,17 @@ impl LearnToken { storage::is_paused(&env) } + /// Returns whether the contract has been initialized (#240). + /// + /// Read-only: performs a single storage existence check and never + /// mutates state. Lets deployment scripts confirm `initialize()` has + /// already run before calling admin-only setup steps, instead of + /// discovering an uninitialized contract only when some other call + /// panics with "not initialized" or "contract not initialized". + pub fn is_initialized(env: Env) -> bool { + storage::is_initialized(&env) + } + /// Returns the admin address. pub fn admin(env: Env) -> Address { storage::get_admin(&env) diff --git a/contracts/progress-tracker/src/lib.rs b/contracts/progress-tracker/src/lib.rs index 3cbf5b4..6b428a3 100644 --- a/contracts/progress-tracker/src/lib.rs +++ b/contracts/progress-tracker/src/lib.rs @@ -88,6 +88,19 @@ impl ProgressTracker { .expect("not initialized") } + /// Returns whether the contract has been initialized (#240). + /// + /// Read-only: performs a single storage existence check and never + /// mutates state. Lets deployment scripts confirm `initialize()` has + /// already run before calling admin-only setup steps, instead of + /// discovering an uninitialized contract only when some other call + /// panics with "not initialized". + pub fn is_initialized(env: Env) -> bool { + env.storage() + .persistent() + .has(&ProgressTrackerDataKey::Admin) + } + /// Register a new course with its modules and quizzes. /// /// # Arguments diff --git a/tests/unit/credential_tests.rs b/tests/unit/credential_tests.rs index 2dfa99f..2409a60 100644 --- a/tests/unit/credential_tests.rs +++ b/tests/unit/credential_tests.rs @@ -456,4 +456,38 @@ mod credential_unit_tests { "intended recipient must not gain the credential" ); } + + // ── Issue #240: contract initialization verification ──────────────────── + + #[test] + fn test_is_initialized_false_before_initialize() { + let env = Env::default(); + let contract_id = env.register_contract(None, CredentialNft); + let client = CredentialNftClient::new(&env, &contract_id); + + assert!(!client.is_initialized()); + } + + #[test] + fn test_is_initialized_true_after_initialize() { + let env = Env::default(); + let (_admin, contract_id, _tracker_id) = setup_contract(&env); + let client = CredentialNftClient::new(&env, &contract_id); + + assert!(client.is_initialized()); + } + + #[test] + fn test_is_initialized_does_not_mutate_state() { + let env = Env::default(); + let (_admin, contract_id, _tracker_id) = setup_contract(&env); + let client = CredentialNftClient::new(&env, &contract_id); + + let before = client.contract_metadata(); + let _ = client.is_initialized(); + let _ = client.is_initialized(); + let after = client.contract_metadata(); + + assert_eq!(before, after, "is_initialized must be read-only"); + } } diff --git a/tests/unit/progress_tests.rs b/tests/unit/progress_tests.rs index a23f537..e24dd94 100644 --- a/tests/unit/progress_tests.rs +++ b/tests/unit/progress_tests.rs @@ -289,9 +289,10 @@ mod progress_unit_tests { prerequisites: Vec::new(&env), }; env.as_contract(&contract_id, || { - env.storage() - .persistent() - .set(&progress_tracker::ProgressTrackerDataKey::Course(course_id.clone()), &course); + env.storage().persistent().set( + &progress_tracker::ProgressTrackerDataKey::Course(course_id.clone()), + &course, + ); }); let learner = Address::generate(&env); @@ -456,7 +457,10 @@ mod progress_unit_tests { client.get_quiz_score(&learner, &course_id, &Symbol::new(&env, "quiz_1")), 70 ); - assert_eq!(client.get_progress(&learner, &course_id).quizzes_submitted, 1); + assert_eq!( + client.get_progress(&learner, &course_id).quizzes_submitted, + 1 + ); } #[test] @@ -625,9 +629,18 @@ mod progress_unit_tests { // up the module list. let course = client.get_course(&course_id); assert_eq!(course.module_ids.len(), 3); - assert_eq!(course.module_ids.get(0).unwrap(), Symbol::new(&env, "mod_1")); - assert_eq!(course.module_ids.get(1).unwrap(), Symbol::new(&env, "mod_2")); - assert_eq!(course.module_ids.get(2).unwrap(), Symbol::new(&env, "mod_3")); + assert_eq!( + course.module_ids.get(0).unwrap(), + Symbol::new(&env, "mod_1") + ); + assert_eq!( + course.module_ids.get(1).unwrap(), + Symbol::new(&env, "mod_2") + ); + assert_eq!( + course.module_ids.get(2).unwrap(), + Symbol::new(&env, "mod_3") + ); // Ordering/ existence checks driven by Course::module_ids still work. let learner = Address::generate(&env); @@ -661,7 +674,10 @@ mod progress_unit_tests { progress.eligible_for_credential = true; env.as_contract(&contract_id, || { env.storage().persistent().set( - &progress_tracker::ProgressTrackerDataKey::Progress(learner.clone(), course_id.clone()), + &progress_tracker::ProgressTrackerDataKey::Progress( + learner.clone(), + course_id.clone(), + ), &progress, ); }); @@ -859,7 +875,11 @@ mod progress_unit_tests { client.complete_module(&learner, &course_a, &Symbol::new(&env, "mod_3")); client.submit_quiz_score(&learner, &course_a, &Symbol::new(&env, "quiz_1"), &80); client.submit_quiz_score(&learner, &course_a, &Symbol::new(&env, "quiz_2"), &70); - assert!(client.get_progress(&learner, &course_a).eligible_for_credential); + assert!( + client + .get_progress(&learner, &course_a) + .eligible_for_credential + ); // Course B: enrolled and quizzed, but the module is never completed, // so it must not count toward courses_completed. @@ -871,7 +891,11 @@ mod progress_unit_tests { client.create_course(&course_b, &1, &1, &module_ids, &quiz_ids); client.enroll(&learner, &course_b); client.submit_quiz_score(&learner, &course_b, &Symbol::new(&env, "quiz_a"), &60); - assert!(!client.get_progress(&learner, &course_b).eligible_for_credential); + assert!( + !client + .get_progress(&learner, &course_b) + .eligible_for_credential + ); let stats = client.get_learner_stats(&learner); assert_eq!(stats.courses_enrolled, 2); @@ -985,7 +1009,11 @@ mod progress_unit_tests { client.complete_module(&learner, &course_id, &Symbol::new(&env, "mod_3")); client.submit_quiz_score(&learner, &course_id, &quiz_id, &20); client.submit_quiz_score(&learner, &course_id, &Symbol::new(&env, "quiz_2"), &20); - assert!(!client.get_progress(&learner, &course_id).eligible_for_credential); + assert!( + !client + .get_progress(&learner, &course_id) + .eligible_for_credential + ); client.retake_quiz(&learner, &course_id, &quiz_id, &90); @@ -2023,4 +2051,38 @@ mod progress_unit_tests { "existing progress must be preserved after archiving" ); } + + // ── Issue #240: contract initialization verification ──────────────────── + + #[test] + fn test_is_initialized_false_before_initialize() { + let env = Env::default(); + let contract_id = env.register_contract(None, ProgressTracker); + let client = ProgressTrackerClient::new(&env, &contract_id); + + assert!(!client.is_initialized()); + } + + #[test] + fn test_is_initialized_true_after_initialize() { + let env = Env::default(); + let (_admin, contract_id) = setup_contract(&env); + let client = ProgressTrackerClient::new(&env, &contract_id); + + assert!(client.is_initialized()); + } + + #[test] + fn test_is_initialized_does_not_mutate_state() { + let env = Env::default(); + let (_admin, contract_id) = setup_contract(&env); + let client = ProgressTrackerClient::new(&env, &contract_id); + + let before = client.contract_metadata(); + let _ = client.is_initialized(); + let _ = client.is_initialized(); + let after = client.contract_metadata(); + + assert_eq!(before, after, "is_initialized must be read-only"); + } } diff --git a/tests/unit/token_tests.rs b/tests/unit/token_tests.rs index b5d72c8..f3a9047 100644 --- a/tests/unit/token_tests.rs +++ b/tests/unit/token_tests.rs @@ -515,7 +515,11 @@ mod token_unit_tests { &env, ( contract_id, - (Symbol::new(&env, "transfer_from"), owner.clone(), recipient.clone()) + ( + Symbol::new(&env, "transfer_from"), + owner.clone(), + recipient.clone() + ) .into_val(&env), (spender, 300i128).into_val(&env), ) @@ -994,7 +998,11 @@ mod token_unit_tests { client.transfer_admin(&new_admin); - assert_eq!(client.admin(), admin, "admin must not change until accepted"); + assert_eq!( + client.admin(), + admin, + "admin must not change until accepted" + ); let pending = client.pending_admin().expect("pending transfer expected"); assert_eq!(pending.new_admin, new_admin); } @@ -1059,7 +1067,8 @@ mod token_unit_tests { // silently dropped or swapped for a different address. client.accept_admin(); assert_eq!( - env.auths()[0].0, new_admin, + env.auths()[0].0, + new_admin, "accept_admin must require new_admin's auth, not the caller's" ); } @@ -1101,7 +1110,8 @@ mod token_unit_tests { client.cancel_admin_transfer(); assert_eq!( - env.auths()[0].0, admin, + env.auths()[0].0, + admin, "cancel_admin_transfer must require the current admin's auth" ); } @@ -1227,5 +1237,39 @@ mod token_unit_tests { ); assert_ne!(client.admin(), first_candidate); } + + // ── Issue #240: contract initialization verification ──────────────────── + + #[test] + fn test_is_initialized_false_before_initialize() { + let env = Env::default(); + let contract_id = env.register_contract(None, LearnToken); + let client = LearnTokenClient::new(&env, &contract_id); + + assert!(!client.is_initialized()); + } + + #[test] + fn test_is_initialized_true_after_initialize() { + let env = Env::default(); + let (_admin, contract_id, _pt_contract_id) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + + assert!(client.is_initialized()); + } + + #[test] + fn test_is_initialized_does_not_mutate_state() { + let env = Env::default(); + let (_admin, contract_id, _pt_contract_id) = setup_token(&env); + let client = LearnTokenClient::new(&env, &contract_id); + + let before = client.contract_metadata(); + let _ = client.is_initialized(); + let _ = client.is_initialized(); + let after = client.contract_metadata(); + + assert_eq!(before, after, "is_initialized must be read-only"); + } } From f2251369e1ec4bb8178e1e17ee9e1656dce41b4d Mon Sep 17 00:00:00 2001 From: Nwokedi Uche Date: Sun, 30 Aug 2026 16:34:04 +0100 Subject: [PATCH 4/4] feat: add get_storage_size() with incremental tracking (#239) Adds a get_storage_size() read-only function to learn-token, credential-nft, and progress-tracker that returns the number of persistent storage entries each contract has written. Soroban has no API to enumerate or count a contract's storage entries at runtime, so the count can't be computed by scanning -- there is nothing to scan. Instead each contract maintains an ordinary persistent counter (StorageSize), kept in sync by routing every persistent write (and, for learn-token, every removal) through a write_entry()/remove_entry() wrapper instead of calling env.storage().persistent().set()/remove() directly. Both wrappers check whether the key already exists before mutating, so overwriting an existing key never double-counts it and removing a key that was never set never underflows the counter. Every existing persistent-storage call site in all three contracts was migrated to the wrappers so the counter reflects real entry counts, not just newly-added code paths. Tests added per contract verify: zero before initialize, the exact count after initialize, +1/+N on genuinely new keys, unchanged on overwrites of existing keys, and (learn-token) -1 on removal with no underflow when removing a nonexistent entry. --- contracts/credential-nft/src/lib.rs | 37 +++++----- contracts/credential-nft/src/metadata.rs | 70 +++++++++++++++++- contracts/credential-nft/src/mint.rs | 16 ++--- contracts/credential-nft/src/verify.rs | 40 +++++------ contracts/progress-tracker/src/lib.rs | 90 +++++++++++++----------- contracts/progress-tracker/src/types.rs | 52 +++++++++++++- tests/unit/credential_tests.rs | 88 +++++++++++++++++++++++ tests/unit/progress_tests.rs | 73 +++++++++++++++++++ tests/unit/token_tests.rs | 6 ++ 9 files changed, 381 insertions(+), 91 deletions(-) diff --git a/contracts/credential-nft/src/lib.rs b/contracts/credential-nft/src/lib.rs index 24b0436..ed446f1 100644 --- a/contracts/credential-nft/src/lib.rs +++ b/contracts/credential-nft/src/lib.rs @@ -53,16 +53,11 @@ impl CredentialNft { if env.storage().persistent().has(&CredentialDataKey::Admin) { return Err(ContractError::AlreadyInitialized); } - env.storage() - .persistent() - .set(&CredentialDataKey::Admin, &admin); - env.storage() - .persistent() - .set(&CredentialDataKey::ProgressTracker, &progress_tracker); - env.storage() - .persistent() - .set(&CredentialDataKey::CredentialCounter, &0u64); - env.storage().persistent().set( + metadata::write_entry(&env, &CredentialDataKey::Admin, &admin); + metadata::write_entry(&env, &CredentialDataKey::ProgressTracker, &progress_tracker); + metadata::write_entry(&env, &CredentialDataKey::CredentialCounter, &0u64); + metadata::write_entry( + &env, &CredentialDataKey::Metadata, &ContractMetadata::new(&env, "credential-nft"), ); @@ -80,6 +75,16 @@ impl CredentialNft { env.storage().persistent().has(&CredentialDataKey::Admin) } + /// Returns the number of persistent storage entries this contract has + /// written (#239). + /// + /// Maintained as a running counter updated on every persistent write and + /// removal, since Soroban has no API to enumerate or count a contract's + /// storage entries at runtime. Read-only and O(1): reads one counter entry. + pub fn get_storage_size(env: Env) -> u64 { + metadata::get_storage_size(&env) + } + /// Get the contract's on-chain name and version (#107). /// /// Lets external tools (indexers, block explorers, upgrade tooling) @@ -423,9 +428,7 @@ impl CredentialNft { .get(&CredentialDataKey::Admin) .expect("not initialized"); admin.require_auth(); - env.storage() - .persistent() - .set(&CredentialDataKey::Paused, &true); + metadata::write_entry(&env, &CredentialDataKey::Paused, &true); // Event would ideally be emitted here, but we will omit it for simplicity if it wasn't added to events.rs } @@ -437,9 +440,7 @@ impl CredentialNft { .get(&CredentialDataKey::Admin) .expect("not initialized"); admin.require_auth(); - env.storage() - .persistent() - .set(&CredentialDataKey::Paused, &false); + metadata::write_entry(&env, &CredentialDataKey::Paused, &false); } /// Returns the admin address. @@ -550,7 +551,7 @@ impl CredentialNft { } let cert_uri = Symbol::new(&env, "cert_uri"); - env.storage().persistent().set(&cert_key, &cert_uri); + metadata::write_entry(&env, &cert_key, &cert_uri); // If credential already minted, update metadata_uri in CredentialInfo if let Some(cred_id) = env.storage().persistent().get::<_, u64>(&dup_key) { @@ -561,7 +562,7 @@ impl CredentialNft { .get::<_, CredentialInfo>(&cred_key) { info.metadata_uri = cert_uri.clone(); - env.storage().persistent().set(&cred_key, &info); + metadata::write_entry(&env, &cred_key, &info); } } diff --git a/contracts/credential-nft/src/metadata.rs b/contracts/credential-nft/src/metadata.rs index bdfed9c..f109536 100644 --- a/contracts/credential-nft/src/metadata.rs +++ b/contracts/credential-nft/src/metadata.rs @@ -1,4 +1,4 @@ -use soroban_sdk::{contracttype, Address, Symbol}; +use soroban_sdk::{contracttype, Address, Env, IntoVal, Symbol, Val}; /// On-chain metadata for a minted credential NFT. #[contracttype] @@ -45,6 +45,74 @@ pub enum CredentialDataKey { CertificateURI(Address, Symbol), /// Emergency pause state (#189). Paused, + /// Running count of persistent storage entries this contract has + /// written, excluding this counter entry itself (#239). + StorageSize, +} + +// ── Storage Size Tracking (#239) ───────────────────────────────────────────── +// +// Soroban has no API to enumerate or count a contract's storage entries at +// runtime, so the count is maintained as an ordinary persistent counter, +// kept in sync by routing every persistent write and removal through +// `write_entry`/`remove_entry` below instead of calling +// `env.storage().persistent().set/remove` directly. Both check whether the +// key already exists before mutating, so overwriting an existing key does +// not double-count it, and removing a key that was never set does not +// underflow the counter. + +/// Get the current persistent-entry count (#239). +/// +/// O(1): reads a single counter entry, never scans storage. +pub fn get_storage_size(env: &Env) -> u64 { + env.storage() + .persistent() + .get(&CredentialDataKey::StorageSize) + .unwrap_or(0) +} + +fn bump_storage_size(env: &Env, delta: i64) { + let current = get_storage_size(env); + let next = if delta >= 0 { + current.saturating_add(delta as u64) + } else { + current.saturating_sub((-delta) as u64) + }; + env.storage() + .persistent() + .set(&CredentialDataKey::StorageSize, &next); +} + +/// Write `value` to persistent storage at `key`, incrementing +/// [`get_storage_size`] iff `key` did not already exist. Use this (instead +/// of `env.storage().persistent().set` directly) for every persistent write +/// so the counter stays accurate. +pub fn write_entry(env: &Env, key: &K, value: &V) +where + K: IntoVal, + V: IntoVal, +{ + let is_new = !env.storage().persistent().has(key); + env.storage().persistent().set(key, value); + if is_new { + bump_storage_size(env, 1); + } +} + +/// Remove `key` from persistent storage, decrementing [`get_storage_size`] +/// iff `key` existed. Use this (instead of +/// `env.storage().persistent().remove` directly) for every persistent +/// removal so the counter stays accurate. +#[allow(dead_code)] +pub fn remove_entry(env: &Env, key: &K) +where + K: IntoVal, +{ + let existed = env.storage().persistent().has(key); + env.storage().persistent().remove(key); + if existed { + bump_storage_size(env, -1); + } } /// Display properties for a credential NFT (#244). diff --git a/contracts/credential-nft/src/mint.rs b/contracts/credential-nft/src/mint.rs index d0caa7c..8a0be03 100644 --- a/contracts/credential-nft/src/mint.rs +++ b/contracts/credential-nft/src/mint.rs @@ -125,9 +125,7 @@ pub fn mint_credential( Some(id) => id, None => panic!("credential ID counter overflow"), }; - env.storage() - .persistent() - .set(&CredentialDataKey::CredentialCounter, &credential_id); + crate::metadata::write_entry(env, &CredentialDataKey::CredentialCounter, &credential_id); // Build credential info let info = CredentialInfo { @@ -142,9 +140,7 @@ pub fn mint_credential( // Store credential data. The owner is available as `info.learner`, so no // separate owner key is kept (#116). - env.storage() - .persistent() - .set(&CredentialDataKey::Credential(credential_id), &info); + crate::metadata::write_entry(env, &CredentialDataKey::Credential(credential_id), &info); // Track credentials per learner let mut learner_creds: soroban_sdk::Vec = env @@ -153,13 +149,14 @@ pub fn mint_credential( .get(&CredentialDataKey::LearnerCredentials(to.clone())) .unwrap_or(soroban_sdk::Vec::new(env)); learner_creds.push_back(credential_id); - env.storage().persistent().set( + crate::metadata::write_entry( + env, &CredentialDataKey::LearnerCredentials(to.clone()), &learner_creds, ); // Store the course-credential mapping to prevent duplicates - env.storage().persistent().set(&dup_key, &credential_id); + crate::metadata::write_entry(env, &dup_key, &credential_id); // Index credentials by course for reverse lookup (#105) let mut course_creds: soroban_sdk::Vec = env @@ -168,7 +165,8 @@ pub fn mint_credential( .get(&CredentialDataKey::CourseCredentials(course_id.clone())) .unwrap_or(soroban_sdk::Vec::new(env)); course_creds.push_back(credential_id); - env.storage().persistent().set( + crate::metadata::write_entry( + env, &CredentialDataKey::CourseCredentials(course_id.clone()), &course_creds, ); diff --git a/contracts/credential-nft/src/verify.rs b/contracts/credential-nft/src/verify.rs index 656ff50..107d71c 100644 --- a/contracts/credential-nft/src/verify.rs +++ b/contracts/credential-nft/src/verify.rs @@ -174,14 +174,10 @@ pub fn revoke_credential(env: &Env, credential_id: u64) { } info.revoked = true; - env.storage() - .persistent() - .set(&CredentialDataKey::Credential(credential_id), &info); + crate::metadata::write_entry(env, &CredentialDataKey::Credential(credential_id), &info); // Kept in sync with `info.revoked` so `is_credential_valid` can check // revocation without deserializing the full `CredentialInfo` (#109). - env.storage() - .persistent() - .set(&CredentialDataKey::Revoked(credential_id), &true); + crate::metadata::write_entry(env, &CredentialDataKey::Revoked(credential_id), &true); // #104 — prune from learner's credential list let mut learner_list: Vec = env @@ -193,7 +189,8 @@ pub fn revoke_credential(env: &Env, credential_id: u64) { (0..learner_list.len()).find(|&i| learner_list.get(i).unwrap() == credential_id) { learner_list.remove(pos); - env.storage().persistent().set( + crate::metadata::write_entry( + env, &CredentialDataKey::LearnerCredentials(info.learner.clone()), &learner_list, ); @@ -211,7 +208,8 @@ pub fn revoke_credential(env: &Env, credential_id: u64) { (0..course_list.len()).find(|&i| course_list.get(i).unwrap() == credential_id) { course_list.remove(pos); - env.storage().persistent().set( + crate::metadata::write_entry( + env, &CredentialDataKey::CourseCredentials(info.course_id.clone()), &course_list, ); @@ -250,16 +248,14 @@ pub fn revoke_credential_with_reason(env: &Env, credential_id: u64, reason: Symb } info.revoked = true; - env.storage() - .persistent() - .set(&CredentialDataKey::Credential(credential_id), &info); - env.storage() - .persistent() - .set(&CredentialDataKey::Revoked(credential_id), &true); + crate::metadata::write_entry(env, &CredentialDataKey::Credential(credential_id), &info); + crate::metadata::write_entry(env, &CredentialDataKey::Revoked(credential_id), &true); // Store the revocation reason (#194) - env.storage() - .persistent() - .set(&CredentialDataKey::RevocationReason(credential_id), &reason); + crate::metadata::write_entry( + env, + &CredentialDataKey::RevocationReason(credential_id), + &reason, + ); // #104 — prune from learner's credential list let mut learner_list: Vec = env @@ -271,7 +267,8 @@ pub fn revoke_credential_with_reason(env: &Env, credential_id: u64, reason: Symb (0..learner_list.len()).find(|&i| learner_list.get(i).unwrap() == credential_id) { learner_list.remove(pos); - env.storage().persistent().set( + crate::metadata::write_entry( + env, &CredentialDataKey::LearnerCredentials(info.learner.clone()), &learner_list, ); @@ -289,7 +286,8 @@ pub fn revoke_credential_with_reason(env: &Env, credential_id: u64, reason: Symb (0..course_list.len()).find(|&i| course_list.get(i).unwrap() == credential_id) { course_list.remove(pos); - env.storage().persistent().set( + crate::metadata::write_entry( + env, &CredentialDataKey::CourseCredentials(info.course_id.clone()), &course_list, ); @@ -340,9 +338,7 @@ pub fn renew_credential(env: &Env, credential_id: u64, new_expiry: u32) { } info.expires_at = new_expiry; - env.storage() - .persistent() - .set(&CredentialDataKey::Credential(credential_id), &info); + crate::metadata::write_entry(env, &CredentialDataKey::Credential(credential_id), &info); env.events().publish( (Symbol::new(env, "credential_renewed"),), diff --git a/contracts/progress-tracker/src/lib.rs b/contracts/progress-tracker/src/lib.rs index 6b428a3..c9f5ca7 100644 --- a/contracts/progress-tracker/src/lib.rs +++ b/contracts/progress-tracker/src/lib.rs @@ -38,10 +38,9 @@ impl ProgressTracker { { return Err(ContractError::AlreadyInitialized); } - env.storage() - .persistent() - .set(&ProgressTrackerDataKey::Admin, &admin); - env.storage().persistent().set( + types::write_entry(&env, &ProgressTrackerDataKey::Admin, &admin); + types::write_entry( + &env, &ProgressTrackerDataKey::Metadata, &ContractMetadata::new(&env, "progress-tracker"), ); @@ -101,6 +100,16 @@ impl ProgressTracker { .has(&ProgressTrackerDataKey::Admin) } + /// Returns the number of persistent storage entries this contract has + /// written (#239). + /// + /// Maintained as a running counter updated on every persistent write, + /// since Soroban has no API to enumerate or count a contract's storage + /// entries at runtime. Read-only and O(1): reads one counter entry. + pub fn get_storage_size(env: Env) -> u64 { + types::get_storage_size(&env) + } + /// Register a new course with its modules and quizzes. /// /// # Arguments @@ -192,9 +201,11 @@ impl ProgressTracker { version: 1, }; - env.storage() - .persistent() - .set(&ProgressTrackerDataKey::Course(course_id.clone()), &course); + types::write_entry( + &env, + &ProgressTrackerDataKey::Course(course_id.clone()), + &course, + ); env.events().publish( (Symbol::new(&env, "course_created"),), @@ -290,7 +301,7 @@ impl ProgressTracker { completed_version: None, }; - env.storage().persistent().set(&key, &progress); + types::write_entry(&env, &key, &progress); // Index the enrollment so learner-wide aggregates can be computed // without scanning every course in the contract (#232). @@ -301,7 +312,7 @@ impl ProgressTracker { .get(&courses_key) .unwrap_or_else(|| Vec::new(&env)); courses.push_back(course_id.clone()); - env.storage().persistent().set(&courses_key, &courses); + types::write_entry(&env, &courses_key, &courses); env.events().publish( (symbol_short!("enrolled"),), @@ -490,26 +501,24 @@ impl ProgressTracker { } // Mark module as completed - env.storage().persistent().set(&completed_key, &true); + types::write_entry(&env, &completed_key, &true); progress.modules_completed_bitmap |= 1 << idx; let was_eligible = progress.eligible_for_credential; progress.overall_progress = rewards::calculate_progress(&course, &progress); - progress.eligible_for_credential = - rewards::is_eligible_for_credential(&course, &progress); + progress.eligible_for_credential = rewards::is_eligible_for_credential(&course, &progress); // Record the course version at the moment eligibility is reached (#245). if !was_eligible && progress.eligible_for_credential { progress.completed_version = Some(course.version); } - env.storage().persistent().set( + types::write_entry( + &env, &ProgressTrackerDataKey::Progress(learner.clone(), course_id.clone()), - &progress, + &*progress, ); - progress.overall_progress = rewards::calculate_progress(course, progress); - progress.eligible_for_credential = rewards::is_eligible_for_credential(course, progress); env.events().publish( (Symbol::new(env, "module_completed"),), @@ -743,7 +752,7 @@ impl ProgressTracker { submitted_at: env.ledger().timestamp(), }; - env.storage().persistent().set(&quiz_key, &result); + types::write_entry(&env, &quiz_key, &result); progress.quizzes_submitted += 1; progress.total_quiz_score += score as u64; @@ -753,8 +762,7 @@ 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(&course, &progress); - progress.eligible_for_credential = - rewards::is_eligible_for_credential(&course, &progress); + progress.eligible_for_credential = rewards::is_eligible_for_credential(&course, &progress); // Record the course version at the moment eligibility is reached (#245). if !was_eligible && progress.eligible_for_credential { @@ -762,12 +770,11 @@ impl ProgressTracker { } // Single write with all updated fields - env.storage().persistent().set( + types::write_entry( + &env, &ProgressTrackerDataKey::Progress(learner.clone(), course_id.clone()), - &progress, + &*progress, ); - progress.overall_progress = rewards::calculate_progress(course, progress); - progress.eligible_for_credential = rewards::is_eligible_for_credential(course, progress); env.events().publish( (Symbol::new(env, "quiz_submitted"),), @@ -882,7 +889,7 @@ impl ProgressTracker { // divisor -- is unchanged by a retake. result.score = new_score; result.submitted_at = env.ledger().timestamp(); - env.storage().persistent().set(&quiz_key, &result); + types::write_entry(&env, &quiz_key, &result); progress.total_quiz_score += (new_score - previous_score) as u64; @@ -896,7 +903,8 @@ impl ProgressTracker { progress.completed_version = Some(course.version); } - env.storage().persistent().set( + types::write_entry( + &env, &ProgressTrackerDataKey::Progress(learner.clone(), course_id.clone()), &progress, ); @@ -1199,9 +1207,11 @@ impl ProgressTracker { } course.archived = true; - env.storage() - .persistent() - .set(&ProgressTrackerDataKey::Course(course_id.clone()), &course); + types::write_entry( + &env, + &ProgressTrackerDataKey::Course(course_id.clone()), + &course, + ); env.events() .publish((Symbol::new(&env, "course_archived"),), (&course_id,)); @@ -1231,9 +1241,11 @@ impl ProgressTracker { .expect("course not found"); course.content_hash = content_hash.clone(); - env.storage() - .persistent() - .set(&ProgressTrackerDataKey::Course(course_id.clone()), &course); + types::write_entry( + &env, + &ProgressTrackerDataKey::Course(course_id.clone()), + &course, + ); env.events().publish( (Symbol::new(&env, "content_hash_set"),), @@ -1349,9 +1361,11 @@ impl ProgressTracker { } course.prerequisites = prerequisites.clone(); - env.storage() - .persistent() - .set(&ProgressTrackerDataKey::Course(course_id.clone()), &course); + types::write_entry( + &env, + &ProgressTrackerDataKey::Course(course_id.clone()), + &course, + ); env.events().publish( (Symbol::new(&env, "prerequisites_set"),), @@ -1500,9 +1514,7 @@ impl ProgressTracker { .get(&ProgressTrackerDataKey::Admin) .expect("not initialized"); admin.require_auth(); - env.storage() - .persistent() - .set(&ProgressTrackerDataKey::Paused, &true); + types::write_entry(&env, &ProgressTrackerDataKey::Paused, &true); // We omit events here to avoid adding it to events.rs } @@ -1514,9 +1526,7 @@ impl ProgressTracker { .get(&ProgressTrackerDataKey::Admin) .expect("not initialized"); admin.require_auth(); - env.storage() - .persistent() - .set(&ProgressTrackerDataKey::Paused, &false); + types::write_entry(&env, &ProgressTrackerDataKey::Paused, &false); } /// Returns the admin address. diff --git a/contracts/progress-tracker/src/types.rs b/contracts/progress-tracker/src/types.rs index 48431ce..9d3c233 100644 --- a/contracts/progress-tracker/src/types.rs +++ b/contracts/progress-tracker/src/types.rs @@ -1,5 +1,5 @@ use chainlearn_shared::ContractMetadata; -use soroban_sdk::{contracttype, Address, Symbol, Vec}; +use soroban_sdk::{contracttype, Address, Env, IntoVal, Symbol, Val, Vec}; /// Represents a course with its modules and total module count. #[contracttype] @@ -161,4 +161,54 @@ pub enum ProgressTrackerDataKey { /// The address a learner has delegated progress-tracking to, if any /// (#222). Absent when the learner has no active delegation. DelegatedTo(Address), + /// Running count of persistent storage entries this contract has + /// written, excluding this counter entry itself (#239). + StorageSize, +} + +// ── Storage Size Tracking (#239) ───────────────────────────────────────────── +// +// Soroban has no API to enumerate or count a contract's storage entries at +// runtime, so the count is maintained as an ordinary persistent counter, +// kept in sync by routing every persistent write through `write_entry` +// below instead of calling `env.storage().persistent().set` directly. It +// checks whether the key already exists before writing, so overwriting an +// existing key does not double-count it. + +/// Get the current persistent-entry count (#239). +/// +/// O(1): reads a single counter entry, never scans storage. +pub fn get_storage_size(env: &Env) -> u64 { + env.storage() + .persistent() + .get(&ProgressTrackerDataKey::StorageSize) + .unwrap_or(0) +} + +fn bump_storage_size(env: &Env, delta: i64) { + let current = get_storage_size(env); + let next = if delta >= 0 { + current.saturating_add(delta as u64) + } else { + current.saturating_sub((-delta) as u64) + }; + env.storage() + .persistent() + .set(&ProgressTrackerDataKey::StorageSize, &next); +} + +/// Write `value` to persistent storage at `key`, incrementing +/// [`get_storage_size`] iff `key` did not already exist. Use this (instead +/// of `env.storage().persistent().set` directly) for every persistent write +/// so the counter stays accurate. +pub fn write_entry(env: &Env, key: &K, value: &V) +where + K: IntoVal, + V: IntoVal, +{ + let is_new = !env.storage().persistent().has(key); + env.storage().persistent().set(key, value); + if is_new { + bump_storage_size(env, 1); + } } diff --git a/tests/unit/credential_tests.rs b/tests/unit/credential_tests.rs index 2409a60..e599df1 100644 --- a/tests/unit/credential_tests.rs +++ b/tests/unit/credential_tests.rs @@ -490,4 +490,92 @@ mod credential_unit_tests { assert_eq!(before, after, "is_initialized must be read-only"); } + + // ── Issue #239: storage size tracking ──────────────────────────────────── + + #[test] + fn test_storage_size_zero_before_initialize() { + let env = Env::default(); + let contract_id = env.register_contract(None, CredentialNft); + let client = CredentialNftClient::new(&env, &contract_id); + + assert_eq!(client.get_storage_size(), 0); + } + + #[test] + fn test_storage_size_increases_after_initialize() { + let env = Env::default(); + let (_admin, contract_id, _tracker_id) = setup_contract(&env); + let client = CredentialNftClient::new(&env, &contract_id); + + // initialize() writes Admin, ProgressTracker, CredentialCounter, and + // Metadata -- 4 distinct new keys. + assert_eq!(client.get_storage_size(), 4); + } + + #[test] + fn test_storage_size_increases_on_mint() { + let env = Env::default(); + let (_admin, contract_id, tracker_id) = setup_contract(&env); + let client = CredentialNftClient::new(&env, &contract_id); + env.mock_all_auths(); + + let learner = Address::generate(&env); + let course_id = Symbol::new(&env, "rust_101"); + enrolled_and_completed_with_score(&env, &tracker_id, &learner, &course_id, 85); + + let before = client.get_storage_size(); + client.mint_credential(&learner, &course_id, &85, &Symbol::new(&env, "ipfs_Qm123")); + + // A first-time mint for a fresh learner+course creates 4 new keys: + // Credential(id), LearnerCredentials(learner), the + // CourseCredential(learner, course) dedup key, and + // CourseCredentials(course). CredentialCounter already existed. + assert_eq!(client.get_storage_size(), before + 4); + } + + #[test] + fn test_storage_size_unchanged_on_overwrite() { + let env = Env::default(); + let (_admin, contract_id, tracker_id) = setup_contract(&env); + let client = CredentialNftClient::new(&env, &contract_id); + env.mock_all_auths(); + + let learner = Address::generate(&env); + let course_id = Symbol::new(&env, "rust_101"); + enrolled_and_completed_with_score(&env, &tracker_id, &learner, &course_id, 85); + let cred_id = + client.mint_credential(&learner, &course_id, &85, &Symbol::new(&env, "ipfs_Qm123")); + + let before = client.get_storage_size(); + // Revoking overwrites the existing Credential(id) entry and creates + // exactly one new entry: Revoked(id). + client.revoke_credential(&cred_id); + assert_eq!(client.get_storage_size(), before + 1); + } + + #[test] + fn test_storage_size_does_not_change_on_read_only_calls() { + let env = Env::default(); + let (_admin, contract_id, tracker_id) = setup_contract(&env); + let client = CredentialNftClient::new(&env, &contract_id); + env.mock_all_auths(); + + let learner = Address::generate(&env); + let course_id = Symbol::new(&env, "rust_101"); + enrolled_and_completed_with_score(&env, &tracker_id, &learner, &course_id, 85); + let cred_id = client.mint_credential(&learner, &course_id, &85, &Symbol::new(&env, "ipfs_Qm123")); + + let before = client.get_storage_size(); + let _ = client.verify_credential(&cred_id); + let _ = client.is_credential_valid(&cred_id); + let _ = client.get_credentials_for(&learner, &0, &10); + let _ = client.try_transfer(&learner, &Address::generate(&env), &cred_id); + + assert_eq!( + client.get_storage_size(), + before, + "read-only calls (including the rejected transfer) must not change storage size" + ); + } } diff --git a/tests/unit/progress_tests.rs b/tests/unit/progress_tests.rs index e24dd94..04a1c15 100644 --- a/tests/unit/progress_tests.rs +++ b/tests/unit/progress_tests.rs @@ -2085,4 +2085,77 @@ mod progress_unit_tests { assert_eq!(before, after, "is_initialized must be read-only"); } + + // ── Issue #239: storage size tracking ──────────────────────────────────── + + #[test] + fn test_storage_size_zero_before_initialize() { + let env = Env::default(); + let contract_id = env.register_contract(None, ProgressTracker); + let client = ProgressTrackerClient::new(&env, &contract_id); + + assert_eq!(client.get_storage_size(), 0); + } + + #[test] + fn test_storage_size_increases_after_initialize() { + let env = Env::default(); + let (_admin, contract_id) = setup_contract(&env); + let client = ProgressTrackerClient::new(&env, &contract_id); + + // initialize() writes Admin and Metadata -- 2 distinct new keys. + assert_eq!(client.get_storage_size(), 2); + } + + #[test] + fn test_storage_size_increases_on_new_entry() { + let env = Env::default(); + let (_admin, contract_id) = setup_contract(&env); + let client = ProgressTrackerClient::new(&env, &contract_id); + env.mock_all_auths(); + + let before = client.get_storage_size(); + let _course_id = create_test_course(&env, &client); + + // create_course() writes exactly one new Course(course_id) entry. + assert_eq!(client.get_storage_size(), before + 1); + } + + #[test] + fn test_storage_size_unchanged_on_overwrite() { + let env = Env::default(); + let (_admin, contract_id) = setup_contract(&env); + let client = ProgressTrackerClient::new(&env, &contract_id); + env.mock_all_auths(); + + let course_id = create_test_course(&env, &client); + let before = client.get_storage_size(); + + // Archiving overwrites the existing Course(course_id) entry -- + // no new key is created. + client.archive_course(&course_id); + assert_eq!( + client.get_storage_size(), + before, + "overwriting an existing key must not change the count" + ); + } + + #[test] + fn test_storage_size_tracks_enrollment_writes() { + let env = Env::default(); + let (_admin, contract_id) = setup_contract(&env); + let client = ProgressTrackerClient::new(&env, &contract_id); + env.mock_all_auths(); + + let course_id = create_test_course(&env, &client); + let learner = Address::generate(&env); + + let before = client.get_storage_size(); + client.enroll(&learner, &course_id); + + // enroll() writes Progress(learner, course_id) and + // LearnerCourses(learner) -- 2 new entries for a first-time learner. + assert_eq!(client.get_storage_size(), before + 2); + } } diff --git a/tests/unit/token_tests.rs b/tests/unit/token_tests.rs index f3a9047..fe58867 100644 --- a/tests/unit/token_tests.rs +++ b/tests/unit/token_tests.rs @@ -1271,5 +1271,11 @@ mod token_unit_tests { assert_eq!(before, after, "is_initialized must be read-only"); } + + // Issue #239 (storage-size tracking) is already implemented and covered + // by an existing, more thorough test suite in + // `contracts/learn-token/src/lib.rs` (see its "Issue #254: storage size + // tracking" section) -- that implementation predates this branch on + // `main`, so no duplicate tests are added here. }