Skip to content

Commit

Permalink
Remove NftTransferError variants in favor of DecodingError::InvalidJson
Browse files Browse the repository at this point in the history
  • Loading branch information
seanchen1991 committed Sep 3, 2024
1 parent 2952291 commit 3ffdf2f
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 25 deletions.
6 changes: 3 additions & 3 deletions ibc-apps/ics20-transfer/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ mod test {
}
.into(),
),
r#"{"error":"failed to deserialize packet data"}"#,
r#"{"error":"invalid JSON data: `failed to deserialize packet data`"}"#,
);
}

Expand Down Expand Up @@ -381,7 +381,7 @@ mod test {
// which would make the conversion to `Acknowledgement` panic
assert_eq!(
ack_error,
br#"{"error":"failed to deserialize packet data"}"#
br#"{"error":"invalid JSON data: `failed to deserialize packet data`"}"#
);
}

Expand All @@ -397,7 +397,7 @@ mod test {
AcknowledgementStatus::success(ack_success_b64()),
);
de_json_assert_eq(
r#"{"error":"failed to deserialize packet data"}"#,
r#"{"error":"invalid JSON data: `failed to deserialize packet data`"}"#,
AcknowledgementStatus::error(
DecodingError::InvalidJson {
description: "failed to deserialize packet data".to_string(),
Expand Down
72 changes: 54 additions & 18 deletions ibc-apps/ics721-nft-transfer/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use ibc_core::channel::types::channel::{Counterparty, Order};
use ibc_core::channel::types::packet::Packet;
use ibc_core::channel::types::Version;
use ibc_core::handler::types::error::ContextError;
use ibc_core::host::types::error::DecodingError;
use ibc_core::host::types::identifiers::{ChannelId, ConnectionId, PortId};
use ibc_core::primitives::prelude::*;
use ibc_core::primitives::Signer;
Expand Down Expand Up @@ -172,8 +173,12 @@ pub fn on_recv_packet_execute(
packet: &Packet,
) -> (ModuleExtras, Acknowledgement) {
let Ok(data) = serde_json::from_slice::<PacketData>(&packet.data) else {
let ack =
AcknowledgementStatus::error(NftTransferError::FailedToDeserializePacketData.into());
let ack = AcknowledgementStatus::error(
DecodingError::InvalidJson {
description: "failed to deserialize packet data".to_string(),
}
.into(),
);

Check warning on line 181 in ibc-apps/ics721-nft-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics721-nft-transfer/src/module.rs#L176-L181

Added lines #L176 - L181 were not covered by tests
return (ModuleExtras::empty(), ack.into());
};

Expand Down Expand Up @@ -204,11 +209,16 @@ pub fn on_acknowledgement_packet_validate(
acknowledgement: &Acknowledgement,
_relayer: &Signer,
) -> Result<(), NftTransferError> {
let data = serde_json::from_slice::<PacketData>(&packet.data)
.map_err(|_| NftTransferError::FailedToDeserializePacketData)?;
let data = serde_json::from_slice::<PacketData>(&packet.data).map_err(|e| {
DecodingError::InvalidJson {
description: format!("failed to deserialize packet data: {e}"),
}
})?;

Check warning on line 216 in ibc-apps/ics721-nft-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics721-nft-transfer/src/module.rs#L212-L216

Added lines #L212 - L216 were not covered by tests

let acknowledgement = serde_json::from_slice::<AcknowledgementStatus>(acknowledgement.as_ref())
.map_err(|_| NftTransferError::FailedToDeserializeAck)?;
.map_err(|e| DecodingError::InvalidJson {
description: format!("failed to deserialize acknowledgment: {e}"),
})?;

Check warning on line 221 in ibc-apps/ics721-nft-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics721-nft-transfer/src/module.rs#L219-L221

Added lines #L219 - L221 were not covered by tests

if !acknowledgement.is_successful() {
refund_packet_nft_validate(ctx, packet, &data)?;
Expand All @@ -226,7 +236,10 @@ pub fn on_acknowledgement_packet_execute(
let Ok(data) = serde_json::from_slice::<PacketData>(&packet.data) else {
return (
ModuleExtras::empty(),
Err(NftTransferError::FailedToDeserializePacketData),
Err(DecodingError::InvalidJson {
description: "failed to deserialize packet data".to_string(),
}
.into()),

Check warning on line 242 in ibc-apps/ics721-nft-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics721-nft-transfer/src/module.rs#L239-L242

Added lines #L239 - L242 were not covered by tests
);
};

Expand All @@ -235,7 +248,10 @@ pub fn on_acknowledgement_packet_execute(
else {
return (
ModuleExtras::empty(),
Err(NftTransferError::FailedToDeserializeAck),
Err(DecodingError::InvalidJson {
description: "failed to deserialize acknowledgment".to_string(),
}
.into()),

Check warning on line 254 in ibc-apps/ics721-nft-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics721-nft-transfer/src/module.rs#L251-L254

Added lines #L251 - L254 were not covered by tests
);
};

Expand Down Expand Up @@ -267,8 +283,11 @@ pub fn on_timeout_packet_validate(
packet: &Packet,
_relayer: &Signer,
) -> Result<(), NftTransferError> {
let data = serde_json::from_slice::<PacketData>(&packet.data)
.map_err(|_| NftTransferError::FailedToDeserializePacketData)?;
let data = serde_json::from_slice::<PacketData>(&packet.data).map_err(|e| {
DecodingError::InvalidJson {
description: format!("failed to deserialize packet data: {e}"),
}
})?;

Check warning on line 290 in ibc-apps/ics721-nft-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics721-nft-transfer/src/module.rs#L286-L290

Added lines #L286 - L290 were not covered by tests

refund_packet_nft_validate(ctx, packet, &data)?;

Expand All @@ -283,7 +302,10 @@ pub fn on_timeout_packet_execute(
let Ok(data) = serde_json::from_slice::<PacketData>(&packet.data) else {
return (
ModuleExtras::empty(),
Err(NftTransferError::FailedToDeserializePacketData),
Err(DecodingError::InvalidJson {
description: "failed to deserialize packet data".to_string(),
}
.into()),

Check warning on line 308 in ibc-apps/ics721-nft-transfer/src/module.rs

View check run for this annotation

Codecov / codecov/patch

ibc-apps/ics721-nft-transfer/src/module.rs#L305-L308

Added lines #L305 - L308 were not covered by tests
);
};

Expand Down Expand Up @@ -322,8 +344,13 @@ mod test {
r#"{"result":"AQ=="}"#,
);
ser_json_assert_eq(
AcknowledgementStatus::error(NftTransferError::FailedToDeserializePacketData.into()),
r#"{"error":"failed to deserialize packet data"}"#,
AcknowledgementStatus::error(
DecodingError::InvalidJson {
description: "failed to deserialize packet data".to_string(),
}
.into(),
),
r#"{"error":"invalid JSON data: `failed to deserialize packet data`"}"#,
);
}

Expand All @@ -339,16 +366,20 @@ mod test {

#[test]
fn test_ack_error_to_vec() {
let ack_error: Vec<u8> =
AcknowledgementStatus::error(NftTransferError::FailedToDeserializePacketData.into())
.into();
let ack_error: Vec<u8> = AcknowledgementStatus::error(
DecodingError::InvalidJson {
description: "failed to deserialize packet data".to_string(),
}
.into(),
)
.into();

// Check that it's the same output as ibc-go
// Note: this also implicitly checks that the ack bytes are non-empty,
// which would make the conversion to `Acknowledgement` panic
assert_eq!(
ack_error,
br#"{"error":"failed to deserialize packet data"}"#
br#"{"error":"invalid JSON data: `failed to deserialize packet data`"}"#
);
}

Expand All @@ -364,8 +395,13 @@ mod test {
AcknowledgementStatus::success(ack_success_b64()),
);
de_json_assert_eq(
r#"{"error":"failed to deserialize packet data"}"#,
AcknowledgementStatus::error(NftTransferError::FailedToDeserializePacketData.into()),
r#"{"error":"invalid JSON data: `failed to deserialize packet data`"}"#,
AcknowledgementStatus::error(
DecodingError::InvalidJson {
description: "failed to deserialize packet data".to_string(),
}
.into(),
),
);

assert!(serde_json::from_str::<AcknowledgementStatus>(r#"{"success":"AQ=="}"#).is_err());
Expand Down
4 changes: 0 additions & 4 deletions ibc-apps/ics721-nft-transfer/types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@ pub enum NftTransferError {
MismatchedChannelOrders { expected: Order, actual: Order },
/// mismatched port IDs: expected `{expected}`, actual `{actual}`
MismatchedPortIds { expected: PortId, actual: PortId },
/// failed to deserialize packet data
FailedToDeserializePacketData,
/// failed to deserialize acknowledgement
FailedToDeserializeAck,
/// failed to parse account ID
FailedToParseAccount,
/// channel cannot be closed
Expand Down

0 comments on commit 3ffdf2f

Please sign in to comment.