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

modules: Clean up modules' errors #1334

Merged
merged 4 commits into from
Sep 14, 2021
Merged

modules: Clean up modules' errors #1334

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
5e911ec
clean up redundant errors
yito88 Sep 7, 2021
ac991d5
add a log
yito88 Sep 8, 2021
bca1904
remove ics02_error from ics04_error
yito88 Sep 9, 2021
ceddb00
fix a test to check the correct error
yito88 Sep 9, 2021
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
9 changes: 9 additions & 0 deletions .changelog/unreleased/improvements/ibc/1333-modules-error.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!--
Add your entry's details here (in Markdown format).

If you don't change this message, or if this file is empty, the entry will
not be created.
-->
- Clean up modules' errors ([#1333])

[#1333]: https://github.com/informalsystems/ibc-rs/issues/1333
8 changes: 0 additions & 8 deletions modules/src/application/ics20_fungible_token_transfer/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,6 @@ define_error! {
[ channel_error::Error ]
|_ | { "Ics04 channel error" },

SequenceSendNotFound
{ port_id: PortId, channel_id: ChannelId }
| e | { format_args!("sending sequence number not found for port {0} and channel {1}", e.port_id, e.channel_id) },

ChannelNotFound
{ port_id: PortId, channel_id: ChannelId }
| e | { format_args!("sending sequence number not found for port {0} and channel {1}", e.port_id, e.channel_id) },

DestinationChannelNotFound
{ port_id: PortId, channel_id: ChannelId }
| e | { format_args!("destination channel not found in the counterparty of port_id {0} and channel_id {1} ", e.port_id, e.channel_id) },
Expand Down
24 changes: 0 additions & 24 deletions modules/src/ics03_connection/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ define_error! {
e.connection_id)
},

UninitializedConnection
{ connection_id: ConnectionId }
| e | {
format_args!("connection end for identifier {0} was never initialized",
e.connection_id)
},

InvalidConsensusHeight
{
target_height: Height,
Expand Down Expand Up @@ -120,13 +113,6 @@ define_error! {
MissingCounterpartyPrefix
| _ | { "missing counterparty prefix" },

MissingClient
{ client_id: ClientId }
| e | {
format_args!("the client id does not match any client state: {0}",
e.client_id)
},

NullClientProof
| _ | { "client proof must be present" },

Expand All @@ -140,16 +126,6 @@ define_error! {
ConnectionVerificationFailure
| _ | { "the connection proof verification failed" },

MissingClientConsensusState
{
height: Height,
client_id: ClientId,
}
| e | {
format_args!("the consensus state at height {0} for client id {1} could not be retrieved",
e.height, e.client_id)
},

MissingLocalConsensusState
{ height: Height }
| e | { format_args!("the local consensus state could not be retrieved for height {}", e.height) },
Expand Down
50 changes: 22 additions & 28 deletions modules/src/ics04_channel/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::packet::Sequence;
use crate::ics02_client::error as client_error;
use crate::ics03_connection::error as connection_error;
use crate::ics04_channel::channel::State;
use crate::ics24_host::error::ValidationError;
use crate::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId};
Expand All @@ -12,6 +13,10 @@ use tendermint_proto::Error as TendermintError;
define_error! {
#[derive(Debug, PartialEq, Eq)]
Error {
Ics03Connection
[ connection_error::Error ]
| _ | { "ics03 connection error" },

UnknownState
{ state: i32 }
| e | { format_args!("channel state unknown: {}", e.state) },
Expand Down Expand Up @@ -58,7 +63,12 @@ define_error! {
| _ | { "invalid proof: missing height" },

MissingNextRecvSeq
| _ | { "Missing sequence number for receiving packets" },
{ port_channel_id: (PortId, ChannelId) }
| e | {
format_args!("Missing sequence number for receiving packets on port {0} and channel {1}",
e.port_channel_id.0,
e.port_channel_id.1)
},

ZeroPacketSequence
| _ | { "packet sequence cannot be 0" },
Expand Down Expand Up @@ -95,14 +105,6 @@ define_error! {
MissingChannel
| _ | { "missing channel end" },

MissingConnection
{ connection_id: ConnectionId }
| e | {
format_args!(
"given connection hop {0} does not exist",
e.connection_id)
},

NoPortCapability
{ port_id: PortId }
| e | {
Expand Down Expand Up @@ -178,17 +180,14 @@ define_error! {
e.sequence)
},

MissingClientState
{ client_id: ClientId }
MissingNextSendSeq
{ port_channel_id: (PortId, ChannelId) }
| e | {
format_args!(
"No client state associated with client id {0}",
e.client_id)
format_args!("Missing sequence number for sending packets on port {0} and channel {1}",
e.port_channel_id.0,
e.port_channel_id.1)
},

MissingNextSendSeq
| _ | { "Missing sequence number for send packets" },

InvalidStringAsSequence
{ value: String }
[ TraceError<std::num::ParseIntError> ]
Expand Down Expand Up @@ -259,21 +258,11 @@ define_error! {
"Client with id {0} is frozen",
e.client_id)
},
MissingClientConsensusState
{ client_id: ClientId, height: Height }
| e | {
format_args!(
"Missing client consensus state for client id {0} at height {1}",
e.client_id, e.height)
},

InvalidCounterpartyChannelId
[ ValidationError ]
| _ | { "Invalid channel id in counterparty" },

ClientNotFound
| _ | { "Client not found in chan open verification" },

InvalidChannelState
{ channel_id: ChannelId, state: State }
| e | {
Expand Down Expand Up @@ -326,7 +315,12 @@ define_error! {
},

MissingNextAckSeq
| _ | { "Missing sequence number for ack packets" },
{ port_channel_id: (PortId, ChannelId) }
| e | {
format_args!("Missing sequence number for ack packets on port {0} and channel {1}",
e.port_channel_id.0,
e.port_channel_id.1)
},

ImplementationSpecific
| _ | { "implementation specific error" },
Expand Down
26 changes: 23 additions & 3 deletions modules/src/ics04_channel/handler/chan_open_try.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,11 @@ mod tests {

use crate::events::IbcEvent;
use crate::ics02_client::client_type::ClientType;
use crate::ics02_client::error as ics02_error;
use crate::ics03_connection::connection::ConnectionEnd;
use crate::ics03_connection::connection::Counterparty as ConnectionCounterparty;
use crate::ics03_connection::connection::State as ConnectionState;
use crate::ics03_connection::error as ics03_error;
use crate::ics03_connection::msgs::test_util::get_dummy_raw_counterparty;
use crate::ics03_connection::version::get_compatible_versions;
use crate::ics04_channel::channel::{ChannelEnd, State};
Expand Down Expand Up @@ -273,8 +275,13 @@ mod tests {
match_error: {
let connection_id = msg.channel().connection_hops()[0].clone();
Box::new(move |e| match e {
error::ErrorDetail::MissingConnection(e) => {
assert_eq!(e.connection_id, connection_id);
error::ErrorDetail::Ics03Connection(e) => {
assert_eq!(
e.source,
ics03_error::ErrorDetail::ConnectionNotFound(
ics03_error::ConnectionNotFoundSubdetail { connection_id }
)
);
}
_ => {
panic!("Expected MissingConnection, instead got {}", e)
Expand Down Expand Up @@ -363,7 +370,20 @@ mod tests {
msg: ChannelMsg::ChannelOpenTry(msg.clone()),
want_pass: false,
match_error: Box::new(|e| match e {
error::ErrorDetail::MissingClientState(_) => {}
error::ErrorDetail::Ics03Connection(e) => {
assert_eq!(
e.source,
ics03_error::ErrorDetail::Ics02Client(
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit of a mouthful, but on the other hand, it's good we can decompose the error here and assert on the precise details. This is just a remark, not a suggestion for any change.

ics03_error::Ics02ClientSubdetail {
source: ics02_error::ErrorDetail::ClientNotFound(
ics02_error::ClientNotFoundSubdetail {
client_id: ClientId::new(ClientType::Mock, 45).unwrap()
}
)
}
)
);
}
_ => {
panic!("Expected MissingClientState, instead got {}", e)
}
Expand Down
20 changes: 8 additions & 12 deletions modules/src/mock/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,10 +461,7 @@ impl ChannelReader for MockContext {
}

fn connection_end(&self, cid: &ConnectionId) -> Result<ConnectionEnd, Ics04Error> {
match self.connections.get(cid) {
Some(connection_end) => Ok(connection_end.clone()),
None => Err(Ics04Error::missing_connection(cid.clone())),
}
ConnectionReader::connection_end(self, cid).map_err(Ics04Error::ics03_connection)
}

fn connection_channels(
Expand All @@ -479,7 +476,7 @@ impl ChannelReader for MockContext {

fn client_state(&self, client_id: &ClientId) -> Result<AnyClientState, Ics04Error> {
ClientReader::client_state(self, client_id)
.map_err(|_| Ics04Error::missing_client_state(client_id.clone()))
.map_err(|e| Ics04Error::ics03_connection(Ics03Error::ics02_client(e)))
}

fn client_consensus_state(
Expand All @@ -488,7 +485,7 @@ impl ChannelReader for MockContext {
height: Height,
) -> Result<AnyConsensusState, Ics04Error> {
ClientReader::consensus_state(self, client_id, height)
.map_err(|_| Ics04Error::missing_client_consensus_state(client_id.clone(), height))
.map_err(|e| Ics04Error::ics03_connection(Ics03Error::ics02_client(e)))
}

fn authenticated_capability(&self, port_id: &PortId) -> Result<Capability, Ics04Error> {
Expand All @@ -513,7 +510,7 @@ impl ChannelReader for MockContext {
) -> Result<Sequence, Ics04Error> {
match self.next_sequence_send.get(port_channel_id) {
Some(sequence) => Ok(*sequence),
None => Err(Ics04Error::missing_next_send_seq()),
None => Err(Ics04Error::missing_next_send_seq(port_channel_id.clone())),
}
}

Expand All @@ -523,7 +520,7 @@ impl ChannelReader for MockContext {
) -> Result<Sequence, Ics04Error> {
match self.next_sequence_recv.get(port_channel_id) {
Some(sequence) => Ok(*sequence),
None => Err(Ics04Error::missing_next_recv_seq()),
None => Err(Ics04Error::missing_next_recv_seq(port_channel_id.clone())),
}
}

Expand All @@ -533,7 +530,7 @@ impl ChannelReader for MockContext {
) -> Result<Sequence, Ics04Error> {
match self.next_sequence_ack.get(port_channel_id) {
Some(sequence) => Ok(*sequence),
None => Err(Ics04Error::missing_next_ack_seq()),
None => Err(Ics04Error::missing_next_ack_seq(port_channel_id.clone())),
}
}

Expand Down Expand Up @@ -698,8 +695,7 @@ impl ConnectionReader for MockContext {

fn client_state(&self, client_id: &ClientId) -> Result<AnyClientState, Ics03Error> {
// Forward method call to the Ics2 Client-specific method.
ClientReader::client_state(self, client_id)
.map_err(|_| Ics03Error::missing_client(client_id.clone()))
ClientReader::client_state(self, client_id).map_err(Ics03Error::ics02_client)
}

fn host_current_height(&self) -> Height {
Expand All @@ -722,7 +718,7 @@ impl ConnectionReader for MockContext {
) -> Result<AnyConsensusState, Ics03Error> {
// Forward method call to the Ics2Client-specific method.
self.consensus_state(client_id, height)
.map_err(|_| Ics03Error::missing_client_consensus_state(height, client_id.clone()))
.map_err(Ics03Error::ics02_client)
}

fn host_consensus_state(&self, height: Height) -> Result<AnyConsensusState, Ics03Error> {
Expand Down
16 changes: 4 additions & 12 deletions modules/tests/runner/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,10 @@ impl modelator::step_runner::StepRunner<Step> for IbcTestRunner {
Self::extract_ics02_error_kind(result),
client_error::ErrorDetail::ClientNotFound(_)
),
ActionOutcome::Ics02ConsensusStateNotFound => matches!(
Self::extract_ics02_error_kind(result),
client_error::ErrorDetail::ConsensusStateNotFound(_)
),
ActionOutcome::Ics02HeaderVerificationFailure => {
matches!(
Self::extract_ics02_error_kind(result),
Expand All @@ -498,10 +502,6 @@ impl modelator::step_runner::StepRunner<Step> for IbcTestRunner {
)
}
ActionOutcome::Ics03ConnectionOpenInitOk => result.is_ok(),
ActionOutcome::Ics03MissingClient => matches!(
Self::extract_ics03_error_kind(result),
connection_error::ErrorDetail::MissingClient(_)
),
ActionOutcome::Ics03ConnectionOpenTryOk => result.is_ok(),
ActionOutcome::Ics03InvalidConsensusHeight => matches!(
Self::extract_ics03_error_kind(result),
Expand All @@ -515,19 +515,11 @@ impl modelator::step_runner::StepRunner<Step> for IbcTestRunner {
Self::extract_ics03_error_kind(result),
connection_error::ErrorDetail::ConnectionMismatch(_)
),
ActionOutcome::Ics03MissingClientConsensusState => matches!(
Self::extract_ics03_error_kind(result),
connection_error::ErrorDetail::MissingClientConsensusState(_)
),
ActionOutcome::Ics03InvalidProof => matches!(
Self::extract_ics03_error_kind(result),
connection_error::ErrorDetail::InvalidProof(_)
),
ActionOutcome::Ics03ConnectionOpenAckOk => result.is_ok(),
ActionOutcome::Ics03UninitializedConnection => matches!(
Self::extract_ics03_error_kind(result),
connection_error::ErrorDetail::UninitializedConnection(_)
),
ActionOutcome::Ics03ConnectionOpenConfirmOk => result.is_ok(),
};

Expand Down
4 changes: 1 addition & 3 deletions modules/tests/runner/step.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,22 +124,20 @@ pub enum ActionOutcome {
Ics02CreateOk,
Ics02UpdateOk,
Ics02ClientNotFound,
Ics02ConsensusStateNotFound,
Ics02HeaderVerificationFailure,

Ics07UpgradeOk,
Ics07ClientNotFound,
Ics07HeaderVerificationFailure,

Ics03ConnectionOpenInitOk,
Ics03MissingClient,
Ics03ConnectionOpenTryOk,
Ics03InvalidConsensusHeight,
Ics03ConnectionNotFound,
Ics03ConnectionMismatch,
Ics03MissingClientConsensusState,
Ics03InvalidProof,
Ics03ConnectionOpenAckOk,
Ics03UninitializedConnection,
Ics03ConnectionOpenConfirmOk,
}

Expand Down
2 changes: 0 additions & 2 deletions modules/tests/support/model_based/IBC.tla
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,11 @@ ActionOutcomes == {
"Ics07HeaderVerificationFailure",
\* ICS03_ConnectionOpenInit outcomes:
"Ics03ConnectionOpenInitOk",
"Ics03MissingClient",
\* ICS03_ConnectionOpenTry outcomes:
"Ics03ConnectionOpenTryOk",
"Ics03InvalidConsensusHeight",
"Ics03ConnectionNotFound",
"Ics03ConnectionMismatch",
"Ics03MissingClientConsensusState",
"Ics03InvalidProof",
\* ICS03_ConnectionOpenAck outcomes:
"Ics03ConnectionOpenAckOk",
Expand Down
Loading