Merge #1756

1756: Remove Unknown variants r=tcharding a=tcharding

Currently we have `Unknown` variants in `AssetKind`, `LedgerKind`, and
`SwapProtocol`.  These are unnecessary if we handle the errors during
de/serialization.

Resolves: #1330 

Co-authored-by: Tobin C. Harding <[email protected]>

Author: bors[bot]

cnd/src/comit_api/mod.rs

@@ -11,11 +11,6 @@ use crate::{
 use bitcoin::util::amount::Denomination;
 use libp2p_comit::frame::Header;
 use serde::de::Error;
-use std::fmt;
-
-fn fail_serialize_unknown<D: fmt::Debug>(unknown: D) -> serde_json::Error {
-    ::serde::de::Error::custom(format!("serialization of {:?} is undefined.", unknown))
-}
 
 impl FromHeader for LedgerKind {
     fn from_header(mut header: Header) -> Result<Self, serde_json::Error> {
@@ -33,7 +28,12 @@ impl FromHeader for LedgerKind {
                 },
             )),
             "ethereum" => LedgerKind::Ethereum(Ethereum::new(header.take_parameter("network")?)),
-            other => LedgerKind::Unknown(other.to_string()),
+            unknown => {
+                return Err(serde_json::Error::custom(format!(
+                    "unknown ledger: {}",
+                    unknown
+                )))
+            }
         })
     }
 }
@@ -52,7 +52,6 @@ impl ToHeader for LedgerKind {
             LedgerKind::Ethereum(ethereum) => {
                 Header::with_str_value("ethereum").with_parameter("network", ethereum.chain_id)?
             }
-            unknown @ LedgerKind::Unknown(_) => return Err(fail_serialize_unknown(unknown)),
         })
     }
 }
@@ -73,7 +72,12 @@ impl FromHeader for SwapProtocol {
     fn from_header(mut header: Header) -> Result<Self, serde_json::Error> {
         Ok(match header.value::<String>()?.as_str() {
             "comit-rfc-003" => SwapProtocol::Rfc003(header.take_parameter("hash_function")?),
-            other => SwapProtocol::Unknown(other.to_string()),
+            unknown => {
+                return Err(serde_json::Error::custom(format!(
+                    "unknown swap protocol: {}",
+                    unknown
+                )))
+            }
         })
     }
 }
@@ -83,7 +87,6 @@ impl ToHeader for SwapProtocol {
         Ok(match self {
             SwapProtocol::Rfc003(hash_function) => Header::with_str_value("comit-rfc-003")
                 .with_parameter("hash_function", hash_function)?,
-            unknown @ SwapProtocol::Unknown(_) => return Err(fail_serialize_unknown(unknown)),
         })
     }
 }
@@ -103,7 +106,12 @@ impl FromHeader for AssetKind {
                 header.take_parameter("address")?,
                 header.take_parameter("quantity")?,
             )),
-            other => AssetKind::Unknown(other.to_string()),
+            unknown => {
+                return Err(serde_json::Error::custom(format!(
+                    "unknown asset: {}",
+                    unknown
+                )))
+            }
         })
     }
 }
@@ -119,7 +127,6 @@ impl ToHeader for AssetKind {
             AssetKind::Erc20(erc20) => Header::with_str_value("erc20")
                 .with_parameter("address", erc20.token_contract)?
                 .with_parameter("quantity", erc20.quantity)?,
-            unknown @ AssetKind::Unknown(_) => return Err(fail_serialize_unknown(unknown)),
         })
     }
 }
@@ -151,7 +158,6 @@ mod tests {
         swap_protocols::{ledger::ethereum, HashFunction},
     };
     use bitcoin::Amount;
-    use spectral::prelude::*;
 
     #[test]
     fn erc20_quantity_to_header() -> Result<(), serde_json::Error> {
@@ -171,15 +177,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn serializing_unknown_ledgerkind_doesnt_panic() {
-        let ledger_kind = LedgerKind::Unknown("USD".to_string());
-
-        let header = ledger_kind.to_header();
-
-        assert_that(&header).is_err();
-    }
-
     #[test]
     fn swap_protocol_to_header() {
         // From comit-network/RFCs/RFC-003-SWAP-Basic.md SWAP REQUEST example.

cnd/src/db/swap_types.rs

@@ -130,9 +130,6 @@ impl From<ledger::LedgerKind> for LedgerKind {
         match ledger {
             ledger::LedgerKind::Bitcoin(_) => LedgerKind::Bitcoin,
             ledger::LedgerKind::Ethereum(_) => LedgerKind::Ethereum,
-            // In order to remove this ledger::LedgerKind::Unknown should be removed.
-            // Doing so requires handling unknown ledger during deserialization.
-            _ => unreachable!(),
         }
     }
 }
@@ -150,9 +147,6 @@ impl From<asset::AssetKind> for AssetKind {
             asset::AssetKind::Bitcoin(_) => AssetKind::Bitcoin,
             asset::AssetKind::Ether(_) => AssetKind::Ether,
             asset::AssetKind::Erc20(_) => AssetKind::Erc20,
-            // In order to remove this ledger::AssetKind::Unknown should be removed.
-            // Doing so requires handling unknown asset during deserialization.
-            _ => unreachable!(),
         }
     }
 }

cnd/src/http_api/mod.rs

@@ -108,7 +108,6 @@ impl Serialize for Http<SwapProtocol> {
         match &self.0 {
             // Currently we do not expose the hash_function protocol parameter via REST.
             SwapProtocol::Rfc003(_hash_function) => serializer.serialize_str("rfc003"),
-            SwapProtocol::Unknown(name) => serializer.serialize_str(name.as_str()),
         }
     }
 }

cnd/src/network/mod.rs

@@ -292,25 +292,6 @@ async fn handle_request(
                         }
                     }
                 }
-                SwapProtocol::Unknown(protocol) => {
-                    log::warn!("the swap protocol {} is currently not supported", protocol);
-
-                    let decline_body = DeclineResponseBody {
-                        reason: Some(SwapDeclineReason::UnsupportedProtocol),
-                    };
-                    Err(Response::empty()
-                        .with_header(
-                            "decision",
-                            Decision::Declined
-                                .to_header()
-                                .expect("Decision should not fail to serialize"),
-                        )
-                        .with_body(
-                            serde_json::to_value(decline_body).expect(
-                                "decline body should always serialize into serde_json::Value",
-                            ),
-                        ))
-                }
             }
         }
 

cnd/src/swap_protocols/asset.rs

@@ -34,7 +34,6 @@ pub enum AssetKind {
     Bitcoin(Amount),
     Ether(EtherQuantity),
     Erc20(Erc20Token),
-    Unknown(String),
 }
 
 impl From<Amount> for AssetKind {

cnd/src/swap_protocols/ledger/mod.rs

@@ -36,5 +36,4 @@ pub trait Ledger:
 pub enum LedgerKind {
     Bitcoin(Bitcoin),
     Ethereum(Ethereum),
-    Unknown(String),
 }

cnd/src/swap_protocols/mod.rs

@@ -36,7 +36,6 @@ pub enum HashFunction {
 #[derive(Debug)]
 pub enum SwapProtocol {
     Rfc003(HashFunction),
-    Unknown(String),
 }
 
 #[derive(Clone, Copy, Debug, Display, EnumString, PartialEq)]