chore: audit databus - small cleanups and tests coverage #16703
Conversation
| w_value = index.get_value(); | ||
| value.assert_equal(calldata_array[index]); | ||
| if (!has_valid_witness_assignments) { | ||
| index.is_zero(); |
There was a problem hiding this comment.
Do we actually need to do this? This if is only reached when mocking. We were already checking that the index value is zero with the copy constraint. But if it's only in mock cases, do we need to assert it's zero in circuit? I think the change I made causes issues because the mock would have an extra gate, so probably vk generation will be wrong
kashbrti
changed the title
| if (has_valid_witness_assignments) { | ||
| // If witness are assigned, we use the correct value for w | ||
| w_value = index.get_value(); | ||
|
|
There was a problem hiding this comment.
much better! can you update the comment to say why this is done?
There was a problem hiding this comment.
there's a comment inside the if. but will add some extra explanation
| * @brief An expository test demonstrating the functionality of the databus in a small use case when the entries are | ||
| * constant witnesses | ||
| */ | ||
| TEST(Databus, ConstantEntryAccess) |
There was a problem hiding this comment.
what happens if the read indices are constant witnesses?
There was a problem hiding this comment.
handled in another test. I chucked both cases for the indices in the same test.
| * @brief An expository test demonstrating the functionality of the databus in a small use case where the indices are | ||
| * constant and/or unnormalized | ||
| */ | ||
| TEST(Databus, ConstantAndUnnormalizedIndices) |
…6703) a small PR doing the following as a part of the `databus` audit - added tests to check the handling of constant or unnormalized indexes - removed some ugliness with dangling witnesses and replaced them with `set_variable` - some minor cleanups
…6703) a small PR doing the following as a part of the `databus` audit - added tests to check the handling of constant or unnormalized indexes - removed some ugliness with dangling witnesses and replaced them with `set_variable` - some minor cleanups
a small PR doing the following as a part of the
databusauditset_variable