Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: witness changes in file sponge.hpp #10345

Merged
merged 1 commit into from
Dec 2, 2024
+4 −4
Merged

fix: witness changes in file sponge.hpp #10345

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/sponge/sponge.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,16 @@ template <size_t rate, size_t capacity, size_t t, typename Permutation, typename
: builder(&builder_)
{
for (size_t i = 0; i < rate; ++i) {
state[i] = witness_t<Builder>(builder, 0);
state[i] = witness_t<Builder>::create_constant_witness(builder, 0);
}
state[rate] = witness_t<Builder>(builder, domain_iv.get_value());
state[rate] = witness_t<Builder>::create_constant_witness(builder, domain_iv.get_value());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this .get_value seems a little suspicious. Should we just change the argument to be a bb::fr instead so its explicitly a value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My analyzer didn't noticed this variable as dangerous...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seem's like we have created constraint on domain_iv.get_value() as constant value, and after that there were created constraints for this variable in permutation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, I'm not saying its dangerous in our current code but it looks a little weird to be passing in a field_t when we just always pass in a bb::fr?

}

std::array<field_t, rate> perform_duplex()
{
// zero-pad the cache
for (size_t i = cache_size; i < rate; ++i) {
cache[i] = witness_t<Builder>(builder, 0);
cache[i] = witness_t<Builder>::create_constant_witness(builder, 0);
}
// add the cache into sponge state
for (size_t i = 0; i < rate; ++i) {
Expand Down Expand Up @@ -122,7 +122,7 @@ template <size_t rate, size_t capacity, size_t t, typename Permutation, typename
cache[i - 1] = cache[i];
}
cache_size -= 1;
cache[cache_size] = witness_t<Builder>(builder, 0);
cache[cache_size] = witness_t<Builder>::create_constant_witness(builder, 0);
return result;
}

Expand Down
Loading