fix(standards): stop NoteExecutionHint::can_be_consumed overflowing

[erkancamli]

Third one in this area, independent of #3867 and #3868.

What

OnBlockSlot stores round_len and slot_len as u8, so both can be 32 or more. can_be_consumed shifts by them unconditionally:

let round_len_blocks: u32 = 1 << round_len;
let slot_len_blocks: u32 = 1 << slot_len;

let slot_start_block =
    block_round_index * round_len_blocks + (*slot_offset as u32) * slot_len_blocks;
let slot_end_block = slot_start_block + slot_len_blocks;

A shift of 32 or more is not defined for u32, so this panics with attempt to shift left with overflow in debug and silently masks the shift amount in release, where 1 << 33 becomes 1 << 1. The same hint then answers differently depending on how the binary was built. The offset arithmetic can overflow in the same two ways once slot_len is large.

These are not hypothetical values. from_parts only rejects a non-zero top byte of the payload, so any length in 0..=255 decodes fine, and this repo's own test_encode_round_trip round trips OnBlockSlot { round_len: 22, slot_len: 33, slot_offset: 44 }. NetworkAccountTarget::try_from also builds a hint straight out of an attachment word (NoteExecutionHint::from(word[2])), so the value can come from a note that someone else published.

A wallet or scanner that filters candidate notes with can_be_consumed(tip) therefore aborts on one such note, or, in release, mis-schedules it.

Fix

checked_shl for the lengths, and None when the length does not fit. None already means "we do not know whether this note can be consumed", which is exactly the situation: the hint cannot be evaluated.

For the offset, saturating arithmetic. A slot that starts past the last block never comes around, so Some(false) is the honest answer and no case overflows.

I deliberately did not tighten from_parts to reject lengths above 31, which would be the other way to fix this. That would change decoding, and test_encode_round_trip shows you currently treat those payloads as valid encodings. If you would rather have the validation at the decode boundary, and that test updated, say so and I will send that instead.

Test

out_of_range_block_slot_lengths_are_not_consumable covers both lengths, the decoded path, and the offset overflow.

before: panicked at core/src/ops/bit.rs: attempt to shift left with overflow
after:  test result: ok. 280 passed; 0 failed (cargo test -p miden-standards)

Notes

Same environment caveats as #3867 and #3868: stable 1.95 with --ignore-rust-version because the pinned 1.98.1 toolchain is not reachable from here, and stable cargo fmt --check disagrees with this repo's nightly config across the crate, so I matched the surrounding style by hand. Please lean on CI for both.

Happy to add the CHANGELOG entry once this has a number.

[erkancamli]

Small follow up offer on this one. The out of range behaviour is currently explained only in an inline comment; the public doc comments on can_be_consumed and on the OnBlockSlot variant do not say that a round_len or slot_len of 32 or more yields None. Happy to add that if you want it in the rendered docs.

I also want to be explicit about why the check sits at the evaluation site rather than at construction, since that is the obvious alternative. Rejecting out of range lengths in from_parts would break test_encode_round_trip, which deliberately round trips OnBlockSlot { round_len: 22, slot_len: 33, slot_offset: 44 }: a decoder must not reject bits it cannot interpret, or re encoding loses them. So the evaluation site is the defensible layer.

Worth knowing that out of range lengths are already constructed in tree: miden-testing/src/kernel_tests/tx/test_output_note.rs:1525 builds on_block_slot(5, 32, 3). That test never calls can_be_consumed, so it is unaffected either way, but these are not hypothetical values.

Finally, a heads up across #3867, #3868 and #3869: all three add a line to the same ### Fixes list, so whichever two merge second and third will conflict textually on CHANGELOG.md. Trivial to resolve, and I am happy to rebase whichever ones are left once the first lands.

[erkancamli]

Correcting a bug I introduced in my own fix, before anyone spends review time on it.

The first revision saturated the slot bounds in u32:

let slot_end_block = slot_start_block.saturating_add(slot_len_blocks);

A slot can end at exactly 2^32. That end does not fit a u32 even though every block inside the slot does, so saturating clamped it to u32::MAX and the block_num < slot_end_block test then answered false for BlockNumber::MAX, which the slot actually contains.

The simplest case is on_block_slot(0, 0, 0), which puts every block in its own slot:

hint	block	before this push	correct
on_block_slot(0, 0, 0)	u32::MAX	Some(false)	Some(true)
on_block_slot(1, 0, 1)	u32::MAX	Some(false)	Some(true)
on_block_slot(8, 7, 1)	u32::MAX	Some(false)	Some(true)

So the first revision turned a debug panic into a silently wrong answer rather than into the right one, at exactly the boundary a reviewer would probe. That is worse than the bug it replaced, and it was mine.

Now the bounds are computed in u64. Nothing there can overflow: block_round_index * round_len_blocks is at most block_num, slot_offset is a u8, and both lengths are at most 2^31, so the total stays under 2^41. I checked the new arithmetic against an exact i128 reference over every round_len and slot_len in 0..=40, nine offsets, and nineteen block numbers picked around the power-of-two boundaries and u32::MAX: zero mismatches for all in-range lengths, and 1284 inputs where the old saturating version disagreed.

a_slot_ending_past_the_block_space_still_contains_the_last_block pins it, including the block before the last one, which both versions answer the same way, so the test isolates the boundary rather than the whole path.

I also widened the CHANGELOG entry. It named only the shift overflow, and this patch also fixes the slot_offset overflow, which panics in debug for round and slot lengths that are entirely in range (on_block_slot(31, 31, 255)). That half was doing real work and the entry did not say so.

One thing I have not been able to do myself: the repo pins Rust 1.98.1 and my sandbox cannot reach static.rust-lang.org, so I could not run the crate suite. The arithmetic above was verified by compiling the exact expressions standalone, and rustfmt with the repo's rustfmt.toml reports no diff on the file. CI is the real check.

[mmagician]

Thanks, but we're not accepting external contributions at this point.