-
Notifications
You must be signed in to change notification settings - Fork 94
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.
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
feat(ultrahonk): Added a simple filler table to minimize the amount of entries used to make UltraHonk polynomials non-zero #531
Conversation
@@ -230,7 +220,7 @@ std::shared_ptr<typename Flavor::ProvingKey> UltraHonkComposerHelper_<Flavor>::c | |||
polynomial poly_q_table_column_3(subgroup_size); | |||
polynomial poly_q_table_column_4(subgroup_size); | |||
|
|||
size_t offset = subgroup_size - tables_size - s_randomness - 1; | |||
size_t offset = subgroup_size - tables_size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was the -1 for previously?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We inserted a 1 at the end so that the polynomial would not be zero. Don't know why since we add randomness anyway
@@ -100,23 +100,17 @@ void UltraCircuitConstructor::add_gates_to_ensure_all_polys_are_non_zero() | |||
// to get a non-zero value in table_4. I assume this index is arbitrary and could | |||
// start from 1 instead of 0? | |||
uint32_t left_value = 3; | |||
uint32_t right_value = 5; | |||
uint32_t right_value = 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you explain what this value is because the change looks arbitrary - maybe add comment as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change is because the multitable only works on 2-bit values, so 5 is too big
namespace dummy_tables { | ||
|
||
/** | ||
* @brief Lookup the value corresponding to a sepcific key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo in specific
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
inline BasicTable generate_honk_dummy_table(const BasicTableId id, const size_t table_index) | ||
{ | ||
|
||
ASSERT(table_id == static_cast<uint64_t>(id)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do this two have to be equal - i understand that id
is static while table_id
can change depending on function (?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using the same function in 2 places by templating it, but the API already includes the id in arguments. This is just a simple bug check to ensure that we don't use the wrong instantiation
…f entries used to make UltraHonk polynomials non-zero (AztecProtocol/barretenberg#531) feat(ultrahonk): Added a simple filler table to minimize the amount of entries used to make UltraHonk polynomials non-zero
…f entries used to make UltraHonk polynomials non-zero (AztecProtocol/barretenberg#531) feat(ultrahonk): Added a simple filler table to minimize the amount of entries used to make UltraHonk polynomials non-zero
Description
In the past we used UINT32 XOR and ADD lookup tables to ensure that UltraHonk! polynomials are not zero. This made the minimum size of the circuit 2^14. This PR adds a dummy table which can be used for the same purpose but only has 2 basic tables each consisting of just 4 entries. In addition I got rid of s_randomness in UltraHonkComposerHelper, which produced bugs, since we don't randomise the polynomials in the same way in Honk!
Checklist:
/markdown/specs
have been updated.@brief
describing the intended functionality.