Informational 19 Bitcoin Committed Apr 16, 2026 · 21:22 UTC

Remove useless `Option`s in encoders

06623b81 View source ↗

Public commit record

What the developer wrote

Authored by Martin Habovstiak

78/100 · Adequate

Remove useless `Option`s in encoders

The `Encoder` contract states that implementors are not obligated to
make the state after `false` was returned defined, however they so far
did so anyway and this was even tested. This was a waste of performance
and complication of the code that would get amplified with multiple such
encoders chained as each would have to track the final state.

This change removes all those `Option`s, associated tests and in
addition replaces `&[]` return with panic in invalid state of a
composite encoder.

✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Mentions testing or verification

The short version

What changed, and why it matters

This commit is a routine internal cleanup in the rust-bitcoin library's data-encoding machinery. It removes unnecessary 'Option' wrappers from several encoder types and stops guaranteeing that encoders return an empty slice after they are exhausted. Instead, calling 'current_chunk()' on an exhausted encoder may now panic in some composite cases. The change is framed by the author as a performance and code-simplification improvement, not as a security fix. There is no evidence in the commit or supplied references of an exploitable vulnerability.

Recommended action

Treat as a normal refactor. Review downstream callers of 'Encoder::current_chunk()' to ensure they honor the 'advance()' return value and do not call 'current_chunk()' after exhaustion. No security response is indicated by the available evidence.

Security signals we found

Behavioral contract change: post-exhaustion state is now undefined and may panic instead of returning an empty slice

Removal of defensive 'Option' state clearing in encoders

Replacement of '&[]' fallback with 'unreachable!()' in composite encoder

No mention of CVE, security bug, or vulnerability in commit message or diff

Technical analysis

Evidence from the diff

The patch refactors ‘BytesEncoder’, ‘ArrayEncoder’, and ‘ArrayRefEncoder’ to hold their data directly rather than inside ‘Option’. ‘advance()’ now simply returns ‘false’ without clearing state. Tests that asserted ‘current_chunk().is_empty()’ after exhaustion are removed. In the composite encoder macro, the exhausted fallback changes from returning ‘&[]’ to ‘unreachable!(…)’. This relies on the documented ‘Encoder’ contract that callers must stop after ‘advance()’ returns ‘false’. The change is API-behavioral but not a security patch; misuse by callers could now trigger a panic instead of yielding an empty slice.

Changed components

consensus_encoding/src/encode/encoders.rsconsensus_encoding/tests/composition.rsconsensus_encoding/tests/encode.rsp2p/src/message.rsprimitives/src/transaction.rsprimitives/src/witness.rs

Inspect captured patch +13 / −78

diff --git a/consensus_encoding/src/encode/encoders.rs b/consensus_encoding/src/encode/encoders.rs
index 561bf07e..03db6012 100644
--- a/consensus_encoding/src/encode/encoders.rs
+++ b/consensus_encoding/src/encode/encoders.rs
@@ -17,53 +17,51 @@ use super::{Encodable, Encoder, ExactSizeEncoder};
 /// An encoder for a single byte slice.
 #[derive(Debug, Clone)]
 pub struct BytesEncoder<'sl> {
-    sl: Option<&'sl [u8]>,
+    sl: &'sl [u8],
 }
 
 impl<'sl> BytesEncoder<'sl> {
     /// Constructs a byte encoder which encodes the given byte slice, with no length prefix.
-    pub const fn without_length_prefix(sl: &'sl [u8]) -> Self { Self { sl: Some(sl) } }
+    pub const fn without_length_prefix(sl: &'sl [u8]) -> Self { Self { sl } }
 }
 
 impl Encoder for BytesEncoder<'_> {
-    fn current_chunk(&self) -> &[u8] { self.sl.unwrap_or_default() }
+    fn current_chunk(&self) -> &[u8] { self.sl }
 
     fn advance(&mut self) -> bool {
-        self.sl = None;
         false
     }
 }
 
 impl<'sl> ExactSizeEncoder for BytesEncoder<'sl> {
     #[inline]
-    fn len(&self) -> usize { self.sl.map_or(0, <[u8]>::len) }
+    fn len(&self) -> usize { self.sl.len() }
 }
 
 /// An encoder for a single array.
 #[derive(Debug, Clone)]
 pub struct ArrayEncoder<const N: usize> {
-    arr: Option<[u8; N]>,
+    arr: [u8; N],
 }
 
 impl<const N: usize> ArrayEncoder<N> {
     /// Constructs an encoder which encodes the array with no length prefix.
-    pub const fn without_length_prefix(arr: [u8; N]) -> Self { Self { arr: Some(arr) } }
+    pub const fn without_length_prefix(arr: [u8; N]) -> Self { Self { arr } }
 }
 
 impl<const N: usize> Encoder for ArrayEncoder<N> {
     #[inline]
-    fn current_chunk(&self) -> &[u8] { self.arr.as_ref().map(|x| &x[..]).unwrap_or_default() }
+    fn current_chunk(&self) -> &[u8] { &self.arr }
 
     #[inline]
     fn advance(&mut self) -> bool {
-        self.arr = None;
         false
     }
 }
 
 impl<const N: usize> ExactSizeEncoder for ArrayEncoder<N> {
     #[inline]
-    fn len(&self) -> usize { self.arr.map_or(0, |a| a.len()) }
+    fn len(&self) -> usize { self.arr.len() }
 }
 
 /// An encoder for a reference to an array.
@@ -72,28 +70,27 @@ impl<const N: usize> ExactSizeEncoder for ArrayEncoder<N> {
 /// when the array is already available by reference (e.g., as a struct field).
 #[derive(Debug, Clone)]
 pub struct ArrayRefEncoder<'e, const N: usize> {
-    arr: Option<&'e [u8; N]>,
+    arr: &'e [u8; N],
 }
 
 impl<'e, const N: usize> ArrayRefEncoder<'e, N> {
     /// Constructs an encoder which encodes the array reference with no length prefix.
-    pub const fn without_length_prefix(arr: &'e [u8; N]) -> Self { Self { arr: Some(arr) } }
+    pub const fn without_length_prefix(arr: &'e [u8; N]) -> Self { Self { arr } }
 }
 
 impl<const N: usize> Encoder for ArrayRefEncoder<'_, N> {
     #[inline]
-    fn current_chunk(&self) -> &[u8] { self.arr.map(|x| &x[..]).unwrap_or_default() }
+    fn current_chunk(&self) -> &[u8] { self.arr }
 
     #[inline]
     fn advance(&mut self) -> bool {
-        self.arr = None;
         false
     }
 }
 
 impl<const N: usize> ExactSizeEncoder for ArrayRefEncoder<'_, N> {
     #[inline]
-    fn len(&self) -> usize { self.arr.map_or(0, |a| a.len()) }
+    fn len(&self) -> usize { self.arr.len() }
 }
 
 /// An encoder for a list of encodable types.
@@ -199,7 +196,7 @@ macro_rules! define_encoder_n {
             fn current_chunk(&self) -> &[u8] {
                 match self.cur_idx {
                     $($enc_idx => self.$enc_field.current_chunk(),)*
-                    _ => &[],
+                    _ => unreachable!("index never reaches this value"),
                 }
             }
 
diff --git a/consensus_encoding/tests/composition.rs b/consensus_encoding/tests/composition.rs
index 9b180ac8..efb2134a 100644
--- a/consensus_encoding/tests/composition.rs
+++ b/consensus_encoding/tests/composition.rs
@@ -338,7 +338,4 @@ fn empty_encoders() {
 
     assert_eq!(encoder.current_chunk(), &[3, 4][..]);
     assert!(!encoder.advance());
-
-    // Exhausted.
-    assert!(encoder.current_chunk().is_empty());
 }
diff --git a/consensus_encoding/tests/encode.rs b/consensus_encoding/tests/encode.rs
index 868ea9db..79c809ef 100644
--- a/consensus_encoding/tests/encode.rs
+++ b/consensus_encoding/tests/encode.rs
@@ -174,7 +174,6 @@ fn encode_slice_encoder_mixed_empty_and_data() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[3]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -209,7 +208,6 @@ fn encode_encoder2_with_first_empty_encoder() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[1, 2, 3]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -219,11 +217,6 @@ fn encode_encoder_advance_multiple_times_when_exhausted() {
 
     assert_eq!(encoder.current_chunk(), &[10, 20]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
-    assert!(!encoder.advance());
-    assert!(!encoder.advance());
-    assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -231,7 +224,6 @@ fn encode_option_encoder_some() {
     let mut encoder = Some(ArrayEncoder::<3>::without_length_prefix([1, 2, 3]));
     assert_eq!(encoder.current_chunk(), &[1, 2, 3]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -239,7 +231,6 @@ fn encode_option_encoder_none() {
     let mut encoder: Option<ArrayEncoder<3>> = None;
     assert!(encoder.current_chunk().is_empty());
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -251,7 +242,6 @@ fn encode_array_with_data() {
     assert!(!encoder.is_empty());
     assert_eq!(encoder.current_chunk(), &[1u8, 2, 3, 4][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -263,7 +253,6 @@ fn encode_empty_array() {
     assert!(encoder.is_empty());
     assert!(encoder.current_chunk().is_empty());
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -275,8 +264,6 @@ fn encode_array_ref_with_data() {
     assert!(!encoder.is_empty());
     assert_eq!(encoder.current_chunk(), &[1u8, 2, 3, 4][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
-    assert_eq!(encoder.len(), 0);
 }
 
 #[test]
@@ -288,7 +275,6 @@ fn encode_empty_array_ref() {
     assert!(encoder.is_empty());
     assert!(encoder.current_chunk().is_empty());
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -303,7 +289,6 @@ fn encode_byte_slice_without_prefix() {
 
     assert_eq!(encoder.current_chunk(), &[1u8, 2, 3][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -318,7 +303,6 @@ fn encode_empty_byte_slice_without_prefix() {
 
     assert!(encoder.current_chunk().is_empty());
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -331,7 +315,6 @@ fn encode_slice_with_elements() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[0x78, 0x56, 0x00, 0x00][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -341,7 +324,6 @@ fn encode_empty_slice() {
     let mut encoder = SliceEncoder::without_length_prefix(slice);
 
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -353,7 +335,6 @@ fn encode_slice_with_zero_sized_arrays() {
     assert!(encoder.current_chunk().is_empty());
     // The slice advanced is optimized to skip over empty chunks.
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -370,7 +351,6 @@ fn encode_two_arrays() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[3u8, 4][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -387,7 +367,6 @@ fn encode_two_empty_arrays() {
     assert!(encoder.advance());
     assert!(encoder.current_chunk().is_empty());
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -407,7 +386,6 @@ fn encode_three_arrays() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[4u8, 5u8, 6u8][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -430,7 +408,6 @@ fn encode_four_arrays() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[0x40][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -459,7 +436,6 @@ fn encode_six_arrays() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[0x06][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -476,7 +452,6 @@ fn encode_mixed_composition_with_byte_slices() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[0xDD, 0xCC][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -496,7 +471,6 @@ fn encode_nested_composition() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[0x43, 0x44, 0x45][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -513,7 +487,6 @@ fn encode_slice_with_array_composition() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[0x20, 0x21][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -532,7 +505,6 @@ fn encode_array_with_slice_composition() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[0x03][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -561,7 +533,6 @@ fn encode_multiple_slices_composition() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[0xC3][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -581,7 +552,6 @@ fn encode_complex_nested_structure() {
     assert!(encoder.advance());
     assert_eq!(encoder.current_chunk(), &[0xBE, 0xEF][..]);
     assert!(!encoder.advance());
-    assert!(encoder.current_chunk().is_empty());
 }
 
 #[test]
@@ -591,39 +561,33 @@ fn encode_compact_size() {
     assert_eq!(e.current_chunk(), &[0x10][..]);
     assert_eq!(e.len(), 1);
     assert!(!e.advance());
-    assert!(e.current_chunk().is_empty());
 
     let mut e = CompactSizeEncoder::new(0xFCusize);
     assert_eq!(e.current_chunk(), &[0xFC][..]);
     assert_eq!(e.len(), 1);
     assert!(!e.advance());
-    assert!(e.current_chunk().is_empty());
 
     // 0xFD + u16
     let mut e = CompactSizeEncoder::new(0x00FDusize);
     assert_eq!(e.current_chunk(), &[0xFD, 0xFD, 0x00][..]);
     assert_eq!(e.len(), 3);
     assert!(!e.advance());
-    assert!(e.current_chunk().is_empty());
 
     let mut e = CompactSizeEncoder::new(0x0FFFusize);
     assert_eq!(e.current_chunk(), &[0xFD, 0xFF, 0x0F][..]);
     assert_eq!(e.len(), 3);
     assert!(!e.advance());
-    assert!(e.current_chunk().is_empty());
 
     // 0xFE + u32
     let mut e = CompactSizeEncoder::new(0x0001_0000usize);
     assert_eq!(e.current_chunk(), &[0xFE, 0x00, 0x00, 0x01, 0x00][..]);
     assert_eq!(e.len(), 5);
     assert!(!e.advance());
-    assert!(e.current_chunk().is_empty());
 
     let mut e = CompactSizeEncoder::new(0x0F0F_0F0Fusize);
     assert_eq!(e.current_chunk(), &[0xFE, 0x0F, 0x0F, 0x0F, 0x0F][..]);
     assert_eq!(e.len(), 5);
     assert!(!e.advance());
-    assert!(e.current_chunk().is_empty());
 
     // 0xFF + u64
     // This test only runs on systems with >= 64 bit usize.
@@ -632,7 +596,6 @@ fn encode_compact_size() {
         assert_eq!(e.current_chunk(), &[0xFF, 0xE0, 0xF0, 0xF0, 0xF0, 0xF0, 0xF0, 0x00, 0x00][..]);
         assert_eq!(e.len(), 9);
         assert!(!e.advance());
-        assert!(e.current_chunk().is_empty());
     }
 
     // > u64::MAX encodes as u64::MAX.
@@ -642,7 +605,6 @@ fn encode_compact_size() {
         assert_eq!(e.current_chunk(), &[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF][..]);
         assert_eq!(e.len(), 9);
         assert!(!e.advance());
-        assert!(e.current_chunk().is_empty());
     }
 
     // new_u64 works on all platforms, no guard needed.
@@ -650,7 +612,6 @@ fn encode_compact_size() {
     assert_eq!(e.current_chunk(), &[0xFF, 0xE0, 0xF0, 0xF0, 0xF0, 0xF0, 0xF0, 0x00, 0x00][..]);
     assert_eq!(e.len(), 9);
     assert!(!e.advance());
-    assert!(e.current_chunk().is_empty());
 }
 
 #[test]
diff --git a/p2p/src/message.rs b/p2p/src/message.rs
index 693b1261..91902764 100644
--- a/p2p/src/message.rs
+++ b/p2p/src/message.rs
@@ -2933,7 +2933,6 @@ mod test {
         assert_eq!(encoder.len(), expected_bytes.len());
 
         let encoded = encoding::flush_to_vec(&mut encoder);
-        assert_eq!(encoder.len(), 0);
         assert_eq!(encoded, expected_bytes);
     }
 }
diff --git a/primitives/src/transaction.rs b/primitives/src/transaction.rs
index 77d4d5ad..80ce6e9d 100644
--- a/primitives/src/transaction.rs
+++ b/primitives/src/transaction.rs
@@ -1986,9 +1986,6 @@ mod tests {
         // The vout
         assert_eq!(encoder.current_chunk(), &TC_VOUT_BYTES[..]);
         assert!(!encoder.advance());
-
-        // Exhausted
-        assert!(encoder.current_chunk().is_empty());
     }
 
     #[test]
@@ -2008,9 +2005,6 @@ mod tests {
         // The script pubkey data.
         assert_eq!(encoder.current_chunk(), &TC_SCRIPT_BYTES[..]);
         assert!(!encoder.advance());
-
-        // Exhausted
-        assert!(encoder.current_chunk().is_empty());
     }
 
     #[test]
@@ -2035,9 +2029,6 @@ mod tests {
         // The sequence
         assert_eq!(encoder.current_chunk(), &TC_SEQ_MAX_BYTES[..]);
         assert!(!encoder.advance());
-
-        // Exhausted
-        assert!(encoder.current_chunk().is_empty());
     }
 
     #[test]
@@ -2094,9 +2085,6 @@ mod tests {
         // The lock time.
         assert_eq!(encoder.current_chunk(), &TC_LOCK_TIME_ZERO_BYTES[..]);
         assert!(!encoder.advance());
-
-        // Exhausted
-        assert!(encoder.current_chunk().is_empty());
     }
 
     #[test]
@@ -2152,9 +2140,6 @@ mod tests {
         // The lock time.
         assert_eq!(encoder.current_chunk(), &TC_LOCK_TIME_ZERO_BYTES[..]);
         assert!(!encoder.advance());
-
-        // Exhausted
-        assert!(encoder.current_chunk().is_empty());
     }
 
     // FIXME: Move all these encoding tests to a single file in `primitives/tests/`.
@@ -2261,9 +2246,6 @@ mod tests {
         // The lock time.
         assert_eq!(encoder.current_chunk(), &TC_LOCK_TIME_ZERO_BYTES[..]);
         assert!(!encoder.advance());
-
-        // Exhausted
-        assert!(encoder.current_chunk().is_empty());
     }
 
     #[test]
diff --git a/primitives/src/witness.rs b/primitives/src/witness.rs
index 47bf5382..9f9bdf6d 100644
--- a/primitives/src/witness.rs
+++ b/primitives/src/witness.rs
@@ -1381,7 +1381,6 @@ mod test {
         // We don't encode one element at a time, rather we encode the whole content slice at once.
         assert_eq!(encoder.current_chunk(), &[3u8, 1, 2, 3, 2, 4, 5][..]);
         assert!(!encoder.advance());
-        assert!(encoder.current_chunk().is_empty());
     }
 
     #[test]

Risk score

Why this scored 19/100

Our methodology →

Potential impact 2/30

Exploitability 1/25

Stealth signal 1/15

Affected reach 3/15

Confidence 8/10

Evidence quality 4/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 19/100

Community notes

Related watches

Merge rust-bitcoin/rust-bitcoin#6670: ci: fix labeller permissions

Merge rust-bitcoin/rust-bitcoin#6657: Release tracking PR: `base58ck 0.5.0`

Merge rust-bitcoin/rust-bitcoin#6440: base58: Introduce `decode_check_to_array` for alloc-less decoding