Merge rust-bitcoin#5344: Use usize for CompactSizeEncoder::new

b486c83 Use usize for CompactSizeEncoder::new (Mitchell Bagot)

Pull request description:

  The current CompactSizeEncoder::new() constructor uses an impl ToU64 for its first parameter. The ToU64 trait is part of internals, and thus isn't permitted to be part of the public API of consensus_encoding.
  
  Change the ToU64 parameter type to usize, encoding values outside of the u64 range (such as on 128 bit system) as u64::MAX.
  
  Closes rust-bitcoin#5343


ACKs for top commit:
  apoelstra:
    ACK b486c83; successfully ran local tests
  tcharding:
    ACK b486c83


Tree-SHA512: 0a7064e3485949414590002e6fbfb6325bb6a76ce759bd6f40f4c7c1ca4817613e51e3d344f8c2b967855d2ba9d430667141f8196561c107275cbe5a63ced5ca

Author: apoelstra

api/consensus_encoding/all-features.txt

@@ -334,7 +334,7 @@ pub fn bitcoin_consensus_encoding::CompactSizeDecoderError::eq(&self, other: &bi
 pub fn bitcoin_consensus_encoding::CompactSizeDecoderError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
 pub fn bitcoin_consensus_encoding::CompactSizeEncoder::advance(&mut self) -> bool
 pub fn bitcoin_consensus_encoding::CompactSizeEncoder::current_chunk(&self) -> &[u8]
-pub fn bitcoin_consensus_encoding::CompactSizeEncoder::new(value: impl bitcoin_internals::ToU64) -> Self
+pub fn bitcoin_consensus_encoding::CompactSizeEncoder::new(value: usize) -> Self
 pub fn bitcoin_consensus_encoding::Decodable::decoder() -> Self::Decoder
 pub fn bitcoin_consensus_encoding::Decoder2<A, B>::end(self) -> core::result::Result<Self::Output, Self::Error>
 pub fn bitcoin_consensus_encoding::Decoder2<A, B>::push_bytes(&mut self, bytes: &mut &[u8]) -> core::result::Result<bool, Self::Error>

api/consensus_encoding/alloc-only.txt

@@ -311,7 +311,7 @@ pub fn bitcoin_consensus_encoding::CompactSizeDecoderError::eq(&self, other: &bi
 pub fn bitcoin_consensus_encoding::CompactSizeDecoderError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
 pub fn bitcoin_consensus_encoding::CompactSizeEncoder::advance(&mut self) -> bool
 pub fn bitcoin_consensus_encoding::CompactSizeEncoder::current_chunk(&self) -> &[u8]
-pub fn bitcoin_consensus_encoding::CompactSizeEncoder::new(value: impl bitcoin_internals::ToU64) -> Self
+pub fn bitcoin_consensus_encoding::CompactSizeEncoder::new(value: usize) -> Self
 pub fn bitcoin_consensus_encoding::Decodable::decoder() -> Self::Decoder
 pub fn bitcoin_consensus_encoding::Decoder2<A, B>::end(self) -> core::result::Result<Self::Output, Self::Error>
 pub fn bitcoin_consensus_encoding::Decoder2<A, B>::push_bytes(&mut self, bytes: &mut &[u8]) -> core::result::Result<bool, Self::Error>

api/consensus_encoding/no-features.txt

@@ -245,7 +245,7 @@ pub fn bitcoin_consensus_encoding::CompactSizeDecoderError::eq(&self, other: &bi
 pub fn bitcoin_consensus_encoding::CompactSizeDecoderError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
 pub fn bitcoin_consensus_encoding::CompactSizeEncoder::advance(&mut self) -> bool
 pub fn bitcoin_consensus_encoding::CompactSizeEncoder::current_chunk(&self) -> &[u8]
-pub fn bitcoin_consensus_encoding::CompactSizeEncoder::new(value: impl bitcoin_internals::ToU64) -> Self
+pub fn bitcoin_consensus_encoding::CompactSizeEncoder::new(value: usize) -> Self
 pub fn bitcoin_consensus_encoding::Decodable::decoder() -> Self::Decoder
 pub fn bitcoin_consensus_encoding::Decoder2<A, B>::end(self) -> core::result::Result<Self::Output, Self::Error>
 pub fn bitcoin_consensus_encoding::Decoder2<A, B>::push_bytes(&mut self, bytes: &mut &[u8]) -> core::result::Result<bool, Self::Error>

consensus_encoding/src/encode/encoders.rs

@@ -12,7 +12,7 @@
 //!
 
 use internals::array_vec::ArrayVec;
-use internals::{compact_size, ToU64};
+use internals::compact_size;
 
 use super::{Encodable, Encoder};
 
@@ -226,7 +226,16 @@ pub struct CompactSizeEncoder {
 
 impl CompactSizeEncoder {
     /// Constructs a new `CompactSizeEncoder`.
-    pub fn new(value: impl ToU64) -> Self { Self { buf: Some(compact_size::encode(value)) } }
+    ///
+    /// Encodings are defined only for the range of u64. On systems where usize is
+    /// larger than u64, it will be possible to call this method with out-of-range
+    /// values. In such cases we will ignore the passed value and encode [`u64::MAX`].
+    /// But even on such exotic systems, we expect users to pass the length of an
+    /// in-memory object, meaning that such large values are impossible to obtain.
+    pub fn new(value: usize) -> Self {
+        let enc_value = value.try_into().unwrap_or(u64::MAX);
+        Self { buf: Some(compact_size::encode(enc_value)) }
+    }
 }
 
 impl Encoder for CompactSizeEncoder {
@@ -554,43 +563,55 @@ mod tests {
     #[test]
     fn encode_compact_size() {
         // 1-byte
-        let mut e = CompactSizeEncoder::new(0x10u64);
+        let mut e = CompactSizeEncoder::new(0x10usize);
         assert_eq!(e.current_chunk(), &[0x10][..]);
         assert!(!e.advance());
         assert!(e.current_chunk().is_empty());
 
-        let mut e = CompactSizeEncoder::new(0xFCu64);
+        let mut e = CompactSizeEncoder::new(0xFCusize);
         assert_eq!(e.current_chunk(), &[0xFC][..]);
         assert!(!e.advance());
         assert!(e.current_chunk().is_empty());
 
         // 0xFD + u16
-        let mut e = CompactSizeEncoder::new(0x00FDu64);
+        let mut e = CompactSizeEncoder::new(0x00FDusize);
         assert_eq!(e.current_chunk(), &[0xFD, 0xFD, 0x00][..]);
         assert!(!e.advance());
         assert!(e.current_chunk().is_empty());
 
-        let mut e = CompactSizeEncoder::new(0x0FFFu64);
+        let mut e = CompactSizeEncoder::new(0x0FFFusize);
         assert_eq!(e.current_chunk(), &[0xFD, 0xFF, 0x0F][..]);
         assert!(!e.advance());
         assert!(e.current_chunk().is_empty());
 
         // 0xFE + u32
-        let mut e = CompactSizeEncoder::new(0x0001_0000u64);
+        let mut e = CompactSizeEncoder::new(0x0001_0000usize);
         assert_eq!(e.current_chunk(), &[0xFE, 0x00, 0x00, 0x01, 0x00][..]);
         assert!(!e.advance());
         assert!(e.current_chunk().is_empty());
 
-        let mut e = CompactSizeEncoder::new(0x0F0F_0F0Fu64);
+        let mut e = CompactSizeEncoder::new(0x0F0F_0F0Fusize);
         assert_eq!(e.current_chunk(), &[0xFE, 0x0F, 0x0F, 0x0F, 0x0F][..]);
         assert!(!e.advance());
         assert!(e.current_chunk().is_empty());
 
         // 0xFF + u64
-        let mut e = CompactSizeEncoder::new(0x0000_F0F0_F0F0_F0E0u64);
-        assert_eq!(e.current_chunk(), &[0xFF, 0xE0, 0xF0, 0xF0, 0xF0, 0xF0, 0xF0, 0x00, 0x00][..]);
-        assert!(!e.advance());
-        assert!(e.current_chunk().is_empty());
+        // This test only runs on systems with >= 64 bit usize.
+        if core::mem::size_of::<usize>() >= 8 {
+            let mut e = CompactSizeEncoder::new(0x0000_F0F0_F0F0_F0E0u64 as usize);
+            assert_eq!(e.current_chunk(), &[0xFF, 0xE0, 0xF0, 0xF0, 0xF0, 0xF0, 0xF0, 0x00, 0x00][..]);
+            assert!(!e.advance());
+            assert!(e.current_chunk().is_empty());
+        }
+
+        // > u64::MAX encodes as u64::MAX.
+        // This test only runs on systems with > 64 bit usize.
+        if core::mem::size_of::<usize>() > 8 {
+            let mut e = CompactSizeEncoder::new((u128::from(u64::MAX) + 5) as usize);
+            assert_eq!(e.current_chunk(), &[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF][..]);
+            assert!(!e.advance());
+            assert!(e.current_chunk().is_empty());
+        }
     }
 
     #[test]

consensus_encoding/tests/encode.rs

@@ -153,19 +153,19 @@ fn encode_slice_encoder_mixed_empty_and_data() {
 #[test]
 fn encode_compact_size_boundary_values() {
     // Test CompactSizeEncoder with boundary values.
-    let mut encoder = CompactSizeEncoder::new(252u32);
+    let mut encoder = CompactSizeEncoder::new(252usize);
     assert_eq!(encoder.current_chunk(), &[252]);
     assert!(!encoder.advance());
 
-    let mut encoder = CompactSizeEncoder::new(253u32);
+    let mut encoder = CompactSizeEncoder::new(253usize);
     assert_eq!(encoder.current_chunk(), &[0xFD, 253, 0]);
     assert!(!encoder.advance());
 
-    let mut encoder = CompactSizeEncoder::new(0x10000u32);
+    let mut encoder = CompactSizeEncoder::new(0x10000usize);
     assert_eq!(encoder.current_chunk(), &[0xFE, 0, 0, 1, 0]);
     assert!(!encoder.advance());
 
-    let mut encoder = CompactSizeEncoder::new(0u32);
+    let mut encoder = CompactSizeEncoder::new(0usize);
     assert_eq!(encoder.current_chunk(), &[0]);
     assert!(!encoder.advance());
 }

primitives/src/transaction.rs

@@ -330,11 +330,11 @@ impl Encodable for Transaction {
     fn encoder(&self) -> Self::Encoder<'_> {
         let version = self.version.encoder();
         let inputs = Encoder2::new(
-            CompactSizeEncoder::new(self.inputs.len() as u64),
+            CompactSizeEncoder::new(self.inputs.len()),
             SliceEncoder::without_length_prefix(self.inputs.as_ref()),
         );
         let outputs = Encoder2::new(
-            CompactSizeEncoder::new(self.outputs.len() as u64),
+            CompactSizeEncoder::new(self.outputs.len()),
             SliceEncoder::without_length_prefix(self.outputs.as_ref()),
         );
         let lock_time = self.lock_time.encoder();