feat(public->private): create commitments in public contexts (in noir) #810

Maddiaa0 · 2023-06-12T15:59:27Z

Description

Solves

completes Public -> Private Tracking Issue #750
baseline for feat(public->private): Noir syntax for public -> private message passing #809

Creates an end to end test where a public function creates a commitment which is later consumed within the private execution context.

Please provide a paragraph or two giving a summary of the change, including relevant motivation and context.

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.

fix: fmt

LHerskind

Looks good, minor nits.

LHerskind · 2023-06-13T14:52:55Z

yarn-project/acir-simulator/src/client/client_execution_context.ts

+   * @param commitment - The commitment.
+   * @returns The commitment data.
+   */
+  public async getCommitment(contractAddress: AztecAddress, commitment: Fr) {


Can you add the return type explicity

LHerskind · 2023-06-13T15:42:10Z

yarn-project/acir-simulator/src/public/index.test.ts

+      // Assert the commitment was created
+      expect(result.newCommitments.length).toEqual(1);
+
+      const expectedNewCommitmentValue = pedersenCompressInputs(


Should we handle this separately such that there were no pedersen calls directly in here?

In reality this calculation will be performed in noir on the other side, it is just done here in the test to prevent writing a cbind for two values. I dont think it's worth it since it is just a test.

LHerskind · 2023-06-13T15:46:44Z

...project/noir-contracts/src/contracts/noir-aztec3/src/messaging/get_commitment_getter_data.nr

+    1 + 1 + 1
+}
+
+// TODO: 3 as const


LHerskind · 2023-06-13T15:56:15Z

yarn-project/noir-contracts/src/contracts/public_private_contract/src/transparent_note.nr

+    }
+
+    // Gets the value of the commitment
+    // TODO(maddiaa): this will need to be hashed with a slot to keep it unique, which slot to pick?


We could use slots similar to some of the Eips like keccak256("TransparentNote") - 1.

made an issue

rahul-kothari · 2023-06-14T10:28:46Z

yarn-project/acir-simulator/src/client/private_execution.test.ts

+      const secret = new Fr(1n);
+      const secretHash = computeSecretMessageHash(wasm, secret);
+      const commitment = Fr.fromBuffer(pedersenCompressInputs(wasm, [toBufferBE(amount, 32), secretHash.toBuffer()]));
+      const siloedCommitment = siloCommitment(wasm, contractAddress, commitment);


somewhere in the code, can you add a comment on why it is called "silo"?

Added in the siloCommitment function natspec

rahul-kothari · 2023-06-14T10:33:46Z

yarn-project/acir-simulator/src/public/executor.ts

@@ -57,7 +70,19 @@ export class PublicExecutor {
      emitEncryptedLog: notAvailable,
      viewNotesPage: notAvailable,
      debugLog: notAvailable,
-      getL1ToL2Message: notAvailable, // l1 to l2 messages in public contexts TODO: https://github.com/AztecProtocol/aztec-packages/issues/616
+
+      // TODO(Maddiaa): both getL1ToL2 and getCommitment share alot of code with the private conterpart? could be refactored


are you planning to fix it in this PR or another one? If latter feel free to create an appropriate issue!

They've since diverged after merging read requests so this todo is stale, will remove

rahul-kothari · 2023-06-14T10:34:22Z

yarn-project/acir-simulator/src/public/executor.ts

@@ -72,6 +97,16 @@ export class PublicExecutor {
        this.log(`Oracle storage write: slot=${storageSlot.toShortString()} value=${value.toString()}`);
        return [toACVMField(newValue)];
      },
+      createCommitment: async ([commitment]) => {
+        this.log('Creating commitment: ' + commitment.toString());


do we need to print these long numbers?

Just keeping with the style of the other oracle calls where they log what they do

rahul-kothari · 2023-06-14T10:38:41Z

yarn-project/end-to-end/src/e2e_cross_chain_messaging.test.ts

@@ -70,7 +70,7 @@ describe('e2e_cross_chain_messaging', () => {
    tokenPortalAddress = contracts.tokenPortalAddress;
    await expectBalance(accounts[0], initialBalance);
    logger('Successfully deployed contracts and initialized portal');
-  }, 40_000);
+  }, 60_000);


why this change when nothing really changed here?

it just fails intermittently when running locally, increasing it to not have the pain of rerunning

rahul-kothari · 2023-06-14T10:44:52Z

yarn-project/noir-contracts/src/contracts/noir-aztec3/src/oracle/get_commitment.nr

@@ -0,0 +1,12 @@
+// Oracle function to get a commitment, its sibling path and index, without getting its preimage.
+


nittiest of all nit: extra empty line?

rahul-kothari · 2023-06-14T10:48:10Z

yarn-project/noir-contracts/src/contracts/public_private_contract/src/transparent_note.nr

+        pedersen([self.amount, self.secretHash])[0]
+    }
+
+    fn consume_in_private(self: Self, mut context: PrivateFunctionContext, root: Field, secret: Field) -> PrivateFunctionContext {


idk how I feel about this naming. Noir uses "secret" because when devs think "private", they would think of internal methods. Maybe we should use that too in our names? or is that overkill?

The name was to keep with other things being called private. Secret has stronger connotations, so ive renamed

rahul-kothari · 2023-06-14T10:52:20Z

yarn-project/noir-contracts/src/contracts/public_private_contract/src/main.nr

+
+        // Public oracle call to emit new commitment.
+        create_commitment(note.get_commitment());
+        0


do we need to return anything here at all?

originally when doing this public function required a return value, but that doesnt seem to be the case anymore! Will remove

rahul-kothari · 2023-06-14T10:52:35Z

yarn-project/noir-contracts/src/contracts/public_private_contract/src/main.nr

+
+        // Public oracle call to emit new commitment.
+        create_l2_to_l1_message(note.get_commitment());
+        0


why do we need to return anything here?

rahul-kothari · 2023-06-14T10:58:41Z

yarn-project/noir-contracts/src/contracts/public_private_contract/src/main.nr

+    }
+
+
+    fn mintFromPublicMessage(


should this not be a secret fn?

it is secret by default, the keyword doesnt exist in the lang yet

ludamad · 2023-06-14T21:01:34Z

Base automatically changed from md/commitments-in-public to master June 12, 2023 16:26

Maddiaa0 added 4 commits June 12, 2023 18:19

temp

e3c69bc

feat: create commitments and l1 messages in a public call

1242ff1

feat: consume public message in private context

28915ce

chore: formatting:fix

7dc2ca3

Maddiaa0 force-pushed the md/com-pub branch from ac1e483 to 7dc2ca3 Compare June 12, 2023 18:20

Maddiaa0 added 4 commits June 12, 2023 18:40

refactor: implement with read requests

230bf19

feat: e2e consume public commitment test

902fc72

chore(ci): add to circle ci

cccbf0a

chore: fmt

b0741d3

fix: fmt

Maddiaa0 force-pushed the md/com-pub branch from e721d86 to b0741d3 Compare June 13, 2023 11:44

Maddiaa0 added 2 commits June 13, 2023 14:38

fix: tidy up noir syntax

366a761

Merge branch 'master' into md/com-pub

a5fca67

Maddiaa0 marked this pull request as ready for review June 13, 2023 14:39

Maddiaa0 requested review from LHerskind and rahul-kothari June 13, 2023 14:39

fix: lint

331526d

LHerskind approved these changes Jun 13, 2023

View reviewed changes

Maddiaa0 mentioned this pull request Jun 13, 2023

feat(messaging): public cross chain message e2e #841

Merged

6 tasks

rahul-kothari approved these changes Jun 14, 2023

View reviewed changes

fix: update noir review comments

73a119a

Maddiaa0 mentioned this pull request Jun 14, 2023

spike: What slot should public > private messages be assigned to ? #847

Closed

Maddiaa0 added 2 commits June 14, 2023 11:54

fix: update nits

bc9b611

chore(fmt)

6c2310b

Maddiaa0 enabled auto-merge (squash) June 14, 2023 12:14

Maddiaa0 merged commit 27dc70f into master Jun 14, 2023

Maddiaa0 deleted the md/com-pub branch June 14, 2023 12:19

This was referenced Jun 20, 2023

feat(public->private): Noir syntax for public -> private message passing #809

Closed

feat(public->private): Make commitments trees available in the public execution context #784

Closed

This was referenced Jul 25, 2023

chore(aztec-sandbox): Release 0.2.0-alpha27 #1180

Closed

chore(aztec-sandbox): Release 0.2.0 #1181

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(public->private): create commitments in public contexts (in noir) #810

feat(public->private): create commitments in public contexts (in noir) #810

Maddiaa0 commented Jun 12, 2023 •

edited

Loading

LHerskind left a comment

LHerskind Jun 13, 2023

LHerskind Jun 13, 2023

Maddiaa0 Jun 13, 2023

LHerskind Jun 13, 2023

Maddiaa0 Jun 13, 2023

LHerskind Jun 13, 2023

Maddiaa0 Jun 14, 2023

rahul-kothari Jun 14, 2023

Maddiaa0 Jun 14, 2023

rahul-kothari Jun 14, 2023

Maddiaa0 Jun 14, 2023

rahul-kothari Jun 14, 2023

Maddiaa0 Jun 14, 2023

rahul-kothari Jun 14, 2023

Maddiaa0 Jun 14, 2023

rahul-kothari Jun 14, 2023

rahul-kothari Jun 14, 2023

Maddiaa0 Jun 14, 2023 •

edited

Loading

rahul-kothari Jun 14, 2023

Maddiaa0 Jun 14, 2023

rahul-kothari Jun 14, 2023

Maddiaa0 Jun 14, 2023

rahul-kothari Jun 14, 2023

Maddiaa0 Jun 14, 2023

ludamad commented Jun 14, 2023

		@@ -0,0 +1,12 @@
		// Oracle function to get a commitment, its sibling path and index, without getting its preimage.

feat(public->private): create commitments in public contexts (in noir) #810

feat(public->private): create commitments in public contexts (in noir) #810

Conversation

Maddiaa0 commented Jun 12, 2023 • edited Loading

Description

Checklist:

LHerskind left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Maddiaa0 Jun 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ludamad commented Jun 14, 2023

Maddiaa0 commented Jun 12, 2023 •

edited

Loading

Maddiaa0 Jun 14, 2023 •

edited

Loading