Skip to content

fix: use standard serialized vector of frs in function tree cbinds#314

Merged
dbanks12 merged 11 commits intomasterfrom
db/fix_func_tree_cbinds
Apr 24, 2023
Merged

fix: use standard serialized vector of frs in function tree cbinds#314
dbanks12 merged 11 commits intomasterfrom
db/fix_func_tree_cbinds

Conversation

@dbanks12
Copy link
Copy Markdown
Contributor

@dbanks12 dbanks12 commented Apr 20, 2023

Description

Contents

  • update function tree cbinds to expect a vector of frs using standard serialization
  • revert TS to use serializeBufferArrayToVector again for cbind args now that C++ expects vectors
  • rename/cleanup cbind arg names and remove num_leaves arg (not necessary since size of vec is first word of serialization)
  • remove unnecessary tree helper functions in C++ and replace with simpler helper
  • some autoformatting of C++ from an unformatted merge 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.
  • The branch has been merged or rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.

@dbanks12 dbanks12 changed the title Use standard serialized vector of frs in function tree cbinds fix: use standard serialized vector of frs in function tree cbinds Apr 20, 2023
@ludamad
Copy link
Copy Markdown
Collaborator

ludamad commented Apr 20, 2023 via email

@dbanks12 dbanks12 marked this pull request as ready for review April 21, 2023 13:11
// fill in nonzero leaves to start
read(function_leaves_in, leaves);
// fill in zero leaves to complete tree
NT::fr zero_leaf = FunctionLeafPreimage<NT>().hash(); // hash of empty/0 preimage
Copy link
Copy Markdown
Member

@Maddiaa0 Maddiaa0 Apr 21, 2023

Choose a reason for hiding this comment

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

Do we eventually want to have this also be 0, as we did for the NullifierTree?

Copy link
Copy Markdown
Contributor Author

@dbanks12 dbanks12 Apr 21, 2023

Choose a reason for hiding this comment

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

Not sure, what are advantages of each way?

Comment thread yarn-project/circuits.js/src/abis/abis.ts Outdated
@dbanks12 dbanks12 requested a review from PhilWindle April 21, 2023 17:33
@dbanks12 dbanks12 merged commit 6d75664 into master Apr 24, 2023
@dbanks12 dbanks12 deleted the db/fix_func_tree_cbinds branch April 24, 2023 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants