Skip to content

feat(public_executor): allow access to the commitment trees in public execution#794

Merged
Maddiaa0 merged 23 commits into
masterfrom
md/commitments-in-public
Jun 12, 2023
Merged

feat(public_executor): allow access to the commitment trees in public execution#794
Maddiaa0 merged 23 commits into
masterfrom
md/commitments-in-public

Conversation

@Maddiaa0

@Maddiaa0 Maddiaa0 commented Jun 9, 2023

Copy link
Copy Markdown
Member

Description

CommitmentsDB interface has been added to both executors, I have chosen not to completely unify their implementations as private goes through an aztec node while the public just accesses the merkle tree db directly.

How the executors were:
image

How they are after this pr:
image

I have altered the contexts for noir, both now have access to the roots

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.

@Maddiaa0 Maddiaa0 marked this pull request as ready for review June 9, 2023 18:46
Comment thread circuits/cpp/src/aztec3/circuits/abis/c_bind.cpp Outdated
Comment thread circuits/cpp/src/aztec3/circuits/abis/c_bind.cpp Outdated
* @param leafValue - The value to search for.
* @returns The index of the given leaf of undefined if not found.
*/
findCommitmentIndex(leafValue: Buffer): Promise<bigint | undefined>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How should we handle duplicate leafs? The first one you know have not been spent? Just thinking with the idea proposed around also putting L1 to L2 in the commitments tree, would be a pity if you keep fetching the wrong index and cannot spend the note because there was two insertions with "deposit 10 eth to A".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My current idea is that if you have a transparent commitment then the nullifier's will need to be created with a different process. (Including the index) In the end to end test I was hoping to make a library that does this, but you are correct it does feel like a footgun.

Comment thread yarn-project/aztec-rpc/src/database/database.ts
Comment thread yarn-project/aztec-rpc/src/simulator_oracle/index.ts Outdated
Comment thread yarn-project/noir-contracts/src/contracts/noir-aztec3/src/abi.nr Outdated
Comment thread yarn-project/noir-contracts/src/contracts/noir-aztec3/src/abi.nr Outdated
Comment thread yarn-project/world-state/src/merkle-tree/merkle_tree_operations_facade.ts Outdated
@Maddiaa0 Maddiaa0 requested a review from LHerskind June 12, 2023 12:26
@Maddiaa0 Maddiaa0 enabled auto-merge (squash) June 12, 2023 16:06
@Maddiaa0 Maddiaa0 merged commit 9402835 into master Jun 12, 2023
@Maddiaa0 Maddiaa0 deleted the md/commitments-in-public branch June 12, 2023 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants