diff --git a/contracts/remittance_nft/src/lib.rs b/contracts/remittance_nft/src/lib.rs index 5e1b1df0..44e0b25d 100644 --- a/contracts/remittance_nft/src/lib.rs +++ b/contracts/remittance_nft/src/lib.rs @@ -813,15 +813,37 @@ impl RemittanceNFT { Ok(()) } + /// Update the minimum repayment amount accepted by `update_score`. + /// + /// Emits a `MinRepaymentUpdated` event carrying both the previous and the + /// new amount (#1146) so this risk-parameter change is observable off-chain + /// by the indexer and by audit trails. + /// + /// Keeps the typed-error contract introduced on `main` (#1843): a negative + /// amount is rejected with `NftError::InvalidAmount` rather than panicking. pub fn set_min_repayment_amount(env: Env, amount: i128) -> Result<(), NftError> { - Self::admin(&env).require_auth(); + // Hoisted so the already-read admin can be reused as the event's actor + // topic. Evaluation order is unchanged: `admin()` is read before + // `require_auth()`, exactly as it was when inlined. + let admin = Self::admin(&env); + admin.require_auth(); if amount < 0 { return Err(NftError::InvalidAmount); } + // Capture the outgoing value before the overwrite so the event reports + // the full old -> new transition rather than just the new value. + let old_amount = Self::min_repayment_amount(&env); env.storage() .instance() .set(&DataKey::MinRepaymentAmount, &amount); Self::bump_instance_ttl(&env); + // Topics `(event, admin)` and data `(old, new)` follow the admin + // config-update convention used by loan_manager/lending_pool so the + // existing indexer decoding of `MinRepaymentUpdated` applies as-is. + env.events().publish( + (Symbol::new(&env, "MinRepaymentUpdated"), admin), + (old_amount, amount), + ); Ok(()) } @@ -1227,18 +1249,37 @@ impl RemittanceNFT { Ok(()) } + /// Update the number of defaults after which an NFT is auto-burned. + /// + /// Emits a `DefaultBurnThresholdUpdated` event carrying both the previous + /// and the new threshold (#1146) so this risk-parameter change is + /// observable off-chain by the indexer and by audit trails. pub fn set_default_burn_threshold(env: Env, threshold: u32) -> Result<(), NftError> { if threshold == 0 || threshold > Self::MAX_ALLOWED_BURN_THRESHOLD { return Err(NftError::InvalidThreshold); } - Self::admin(&env).require_auth(); + // Hoisted so the already-read admin can be reused as the event's actor + // topic. Evaluation order is unchanged: `admin()` is read before + // `require_auth()`, exactly as it was when inlined. + let admin = Self::admin(&env); + admin.require_auth(); Self::assert_not_paused(&env)?; + // Capture the outgoing value before the overwrite so the event reports + // the full old -> new transition rather than just the new value. + let old_threshold = Self::default_burn_threshold(&env); env.storage() .instance() .set(&Self::burn_threshold_key(), &threshold); Self::bump_instance_ttl(&env); + // Topics `(event, admin)` and data `(old, new)` follow the admin + // config-update convention used by loan_manager/lending_pool. + env.events().publish( + (Symbol::new(&env, "DefaultBurnThresholdUpdated"), admin), + (old_threshold, threshold), + ); + Ok(()) } diff --git a/contracts/remittance_nft/src/test.rs b/contracts/remittance_nft/src/test.rs index bc4902a6..afc92110 100644 --- a/contracts/remittance_nft/src/test.rs +++ b/contracts/remittance_nft/src/test.rs @@ -3074,3 +3074,159 @@ fn test_lifecycle_methods_resume_after_unpause() { client.burn(&second_user, &None); assert!(client.get_metadata(&second_user).is_none()); } + +// ── #1146: admin parameter setter events ───────────────────────────────────── + +/// `set_default_burn_threshold` must publish an event that carries both the +/// previous and the new threshold, using the same `(event, admin)` topic shape +/// and `(old, new)` payload shape as the other admin config-change events. +#[test] +fn test_set_default_burn_threshold_emits_old_and_new_value() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let contract_id = env.register(RemittanceNFT, ()); + let client = RemittanceNFTClient::new(&env, &contract_id); + + client.initialize(&admin); + + // initialize() seeds DEFAULT_BURN_THRESHOLD, so the first change is a real + // default -> new transition. + let initial = client.get_default_burn_threshold(); + + // The test host only retains the most recent invocation's events, so the + // event has to be captured before any other contract call is made. + client.set_default_burn_threshold(&5); + let first_events = env.events().all(); + assert_eq!(first_events.len(), 1); + let first_event = first_events.get(0).unwrap(); + let first_topic = Symbol::from_val(&env, &first_event.1.get(0).unwrap()); + let first_topic_1 = Address::from_val(&env, &first_event.1.get(1).unwrap()); + let first_data = <(u32, u32)>::from_val(&env, &first_event.2); + assert_eq!( + first_topic, + Symbol::new(&env, "DefaultBurnThresholdUpdated") + ); + assert_eq!(first_topic_1, admin); + assert_eq!(first_data, (initial, 5u32)); + + // State is persisted only after the event is published, and reading it back + // replaces the event buffer, so it is asserted after the capture above. + assert_eq!(client.get_default_burn_threshold(), 5); + + // A second change proves the `old` value is read from storage before the + // overwrite rather than being a hard-coded default. + client.set_default_burn_threshold(&9); + + let events = env.events().all(); + assert_eq!(events.len(), 1); + let event = events.get(0).unwrap(); + let topic_0 = Symbol::from_val(&env, &event.1.get(0).unwrap()); + let topic_1 = Address::from_val(&env, &event.1.get(1).unwrap()); + let data = <(u32, u32)>::from_val(&env, &event.2); + + assert_eq!(topic_0, Symbol::new(&env, "DefaultBurnThresholdUpdated")); + assert_eq!(topic_1, admin); + assert_eq!(data, (5u32, 9u32)); + assert_eq!(client.get_default_burn_threshold(), 9); +} + +/// `set_min_repayment_amount` must publish an event that carries both the +/// previous and the new amount, using the same convention as above. +#[test] +fn test_set_min_repayment_amount_emits_old_and_new_value() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let contract_id = env.register(RemittanceNFT, ()); + let client = RemittanceNFTClient::new(&env, &contract_id); + + client.initialize(&admin); + + let initial = client.get_min_repayment_amount(); + + // The test host only retains the most recent invocation's events, so the + // event has to be captured before any other contract call is made. + client.set_min_repayment_amount(&1_000_000); + let first_events = env.events().all(); + assert_eq!(first_events.len(), 1); + let first_event = first_events.get(0).unwrap(); + let first_topic = Symbol::from_val(&env, &first_event.1.get(0).unwrap()); + let first_topic_1 = Address::from_val(&env, &first_event.1.get(1).unwrap()); + let first_data = <(i128, i128)>::from_val(&env, &first_event.2); + assert_eq!(first_topic, Symbol::new(&env, "MinRepaymentUpdated")); + assert_eq!(first_topic_1, admin); + assert_eq!(first_data, (initial, 1_000_000i128)); + + // State is persisted only after the event is published, and reading it back + // replaces the event buffer, so it is asserted after the capture above. + assert_eq!(client.get_min_repayment_amount(), 1_000_000); + + client.set_min_repayment_amount(&2_500_000); + + let events = env.events().all(); + assert_eq!(events.len(), 1); + let event = events.get(0).unwrap(); + let topic_0 = Symbol::from_val(&env, &event.1.get(0).unwrap()); + let topic_1 = Address::from_val(&env, &event.1.get(1).unwrap()); + let data = <(i128, i128)>::from_val(&env, &event.2); + + assert_eq!(topic_0, Symbol::new(&env, "MinRepaymentUpdated")); + assert_eq!(topic_1, admin); + assert_eq!(data, (1_000_000i128, 2_500_000i128)); + assert_eq!(client.get_min_repayment_amount(), 2_500_000); +} + +/// The event is emitted only after validation succeeds, so a rejected change +/// must leave no parameter-update event behind. +#[test] +fn test_set_default_burn_threshold_rejected_value_emits_no_event() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let contract_id = env.register(RemittanceNFT, ()); + let client = RemittanceNFTClient::new(&env, &contract_id); + + client.initialize(&admin); + + // Both rejected calls are the most recent invocation, so an empty event + // buffer proves validation ran before any emission. + assert_eq!( + client.try_set_default_burn_threshold(&0), + Err(Ok(NftError::InvalidThreshold)) + ); + assert_eq!(env.events().all().len(), 0); + + assert_eq!( + client.try_set_default_burn_threshold(&(RemittanceNFT::MAX_ALLOWED_BURN_THRESHOLD + 1)), + Err(Ok(NftError::InvalidThreshold)) + ); + assert_eq!(env.events().all().len(), 0); + + // A rejected change must also leave the stored value untouched. + assert_eq!( + client.get_default_burn_threshold(), + RemittanceNFT::DEFAULT_BURN_THRESHOLD + ); +} + +/// The setter is still admin-only: adding the event emission did not weaken the +/// authorization check. +#[test] +#[should_panic] +fn test_set_min_repayment_amount_requires_admin_auth() { + let env = Env::default(); + let admin = Address::generate(&env); + + let contract_id = env.register(RemittanceNFT, ()); + let client = RemittanceNFTClient::new(&env, &contract_id); + + env.mock_all_auths(); + client.initialize(&admin); + + env.mock_auths(&[]); + client.set_min_repayment_amount(&1_000_000); +}