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

feat(api): external cpp header for circuits #489

Merged
merged 10 commits into from
May 31, 2023
Merged

Conversation

dbanks12
Copy link
Contributor

@dbanks12 dbanks12 commented May 31, 2023

Description

After this PR is merged followed by its blocked aztec PR, .aztec-packages-commit can be set to master or latest aztec master commit.

Checklist:

  • I have reviewed my diff in github, line by line.
  • Every change is related to the PR description.
  • I have linked this pull request to the issue(s) that it resolves.
  • There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
  • There are no circuit changes, OR specifications in /markdown/specs have been updated.
  • There are no circuit changes, OR a cryptographer has been assigned for review.
  • I've updated any terraform that needs updating (e.g. environment variables) for deployment.
  • The branch has been rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.
  • New functions, classes, etc. have been documented according to the doxygen comment format. Classes and structs must have @brief describing the intended functionality.
  • If existing code has been modified, such documentation has been added or updated.

@dbanks12 dbanks12 self-assigned this May 31, 2023
@dbanks12 dbanks12 added the api Changes to the barretenberg library or binary API label May 31, 2023
@dbanks12 dbanks12 changed the title Draft: api: external cpp header for circuits Draft: feat(api): external cpp header for circuits May 31, 2023
@dbanks12 dbanks12 marked this pull request as ready for review May 31, 2023 20:32
@dbanks12 dbanks12 changed the title Draft: feat(api): external cpp header for circuits feat(api): external cpp header for circuits May 31, 2023
Copy link
Collaborator

@ludamad ludamad left a comment

Choose a reason for hiding this comment

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

As noted, I never intended
#include "serialize/cbind.hpp"
#include "serialize/msgpack.hpp"
#include "serialize/test_helper.hpp"

To be exposed to header files. It may slow down compilation (although only really if we have a lot of .cpp files). Otherwise LGTM.

Copy link
Collaborator

@codygunton codygunton left a comment

Choose a reason for hiding this comment

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

Thanks for this. This is crucial as a first step, and in that sense it looks right to me. That said, this API is really large and hence we can expect regular breaking changes until we can pare it down as discussed earlier today.

@dbanks12 dbanks12 merged commit fbbb342 into master May 31, 2023
@dbanks12 dbanks12 deleted the db/external-cpp-header branch May 31, 2023 21:20
ludamad pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jul 22, 2023
…rg#489)

* missing header includes

* comments in api header with issues/todos
ludamad pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jul 24, 2023
…rg#489)

* missing header includes

* comments in api header with issues/todos
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Changes to the barretenberg library or binary API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create single external header for use by circuits
3 participants