Contract/nft querier #184
Contract/nft querier #184
Conversation
faust403
requested review from
ratik,
foxpy and
albertandrejev
and removed request for
foxpy
| state::{ | ||
| factory::{State, STATE}, | ||
| pump::PumpTimeout, | ||
| }, |
There was a problem hiding this comment.
this seem to be out of the scope of this PR
There was a problem hiding this comment.
moving messages from factory to package
There was a problem hiding this comment.
also as far as I remember it was made in the other pr
There was a problem hiding this comment.
It's related because it's used in the contract itself to query a factory state
There was a problem hiding this comment.
Yeah, it was done in #81. I guess it's okay to do that in both of these PR's and merge updates between them in future
There was a problem hiding this comment.
oh, my bad. IC. so this way I see you are already trying to query factory in phonebook-like manner. this way I think it we be great to adopt actual phonebook implementation
| Err(_) => return Err(crate::error::ContractError::UnknownNftId {}), | ||
| }; | ||
|
|
||
| let batch_id = nft_details.info.extension.unwrap().batch_id; |
There was a problem hiding this comment.
Please add a comment explaining why a call to this .unwrap() is safe (because our NFT contract always uses the extension field).
There was a problem hiding this comment.
The idea of this review request was to make it obvious for anyone reading the code why a call to .unwrap() is safe. I don't think your current comment is descriptive enough :D
Please write explicitly that an NFT contract we are interacting with here is written and controlled fully by us, and it always uses an extension.
Co-authored-by: Murad <muradkarammaev@gmail.com>
Co-authored-by: Murad <muradkarammaev@gmail.com>
|
|
||
| #[cw_serde] | ||
| pub struct InstantiateMsg { | ||
| pub factory_contract: Addr, |
There was a problem hiding this comment.
Never accept Addr from external sources. There are a few basic rules to follow:
- if you receive an address from an external source, it must be of type
String. Examples of external sources:ExecuteMsg,QueryMsg,InstantiateMsg, etc.; - once you receive an address from an external source, run
deps.api.addr_validate()on it to convert it intocosmwasm_std::Addr; - if you need to save an address to persistent storage, save it as
Addr.
| pub factory_contract: Addr, | |
| pub factory_contract: String, |
| #[cw_ownable_execute] | ||
| #[cw_serde] | ||
| pub enum ExecuteMsg { | ||
| UpdateConfig { new_config: Config }, |
There was a problem hiding this comment.
This also applies here. Since in this context a struct of Config is used as an external source, you are at risk of accepting an unvalidated Addr.
Normally, in other contracts, we define a separate UpdateConfig struct to mitigate this issue.
No description provided.