Skip to content

Commit

Permalink
Merge pull request #44 from AbstractSDK/misha/abs-104-standalone-modu…
Browse files Browse the repository at this point in the history
…les-shouldnt-check-cw2-in-version-control

Fix/standalone modules
  • Loading branch information
CyberHoward committed Aug 25, 2023
2 parents 27cabfb + 020e8bb commit eabd4f4
Show file tree
Hide file tree
Showing 9 changed files with 239 additions and 20 deletions.
55 changes: 55 additions & 0 deletions framework/contracts/account/manager/tests/common/mock_modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,58 @@ pub mod app_1 {
);
}
}

// this standalone have cw2
pub mod standalone_cw2 {
pub use super::*;
pub const MOCK_STANDALONE_ID: &str = "crate.io:mock-standalone1";

#[cosmwasm_schema::cw_serde]
pub struct MockMsg;

pub fn mock_instantiate(
deps: cosmwasm_std::DepsMut,
env: cosmwasm_std::Env,
info: cosmwasm_std::MessageInfo,
msg: MockInitMsg,
) -> cosmwasm_std::StdResult<cosmwasm_std::Response> {
cw2::set_contract_version(deps.storage, MOCK_STANDALONE_ID, V1);
Ok(cosmwasm_std::Response::new())
}

/// Execute entrypoint
pub fn mock_execute(
deps: cosmwasm_std::DepsMut,
env: cosmwasm_std::Env,
info: cosmwasm_std::MessageInfo,
msg: MockMsg,
) -> cosmwasm_std::StdResult<cosmwasm_std::Response> {
Ok(cosmwasm_std::Response::new())
}

/// Query entrypoint
pub fn mock_query(
deps: cosmwasm_std::Deps,
env: cosmwasm_std::Env,
msg: MockMsg,
) -> cosmwasm_std::StdResult<cosmwasm_std::Binary> {
Ok(cosmwasm_std::Binary::default())
}
}

// this standalone does not have cw2
pub mod standalone_no_cw2 {
pub use super::*;
pub const MOCK_STANDALONE_ID: &str = "crates.io:mock-standalone2";

pub use super::standalone_cw2::{mock_execute, mock_query, MockMsg};

pub fn mock_instantiate(
deps: cosmwasm_std::DepsMut,
env: cosmwasm_std::Env,
info: cosmwasm_std::MessageInfo,
msg: MockInitMsg,
) -> cosmwasm_std::StdResult<cosmwasm_std::Response> {
Ok(cosmwasm_std::Response::new())
}
}
111 changes: 110 additions & 1 deletion framework/contracts/account/manager/tests/proxy.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
mod common;
use abstract_adapter::mock::MockExecMsg;
use abstract_core::adapter::AdapterRequestMsg;
use abstract_core::objects::module::{ModuleInfo, ModuleVersion};
use abstract_core::objects::module_reference::ModuleReference;
use abstract_core::objects::namespace::Namespace;
use abstract_core::{manager::ManagerModuleInfo, PROXY};
use abstract_interface::*;
use abstract_manager::contract::CONTRACT_VERSION;
use abstract_testing::prelude::{TEST_ACCOUNT_ID, TEST_MODULE_ID};
use common::{create_default_account, init_mock_adapter, install_adapter, AResult, TEST_COIN};
use common::{
create_default_account, init_mock_adapter, install_adapter, mock_modules, AResult, TEST_COIN,
};
use cosmwasm_std::{wasm_execute, Addr, Coin, CosmosMsg};
use cw_orch::deploy::Deploy;
use cw_orch::prelude::*;
use module_factory::error::ModuleFactoryError;
use speculoos::prelude::*;

#[test]
Expand Down Expand Up @@ -162,3 +168,106 @@ fn with_response_data() -> AResult {

Ok(())
}

#[test]
fn install_standalone_modules() -> AResult {
let sender = Addr::unchecked(common::OWNER);
let chain = Mock::new(&sender);
let deployment = Abstract::deploy_on(chain.clone(), sender.to_string())?;
let account = AbstractAccount::new(&deployment, Some(0));

let standalone1_contract = Box::new(ContractWrapper::new(
mock_modules::standalone_cw2::mock_execute,
mock_modules::standalone_cw2::mock_instantiate,
mock_modules::standalone_cw2::mock_query,
));
let standalone1_id = chain.app.borrow_mut().store_code(standalone1_contract);

let standalone2_contract = Box::new(ContractWrapper::new(
mock_modules::standalone_no_cw2::mock_execute,
mock_modules::standalone_no_cw2::mock_instantiate,
mock_modules::standalone_no_cw2::mock_query,
));
let standalone2_id = chain.app.borrow_mut().store_code(standalone2_contract);

// install first standalone
deployment.version_control.propose_modules(vec![(
ModuleInfo {
namespace: Namespace::new("abstract")?,
name: "standalone1".to_owned(),
version: ModuleVersion::Version(mock_modules::V1.to_owned()),
},
ModuleReference::Standalone(standalone1_id),
)])?;

account.install_module(
"abstract:standalone1",
&mock_modules::standalone_cw2::MockMsg,
None,
)?;

// install second standalone
deployment.version_control.propose_modules(vec![(
ModuleInfo {
namespace: Namespace::new("abstract")?,
name: "standalone2".to_owned(),
version: ModuleVersion::Version(mock_modules::V1.to_owned()),
},
ModuleReference::Standalone(standalone2_id),
)])?;

account.install_module(
"abstract:standalone2",
&mock_modules::standalone_no_cw2::MockMsg,
None,
)?;
Ok(())
}

#[test]
fn install_standalone_versions_not_met() -> AResult {
let sender = Addr::unchecked(common::OWNER);
let chain = Mock::new(&sender);
let deployment = Abstract::deploy_on(chain.clone(), sender.to_string())?;
let account = AbstractAccount::new(&deployment, Some(0));

let standalone1_contract = Box::new(ContractWrapper::new(
mock_modules::standalone_cw2::mock_execute,
mock_modules::standalone_cw2::mock_instantiate,
mock_modules::standalone_cw2::mock_query,
));
let standalone1_id = chain.app.borrow_mut().store_code(standalone1_contract);

// install first standalone
deployment.version_control.propose_modules(vec![(
ModuleInfo {
namespace: Namespace::new("abstract")?,
name: "standalone1".to_owned(),
version: ModuleVersion::Version(mock_modules::V2.to_owned()),
},
ModuleReference::Standalone(standalone1_id),
)])?;

let err = account
.install_module(
"abstract:standalone1",
&mock_modules::standalone_cw2::MockMsg,
None,
)
.unwrap_err();

if let AbstractInterfaceError::Orch(err) = err {
let err: ModuleFactoryError = err.downcast()?;
assert_eq!(
err,
ModuleFactoryError::Abstract(abstract_core::AbstractError::UnequalModuleData {
cw2: mock_modules::V1.to_owned(),
module: mock_modules::V2.to_owned(),
})
);
} else {
panic!("wrong error type")
};

Ok(())
}
2 changes: 1 addition & 1 deletion framework/contracts/native/module-factory/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
mod commands;
pub mod contract;
mod error;
pub mod error;
mod querier;
mod response;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl<
self.base_state.save(deps.storage, &state)?;

let Some(handler) = self.maybe_instantiate_handler() else {
return Ok(Response::new())
return Ok(Response::new());
};
handler(deps, env, info, self, msg.module)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl<
self.admin.set(deps.branch(), Some(account_base.manager))?;

let Some(handler) = self.maybe_instantiate_handler() else {
return Ok(Response::new())
return Ok(Response::new());
};
handler(deps, env, info, self, msg.module)
}
Expand Down
53 changes: 46 additions & 7 deletions framework/packages/abstract-core/src/objects/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,14 +364,34 @@ pub fn assert_module_data_validity(
let Some(addr) = module_address else {
// if no addr provided and module doesn't have it, just return
// this will be the case when registering a code-id on VC
return Ok(())
return Ok(());
};
addr
}
};

let ModuleVersion::Version(version) = &module_claim.info.version else {
panic!("Module version is not versioned, context setting is wrong")
};

// verify that the contract's data is equal to its registered data
let cw_2_data = cw2::CONTRACT.query(querier, module_address.clone())?;
let cw_2_data_res = cw2::CONTRACT.query(querier, module_address.clone());

// For standalone we only check the version if cw2 exists
if let ModuleReference::Standalone(_) = module_claim.reference {
if let Ok(cw_2_data) = cw_2_data_res {
ensure_eq!(
version,
&cw_2_data.version,
AbstractError::UnequalModuleData {
cw2: cw_2_data.version,
module: version.to_owned()
}
);
}
return Ok(());
}
let cw_2_data = cw_2_data_res?;

// Assert that the contract name is equal to the module name
ensure_eq!(
Expand All @@ -383,10 +403,6 @@ pub fn assert_module_data_validity(
}
);

let ModuleVersion::Version(version) = &module_claim.info.version else {
panic!("Module version is not versioned, context setting is wrong")
};

// Assert that the contract version is equal to the module version
ensure_eq!(
version,
Expand All @@ -400,7 +416,6 @@ pub fn assert_module_data_validity(
match module_claim.reference {
ModuleReference::AccountBase(_) => return Ok(()),
ModuleReference::Native(_) => return Ok(()),
ModuleReference::Standalone(_) => return Ok(()),
_ => {}
}

Expand Down Expand Up @@ -751,4 +766,28 @@ mod test {
assert_that!(actual).is_err();
}
}

mod standalone_modules_valid {
use cosmwasm_std::testing::MOCK_CONTRACT_ADDR;

use super::*;

#[test]
fn no_cw2_contract() {
let deps = mock_dependencies();
let res = assert_module_data_validity(
&deps.as_ref().querier,
&Module {
info: ModuleInfo {
namespace: Namespace::new("counter").unwrap(),
name: "counter".to_owned(),
version: ModuleVersion::Version("1.1.0".to_owned()),
},
reference: ModuleReference::Standalone(0),
},
Some(Addr::unchecked(MOCK_CONTRACT_ADDR)),
);
assert!(res.is_ok());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<
self.base_state.save(deps.storage, &state)?;

let Some(handler) = self.maybe_instantiate_handler() else {
return Ok(Response::new())
return Ok(Response::new());
};
self.admin.set(deps.branch(), Some(info.sender.clone()))?;
handler(deps, env, info, self, msg.module)
Expand Down
4 changes: 3 additions & 1 deletion framework/packages/abstract-sdk/src/apis/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ impl<'a, T: ModuleInterface> Modules<'a, T> {
let maybe_module_addr =
ACCOUNT_MODULES.query(&self.deps.querier, manager_addr, module_id)?;
let Some(module_addr) = maybe_module_addr else {
return Err(crate::AbstractSdkError::MissingModule { module: module_id.to_string() });
return Err(crate::AbstractSdkError::MissingModule {
module: module_id.to_string(),
});
};
Ok(module_addr)
}
Expand Down
28 changes: 21 additions & 7 deletions framework/packages/abstract-sdk/src/base/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ where
&self,
) -> AbstractSdkResult<ExecuteHandlerFn<Self, Self::CustomExecMsg, Self::Error>> {
let Some(handler) = self.maybe_execute_handler() else {
return Err(AbstractSdkError::MissingHandler { endpoint: "execution handler".to_string() })
return Err(AbstractSdkError::MissingHandler {
endpoint: "execution handler".to_string(),
});
};
Ok(handler)
}
Expand All @@ -82,7 +84,9 @@ where
&self,
) -> AbstractSdkResult<InstantiateHandlerFn<Self, Self::CustomInitMsg, Self::Error>> {
let Some(handler) = self.maybe_instantiate_handler() else {
return Err(AbstractSdkError::MissingHandler { endpoint: "instantiate".to_string() })
return Err(AbstractSdkError::MissingHandler {
endpoint: "instantiate".to_string(),
});
};
Ok(handler)
}
Expand All @@ -99,7 +103,9 @@ where
&self,
) -> AbstractSdkResult<QueryHandlerFn<Self, Self::CustomQueryMsg, Self::Error>> {
let Some(handler) = self.maybe_query_handler() else {
return Err(AbstractSdkError::MissingHandler { endpoint: "query".to_string() })
return Err(AbstractSdkError::MissingHandler {
endpoint: "query".to_string(),
});
};
Ok(handler)
}
Expand All @@ -116,7 +122,9 @@ where
&self,
) -> AbstractSdkResult<MigrateHandlerFn<Self, Self::CustomMigrateMsg, Self::Error>> {
let Some(handler) = self.maybe_migrate_handler() else {
return Err(AbstractSdkError::MissingHandler { endpoint: "migrate".to_string() })
return Err(AbstractSdkError::MissingHandler {
endpoint: "migrate".to_string(),
});
};
Ok(handler)
}
Expand All @@ -129,7 +137,9 @@ where
/// Get a sudo handler or return an error.
fn sudo_handler(&self) -> AbstractSdkResult<SudoHandlerFn<Self, Self::SudoMsg, Self::Error>> {
let Some(handler) = self.maybe_sudo_handler() else {
return Err(AbstractSdkError::MissingHandler { endpoint: "sudo".to_string() })
return Err(AbstractSdkError::MissingHandler {
endpoint: "sudo".to_string(),
});
};
Ok(handler)
}
Expand All @@ -146,7 +156,9 @@ where
&self,
) -> AbstractSdkResult<ReceiveHandlerFn<Self, Self::ReceiveMsg, Self::Error>> {
let Some(handler) = self.maybe_receive_handler() else {
return Err(AbstractSdkError::MissingHandler { endpoint: "receive".to_string() })
return Err(AbstractSdkError::MissingHandler {
endpoint: "receive".to_string(),
});
};
Ok(handler)
}
Expand Down Expand Up @@ -178,7 +190,9 @@ where
/// Get a reply handler or return an error.
fn reply_handler(&self, id: u64) -> AbstractSdkResult<ReplyHandlerFn<Self, Self::Error>> {
let Some(handler) = self.maybe_reply_handler(id) else {
return Err(AbstractSdkError::MissingHandler { endpoint: format! {"reply with id {id}"} })
return Err(AbstractSdkError::MissingHandler {
endpoint: format! {"reply with id {id}"},
});
};
Ok(handler)
}
Expand Down

0 comments on commit eabd4f4

Please sign in to comment.