fix(remittance_nft): emit old/new value events from the two parameter setters (#1146)

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

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