diff --git a/l1-contracts/test/portals/TokenPortal.sol b/l1-contracts/test/portals/TokenPortal.sol index c696b3ec09b..3b74d240ec8 100644 --- a/l1-contracts/test/portals/TokenPortal.sol +++ b/l1-contracts/test/portals/TokenPortal.sol @@ -27,8 +27,8 @@ contract TokenPortal { * @param _to - The aztec address of the recipient * @param _amount - The amount to deposit * @param _deadline - The timestamp after which the entry can be cancelled - * @param _secretHash - The hash of the secret consumable message - * @param _canceller - The address that can cancel the L1 to L2 message. + * @param _secretHash - The hash of the secret consumable message. The hash should be 254 bits (so it can fit in a Field element) + * @param _canceller - The address that can cancel the L1 to L2 message * @return The key of the entry in the Inbox */ function depositToAztecPublic( @@ -39,7 +39,6 @@ contract TokenPortal { address _canceller ) external payable returns (bytes32) { // Preamble - // @todo: (issue #624) handle different versions IInbox inbox = registry.getInbox(); DataStructures.L2Actor memory actor = DataStructures.L2Actor(l2TokenAddress, 1); @@ -61,7 +60,7 @@ contract TokenPortal { * @param _deadline - The timestamp after which the entry can be cancelled * @param _secretHashForL2MessageConsumption - The hash of the secret consumable L1 to L2 message. The hash should be 254 bits (so it can fit in a Field element) * @param _secretHashForL2MessageConsumption - The hash of the secret to redeem minted notes privately on Aztec. The hash should be 254 bits (so it can fit in a Field element) - * @param _canceller - The address that can cancel the L1 to L2 message. + * @param _canceller - The address that can cancel the L1 to L2 message * @return The key of the entry in the Inbox */ function depositToAztecPrivate( @@ -72,7 +71,6 @@ contract TokenPortal { address _canceller ) external payable returns (bytes32) { // Preamble - // @todo: (issue #624) handle different versions IInbox inbox = registry.getInbox(); DataStructures.L2Actor memory actor = DataStructures.L2Actor(l2TokenAddress, 1); @@ -113,7 +111,6 @@ contract TokenPortal { bytes32 _secretHash, uint64 _fee ) external returns (bytes32) { - // @todo: (issue #624) handle different versions IInbox inbox = registry.getInbox(); DataStructures.L1Actor memory l1Actor = DataStructures.L1Actor(address(this), block.chainid); DataStructures.L2Actor memory l2Actor = DataStructures.L2Actor(l2TokenAddress, 1); @@ -151,7 +148,6 @@ contract TokenPortal { bytes32 _secretHashForRedeemingMintedNotes, uint64 _fee ) external returns (bytes32) { - // @todo: (issue #624) handle different versions IInbox inbox = registry.getInbox(); DataStructures.L1Actor memory l1Actor = DataStructures.L1Actor(address(this), block.chainid); DataStructures.L2Actor memory l2Actor = DataStructures.L2Actor(l2TokenAddress, 1); @@ -206,7 +202,6 @@ contract TokenPortal { ) }); - // @todo: (issue #624) handle different versions bytes32 entryKey = registry.getOutbox().consume(message); underlying.transfer(_recipient, _amount); diff --git a/l1-contracts/test/portals/TokenPortal.t.sol b/l1-contracts/test/portals/TokenPortal.t.sol index 93822b08c92..d914124e607 100644 --- a/l1-contracts/test/portals/TokenPortal.t.sol +++ b/l1-contracts/test/portals/TokenPortal.t.sol @@ -48,8 +48,10 @@ contract TokenPortalTest is Test { bytes32 internal to = bytes32(0x2d749407d8c364537cdeb799c1574929cb22ff1ece2b96d2a1c6fa287a0e0171); uint256 internal amount = 100; uint256 internal mintAmount = 1 ether; + // this hash is just a random 32 byte string bytes32 internal secretHashForL2MessageConsumption = 0x147e4fec49805c924e28150fc4b36824679bc17ecb1d7d9f6a9effb7fde6b6a0; + // this hash is just a random 32 byte string bytes32 internal secretHashForRedeemingMintedNotes = 0x157e4fec49805c924e28150fc4b36824679bc17ecb1d7d9f6a9effb7fde6b6a0; uint64 internal bid = 1 ether; @@ -123,7 +125,7 @@ contract TokenPortalTest is Test { _createExpectedMintPrivateL1ToL2Message(address(this)); bytes32 expectedEntryKey = inbox.computeEntryKey(expectedMessage); - // Check the even was emitted + // Check the event was emitted vm.expectEmit(true, true, true, true); // event we expect emit MessageAdded( @@ -166,7 +168,7 @@ contract TokenPortalTest is Test { _createExpectedMintPublicL1ToL2Message(address(this)); bytes32 expectedEntryKey = inbox.computeEntryKey(expectedMessage); - // Check the even was emitted + // Check the event was emitted vm.expectEmit(true, true, true, true); // event we expect emit MessageAdded( diff --git a/yarn-project/aztec-nr/aztec/src/types/address.nr b/yarn-project/aztec-nr/aztec/src/types/address.nr index bf216f0c15f..6849767b8d0 100644 --- a/yarn-project/aztec-nr/aztec/src/types/address.nr +++ b/yarn-project/aztec-nr/aztec/src/types/address.nr @@ -29,7 +29,7 @@ impl EthereumAddress { // Check that it actually will fit. Spending a lot of constraints here :grimacing: let bytes = address.to_be_bytes(32); for i in 0..12 { - assert(bytes[i] == 0, "Value too large for SafeU120"); + assert(bytes[i] == 0, "Value too large for an ethereum address"); } Self { address diff --git a/yarn-project/end-to-end/src/e2e_cross_chain_messaging.test.ts b/yarn-project/end-to-end/src/e2e_cross_chain_messaging.test.ts index 3e23f58f7b9..76b18022a0f 100644 --- a/yarn-project/end-to-end/src/e2e_cross_chain_messaging.test.ts +++ b/yarn-project/end-to-end/src/e2e_cross_chain_messaging.test.ts @@ -167,12 +167,10 @@ describe('e2e_cross_chain_messaging', () => { .withWallet(user2Wallet) .methods.claim_private( bridgeAmount, + secretHashForL2MessageConsumption, + { address: ethAccount.toField() }, messageKey, secretForL2MessageConsumption, - secretHashForL2MessageConsumption, - { - address: ethAccount.toField(), - }, ) .simulate(), ).rejects.toThrowError("Cannot satisfy constraint 'l1_to_l2_message_data.message.content == content"); @@ -182,12 +180,10 @@ describe('e2e_cross_chain_messaging', () => { .withWallet(user2Wallet) .methods.claim_private( bridgeAmount, + secretHashForRedeemingMintedNotes, + { address: ethAccount.toField() }, messageKey, secretForL2MessageConsumption, - secretHashForRedeemingMintedNotes, - { - address: ethAccount.toField(), - }, ) .send(); const consumptionReceipt = await consumptionTx.wait(); @@ -217,9 +213,9 @@ describe('e2e_cross_chain_messaging', () => { l2Bridge .withWallet(user1Wallet) .methods.exit_to_l1_private( + { address: ethAccount.toField() }, { address: l2Token.address }, withdrawAmount, - { address: ethAccount.toField() }, { address: EthAddress.ZERO.toField() }, nonce, ) diff --git a/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts b/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts index c6dce6d5c11..82afbdce887 100644 --- a/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts +++ b/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts @@ -151,9 +151,13 @@ describe('e2e_public_cross_chain_messaging', () => { await expect( l2Bridge .withWallet(user2Wallet) - .methods.claim_public({ address: user2Wallet.getAddress() }, bridgeAmount, messageKey, secret, { - address: ownerEthAddress.toField(), - }) + .methods.claim_public( + { address: user2Wallet.getAddress() }, + bridgeAmount, + { address: ownerEthAddress.toField() }, + messageKey, + secret, + ) .simulate(), ).rejects.toThrow(); @@ -161,9 +165,13 @@ describe('e2e_public_cross_chain_messaging', () => { logger("user2 consumes owner's message on L2 Publicly"); const tx = l2Bridge .withWallet(user2Wallet) - .methods.claim_public({ address: ownerAddress }, bridgeAmount, messageKey, secret, { - address: ownerEthAddress.toField(), - }) + .methods.claim_public( + { address: ownerAddress }, + bridgeAmount, + { address: ownerEthAddress.toField() }, + messageKey, + secret, + ) .send(); const receipt = await tx.wait(); expect(receipt.status).toBe(TxStatus.MINED); @@ -183,8 +191,8 @@ describe('e2e_public_cross_chain_messaging', () => { l2Bridge .withWallet(ownerWallet) .methods.exit_to_l1_public( - withdrawAmount, { address: ownerEthAddress.toField() }, + withdrawAmount, { address: EthAddress.ZERO.toField() }, nonce, ) diff --git a/yarn-project/end-to-end/src/fixtures/cross_chain_test_harness.ts b/yarn-project/end-to-end/src/fixtures/cross_chain_test_harness.ts index 0889a3d501a..ca6866dafb0 100644 --- a/yarn-project/end-to-end/src/fixtures/cross_chain_test_harness.ts +++ b/yarn-project/end-to-end/src/fixtures/cross_chain_test_harness.ts @@ -211,16 +211,20 @@ export class CrossChainTestHarness { async consumeMessageOnAztecAndMintSecretly( bridgeAmount: bigint, + secretHashForRedeemingMintedNotes: Fr, messageKey: Fr, secretForL2MessageConsumption: Fr, - secretHashForL2MessageConsumption: Fr, ) { this.logger('Consuming messages on L2 secretively'); // Call the mint tokens function on the Aztec.nr contract const consumptionTx = this.l2Bridge.methods - .claim_private(bridgeAmount, messageKey, secretForL2MessageConsumption, secretHashForL2MessageConsumption, { - address: this.ethAccount.toField(), - }) + .claim_private( + bridgeAmount, + secretHashForRedeemingMintedNotes, + { address: this.ethAccount.toField() }, + messageKey, + secretForL2MessageConsumption, + ) .send(); const consumptionReceipt = await consumptionTx.wait(); expect(consumptionReceipt.status).toBe(TxStatus.MINED); @@ -230,9 +234,13 @@ export class CrossChainTestHarness { this.logger('Consuming messages on L2 Publicly'); // Call the mint tokens function on the Aztec.nr contract const tx = this.l2Bridge.methods - .claim_public({ address: this.ownerAddress }, bridgeAmount, messageKey, secret, { - address: this.ethAccount.toField(), - }) + .claim_public( + { address: this.ownerAddress }, + bridgeAmount, + { address: this.ethAccount.toField() }, + messageKey, + secret, + ) .send(); const receipt = await tx.wait(); expect(receipt.status).toBe(TxStatus.MINED); @@ -241,9 +249,9 @@ export class CrossChainTestHarness { async withdrawPrivateFromAztecToL1(withdrawAmount: bigint, nonce: Fr = Fr.ZERO) { const withdrawTx = this.l2Bridge.methods .exit_to_l1_private( + { address: this.ethAccount.toField() }, { address: this.l2Token.address }, withdrawAmount, - { address: this.ethAccount.toField() }, { address: EthAddress.ZERO.toField() }, nonce, ) @@ -255,8 +263,8 @@ export class CrossChainTestHarness { async withdrawPublicFromAztecToL1(withdrawAmount: bigint, nonce: Fr = Fr.ZERO) { const withdrawTx = this.l2Bridge.methods .exit_to_l1_public( - withdrawAmount, { address: this.ethAccount.toField() }, + withdrawAmount, { address: EthAddress.ZERO.toField() }, nonce, ) diff --git a/yarn-project/noir-contracts/src/contracts/token_bridge_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/token_bridge_contract/src/main.nr index afad9569c5d..07ac537644c 100644 --- a/yarn-project/noir-contracts/src/contracts/token_bridge_contract/src/main.nr +++ b/yarn-project/noir-contracts/src/contracts/token_bridge_contract/src/main.nr @@ -4,7 +4,7 @@ mod token_interface; // Minimal implementation of the token bridge that can move funds between L1 <> L2. // The bridge has a corresponding Portal contract on L1 that it is attached to // And corresponds to a Token on L2 that uses the `AuthWit` accounts pattern. -// Bridge has to be set as a minter on the token before it can be sued +// Bridge has to be set as a minter on the token before it can be used contract TokenBridge { use dep::aztec::{ @@ -50,13 +50,13 @@ contract TokenBridge { fn claim_public( to: AztecAddress, amount: Field, + canceller: EthereumAddress, msg_key: Field, secret: Field, - canceller: EthereumAddress, ) -> Field { let storage = Storage::init(Context::public(&mut context)); - let content_hash = get_mint_public_content_hash(amount, to.address, canceller.address); + let content_hash = get_mint_public_content_hash(to.address, amount, canceller.address); // Consume message and emit nullifier context.consume_l1_to_l2_message(msg_key, content_hash, secret); @@ -70,15 +70,15 @@ contract TokenBridge { // Requires `msg.sender` to give approval to the bridge to burn tokens on their behalf using witness signatures #[aztec(public)] fn exit_to_l1_public( - amount: Field, recipient: EthereumAddress, // ethereum address to withdraw to + amount: Field, callerOnL1: EthereumAddress, // ethereum address that can call this function on the L1 portal (0x0 if anyone can call) - nonce: Field, + nonce: Field, // nonce used in the approval message by `msg.sender` to let bridge burn their tokens on L2 ) -> Field { let storage = Storage::init(Context::public(&mut context)); // Send an L2 to L1 message - let content = get_withdraw_content_hash(amount, recipient.address, callerOnL1.address); + let content = get_withdraw_content_hash(recipient.address, amount, callerOnL1.address); context.message_portal(content); // Burn tokens @@ -92,19 +92,19 @@ contract TokenBridge { #[aztec(private)] fn claim_private( amount: Field, - msg_key: Field, // L1 to L2 message key as derived from the inbox contract - secret_for_L1_to_L2_message_consumption: Field, // secret used to consume the L1 to L2 message secret_hash_for_redeeming_minted_notes: Field, // secret hash used to redeem minted notes at a later time. This enables anyone to call this function and mint tokens to a user on their behalf canceller: EthereumAddress, + msg_key: Field, // L1 to L2 message key as derived from the inbox contract + secret_for_L1_to_L2_message_consumption: Field, // secret used to consume the L1 to L2 message ) -> Field { // Consume L1 to L2 message and emit nullifier let content_hash = get_mint_private_content_hash(amount, secret_hash_for_redeeming_minted_notes, canceller.address); context.consume_l1_to_l2_message(msg_key, content_hash, secret_for_L1_to_L2_message_consumption); // Mint tokens on L2 - // We hash the secret privately and send the hash in a public call so the secret isn't leaked - // Furthermore, `mint_private` on token is public. So we can an internal public function - // which then calls the token contract + // `mint_private` on token is public. So we call an internal public function + // which then calls the public method on the token contract. + // Since the secret_hash is passed, no secret is leaked. context.call_public_function( context.this_address(), compute_selector("_call_mint_on_token(Field,Field)"), @@ -115,17 +115,17 @@ contract TokenBridge { } // Burns the appropriate amount of tokens and creates a L2 to L1 withdraw message privately - // Requires `from` to give approval to the bridge to burn tokens on their behalf using witness signatures + // Requires `msg.sender` (caller of the method) to give approval to the bridge to burn tokens on their behalf using witness signatures #[aztec(private)] fn exit_to_l1_private( + recipient: EthereumAddress, // ethereum address to withdraw to token: AztecAddress, amount: Field, - recipient: EthereumAddress, // ethereum address to withdraw to callerOnL1: EthereumAddress, // ethereum address that can call this function on the L1 portal (0x0 if anyone can call) - nonce: Field, + nonce: Field, // nonce used in the approval message by `msg.sender` to let bridge burn their tokens on L2 ) -> Field { // Send an L2 to L1 message - let content = get_withdraw_content_hash(amount, recipient.address, callerOnL1.address); + let content = get_withdraw_content_hash(recipient.address, amount, callerOnL1.address); context.message_portal(content); // Assert that user provided token address is same as seen in storage. diff --git a/yarn-project/noir-contracts/src/contracts/token_bridge_contract/src/util.nr b/yarn-project/noir-contracts/src/contracts/token_bridge_contract/src/util.nr index a9203f4fe24..7ecb7d547cd 100644 --- a/yarn-project/noir-contracts/src/contracts/token_bridge_contract/src/util.nr +++ b/yarn-project/noir-contracts/src/contracts/token_bridge_contract/src/util.nr @@ -49,7 +49,7 @@ fn get_mint_private_content_hash(amount: Field, secret_hash_for_redeeming_minted // Computes a content hash of a deposit/mint_public message. // Refer TokenPortal.sol for reference on L1. -fn get_mint_public_content_hash(amount: Field, owner_address: Field, canceller: Field) -> Field { +fn get_mint_public_content_hash(owner_address: Field, amount: Field, canceller: Field) -> Field { let mut hash_bytes: [u8; 100] = [0; 100]; let amount_bytes = amount.to_be_bytes(32); let recipient_bytes = owner_address.to_be_bytes(32); @@ -86,7 +86,7 @@ fn get_mint_public_content_hash(amount: Field, owner_address: Field, canceller: } // Computes a content hash of a withdraw message. -fn get_withdraw_content_hash(amount: Field, recipient: Field, callerOnL1: Field) -> Field { +fn get_withdraw_content_hash(recipient: Field, amount: Field, callerOnL1: Field) -> Field { // Compute the content hash // Compute sha256(selector || amount || recipient) // then convert to a single field element