Merge rust-bitcoin#5006: Remove the implicit lifetime name requirement in the encoder_newtype macro

cf99839 consensus_encoding: API updates (Nick Johnson)
9547fe3 consensus_encoding: remove implicit lifetime name (Nick Johnson)

Pull request description:

  Inspired by rust-bitcoin#5003 (which I believe is the [elidable_lifetime_names](https://rust-lang.github.io/rust-clippy/master/index.html#elidable_lifetime_names) clippy rule) I experimented with some lifetime elision in the encoder_newtype macro. This drops the implicit requirement that the encoder struct's lifetime be called `'e` to match the internal macro's usage.
  
  This might be a subtle change in the requirements themselves since they are no longer explicitly tied together, however, I haven't been able to come up with a unit test to break that in any way.


ACKs for top commit:
  apoelstra:
    ACK cf99839; successfully ran local tests; neat!


Tree-SHA512: 5ce2a32017bf66e21bce95b38aeb6c4e4e317a68c5adc48c03cd815bc3efb67638d04eb4e42980231656761be91b21e0750efe293ea6c506cf3c1631be0f983f

Author: apoelstra

api/units/all-features.txt

@@ -62,6 +62,10 @@ impl consensus_encoding::encode::Encodable for bitcoin_units::BlockTime
 impl consensus_encoding::encode::Encodable for bitcoin_units::block::BlockHeight
 impl consensus_encoding::encode::Encodable for bitcoin_units::locktime::absolute::LockTime
 impl consensus_encoding::encode::Encodable for bitcoin_units::sequence::Sequence
+impl consensus_encoding::encode::Encoder for bitcoin_units::block::BlockHeightEncoder
+impl consensus_encoding::encode::Encoder for bitcoin_units::locktime::absolute::LockTimeEncoder
+impl consensus_encoding::encode::Encoder for bitcoin_units::sequence::SequenceEncoder
+impl consensus_encoding::encode::Encoder for bitcoin_units::time::BlockTimeEncoder
 impl core::clone::Clone for bitcoin_units::Amount
 impl core::clone::Clone for bitcoin_units::BlockTime
 impl core::clone::Clone for bitcoin_units::FeeRate
@@ -1242,10 +1246,6 @@ impl<'de> serde::de::Deserialize<'de> for bitcoin_units::block::BlockMtpInterval
 impl<'de> serde::de::Deserialize<'de> for bitcoin_units::locktime::absolute::LockTime
 impl<'de> serde::de::Deserialize<'de> for bitcoin_units::locktime::relative::LockTime
 impl<'de> serde::de::Deserialize<'de> for bitcoin_units::sequence::Sequence
-impl<'e> consensus_encoding::encode::Encoder<'e> for bitcoin_units::block::BlockHeightEncoder
-impl<'e> consensus_encoding::encode::Encoder<'e> for bitcoin_units::locktime::absolute::LockTimeEncoder
-impl<'e> consensus_encoding::encode::Encoder<'e> for bitcoin_units::sequence::SequenceEncoder
-impl<'e> consensus_encoding::encode::Encoder<'e> for bitcoin_units::time::BlockTimeEncoder
 impl<T: core::clone::Clone> core::clone::Clone for bitcoin_units::result::NumOpResult<T>
 impl<T: core::cmp::Eq> core::cmp::Eq for bitcoin_units::result::NumOpResult<T>
 impl<T: core::cmp::PartialEq> core::cmp::PartialEq for bitcoin_units::result::NumOpResult<T>

consensus_encoding/src/encode/encoders.rs

@@ -37,7 +37,7 @@ impl<'sl> BytesEncoder<'sl> {
     }
 }
 
-impl Encoder<'_> for BytesEncoder<'_> {
+impl Encoder for BytesEncoder<'_> {
     fn current_chunk(&self) -> Option<&[u8]> {
         if let Some(compact_size) = self.compact_size.as_ref() {
             Some(compact_size)
@@ -67,7 +67,7 @@ impl<const N: usize> ArrayEncoder<N> {
     pub fn without_length_prefix(arr: [u8; N]) -> Self { Self { arr: Some(arr) } }
 }
 
-impl<const N: usize> Encoder<'_> for ArrayEncoder<N> {
+impl<const N: usize> Encoder for ArrayEncoder<N> {
     fn current_chunk(&self) -> Option<&[u8]> { self.arr.as_ref().map(|x| &x[..]) }
 
     fn advance(&mut self) -> bool {
@@ -104,7 +104,7 @@ impl<'e, T: Encodable> SliceEncoder<'e, T> {
     }
 }
 
-impl<'e, T: Encodable> Encoder<'e> for SliceEncoder<'e, T> {
+impl<'e, T: Encodable> Encoder for SliceEncoder<'e, T> {
     fn current_chunk(&self) -> Option<&[u8]> {
         if let Some(compact_size) = self.compact_size.as_ref() {
             return Some(compact_size);
@@ -160,7 +160,7 @@ impl<A, B> Encoder2<A, B> {
     pub fn new(enc_1: A, enc_2: B) -> Self { Self { enc_idx: 0, enc_1, enc_2 } }
 }
 
-impl<'e, A: Encoder<'e>, B: Encoder<'e>> Encoder<'e> for Encoder2<A, B> {
+impl<A: Encoder, B: Encoder> Encoder for Encoder2<A, B> {
     fn current_chunk(&self) -> Option<&[u8]> {
         if self.enc_idx == 0 {
             self.enc_1.current_chunk()
@@ -198,7 +198,7 @@ impl<A, B, C> Encoder3<A, B, C> {
     }
 }
 
-impl<'e, A: Encoder<'e>, B: Encoder<'e>, C: Encoder<'e>> Encoder<'e> for Encoder3<A, B, C> {
+impl<A: Encoder, B: Encoder, C: Encoder> Encoder for Encoder3<A, B, C> {
     fn current_chunk(&self) -> Option<&[u8]> { self.inner.current_chunk() }
     fn advance(&mut self) -> bool { self.inner.advance() }
 }
@@ -215,9 +215,7 @@ impl<A, B, C, D> Encoder4<A, B, C, D> {
     }
 }
 
-impl<'e, A: Encoder<'e>, B: Encoder<'e>, C: Encoder<'e>, D: Encoder<'e>> Encoder<'e>
-    for Encoder4<A, B, C, D>
-{
+impl<A: Encoder, B: Encoder, C: Encoder, D: Encoder> Encoder for Encoder4<A, B, C, D> {
     fn current_chunk(&self) -> Option<&[u8]> { self.inner.current_chunk() }
     fn advance(&mut self) -> bool { self.inner.advance() }
 }
@@ -239,15 +237,8 @@ impl<A, B, C, D, E, F> Encoder6<A, B, C, D, E, F> {
     }
 }
 
-impl<
-        'e,
-        A: Encoder<'e>,
-        B: Encoder<'e>,
-        C: Encoder<'e>,
-        D: Encoder<'e>,
-        E: Encoder<'e>,
-        F: Encoder<'e>,
-    > Encoder<'e> for Encoder6<A, B, C, D, E, F>
+impl<A: Encoder, B: Encoder, C: Encoder, D: Encoder, E: Encoder, F: Encoder> Encoder
+    for Encoder6<A, B, C, D, E, F>
 {
     fn current_chunk(&self) -> Option<&[u8]> { self.inner.current_chunk() }
     fn advance(&mut self) -> bool { self.inner.advance() }
@@ -261,7 +252,7 @@ mod tests {
     use super::*;
 
     // Run the encoder i.e., use it to encode into a vector.
-    fn run_encoder<'e>(mut encoder: impl Encoder<'e>) -> Vec<u8> {
+    fn run_encoder(mut encoder: impl Encoder) -> Vec<u8> {
         let mut vec = Vec::new();
         while let Some(chunk) = encoder.current_chunk() {
             vec.extend_from_slice(chunk);
@@ -311,4 +302,4 @@ mod tests {
         let want = [0u8];
         assert_eq!(got, want);
     }
- }
+}

consensus_encoding/src/encode/mod.rs

@@ -15,7 +15,7 @@ pub mod encoders;
 pub trait Encodable {
     /// The encoder associated with this type. Conceptually, the encoder is like
     /// an iterator which yields byte slices.
-    type Encoder<'s>: Encoder<'s>
+    type Encoder<'s>: Encoder
     where
         Self: 's;
 
@@ -24,7 +24,7 @@ pub trait Encodable {
 }
 
 /// An encoder for a consensus-encodable object.
-pub trait Encoder<'e> {
+pub trait Encoder {
     /// Yields the current encoded byteslice.
     ///
     /// Will always return the same value until [`Self::advance`] is called.
@@ -55,7 +55,7 @@ macro_rules! encoder_newtype{
         $(#[$($struct_attr)*])*
         pub struct $name$(<$lt>)?($encoder);
 
-        impl<'e> $crate::Encoder<'e> for $name$(<$lt>)? {
+        impl$(<$lt>)? $crate::Encoder for $name$(<$lt>)? {
             #[inline]
             fn current_chunk(&self) -> Option<&[u8]> { self.0.current_chunk() }
 

consensus_encoding/tests/encode.rs

@@ -5,7 +5,7 @@
 #[cfg(feature = "std")]
 use std::io::{Cursor, Write};
 
-use consensus_encoding::{ArrayEncoder, Encodable};
+use consensus_encoding::{ArrayEncoder, BytesEncoder, Encodable, Encoder};
 
 // Simple test type that implements Encodable.
 struct TestData(u32);
@@ -94,3 +94,22 @@ fn encode_std_writer_io_error() {
     assert!(result.is_err());
     assert_eq!(result.unwrap_err().kind(), std::io::ErrorKind::Other);
 }
+
+#[test]
+fn encode_newtype_lifetime_flexibility() {
+    // Test that the encoder_newtype macro allows different lifetime names.
+
+    consensus_encoding::encoder_newtype! {
+        pub struct CustomEncoder<'data>(BytesEncoder<'data>);
+    }
+    consensus_encoding::encoder_newtype! {
+        pub struct NoLifetimeEncoder(ArrayEncoder<4>);
+    }
+
+    let test_data = b"hello world";
+    let custom_encoder = CustomEncoder(BytesEncoder::without_length_prefix(test_data));
+    let no_lifetime_encoder = NoLifetimeEncoder(ArrayEncoder::without_length_prefix([1, 2, 3, 4]));
+
+    assert_eq!(custom_encoder.current_chunk(), Some(test_data.as_slice()));
+    assert_eq!(no_lifetime_encoder.current_chunk(), Some(&[1, 2, 3, 4][..]));
+}