refactor: move interface storage into mod.rs

packages/axelar-soroban-std/src/interfaces/mod.rs

@@ -7,3 +7,31 @@ mod upgradable;
 pub use operatable::*;
 pub use ownable::*;
 pub use upgradable::*;
+
+/// This submodule encapsulates data keys for the separate interfaces. These keys break naming conventions on purpose.
+/// If a contract implements a contract type that would result in a collision with a key defined here,
+/// the linter will complain about it. So as long as contracts follow regular naming conventions,
+/// there is no risk of collisions.
+mod storage {
+    #![allow(non_camel_case_types)]
+
+    use soroban_sdk::contracttype;
+
+    // add a separate data key type for each interface. Using a single enum could lead to unintentionally breaks
+    // of unrelated interfaces, because the key serialization is variant order dependent.
+
+    #[contracttype]
+    pub enum OperatorDataKey {
+        Interfaces_Operator,
+    }
+
+    #[contracttype]
+    pub enum OwnerDataKey {
+        Interfaces_Owner,
+    }
+
+    #[contracttype]
+    pub enum MigratingDataKey {
+        Interfaces_Migrating,
+    }
+}

packages/axelar-soroban-std/src/interfaces/operatable.rs

@@ -1,6 +1,7 @@
 use crate::events::Event;
 #[cfg(any(test, feature = "testutils"))]
 use crate::impl_event_testutils;
+use crate::interfaces::storage;
 use core::fmt::Debug;
 use soroban_sdk::{contractclient, Address, Env, IntoVal, Symbol, Topics, Val, Vec};
 
@@ -17,7 +18,7 @@ pub trait OperatableInterface {
 pub fn operator(env: &Env) -> Address {
     env.storage()
         .instance()
-        .get(&storage::DataKey::Interfaces_Operator)
+        .get(&storage::OperatorDataKey::Interfaces_Operator)
         .expect("operator must be set during contract construction")
 }
 
@@ -40,7 +41,7 @@ pub fn transfer_operatorship<T: OperatableInterface>(env: &Env, new_operator: Ad
 pub fn set_operator(env: &Env, operator: &Address) {
     env.storage()
         .instance()
-        .set(&storage::DataKey::Interfaces_Operator, operator);
+        .set(&storage::OperatorDataKey::Interfaces_Operator, operator);
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -66,23 +67,6 @@ impl Event for OperatorshipTransferredEvent {
 #[cfg(any(test, feature = "testutils"))]
 impl_event_testutils!(OperatorshipTransferredEvent, (Symbol, Address, Address), ());
 
-// submodule to encapsulate the disabled linting
-mod storage {
-    // linting is disabled for the enum variant names on purpose, so we can define names that would otherwise be invalid.
-    // This way, if a contract that implements a shared interface defines a variant with the same name, the linter will
-    // complain about it.
-    #![allow(non_camel_case_types)]
-
-    use soroban_sdk::contracttype;
-
-    #[contracttype]
-    /// Variants do not follow the naming convention of other variants to let the linter help to avoid
-    /// collisions with contract types defined in other contracts that implement a shared interface.
-    pub enum DataKey {
-        Interfaces_Operator,
-    }
-}
-
 #[cfg(test)]
 mod test {
     use crate::interfaces::testdata::Contract;

packages/axelar-soroban-std/src/interfaces/ownable.rs

@@ -1,6 +1,7 @@
 use crate::events::Event;
 #[cfg(any(test, feature = "testutils"))]
 use crate::impl_event_testutils;
+use crate::interfaces::storage;
 use core::fmt::Debug;
 use soroban_sdk::{contractclient, Address, Env, IntoVal, Symbol, Topics, Val, Vec};
 
@@ -17,7 +18,7 @@ pub trait OwnableInterface {
 pub fn owner(env: &Env) -> Address {
     env.storage()
         .instance()
-        .get(&storage::DataKey::Interfaces_Owner)
+        .get(&storage::OwnerDataKey::Interfaces_Owner)
         .expect("owner must be set during contract construction")
 }
 
@@ -40,7 +41,7 @@ pub fn transfer_ownership<T: OwnableInterface>(env: &Env, new_owner: Address) {
 pub fn set_owner(env: &Env, owner: &Address) {
     env.storage()
         .instance()
-        .set(&storage::DataKey::Interfaces_Owner, owner);
+        .set(&storage::OwnerDataKey::Interfaces_Owner, owner);
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -66,23 +67,6 @@ impl Event for OwnershipTransferredEvent {
 #[cfg(any(test, feature = "testutils"))]
 impl_event_testutils!(OwnershipTransferredEvent, (Symbol, Address, Address), ());
 
-// submodule to encapsulate the disabled linting
-mod storage {
-    // linting is disabled for the enum variant names on purpose, so we can define names that would otherwise be invalid.
-    // This way, if a contract that implements a shared interface defines a variant with the same name, the linter will
-    // complain about it.
-    #![allow(non_camel_case_types)]
-
-    use soroban_sdk::contracttype;
-
-    #[contracttype]
-    /// Variants do not follow the naming convention of other variants to let the linter help to avoid
-    /// collisions with contract types defined in other contracts that implement a shared interface.
-    pub enum DataKey {
-        Interfaces_Owner,
-    }
-}
-
 #[cfg(test)]
 mod test {
     use crate::interfaces::testdata::Contract;
@@ -91,6 +75,12 @@ mod test {
     use soroban_sdk::testutils::Address as _;
     use soroban_sdk::{Address, Env};
 
+    fn prepare_client(env: &Env, owner: Option<Address>) -> OwnableClient {
+        let operator = Address::generate(env);
+        let contract_id = env.register(Contract, (owner, operator));
+        OwnableClient::new(env, &contract_id)
+    }
+
     #[test]
     fn owner_fails_if_owner_not_set() {
         let env = Env::default();
@@ -135,10 +125,4 @@ mod test {
 
         assert_eq!(client.owner(), new_owner);
     }
-
-    fn prepare_client(env: &Env, owner: Option<Address>) -> OwnableClient {
-        let operator = Address::generate(env);
-        let contract_id = env.register(Contract, (owner, operator));
-        OwnableClient::new(env, &contract_id)
-    }
 }

packages/axelar-soroban-std/src/interfaces/upgradable.rs

@@ -2,7 +2,7 @@ use crate::ensure;
 use crate::events::Event;
 #[cfg(any(test, feature = "testutils"))]
 use crate::impl_event_testutils;
-use crate::interfaces::OwnableInterface;
+use crate::interfaces::{storage, OwnableInterface};
 use core::fmt::Debug;
 use soroban_sdk::{
     contractclient, symbol_short, BytesN, Env, FromVal, IntoVal, String, Topics, Val,
@@ -62,14 +62,14 @@ pub fn migrate<T: UpgradableInterface>(
 fn start_migration(env: &Env) {
     env.storage()
         .instance()
-        .set(&storage::DataKey::Interfaces_Migrating, &());
+        .set(&storage::MigratingDataKey::Interfaces_Migrating, &());
 }
 
 fn ensure_is_migrating(env: &Env) -> Result<(), MigrationError> {
     ensure!(
         env.storage()
             .instance()
-            .has(&storage::DataKey::Interfaces_Migrating),
+            .has(&storage::MigratingDataKey::Interfaces_Migrating),
         MigrationError::NotAllowed
     );
 
@@ -79,7 +79,7 @@ fn ensure_is_migrating(env: &Env) -> Result<(), MigrationError> {
 fn complete_migration(env: &Env) {
     env.storage()
         .instance()
-        .remove(&storage::DataKey::Interfaces_Migrating);
+        .remove(&storage::MigratingDataKey::Interfaces_Migrating);
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -100,23 +100,6 @@ impl Event for UpgradedEvent {
 #[cfg(any(test, feature = "testutils"))]
 impl_event_testutils!(UpgradedEvent, (soroban_sdk::Symbol), (String));
 
-// submodule to encapsulate the disabled linting
-mod storage {
-    // linting is disabled for the enum variant names on purpose, so we can define names that would otherwise be invalid.
-    // This way, if a contract that implements a shared interface defines a variant with the same name, the linter will
-    // complain about it.
-    #![allow(non_camel_case_types)]
-
-    use soroban_sdk::contracttype;
-
-    #[contracttype]
-    /// Variants do not follow the naming convention of other variants to let the linter help to avoid
-    /// collisions with contract types defined in other contracts that implement a shared interface.
-    pub enum DataKey {
-        Interfaces_Migrating,
-    }
-}
-
 pub enum MigrationError {
     NotAllowed,
 }