Merge rust-bitcoin/rust-bitcoin#1227: Remove some easy todo's

fea908c8af Add private::Sealed trait bound to SerdeAmount (Tobin C. Harding)
08e4b28dd0 Add integer serialization tests (Tobin C. Harding)
a8f9e8ad96 Add serde roundtrip tests (Tobin C. Harding)

Pull request description:

  Remove some easy todos from the code. The three patches are unrelated except each removes a single `TODO`. Patch 1 and 2 are unit tests and trivial to review. Patch 3 requires more thought, it was't clear exactly why it was left as a todo and not done at the time.

  cc JeremyRubin for patch 3 as the original author of that code.

ACKs for top commit:
  apoelstra:
    utACK fea908c8af

Tree-SHA512: 70a041820fe52bc3d70cc110c16acfa86469fd8556793c4204671ddcb7457f336e57b43b0f03cb68d3c6b96217502f29f1628d1acbe7381a9e9d8d21c67759d2

src/address.rs

@@ -951,7 +951,13 @@ mod tests {
             "script round-trip failed for {}",
             addr,
         );
-        //TODO: add serde roundtrip after no-strason PR
+
+        #[cfg(feature = "serde")]
+        {
+            let ser = serde_json::to_string(addr).expect("failed to serialize address");
+            let back: Address = serde_json::from_str(&ser).expect("failed to deserialize address");
+            assert_eq!(back, *addr, "serde round-trip failed for {}", addr)
+        }
     }
 
     #[test]

src/consensus/encode.rs

@@ -1009,14 +1009,31 @@ mod tests {
         let failure16: Result<u16, _> = deserialize(&[1u8]);
         assert!(failure16.is_err());
 
+        // i16
+        assert_eq!(deserialize(&[0x32_u8, 0xF4]).ok(), Some(-0x0bce_i16));
+        assert_eq!(deserialize(&[0xFF_u8, 0xFE]).ok(), Some(-0x0101_i16));
+        assert_eq!(deserialize(&[0x00_u8, 0x00]).ok(), Some(-0_i16));
+        assert_eq!(deserialize(&[0xFF_u8, 0xFA]).ok(), Some(-0x0501_i16));
+
         // u32
         assert_eq!(deserialize(&[0xABu8, 0xCD, 0, 0]).ok(), Some(0xCDABu32));
         assert_eq!(deserialize(&[0xA0u8, 0x0D, 0xAB, 0xCD]).ok(), Some(0xCDAB0DA0u32));
+
         let failure32: Result<u32, _> = deserialize(&[1u8, 2, 3]);
         assert!(failure32.is_err());
-        // TODO: test negative numbers
+
+        // i32
         assert_eq!(deserialize(&[0xABu8, 0xCD, 0, 0]).ok(), Some(0xCDABi32));
         assert_eq!(deserialize(&[0xA0u8, 0x0D, 0xAB, 0x2D]).ok(), Some(0x2DAB0DA0i32));
+
+        assert_eq!(deserialize(&[0, 0, 0, 0]).ok(), Some(-0_i32));
+        assert_eq!(deserialize(&[0, 0, 0, 0]).ok(), Some(0_i32));
+
+        assert_eq!(deserialize(&[0xFF, 0xFF, 0xFF, 0xFF]).ok(), Some(-1_i32));
+        assert_eq!(deserialize(&[0xFE, 0xFF, 0xFF, 0xFF]).ok(), Some(-2_i32));
+        assert_eq!(deserialize(&[0x01, 0xFF, 0xFF, 0xFF]).ok(), Some(-255_i32));
+        assert_eq!(deserialize(&[0x02, 0xFF, 0xFF, 0xFF]).ok(), Some(-254_i32));
+
         let failurei32: Result<i32, _> = deserialize(&[1u8, 2, 3]);
         assert!(failurei32.is_err());
 
@@ -1025,11 +1042,18 @@ mod tests {
         assert_eq!(deserialize(&[0xA0u8, 0x0D, 0xAB, 0xCD, 0x99, 0, 0, 0x99]).ok(), Some(0x99000099CDAB0DA0u64));
         let failure64: Result<u64, _> = deserialize(&[1u8, 2, 3, 4, 5, 6, 7]);
         assert!(failure64.is_err());
-        // TODO: test negative numbers
+
+        // i64
         assert_eq!(deserialize(&[0xABu8, 0xCD, 0, 0, 0, 0, 0, 0]).ok(), Some(0xCDABi64));
         assert_eq!(deserialize(&[0xA0u8, 0x0D, 0xAB, 0xCD, 0x99, 0, 0, 0x99]).ok(), Some(-0x66ffff663254f260i64));
+        assert_eq!(deserialize(&[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]).ok(), Some(-1_i64));
+        assert_eq!(deserialize(&[0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]).ok(), Some(-2_i64));
+        assert_eq!(deserialize(&[0x01, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]).ok(), Some(-255_i64));
+        assert_eq!(deserialize(&[0x02, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]).ok(), Some(-254_i64));
+
         let failurei64: Result<i64, _> = deserialize(&[1u8, 2, 3, 4, 5, 6, 7]);
         assert!(failurei64.is_err());
+
     }
 
     #[test]
@@ -1145,6 +1169,5 @@ mod tests {
             );
         }
     }
-
 }
 

src/util/amount.rs

@@ -1281,9 +1281,7 @@ pub mod serde {
 
     /// This trait is used only to avoid code duplication and naming collisions
     /// of the different serde serialization crates.
-    ///
+    pub trait SerdeAmount: Copy + Sized + private::Sealed {
-    /// TODO: Add the private::Sealed bound in next breaking release
-    pub trait SerdeAmount: Copy + Sized {
         fn ser_sat<S: Serializer>(self, s: S) -> Result<S::Ok, S::Error>;
         fn des_sat<'d, D: Deserializer<'d>>(d: D) -> Result<Self, D::Error>;
         fn ser_btc<S: Serializer>(self, s: S) -> Result<S::Ok, S::Error>;