feat: bridge l1 assets to l2 acvm by Maddiaa0 · Pull Request #578 · AztecProtocol/aztec-packages

Maddiaa0 · 2023-05-15T14:24:40Z

Description

Overview

Adds a series of functionality to the acvm as well as a noir contract that can consume an l1 to l2 message and create a nullifier for it.

Creates a cbind to recreate a secret message hash which has its own pedesen generator index.
Create a small TokenPortal that is capable of holding assets and sending messages instructing tokens to be minted on the L2.
ACVM oracle opcodes

getL1ToL2Message: The implementation of this is currently stubbed, but this opcode fetches a message based on its msgKey, and returns key information to the noir context, essentially the same info that is included within the solidity struct
emitData: This is a utility function to aid development, it allows us to log out values from noir when the acvm is simulating them. I found it useful for debugging and think it should be kept for other people to use too.

Adds a test to consume an l1 to l2 message, mocking the stubbed getL1ToL2Message mentioned above
Begins an e2e test that mints some tokens and has the scaffolding to make the corresponding noir call. however, in a following pr the sequencer side of this will be added.

Notes

Sha256 inside the circuit is unrolled, similar to lasse's implementation as the loops be quite slow
removes use of const p = prime_order from typescript and replaces it with the static field Fr.MODULO which is the same

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: rename

rahul-kothari · 2023-05-17T11:32:18Z

+{
+    NT::fr message_secret;
+    read(secret, message_secret);
+    // TODO(sean): This is not using the generator correctly and is unsafe, update


wdym? does this have an issue we are tracking it in?

Think it is because Noir currently does not support the generators specified as we are doing in cpp. The message_secret is to be used in noir, so @Maddiaa0 using this method to have a hash that can be reproduced inside and outside noir.

It's todo with not being able to use pedersen generator indexes natively inside noir yet.

[index, ...hashValues], is not the same as [hashValues], hashed under a different generator index.

Alvaro has added the functionality to noir but it has not made it into the aztec3 branch yet so cannot be used here, (branch: https://github.com/noir-lang/noir/tree/arv/pedersen_with_hash_index)

I've made an issue for this here: #610

rahul-kothari · 2023-05-17T11:34:01Z

@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: Apache-2.0


why do you have 2 IInbox interfaces?

Seems this was left over from the merge, as your previous pr had it in a different location it did not cop (me also) that they were really the same file.
This has been addressed and they have been removed

rahul-kothari · 2023-05-17T11:36:25Z

+
+    // Check that the message is in the inbox
+    IMessageBox.Entry memory entry = IInbox(inbox).get(entryKey);
+    assertEq(entry.count, 1);


nit; you may also check if the corresponding event (MessageAdded) got fired

rahul-kothari · 2023-05-17T11:40:21Z

+    let recipientPk: Buffer;
+    let recipient: NoirPoint;
+
+    const buildMessage = async (content: Fr[], targetContract: AztecAddress, secret: Fr) => {


nit: rename this to buildL1ToL2Message

rahul-kothari · 2023-05-17T11:41:10Z

+        AztecAddress.random(),
+        contractAddress,
+        new FunctionData(Buffer.alloc(4), true, true),
+        // BUG: placing a fr in args will result in a fr wrapped in an fr


have you flagged this issue somewhere?

Ah I have not, but it is a known issue, #611 made this for tracking

If you put the issue in the comment would be neat 👍

rahul-kothari · 2023-05-17T11:41:33Z

  private writeInputs() {
    const argsSize = this.abi.parameters.reduce((acc, param) => acc + sizeOfType(param.type), 0);

+    console.log('storage contract address: ', this.callContext.storageContractAddress);


nit: remove

rahul-kothari · 2023-05-17T11:44:44Z

 global NoteStorageSlot = 3;
 global MappingStorageSlot = 4;
 global Nullifier = 5;
+global L1ToL2MessageSecret = 29;


I am out of the loop - why is this 29? is this the pedereson generator constant?

Its index 29 here:

aztec-packages/circuits/cpp/src/aztec3/constants.hpp

Line 89 in 35b3b7c

L1_TO_L2_MESSAGE_SECRET

rahul-kothari · 2023-05-17T11:49:33Z

+        secret: pub Field,
+    ) -> pub [Field; dep::aztec3::abi::PUBLIC_INPUTS_LENGTH] {
+        let mut context = PrivateFunctionContext::new();
+        context.args = context.args.push_array([amount, owner.x, owner.y, msg_key, secret]);


damn is this how we create private vars in noir now?

haha, nah its just to get around a noir mutability quirk when constructing the context for the kernel circuit

rahul-kothari · 2023-05-17T11:50:52Z

+
+// Checks is a msg is within the l1Msg tree
+#[oracle(getL1ToL2Message)]
+fn get_l1_to_l2_msg_oracle(_msg_key: Field) -> [Field; L1_MESSAGE_ORACLE_CALL_LENGTH] {}


question so I understand this better -> how does this work?

In the acvm you can register callbacks:

aztec-packages/yarn-project/acir-simulator/src/acvm/acvm.ts

Line 21 in e61b65f

export interface ACIRCallback {

When making an oracle call, it will look for the callback with the given name, then inject into this function response / send the inputs to the ts context. You can see in the tests where you can mock what the function returns.

rahul-kothari · 2023-05-17T11:56:11Z

+        hash_bytes[30] = amount_bytes[30];
+        hash_bytes[31] = amount_bytes[31];
+
+        hash_bytes[0 + 32] = recipient_bytes[0];


BAHAHHAHAH damn :/

LHerskind

A bit inconsistenct on L1 or L1ToL2 naming, but nice job 👍

LHerskind · 2023-05-18T09:22:12Z

-  "@aztec/verifier/=lib/aztec-verifier-contracts/src/"
+  "@aztec/interfaces/=src/interfaces/",
+  "@aztec/verifier/=lib/aztec-verifier-contracts/src/",
+  "@aztec/mocks/=/src/mock/"


This seems strange. Why is there both a mock and mocks now? And the interfaces don't seem to exist here, is in the core/interfaces?

LHerskind · 2023-05-18T09:23:33Z

 import {MockVerifier} from "@aztec/mock/MockVerifier.sol";
 import {Decoder} from "./Decoder.sol";

+// Messaging


Think this needs a rebase to get the updates from #609

LHerskind · 2023-05-18T09:25:26Z

 pragma solidity >=0.8.18;

 library DataStructures {
+  // Prime field modulus


Think this is more fitting in a Constants.sol as it is not a data structure. In #552 I added one after your comment.

LHerskind · 2023-05-18T09:30:15Z

+  {
+    // Preamble
+    IInbox inbox = REGISTRY.getInbox();
+    DataStructures.L2Actor memory actor = DataStructures.L2Actor(_to, 1);


Using the same _to here means that the messages is received by the same as will be receiving funds. Won't you just lock the funds forever in the rollup to belong to the token contract itself.

Think the L2Actor aztec address should not be specified by the user, but from storage in the contract, e.g., use an initialize function or something for making the "link" to the L2 contract and use that value for the actor, then the user passed in _to can be part of the content.

Yep, this is correct

LHerskind · 2023-05-18T09:34:05Z

+   * @param msgKey - A buffer representing the message key.
+   * @returns The message data
+   */
+  public async getL1ToL2Message(msgKey: Fr) {


A specified return type would be neat.

LHerskind · 2023-05-18T10:28:16Z

+    // Deposit tokens to the TokenPortal
+    const contractString = deployedContract.address.toString() as `0x${string}`;
+    const secretString = `0x${claimSecretHash.toBuffer().toString('hex')}` as `0x${string}`;
+    const deadline = 4_294_967_295 - 1; // max uint - 1


Why do you want max uint - 1? The maxuint is already 2**32 - 1

LHerskind · 2023-05-18T10:29:47Z

    "noir:build:zk_token": "(cd src/contracts/zk_token_contract && nargo compile main --contracts) && ts-node --esm src/scripts/copy_output.ts zk_token",
-    "noir:build:pub_token": "(cd src/contracts/public_token_contract && nargo compile main --contracts) && ts-node --esm src/scripts/copy_output.ts public_token"
+    "noir:build:pub_token": "(cd src/contracts/public_token_contract && nargo compile main --contracts) && ts-node --esm src/scripts/copy_output.ts public_token",
+    "noir:build:l1_token": "(cd src/contracts/non_native_token_contract && nargo compile main --contracts) && ts-node --esm src/scripts/copy_output.ts non_native_token"


Think you run into size limits.

LHerskind · 2023-05-18T10:33:48Z

+
+// TODO: Still need to check that the secret hash -> note hash
+// this should be another pedersen generator ?
+global L1_MESSAGE_LEN = 8;


For consistency, think it would be nice if it is referred to as L1_TO_L2, generally for this file some places it is L1Message but and other L1ToL2.

LHerskind · 2023-05-18T10:34:50Z

+    }
+
+    fn compute_msg_hash(_self: Self) -> Field {
+        // Todo a sha256 hash of the message


Is there an issue for this 👀?

Implementing in current interation

rahul-kothari and others added 24 commits May 15, 2023 10:28

inbox and message box contracts

e750199

outbox

a980f43

address PR comments

cb3ce28

add registry

293acc3

s/keccak/sha and optimise insert()

9e9e085

chore: move reused test mnemonic into a fixtures file

44bb000

e2e mock solidity contracts checkpoint

a84151a

feat: draft noir circuit to consume l1 message

c3d2738

rename l1_token -> non native

c9ccbfd

fix: rename

feat: add pedersen generator for l1 to l2 message secret

12372dc

Merge branch 'rk/inbox' into md/scaffold-l1-l2-e2e

053d49d

feat: create interfaces

9bf82e9

feat: scaffold test

353ad52

feat: initial sol token contract tests

d2cb8eb

feat: add getL1ToL2Message oracle call

2dfd851

feat: consume dummy cross chain message test

cc20181

temp

54c5a6d

Merge branch 'master' into md/scaffold-l1-l2-e2e

70043ca

fix: merge remedy

b3f7e70

fix: yarn prepare

5a3e1be

fix: merge update contracts

1abf3d0

feat: scaffold e2e as far as possible, include contentHash in acvm

8203d41

fix: jsdoc l1_to_l2_message

29f9ec4

fix: forge fmt

39336a2

Maddiaa0 requested review from LHerskind and rahul-kothari May 17, 2023 10:52

Maddiaa0 added 2 commits May 17, 2023 11:50

fix: formatting acir sim

22f4ee9

refactor : remove naked modulus declarations

134f6af

rahul-kothari reviewed May 17, 2023

View reviewed changes

fix: post merge solidity cleanup

d83863c

Maddiaa0 added 3 commits May 17, 2023 12:33

fix: convert msgKey to a field element as it will always be one

6af592a

chore(fmt): forge fmt baby

35b3b7c

fix type

b6eb3a7

Maddiaa0 changed the title ~~feat: (wip) bridge l1 assets to l2 test~~ feat: bridge l1 assets to l2 acvm May 17, 2023

Maddiaa0 marked this pull request as ready for review May 17, 2023 16:31

Maddiaa0 added 2 commits May 17, 2023 17:21

nit: check for emitted event

a477f8d

Merge branch 'master' into md/scaffold-l1-l2-e2e

1f9ca70

Maddiaa0 requested a review from rahul-kothari May 17, 2023 17:33

Maddiaa0 added 3 commits May 17, 2023 17:44

fix: post merge

532faa4

fix: we please the linter

82cccb0

fix: bruh

f481b3d

LHerskind reviewed May 18, 2023

View reviewed changes

Maddiaa0 added 7 commits May 18, 2023 13:29

fix: refactor contract to be initialisable, fix too issue

19f36d9

refactor(nr): emit_data -> debug_log

ac2c933

nits

480ea67

Merge branch 'master' into md/scaffold-l1-l2-e2e

fd98dfb

fix: post merge spring cleaning

9c6cff7

feat: keccak(message[0:4]) in messages

e57d0bd

fix: i stg mr format

d9733bf

LHerskind approved these changes May 18, 2023

View reviewed changes

add .skip to unfinished e2e test

941d451

Maddiaa0 merged commit a19c36e into master May 18, 2023

Maddiaa0 deleted the md/scaffold-l1-l2-e2e branch May 18, 2023 15:06

ludamad pushed a commit that referenced this pull request Jul 14, 2023

fix: Avoid bb.js memory issues. (#578)

05e2636

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

Conversation

Maddiaa0 commented May 15, 2023 • edited by LHerskind Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Overview

Notes

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Maddiaa0 May 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Maddiaa0 May 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Maddiaa0 May 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LHerskind left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Maddiaa0 commented May 15, 2023 •

edited by LHerskind

Loading

Maddiaa0 May 17, 2023 •

edited

Loading

Maddiaa0 May 17, 2023 •

edited

Loading

Maddiaa0 May 17, 2023 •

edited

Loading