Merge pull request #110 from CosmWasm/larry/cw2-fix

Prevent repetitive write of cw2 version info during instantiation
public-awesome · Feb 18, 2023 · a662fac · a662fac
2 parents a9e4ca6 + f4d3f17
commit a662fac
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 20 deletions.
diff --git a/contracts/cw2981-royalties/src/lib.rs b/contracts/cw2981-royalties/src/lib.rs
@@ -5,7 +5,6 @@ pub use query::{check_royalties, query_royalties_info};
 
 use cosmwasm_schema::cw_serde;
 use cosmwasm_std::{to_binary, Empty};
-use cw2::set_contract_version;
 use cw721_base::Cw721Contract;
 pub use cw721_base::{ContractError, InstantiateMsg, MinterResponse};
 
@@ -65,12 +64,10 @@ pub mod entry {
         env: Env,
         info: MessageInfo,
         msg: InstantiateMsg,
-    ) -> Result<Response, ContractError> {
-        let res = Cw2981Contract::default().instantiate(deps.branch(), env, info, msg)?;
-        // Explicitly set contract name and version, otherwise set to cw721-base info
-        set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)
-            .map_err(ContractError::Std)?;
-        Ok(res)
+    ) -> StdResult<Response> {
+        cw2::set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
+
+        Cw2981Contract::default().instantiate(deps.branch(), env, info, msg)
     }
 
     #[entry_point]

diff --git a/contracts/cw721-base/src/execute.rs b/contracts/cw721-base/src/execute.rs
@@ -11,10 +11,7 @@ use crate::error::ContractError;
 use crate::msg::{ExecuteMsg, InstantiateMsg};
 use crate::state::{Approval, Cw721Contract, TokenInfo};
 use crate::upgrades;
-
-// Version info for migration
-const CONTRACT_NAME: &str = "crates.io:cw721-base";
-const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION");
+use crate::{CONTRACT_NAME, CONTRACT_VERSION};
 
 impl<'a, T, C, E, Q> Cw721Contract<'a, T, C, E, Q>
 where
@@ -30,14 +27,14 @@ where
         _info: MessageInfo,
         msg: InstantiateMsg,
     ) -> StdResult<Response<C>> {
-        set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
-
         let info = ContractInfoResponse {
             name: msg.name,
             symbol: msg.symbol,
         };
         self.contract_info.save(deps.storage, &info)?;
+
         cw_ownable::initialize_owner(deps.storage, deps.api, Some(&msg.minter))?;
+
         Ok(Response::default())
     }
 

diff --git a/contracts/cw721-base/src/lib.rs b/contracts/cw721-base/src/lib.rs
@@ -28,6 +28,10 @@ use cosmwasm_std::Empty;
 // This is a simple type to let us handle empty extensions
 pub type Extension = Option<Empty>;
 
+// Version info for migration
+pub const CONTRACT_NAME: &str = "crates.io:cw721-base";
+pub const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION");
+
 pub mod entry {
     use super::*;
 
@@ -43,6 +47,8 @@ pub mod entry {
         info: MessageInfo,
         msg: InstantiateMsg,
     ) -> StdResult<Response> {
+        cw2::set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
+
         let tract = Cw721Contract::<Extension, Empty, Empty, Empty>::default();
         tract.instantiate(deps, env, info, msg)
     }
@@ -69,3 +75,38 @@ pub mod entry {
         Cw721Contract::<Extension, Empty, Empty, Empty>::migrate(deps, env)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use cosmwasm_std::testing::{mock_dependencies, mock_env, mock_info};
+    use cw2::ContractVersion;
+
+    use super::*;
+
+    /// Make sure cw2 version info is properly initialized during instantiation.
+    #[test]
+    fn proper_cw2_initialization() {
+        let mut deps = mock_dependencies();
+
+        entry::instantiate(
+            deps.as_mut(),
+            mock_env(),
+            mock_info("larry", &[]),
+            InstantiateMsg {
+                name: "".into(),
+                symbol: "".into(),
+                minter: "larry".into(),
+            },
+        )
+        .unwrap();
+
+        let version = cw2::get_contract_version(deps.as_ref().storage).unwrap();
+        assert_eq!(
+            version,
+            ContractVersion {
+                contract: CONTRACT_NAME.into(),
+                version: CONTRACT_VERSION.into(),
+            },
+        );
+    }
+}
diff --git a/contracts/cw721-metadata-onchain/src/lib.rs b/contracts/cw721-metadata-onchain/src/lib.rs
@@ -1,6 +1,5 @@
 use cosmwasm_schema::cw_serde;
 use cosmwasm_std::Empty;
-use cw2::set_contract_version;
 pub use cw721_base::{ContractError, InstantiateMsg, MinterResponse};
 
 // Version info for migration
@@ -49,12 +48,10 @@ pub mod entry {
         env: Env,
         info: MessageInfo,
         msg: InstantiateMsg,
-    ) -> Result<Response, ContractError> {
-        let res = Cw721MetadataContract::default().instantiate(deps.branch(), env, info, msg)?;
-        // Explicitly set contract name and version, otherwise set to cw721-base info
-        set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)
-            .map_err(ContractError::Std)?;
-        Ok(res)
+    ) -> StdResult<Response> {
+        cw2::set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
+
+        Cw721MetadataContract::default().instantiate(deps.branch(), env, info, msg)
     }
 
     #[entry_point]
@@ -82,6 +79,29 @@ mod tests {
 
     const CREATOR: &str = "creator";
 
+    /// Make sure cw2 version info is properly initialized during instantiation,
+    /// and NOT overwritten by the base contract.
+    #[test]
+    fn proper_cw2_initialization() {
+        let mut deps = mock_dependencies();
+
+        entry::instantiate(
+            deps.as_mut(),
+            mock_env(),
+            mock_info("larry", &[]),
+            InstantiateMsg {
+                name: "".into(),
+                symbol: "".into(),
+                minter: "larry".into(),
+            },
+        )
+        .unwrap();
+
+        let version = cw2::get_contract_version(deps.as_ref().storage).unwrap();
+        assert_eq!(version.contract, CONTRACT_NAME);
+        assert_ne!(version.contract, cw721_base::CONTRACT_NAME);
+    }
+
     #[test]
     fn use_metadata_extension() {
         let mut deps = mock_dependencies();