-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(e2e): public flow for uniswap #2596
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ mod util; | |
// Uses the token bridge contract, which tells which input token we need to talk to and handles the exit funds to L1 | ||
contract Uniswap { | ||
use dep::aztec::{ | ||
auth::IS_VALID_SELECTOR, | ||
auth::{IS_VALID_SELECTOR, assert_valid_public_message_for}, | ||
context::{PrivateContext, PublicContext, Context}, | ||
oracle::compute_selector::compute_selector, | ||
oracle::context::get_portal_address, | ||
|
@@ -21,7 +21,7 @@ contract Uniswap { | |
}; | ||
|
||
use crate::interfaces::{Token, TokenBridge}; | ||
use crate::util::{compute_message_hash, compute_swap_private_content_hash}; | ||
use crate::util::{compute_message_hash, compute_swap_private_content_hash, compute_swap_public_content_hash}; | ||
|
||
struct Storage { | ||
// like with account contracts, stores the approval message on a slot and tracks if they are active | ||
|
@@ -49,6 +49,96 @@ contract Uniswap { | |
#[aztec(private)] | ||
fn constructor() {} | ||
|
||
#[aztec(public)] | ||
fn swap_public( | ||
sender: AztecAddress, | ||
input_asset_bridge: AztecAddress, | ||
input_amount: Field, | ||
output_asset_bridge: AztecAddress, | ||
// params for using the transfer approval | ||
nonce_for_transfer_approval: Field, | ||
// params for the swap | ||
uniswap_fee_tier: Field, | ||
minimum_output_amount: Field, | ||
// params for the depositing output_asset back to Aztec | ||
recipient: AztecAddress, | ||
secret_hash_for_L1_to_l2_message: Field, | ||
deadline_for_L1_to_l2_message: Field, | ||
canceller_for_L1_to_L2_message: EthereumAddress, | ||
caller_on_L1: EthereumAddress, | ||
// nonce for someone to call swap on sender's behalf | ||
nonce_for_swap_approval: Field, | ||
) -> Field { | ||
|
||
if (sender.address != context.msg_sender()) { | ||
// if someone else is calling on swap on sender's behalf, they need to have authorisation to do so: | ||
let selector = compute_selector( | ||
"swap_public((Field),(Field),Field,(Field),Field,Field,Field,(Field),Field,Field,(Field),(Field),Field)" | ||
); | ||
let message_field = compute_message_hash([ | ||
context.msg_sender(), | ||
context.this_address(), | ||
selector, | ||
sender.address, | ||
input_asset_bridge.address, | ||
input_amount, | ||
output_asset_bridge.address, | ||
nonce_for_transfer_approval, | ||
uniswap_fee_tier, | ||
minimum_output_amount, | ||
recipient.address, | ||
secret_hash_for_L1_to_l2_message, | ||
deadline_for_L1_to_l2_message, | ||
canceller_for_L1_to_L2_message.address, | ||
caller_on_L1.address, | ||
nonce_for_swap_approval, | ||
]); | ||
// this also emits a nullifier for the message | ||
assert_valid_public_message_for(&mut context,sender.address,message_field); | ||
} | ||
|
||
let input_asset = AztecAddress::new(TokenBridge::at(input_asset_bridge.address).token(context)); | ||
|
||
// Transfer funds to this contract | ||
Token::at(input_asset.address).transfer_public( | ||
context, | ||
sender.address, | ||
context.this_address(), | ||
input_amount, | ||
nonce_for_transfer_approval, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just thinking, for public calls does nonce still need to be send around? Could the account contract itself maintain it in state? then it an be infered? Bare with me here as i might be horribly wrong There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Say you storage the nonce in the contract as a counter or whatever. What nonce would the uniswap contract read if you have done multiple approvals? If we expect only one approval to be active at a time you can do it quite easily, but in some cases, we might want multiple approvals within the same tx as well, then the ordering we approve them in suddenly becomes important as well and we also end up having to update more storage. It can work more easily when every token have their own nonces, but with all nonces handled by the account contract, you might need quite a bit of business logic to handle races etc. |
||
); | ||
|
||
// Approve bridge to burn this contract's funds and exit to L1 Uniswap Portal | ||
let _void = context.call_public_function( | ||
context.this_address(), | ||
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 | ||
// this ensures the integrity of what the user originally intends to do on L1. | ||
let input_asset_bridge_portal_address = get_portal_address(input_asset_bridge.address); | ||
let output_asset_bridge_portal_address = get_portal_address(output_asset_bridge.address); | ||
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_public_content_hash( | ||
input_asset_bridge_portal_address, | ||
input_amount, | ||
uniswap_fee_tier, | ||
output_asset_bridge_portal_address, | ||
minimum_output_amount, | ||
recipient.address, | ||
secret_hash_for_L1_to_l2_message, | ||
deadline_for_L1_to_l2_message, | ||
canceller_for_L1_to_L2_message.address, | ||
caller_on_L1.address, | ||
); | ||
context.message_portal(content_hash); | ||
|
||
1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May aswell just return a bool if its being used for a check? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably a leftover from doing similar to what I did in some of the other contracts when bools was not supported, don't think we changed it other places as well 🤔 Might be fine to go over all the contracts and alter those separately as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made #2615 |
||
} | ||
|
||
#[aztec(private)] | ||
fn swap( | ||
input_asset: AztecAddress, // since private, we pass here and later assert that this is as expected by input_bridge | ||
|
@@ -68,6 +158,10 @@ 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, | ||
|
@@ -122,7 +216,8 @@ contract Uniswap { | |
} | ||
|
||
// This helper method approves the bridge to burn this contract's funds and exits the input asset to L1 | ||
// Assumes contract already has funds | ||
// Assumes contract already has funds. | ||
// Assume `token` relates to `token_bridge` (ie token_bridge.token == token) | ||
// Note that private can't read public return values so created an internal public that handles everything | ||
// this method is used for both private and public swaps. | ||
#[aztec(public)] | ||
|
@@ -131,9 +226,6 @@ contract Uniswap { | |
token_bridge: AztecAddress, | ||
amount: Field, | ||
) { | ||
// 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)"); | ||
|
@@ -152,4 +244,9 @@ 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"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kinda nicely shows why #2199 #2448 is desired.