fix: map governance failures to typed error (#482)

bettapay_common/src/storage.rs

@@ -208,12 +208,20 @@ mod compatibility_tests {
     fn common_data_key_encoding_matches_legacy() {
         let env = Env::default();
         let mut map: soroban_sdk::Map<Val, u32> = soroban_sdk::Map::new(&env);
-        
+
         map.set(LegacyDataKey::RecoveryAddress.into_val(&env), 1u32);
-        assert_eq!(map.get(CommonDataKey::RecoveryAddress.into_val(&env)), Some(1u32), "RecoveryAddress encoding mismatch");
+        assert_eq!(
+            map.get(CommonDataKey::RecoveryAddress.into_val(&env)),
+            Some(1u32),
+            "RecoveryAddress encoding mismatch"
+        );
 
         map.set(LegacyDataKey::PendingRecovery.into_val(&env), 2u32);
-        assert_eq!(map.get(CommonDataKey::PendingRecovery.into_val(&env)), Some(2u32), "PendingRecovery encoding mismatch");
+        assert_eq!(
+            map.get(CommonDataKey::PendingRecovery.into_val(&env)),
+            Some(2u32),
+            "PendingRecovery encoding mismatch"
+        );
 
         // Note: Paused was also a unit variant in the legacy DataKey.
         // We'll just define another legacy enum for it or reuse the same.
@@ -223,11 +231,19 @@ mod compatibility_tests {
             Paused,
             SystemParam(soroban_sdk::Symbol),
         }
-        
+
         map.set(LegacyDataKey2::Paused.into_val(&env), 3u32);
-        assert_eq!(map.get(CommonDataKey::Paused.into_val(&env)), Some(3u32), "Paused encoding mismatch");
+        assert_eq!(
+            map.get(CommonDataKey::Paused.into_val(&env)),
+            Some(3u32),
+            "Paused encoding mismatch"
+        );
 
         map.set(LegacyDataKey::Threshold.into_val(&env), 4u32);
-        assert_eq!(map.get(CommonDataKey::Threshold.into_val(&env)), Some(4u32), "Threshold encoding mismatch");
+        assert_eq!(
+            map.get(CommonDataKey::Threshold.into_val(&env)),
+            Some(4u32),
+            "Threshold encoding mismatch"
+        );
     }
 }

governance_contract/src/anchor_no_event_error_tests.rs

@@ -224,7 +224,8 @@ fn change_threshold_emits_no_event_when_insufficient_signatures() {
     let recovery = Address::generate(&env);
     let contract_id = env.register_contract(None, GovernanceContract);
     let client = GovernanceContractClient::new(&env, &contract_id);
-    client.init(&admins, &2, &recovery);
+    let deployer = Address::generate(&env);
+    client.init(&deployer, &admins, &2, &recovery);
 
     let single_signer = vec![&env, a1.clone()];
     let prev = env.events().all().len();

governance_contract/src/lib.rs

@@ -339,7 +339,13 @@ impl GovernanceContract {
     /// # Errors
     ///
     /// Panics with `GovernanceError::AlreadyInitialized` if already initialised.
-    pub fn init(env: Env, deployer: Address, admins: Vec<Address>, threshold: u32, recovery_address: Address) {
+    pub fn init(
+        env: Env,
+        deployer: Address,
+        admins: Vec<Address>,
+        threshold: u32,
+        recovery_address: Address,
+    ) {
         if env.storage().instance().has(&DataKey::Admin) {
             panic_with_error!(&env, GovernanceError::AlreadyInitialized);
         }
@@ -392,11 +398,7 @@ impl GovernanceContract {
     pub fn update_recovery_address(env: Env, signers: Vec<Address>, new_recovery: Address) {
         verify_admin_auth(&env, &signers, read_threshold(&env));
         let admin = signers.get(0).unwrap();
-        assert_not_zero(
-            &env,
-            &new_recovery,
-            GovernanceError::InvalidRecoveryAddress,
-        );
+        assert_not_zero(&env, &new_recovery, GovernanceError::InvalidRecoveryAddress);
         env.storage()
             .instance()
             .set(&CommonDataKey::RecoveryAddress, &new_recovery);
@@ -512,7 +514,9 @@ impl GovernanceContract {
 
         let new_admins = soroban_sdk::vec![&env, pending.new_admin.clone()];
         env.storage().instance().set(&DataKey::Admin, &new_admins);
-        env.storage().instance().set(&CommonDataKey::Threshold, &1u32);
+        env.storage()
+            .instance()
+            .set(&CommonDataKey::Threshold, &1u32);
         env.storage()
             .instance()
             .remove(&CommonDataKey::PendingRecovery);
@@ -706,7 +710,9 @@ impl GovernanceContract {
         let key = DataKey::Anchor(asset.clone());
         let old_anchor: Option<Address> = env.storage().persistent().get(&key);
         env.storage().persistent().set(&key, &anchor.clone());
-        env.storage().persistent().extend_ttl(&key, ANCHOR_TTL_THRESHOLD, ANCHOR_TTL_BUMP);
+        env.storage()
+            .persistent()
+            .extend_ttl(&key, ANCHOR_TTL_THRESHOLD, ANCHOR_TTL_BUMP);
         env.events().publish(
             (Symbol::new(&env, events::ANCHOR_UPSERTED_EVENT), asset),
             (old_anchor, anchor),
@@ -908,8 +914,8 @@ mod real_auth_tests;
 mod tests {
     use super::*;
     use proptest::prelude::*;
-    use soroban_sdk::testutils::{Address as _, Events};
     use soroban_sdk::testutils::storage::Persistent;
+    use soroban_sdk::testutils::{Address as _, Events};
     use soroban_sdk::{vec, Bytes, FromVal, String};
 
     fn setup() -> (
@@ -927,7 +933,7 @@ mod tests {
         let recovery_address = Address::generate(&env);
         let contract_id = env.register_contract(None, GovernanceContract);
         let client = GovernanceContractClient::new(&env, &contract_id);
-    let deployer = Address::generate(&env);
+        let deployer = Address::generate(&env);
         client.init(&deployer, &admins, &2, &recovery_address);
         (env, client, admins, recovery_address)
     }
@@ -1015,7 +1021,10 @@ mod tests {
         let bad_hash = upload_test_wasm(&env); // empty wasm — no supports_interface
 
         let result = client.try_upgrade(&admins, &bad_hash);
-        assert!(result.is_err(), "upgrade with non-conforming wasm must be rejected");
+        assert!(
+            result.is_err(),
+            "upgrade with non-conforming wasm must be rejected"
+        );
 
         // Contract is intact after the failed upgrade.
         let live_client = GovernanceContractClient::new(&env, &client.address);
@@ -1025,8 +1034,8 @@ mod tests {
     #[test]
     #[should_panic(expected = "Error(Contract, #1)")]
     fn governance_rejects_double_initialization() {
-        let (_env, client, admins, recovery) = setup();
-    let deployer = Address::generate(&env);
+        let (env, client, admins, recovery) = setup();
+        let deployer = Address::generate(&env);
         client.init(&deployer, &admins, &2, &recovery);
     }
 
@@ -1039,8 +1048,8 @@ mod tests {
         let recovery = Address::generate(&env);
         let contract_id = env.register_contract(None, GovernanceContract);
         let client = GovernanceContractClient::new(&env, &contract_id);
-    let deployer = Address::generate(&env);
-        client.init(&deployer, &vec![env, admin], &0, &recovery);
+        let deployer = Address::generate(&env);
+        client.init(&deployer, &soroban_sdk::vec![&env, admin], &0, &recovery);
     }
 
     #[test]
@@ -1175,7 +1184,7 @@ mod tests {
     #[should_panic(expected = "Error(Contract, #4)")]
     fn set_fee_config_rejects_fees_exceeding_ceiling() {
         let (_env, client, admins, _recovery) = setup();
-        
+
         // Sum exceeds BPS_DENOMINATOR
         let cfg = FeeConfig {
             platform_fee_bps: 5_000,
@@ -1189,7 +1198,7 @@ mod tests {
     #[should_panic(expected = "Error(Contract, #4)")]
     fn set_fee_config_rejects_individual_fee_exceeding_max() {
         let (_env, client, admins, _recovery) = setup();
-        
+
         // Individual fee exceeds MAX_FEE_BPS (governance trust root)
         let cfg = FeeConfig {
             platform_fee_bps: 5_001,
@@ -1436,7 +1445,8 @@ mod tests {
             let contract_id = env.register_contract(None, GovernanceContract);
             let client = GovernanceContractClient::new(&env, &contract_id);
 
-            let result = client.try_init(&admins, &threshold, &recovery);
+            let deployer = Address::generate(&env);
+            let result = client.try_init(&deployer, &admins, &threshold, &recovery);
             if threshold == 0 || threshold > admin_count {
                 prop_assert!(result.is_err());
             } else {
@@ -1464,8 +1474,13 @@ mod tests {
         let client = GovernanceContractClient::new(&env, &contract_id);
 
         assert!(!client.is_initialized());
-    let deployer = Address::generate(&env);
-        client.init(&deployer, &vec![env, admin.clone()], &1, &recovery_address);
+        let deployer = Address::generate(&env);
+        client.init(
+            &deployer,
+            &soroban_sdk::vec![&env, admin.clone()],
+            &1,
+            &recovery_address,
+        );
         assert!(client.is_initialized());
     }
 
@@ -1480,7 +1495,7 @@ mod tests {
         let client = GovernanceContractClient::new(&env, &contract_id);
 
         let admins = vec![&env, admin.clone()];
-    let deployer = Address::generate(&env);
+        let deployer = Address::generate(&env);
         client.init(&deployer, &admins, &1, &recovery_address);
         assert!(client.is_initialized());
         assert_eq!(client.get_admin(), admins);
@@ -1545,7 +1560,7 @@ mod tests {
 
         let contract_id = env.register_contract(None, GovernanceContract);
         let client = GovernanceContractClient::new(&env, &contract_id);
-    let deployer = Address::generate(&env);
+        let deployer = Address::generate(&env);
         client.init(&deployer, &admins, &1, &recovery);
 
         assert_eq!(client.get_threshold(), 1);
@@ -1569,7 +1584,7 @@ mod tests {
 
         let contract_id = env.register_contract(None, GovernanceContract);
         let client = GovernanceContractClient::new(&env, &contract_id);
-    let deployer = Address::generate(&env);
+        let deployer = Address::generate(&env);
         client.init(&deployer, &admins, &1, &recovery);
 
         // Current threshold is 1, needs 2 signatures for change_threshold, but only 1 provided.
@@ -1592,7 +1607,7 @@ mod tests {
 
         let contract_id = env.register_contract(None, GovernanceContract);
         let client = GovernanceContractClient::new(&env, &contract_id);
-    let deployer = Address::generate(&env);
+        let deployer = Address::generate(&env);
         client.init(&deployer, &admins, &1, &recovery);
 
         // Threshold 3 > admins.len() 2 — must fail with InvalidThreshold, not auth.
@@ -1613,7 +1628,7 @@ mod tests {
 
         let contract_id = env.register_contract(None, GovernanceContract);
         let client = GovernanceContractClient::new(&env, &contract_id);
-    let deployer = Address::generate(&env);
+        let deployer = Address::generate(&env);
         client.init(&deployer, &admins, &2, &recovery);
 
         client.change_threshold(&admins, &0);
@@ -1825,7 +1840,7 @@ mod tests {
         let recovery_address = Address::generate(&env);
         let contract_id = env.register_contract(None, GovernanceContract);
         let client = GovernanceContractClient::new(&env, &contract_id);
-    let deployer = Address::generate(&env);
+        let deployer = Address::generate(&env);
         client.init(&deployer, &admins, &1, &recovery_address);
 
         client.pause(&admins);

governance_contract/src/real_auth_tests.rs

@@ -17,7 +17,9 @@ fn fee_anchor_and_system_param_writes_require_real_authorization() {
     let key = Symbol::new(&env, "real_auth");
     env.mock_auths(&[]);
 
-    assert!(client.try_set_fee_config(&admins, &valid_fee_config()).is_err());
+    assert!(client
+        .try_set_fee_config(&admins, &valid_fee_config())
+        .is_err());
     assert!(client.try_upsert_anchor(&admins, &asset, &anchor).is_err());
     assert!(client.try_update_system_param(&admins, &key, &1).is_err());
 }
@@ -40,11 +42,7 @@ fn admin_transfer_and_threshold_change_require_real_authorization() {
     env.mock_auths(&[]);
 
     assert!(client
-        .try_transfer_admin(
-            &admins,
-            &soroban_sdk::vec![&env, replacement_admin],
-            &1,
-        )
+        .try_transfer_admin(&admins, &soroban_sdk::vec![&env, replacement_admin], &1,)
         .is_err());
 
     let env = Env::default();
@@ -56,7 +54,8 @@ fn admin_transfer_and_threshold_change_require_real_authorization() {
     let recovery = Address::generate(&env);
     let contract_id = env.register_contract(None, GovernanceContract);
     let client = GovernanceContractClient::new(&env, &contract_id);
-    client.init(&admins, &1, &recovery);
+    let deployer = Address::generate(&env);
+    client.init(&deployer, &admins, &1, &recovery);
     env.mock_auths(&[]);
 
     assert!(client.try_change_threshold(&admins, &2).is_err());
@@ -98,7 +97,8 @@ fn initialization_and_upgrade_require_real_authorization() {
     let client = GovernanceContractClient::new(&env, &contract_id);
     let admins = soroban_sdk::vec![&env, admin];
     env.mock_auths(&[]);
-    assert!(client.try_init(&admins, &1, &recovery).is_err());
+    let deployer = Address::generate(&env);
+    assert!(client.try_init(&deployer, &admins, &1, &recovery).is_err());
 
     let (env, client, admins) = super::setup();
     let wasm_hash = env

settlement_contract/src/admin.rs

@@ -11,9 +11,8 @@ use crate::errors::SettlementError;
 use crate::storage::{
     assert_not_paused, is_merchant_registered_and_bump_ttl, read_admin, read_admins,
     read_fallback_rule, read_governance, read_optional_primary_admin, read_pending_recovery,
-    read_recovery_address, read_rule_or_default, read_threshold,
-    validate_admins_and_threshold, validate_governance, validate_nonzero_address,
-    verify_admin_auth, write_admins,
+    read_recovery_address, read_rule_or_default, read_threshold, validate_admins_and_threshold,
+    validate_governance, validate_nonzero_address, verify_admin_auth, write_admins,
 };
 use crate::types::{DataKey, Operation, ScheduledOp, SettlementRule};
 use crate::{
@@ -389,14 +388,20 @@ impl SettlementContract {
         env.storage().persistent().remove(&key);
 
         match operation {
-            Operation::UpdateGovernance(new_gov) => Self::_update_governance(&env, &executor, new_gov),
+            Operation::UpdateGovernance(new_gov) => {
+                Self::_update_governance(&env, &executor, new_gov)
+            }
             Operation::CancelRecovery => Self::_cancel_recovery(&env, &executor),
             Operation::TransferAdmin(new_admins, new_threshold) => {
                 Self::_transfer_admin(&env, &executor, new_admins, new_threshold)
             }
             Operation::Upgrade(wasm_hash) => Self::_upgrade(&env, &executor, wasm_hash),
-            Operation::RegisterMerchant(merchant) => Self::_register_merchant(&env, &executor, merchant),
-            Operation::UnregisterMerchant(merchant) => Self::_unregister_merchant(&env, &executor, merchant),
+            Operation::RegisterMerchant(merchant) => {
+                Self::_register_merchant(&env, &executor, merchant)
+            }
+            Operation::UnregisterMerchant(merchant) => {
+                Self::_unregister_merchant(&env, &executor, merchant)
+            }
             Operation::SetSettlementRule(merchant, rule) => {
                 Self::_set_settlement_rule(&env, &executor, merchant, rule)
             }
@@ -474,7 +479,12 @@ impl SettlementContract {
         events::emit_recovery_cancelled(env, executor);
     }
 
-    fn _transfer_admin(env: &Env, executor: &Address, new_admins: Vec<Address>, new_threshold: u32) {
+    fn _transfer_admin(
+        env: &Env,
+        _executor: &Address,
+        new_admins: Vec<Address>,
+        new_threshold: u32,
+    ) {
         let old_admin = read_admin(env);
         validate_admins_and_threshold(env, &new_admins, new_threshold);
         // Enforce admin/merchant exclusivity in both directions (issue #692).
@@ -522,7 +532,7 @@ impl SettlementContract {
             SettlementError::EmptyAddress,
             SettlementError::ZeroAddress,
         );
-        
+
         // Prevent an admin from being registered as a merchant
         let admins = read_admins(env);
         for i in 0..admins.len() {
@@ -602,7 +612,12 @@ impl SettlementContract {
         );
     }
 
-    fn _set_settlement_rule(env: &Env, executor: &Address, merchant: Address, rule: SettlementRule) {
+    fn _set_settlement_rule(
+        env: &Env,
+        executor: &Address,
+        merchant: Address,
+        rule: SettlementRule,
+    ) {
         assert_not_paused(env);
 
         if !is_merchant_registered_and_bump_ttl(env, merchant.clone()) {