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.

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

Refactor pedersen generator initialisation #337

Closed
iAmMichaelConnor opened this issue Apr 10, 2023 · 5 comments
Closed

Refactor pedersen generator initialisation #337

iAmMichaelConnor opened this issue Apr 10, 2023 · 5 comments
Labels
aztec3 Critical for Aztec 3 optimization

Comments

@iAmMichaelConnor
Copy link
Contributor

iAmMichaelConnor commented Apr 10, 2023

At the moment, the configuration for the number of generator indices (and sub indices) is done with two constexpr global variables.

constexpr size_t num_default_generators = 64;

This has a few downsides:

  • Since they're global, they slow down initialisation of bberg; even if you don't need to use pedersen in your application. (@kevaundray I think you've stated this as a problem Noir has encountered in the past).
  • Configuration has to be done by tinkering with these constexpr variables, so can't be done programmatically, varying depending on the application.
  • For an example configuration of 16 generators and 32 sub-indices, we end up with a lot of wastage.
    • Many pedersen commits/hashes will only need a handful of sub-indices. E.g. Merkle membership hashing only needs two; not 32. So we end up 'using up' precious, scarce* generator points for no reason. *We're limited in the number of generators we can initialise, so careful instantiation would be better.

Suggestion for improvement:

  • Don't initialise generators as globals. Have an initialise_generators function which must be called (by the application using bberg as a library) to generate points. Give the option of holding these points in memory, or return the points to the application (more discussion needed here, please).
  • Allow the 'configuration' of the generators to be expressed more granularly, e.g.:
    • std::map<int, size_t> num_sub_indices; a mapping from some enum (whose underlying types are int by default) of generator indices to the number of sub indices required per index.
    • So if you had an enum:
      enum GeneratorIndices {
          MERKLE_TREE = 1,
          NULLIFIER,
          // etc...
      };
      
      
      You could specify the number of sub-indices needed per enum value. Sorry about the pseudo syntax here:
      std::map<int, size_t> num_sub_indices = {
          GeneratorIndices::MERKLE_TREE: 2,
          GeneratorIndices::NULLIFIER: 3,
          // etc...
      };
      
@iAmMichaelConnor iAmMichaelConnor added aztec3 Critical for Aztec 3 optimization labels Apr 10, 2023
@kevaundray
Copy link
Contributor

Yeah I'd prefer to have a process to generate these points into a context and then I can feed the context to whatever function requires it. This would require a bigger refactor though if all functions are expecting a global

@kevaundray
Copy link
Contributor

@iAmMichaelConnor This seems related to #338

@iAmMichaelConnor
Copy link
Contributor Author

@iAmMichaelConnor This seems related to #338

Yeah, this issue's proposal should reduce the memory footprint for both applications.
We can probably have a method for 'granular' initialisation of generators as I propose above (for aztec 3 where we know exactly how many we need for each domain separated pedersen hash), and a 'coarse' initialisation which matches what Noir is used to today (i.e. the constants that were previously being used).

@iAmMichaelConnor
Copy link
Contributor Author

Blocked - need to identify stakeholders and get them on a call to decide best approach.

@iAmMichaelConnor iAmMichaelConnor moved this from Todo to Blocked in A3 Aug 7, 2023
@codygunton
Copy link
Collaborator

To be closed by AztecProtocol/aztec-packages#2592

@github-project-automation github-project-automation bot moved this from Blocked to Done in A3 Oct 17, 2023
@codygunton codygunton reopened this Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aztec3 Critical for Aztec 3 optimization
Projects
Archived in project
Development

No branches or pull requests

4 participants