Packages versions
miden-node: 0.16.0-rc.3 (the code is in crates/proto/src/domain/block.rs, so this is miden-node-proto)
Bug description
BlockRange::into_inclusive_range checks two conditions. It first returns StartGreaterThanEnd when start > end, and then returns EmptyRange when block_range.is_empty().
For a freshly constructed RangeInclusive, is_empty() is true exactly when start > end — which the first check has already rejected. So the second branch can never run, and since that is the only place EmptyRange is constructed anywhere in the repo, that variant of the public InvalidBlockRange error can never be produced.
The effect is that the error type advertises a failure mode that callers may write handling for, but which cannot occur. It also reads as though empty ranges were a separate case being guarded against, when there is no such case: an inclusive range with start == end covers exactly one block and is valid.
I have a fix ready that removes the unreachable branch and the variant, together with tests covering the boundaries — inverted ranges rejected, start == end accepted as a single-block range, ascending ranges accepted. Happy to open a PR if someone assigns this to me.
One note: removing the variant is a breaking change to miden-node-proto's public error type, although nothing in the repo matches on it. If you would rather keep the variant and only drop the dead branch, that is an easy change to make.
How can this be reproduced?
- Read
into_inclusive_range in crates/proto/src/domain/block.rs.
RangeInclusive::new(a, b).is_empty() is true exactly when a > b, and the preceding check already returns StartGreaterThanEnd for that case, so the is_empty() branch is unreachable.
grep -rn "EmptyRange" --include=*.rs . returns only the variant declaration and that single unreachable construction site.
Relevant log output
Packages versions
miden-node: 0.16.0-rc.3 (the code is in
crates/proto/src/domain/block.rs, so this ismiden-node-proto)Bug description
BlockRange::into_inclusive_rangechecks two conditions. It first returnsStartGreaterThanEndwhenstart > end, and then returnsEmptyRangewhenblock_range.is_empty().For a freshly constructed
RangeInclusive,is_empty()is true exactly whenstart > end— which the first check has already rejected. So the second branch can never run, and since that is the only placeEmptyRangeis constructed anywhere in the repo, that variant of the publicInvalidBlockRangeerror can never be produced.The effect is that the error type advertises a failure mode that callers may write handling for, but which cannot occur. It also reads as though empty ranges were a separate case being guarded against, when there is no such case: an inclusive range with
start == endcovers exactly one block and is valid.I have a fix ready that removes the unreachable branch and the variant, together with tests covering the boundaries — inverted ranges rejected,
start == endaccepted as a single-block range, ascending ranges accepted. Happy to open a PR if someone assigns this to me.One note: removing the variant is a breaking change to
miden-node-proto's public error type, although nothing in the repo matches on it. If you would rather keep the variant and only drop the dead branch, that is an easy change to make.How can this be reproduced?
into_inclusive_rangeincrates/proto/src/domain/block.rs.RangeInclusive::new(a, b).is_empty()is true exactly whena > b, and the preceding check already returnsStartGreaterThanEndfor that case, so theis_empty()branch is unreachable.grep -rn "EmptyRange" --include=*.rs .returns only the variant declaration and that single unreachable construction site.Relevant log output