Merge rust-bitcoin#4640: Remove non_exhausive from Network

bcf0c3d bitcoin: Add a test for matching on network (Tobin C. Harding)
1e69e63 Remove non_exhausive from Network (Tobin C. Harding)

Pull request description:

  Note that Kix is AWOL at the moment so we will need to let this sit for a good while so he gets a chance to see it since he is central to the debate in rust-bitcoin#2225.
  
  Issue rust-bitcoin#2225 is long and has many valid opposing opinions. The main argument for having non_exhaustive is that it helps future proof the ecosystem at the cost of pain now. The main argument against having non_exhaustive is why have pain now when adding a network is so rare that having pain then is ok.
  
  At the end of the thread Andrew posts:
  
  > I continue to think we should have an exhaustive enum, with a bunch of
  > documentation about how to use it properly. I am warming up to the
  > "don't have an enum, just have rules for defining your own" but I think
  > this would be needless work for people who just want to grab an
  > off-the-shelf set of networks or people who want to make their own enum
  > but want to see an example of how to do it first.
  
  In order to make some forward progress lets remove the `non_exhaustive` now and backport this change to 0.32, 0.31, an 0.30.
  
  Later we can add, and release in 0.33, whatever forward protection / libapocalyse protection we want to add.
  
  This removes the pain now and gives us a path to prevent future pain - that should keep all parties happy.


ACKs for top commit:
  apoelstra:
    ACK bcf0c3d; successfully ran local tests
  luisschwab:
    ACK bcf0c3d


Tree-SHA512: d8c24e8d2320b289aafdfa4b03a96407f40dd120dfcbd55973c35f62cd8f74eff8e0d961a7bb31ae7fa9dc4f6e68949c225bdbacbc1733042679c4913a2231b1

Author: apoelstra

bitcoin/src/network/mod.rs

@@ -59,9 +59,18 @@ pub enum TestnetVersion {
     V4,
 }
 
+
 /// The cryptocurrency network to act on.
+///
+/// This is an exhaustive enum, meaning that we cannot add any future networks without defining a
+/// new, incompatible version of this type. If you are using this type directly and wish to support the
+/// new network, this will be a breaking change to your APIs and likely require changes in your code.
+///
+/// If you are concerned about forward compatibility, consider using `T: Into<Params>` instead of this
+/// this type as a parameter to functions in your public API, or directly using the `Params` type.
+// For extensive discussion on the usage of `non_exhaustive` please see:
+// https://github.com/rust-bitcoin/rust-bitcoin/issues/2225
 #[derive(Copy, PartialEq, Eq, PartialOrd, Ord, Clone, Hash, Debug)]
-#[non_exhaustive]
 pub enum Network {
     /// Mainnet Bitcoin.
     Bitcoin,

bitcoin/tests/network.rs

@@ -0,0 +1,74 @@
+//! Tests what a user can do when pattern matching on `Network` and associated types.
+
+use bitcoin::network::{Network, NetworkKind, TestnetVersion};
+
+#[test]
+fn can_match_exhaustively_on_network() {
+    // Returns true if `n` is mainnet.
+    fn is_mainnet(n: Network) -> bool {
+        matches!(n, Network::Bitcoin)
+    }
+
+    assert!(is_mainnet(Network::Bitcoin));
+}
+
+#[test]
+fn can_match_exhaustively_on_testnet() {
+    // Returns true if `n` is any testnet.
+    fn is_testnet(n: Network) -> bool {
+        matches!(n, Network::Testnet(_))
+    }
+
+    assert!(is_testnet(Network::Testnet(TestnetVersion::V3)));
+}
+
+#[test]
+fn can_use_network_kind() {
+    // Returns true if `n` is any mainnet.
+    fn is_mainnet(n: Network) -> bool {
+        NetworkKind::from(n).is_mainnet()
+    }
+
+    // Returns true if `n` is a any testnet.
+    fn is_testnet(n: Network) -> bool {
+        !NetworkKind::from(n).is_mainnet()
+    }
+
+    assert!(is_mainnet(Network::Bitcoin));
+    assert!(!is_testnet(Network::Bitcoin));
+
+    assert!(is_testnet(Network::Testnet(TestnetVersion::V3)));
+    assert!(!is_mainnet(Network::Testnet(TestnetVersion::V3)));
+
+}
+
+#[test]
+fn can_not_match_exaustively_on_testnet_version() {
+    // Returns true if `n` is testnet version 3.
+    fn is_testnet_v3(n: Network) -> bool {
+        match n {
+            Network::Testnet(TestnetVersion::V3) => true,
+            Network::Testnet(TestnetVersion::V4) => false,
+            // Catchall because of `non_exhaustive` attribute.
+            Network::Testnet(_) => false,
+            _ => false,
+        }
+    }
+
+    assert!(is_testnet_v3(Network::Testnet(TestnetVersion::V3)));
+}
+
+#[test]
+fn can_match_on_testnet_version_3_only() {
+    // Returns true if `n` is testnet version 3.
+    fn is_testnet_v3(n: Network) -> bool {
+        match n {
+            Network::Testnet(TestnetVersion::V3) => true,
+            // Catchall because its logically correct to do so.
+            Network::Testnet(_) => false,
+            _ => false,
+        }
+    }
+
+    assert!(is_testnet_v3(Network::Testnet(TestnetVersion::V3)));
+}