barretenberg
barretenberg copied to clipboard
Clarify Protogalaxy state
We should clarify and simplify the handling of state in Protogalaxy. Examples:
-
auto
is overused and makes it hard to navigate. - the name "accumulator" is used for objects that are not being treated as an accumulator (the idea is that these objects have the same type as an accumulator, but IMO the naming here is confusing and should be improved)
- the
is_accumulator
flag in theProverInstance
is a bit of a misnomer--I believe this sometimes is more properly an "is_first_round
". - There is a bad pattern in that part of the code where objects are default constructed when probably they should be copy constructed.
Example of the final point: I tried to rewrite
auto next_accumulator = std::make_shared<Instance>();
next_accumulator->is_accumulator = true;
next_accumulator->instance_size = instances[0]->instance_size;
next_accumulator->log_instance_size = instances[0]->log_instance_size;
to
auto next_accumulator = instances[0];
next_accumulator->is_accumulator = true;
next_accumulator->instance_size = instances[0]->instance_size;
next_accumulator->log_instance_size = instances[0]->log_instance_size;
in order to give next_accumulator
a copy of the commitment key. This ran into trouble (Decider tests failed) but
auto next_accumulator = std::make_shared<Instance>();
next_accumulator->is_accumulator = true;
next_accumulator->instance_size = instances[0]->instance_size;
next_accumulator->log_instance_size = instances[0]->log_instance_size;
next_accumulator->commitment_key = instances[0]->commitment_key;
works. IMO this is clear example of brittleness in our handling of state.