Skip to content

Commit

Permalink
PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
rahul-kothari committed Sep 28, 2023
1 parent 94dcc2a commit a90126c
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 60 deletions.
17 changes: 6 additions & 11 deletions yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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');
Expand All @@ -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);
Expand Down Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/canary/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/end-to-end/src/fixtures/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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];
Expand All @@ -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;
Expand Down

0 comments on commit a90126c

Please sign in to comment.