From 4f43965ade3b2052cf4056252e4988265035e294 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 31 May 2023 19:12:38 +1000 Subject: [PATCH] Make Encodable/Decodable usage uniform One encodes to a writer and decodes from a reader, most of the time in the consensus `Encodable`/`Decodable` traits we use generic `R`/`W` and variable `r`/`w` but there are other places that use other characters. While touching these lines note also that there are a bunch of unneeded `mut`s, I'm not sure why since usually between the compiler and the linter `mut` is handled correctly. Make implementations of `Encodable` and `Decodable` uniform by: - Use R/W and r/w for trait and variable name - Remove unneeded mut --- bitcoin/src/bip152.rs | 42 +++++++++++++++------------------ bitcoin/src/consensus/encode.rs | 8 +++---- bitcoin/src/p2p/address.rs | 16 ++++++------- 3 files changed, 31 insertions(+), 35 deletions(-) diff --git a/bitcoin/src/bip152.rs b/bitcoin/src/bip152.rs index 39679a6c..2a55e8f6 100644 --- a/bitcoin/src/bip152.rs +++ b/bitcoin/src/bip152.rs @@ -73,20 +73,18 @@ impl convert::AsRef for PrefilledTransaction { impl Encodable for PrefilledTransaction { #[inline] - fn consensus_encode(&self, mut s: &mut S) -> Result { - Ok(VarInt::from(self.idx).consensus_encode(&mut s)? + self.tx.consensus_encode(&mut s)?) + fn consensus_encode(&self, w: &mut W) -> Result { + Ok(VarInt::from(self.idx).consensus_encode(w)? + self.tx.consensus_encode(w)?) } } impl Decodable for PrefilledTransaction { #[inline] - fn consensus_decode( - mut d: &mut D, - ) -> Result { - let idx = VarInt::consensus_decode(&mut d)?.0; + fn consensus_decode(r: &mut R) -> Result { + let idx = VarInt::consensus_decode(r)?.0; let idx = u16::try_from(idx) .map_err(|_| encode::Error::ParseFailed("BIP152 prefilled tx index out of bounds"))?; - let tx = Transaction::consensus_decode(&mut d)?; + let tx = Transaction::consensus_decode(r)?; Ok(PrefilledTransaction { idx, tx }) } } @@ -131,15 +129,15 @@ impl ShortId { impl Encodable for ShortId { #[inline] - fn consensus_encode(&self, s: &mut S) -> Result { - self.0.consensus_encode(s) + fn consensus_encode(&self, w: &mut W) -> Result { + self.0.consensus_encode(w) } } impl Decodable for ShortId { #[inline] - fn consensus_decode(d: &mut D) -> Result { - Ok(ShortId(Decodable::consensus_decode(d)?)) + fn consensus_decode(r: &mut R) -> Result { + Ok(ShortId(Decodable::consensus_decode(r)?)) } } @@ -260,13 +258,13 @@ impl Encodable for BlockTransactionsRequest { /// /// Panics if the index overflows [`u64::MAX`]. This happens when [`BlockTransactionsRequest::indexes`] /// contains an entry with the value [`u64::MAX`] as `u64` overflows during differential encoding. - fn consensus_encode(&self, mut s: &mut S) -> Result { - let mut len = self.block_hash.consensus_encode(&mut s)?; + fn consensus_encode(&self, w: &mut W) -> Result { + let mut len = self.block_hash.consensus_encode(w)?; // Manually encode indexes because they are differentially encoded VarInts. - len += VarInt(self.indexes.len() as u64).consensus_encode(&mut s)?; + len += VarInt(self.indexes.len() as u64).consensus_encode(w)?; let mut last_idx = 0; for idx in &self.indexes { - len += VarInt(*idx - last_idx).consensus_encode(&mut s)?; + len += VarInt(*idx - last_idx).consensus_encode(w)?; last_idx = *idx + 1; // can panic here } Ok(len) @@ -274,14 +272,12 @@ impl Encodable for BlockTransactionsRequest { } impl Decodable for BlockTransactionsRequest { - fn consensus_decode( - mut d: &mut D, - ) -> Result { + fn consensus_decode(r: &mut R) -> Result { Ok(BlockTransactionsRequest { - block_hash: BlockHash::consensus_decode(&mut d)?, + block_hash: BlockHash::consensus_decode(r)?, indexes: { // Manually decode indexes because they are differentially encoded VarInts. - let nb_indexes = VarInt::consensus_decode(&mut d)?.0 as usize; + let nb_indexes = VarInt::consensus_decode(r)?.0 as usize; // Since the number of indices ultimately represent transactions, // we can limit the number of indices to the maximum number of @@ -299,14 +295,14 @@ impl Decodable for BlockTransactionsRequest { let mut indexes = Vec::with_capacity(nb_indexes); let mut last_index: u64 = 0; for _ in 0..nb_indexes { - let differential: VarInt = Decodable::consensus_decode(&mut d)?; + let differential: VarInt = Decodable::consensus_decode(r)?; last_index = match last_index.checked_add(differential.0) { - Some(r) => r, + Some(i) => i, None => return Err(encode::Error::ParseFailed("block index overflow")), }; indexes.push(last_index); last_index = match last_index.checked_add(1) { - Some(r) => r, + Some(i) => i, None => return Err(encode::Error::ParseFailed("block index overflow")), }; } diff --git a/bitcoin/src/consensus/encode.rs b/bitcoin/src/consensus/encode.rs index 1aa927f0..24b8ac9a 100644 --- a/bitcoin/src/consensus/encode.rs +++ b/bitcoin/src/consensus/encode.rs @@ -642,12 +642,12 @@ impl_vec!((u32, Address)); #[cfg(feature = "std")] impl_vec!(AddrV2Message); -pub(crate) fn consensus_encode_with_size( +pub(crate) fn consensus_encode_with_size( data: &[u8], - mut s: S, + mut w: W, ) -> Result { - let vi_len = VarInt(data.len() as u64).consensus_encode(&mut s)?; - s.emit_slice(data)?; + let vi_len = VarInt(data.len() as u64).consensus_encode(&mut w)?; + w.emit_slice(data)?; Ok(vi_len + data.len()) } diff --git a/bitcoin/src/p2p/address.rs b/bitcoin/src/p2p/address.rs index 11cbd882..0c4a085f 100644 --- a/bitcoin/src/p2p/address.rs +++ b/bitcoin/src/p2p/address.rs @@ -141,7 +141,7 @@ pub enum AddrV2 { } impl Encodable for AddrV2 { - fn consensus_encode(&self, e: &mut W) -> Result { + fn consensus_encode(&self, w: &mut W) -> Result { fn encode_addr( w: &mut W, network: u8, @@ -154,13 +154,13 @@ impl Encodable for AddrV2 { Ok(len) } Ok(match *self { - AddrV2::Ipv4(ref addr) => encode_addr(e, 1, &addr.octets())?, - AddrV2::Ipv6(ref addr) => encode_addr(e, 2, &addr.octets())?, - AddrV2::TorV2(ref bytes) => encode_addr(e, 3, bytes)?, - AddrV2::TorV3(ref bytes) => encode_addr(e, 4, bytes)?, - AddrV2::I2p(ref bytes) => encode_addr(e, 5, bytes)?, - AddrV2::Cjdns(ref addr) => encode_addr(e, 6, &addr.octets())?, - AddrV2::Unknown(network, ref bytes) => encode_addr(e, network, bytes)?, + AddrV2::Ipv4(ref addr) => encode_addr(w, 1, &addr.octets())?, + AddrV2::Ipv6(ref addr) => encode_addr(w, 2, &addr.octets())?, + AddrV2::TorV2(ref bytes) => encode_addr(w, 3, bytes)?, + AddrV2::TorV3(ref bytes) => encode_addr(w, 4, bytes)?, + AddrV2::I2p(ref bytes) => encode_addr(w, 5, bytes)?, + AddrV2::Cjdns(ref addr) => encode_addr(w, 6, &addr.octets())?, + AddrV2::Unknown(network, ref bytes) => encode_addr(w, network, bytes)?, }) } }