PR comments

yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts

@@ -30,7 +30,7 @@ import {
 import { mnemonicToAccount } from 'viem/accounts';
 import { Chain, foundry } from 'viem/chains';
 
-import { deployAndInitializeStandardizedTokenAndBridgeContracts, deployL1Contract, hashPayload } from './utils.js';
+import { deployAndInitializeTokenAndBridgeContracts, deployL1Contract, hashPayload } from './utils.js';
 
 const logger = createDebugLogger('aztec:canary');
 
@@ -67,7 +67,7 @@ async function deployAllContracts(
 ) {
   const l1ContractsAddresses = await getL1ContractAddresses(pxeRpcUrl);
   logger('Deploying DAI Portal, initializing and deploying l2 contract...');
-  const daiContracts = await deployAndInitializeStandardizedTokenAndBridgeContracts(
+  const daiContracts = await deployAndInitializeTokenAndBridgeContracts(
     ownerWallet,
     walletClient,
     publicClient,
@@ -81,7 +81,7 @@ async function deployAllContracts(
   const daiTokenPortalAddress = daiContracts.tokenPortalAddress;
 
   logger('Deploying WETH Portal, initializing and deploying l2 contract...');
-  const wethContracts = await deployAndInitializeStandardizedTokenAndBridgeContracts(
+  const wethContracts = await deployAndInitializeTokenAndBridgeContracts(
     ownerWallet,
     walletClient,
     publicClient,
@@ -296,8 +296,7 @@ describe('uniswap_trade_on_l1_from_l2', () => {
     // Wait for the archiver to process the message
     await sleep(5000);
     // send a transfer tx to force through rollup with the message included
-    const transferAmount = 0n;
-    await transferWethOnL2(wethL2Contract, ownerAddress, receiver, transferAmount);
+    await transferWethOnL2(wethL2Contract, ownerAddress, receiver, 0n);
 
     // 3. Claim WETH on L2
     logger('Minting weth on L2');
@@ -310,11 +309,7 @@ describe('uniswap_trade_on_l1_from_l2', () => {
       secretForMintingWeth,
     );
     await redeemShieldPrivatelyOnL2(wethL2Contract, ownerAddress, wethAmountToBridge, secretForRedeemingWeth);
-    await expectPrivateBalanceOnL2(
-      ownerAddress,
-      wethAmountToBridge + BigInt(ownerInitialBalance) - transferAmount,
-      wethL2Contract,
-    );
+    await expectPrivateBalanceOnL2(ownerAddress, wethAmountToBridge + BigInt(ownerInitialBalance), wethL2Contract);
 
     // Store balances
     const wethL2BalanceBeforeSwap = await getL2PrivateBalanceOf(ownerAddress, wethL2Contract);
@@ -397,7 +392,7 @@ describe('uniswap_trade_on_l1_from_l2', () => {
     // Wait for the archiver to process the message
     await sleep(5000);
     // send a transfer tx to force through rollup with the message included
-    await transferWethOnL2(wethL2Contract, ownerAddress, receiver, transferAmount);
+    await transferWethOnL2(wethL2Contract, ownerAddress, receiver, 0n);
 
     // 7. claim dai on L2
     logger('Consuming messages to mint dai on L2');

yarn-project/canary/src/utils.ts

@@ -17,7 +17,7 @@ import { Account, Chain, Hex, HttpTransport, PublicClient, WalletClient, getCont
  * @param underlyingERC20Address - address of the underlying ERC20 contract to use (if none supplied, it deploys one)
  * @returns l2 contract instance, bridge contract instance, token portal instance, token portal address and the underlying ERC20 instance
  */
-export async function deployAndInitializeStandardizedTokenAndBridgeContracts(
+export async function deployAndInitializeTokenAndBridgeContracts(
   wallet: Wallet,
   walletClient: WalletClient<HttpTransport, Chain, Account>,
   publicClient: PublicClient<HttpTransport, Chain>,

yarn-project/end-to-end/src/fixtures/cross_chain_test_harness.ts

@@ -12,7 +12,7 @@ import { PXE, TxStatus } from '@aztec/types';
 
 import { Chain, HttpTransport, PublicClient, getContract } from 'viem';
 
-import { deployAndInitializeStandardizedTokenAndBridgeContracts } from './utils.js';
+import { deployAndInitializeTokenAndBridgeContracts } from './utils.js';
 
 /**
  * A Class for testing cross chain interactions, contains common interactions
@@ -43,7 +43,7 @@ export class CrossChainTestHarness {
 
     // Deploy and initialize all required contracts
     logger('Deploying and initializing token, portal and its bridge...');
-    const contracts = await deployAndInitializeStandardizedTokenAndBridgeContracts(
+    const contracts = await deployAndInitializeTokenAndBridgeContracts(
       wallet,
       walletClient,
       publicClient,

yarn-project/end-to-end/src/fixtures/utils.ts

@@ -351,7 +351,7 @@ export function getLogger() {
  * @param underlyingERC20Address - address of the underlying ERC20 contract to use (if none supplied, it deploys one)
  * @returns l2 contract instance, bridge contract instance, token portal instance, token portal address and the underlying ERC20 instance
  */
-export async function deployAndInitializeStandardizedTokenAndBridgeContracts(
+export async function deployAndInitializeTokenAndBridgeContracts(
   wallet: Wallet,
   walletClient: WalletClient<HttpTransport, Chain, Account>,
   publicClient: PublicClient<HttpTransport, Chain>,

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

@@ -138,19 +138,18 @@ contract TokenBridge {
         1
     }
     /// docs:end:exit_to_l1_private
-
-    // /// Unconstrained /// 
 
-    unconstrained fn token() -> Field {
+    // View function that is callable by other contracts.
+    // Unconstrained can't be called by others since it isn't safe. 
+    #[aztec(public)]
+    fn get_token() -> Field {
         storage.token.read()
     }
+
+    // /// Unconstrained /// 
 
-    // Since unconstrained functions can't be called by external contracts, this is a public function that can be called by other contracts.
-    #[aztec(public)]
-    fn get_token() -> Field {
-        // TODO: Figure out how to call unconstrained fn `token()` from here.
+    unconstrained fn token() -> Field {
         storage.token.read()
-
     }
 
     /// SHOULD BE Internal ///

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

@@ -21,7 +21,7 @@ contract Uniswap {
     };
 
     use crate::interfaces::{Token, TokenBridge};
-    use crate::util::{compute_message_hash, compute_swap_content_hash};
+    use crate::util::{compute_message_hash, compute_swap_private_content_hash};
 
     struct Storage {
         // like with account contracts, stores the approval message on a slot and tracks if they are active
@@ -68,10 +68,6 @@ contract Uniswap {
         caller_on_L1: EthereumAddress, // ethereum address that can call this function on the L1 portal (0x0 if anyone can call)
     ) -> Field {
 
-        // Assert that user provided token address is same as expected by token bridge.
-        // we can't directly use `input_asset_bridge.token` because that is a public method and public can't return data to private
-        context.call_public_function(context.this_address(), compute_selector("_assert_token_is_same(Field,Field)"), [input_asset.address, input_asset_bridge.address]);
-
         // Transfer funds to this contract
         Token::at(input_asset.address).unshield(
             &mut context,
@@ -84,8 +80,8 @@ contract Uniswap {
         // Approve bridge to burn this contract's funds and exit to L1 Uniswap Portal
         context.call_public_function(
             context.this_address(), 
-            compute_selector("_approve_bridge_and_exit_input_asset_to_L1((Field),Field)"), 
-            [input_asset_bridge.address, input_amount],
+            compute_selector("_approve_bridge_and_exit_input_asset_to_L1((Field),(Field),Field)"), 
+            [input_asset.address, input_asset_bridge.address, input_amount],
         );
 
         // Create swap message and send to Outbox for Uniswap Portal
@@ -95,8 +91,7 @@ contract Uniswap {
         assert(input_asset_bridge_portal_address != 0, "L1 portal address of input_asset's bridge is 0");
         assert(output_asset_bridge_portal_address != 0, "L1 portal address of output_asset's bridge is 0");
 
-        let content_hash = compute_swap_content_hash(
-            true,
+        let content_hash = compute_swap_private_content_hash(
             input_asset_bridge_portal_address,
             input_amount,
             uniswap_fee_tier,
@@ -132,15 +127,17 @@ contract Uniswap {
     // this method is used for both private and public swaps.
     #[aztec(public)]
     internal fn _approve_bridge_and_exit_input_asset_to_L1(
+        token: AztecAddress,
         token_bridge: AztecAddress, 
         amount: Field,
     ) {
-        let token = TokenBridge::at(token_bridge.address).token(context);
+        // Assert that user provided token address is same as expected by token bridge.
+        assert(token.address == (TokenBridge::at(token_bridge.address).token(context)), "input_asset address is not the same as seen in the bridge contract");
 
         // approve bridge to burn this contract's funds (required when exiting on L1, as it burns funds on L2):
         let nonce_for_burn_approval = storage.nonce_for_burn_approval.read();
         let selector = compute_selector("burn_public((Field),Field,Field)");
-        let message_hash = compute_message_hash([token_bridge.address, token, selector, context.this_address(), amount, nonce_for_burn_approval]);
+        let message_hash = compute_message_hash([token_bridge.address, token.address, selector, context.this_address(), amount, nonce_for_burn_approval]);
         storage.approved_action.at(message_hash).write(true);
 
         // increment nonce_for_burn_approval so it won't be used again
@@ -155,9 +152,4 @@ contract Uniswap {
             nonce_for_burn_approval,
         );
     }
-
-    #[aztec(public)]
-    internal fn _assert_token_is_same(token: Field, token_bridge: Field) {
-        assert(token == (TokenBridge::at(token_bridge).token(context)), "input_asset address is not the same as seen in the bridge contract");
-    }
 }

yarn-project/noir-contracts/src/contracts/uniswap_contract/src/util.nr

@@ -7,17 +7,15 @@ fn compute_message_hash<N>(args: [Field; N]) -> Field {
   pedersen_with_separator(args, GENERATOR_INDEX__SIGNATURE_PAYLOAD)[0]
 }
 
-// This method computes the L2 to L1 message content hash for both private and public flow. 
+// This method computes the L2 to L1 message content hash for the private
 // refer `l1-contracts/test/portals/UniswapPortal.sol` on how L2 to L1 message is expected
-// for private flow, instead of specifying who is recipient on L2 will be, we specify the secret hash for redeeming minted notes
-fn compute_swap_content_hash(
-    is_private_flow: bool,
+fn compute_swap_private_content_hash(
     input_asset_bridge_portal_address: Field,
     input_amount: Field,
     uniswap_fee_tier: Field,
     output_asset_bridge_portal_address: Field,
     minimum_output_amount: Field,
-    recipient_or_secret_hash_for_redeeming_minted_notes: Field,
+    secret_hash_for_redeeming_minted_notes: Field,
     secret_hash_for_L1_to_l2_message: Field,
     deadline_for_L1_to_l2_message: Field,
     canceller_for_L1_to_L2_message: Field,
@@ -30,33 +28,25 @@ fn compute_swap_content_hash(
     let uniswap_fee_tier_bytes = uniswap_fee_tier.to_be_bytes(32);
     let output_token_portal_bytes = output_asset_bridge_portal_address.to_be_bytes(32);
     let amount_out_min_bytes = minimum_output_amount.to_be_bytes(32);
-    let aztec_recipient_or_secret_hash_for_redeeming_minted_notes_bytes = recipient_or_secret_hash_for_redeeming_minted_notes.to_be_bytes(32);
+    let secret_hash_for_redeeming_minted_notes_bytes = secret_hash_for_redeeming_minted_notes.to_be_bytes(32);
     let secret_hash_for_L1_to_l2_message_bytes = secret_hash_for_L1_to_l2_message.to_be_bytes(32);
     let deadline_for_L1_to_l2_message_bytes = deadline_for_L1_to_l2_message.to_be_bytes(32);
     let canceller_bytes = canceller_for_L1_to_L2_message.to_be_bytes(32);
     let caller_on_L1_bytes = caller_on_L1.to_be_bytes(32);
 
-    if (is_private_flow) {
-        // function selector: 0xbd87d14b keccak256("swap_private(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)")
-        hash_bytes[0] = 0xbd;
-        hash_bytes[1] = 0x87;
-        hash_bytes[2] = 0xd1;
-        hash_bytes[3] = 0x4b;
-    } else {
-        // function selector: 0xf3068cac keccak256("swap_public(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)")
-        hash_bytes[0] = 0xf3;
-        hash_bytes[1] = 0x06;
-        hash_bytes[2] = 0x8c;
-        hash_bytes[3] = 0xac;
-    }
+    // function selector: 0xbd87d14b keccak256("swap_private(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)")
+    hash_bytes[0] = 0xbd;
+    hash_bytes[1] = 0x87;
+    hash_bytes[2] = 0xd1;
+    hash_bytes[3] = 0x4b;
 
     for i in 0..32 {
         hash_bytes[i + 4] = input_token_portal_bytes[i];
         hash_bytes[i + 36] = in_amount_bytes[i];
         hash_bytes[i + 68] = uniswap_fee_tier_bytes[i];
         hash_bytes[i + 100] = output_token_portal_bytes[i];
         hash_bytes[i + 132] = amount_out_min_bytes[i];
-        hash_bytes[i + 164] = aztec_recipient_or_secret_hash_for_redeeming_minted_notes_bytes[i];
+        hash_bytes[i + 164] = secret_hash_for_redeeming_minted_notes_bytes[i];
         hash_bytes[i + 196] = secret_hash_for_L1_to_l2_message_bytes[i];
         hash_bytes[i + 228] = deadline_for_L1_to_l2_message_bytes[i];
         hash_bytes[i + 260] = canceller_bytes[i];
@@ -65,7 +55,7 @@ fn compute_swap_content_hash(
 
     let content_sha256 = sha256(hash_bytes);
 
-    // // Convert the content_sha256 to a field element
+    // Convert the content_sha256 to a field element
     let mut v = 1;
     let mut high = 0 as Field;
     let mut low = 0 as Field;