fix(parallel): align stack word max index bound#3014
fix(parallel): align stack word max index bound#3014mk0walsk wants to merge 5 commits into0xMiden:nextfrom
Conversation
|
Automated check (CONTRIBUTING.md) Findings:
Recommendations:
Next steps:
|
huitseeker
left a comment
huitseeker
left a comment
There was a problem hiding this comment.
We could introduce a helper, Something like:
#[inline(always)]
fn word_start_idx(stack_len: usize, start_idx: usize) -> usize {
stack_len - start_idx - WORD_SIZE
}Then both replay and fast paths become obviously the same calculation.
|
|
||
| fn set_word(&mut self, start_idx: usize, word: &Word) { | ||
| debug_assert!(start_idx < MIN_STACK_DEPTH - 4); | ||
| debug_assert!(start_idx <= MIN_STACK_DEPTH - WORD_SIZE); |
There was a problem hiding this comment.
Related to this bound, would it help to switch the next line to MIN_STACK_DEPTH - start_idx - WORD_SIZE instead of - 4? Using WORD_SIZE in both places makes the offset formula much easier to audit.
|
|
||
| fn get_word(&self, start_idx: usize) -> Word { | ||
| debug_assert!(start_idx < MIN_STACK_DEPTH - 4); | ||
| debug_assert!(start_idx <= MIN_STACK_DEPTH - WORD_SIZE); |
There was a problem hiding this comment.
Would it be clearer to write this as debug_assert!(start_idx + WORD_SIZE <= MIN_STACK_DEPTH)? That states the full-word-fit invariant directly, which makes the later offset math easier to check at a glance.
| fn stack_set_word_allows_max_start_idx() { | ||
| let mut processor = build_replay_processor(); | ||
| let start_idx = MIN_STACK_DEPTH - WORD_SIZE; | ||
| let word = [Felt::new(1), Felt::new(2), Felt::new(3), Felt::new(4)].into(); |
There was a problem hiding this comment.
Since this test is about the WORD_SIZE boundary, would it make sense to build the sample word from WORD_SIZE too instead of a 4-element literal? That keeps the test aligned with the same constant the implementation is checking.
2 participants
• ## Describe your changes
This PR fixes a boundary-contract mismatch in parallel stack word access.
ReplayProcessor::get_wordandReplayProcessor::set_wordto allowstart_idx == MIN_STACK_DEPTH - WORD_SIZE(i.e. max valid index12), matchingStackInterfaceand fast-path behavior.stack_set_word_allows_max_start_idxto ensure max-index writes/reads remain valid.Checklist before requesting a review
nextaccording to naming convention.CHANGELOG.md.