feat: add designated caller on token portal and update uniswap to use…

… it. (#896)

* add designated caller on tokenPortal.withdraw() and update accordingly

* address pr comments

* actually use _caller fml

l1-contracts/test/portals/TokenPortal.sol

@@ -98,14 +98,24 @@ contract TokenPortal {
    * @dev Second part of withdraw, must be initiated from L2 first as it will consume a message from outbox
    * @param _amount - The amount to withdraw
    * @param _recipient - The address to send the funds to
+   * @param _withCaller - Flag to use `msg.sender` as caller, otherwise address(0)
+   * Must match the caller of the message (specified from L2) to consume it.
    * @return The key of the entry in the Outbox
    */
-  function withdraw(uint256 _amount, address _recipient) external returns (bytes32) {
+  function withdraw(uint256 _amount, address _recipient, bool _withCaller)
+    external
+    returns (bytes32)
+  {
     DataStructures.L2ToL1Msg memory message = DataStructures.L2ToL1Msg({
       sender: DataStructures.L2Actor(l2TokenAddress, 1),
       recipient: DataStructures.L1Actor(address(this), block.chainid),
       content: Hash.sha256ToField(
-        abi.encodeWithSignature("withdraw(uint256,address)", _amount, _recipient)
+        abi.encodeWithSignature(
+          "withdraw(uint256,address,address)",
+          _amount,
+          _recipient,
+          _withCaller ? msg.sender : address(0)
+        )
         )
     });
 

l1-contracts/test/portals/TokenPortal.t.sol

@@ -40,16 +40,20 @@ contract TokenPortalTest is Test {
   Rollup internal rollup;
   bytes32 internal l2TokenAddress = bytes32(uint256(0x42));
 
-  TokenPortal tokenPortal;
-  PortalERC20 portalERC20;
+  TokenPortal internal tokenPortal;
+  PortalERC20 internal portalERC20;
 
   // input params
-  uint32 deadline = uint32(block.timestamp + 1 days);
-  bytes32 to = bytes32(0x2d749407d8c364537cdeb799c1574929cb22ff1ece2b96d2a1c6fa287a0e0171);
-  uint256 amount = 100;
-  uint256 mintAmount = 1 ether;
-  bytes32 secretHash = 0x147e4fec49805c924e28150fc4b36824679bc17ecb1d7d9f6a9effb7fde6b6a0;
-  uint64 bid = 1 ether;
+  uint32 internal deadline = uint32(block.timestamp + 1 days);
+  bytes32 internal to = bytes32(0x2d749407d8c364537cdeb799c1574929cb22ff1ece2b96d2a1c6fa287a0e0171);
+  uint256 internal amount = 100;
+  uint256 internal mintAmount = 1 ether;
+  bytes32 internal secretHash = 0x147e4fec49805c924e28150fc4b36824679bc17ecb1d7d9f6a9effb7fde6b6a0;
+  uint64 internal bid = 1 ether;
+
+  // params for withdraw:
+  address internal recipient = address(0xdead);
+  uint256 internal withdrawAmount = 654;
 
   function setUp() public {
     registry = new Registry();
@@ -154,35 +158,88 @@ contract TokenPortalTest is Test {
     assertEq(portalERC20.balanceOf(address(tokenPortal)), 0, "portal should have no assets");
   }
 
-  function testWithdraw() public {
-    uint256 withdrawAmount = 654;
-    portalERC20.mint(address(tokenPortal), withdrawAmount);
-    address _recipient = address(0xdead);
-    bytes32[] memory entryKeys = new bytes32[](1);
-    entryKeys[0] = outbox.computeEntryKey(
+  function _createWithdrawMessageForOutbox(address _designatedCaller)
+    internal
+    view
+    returns (bytes32)
+  {
+    bytes32 entryKey = outbox.computeEntryKey(
       DataStructures.L2ToL1Msg({
         sender: DataStructures.L2Actor({actor: l2TokenAddress, version: 1}),
         recipient: DataStructures.L1Actor({actor: address(tokenPortal), chainId: block.chainid}),
         content: Hash.sha256ToField(
-          abi.encodeWithSignature("withdraw(uint256,address)", withdrawAmount, _recipient)
+          abi.encodeWithSignature(
+            "withdraw(uint256,address,address)", withdrawAmount, recipient, _designatedCaller
+          )
           )
       })
     );
+    return entryKey;
+  }
+
+  function _addWithdrawMessageInOutbox(address _designatedCaller) internal returns (bytes32) {
+    // send assets to the portal
+    portalERC20.mint(address(tokenPortal), withdrawAmount);
 
+    // Create the message
+    bytes32[] memory entryKeys = new bytes32[](1);
+    entryKeys[0] = _createWithdrawMessageForOutbox(_designatedCaller);
     // Insert messages into the outbox (impersonating the rollup contract)
     vm.prank(address(rollup));
     outbox.sendL1Messages(entryKeys);
+    return entryKeys[0];
+  }
 
-    assertEq(portalERC20.balanceOf(_recipient), 0);
+  function testAnyoneCanCallWithdrawIfNoDesignatedCaller(address _caller) public {
+    vm.assume(_caller != address(0));
+    bytes32 expectedEntryKey = _addWithdrawMessageInOutbox(address(0));
+    assertEq(portalERC20.balanceOf(recipient), 0);
 
+    vm.startPrank(_caller);
     vm.expectEmit(true, true, true, true);
-    emit MessageConsumed(entryKeys[0], address(tokenPortal));
-    bytes32 entryKey = tokenPortal.withdraw(withdrawAmount, _recipient);
+    emit MessageConsumed(expectedEntryKey, address(tokenPortal));
+    bytes32 actualEntryKey = tokenPortal.withdraw(withdrawAmount, recipient, false);
+    assertEq(expectedEntryKey, actualEntryKey);
     // Should have received 654 RNA tokens
-    assertEq(portalERC20.balanceOf(_recipient), withdrawAmount);
+    assertEq(portalERC20.balanceOf(recipient), withdrawAmount);
 
     // Should not be able to withdraw again
-    vm.expectRevert(abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKey));
-    tokenPortal.withdraw(withdrawAmount, _recipient);
+    vm.expectRevert(
+      abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, actualEntryKey)
+    );
+    tokenPortal.withdraw(withdrawAmount, recipient, false);
+    vm.stopPrank();
+  }
+
+  function testWithdrawWithDesignatedCallerFailsForOtherCallers(address _caller) public {
+    vm.assume(_caller != address(this));
+    // add message with caller as this address
+    _addWithdrawMessageInOutbox(address(this));
+
+    vm.startPrank(_caller);
+    bytes32 entryKeyPortalChecksAgainst = _createWithdrawMessageForOutbox(_caller);
+    vm.expectRevert(
+      abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst)
+    );
+    tokenPortal.withdraw(withdrawAmount, recipient, true);
+
+    entryKeyPortalChecksAgainst = _createWithdrawMessageForOutbox(address(0));
+    vm.expectRevert(
+      abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst)
+    );
+    tokenPortal.withdraw(withdrawAmount, recipient, false);
+    vm.stopPrank();
+  }
+
+  function testWithdrawWithDesignatedCallerSucceedsForDesignatedCaller() public {
+    // add message with caller as this address
+    bytes32 expectedEntryKey = _addWithdrawMessageInOutbox(address(this));
+
+    vm.expectEmit(true, true, true, true);
+    emit MessageConsumed(expectedEntryKey, address(tokenPortal));
+    bytes32 actualEntryKey = tokenPortal.withdraw(withdrawAmount, recipient, true);
+    assertEq(expectedEntryKey, actualEntryKey);
+    // Should have received 654 RNA tokens
+    assertEq(portalERC20.balanceOf(recipient), withdrawAmount);
   }
 }

l1-contracts/test/portals/UniswapPortal.sol

@@ -61,8 +61,7 @@ contract UniswapPortal {
     IERC20 outputAsset = TokenPortal(_outputTokenPortal).underlying();
 
     // Withdraw the input asset from the portal
-    // todo: add `true` when using designated caller
-    TokenPortal(_inputTokenPortal).withdraw(_inAmount, address(this));
+    TokenPortal(_inputTokenPortal).withdraw(_inAmount, address(this), true);
 
     {
       // prevent stack too deep errors

l1-contracts/test/portals/UniswapPortal.t.sol

@@ -72,7 +72,9 @@ contract UniswapPortalTest is Test {
       sender: DataStructures.L2Actor(l2TokenAddress, 1),
       recipient: DataStructures.L1Actor(address(daiTokenPortal), block.chainid),
       content: Hash.sha256ToField(
-        abi.encodeWithSignature("withdraw(uint256,address)", amount, _recipient)
+        abi.encodeWithSignature(
+          "withdraw(uint256,address,address)", amount, _recipient, address(uniswapPortal)
+        )
         )
     });
     entryKey = outbox.computeEntryKey(message);

yarn-project/end-to-end/src/cross_chain/test_harness.ts

@@ -158,15 +158,16 @@ export class CrossChainTestHarness {
     expect(transferReceipt.status).toBe(TxStatus.MINED);
   }
 
-  async checkEntryIsNotInOutbox(withdrawAmount: bigint): Promise<Fr> {
+  async checkEntryIsNotInOutbox(withdrawAmount: bigint, callerOnL1: EthAddress = EthAddress.ZERO): Promise<Fr> {
     this.logger('Ensure that the entry is not in outbox yet');
     const contractInfo = await this.aztecNode.getContractInfo(this.l2Contract.address);
-    // 0x00f714ce, selector for "withdraw(uint256,address)"
+    // 0xb460af94, selector for "withdraw(uint256,address,address)"
     const content = sha256ToField(
       Buffer.concat([
-        Buffer.from([0x00, 0xf7, 0x14, 0xce]),
+        Buffer.from([0xb4, 0x60, 0xaf, 0x94]),
         toBufferBE(withdrawAmount, 32),
         this.ethAccount.toBuffer32(),
+        callerOnL1.toBuffer32(),
       ]),
     );
     const entryKey = sha256ToField(
@@ -189,6 +190,7 @@ export class CrossChainTestHarness {
     const { request: withdrawRequest, result: withdrawEntryKey } = await this.tokenPortal.simulate.withdraw([
       withdrawAmount,
       this.ethAccount.toString(),
+      false,
     ]);
 
     expect(withdrawEntryKey).toBe(entryKey.toString(true));

yarn-project/end-to-end/src/e2e_cross_chain_messaging.test.ts

@@ -80,7 +80,9 @@ describe('e2e_cross_chain_messaging', () => {
 
   const withdrawFundsFromAztec = async (withdrawAmount: bigint) => {
     logger('Send L2 tx to withdraw funds');
-    const withdrawTx = l2Contract.methods.withdraw(withdrawAmount, ownerPub, ethAccount).send({ from: ownerAddress });
+    const withdrawTx = l2Contract.methods
+      .withdraw(withdrawAmount, ownerPub, ethAccount, EthAddress.ZERO.toField())
+      .send({ from: ownerAddress });
 
     await withdrawTx.isMined(0, 0.1);
     const withdrawReceipt = await withdrawTx.getReceipt();

yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts

@@ -74,7 +74,7 @@ describe('e2e_public_cross_chain_messaging', () => {
   const withdrawFundsFromAztec = async (withdrawAmount: bigint) => {
     logger('Send L2 tx to withdraw funds');
     const withdrawTx = l2Contract.methods
-      .withdrawPublic(withdrawAmount, ethAccount.toField())
+      .withdrawPublic(withdrawAmount, ethAccount.toField(), EthAddress.ZERO.toField())
       .send({ from: ownerAddress });
 
     await withdrawTx.isMined(0, 0.1);

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

@@ -96,6 +96,7 @@ contract NonNativeToken {
         amount: pub Field, 
         sender: pub Point, 
         recipient: pub Field, // ethereum address in the field
+        callerOnL1: pub Field, // ethereum address that can call this function on the L1 portal (0x0 if anyone can call)
     ) -> pub [Field; dep::aztec3::abi::PUBLIC_INPUTS_LENGTH] {
         let mut initialContext = PrivateFunctionContext::new();
         initialContext.args = initialContext.args.push_array([amount, sender.x, sender.y, recipient]);
@@ -123,7 +124,7 @@ contract NonNativeToken {
         context = sender_balance.insert(context, change_note);
         constrain emit_encrypted_log(inputs.call_context.storage_contract_address, sender_balance.storage_slot, change_note.owner, change_note) == 0;
 
-        let content = _get_withdraw_content_hash(amount, recipient);
+        let content = _get_withdraw_content_hash(amount, recipient, callerOnL1);
 
         context = context.message_portal(content);
 
@@ -165,6 +166,7 @@ contract NonNativeToken {
         //*********************************/
         amount: Field,
         recipient: Field,
+        callerOnL1: Field, // ethereum address that can call this function on the L1 portal (0x0 if anyone can call)
         _padding: [Field; dep::aztec3::abi::MAX_ARGS - 2]
     ) {
 
@@ -178,7 +180,7 @@ contract NonNativeToken {
         }  
         // TODO: Revert if there is not enough balance
 
-        let content = _get_withdraw_content_hash(amount, recipient);
+        let content = _get_withdraw_content_hash(amount, recipient, callerOnL1);
 
         // Emit the l2 to l1 message
         create_l2_to_l1_message(content);
@@ -425,20 +427,21 @@ contract NonNativeToken {
         content_hash
     }
 
-    fn _get_withdraw_content_hash(amount: Field, recipient: Field) -> pub Field {
+    fn _get_withdraw_content_hash(amount: Field, recipient: Field, callerOnL1: Field) -> pub Field {
         // Compute the content hash
         // Compute sha256(selector || amount || recipient)
         // then convert to a single field element
         // add that to the l2 to l1 messages
-        let mut hash_bytes: [u8; 68] = [0; 68];
+        let mut hash_bytes: [u8; 100] = [0; 100];
         let amount_bytes = amount.to_be_bytes(32);
         let recipient_bytes = recipient.to_be_bytes(32);
+        let callerOnL1_bytes = callerOnL1.to_be_bytes(32);
 
-        // 0x00f714ce, selector for "withdraw(uint256,address)"
-        hash_bytes[0] = 0x00;
-        hash_bytes[1] = 0xf7;
-        hash_bytes[2] = 0x14;
-        hash_bytes[3] = 0xce;
+        //  0xb460af94, selector for "withdraw(uint256,address,address)"
+        hash_bytes[0] = 0xb4;
+        hash_bytes[1] = 0x60;
+        hash_bytes[2] = 0xaf;
+        hash_bytes[3] = 0x94;
 
         // Unroll loops because otherwise takes forever to compile
         // for i in range(32):
@@ -511,6 +514,39 @@ contract NonNativeToken {
         hash_bytes[34 + 32] = recipient_bytes[30];
         hash_bytes[35 + 32] = recipient_bytes[31];
 
+        hash_bytes[4 + 64] = callerOnL1_bytes[0];
+        hash_bytes[5 + 64] = callerOnL1_bytes[1];
+        hash_bytes[6 + 64] = callerOnL1_bytes[2];
+        hash_bytes[7 + 64] = callerOnL1_bytes[3];
+        hash_bytes[8 + 64] = callerOnL1_bytes[4];
+        hash_bytes[9 + 64] = callerOnL1_bytes[5];
+        hash_bytes[10 + 64] = callerOnL1_bytes[6];
+        hash_bytes[11 + 64] = callerOnL1_bytes[7];
+        hash_bytes[12 + 64] = callerOnL1_bytes[8];
+        hash_bytes[13 + 64] = callerOnL1_bytes[9];
+        hash_bytes[14 + 64] = callerOnL1_bytes[10];
+        hash_bytes[15 + 64] = callerOnL1_bytes[11];
+        hash_bytes[16 + 64] = callerOnL1_bytes[12];
+        hash_bytes[17 + 64] = callerOnL1_bytes[13];
+        hash_bytes[18 + 64] = callerOnL1_bytes[14];
+        hash_bytes[19 + 64] = callerOnL1_bytes[15];
+        hash_bytes[20 + 64] = callerOnL1_bytes[16];
+        hash_bytes[21 + 64] = callerOnL1_bytes[17];
+        hash_bytes[22 + 64] = callerOnL1_bytes[18];
+        hash_bytes[23 + 64] = callerOnL1_bytes[19];
+        hash_bytes[24 + 64] = callerOnL1_bytes[20];
+        hash_bytes[25 + 64] = callerOnL1_bytes[21];
+        hash_bytes[26 + 64] = callerOnL1_bytes[22];
+        hash_bytes[27 + 64] = callerOnL1_bytes[23];
+        hash_bytes[28 + 64] = callerOnL1_bytes[24];
+        hash_bytes[29 + 64] = callerOnL1_bytes[25];
+        hash_bytes[30 + 64] = callerOnL1_bytes[26];
+        hash_bytes[31 + 64] = callerOnL1_bytes[27];
+        hash_bytes[32 + 64] = callerOnL1_bytes[28];
+        hash_bytes[33 + 64] = callerOnL1_bytes[29];
+        hash_bytes[34 + 64] = callerOnL1_bytes[30];
+        hash_bytes[35 + 64] = callerOnL1_bytes[31];
+
         let content_sha256 = dep::std::hash::sha256(hash_bytes);
 
         // Convert the content_sha256 to a field element

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

@@ -31,7 +31,7 @@ contract Uniswap {
         uniswapFeeTier: pub Field, // which uniswap tier to use (eg 3000 for 0.3% fee)
         outputAsset: pub Field,
         outputAssetPortalAddress: pub Field, // l1 portal of output asset
-        mminimumOutputAmount: pub Field, // minimum output amount to receive (slippage protection for the swap)
+        minimumOutputAmount: pub Field, // minimum output amount to receive (slippage protection for the swap)
         sender: pub Point, 
         recipient: pub Field, // recevier address of output asset after the swap
         secretHash: pub Field, // for when l1 uniswap portal inserts the message to consume output assets on L2
@@ -49,7 +49,7 @@ contract Uniswap {
             uniswapFeeTier,
             outputAsset,
             outputAssetPortalAddress,
-            mminimumOutputAmount,
+            minimumOutputAmount,
             sender.x,
             sender.y,
             recipient,
@@ -59,12 +59,15 @@ contract Uniswap {
             l1UniswapPortal,
         ]);
 
-        // inputAsset.withdraw(inputAmount, l1UniswapPortal)
+        // inputAsset.withdraw(inputAmount, sender, recipient=l1UniswapPortal, callerOnL1=l1UniswapPortal) 
+        // only uniswap portal can call this (done to safegaurd ordering of message consumption)
+        // ref: https://docs.aztec.network/aztec/how-it-works/l1-l2-messaging#designated-caller
         let mut args = [0; dep::aztec3::abi::MAX_ARGS];
         args[0] = inputAmount;
         args[1] = sender.x;
         args[2] = sender.y;
         args[3] = l1UniswapPortal;
+        args[4] = l1UniswapPortal;
         let (callStackItem, mut context) = PrivateCallStackItem::call(inputAsset, withdrawFnSelector, args, initialContext);
 
         let result = callStackItem.public_inputs.return_values[0];
@@ -76,7 +79,7 @@ contract Uniswap {
            inputAmount,
            uniswapFeeTier,
            outputAssetPortalAddress,
-           mminimumOutputAmount,
+           minimumOutputAmount,
            recipient,
            secretHash,
            deadlineForL1ToL2Message,