consensus_encoding: fix zero element SliceEncoder
What the developer wrote
Authored by Nick Johnson
consensus_encoding: fix zero element SliceEncoder
Ensure that the compact size is removed on the first call to advance
so that even if the slice is empty, current_chunk fulfills its contract
and returns None.
What changed, and why it matters
This commit fixes a bug in a Rust Bitcoin library component that encodes lists of items for the Bitcoin network protocol. When the list was empty, the encoder could fail to consume its internal 'compact size' marker on the first step, breaking its promised behavior of eventually returning 'no more data.' This could cause callers that stream or chunk encoded data to loop forever or behave incorrectly when encoding an empty list.
Review callers of SliceEncoder and current_chunk to confirm whether any production code path could trigger the empty-slice case, and add a regression test for encoding an empty slice. The fix itself should be applied.
Security signals we found
Incorrect state-machine transition in streaming encoder
Potential infinite loop or non-termination when encoding empty slices
Violation of documented contract for current_chunk/advance
Fix is small and targeted, suggesting a real bug rather than refactoring
Evidence from the diff
SliceEncoder encodes a variable-length slice by first yielding a compact-size prefix, then yielding each element’s encoded bytes. The original code only cleared self.compact_size inside the loop body after checking cur_enc, and the comment noted it ‘leaves self.sl alone’ on the first advance. For an empty slice, cur_enc is None, so advance() returned false immediately without ever clearing compact_size. The fix moves the compact-size handling to the top of advance(), so the prefix is consumed regardless of whether there are elements. After the fix, current_chunk can fulfill its contract and return None once the prefix is yielded, even for empty slices.
Changed components
consensus_encoding/src/encode/encoders.rsSliceEncoder<T>Bitcoin consensus serialization / list encoding pathInspect captured patch +12 / −12
diff --git a/consensus_encoding/src/encode/encoders.rs b/consensus_encoding/src/encode/encoders.rs
index 14362693..207ea1f0 100644
--- a/consensus_encoding/src/encode/encoders.rs
+++ b/consensus_encoding/src/encode/encoders.rs
@@ -111,24 +111,24 @@ impl<'e, T: Encodable> Encoder for SliceEncoder<'e, T> {
}
fn advance(&mut self) -> bool {
+ // Handle compact_size first, regardless of whether we have elements.
+ if self.compact_size.is_some() {
+ self.compact_size = None;
+ return self.cur_enc.is_some();
+ }
+
let Some(cur) = self.cur_enc.as_mut() else {
return false;
};
loop {
- if self.compact_size.is_some() {
- // On the first call to advance(), just mark the compact_size as already
- // yielded and leave self.sl alone.
- self.compact_size = None;
- } else {
- // On subsequent calls, attempt to advance the current encoder and return
- // success if this succeeds.
- if cur.advance() {
- return true;
- }
- // self.sl guaranteed to be non-empty if cur is non-None.
- self.sl = &self.sl[1..];
+ // On subsequent calls, attempt to advance the current encoder and return
+ // success if this succeeds.
+ if cur.advance() {
+ return true;
}
+ // self.sl guaranteed to be non-empty if cur is non-None.
+ self.sl = &self.sl[1..];
// If advancing the current encoder failed, attempt to move to the next encoder.
if let Some(x) = self.sl.first() {
Why this scored 35/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.