Skip to content

Clarify Protogalaxy state #881

Closed
Closed

Description

@codygunton

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 the ProverInstance 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions