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.

Sign up for GitHub

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

Audit revision #82

Closed
wants to merge 4 commits into from
Closed

Audit revision #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
461df16
pass nft contract, instead of overriding info.sender
taitruong Jan 22, 2024
84455a3
remove NFT_CONTRACT_TO_CLASS_ID and CLASS_ID_TO_NFT_CONTRACT and merg…
taitruong Jan 23, 2024
8fe5b40
test migration
taitruong Jan 23, 2024
b301bae
test back transfer
taitruong Jan 25, 2024
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
5 changes: 4 additions & 1 deletion packages/ics721/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ pub enum ContractError {
#[error("Transfer Doesn't contain any action, no redemption or creation")]
InvalidTransferNoAction,

#[error("Couldn't find nft contract for this class id: {0}")]
#[error("Couldn't find nft contract for class id: {0}")]
NoNftContractForClassId(String),

#[error("Couldn't find class id for nft contract: {0}")]
NoClassIdForNftContract(String),
}
190 changes: 115 additions & 75 deletions packages/ics721/src/execute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use std::fmt::Debug;

use cosmwasm_std::{
from_json, to_json_binary, Addr, Binary, ContractInfoResponse, Deps, DepsMut, Empty, Env,
IbcMsg, MessageInfo, Response, StdResult, SubMsg, WasmMsg,
IbcMsg, MessageInfo, Order, Response, StdResult, SubMsg, WasmMsg,
};
use cw_storage_plus::Map;
use ics721_types::{
ibc_types::{IbcOutgoingMsg, IbcOutgoingProxyMsg, NonFungibleTokenPacketData},
token_types::{Class, ClassId, Token, TokenId},
Expand All @@ -18,9 +19,10 @@ use crate::{
INSTANTIATE_OUTGOING_PROXY_REPLY_ID,
},
msg::{CallbackMsg, ExecuteMsg, InstantiateMsg, MigrateMsg},
query::{load_class_id_for_nft_contract, load_nft_contract_for_class_id},
state::{
CollectionData, UniversalAllNftInfoResponse, ADMIN_USED_FOR_CW721, CLASS_ID_TO_CLASS,
CLASS_ID_TO_NFT_CONTRACT, CW721_CODE_ID, INCOMING_PROXY, NFT_CONTRACT_TO_CLASS_ID,
ClassIdInfo, CollectionData, UniversalAllNftInfoResponse, ADMIN_USED_FOR_CW721,
CLASS_ID_AND_NFT_CONTRACT_INFO, CLASS_ID_TO_CLASS, CW721_CODE_ID, INCOMING_PROXY,
OUTGOING_CLASS_TOKEN_TO_CHANNEL, OUTGOING_PROXY, PO, TOKEN_METADATA,
},
token_types::{VoucherCreation, VoucherRedemption},
Expand Down Expand Up @@ -132,30 +134,23 @@ where
}
from_json::<IbcOutgoingProxyMsg>(msg.clone())
.ok()
.map(|msg| {
let mut info = info;
match deps.api.addr_validate(&msg.collection) {
Ok(collection_addr) => {
// set collection address as (initial) sender
info.sender = collection_addr;
self.receive_nft(
deps,
env,
info,
TokenId::new(token_id),
nft_owner,
msg.msg,
)
}
Err(err) => Err(ContractError::Std(err)),
}
.map(|msg| match deps.api.addr_validate(&msg.collection) {
Ok(nft_contract) => self.receive_nft(
deps,
env,
&nft_contract,
TokenId::new(token_id),
nft_owner,
msg.msg,
),
Err(err) => Err(ContractError::Std(err)),
})
}
None => from_json::<IbcOutgoingMsg>(msg.clone()).ok().map(|_| {
self.receive_nft(
deps,
env,
info,
&info.sender,
TokenId::new(token_id),
nft_owner,
msg.clone(),
Expand All @@ -171,32 +166,35 @@ where
&self,
deps: DepsMut,
env: Env,
info: MessageInfo,
nft_contract: &Addr,
token_id: TokenId,
nft_owner: String,
msg: Binary,
) -> Result<Response<T>, ContractError> {
let nft_owner = deps.api.addr_validate(&nft_owner)?;
let msg: IbcOutgoingMsg = from_json(msg)?;

let class = match NFT_CONTRACT_TO_CLASS_ID.may_load(deps.storage, info.sender.clone())? {
let class = match load_class_id_for_nft_contract(deps.as_ref().storage, nft_contract)? {
Some(class_id) => CLASS_ID_TO_CLASS.load(deps.storage, class_id)?,
// No class ID being present means that this is a local NFT
// that has never been sent out of this contract.
None => {
let class_data = self.get_class_data(&deps, &info.sender)?;
let class_data = self.get_class_data(&deps, nft_contract)?;
let data = class_data.as_ref().map(to_json_binary).transpose()?;
let class = Class {
id: ClassId::new(info.sender.to_string()),
id: ClassId::new(nft_contract.to_string()),
// There is no collection-level uri nor data in the
// cw721 specification so we set those values to
// `None` for local, cw721 NFTs.
uri: None,
data,
};

NFT_CONTRACT_TO_CLASS_ID.save(deps.storage, info.sender.clone(), &class.id)?;
CLASS_ID_TO_NFT_CONTRACT.save(deps.storage, class.id.clone(), &info.sender)?;
let class_id_info = ClassIdInfo {
class_id: class.id.clone(),
address: nft_contract.clone(),
};
CLASS_ID_AND_NFT_CONTRACT_INFO.save(deps.storage, &class.id, &class_id_info)?;

// Merging and usage of this PR may change that:
// <https://github.com/CosmWasm/cw-nfts/pull/75>
Expand All @@ -207,7 +205,7 @@ where

// make sure NFT is escrowed by ics721
let UniversalAllNftInfoResponse { access, info } = deps.querier.query_wasm_smart(
info.sender,
nft_contract,
&cw721::Cw721QueryMsg::AllNftInfo {
token_id: token_id.clone().into(),
include_expired: None,
Expand Down Expand Up @@ -313,48 +311,52 @@ where
create: VoucherCreation,
) -> Result<Response<T>, ContractError> {
let VoucherCreation { class, tokens } = create;
let instantiate = if CLASS_ID_TO_NFT_CONTRACT.has(deps.storage, class.id.clone()) {
vec![]
} else {
let class_id = ClassId::new(class.id.clone());
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(class_id.as_bytes());
let salt = hasher.finalize().to_vec();

let cw721_addr = get_instantiate2_address(
deps.as_ref(),
env.contract.address.as_str(),
&salt,
cw721_code_id,
)?;

// Save classId <-> contract mappings.
CLASS_ID_TO_NFT_CONTRACT.save(deps.storage, class_id.clone(), &cw721_addr)?;
NFT_CONTRACT_TO_CLASS_ID.save(deps.storage, cw721_addr, &class_id)?;

let admin = ADMIN_USED_FOR_CW721
.load(deps.storage)?
.map(|a| a.to_string());
let message = SubMsg::<T>::reply_on_success(
WasmMsg::Instantiate2 {
admin,
code_id: cw721_code_id,
msg: self.init_msg(deps.as_ref(), &env, &class)?,
funds: vec![],
// Attempting to fit the class ID in the label field
// can make this field too long which causes data
// errors in the SDK.
label: "ics-721 debt-voucher cw-721".to_string(),
salt: salt.into(),
},
INSTANTIATE_CW721_REPLY_ID,
);
vec![message]
};
let instantiate =
if CLASS_ID_AND_NFT_CONTRACT_INFO.has(deps.storage, class.id.to_string().as_str()) {
vec![]
} else {
let class_id = ClassId::new(class.id.clone());
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(class_id.as_bytes());
let salt = hasher.finalize().to_vec();

let nft_contract = get_instantiate2_address(
deps.as_ref(),
env.contract.address.as_str(),
&salt,
cw721_code_id,
)?;

// Save classId <-> contract mappings.
let class_id_info = ClassIdInfo {
class_id: class_id.clone(),
address: nft_contract.clone(),
};
CLASS_ID_AND_NFT_CONTRACT_INFO.save(deps.storage, &class.id, &class_id_info)?;

let admin = ADMIN_USED_FOR_CW721
.load(deps.storage)?
.map(|a| a.to_string());
let message = SubMsg::<T>::reply_on_success(
WasmMsg::Instantiate2 {
admin,
code_id: cw721_code_id,
msg: self.init_msg(deps.as_ref(), &env, &class)?,
funds: vec![],
// Attempting to fit the class ID in the label field
// can make this field too long which causes data
// errors in the SDK.
label: "ics-721 debt-voucher cw-721".to_string(),
salt: salt.into(),
},
INSTANTIATE_CW721_REPLY_ID,
);
vec![message]
};

// Store mapping from classID to classURI. Notably, we don't check
// if this has already been set. If a new NFT belonging to a class
Expand Down Expand Up @@ -416,7 +418,7 @@ where
redeem: VoucherRedemption,
) -> Result<Response<T>, ContractError> {
let VoucherRedemption { class, token_ids } = redeem;
let nft_contract = CLASS_ID_TO_NFT_CONTRACT.load(deps.storage, class.id)?;
let nft_contract = load_nft_contract_for_class_id(deps.storage, class.id.to_string())?;
let receiver = deps.api.addr_validate(&receiver)?;
Ok(Response::default()
.add_attribute("method", "callback_redeem_vouchers")
Expand Down Expand Up @@ -445,7 +447,8 @@ where
receiver: String,
) -> Result<Response<T>, ContractError> {
let receiver = deps.api.addr_validate(&receiver)?;
let cw721_addr = CLASS_ID_TO_NFT_CONTRACT.load(deps.storage, class_id.clone())?;
let nft_contract =
load_nft_contract_for_class_id(deps.as_ref().storage, class_id.to_string())?;

let mint = tokens
.into_iter()
Expand All @@ -463,7 +466,7 @@ where
extension: Empty::default(),
};
Ok(WasmMsg::Execute {
contract_addr: cw721_addr.to_string(),
contract_addr: nft_contract.to_string(),
msg: to_json_binary(&msg)?,
funds: vec![],
})
Expand Down Expand Up @@ -517,7 +520,8 @@ where
.save(deps.storage, &Some(deps.api.addr_validate(&cw721_admin)?))?;
}
}
Ok(Response::default()

let response = Response::default()
.add_attribute("method", "migrate")
.add_attribute("pauser", pauser.map_or_else(|| "none".to_string(), |or| or))
.add_attribute(
Expand All @@ -544,7 +548,43 @@ where
}
},
),
))
);

// TODO: once migrated, this complete block can be deleted
// - get legacy map and migrate it to new indexed map
let nft_contract_to_class_id: Map<Addr, ClassId> = Map::new("f");
match cw_paginate_storage::paginate_map(
deps.as_ref(),
&nft_contract_to_class_id,
None,
None,
Order::Ascending,
) {
Ok(nft_contract_and_class_id) => {
// legacy map needs to be cleared, before migrating to indexed map
let class_id_to_nft_contract: Map<ClassId, Addr> = Map::new("e");
class_id_to_nft_contract.clear(deps.storage);
let response = response.add_attribute(
"migrated nft contracts",
nft_contract_and_class_id.len().to_string(),
);
for (nft_contract, class_id) in nft_contract_and_class_id {
let class_id_info = ClassIdInfo {
class_id: class_id.clone(),
address: nft_contract.clone(),
};
CLASS_ID_AND_NFT_CONTRACT_INFO.save(
deps.storage,
&class_id,
&class_id_info,
)?;
}
// let's clear legacy map, so it wont get migrated again
nft_contract_to_class_id.clear(deps.storage);
Ok(response)
}
Err(err) => Err(ContractError::Std(err)),
}
}
}
}
Expand Down
28 changes: 17 additions & 11 deletions packages/ics721/src/ibc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ use crate::{
helpers::ack_callback_msg,
ibc_helpers::{ack_fail, ack_success, try_get_ack_error, validate_order_and_version},
ibc_packet_receive::receive_ibc_packet,
query::{load_class_id_for_nft_contract, load_nft_contract_for_class_id},
state::{
CLASS_ID_TO_NFT_CONTRACT, INCOMING_CLASS_TOKEN_TO_CHANNEL, INCOMING_PROXY,
NFT_CONTRACT_TO_CLASS_ID, OUTGOING_CLASS_TOKEN_TO_CHANNEL, OUTGOING_PROXY, TOKEN_METADATA,
INCOMING_CLASS_TOKEN_TO_CHANNEL, INCOMING_PROXY, OUTGOING_CLASS_TOKEN_TO_CHANNEL,
OUTGOING_PROXY, TOKEN_METADATA,
},
ContractError,
};
Expand Down Expand Up @@ -121,7 +122,8 @@ where
} else {
let msg: NonFungibleTokenPacketData = from_json(&ack.original_packet.data)?;

let nft_contract = CLASS_ID_TO_NFT_CONTRACT.load(deps.storage, msg.class_id.clone())?;
let nft_contract =
load_nft_contract_for_class_id(deps.storage, msg.class_id.to_string())?;
// Burn all of the tokens being transfered out that were
// previously transfered in on this channel.
let burn_notices = msg.token_ids.iter().cloned().try_fold(
Expand Down Expand Up @@ -188,7 +190,8 @@ where
error: &str,
) -> Result<IbcBasicResponse, ContractError> {
let message: NonFungibleTokenPacketData = from_json(&packet.data)?;
let nft_contract = CLASS_ID_TO_NFT_CONTRACT.load(deps.storage, message.class_id.clone())?;
let nft_contract =
load_nft_contract_for_class_id(deps.storage, message.class_id.to_string())?;
let sender = deps.api.addr_validate(&message.sender)?;

let messages = message
Expand Down Expand Up @@ -241,15 +244,18 @@ where
// `ACK_AND_DO_NOTHING`.

let res = parse_reply_instantiate_data(reply)?;
let cw721_addr = deps.api.addr_validate(&res.contract_address)?;
let nft_contract = deps.api.addr_validate(&res.contract_address)?;

// cw721 addr has already been stored, here just check whether it exists, otherwise a NotFound error is thrown
let class_id = NFT_CONTRACT_TO_CLASS_ID.load(deps.storage, cw721_addr.clone())?;

Ok(Response::default()
.add_attribute("method", "instantiate_cw721_reply")
.add_attribute("class_id", class_id)
.add_attribute("cw721_addr", cw721_addr))
match load_class_id_for_nft_contract(deps.storage, &nft_contract)? {
Some(class_id) => Ok(Response::default()
.add_attribute("method", "instantiate_cw721_reply")
.add_attribute("class_id", class_id)
.add_attribute("cw721_addr", nft_contract)),
None => Err(ContractError::NoClassIdForNftContract(
nft_contract.to_string(),
)),
}
}
INSTANTIATE_OUTGOING_PROXY_REPLY_ID => {
let res = parse_reply_instantiate_data(reply)?;
Expand Down
9 changes: 3 additions & 6 deletions packages/ics721/src/ibc_packet_receive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ use crate::{
ibc::ACK_AND_DO_NOTHING_REPLY_ID,
ibc_helpers::{get_endpoint_prefix, try_pop_source_prefix},
msg::{CallbackMsg, ExecuteMsg},
state::{
CLASS_ID_TO_NFT_CONTRACT, CW721_CODE_ID, INCOMING_CLASS_TOKEN_TO_CHANNEL,
OUTGOING_CLASS_TOKEN_TO_CHANNEL, PO,
},
query::load_nft_contract_for_class_id,
state::{CW721_CODE_ID, INCOMING_CLASS_TOKEN_TO_CHANNEL, OUTGOING_CLASS_TOKEN_TO_CHANNEL, PO},
token_types::{VoucherCreation, VoucherRedemption},
ContractError,
};
Expand Down Expand Up @@ -141,8 +139,7 @@ pub(crate) fn receive_ibc_packet(
let nft_contract = if let Some(voucher) = action_aggregator.redemption.clone() {
// If its a redemption, it means we already have the contract address in storage

CLASS_ID_TO_NFT_CONTRACT
.load(deps.storage, voucher.class.id.clone())
load_nft_contract_for_class_id(deps.storage, voucher.class.id.to_string())
.map_err(|_| ContractError::NoNftContractForClassId(voucher.class.id.to_string()))
} else if let Some(voucher) = action_aggregator.creation.clone() {
// If its a creation action, we can use the instantiate2 function to get the nft contract
Expand Down
Loading
Loading