Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

receive callback fix: query NFT contract from storage (and not instantiate2) #98

Merged
merged 20 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion contracts/ics721-base/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub fn execute(
}

#[cfg_attr(not(feature = "library"), entry_point)]
pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult<Binary> {
pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> Result<Binary, ContractError> {
Ics721Contract::default().query(deps, env, msg)
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/sg-ics721/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub fn execute(
}

#[cfg_attr(not(feature = "library"), entry_point)]
pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult<Binary> {
pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> Result<Binary, ContractError> {
SgIcs721Contract::default().query(deps, env, msg)
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/sg-ics721/src/testing/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ fn execute(
SgIcs721Contract::default().execute(deps, env, info, msg)
}

fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult<Binary> {
fn query(deps: Deps, env: Env, msg: QueryMsg) -> Result<Binary, ContractError> {
SgIcs721Contract::default().query(deps, env, msg)
}

Expand Down
6 changes: 6 additions & 0 deletions packages/ics721-types/src/token_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ impl From<ClassId> for String {
}
}

impl From<String> for ClassId {
fn from(s: String) -> Self {
Self(s)
}
}

impl From<TokenId> for String {
fn from(t: TokenId) -> Self {
t.0
Expand Down
36 changes: 36 additions & 0 deletions packages/ics721/schema/ics721.json
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,36 @@
},
"additionalProperties": false
},
{
"description": "Returns predictable NFT contract using instantiate2. If no cw721_code_id is provided, default cw721_code_id from storage is used.",
"type": "object",
"required": [
"get_instantiate2_nft_contract"
],
"properties": {
"get_instantiate2_nft_contract": {
"type": "object",
"required": [
"class_id"
],
"properties": {
"class_id": {
"type": "string"
},
"cw721_code_id": {
"type": [
"integer",
"null"
],
"format": "uint64",
"minimum": 0.0
}
},
"additionalProperties": false
}
},
"additionalProperties": false
},
{
"description": "Gets the class level metadata URI for the provided class_id. If there is no metadata, returns None. Returns `Option<Class>`.",
"type": "object",
Expand Down Expand Up @@ -1320,6 +1350,12 @@
"format": "uint64",
"minimum": 0.0
},
"get_instantiate2_nft_contract": {
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "Addr",
"description": "A human readable address.\n\nIn Cosmos, this is typically bech32 encoded. But for multi-chain smart contracts no assumptions should be made other than being UTF-8 encoded and of reasonable length.\n\nThis type represents a validated address. It can be created in the following ways 1. Use `Addr::unchecked(input)` 2. Use `let checked: Addr = deps.api.addr_validate(input)?` 3. Use `let checked: Addr = deps.api.addr_humanize(canonical_addr)?` 4. Deserialize from JSON. This must only be done from JSON that was validated before such as a contract's state. `Addr` must not be used in messages sent by the user because this would result in unvalidated instances.\n\nThis type is immutable. If you really need to mutate it (Really? Are you sure?), create a mutable copy using `let mut mutable = Addr::to_string()` and operate on that `String` instance.",
"type": "string"
},
"incoming_channels": {
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "Array_of_Tuple_of_Tuple_of_ClassId_and_TokenId_and_String",
Expand Down
30 changes: 30 additions & 0 deletions packages/ics721/schema/raw/query.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,36 @@
},
"additionalProperties": false
},
{
"description": "Returns predictable NFT contract using instantiate2. If no cw721_code_id is provided, default cw721_code_id from storage is used.",
"type": "object",
"required": [
"get_instantiate2_nft_contract"
],
"properties": {
"get_instantiate2_nft_contract": {
"type": "object",
"required": [
"class_id"
],
"properties": {
"class_id": {
"type": "string"
},
"cw721_code_id": {
"type": [
"integer",
"null"
],
"format": "uint64",
"minimum": 0.0
}
},
"additionalProperties": false
}
},
"additionalProperties": false
},
{
"description": "Gets the class level metadata URI for the provided class_id. If there is no metadata, returns None. Returns `Option<Class>`.",
"type": "object",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "Addr",
"description": "A human readable address.\n\nIn Cosmos, this is typically bech32 encoded. But for multi-chain smart contracts no assumptions should be made other than being UTF-8 encoded and of reasonable length.\n\nThis type represents a validated address. It can be created in the following ways 1. Use `Addr::unchecked(input)` 2. Use `let checked: Addr = deps.api.addr_validate(input)?` 3. Use `let checked: Addr = deps.api.addr_humanize(canonical_addr)?` 4. Deserialize from JSON. This must only be done from JSON that was validated before such as a contract's state. `Addr` must not be used in messages sent by the user because this would result in unvalidated instances.\n\nThis type is immutable. If you really need to mutate it (Really? Are you sure?), create a mutable copy using `let mut mutable = Addr::to_string()` and operate on that `String` instance.",
"type": "string"
}
36 changes: 27 additions & 9 deletions packages/ics721/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::fmt::Debug;

use cosmwasm_std::{
from_json, to_json_binary, Addr, Binary, ContractInfoResponse, Deps, DepsMut, Empty, Env,
IbcMsg, MessageInfo, Order, Response, StdResult, SubMsg, WasmMsg,
Event, IbcMsg, MessageInfo, Order, Response, StdResult, SubMsg, WasmMsg,
};
use cw_storage_plus::Map;
use ics721_types::{
Expand Down Expand Up @@ -157,7 +157,7 @@ where
let token_id = TokenId::new(token_id);
let child_class_id = ClassId::new(child_class_id);
let child_collection = deps.api.addr_validate(&child_collection)?;
match query_nft_contract_for_class_id(deps.storage, child_class_id.to_string())? {
match query_nft_contract_for_class_id(deps.storage, child_class_id.clone())? {
Some(cw721_addr) => {
if cw721_addr != child_collection {
return Err(ContractError::NoNftContractMatch {
Expand Down Expand Up @@ -240,7 +240,7 @@ where
// check given home class id and home collection is the same as stored in the contract
let home_class_id = ClassId::new(home_class_id);
let home_collection = deps.api.addr_validate(&home_collection)?;
match query_nft_contract_for_class_id(deps.storage, home_class_id.to_string())? {
match query_nft_contract_for_class_id(deps.storage, home_class_id.clone())? {
Some(cw721_addr) => {
if cw721_addr != home_collection {
return Err(ContractError::NoNftContractMatch {
Expand Down Expand Up @@ -524,15 +524,23 @@ where
msg: to_json_binary(&ExecuteMsg::Callback(CallbackMsg::Mint {
class_id: class.id.clone(),
receiver,
tokens,
tokens: tokens.clone(),
}))?,
funds: vec![],
};

let instantiate = self.create_instantiate_msg(deps, &env, class.clone())?;
let (class_id_info, instantiate) =
self.create_instantiate_msg(deps, &env, class.clone())?;

let token_ids = format!("{:?}", tokens);
let event = Event::new("ics721_receive_create_vouchers")
.add_attribute("class_id", class_id_info.class_id)
.add_attribute("nft_contract", class_id_info.address)
.add_attribute("token_ids", token_ids);
Comment on lines +532 to +539
Copy link
Collaborator Author

@taitruong taitruong May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 3 new events, for better processing (e.g. for constellations):

  1. event when NFTs are created/minted...


Ok(Response::<T>::default()
.add_attribute("method", "callback_create_vouchers")
.add_event(event)
.add_submessages(instantiate)
.add_message(mint))
}
Expand All @@ -542,9 +550,11 @@ where
deps: DepsMut,
env: &Env,
class: Class,
) -> Result<Vec<SubMsg<T>>, ContractError> {
if CLASS_ID_AND_NFT_CONTRACT_INFO.has(deps.storage, class.id.to_string().as_str()) {
Ok(vec![])
) -> Result<(ClassIdInfo, Vec<SubMsg<T>>), ContractError> {
let maybe_class_id_info =
CLASS_ID_AND_NFT_CONTRACT_INFO.may_load(deps.as_ref().storage, &class.id)?;
if let Some(nft_contract) = maybe_class_id_info {
Ok((nft_contract, vec![]))
} else {
let class_id = ClassId::new(class.id.clone());
let cw721_code_id = CW721_CODE_ID.load(deps.storage)?;
Expand Down Expand Up @@ -586,7 +596,7 @@ where
},
INSTANTIATE_CW721_REPLY_ID,
);
Ok(vec![message])
Ok((class_id_info, vec![message]))
}
}

Expand Down Expand Up @@ -629,8 +639,16 @@ where
let VoucherRedemption { class, token_ids } = redeem;
let nft_contract = load_nft_contract_for_class_id(deps.storage, class.id.to_string())?;
let receiver = deps.api.addr_validate(&receiver)?;

let token_ids_string = format!("{:?}", token_ids);
let event = Event::new("ics721_receive_redeem_vouchers")
.add_attribute("class_id", class.id)
.add_attribute("nft_contract", nft_contract.clone())
.add_attribute("token_ids", token_ids_string);
Comment on lines +643 to +647
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. event when NFTs are unescrowed/returned...


Ok(Response::default()
.add_attribute("method", "callback_redeem_vouchers")
.add_event(event)
.add_messages(
token_ids
.into_iter()
Expand Down
11 changes: 9 additions & 2 deletions packages/ics721/src/ibc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cosmwasm_std::{
from_json, to_json_binary, DepsMut, Empty, Env, IbcBasicResponse, IbcChannelCloseMsg,
from_json, to_json_binary, DepsMut, Empty, Env, Event, IbcBasicResponse, IbcChannelCloseMsg,
IbcChannelConnectMsg, IbcChannelOpenMsg, IbcChannelOpenResponse, IbcPacket, IbcPacketAckMsg,
IbcPacketReceiveMsg, IbcPacketTimeoutMsg, IbcReceiveResponse, Never, Reply, Response,
StdResult, SubMsgResult, WasmMsg,
Expand Down Expand Up @@ -162,14 +162,21 @@ where
None => vec![],
};

let token_ids = format!("{:?}", msg.token_ids);
let event = Event::new("ics721_ack_burn_vouchers")
.add_attribute("class_id", msg.class_id.to_string())
.add_attribute("nft_contract", nft_contract.clone())
.add_attribute("token_ids", token_ids.clone());
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. event when NFTs are burned


Ok(IbcBasicResponse::new()
.add_messages(burn_notices)
.add_submessages(callback)
.add_attribute("method", "acknowledge")
.add_attribute("sender", msg.sender)
.add_attribute("receiver", msg.receiver)
.add_attribute("classId", msg.class_id)
.add_attribute("token_ids", format!("{:?}", msg.token_ids)))
.add_attribute("token_ids", token_ids)
.add_event(event))
}
}

Expand Down
42 changes: 23 additions & 19 deletions packages/ics721/src/ibc_packet_receive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,17 @@ use cosmwasm_std::{
from_json, to_json_binary, Addr, Binary, Deps, DepsMut, Empty, Env, IbcPacket,
IbcReceiveResponse, StdResult, SubMsg, WasmMsg,
};
use sha2::{Digest, Sha256};
use zip_optional::Zippable;

use crate::{
helpers::{
generate_receive_callback_msg, get_incoming_proxy_msg, get_instantiate2_address,
get_receive_callback,
},
helpers::{generate_receive_callback_msg, get_incoming_proxy_msg, get_receive_callback},
ibc::ACK_AND_DO_NOTHING_REPLY_ID,
ibc_helpers::{get_endpoint_prefix, try_pop_source_prefix},
msg::{CallbackMsg, ExecuteMsg},
query::load_nft_contract_for_class_id,
query::{
load_nft_contract_for_class_id, query_get_instantiate2_nft_contract,
query_nft_contract_for_class_id,
},
state::{CW721_CODE_ID, OUTGOING_CLASS_TOKEN_TO_CHANNEL, PO},
token_types::{VoucherCreation, VoucherRedemption},
ContractError,
Expand Down Expand Up @@ -232,19 +231,24 @@ fn create_callback_msg(
load_nft_contract_for_class_id(deps.storage, local_class_id.to_string())
.map_err(|_| ContractError::NoNftContractForClassId(local_class_id.to_string()))
} else {
// If its a creation action, we can use the instantiate2 function to get the nft contract
// we don't care of the contract is instantiated yet or not, as later submessage will instantiate it if its not.
// The reason we use instantiate2 here is because we don't know if it was already instantiated or not.

let cw721_code_id = CW721_CODE_ID.load(deps.storage)?;
// for creating a predictable nft contract using, using instantiate2, we need: checksum, creator, and salt:
// - using class id as salt for instantiating nft contract guarantees a) predictable address and b) uniqueness
// for this salt must be of length 32 bytes, so we use sha256 to hash class id
let mut hasher = Sha256::new();
hasher.update(local_class_id.as_bytes());
let salt = hasher.finalize().to_vec();

get_instantiate2_address(deps, env.contract.address.as_str(), &salt, cw721_code_id)
let nft_contract =
match query_nft_contract_for_class_id(deps.storage, local_class_id.clone()) {
Ok(nft_contract) => nft_contract,
Err(_) => None, // not found, occurs on initial transfer when we don't have the contract address
};
match nft_contract {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

match query_nft_contract_for_class_id(deps.storage, local_class_id.clone()).ok_or(None), can this work?

Some(nft_contract) => Ok(nft_contract),
None => {
// contract not yet instantiated, so we use instantiate2 to get the contract address
let cw721_code_id = CW721_CODE_ID.load(deps.storage)?;
query_get_instantiate2_nft_contract(
deps,
env,
local_class_id.clone(),
Some(cw721_code_id),
)
}
}
}?;

Ok(generate_receive_callback_msg(
Expand Down
8 changes: 8 additions & 0 deletions packages/ics721/src/msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ pub enum QueryMsg {
#[returns(Option<::cosmwasm_std::Addr>)]
NftContract { class_id: String },

/// Returns predictable NFT contract using instantiate2. If no
/// cw721_code_id is provided, default cw721_code_id from storage is used.
#[returns(::cosmwasm_std::Addr)]
GetInstantiate2NftContract {
class_id: String,
cw721_code_id: Option<u64>,
},
Comment on lines +130 to +134
Copy link
Collaborator Author

@taitruong taitruong May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added new GetInstantiate2NftContract, which will be super helpful - e.g. for WLing collections without doing an initial transfer!


/// Gets the class level metadata URI for the provided
/// class_id. If there is no metadata, returns None. Returns
/// `Option<Class>`.
Expand Down
Loading
Loading