From 7a58fe928b658f92afc6914672d64f8742db35bc Mon Sep 17 00:00:00 2001 From: Rahul Kothari Date: Fri, 6 Oct 2023 12:45:16 +0100 Subject: [PATCH] chore(uniswap_tests): test edge cases around uniswap flow (#2620) Fix #2493. Part of #2167 * Adds test cases for both public, private flow * Found an edge case that wasn't tested in l1<>l2 tests so added those too * Also rename `swap` to `swap_private` Runtime currently is ~5 mins --- .../src/uniswap_trade_on_l1_from_l2.test.ts | 4 +- .../src/e2e_cross_chain_messaging.test.ts | 33 ++ .../e2e_public_cross_chain_messaging.test.ts | 22 ++ .../src/fixtures/cross_chain_test_harness.ts | 7 + .../src/uniswap_trade_on_l1_from_l2.test.ts | 365 +++++++++++++++++- .../uniswap_contract/src/interfaces.nr | 4 +- .../contracts/uniswap_contract/src/main.nr | 18 +- 7 files changed, 426 insertions(+), 27 deletions(-) diff --git a/yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts b/yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts index 2224f446025..1b6c4fcb6a1 100644 --- a/yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts +++ b/yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts @@ -350,7 +350,7 @@ describe('uniswap_trade_on_l1_from_l2', () => { const [secretForRedeemingDai, secretHashForRedeemingDai] = await generateClaimSecret(); const withdrawReceipt = await uniswapL2Contract.methods - .swap( + .swap_private( wethL2Contract.address, wethL2Bridge.address, wethAmountToBridge, @@ -372,7 +372,7 @@ describe('uniswap_trade_on_l1_from_l2', () => { // ensure that uniswap contract didn't eat the funds. await expectPublicBalanceOnL2(uniswapL2Contract.address, 0n, wethL2Contract); - // 6. Consume L2 to L1 message by calling uniswapPortal.swap() + // 6. Consume L2 to L1 message by calling uniswapPortal.swap_private() logger('Execute withdraw and swap on the uniswapPortal!'); const daiL1BalanceOfPortalBeforeSwap = await daiContract.read.balanceOf([daiTokenPortalAddress.toString()]); const swapArgs = [ 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 6a3bbacb7df..1f2b8209d66 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 @@ -209,4 +209,37 @@ describe('e2e_cross_chain_messaging', () => { .simulate(), ).rejects.toThrowError(`Unknown auth witness for message hash 0x${expectedBurnMessageHash.toString('hex')}`); }); + + it("Can't claim funds publicly if they were deposited privately", async () => { + // 1. Mint tokens on L1 + const bridgeAmount = 100n; + await crossChainTestHarness.mintTokensOnL1(bridgeAmount); + + // 2. Deposit tokens to the TokenPortal privately + const [secretForL2MessageConsumption, secretHashForL2MessageConsumption] = + await crossChainTestHarness.generateClaimSecret(); + + const messageKey = await crossChainTestHarness.sendTokensToPortalPrivate( + bridgeAmount, + secretHashForL2MessageConsumption, + Fr.random(), + ); + expect(await crossChainTestHarness.getL1BalanceOf(ethAccount)).toBe(0n); + + // Wait for the archiver to process the message + await delay(5000); /// waiting 5 seconds. + + // Perform an unrelated transaction on L2 to progress the rollup. Here we mint public tokens. + await crossChainTestHarness.performL2Transfer(0n); + + // 3. Consume L1-> L2 message and try to mint publicly on L2 - should fail + await expect( + l2Bridge + .withWallet(user2Wallet) + .methods.claim_public(ownerAddress, bridgeAmount, ethAccount, messageKey, secretForL2MessageConsumption) + .simulate(), + ).rejects.toThrowError( + "Failed to solve brillig function, reason: explicit trap hit in brillig 'l1_to_l2_message_data.message.content == content'", + ); + }); }); 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 0111381d8d5..6d77f9826e7 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 @@ -171,4 +171,26 @@ describe('e2e_public_cross_chain_messaging', () => { .simulate(), ).rejects.toThrowError('Assertion failed: Message not authorized by account'); }); + + it("can't claim funds privately which were intended for public deposit from the token portal", async () => { + const bridgeAmount = 100n; + const [secret, secretHash] = await crossChainTestHarness.generateClaimSecret(); + + await crossChainTestHarness.mintTokensOnL1(bridgeAmount); + const messageKey = await crossChainTestHarness.sendTokensToPortalPublic(bridgeAmount, secretHash); + expect(await crossChainTestHarness.getL1BalanceOf(ownerEthAddress)).toBe(0n); + + // Wait for the archiver to process the message + await delay(5000); /// waiting 5 seconds. + + // Perform an unrelated transaction on L2 to progress the rollup. Here we mint public tokens. + await crossChainTestHarness.performL2Transfer(0n); + + await expect( + l2Bridge + .withWallet(user2Wallet) + .methods.claim_private(bridgeAmount, secretHash, ownerEthAddress, messageKey, secret) + .simulate(), + ).rejects.toThrowError("Cannot satisfy constraint 'l1_to_l2_message_data.message.content == content"); + }); }); 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 08972e4d551..6e9c6077b41 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 @@ -201,6 +201,13 @@ export class CrossChainTestHarness { expect(receipt.status).toBe(TxStatus.MINED); } + async mintTokensPrivateOnL2(amount: bigint, secretHash: Fr) { + const tx = this.l2Token.methods.mint_private(amount, secretHash).send(); + const receipt = await tx.wait(); + expect(receipt.status).toBe(TxStatus.MINED); + await this.addPendingShieldNoteToPXE(amount, secretHash, receipt.txHash); + } + async performL2Transfer(transferAmount: bigint) { // send a transfer tx to force through rollup with the message included const transferTx = this.l2Token.methods.transfer_public(this.ownerAddress, this.receiver, transferAmount, 0).send(); diff --git a/yarn-project/end-to-end/src/uniswap_trade_on_l1_from_l2.test.ts b/yarn-project/end-to-end/src/uniswap_trade_on_l1_from_l2.test.ts index f903468f16a..b5e8481d265 100644 --- a/yarn-project/end-to-end/src/uniswap_trade_on_l1_from_l2.test.ts +++ b/yarn-project/end-to-end/src/uniswap_trade_on_l1_from_l2.test.ts @@ -7,6 +7,7 @@ import { UniswapPortalAbi, UniswapPortalBytecode } from '@aztec/l1-artifacts'; import { UniswapContract } from '@aztec/noir-contracts/types'; import { AztecNode, PXE, TxStatus } from '@aztec/types'; +import { jest } from '@jest/globals'; import { getContract, parseEther } from 'viem'; import { CrossChainTestHarness } from './fixtures/cross_chain_test_harness.js'; @@ -24,9 +25,12 @@ const dumpedState = 'src/fixtures/dumps/uniswap_state'; const EXPECTED_FORKED_BLOCK = 0; //17514288; // We tell the archiver to only sync from this block. process.env.SEARCH_START_BLOCK = EXPECTED_FORKED_BLOCK.toString(); +const TIMEOUT = 90_000; // Should mint WETH on L2, swap to DAI using L1 Uniswap and mint this DAI back on L2 describe('uniswap_trade_on_l1_from_l2', () => { + jest.setTimeout(TIMEOUT); + const WETH9_ADDRESS: EthAddress = EthAddress.fromString('0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2'); const DAI_ADDRESS: EthAddress = EthAddress.fromString('0x6B175474E89094C44Da98b954EedeAC495271d0F'); @@ -35,6 +39,8 @@ describe('uniswap_trade_on_l1_from_l2', () => { let logger: DebugLogger; let teardown: () => Promise; + let walletClient: any; + let ownerWallet: AccountWallet; let ownerAddress: AztecAddress; let ownerEthAddress: EthAddress; @@ -52,8 +58,9 @@ describe('uniswap_trade_on_l1_from_l2', () => { const wethAmountToBridge = parseEther('1'); const uniswapFeeTier = 3000n; const minimumOutputAmount = 0n; + const deadlineForDepositingSwappedDai = BigInt(2 ** 32 - 1); // max uint32 - 1 - beforeEach(async () => { + beforeAll(async () => { const { teardown: teardown_, aztecNode: aztecNode_, @@ -64,7 +71,7 @@ describe('uniswap_trade_on_l1_from_l2', () => { logger: logger_, cheatCodes, } = await setup(2, { stateLoad: dumpedState }); - const walletClient = deployL1ContractsValues.walletClient; + walletClient = deployL1ContractsValues.walletClient; const publicClient = deployL1ContractsValues.publicClient; if (Number(await publicClient.getBlockNumber()) < EXPECTED_FORKED_BLOCK) { @@ -123,13 +130,15 @@ describe('uniswap_trade_on_l1_from_l2', () => { [deployL1ContractsValues!.l1ContractAddresses.registryAddress!.toString(), uniswapL2Contract.address.toString()], {} as any, ); + }); + beforeEach(async () => { // Give me some WETH so I can deposit to L2 and do the swap... logger('Getting some weth'); await walletClient.sendTransaction({ to: WETH9_ADDRESS.toString(), value: parseEther('1') }); - }, 100_000); + }); - afterEach(async () => { + afterAll(async () => { await teardown(); await wethCrossChainHarness.stop(); await daiCrossChainHarness.stop(); @@ -157,9 +166,7 @@ describe('uniswap_trade_on_l1_from_l2', () => { await delay(5000); // Perform an unrelated transaction on L2 to progress the rollup. Here we mint public tokens. - const unrelatedMintAmount = 1n; - await wethCrossChainHarness.mintTokensPublicOnL2(unrelatedMintAmount); - await wethCrossChainHarness.expectPublicBalanceOnL2(ownerAddress, unrelatedMintAmount); + await wethCrossChainHarness.mintTokensPublicOnL2(0n); // 2. Claim WETH on L2 logger('Minting weth on L2'); @@ -176,9 +183,13 @@ describe('uniswap_trade_on_l1_from_l2', () => { const wethL2BalanceBeforeSwap = await wethCrossChainHarness.getL2PrivateBalanceOf(ownerAddress); const daiL2BalanceBeforeSwap = await daiCrossChainHarness.getL2PrivateBalanceOf(ownerAddress); + // before swap - check nonce_for_burn_approval stored on uniswap + // (which is used by uniswap to approve the bridge to burn funds on its behalf to exit to L1) + const nonceForBurnApprovalBeforeSwap = await uniswapL2Contract.methods.nonce_for_burn_approval().view(); + // 3. Owner gives uniswap approval to unshield funds to self on its behalf logger('Approving uniswap to unshield funds to self on my behalf'); - const nonceForWETHUnshieldApproval = new Fr(2n); + const nonceForWETHUnshieldApproval = new Fr(1n); const unshieldToUniswapMessageHash = await computeAuthWitMessageHash( uniswapL2Contract.address, wethCrossChainHarness.l2Token.methods @@ -189,13 +200,12 @@ describe('uniswap_trade_on_l1_from_l2', () => { // 4. Swap on L1 - sends L2 to L1 message to withdraw WETH to L1 and another message to swap assets. logger('Withdrawing weth to L1 and sending message to swap to dai'); - const deadlineForDepositingSwappedDai = BigInt(2 ** 32 - 1); // max uint32 - 1 const [secretForDepositingSwappedDai, secretHashForDepositingSwappedDai] = await daiCrossChainHarness.generateClaimSecret(); const [secretForRedeemingDai, secretHashForRedeemingDai] = await daiCrossChainHarness.generateClaimSecret(); const withdrawReceipt = await uniswapL2Contract.methods - .swap( + .swap_private( wethCrossChainHarness.l2Token.address, wethCrossChainHarness.l2Bridge.address, wethAmountToBridge, @@ -216,8 +226,11 @@ describe('uniswap_trade_on_l1_from_l2', () => { await wethCrossChainHarness.expectPrivateBalanceOnL2(ownerAddress, wethL2BalanceBeforeSwap - wethAmountToBridge); // ensure that uniswap contract didn't eat the funds. await wethCrossChainHarness.expectPublicBalanceOnL2(uniswapL2Contract.address, 0n); + // check burn approval nonce incremented: + const nonceForBurnApprovalAfterSwap = await uniswapL2Contract.methods.nonce_for_burn_approval().view(); + expect(nonceForBurnApprovalAfterSwap).toBe(nonceForBurnApprovalBeforeSwap + 1n); - // 5. Consume L2 to L1 message by calling uniswapPortal.swap() + // 5. Consume L2 to L1 message by calling uniswapPortal.swap_private() logger('Execute withdraw and swap on the uniswapPortal!'); const daiL1BalanceOfPortalBeforeSwap = await daiCrossChainHarness.getL1BalanceOf( daiCrossChainHarness.tokenPortalAddress, @@ -271,9 +284,9 @@ describe('uniswap_trade_on_l1_from_l2', () => { logger('WETH balance before swap: ' + wethL2BalanceBeforeSwap.toString()); logger('DAI balance before swap : ' + daiL2BalanceBeforeSwap.toString()); logger('***** 🧚‍♀️ SWAP L2 assets on L1 Uniswap 🧚‍♀️ *****'); - logger('WETH balance after swap : ' + wethL2BalanceAfterSwap.toString()); - logger('DAI balance after swap : ' + daiL2BalanceAfterSwap.toString()); - }, 140_000); + logger('WETH balance after swap : ', wethL2BalanceAfterSwap.toString()); + logger('DAI balance after swap : ', daiL2BalanceAfterSwap.toString()); + }); it('should uniswap trade on L1 from L2 funds publicly (swaps WETH -> DAI)', async () => { const wethL1BeforeBalance = await wethCrossChainHarness.getL1BalanceOf(ownerEthAddress); @@ -311,7 +324,7 @@ describe('uniswap_trade_on_l1_from_l2', () => { const daiL2BalanceBeforeSwap = await daiCrossChainHarness.getL2PublicBalanceOf(ownerAddress); // 3. Owner gives uniswap approval to transfer funds on its behalf - const nonceForWETHTransferApproval = new Fr(2n); + const nonceForWETHTransferApproval = new Fr(1n); const transferMessageHash = await computeAuthWitMessageHash( uniswapL2Contract.address, wethCrossChainHarness.l2Token.methods @@ -320,8 +333,11 @@ describe('uniswap_trade_on_l1_from_l2', () => { ); await ownerWallet.setPublicAuth(transferMessageHash, true).send().wait(); + // before swap - check nonce_for_burn_approval stored on uniswap + // (which is used by uniswap to approve the bridge to burn funds on its behalf to exit to L1) + const nonceForBurnApprovalBeforeSwap = await uniswapL2Contract.methods.nonce_for_burn_approval().view(); + // 4. Swap on L1 - sends L2 to L1 message to withdraw WETH to L1 and another message to swap assets. - const deadlineForDepositingSwappedDai = BigInt(2 ** 32 - 1); // max uint32 - 1 const [secretForDepositingSwappedDai, secretHashForDepositingSwappedDai] = await daiCrossChainHarness.generateClaimSecret(); @@ -354,7 +370,11 @@ describe('uniswap_trade_on_l1_from_l2', () => { // check weth balance of owner on L2 (we first bridged `wethAmountToBridge` into L2 and now withdrew it!) await wethCrossChainHarness.expectPublicBalanceOnL2(ownerAddress, wethL2BalanceBeforeSwap - wethAmountToBridge); - // 5. Perform the swap on L1 with the `uniswapPortal.swap()` (consuming L2 to L1 messages) + // check burn approval nonce incremented: + const nonceForBurnApprovalAfterSwap = await uniswapL2Contract.methods.nonce_for_burn_approval().view(); + expect(nonceForBurnApprovalAfterSwap).toBe(nonceForBurnApprovalBeforeSwap + 1n); + + // 5. Perform the swap on L1 with the `uniswapPortal.swap_private()` (consuming L2 to L1 messages) logger('Execute withdraw and swap on the uniswapPortal!'); const daiL1BalanceOfPortalBeforeSwap = await daiCrossChainHarness.getL1BalanceOf( daiCrossChainHarness.tokenPortalAddress, @@ -407,5 +427,314 @@ describe('uniswap_trade_on_l1_from_l2', () => { logger('***** 🧚‍♀️ SWAP L2 assets on L1 Uniswap 🧚‍♀️ *****'); logger('WETH balance after swap : ', wethL2BalanceAfterSwap.toString()); logger('DAI balance after swap : ', daiL2BalanceAfterSwap.toString()); - }, 140_000); + }); + + // Edge cases for the private flow: + // note - tests for uniswapPortal.sol and minting asset on L2 are covered in other tests. + + it('swap_private reverts without unshield approval', async () => { + // swap should fail since no withdraw approval to uniswap: + const nonceForWETHUnshieldApproval = new Fr(2n); + + const expectedMessageHash = await computeAuthWitMessageHash( + uniswapL2Contract.address, + wethCrossChainHarness.l2Token.methods + .unshield(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHUnshieldApproval) + .request(), + ); + + await expect( + uniswapL2Contract.methods + .swap_private( + wethCrossChainHarness.l2Token.address, + wethCrossChainHarness.l2Bridge.address, + wethAmountToBridge, + daiCrossChainHarness.l2Bridge.address, + nonceForWETHUnshieldApproval, + uniswapFeeTier, + minimumOutputAmount, + Fr.random(), + Fr.random(), + deadlineForDepositingSwappedDai, + ownerEthAddress, + ownerEthAddress, + ) + .simulate(), + ).rejects.toThrowError(`Unknown auth witness for message hash 0x${expectedMessageHash.toString('hex')}`); + }); + + it("can't swap if user passes a token different to what the bridge tracks", async () => { + // 1. give user private funds on L2: + const [secretForRedeemingWeth, secretHashForRedeemingWeth] = await wethCrossChainHarness.generateClaimSecret(); + await wethCrossChainHarness.mintTokensPrivateOnL2(wethAmountToBridge, secretHashForRedeemingWeth); + await wethCrossChainHarness.redeemShieldPrivatelyOnL2(wethAmountToBridge, secretForRedeemingWeth); + await wethCrossChainHarness.expectPrivateBalanceOnL2(ownerAddress, wethAmountToBridge); + + // 2. owner gives uniswap approval to unshield funds: + logger('Approving uniswap to unshield funds to self on my behalf'); + const nonceForWETHUnshieldApproval = new Fr(3n); + const unshieldToUniswapMessageHash = await computeAuthWitMessageHash( + uniswapL2Contract.address, + wethCrossChainHarness.l2Token.methods + .unshield(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHUnshieldApproval) + .request(), + ); + await ownerWallet.createAuthWitness(Fr.fromBuffer(unshieldToUniswapMessageHash)); + + // 3. Swap but send the wrong token address + logger('Swap but send the wrong token address'); + await expect( + uniswapL2Contract.methods + .swap_private( + wethCrossChainHarness.l2Token.address, // send weth token + daiCrossChainHarness.l2Bridge.address, // but dai bridge! + wethAmountToBridge, + daiCrossChainHarness.l2Bridge.address, + nonceForWETHUnshieldApproval, + uniswapFeeTier, + minimumOutputAmount, + Fr.random(), + Fr.random(), + deadlineForDepositingSwappedDai, + ownerEthAddress, + ownerEthAddress, + ) + .simulate(), + ).rejects.toThrowError('Assertion failed: input_asset address is not the same as seen in the bridge contract'); + }); + + // edge cases for public flow: + + it("I don't need approval to call swap_public if I'm swapping on my own behalf", async () => { + // 1. get tokens on l2 + await wethCrossChainHarness.mintTokensPublicOnL2(wethAmountToBridge); + + // 2. Give approval to uniswap to transfer funds to itself + const nonceForWETHTransferApproval = new Fr(2n); + const transferMessageHash = await computeAuthWitMessageHash( + uniswapL2Contract.address, + wethCrossChainHarness.l2Token.methods + .transfer_public(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHTransferApproval) + .request(), + ); + await ownerWallet.setPublicAuth(transferMessageHash, true).send().wait(); + + // No approval to call `swap` but should work even without it: + const [_, secretHashForDepositingSwappedDai] = await daiCrossChainHarness.generateClaimSecret(); + + const withdrawReceipt = await uniswapL2Contract.methods + .swap_public( + ownerAddress, + wethCrossChainHarness.l2Bridge.address, + wethAmountToBridge, + daiCrossChainHarness.l2Bridge.address, + nonceForWETHTransferApproval, + uniswapFeeTier, + minimumOutputAmount, + ownerAddress, + secretHashForDepositingSwappedDai, + deadlineForDepositingSwappedDai, + ownerEthAddress, + ownerEthAddress, + Fr.ZERO, // nonce for swap -> doesn't matter + ) + .send() + .wait(); + expect(withdrawReceipt.status).toBe(TxStatus.MINED); + // check weth balance of owner on L2 (we first bridged `wethAmountToBridge` into L2 and now withdrew it!) + await wethCrossChainHarness.expectPublicBalanceOnL2(ownerAddress, 0n); + }); + + it("someone can't call swap_public on my behalf without approval", async () => { + // Owner approves a a user to swap_public: + const approvedUser = AztecAddress.random(); + + const nonceForWETHTransferApproval = new Fr(3n); + const nonceForSwap = new Fr(3n); + const secretHashForDepositingSwappedDai = new Fr(4n); + const action = uniswapL2Contract + .withWallet(sponsorWallet) + .methods.swap_public( + ownerAddress, + wethCrossChainHarness.l2Bridge.address, + wethAmountToBridge, + daiCrossChainHarness.l2Bridge.address, + nonceForWETHTransferApproval, + uniswapFeeTier, + minimumOutputAmount, + ownerAddress, + secretHashForDepositingSwappedDai, + deadlineForDepositingSwappedDai, + ownerEthAddress, + ownerEthAddress, + nonceForSwap, + ); + const swapMessageHash = await computeAuthWitMessageHash(approvedUser, action.request()); + await ownerWallet.setPublicAuth(swapMessageHash, true).send().wait(); + + // Swap! + await expect(action.simulate()).rejects.toThrowError( + "Assertion failed: Message not authorized by account 'result == IS_VALID_SELECTOR'", + ); + }); + + it("uniswap can't pull funds without transfer approval", async () => { + // swap should fail since no transfer approval to uniswap: + const nonceForWETHTransferApproval = new Fr(4n); + + const transferMessageHash = await computeAuthWitMessageHash( + uniswapL2Contract.address, + wethCrossChainHarness.l2Token.methods + .transfer_public(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHTransferApproval) + .request(), + ); + await ownerWallet.setPublicAuth(transferMessageHash, true).send().wait(); + + await expect( + uniswapL2Contract.methods + .swap_public( + ownerAddress, + wethCrossChainHarness.l2Bridge.address, + wethAmountToBridge, + daiCrossChainHarness.l2Bridge.address, + new Fr(420), // using a different nonce + uniswapFeeTier, + minimumOutputAmount, + ownerAddress, + Fr.random(), + deadlineForDepositingSwappedDai, + ownerEthAddress, + ownerEthAddress, + Fr.ZERO, + ) + .simulate(), + ).rejects.toThrowError(`Assertion failed: Message not authorized by account 'result == IS_VALID_SELECTOR'`); + }); + + // tests when trying to mix private and public flows: + it("can't call swap_public on L1 if called swap_private on L2", async () => { + // get tokens on L2: + const [secretForRedeemingWeth, secretHashForRedeemingWeth] = await wethCrossChainHarness.generateClaimSecret(); + logger('minting weth on L2'); + await wethCrossChainHarness.mintTokensPrivateOnL2(wethAmountToBridge, secretHashForRedeemingWeth); + await wethCrossChainHarness.redeemShieldPrivatelyOnL2(wethAmountToBridge, secretForRedeemingWeth); + + // Owner gives uniswap approval to unshield funds to self on its behalf + logger('Approving uniswap to unshield funds to self on my behalf'); + const nonceForWETHUnshieldApproval = new Fr(4n); + + const unshieldToUniswapMessageHash = await computeAuthWitMessageHash( + uniswapL2Contract.address, + wethCrossChainHarness.l2Token.methods + .unshield(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHUnshieldApproval) + .request(), + ); + await ownerWallet.createAuthWitness(Fr.fromBuffer(unshieldToUniswapMessageHash)); + const wethL2BalanceBeforeSwap = await wethCrossChainHarness.getL2PrivateBalanceOf(ownerAddress); + + // Swap + logger('Withdrawing weth to L1 and sending message to swap to dai'); + const secretHashForDepositingSwappedDai = Fr.random(); + + const withdrawReceipt = await uniswapL2Contract.methods + .swap_private( + wethCrossChainHarness.l2Token.address, + wethCrossChainHarness.l2Bridge.address, + wethAmountToBridge, + daiCrossChainHarness.l2Bridge.address, + nonceForWETHUnshieldApproval, + uniswapFeeTier, + minimumOutputAmount, + Fr.random(), + secretHashForDepositingSwappedDai, + deadlineForDepositingSwappedDai, + ownerEthAddress, + ownerEthAddress, + ) + .send() + .wait(); + expect(withdrawReceipt.status).toBe(TxStatus.MINED); + // ensure that user's funds were burnt + await wethCrossChainHarness.expectPrivateBalanceOnL2(ownerAddress, wethL2BalanceBeforeSwap - wethAmountToBridge); + + // On L1 call swap_public! + logger('call swap_public on L1'); + const swapArgs = [ + wethCrossChainHarness.tokenPortalAddress.toString(), + wethAmountToBridge, + uniswapFeeTier, + daiCrossChainHarness.tokenPortalAddress.toString(), + minimumOutputAmount, + ownerAddress.toString(), + secretHashForDepositingSwappedDai.toString(true), + deadlineForDepositingSwappedDai, + ownerEthAddress.toString(), + true, + ] as const; + await expect( + uniswapPortal.simulate.swapPublic(swapArgs, { + account: ownerEthAddress.toString(), + } as any), + ).rejects.toThrowError('The contract function "swapPublic" reverted.'); + }); + + it("can't call swap_private on L1 if called swap_public on L2", async () => { + // get tokens on L2: + await wethCrossChainHarness.mintTokensPublicOnL2(wethAmountToBridge); + + // Owner gives uniswap approval to transfer funds on its behalf + const nonceForWETHTransferApproval = new Fr(5n); + const transferMessageHash = await computeAuthWitMessageHash( + uniswapL2Contract.address, + wethCrossChainHarness.l2Token.methods + .transfer_public(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHTransferApproval) + .request(), + ); + await ownerWallet.setPublicAuth(transferMessageHash, true).send().wait(); + + // Call swap_public on L2 + const secretHashForDepositingSwappedDai = Fr.random(); + const withdrawReceipt = await uniswapL2Contract.methods + .swap_public( + ownerAddress, + wethCrossChainHarness.l2Bridge.address, + wethAmountToBridge, + daiCrossChainHarness.l2Bridge.address, + nonceForWETHTransferApproval, + uniswapFeeTier, + minimumOutputAmount, + ownerAddress, + secretHashForDepositingSwappedDai, + deadlineForDepositingSwappedDai, + ownerEthAddress, + ownerEthAddress, + Fr.ZERO, + ) + .send() + .wait(); + expect(withdrawReceipt.status).toBe(TxStatus.MINED); + // check weth balance of owner on L2 (we first bridged `wethAmountToBridge` into L2 and now withdrew it!) + await wethCrossChainHarness.expectPublicBalanceOnL2(ownerAddress, 0n); + + // Call swap_private on L1 + const secretHashForRedeemingDai = Fr.random(); // creating my own secret hash + logger('Execute withdraw and swap on the uniswapPortal!'); + const swapArgs = [ + wethCrossChainHarness.tokenPortalAddress.toString(), + wethAmountToBridge, + uniswapFeeTier, + daiCrossChainHarness.tokenPortalAddress.toString(), + minimumOutputAmount, + secretHashForRedeemingDai.toString(true), + secretHashForDepositingSwappedDai.toString(true), + deadlineForDepositingSwappedDai, + ownerEthAddress.toString(), + true, + ] as const; + await expect( + uniswapPortal.simulate.swapPrivate(swapArgs, { + account: ownerEthAddress.toString(), + } as any), + ).rejects.toThrowError('The contract function "swapPrivate" reverted.'); + }); }); diff --git a/yarn-project/noir-contracts/src/contracts/uniswap_contract/src/interfaces.nr b/yarn-project/noir-contracts/src/contracts/uniswap_contract/src/interfaces.nr index 3c86eed3bbf..24feb699e48 100644 --- a/yarn-project/noir-contracts/src/contracts/uniswap_contract/src/interfaces.nr +++ b/yarn-project/noir-contracts/src/contracts/uniswap_contract/src/interfaces.nr @@ -39,9 +39,9 @@ impl TokenBridge { Self { address } } - fn token(self: Self, context: PublicContext) -> Field { + fn token(self: Self, context: PublicContext) -> AztecAddress { let return_values = context.call_public_function(self.address, compute_selector("get_token()"), []); - return_values[0] + AztecAddress::new(return_values[0]) } fn claim_public(self: Self, context: PublicContext, to: Field, amount: Field, canceller: Field, msg_key: Field, secret: Field) { diff --git a/yarn-project/noir-contracts/src/contracts/uniswap_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/uniswap_contract/src/main.nr index 1a24e357d32..754ba49df06 100644 --- a/yarn-project/noir-contracts/src/contracts/uniswap_contract/src/main.nr +++ b/yarn-project/noir-contracts/src/contracts/uniswap_contract/src/main.nr @@ -75,7 +75,7 @@ contract Uniswap { assert_current_call_valid_authwit_public(&mut context, sender); } - let input_asset = AztecAddress::new(TokenBridge::at(input_asset_bridge.address).token(context)); + let input_asset = TokenBridge::at(input_asset_bridge.address).token(context); // Transfer funds to this contract Token::at(input_asset.address).transfer_public( @@ -118,7 +118,7 @@ contract Uniswap { } #[aztec(private)] - fn swap( + fn swap_private( input_asset: AztecAddress, // since private, we pass here and later assert that this is as expected by input_bridge input_asset_bridge: AztecAddress, input_amount: Field, @@ -138,7 +138,7 @@ contract Uniswap { // 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]); + 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( @@ -224,7 +224,15 @@ contract Uniswap { } #[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"); + internal fn _assert_token_is_same(token: AztecAddress, token_bridge: AztecAddress) { + assert(token.eq(TokenBridge::at(token_bridge.address).token(context)), "input_asset address is not the same as seen in the bridge contract"); } + + // /// Unconstrained /// + + // this method exists solely for e2e tests to test that nonce gets incremented each time. + unconstrained fn nonce_for_burn_approval() -> Field { + storage.nonce_for_burn_approval.read() + } + } \ No newline at end of file