#feat DRP-470 Fix - lack of code ID validation and conguration enforcement in the factory

contracts/factory/src/contract.rs

@@ -28,7 +28,7 @@ use drop_staking_base::msg::{
     withdrawal_manager::InstantiateMsg as WithdrawalManagerInstantiateMsg,
     withdrawal_voucher::InstantiateMsg as WithdrawalVoucherInstantiateMsg,
 };
-use drop_staking_base::state::factory::{PreInstantiatedContracts, STATE};
+use drop_staking_base::state::factory::{CodeIds, PreInstantiatedContracts, STATE};
 use drop_staking_base::state::splitter::Config as SplitterConfig;
 use neutron_sdk::{
     bindings::{msg::NeutronMsg, query::NeutronQuery},
@@ -49,7 +49,12 @@ pub fn instantiate(
     cw2::set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
     cw_ownable::initialize_owner(deps.storage, deps.api, Some(info.sender.as_str()))?;
 
-    validate_pre_instantiated_contracts(deps.as_ref(), &env, &msg.pre_instantiated_contracts)?;
+    validate_pre_instantiated_contracts(
+        deps.as_ref(),
+        &env,
+        &msg.pre_instantiated_contracts,
+        &msg.code_ids,
+    )?;
 
     let attrs = vec![
         attr("base_denom", &msg.base_denom),
@@ -516,6 +521,7 @@ pub fn validate_contract_metadata(
     env: &Env,
     contract_addr: &Addr,
     valid_names: &[&str],
+    code_id: u64,
 ) -> ContractResult<()> {
     let contract_version = get_contract_version(deps, contract_addr)?;
 
@@ -553,13 +559,22 @@ pub fn validate_contract_metadata(
         });
     }
 
+    if contract_info.code_id != code_id {
+        return Err(ContractError::InvalidContractCodeId {
+            contract: contract_addr.to_string(),
+            expected: code_id.to_string(),
+            actual: contract_info.code_id.to_string(),
+        });
+    }
+
     Ok(())
 }
 
 fn validate_pre_instantiated_contracts(
     deps: Deps,
     env: &Env,
     pre_instantiated_contracts: &PreInstantiatedContracts,
+    code_ids: &CodeIds,
 ) -> Result<(), ContractError> {
     // Validate native bond provider contract
     validate_contract_metadata(
@@ -570,6 +585,7 @@ fn validate_pre_instantiated_contracts(
             drop_native_bond_provider::contract::CONTRACT_NAME,
             drop_native_sync_bond_provider::contract::CONTRACT_NAME,
         ],
+        code_ids.native_bond_provider_code_id,
     )?;
 
     // Validate val ref address
@@ -579,6 +595,7 @@ fn validate_pre_instantiated_contracts(
             env,
             val_ref_address,
             &[drop_val_ref::contract::CONTRACT_NAME],
+            code_ids.val_ref_code_id.unwrap_or_default(),
         )?;
     }
 
@@ -591,6 +608,7 @@ fn validate_pre_instantiated_contracts(
             env,
             lsm_share_bond_provider_address,
             &[drop_lsm_share_bond_provider::contract::CONTRACT_NAME],
+            code_ids.lsm_share_bond_provider_code_id.unwrap_or_default(),
         )?;
     }
 
@@ -604,6 +622,7 @@ fn validate_pre_instantiated_contracts(
             drop_puppeteer_initia::contract::CONTRACT_NAME,
             drop_puppeteer_native::contract::CONTRACT_NAME,
         ],
+        code_ids.puppeteer_code_id,
     )?;
 
     // Validate unbonding and rewards pump contracts
@@ -613,6 +632,7 @@ fn validate_pre_instantiated_contracts(
             env,
             unbonding_pump_address,
             &[drop_pump::contract::CONTRACT_NAME],
+            code_ids.unbonding_pump_code_id.unwrap_or_default(),
         )?;
     }
     if let Some(rewards_pump_address) = &pre_instantiated_contracts.rewards_pump_address {
@@ -621,6 +641,7 @@ fn validate_pre_instantiated_contracts(
             env,
             rewards_pump_address,
             &[drop_pump::contract::CONTRACT_NAME],
+            code_ids.rewards_pump_code_id.unwrap_or_default(),
         )?;
     }
 

contracts/factory/src/tests.rs

@@ -177,13 +177,15 @@ fn test_instantiate() {
         "crates.io:drop-staking__drop-native-bond-provider".to_string(),
         mocked_env.contract.address.to_string(),
         Some(contract_admin.clone()),
+        13,
     );
     setup_contract_metadata(
         &mut deps.querier,
         "puppeteer_address",
         "crates.io:drop-staking__drop-puppeteer".to_string(),
         mocked_env.contract.address.to_string(),
         Some(contract_admin),
+        14,
     );
 
     let instantiate_msg = InstantiateMsg {
@@ -197,6 +199,12 @@ fn test_instantiate() {
             distribution_code_id: 9,
             rewards_manager_code_id: 10,
             splitter_code_id: 12,
+            native_bond_provider_code_id: 13,
+            puppeteer_code_id: 14,
+            val_ref_code_id: Some(15),
+            lsm_share_bond_provider_code_id: Some(16),
+            unbonding_pump_code_id: Some(17),
+            rewards_pump_code_id: Some(18),
         },
         pre_instantiated_contracts: PreInstantiatedContracts {
             native_bond_provider_address: cosmwasm_std::Addr::unchecked(
@@ -1065,13 +1073,15 @@ fn test_validate_contract_metadata() {
         "contract_name".to_string(),
         mocked_env.contract.address.to_string(),
         Some(contract_admin),
+        161,
     );
 
     validate_contract_metadata(
         deps.as_ref().into_empty(),
         &mocked_env,
         &Addr::unchecked(contract_addr),
         &["contract_name"],
+        161,
     )
     .unwrap();
 }
@@ -1091,13 +1101,15 @@ fn test_validate_contract_metadata_two_names() {
         "contract_name".to_string(),
         mocked_env.contract.address.to_string(),
         Some(contract_admin),
+        161,
     );
 
     validate_contract_metadata(
         deps.as_ref().into_empty(),
         &mocked_env,
         &Addr::unchecked(contract_addr),
         &["another_valid_name", "contract_name"],
+        161,
     )
     .unwrap();
 }
@@ -1109,21 +1121,21 @@ fn test_validate_contract_metadata_wrong_contract_name() {
     let contract_addr = "cosmos2contract";
     let mocked_env = mock_env();
 
-    // let contract_admin = mocked_env.contract.address.to_string();
-
     setup_contract_metadata(
         &mut deps.querier,
         contract_addr,
         "wrong_name".to_string(),
         mocked_env.contract.address.to_string(),
         None,
+        161,
     );
 
     let error = validate_contract_metadata(
         deps.as_ref().into_empty(),
         &mocked_env,
         &Addr::unchecked(contract_addr),
         &["contract_name"],
+        161,
     )
     .unwrap_err();
     assert_eq!(
@@ -1148,13 +1160,15 @@ fn test_validate_contract_metadata_wrong_owner() {
         "contract_name".to_string(),
         "wrong_owner_address".to_string(),
         None,
+        161,
     );
 
     let error = validate_contract_metadata(
         deps.as_ref().into_empty(),
         &mocked_env,
         &Addr::unchecked(contract_addr),
         &["contract_name"],
+        161,
     )
     .unwrap_err();
     assert_eq!(
@@ -1180,13 +1194,15 @@ fn test_validate_contract_metadata_wrong_admin() {
         "contract_name".to_string(),
         mocked_env.contract.address.to_string(),
         Some("wrong_contract_admin".to_string()),
+        161,
     );
 
     let error = validate_contract_metadata(
         deps.as_ref().into_empty(),
         &mocked_env,
         &Addr::unchecked(contract_addr),
         &["contract_name"],
+        161,
     )
     .unwrap_err();
     assert_eq!(
@@ -1212,13 +1228,15 @@ fn test_validate_contract_metadata_empty_admin() {
         "contract_name".to_string(),
         mocked_env.contract.address.to_string(),
         None,
+        161,
     );
 
     let error = validate_contract_metadata(
         deps.as_ref().into_empty(),
         &mocked_env,
         &Addr::unchecked(contract_addr),
         &["contract_name"],
+        161,
     )
     .unwrap_err();
     assert_eq!(
@@ -1231,12 +1249,49 @@ fn test_validate_contract_metadata_empty_admin() {
     );
 }
 
+#[test]
+fn test_validate_contract_metadata_wrong_code_id() {
+    let mut deps = mock_dependencies(&[]);
+
+    let contract_addr = "cosmos2contract";
+    let mocked_env = mock_env();
+
+    let contract_admin = mocked_env.contract.address.to_string();
+
+    setup_contract_metadata(
+        &mut deps.querier,
+        contract_addr,
+        "contract_name".to_string(),
+        mocked_env.contract.address.to_string(),
+        Some(contract_admin),
+        161,
+    );
+
+    let error = validate_contract_metadata(
+        deps.as_ref().into_empty(),
+        &mocked_env,
+        &Addr::unchecked(contract_addr),
+        &["contract_name"],
+        123,
+    )
+    .unwrap_err();
+    assert_eq!(
+        error,
+        drop_staking_base::error::factory::ContractError::InvalidContractCodeId {
+            contract: String::from(contract_addr),
+            expected: "123".to_string(),
+            actual: "161".to_string()
+        }
+    );
+}
+
 fn setup_contract_metadata(
     querier: &mut WasmMockQuerier,
     contract_addr: &str,
     contract_name: String,
     factory_address: String,
     contract_admin: Option<String>,
+    contract_code_id: u64,
 ) {
     querier.add_wasm_query_response(contract_addr, move |_| {
         cosmwasm_std::ContractResult::Ok(
@@ -1262,6 +1317,7 @@ fn setup_contract_metadata(
     querier.add_wasm_query_response(contract_addr, move |_| {
         let mut response = cosmwasm_std::ContractInfoResponse::default();
         response.admin = contract_admin.clone();
+        response.code_id = contract_code_id;
 
         cosmwasm_std::ContractResult::Ok(to_json_binary(&response).unwrap())
     });

integration_tests/src/testcases/auto-withdrawer.test.ts

@@ -662,6 +662,10 @@ describe('Auto withdrawer', () => {
           validators_set_code_id: context.codeIds.validatorsSet,
           rewards_manager_code_id: context.codeIds.rewardsManager,
           splitter_code_id: context.codeIds.splitter,
+          native_bond_provider_code_id: context.codeIds.nativeBondProvider,
+          puppeteer_code_id: context.codeIds.puppeteer,
+          unbonding_pump_code_id: context.codeIds.pump,
+          rewards_pump_code_id: context.codeIds.pump,
         },
         pre_instantiated_contracts: {
           native_bond_provider_address:

integration_tests/src/testcases/bond-mirror.test.ts

@@ -676,6 +676,11 @@ describe('Mirror', () => {
           validators_set_code_id: context.codeIds.validatorsSet,
           rewards_manager_code_id: context.codeIds.rewardsManager,
           splitter_code_id: context.codeIds.splitter,
+          native_bond_provider_code_id: context.codeIds.nativeBondProvider,
+          puppeteer_code_id: context.codeIds.puppeteer,
+          lsm_share_bond_provider_code_id: context.codeIds.lsmShareBondProvider,
+          unbonding_pump_code_id: context.codeIds.pump,
+          rewards_pump_code_id: context.codeIds.pump,
         },
         pre_instantiated_contracts: {
           native_bond_provider_address:

integration_tests/src/testcases/core-native-long-idle.test.ts

@@ -542,6 +542,8 @@ describe('Core', () => {
           validators_set_code_id: context.codeIds.validatorsSet,
           rewards_manager_code_id: context.codeIds.rewardsManager,
           splitter_code_id: context.codeIds.splitter,
+          native_bond_provider_code_id: context.codeIds.nativeBondProvider,
+          puppeteer_code_id: context.codeIds.puppeteer,
         },
         pre_instantiated_contracts: {
           native_bond_provider_address:

integration_tests/src/testcases/core-native.test.ts

@@ -585,6 +585,8 @@ describe('Core', () => {
             validators_set_code_id: context.codeIds.validatorsSet,
             rewards_manager_code_id: context.codeIds.rewardsManager,
             splitter_code_id: context.codeIds.splitter,
+            native_bond_provider_code_id: context.codeIds.nativeBondProvider,
+            puppeteer_code_id: context.codeIds.puppeteer,
           },
           pre_instantiated_contracts: {
             native_bond_provider_address:

integration_tests/src/testcases/core-slashing.test.ts

@@ -690,6 +690,10 @@ describe('Core Slashing', () => {
           validators_set_code_id: context.codeIds.validatorsSet,
           rewards_manager_code_id: context.codeIds.rewardsManager,
           splitter_code_id: context.codeIds.splitter,
+          native_bond_provider_code_id: context.codeIds.nativeBondProvider,
+          puppeteer_code_id: context.codeIds.puppeteer,
+          unbonding_pump_code_id: context.codeIds.pump,
+          rewards_pump_code_id: context.codeIds.pump,
         },
         pre_instantiated_contracts: {
           native_bond_provider_address:

integration_tests/src/testcases/core.test.ts

@@ -757,6 +757,11 @@ describe('Core', () => {
           validators_set_code_id: context.codeIds.validatorsSet,
           rewards_manager_code_id: context.codeIds.rewardsManager,
           splitter_code_id: context.codeIds.splitter,
+          native_bond_provider_code_id: context.codeIds.nativeBondProvider,
+          puppeteer_code_id: context.codeIds.puppeteer,
+          lsm_share_bond_provider_code_id: context.codeIds.lsmShareBondProvider,
+          unbonding_pump_code_id: context.codeIds.pump,
+          rewards_pump_code_id: context.codeIds.pump,
         },
         pre_instantiated_contracts: {
           native_bond_provider_address:

integration_tests/src/testcases/initia.test.ts

@@ -845,6 +845,11 @@ describe('Core', () => {
           validators_set_code_id: context.codeIds.validatorsSet,
           withdrawal_manager_code_id: context.codeIds.withdrawalManager,
           withdrawal_voucher_code_id: context.codeIds.withdrawalVoucher,
+          native_bond_provider_code_id: context.codeIds.nativeBondProvider,
+          puppeteer_code_id: context.codeIds.puppeteer,
+          lsm_share_bond_provider_code_id: context.codeIds.lsmShareBondProvider,
+          unbonding_pump_code_id: context.codeIds.pump,
+          rewards_pump_code_id: context.codeIds.pump,
         },
         core_params: {
           icq_update_delay: 5,

integration_tests/src/testcases/unbonding-mirror.test.ts

@@ -674,6 +674,11 @@ describe('Unbonding mirror', () => {
           validators_set_code_id: context.codeIds.validatorsSet,
           rewards_manager_code_id: context.codeIds.rewardsManager,
           splitter_code_id: context.codeIds.splitter,
+          native_bond_provider_code_id: context.codeIds.nativeBondProvider,
+          puppeteer_code_id: context.codeIds.puppeteer,
+          lsm_share_bond_provider_code_id: context.codeIds.lsmShareBondProvider,
+          unbonding_pump_code_id: context.codeIds.pump,
+          rewards_pump_code_id: context.codeIds.pump,
         },
         pre_instantiated_contracts: {
           native_bond_provider_address:

packages/base/src/error/factory.rs

@@ -42,6 +42,12 @@ pub enum ContractError {
         expected: String,
         actual: String,
     },
+    #[error("Invalid contract code id for {contract}. Expected: {expected}, actual: {actual}")]
+    InvalidContractCodeId {
+        contract: String,
+        expected: String,
+        actual: String,
+    },
     #[error("Contract address not found: {name}")]
     ContractAddressNotFound { name: String },