Skip to content

Commit

Permalink
Support ClassData and TokenData not according to ICS-721 spec (#1039)
Browse files Browse the repository at this point in the history
* skip validation, make some data optional

* check the length of token_uri and token_data

* fix to set TokenData and TokenUri at once

* imp: add validate_basic method for PacketData

* imp: allow any format for Data + define parse_as_ics721_data method

* fmt and clippy

* custom serde packet data with option

* add a test

* restore conversions

---------

Co-authored-by: Farhad Shabani <farhad.shabani@gmail.com>
  • Loading branch information
yito88 and Farhad-Shabani committed Jan 22, 2024
1 parent 3c0fddd commit 1de2bd4
Show file tree
Hide file tree
Showing 16 changed files with 322 additions and 154 deletions.
3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ ibc-client-tendermint-types = { version = "0.49.1", path = "./ibc-clients/ics07-
ibc-app-transfer-types = { version = "0.49.1", path = "./ibc-apps/ics20-transfer/types", default-features = false }
ibc-app-nft-transfer-types = { version = "0.49.1", path = "./ibc-apps/ics721-nft-transfer/types", default-features = false }

#ibc-proto = { version = "0.39.1", default-features = false }
ibc-proto = { git = "https://github.com/heliaxdev/ibc-proto-rs", branch = "yuji/feat/ics721-impl", default-features = false }
ibc-proto = { version = "0.41.0", default-features = false }

# cosmos dependencies
tendermint = { version = "0.34.0", default-features = false }
Expand Down
38 changes: 26 additions & 12 deletions ibc-apps/ics721-nft-transfer/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,21 @@ pub trait NftContext {
fn get_id(&self) -> &TokenId;

/// Get the token URI
fn get_uri(&self) -> &TokenUri;
fn get_uri(&self) -> Option<&TokenUri>;

/// Get the token Data
fn get_data(&self) -> &TokenData;
fn get_data(&self) -> Option<&TokenData>;
}

pub trait NftClassContext {
/// Get the class ID
fn get_id(&self) -> &ClassId;

/// Get the class URI
fn get_uri(&self) -> &ClassUri;
fn get_uri(&self) -> Option<&ClassUri>;

/// Get the class Data
fn get_data(&self) -> &ClassData;
fn get_data(&self) -> Option<&ClassData>;
}

/// Read-only methods required in NFT transfer validation context.
Expand All @@ -50,11 +50,18 @@ pub trait NftTransferValidationContext {
fn can_receive_nft(&self) -> Result<(), NftTransferError>;

/// Validates that the NFT can be created or updated successfully.
///
/// Note: some existing ICS-721 implementations may not strictly adhere to
/// the ICS-721 class data structure. The
/// [`ClassData`] associated with this
/// implementation can take any valid JSON format. If your project requires
/// ICS-721 format for the `ClassData`, ensure correctness by checking with
/// [`parse_as_ics721_data()`](crate::types::Data::parse_as_ics721_data).
fn create_or_update_class_validate(
&self,
class_id: &PrefixedClassId,
class_uri: &ClassUri,
class_data: &ClassData,
class_uri: Option<&ClassUri>,
class_data: Option<&ClassData>,
) -> Result<(), NftTransferError>;

/// Validates that the tokens can be escrowed successfully.
Expand Down Expand Up @@ -83,13 +90,20 @@ pub trait NftTransferValidationContext {
) -> Result<(), NftTransferError>;

/// Validates the receiver account and the NFT input
///
/// Note: some existing ICS-721 implementations may not strictly adhere to
/// the ICS-721 token data structure. The
/// [`TokenData`] associated with this
/// implementation can take any valid JSON format. If your project requires
/// ICS-721 format for `TokenData`, ensure correctness by checking with
/// [`parse_as_ics721_data()`](crate::types::Data::parse_as_ics721_data).
fn mint_nft_validate(
&self,
account: &Self::AccountId,
class_id: &PrefixedClassId,
token_id: &TokenId,
token_uri: &TokenUri,
token_data: &TokenData,
token_uri: Option<&TokenUri>,
token_data: Option<&TokenData>,
) -> Result<(), NftTransferError>;

/// Validates the sender account and the coin input before burning.
Expand Down Expand Up @@ -133,8 +147,8 @@ pub trait NftTransferExecutionContext: NftTransferValidationContext {
fn create_or_update_class_execute(
&self,
class_id: &PrefixedClassId,
class_uri: &ClassUri,
class_data: &ClassData,
class_uri: Option<&ClassUri>,
class_data: Option<&ClassData>,
) -> Result<(), NftTransferError>;

/// Executes the escrow of the NFT in a user account.
Expand Down Expand Up @@ -167,8 +181,8 @@ pub trait NftTransferExecutionContext: NftTransferValidationContext {
account: &Self::AccountId,
class_id: &PrefixedClassId,
token_id: &TokenId,
token_uri: &TokenUri,
token_data: &TokenData,
token_uri: Option<&TokenUri>,
token_data: Option<&TokenData>,
) -> Result<(), NftTransferError>;

/// Executes burning of the NFT in a user account.
Expand Down
28 changes: 12 additions & 16 deletions ibc-apps/ics721-nft-transfer/src/handler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,12 @@ pub fn refund_packet_nft_execute(
}
// mint vouchers back to sender
else {
data.token_ids
.0
.iter()
.zip(data.token_uris.iter())
.zip(data.token_data.iter())
.try_for_each(|((token_id, token_uri), token_data)| {
ctx_a.mint_nft_execute(&sender, &data.class_id, token_id, token_uri, token_data)
})
for (i, token_id) in data.token_ids.0.iter().enumerate() {
let token_uri = data.token_uris.as_ref().and_then(|uris| uris.get(i));
let token_data = data.token_data.as_ref().and_then(|data| data.get(i));
ctx_a.mint_nft_execute(&sender, &data.class_id, token_id, token_uri, token_data)?;
}
Ok(())
}
}

Expand Down Expand Up @@ -77,13 +75,11 @@ pub fn refund_packet_nft_validate(
)
})
} else {
data.token_ids
.0
.iter()
.zip(data.token_uris.iter())
.zip(data.token_data.iter())
.try_for_each(|((token_id, token_uri), token_data)| {
ctx_a.mint_nft_validate(&sender, &data.class_id, token_id, token_uri, token_data)
})
for (i, token_id) in data.token_ids.0.iter().enumerate() {
let token_uri = data.token_uris.as_ref().and_then(|uris| uris.get(i));
let token_data = data.token_data.as_ref().and_then(|data| data.get(i));
ctx_a.mint_nft_validate(&sender, &data.class_id, token_id, token_uri, token_data)?;
}
Ok(())
}
}
31 changes: 14 additions & 17 deletions ibc-apps/ics721-nft-transfer/src/handler/on_recv_packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,10 @@ where
events: vec![],
log: Vec::new(),
};
for ((token_id, token_uri), token_data) in data
.token_ids
.0
.iter()
.zip(data.token_uris.iter())
.zip(data.token_data.iter())
{
for (i, token_id) in data.token_ids.0.iter().enumerate() {
let token_uri = data.token_uris.as_ref().and_then(|uris| uris.get(i));
let token_data = data.token_data.as_ref().and_then(|data| data.get(i));

let trace_event = TokenTraceEvent {
trace_hash: ctx_b.token_hash_string(&class_id, token_id),
class: class_id.clone(),
Expand All @@ -98,19 +95,19 @@ where
// Note: the validation is called before the execution.
// Refer to ICS-20 `process_recv_packet_execute()`.

let class_uri = data
.class_uri
.as_ref()
.ok_or((ModuleExtras::empty(), NftTransferError::NftClassNotFound))?;
let class_data = data
.class_data
.as_ref()
.ok_or((ModuleExtras::empty(), NftTransferError::NftClassNotFound))?;
ctx_b
.create_or_update_class_validate(&class_id, class_uri, class_data)
.create_or_update_class_validate(
&class_id,
data.class_uri.as_ref(),
data.class_data.as_ref(),
)
.map_err(|nft_error| (ModuleExtras::empty(), nft_error))?;
ctx_b
.create_or_update_class_execute(&class_id, class_uri, class_data)
.create_or_update_class_execute(
&class_id,
data.class_uri.as_ref(),
data.class_data.as_ref(),
)
.map_err(|nft_error| (ModuleExtras::empty(), nft_error))?;

ctx_b
Expand Down
72 changes: 55 additions & 17 deletions ibc-apps/ics721-nft-transfer/src/handler/send_transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,12 @@ where
let class_id = &packet_data.class_id;
let token_ids = &packet_data.token_ids;
// overwrite even if they are set in MsgTransfer
packet_data.token_uris.clear();
packet_data.token_data.clear();
if let Some(uris) = &mut packet_data.token_uris {
uris.clear();
}
if let Some(data) = &mut packet_data.token_data {
data.clear();
}
for token_id in token_ids.as_ref() {
if is_sender_chain_source(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone(), class_id) {
transfer_ctx.escrow_nft_validate(
Expand All @@ -77,19 +81,35 @@ where
&msg.chan_id_on_a,
class_id,
token_id,
&packet_data.memo,
&packet_data.memo.clone().unwrap_or_default(),
)?;
} else {
transfer_ctx.burn_nft_validate(&sender, class_id, token_id, &packet_data.memo)?;
transfer_ctx.burn_nft_validate(
&sender,
class_id,
token_id,
&packet_data.memo.clone().unwrap_or_default(),
)?;
}
let nft = transfer_ctx.get_nft(class_id, token_id)?;
packet_data.token_uris.push(nft.get_uri().clone());
packet_data.token_data.push(nft.get_data().clone());
// Set the URI and the data if both exists
if let (Some(uri), Some(data)) = (nft.get_uri(), nft.get_data()) {
match &mut packet_data.token_uris {
Some(uris) => uris.push(uri.clone()),
None => packet_data.token_uris = Some(vec![uri.clone()]),
}
match &mut packet_data.token_data {
Some(token_data) => token_data.push(data.clone()),
None => packet_data.token_data = Some(vec![data.clone()]),
}
}
}

packet_data.validate_basic()?;

let nft_class = transfer_ctx.get_nft_class(class_id)?;
packet_data.class_uri = Some(nft_class.get_uri().clone());
packet_data.class_data = Some(nft_class.get_data().clone());
packet_data.class_uri = nft_class.get_uri().cloned();
packet_data.class_data = nft_class.get_data().cloned();

let packet = {
let data = serde_json::to_vec(&packet_data)
Expand Down Expand Up @@ -150,8 +170,12 @@ where
let class_id = &packet_data.class_id;
let token_ids = &packet_data.token_ids;
// overwrite even if they are set in MsgTransfer
packet_data.token_uris.clear();
packet_data.token_data.clear();
if let Some(uris) = &mut packet_data.token_uris {
uris.clear();
}
if let Some(data) = &mut packet_data.token_data {
data.clear();
}
for token_id in token_ids.as_ref() {
if is_sender_chain_source(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone(), class_id) {
transfer_ctx.escrow_nft_execute(
Expand All @@ -160,19 +184,33 @@ where
&msg.chan_id_on_a,
class_id,
token_id,
&packet_data.memo,
&packet_data.memo.clone().unwrap_or_default(),
)?;
} else {
transfer_ctx.burn_nft_execute(&sender, class_id, token_id, &packet_data.memo)?;
transfer_ctx.burn_nft_execute(
&sender,
class_id,
token_id,
&packet_data.memo.clone().unwrap_or_default(),
)?;
}
let nft = transfer_ctx.get_nft(class_id, token_id)?;
packet_data.token_uris.push(nft.get_uri().clone());
packet_data.token_data.push(nft.get_data().clone());
// Set the URI and the data if both exists
if let (Some(uri), Some(data)) = (nft.get_uri(), nft.get_data()) {
match &mut packet_data.token_uris {
Some(uris) => uris.push(uri.clone()),
None => packet_data.token_uris = Some(vec![uri.clone()]),
}
match &mut packet_data.token_data {
Some(token_data) => token_data.push(data.clone()),
None => packet_data.token_data = Some(vec![data.clone()]),
}
}
}

let nft_class = transfer_ctx.get_nft_class(class_id)?;
packet_data.class_uri = Some(nft_class.get_uri().clone());
packet_data.class_data = Some(nft_class.get_data().clone());
packet_data.class_uri = nft_class.get_uri().cloned();
packet_data.class_data = nft_class.get_data().cloned();

let packet = {
let data = {
Expand Down Expand Up @@ -204,7 +242,7 @@ where
receiver: packet_data.receiver,
class: packet_data.class_id,
tokens: packet_data.token_ids,
memo: packet_data.memo,
memo: packet_data.memo.unwrap_or_default(),
};
send_packet_ctx_a.emit_ibc_event(ModuleEvent::from(transfer_event).into())?;

Expand Down
6 changes: 3 additions & 3 deletions ibc-apps/ics721-nft-transfer/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ pub fn on_recv_packet_execute(
receiver: data.receiver,
class: data.class_id,
tokens: data.token_ids,
memo: data.memo,
memo: data.memo.unwrap_or_default(),
success: ack.is_successful(),
};
extras.events.push(recv_event.into());
Expand Down Expand Up @@ -259,7 +259,7 @@ pub fn on_acknowledgement_packet_execute(
receiver: data.receiver,
class: data.class_id,
tokens: data.token_ids,
memo: data.memo,
memo: data.memo.unwrap_or_default(),
acknowledgement: acknowledgement.clone(),
};

Expand Down Expand Up @@ -307,7 +307,7 @@ pub fn on_timeout_packet_execute(
refund_receiver: data.sender,
refund_class: data.class_id,
refund_tokens: data.token_ids,
memo: data.memo,
memo: data.memo.unwrap_or_default(),
};

let extras = ModuleExtras {
Expand Down
2 changes: 1 addition & 1 deletion ibc-apps/ics721-nft-transfer/types/src/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ impl FromStr for ClassUri {
)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq, derive_more::AsRef)]
pub struct ClassData(Data);

impl Display for ClassData {
Expand Down
Loading

0 comments on commit 1de2bd4

Please sign in to comment.