Cleanup error enum (#490)

wip

light-clients/cf-guest-cw/src/context.rs

@@ -73,22 +73,19 @@ impl<'a> Context<'a> {
 
 impl<'a> Context<'a> {
 	pub fn processed_timestamp(&self, height: Height) -> Result<u64, Error> {
-		let processed_state = ReadonlyProcessedStates::new(self.storage());
-		match processed_state.get_processed_time(height, &mut Vec::new()) {
-			Some(time) => Ok(time),
-			None => Err(Error::implementation_specific(
-				"problem getting processed timestamp".to_string(),
-			)),
-		}
+		ReadonlyProcessedStates::new(self.storage())
+			.get_processed_time(height, &mut Vec::new())
+			.ok_or_else(|| {
+				Error::implementation_specific("problem getting processed timestamp".into())
+			})
 	}
 
 	pub fn processed_height(&self, height: Height) -> Result<u64, Error> {
-		let processed_state = ReadonlyProcessedStates::new(self.storage());
-		match processed_state.get_processed_height(height, &mut Vec::new()) {
-			Some(p_height) => Ok(p_height),
-			None =>
-				Err(Error::implementation_specific("problem getting processed height".to_string())),
-		}
+		ReadonlyProcessedStates::new(self.storage())
+			.get_processed_height(height, &mut Vec::new())
+			.ok_or_else(|| {
+				Error::implementation_specific("problem getting processed height".into())
+			})
 	}
 
 	pub fn consensus_state_prefixed(
@@ -99,13 +96,12 @@ impl<'a> Context<'a> {
 		let bytes = ReadonlyConsensusStates::new(self.storage())
 			.get_prefixed(height, prefix)
 			.ok_or_else(|| {
-				ContractError::Tendermint(format!(
+				ContractError::Other(format!(
 					"no consensus state found for height {height} and prefix {prefix:?}",
 				))
 			})?;
-		Context::decode_consensus_state(&bytes).map_err(|e| {
-			ContractError::Tendermint(format!("error decoding consensus state: {e:?}"))
-		})
+		Context::decode_consensus_state(&bytes)
+			.map_err(|e| ContractError::Other(format!("error decoding consensus state: {e:?}")))
 	}
 
 	pub fn store_consensus_state_prefixed(
@@ -125,10 +121,10 @@ impl<'a> Context<'a> {
 	) -> Result<ClientState<crate::crypto::PubKey>, ContractError> {
 		let bytes =
 			ReadonlyClientStates::new(self.storage()).get_prefixed(prefix).ok_or_else(|| {
-				ContractError::Tendermint(format!("no client state found for prefix {prefix:?}",))
+				ContractError::Other(format!("no client state found for prefix {prefix:?}",))
 			})?;
 		Context::decode_client_state(&bytes)
-			.map_err(|e| ContractError::Tendermint(format!("error decoding client state: {e:?}")))
+			.map_err(|e| ContractError::Other(format!("error decoding client state: {e:?}")))
 	}
 
 	pub fn store_client_state_prefixed(
@@ -137,12 +133,11 @@ impl<'a> Context<'a> {
 		prefix: &[u8],
 	) -> Result<(), ContractError> {
 		let client_states = ReadonlyClientStates::new(self.storage());
-		let data = client_states.get_prefixed(prefix).ok_or_else(|| {
-			ContractError::Tendermint("no client state found for prefix".to_string())
-		})?;
-		let encoded = Context::encode_client_state(client_state, data).map_err(|e| {
-			ContractError::Tendermint(format!("error encoding client state: {e:?}"))
-		})?;
+		let data = client_states
+			.get_prefixed(prefix)
+			.ok_or_else(|| ContractError::Other("no client state found for prefix".to_string()))?;
+		let encoded = Context::encode_client_state(client_state, data)
+			.map_err(|e| ContractError::Other(format!("error encoding client state: {e:?}")))?;
 		let mut client_states = ClientStates::new(self.storage_mut());
 		client_states.insert_prefixed(encoded, prefix);
 		Ok(())

light-clients/cf-guest-cw/src/contract.rs

@@ -53,14 +53,10 @@ pub fn instantiate(
 	let _client = GuestClient::<crate::crypto::PubKey>::default();
 	let mut ctx = Context::new(deps, env);
 	let client_id = ClientId::from_str("08-wasm-0").expect("client id is valid");
-	let client_state = ctx
-		.client_state(&client_id)
-		.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+	let client_state = ctx.client_state(&client_id)?;
 	let latest_height = client_state.latest_height();
-	ctx.store_update_height(client_id.clone(), latest_height, ctx.host_height())
-		.map_err(|e| ContractError::Tendermint(e.to_string()))?;
-	ctx.store_update_time(client_id, latest_height, ctx.host_timestamp())
-		.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+	ctx.store_update_height(client_id.clone(), latest_height, ctx.host_height())?;
+	ctx.store_update_time(client_id, latest_height, ctx.host_timestamp())?;
 
 	Ok(Response::default())
 }
@@ -90,46 +86,35 @@ fn process_message(
 	//log!(ctx, "process_message: {:?}", msg);
 	let result = match msg {
 		ExecuteMsg::VerifyMembership(msg) => {
-			let client_state = ctx
-				.client_state(&client_id)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+			let client_state = ctx.client_state(&client_id)?;
 			let msg = VerifyMembershipMsg::try_from(msg)?;
 			crate::helpers::verify_delay_passed(
 				ctx,
 				msg.height,
 				msg.delay_time_period,
 				msg.delay_block_period,
-			)
-			.map_err(|e| ContractError::Tendermint(e.to_string()))?;
-			let consensus_state = ctx
-				.consensus_state(&client_id, msg.height)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+			)?;
+			let consensus_state = ctx.consensus_state(&client_id, msg.height)?;
 			// TODO(blas)
 			verify(
 				&CommitmentPrefix::default(),
 				&msg.proof,
 				&consensus_state.root(),
 				msg.path,
 				Some(msg.value.as_ref()),
-			)
-			.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+			)?;
 			Ok(()).map(|_| to_binary(&ContractResult::success()))
 		},
 		ExecuteMsg::VerifyNonMembership(msg) => {
-			let client_state = ctx
-				.client_state(&client_id)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+			let client_state = ctx.client_state(&client_id)?;
 			let msg = VerifyNonMembershipMsg::try_from(msg)?;
 			crate::helpers::verify_delay_passed(
 				ctx,
 				msg.height,
 				msg.delay_time_period,
 				msg.delay_block_period,
-			)
-			.map_err(|e| ContractError::Tendermint(e.to_string()))?;
-			let consensus_state = ctx
-				.consensus_state(&client_id, msg.height)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+			)?;
+			let consensus_state = ctx.consensus_state(&client_id, msg.height)?;
 
 			verify(
 				&CommitmentPrefix::default(),
@@ -138,47 +123,38 @@ fn process_message(
 				msg.path,
 				None,
 			)
-			.map_err(|e| ContractError::Tendermint(e.to_string()))
+			.map_err(ContractError::from)
 			.map(|_| to_binary(&ContractResult::success()))
 		},
 		ExecuteMsg::VerifyClientMessage(msg) => {
-			let client_state = ctx
-				.client_state(&client_id)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+			let client_state = ctx.client_state(&client_id)?;
 			let msg = VerifyClientMessage::try_from(msg)?;
 			client
 				.verify_client_message(ctx, client_id, client_state, msg.client_message)
-				.map_err(|e| ContractError::Tendermint(format!("{e:?}")))
+				.map_err(ContractError::from)
 				.map(|_| to_binary(&ContractResult::success()))
 		},
 		ExecuteMsg::CheckForMisbehaviour(msg) => {
-			let client_state = ctx
-				.client_state(&client_id)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+			let client_state = ctx.client_state(&client_id)?;
 			let msg = CheckForMisbehaviourMsg::try_from(msg)?;
 			client
 				.check_for_misbehaviour(ctx, client_id, client_state, msg.client_message)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))
+				.map_err(ContractError::from)
 				.map(|result| to_binary(&ContractResult::success().misbehaviour(result)))
 		},
 		ExecuteMsg::UpdateStateOnMisbehaviour(msg_raw) => {
-			let client_state = ctx
-				.client_state(&client_id)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+			let client_state = ctx.client_state(&client_id)?;
 			let msg = UpdateStateOnMisbehaviourMsg::try_from(msg_raw)?;
 			client
 				.update_state_on_misbehaviour(client_state, msg.client_message)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))
+				.map_err(ContractError::from)
 				.and_then(|cs| {
-					ctx.store_client_state(client_id, cs)
-						.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+					ctx.store_client_state(client_id, cs)?;
 					Ok(to_binary(&ContractResult::success()))
 				})
 		},
 		ExecuteMsg::UpdateState(msg_raw) => {
-			let client_state = ctx
-				.client_state(&client_id)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+			let client_state = ctx.client_state(&client_id)?;
 			let msg = UpdateStateMsg::try_from(msg_raw)?;
 			let latest_revision_height = client_state.latest_height().revision_height;
 			helpers::prune_oldest_consensus_state(
@@ -188,23 +164,20 @@ fn process_message(
 			);
 			client
 				.update_state(ctx, client_id.clone(), client_state, msg.client_message)
-				.map_err(|e| ContractError::Tendermint(e.to_string()))
+				.map_err(ContractError::from)
 				.and_then(|(cs, cu)| {
 					let height = cs.latest_height();
 					match cu {
 						ConsensusUpdateResult::Single(cs) => {
-							ctx.store_consensus_state(client_id.clone(), height, cs)
-								.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+							ctx.store_consensus_state(client_id.clone(), height, cs)?;
 						},
 						ConsensusUpdateResult::Batch(css) =>
 							for (height, cs) in css {
-								ctx.store_consensus_state(client_id.clone(), height, cs)
-									.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+								ctx.store_consensus_state(client_id.clone(), height, cs)?;
 							},
 					}
 					if u64::from(cs.0.latest_height) > latest_revision_height {
-						ctx.store_client_state(client_id, cs)
-							.map_err(|e| ContractError::Tendermint(e.to_string()))?;
+						ctx.store_client_state(client_id, cs)?;
 					}
 					Ok(to_binary(&ContractResult::success()))
 				})

light-clients/cf-guest-cw/src/error.rs

@@ -13,44 +13,24 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use cf_guest::error::Error as CfGuestError;
 use cosmwasm_std::StdError;
-use derive_more::{Display, From};
 use std::error::Error;
 
-#[derive(From, Display, Debug)]
+#[derive(derive_more::From, derive_more::Display, Debug)]
 pub enum ContractError {
 	Std(StdError),
-	#[display(fmt = "Unauthorized")]
-	Unauthorized {},
-	// Add any other custom errors you like here.
-	// Look at https://docs.rs/thiserror/1.0.21/thiserror/ for details.
-	#[display(fmt = "Storage error")]
-	StorageError,
-	// TODO: use `ics07-tendermint`'s error type here
-	#[display(fmt = "Tendermint error: {_0}")]
-	#[from(ignore)]
-	Tendermint(String),
-	#[display(fmt = "Protobuf error: {_0}")]
-	Protobuf(ibc::protobuf::Error),
-	#[display(fmt = "IBC validation error: {_0}")]
-	Validation(ibc::core::ics24_host::error::ValidationError),
-	#[display(fmt = "IBC path error: {_0}")]
+
+	ProofVerification(cf_guest::proof::VerifyError),
+
+	Client(ibc::core::ics02_client::error::Error),
 	Path(ibc::core::ics24_host::path::PathError),
-	#[display(fmt = "IBC proof error: {_0}")]
 	Proof(ibc::proofs::ProofError),
-	#[display(fmt = "IBC commitment error: {_0}")]
 	Commitment(ibc::core::ics23_commitment::error::Error),
-	#[display(fmt = "Proto decode error: {_0}")]
-	ProtoDecode(prost::DecodeError),
-	#[display(fmt = "From UTF8 error: {_0}")]
-	FromUtf8(alloc::string::FromUtf8Error),
-}
+	Protobuf(ibc::protobuf::Error),
 
-impl Error for ContractError {}
+	Prost(prost::DecodeError),
 
-impl From<CfGuestError> for ContractError {
-	fn from(e: CfGuestError) -> Self {
-		ContractError::Tendermint(e.to_string())
-	}
+	Other(String),
 }
+
+impl Error for ContractError {}

light-clients/cf-guest-cw/src/msg.rs

@@ -358,9 +358,7 @@ impl TryFrom<VerifyUpgradeAndUpdateStateMsgRaw> for VerifyUpgradeAndUpdateStateM
 		let upgrade_client_state: ClientState<crate::crypto::PubKey> =
 			ClientState::decode_vec(&any.value)?;
 		if upgrade_client_state.0.is_frozen {
-			return ibc::prelude::Err(ContractError::Tendermint(
-				"Upgrade client state not zeroed".to_string(),
-			))
+			return Err(ContractError::Other("Upgrade client state not zeroed".into()))
 		}
 
 		Ok(VerifyUpgradeAndUpdateStateMsg {

light-clients/cf-guest/src/error.rs

@@ -17,11 +17,7 @@ use alloc::{
 	fmt,
 	string::{String, ToString},
 };
-use ibc::{
-	core::{ics02_client::error::Error as Ics02Error, ics24_host::identifier::ClientId},
-	timestamp::Timestamp,
-	Height,
-};
+use ibc::{core::ics24_host::identifier::ClientId, timestamp::Timestamp, Height};
 
 #[derive(Clone, Debug)]
 pub enum Error {
@@ -35,13 +31,13 @@ pub enum Error {
 }
 
 impl fmt::Display for Error {
-	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		write!(f, "{:?}", self)
+	fn fmt(&self, fmtr: &mut fmt::Formatter) -> fmt::Result {
+		fmt::Debug::fmt(self, fmtr)
 	}
 }
 
-impl From<Error> for Ics02Error {
-	fn from(e: Error) -> Self {
-		Ics02Error::client_error(CLIENT_TYPE.to_string(), e.to_string())
+impl From<Error> for ibc::core::ics02_client::error::Error {
+	fn from(err: Error) -> Self {
+		Self::client_error(CLIENT_TYPE.into(), err.to_string())
 	}
 }