fix rws padding logic and error handling#1515
Conversation
7851c6a to
2914f5c
Compare
2914f5c to
3ad660f
Compare
davidnevadoc
left a comment
There was a problem hiding this comment.
Thanks for taking care of this! The comments and new variables make the calculations around the number of rows much easier to follow :)
LGTM 👍
| let padding_length = RwMap::padding_len(self.rows.len(), self.n_rows); | ||
| let padding_length = if self.rows.len() < self.n_rows { | ||
| RwMap::padding_len(self.rows.len(), self.n_rows) | ||
| } else { |
There was a problem hiding this comment.
I have a small question here:
Is there a valid scenario where we enter this branch? In other words, is it ok to have self.rows.len() == self.n_rows in some case?
I believe this is the situation where the log error was being triggered but the circuits were still correct.
There was a problem hiding this comment.
y here is exactly the case self.rows.len() == self.n_rows Because state_circuit receive RwMap which is padding ready, also we set self.n_rows == FixedCParams.max_rws that's why here is strictly equal.
Besides, the result of padding_length is not used for padding here. It's for override, because override idx is counting after padding rows.
I will rename padding_length to first_non_padding_index instead
There was a problem hiding this comment.
Fixed to have better naming in commit 6d5fced
ed255
left a comment
There was a problem hiding this comment.
LGTM! The comments in the code are very useful to explain how the extra rows are calculated :)
Description
Fixed rws padding logic and padding error handling.
Issue Link
#1507
Type of change
Contents