Fix trait to be compatible for custom modules #77
Conversation
contracts/cw721-base/src/execute.rs
Outdated
| E1: DeserializeOwned, | ||
| E2: DeserializeOwned, |
There was a problem hiding this comment.
E1 is the execution message and E2 is the execution query message to the contract. They are the messages to the contract but not to the chain custom modules, so they are set as DeserializeOwned.
contracts/cw721-base/src/execute.rs
Outdated
| { | ||
| pub fn instantiate( | ||
| &self, | ||
| deps: DepsMut, | ||
| deps: DepsMut<Q>, |
There was a problem hiding this comment.
Assigning CustomQuery to all the Deps in order to make communication to the custom modules available.
JakeHartnell
requested review from
orkunkl,
JakeHartnell,
Callum-A,
cypherape,
0xekez,
shanev and
yubrew
and removed request for
orkunkl
contracts/cw721-base/src/execute.rs
Outdated
| T: Serialize + DeserializeOwned + Clone, | ||
| E1: DeserializeOwned, | ||
| E2: DeserializeOwned, | ||
| C: CustomMsg, | ||
| E: CustomMsg, | ||
| Q: CustomMsg, | ||
| Q: CustomQuery, |
There was a problem hiding this comment.
This is going to need to get rebased on this change which also changes these generic names: #75
How would you feel about giving Q here a more descriptive name? DepsQueryExt?
There was a problem hiding this comment.
I would name Q into ModuleQuery and C into ModuleMsg. What do you think?
contracts/cw721-base/src/execute.rs
Outdated
| @@ -77,16 +80,16 @@ where | |||
| } | |||
|
|
|||
| // TODO pull this into some sort of trait extension?? | |||
| impl<'a, T, C, E, Q> Cw721Contract<'a, T, C, E, Q> | |||
| impl<'a, T, E1, E2, C, Q> Cw721Contract<'a, T, E1, E2, C, Q> | |||
There was a problem hiding this comment.
| impl<'a, T, E1, E2, C, Q> Cw721Contract<'a, T, E1, E2, C, Q> | |
| impl<'a, T, E1, E2, C, Q> Cw721Contract<'a, T, E1, E2, C, Q = Empty> |
How about giving this the same default value as CosmWasm does?
There was a problem hiding this comment.
Yeah, it would be great adding default on its struct.
contracts/cw721-base/src/helpers.rs
Outdated
| C: CustomMsg, | ||
| Q: CustomQuery, |
There was a problem hiding this comment.
| C: CustomMsg, | |
| Q: CustomQuery, | |
| ModuleLevelCustomMsg: CustomMsg = Empty, | |
| ModuleLevelCustomQuery: CustomQuery = Empty, |
dadamu
requested review from
0xekez
and removed request for
orkunkl,
JakeHartnell,
Callum-A,
cypherape,
shanev and
yubrew
|
Rebased. |
|
Updated to |
|
Updated to main branch, any updates for it? @0xekez |
Currently,
CustomMsgisn't thecosmwasm-stdversion andCustomQueryis not imported, it makes the current version is not compatible for custom modules, likex/profilesin Desmos chain. In addition, any messages to the contract should not beCustomMsgsince they are not for the custom modules, they should beDeserializeOwned, instead.This PR intends to fix the issues mentioned above.