From 2ecb9ea4e53c1e1abedc759979121caac0fb88f0 Mon Sep 17 00:00:00 2001 From: Rahul Kothari Date: Thu, 28 Sep 2023 11:07:31 +0000 Subject: [PATCH] change param ordering in portal, split uniswap portal fns --- l1-contracts/test/portals/TokenPortal.sol | 28 +- l1-contracts/test/portals/TokenPortal.t.sol | 20 +- l1-contracts/test/portals/UniswapPortal.sol | 132 +++++++--- l1-contracts/test/portals/UniswapPortal.t.sol | 240 +++++++++--------- .../src/uniswap_trade_on_l1_from_l2.test.ts | 11 +- .../src/fixtures/cross_chain_test_harness.ts | 8 +- .../src/integration_archiver_l1_to_l2.test.ts | 4 +- .../src/uniswap_trade_on_l1_from_l2.test.ts | 5 +- 8 files changed, 257 insertions(+), 191 deletions(-) diff --git a/l1-contracts/test/portals/TokenPortal.sol b/l1-contracts/test/portals/TokenPortal.sol index e9c75c4376a..37d5f5cb472 100644 --- a/l1-contracts/test/portals/TokenPortal.sol +++ b/l1-contracts/test/portals/TokenPortal.sol @@ -25,19 +25,19 @@ contract TokenPortal { // docs:start:deposit_public /** * @notice Deposit funds into the portal and adds an L2 message which can only be consumed publicly on Aztec - * @param _to - The aztec address of the recipient * @param _amount - The amount to deposit + * @param _to - The aztec address of the recipient + * @param _canceller - The address that can cancel the L1 to L2 message * @param _deadline - The timestamp after which the entry can be cancelled * @param _secretHash - The hash of the secret consumable message. The hash should be 254 bits (so it can fit in a Field element) - * @param _canceller - The address that can cancel the L1 to L2 message * @return The key of the entry in the Inbox */ function depositToAztecPublic( - bytes32 _to, uint256 _amount, + bytes32 _to, + address _canceller, uint32 _deadline, - bytes32 _secretHash, - address _canceller + bytes32 _secretHash ) external payable returns (bytes32) { // Preamble IInbox inbox = registry.getInbox(); @@ -59,18 +59,18 @@ contract TokenPortal { /** * @notice Deposit funds into the portal and adds an L2 message which can only be consumed privately on Aztec * @param _amount - The amount to deposit - * @param _deadline - The timestamp after which the entry can be cancelled - * @param _secretHashForL2MessageConsumption - The hash of the secret consumable L1 to L2 message. The hash should be 254 bits (so it can fit in a Field element) * @param _secretHashForRedeemingMintedNotes - The hash of the secret to redeem minted notes privately on Aztec. The hash should be 254 bits (so it can fit in a Field element) * @param _canceller - The address that can cancel the L1 to L2 message + * @param _deadline - The timestamp after which the entry can be cancelled + * @param _secretHashForL2MessageConsumption - The hash of the secret consumable L1 to L2 message. The hash should be 254 bits (so it can fit in a Field element) * @return The key of the entry in the Inbox */ function depositToAztecPrivate( uint256 _amount, - uint32 _deadline, - bytes32 _secretHashForL2MessageConsumption, bytes32 _secretHashForRedeemingMintedNotes, - address _canceller + address _canceller, + uint32 _deadline, + bytes32 _secretHashForL2MessageConsumption ) external payable returns (bytes32) { // Preamble IInbox inbox = registry.getInbox(); @@ -99,16 +99,16 @@ contract TokenPortal { /** * @notice Cancel a public depositToAztec L1 to L2 message * @dev only callable by the `canceller` of the message - * @param _to - The aztec address of the recipient in the original message * @param _amount - The amount to deposit per the original message + * @param _to - The aztec address of the recipient in the original message * @param _deadline - The timestamp after which the entry can be cancelled * @param _secretHash - The hash of the secret consumable message in the original message * @param _fee - The fee paid to the sequencer * @return The key of the entry in the Inbox */ function cancelL1ToAztecMessagePublic( - bytes32 _to, uint256 _amount, + bytes32 _to, uint32 _deadline, bytes32 _secretHash, uint64 _fee @@ -137,17 +137,17 @@ contract TokenPortal { * @notice Cancel a private depositToAztec L1 to L2 message * @dev only callable by the `canceller` of the message * @param _amount - The amount to deposit per the original message + * @param _secretHashForRedeemingMintedNotes - The hash of the secret to redeem minted notes privately on Aztec * @param _deadline - The timestamp after which the entry can be cancelled * @param _secretHashForL2MessageConsumption - The hash of the secret consumable L1 to L2 message - * @param _secretHashForL2MessageConsumption - The hash of the secret to redeem minted notes privately on Aztec * @param _fee - The fee paid to the sequencer * @return The key of the entry in the Inbox */ function cancelL1ToAztecMessagePrivate( uint256 _amount, + bytes32 _secretHashForRedeemingMintedNotes, uint32 _deadline, bytes32 _secretHashForL2MessageConsumption, - bytes32 _secretHashForRedeemingMintedNotes, uint64 _fee ) external returns (bytes32) { IInbox inbox = registry.getInbox(); diff --git a/l1-contracts/test/portals/TokenPortal.t.sol b/l1-contracts/test/portals/TokenPortal.t.sol index d914124e607..4b3d35e32a2 100644 --- a/l1-contracts/test/portals/TokenPortal.t.sol +++ b/l1-contracts/test/portals/TokenPortal.t.sol @@ -143,10 +143,10 @@ contract TokenPortalTest is Test { // Perform op bytes32 entryKey = tokenPortal.depositToAztecPrivate{value: bid}( amount, - deadline, - secretHashForL2MessageConsumption, secretHashForRedeemingMintedNotes, - address(this) + address(this), + deadline, + secretHashForL2MessageConsumption ); assertEq(entryKey, expectedEntryKey, "returned entry key and calculated entryKey should match"); @@ -185,7 +185,7 @@ contract TokenPortalTest is Test { // Perform op bytes32 entryKey = tokenPortal.depositToAztecPublic{value: bid}( - to, amount, deadline, secretHashForL2MessageConsumption, address(this) + amount, to, address(this), deadline, secretHashForL2MessageConsumption ); assertEq(entryKey, expectedEntryKey, "returned entry key and calculated entryKey should match"); @@ -210,7 +210,7 @@ contract TokenPortalTest is Test { abi.encodeWithSelector(Errors.Inbox__NothingToConsume.selector, expectedWrongEntryKey) ); tokenPortal.cancelL1ToAztecMessagePublic( - to, amount, deadline, secretHashForL2MessageConsumption, bid + amount, to, deadline, secretHashForL2MessageConsumption, bid ); vm.stopPrank(); @@ -221,7 +221,7 @@ contract TokenPortalTest is Test { abi.encodeWithSelector(Errors.Inbox__NothingToConsume.selector, expectedWrongEntryKey) ); tokenPortal.cancelL1ToAztecMessagePrivate( - amount, deadline, secretHashForL2MessageConsumption, secretHashForRedeemingMintedNotes, bid + amount, secretHashForRedeemingMintedNotes, deadline, secretHashForL2MessageConsumption, bid ); // actually cancel the message @@ -231,7 +231,7 @@ contract TokenPortalTest is Test { emit L1ToL2MessageCancelled(expectedEntryKey); // perform op bytes32 entryKey = tokenPortal.cancelL1ToAztecMessagePublic( - to, amount, deadline, secretHashForL2MessageConsumption, bid + amount, to, deadline, secretHashForL2MessageConsumption, bid ); assertEq(entryKey, expectedEntryKey, "returned entry key and calculated entryKey should match"); @@ -257,7 +257,7 @@ contract TokenPortalTest is Test { abi.encodeWithSelector(Errors.Inbox__NothingToConsume.selector, expectedWrongEntryKey) ); tokenPortal.cancelL1ToAztecMessagePrivate( - amount, deadline, secretHashForL2MessageConsumption, secretHashForRedeemingMintedNotes, bid + amount, secretHashForRedeemingMintedNotes, deadline, secretHashForL2MessageConsumption, bid ); vm.stopPrank(); @@ -268,7 +268,7 @@ contract TokenPortalTest is Test { abi.encodeWithSelector(Errors.Inbox__NothingToConsume.selector, expectedWrongEntryKey) ); tokenPortal.cancelL1ToAztecMessagePublic( - to, amount, deadline, secretHashForL2MessageConsumption, bid + amount, to, deadline, secretHashForL2MessageConsumption, bid ); // actually cancel the message @@ -278,7 +278,7 @@ contract TokenPortalTest is Test { emit L1ToL2MessageCancelled(expectedEntryKey); // perform op bytes32 entryKey = tokenPortal.cancelL1ToAztecMessagePrivate( - amount, deadline, secretHashForL2MessageConsumption, secretHashForRedeemingMintedNotes, bid + amount, secretHashForRedeemingMintedNotes, deadline, secretHashForL2MessageConsumption, bid ); assertEq(entryKey, expectedEntryKey, "returned entry key and calculated entryKey should match"); diff --git a/l1-contracts/test/portals/UniswapPortal.sol b/l1-contracts/test/portals/UniswapPortal.sol index c528dafb207..76b0287c2fb 100644 --- a/l1-contracts/test/portals/UniswapPortal.sol +++ b/l1-contracts/test/portals/UniswapPortal.sol @@ -36,7 +36,7 @@ contract UniswapPortal { // docs:start:solidity_uniswap_swap /** - * @notice Exit with funds from L2, perform swap on L1 and deposit output asset to L2 again + * @notice Exit with funds from L2, perform swap on L1 and deposit output asset to L2 again publicly * @dev `msg.value` indicates fee to submit message to inbox. Currently, anyone can call this method on your behalf. * They could call it with 0 fee causing the sequencer to never include in the rollup. * In this case, you will have to cancel the message and then make the deposit later @@ -45,26 +45,24 @@ contract UniswapPortal { * @param _uniswapFeeTier - The fee tier for the swap on UniswapV3 * @param _outputTokenPortal - The ethereum address of the output token portal * @param _amountOutMinimum - The minimum amount of output assets to receive from the swap (slippage protection) - * @param _aztecRecipientOrSecretHashForRedeemingMintedNotes - If public flow, the aztec address to receive the output assets. If private, the hash of the secret to redeem minted notes privately on Aztec. - * @param _secretHashForL1ToL2Message - The hash of the secret consumable message + * @param _aztecRecipient - The aztec address to receive the output assets + * @param _secretHashForL1ToL2Message - The hash of the secret consumable message. The hash should be 254 bits (so it can fit in a Field element) * @param _deadlineForL1ToL2Message - deadline for when the L1 to L2 message (to mint outpiut assets in L2) must be consumed by * @param _canceller - The ethereum address that can cancel the deposit * @param _withCaller - When true, using `msg.sender` as the caller, otherwise address(0) - * @param _isPrivateFlow - When true, the output assets will be minted privately on Aztec, otherwise publicly * @return The entryKey of the deposit transaction in the Inbox */ - function swap( + function swapPublic( address _inputTokenPortal, uint256 _inAmount, uint24 _uniswapFeeTier, address _outputTokenPortal, uint256 _amountOutMinimum, - bytes32 _aztecRecipientOrSecretHashForRedeemingMintedNotes, + bytes32 _aztecRecipient, bytes32 _secretHashForL1ToL2Message, uint32 _deadlineForL1ToL2Message, address _canceller, - bool _withCaller, - bool _isPrivateFlow + bool _withCaller ) public payable returns (bytes32) { LocalSwapVars memory vars; @@ -75,22 +73,15 @@ contract UniswapPortal { TokenPortal(_inputTokenPortal).withdraw(_inAmount, address(this), true); { // prevent stack too deep errors - - // having two different hashes mean you can't consume a message intended for private in public. - string memory functionSignature = _isPrivateFlow - ? - "swap_private(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)" - : "swap_public(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)"; - vars.contentHash = Hash.sha256ToField( abi.encodeWithSignature( - functionSignature, + "swap_public(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)", _inputTokenPortal, _inAmount, _uniswapFeeTier, _outputTokenPortal, _amountOutMinimum, - _aztecRecipientOrSecretHashForRedeemingMintedNotes, + _aztecRecipient, _secretHashForL1ToL2Message, _deadlineForL1ToL2Message, _canceller, @@ -131,22 +122,105 @@ contract UniswapPortal { vars.outputAsset.approve(address(_outputTokenPortal), amountOut); // Deposit the output asset to the L2 via its portal - if (_isPrivateFlow) { - return TokenPortal(_outputTokenPortal).depositToAztecPrivate{value: msg.value}( - amountOut, - _deadlineForL1ToL2Message, - _secretHashForL1ToL2Message, - _aztecRecipientOrSecretHashForRedeemingMintedNotes, - _canceller + return TokenPortal(_outputTokenPortal).depositToAztecPublic{value: msg.value}( + amountOut, _aztecRecipient, _canceller, _deadlineForL1ToL2Message, _secretHashForL1ToL2Message + ); + } + // docs:end:solidity_uniswap_swap + + /** + * @notice Exit with funds from L2, perform swap on L1 and deposit output asset to L2 again privately + * @dev `msg.value` indicates fee to submit message to inbox. Currently, anyone can call this method on your behalf. + * They could call it with 0 fee causing the sequencer to never include in the rollup. + * In this case, you will have to cancel the message and then make the deposit later + * @param _inputTokenPortal - The ethereum address of the input token portal + * @param _inAmount - The amount of assets to swap (same amount as withdrawn from L2) + * @param _uniswapFeeTier - The fee tier for the swap on UniswapV3 + * @param _outputTokenPortal - The ethereum address of the output token portal + * @param _amountOutMinimum - The minimum amount of output assets to receive from the swap (slippage protection) + * @param _secretHashForRedeemingMintedNotes - The hash of the secret to redeem minted notes privately on Aztec. The hash should be 254 bits (so it can fit in a Field element) + * @param _secretHashForL1ToL2Message - The hash of the secret consumable message. The hash should be 254 bits (so it can fit in a Field element) + * @param _deadlineForL1ToL2Message - deadline for when the L1 to L2 message (to mint outpiut assets in L2) must be consumed by + * @param _canceller - The ethereum address that can cancel the deposit + * @param _withCaller - When true, using `msg.sender` as the caller, otherwise address(0) + * @return The entryKey of the deposit transaction in the Inbox + */ + function swapPrivate( + address _inputTokenPortal, + uint256 _inAmount, + uint24 _uniswapFeeTier, + address _outputTokenPortal, + uint256 _amountOutMinimum, + bytes32 _secretHashForRedeemingMintedNotes, + bytes32 _secretHashForL1ToL2Message, + uint32 _deadlineForL1ToL2Message, + address _canceller, + bool _withCaller + ) public payable returns (bytes32) { + LocalSwapVars memory vars; + + vars.inputAsset = TokenPortal(_inputTokenPortal).underlying(); + vars.outputAsset = TokenPortal(_outputTokenPortal).underlying(); + + // Withdraw the input asset from the portal + TokenPortal(_inputTokenPortal).withdraw(_inAmount, address(this), true); + { + // prevent stack too deep errors + vars.contentHash = Hash.sha256ToField( + abi.encodeWithSignature( + "swap_private(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)", + _inputTokenPortal, + _inAmount, + _uniswapFeeTier, + _outputTokenPortal, + _amountOutMinimum, + _secretHashForRedeemingMintedNotes, + _secretHashForL1ToL2Message, + _deadlineForL1ToL2Message, + _canceller, + _withCaller ? msg.sender : address(0) + ) ); } - return TokenPortal(_outputTokenPortal).depositToAztecPublic{value: msg.value}( - _aztecRecipientOrSecretHashForRedeemingMintedNotes, + + // Consume the message from the outbox + registry.getOutbox().consume( + DataStructures.L2ToL1Msg({ + sender: DataStructures.L2Actor(l2UniswapAddress, 1), + recipient: DataStructures.L1Actor(address(this), block.chainid), + content: vars.contentHash + }) + ); + + // Perform the swap + ISwapRouter.ExactInputSingleParams memory swapParams; + { + swapParams = ISwapRouter.ExactInputSingleParams({ + tokenIn: address(vars.inputAsset), + tokenOut: address(vars.outputAsset), + fee: _uniswapFeeTier, + recipient: address(this), + deadline: block.timestamp, + amountIn: _inAmount, + amountOutMinimum: _amountOutMinimum, + sqrtPriceLimitX96: 0 + }); + } + // Note, safeApprove was deprecated from Oz + vars.inputAsset.approve(address(ROUTER), _inAmount); + uint256 amountOut = ROUTER.exactInputSingle(swapParams); + + // approve the output token portal to take funds from this contract + // Note, safeApprove was deprecated from Oz + vars.outputAsset.approve(address(_outputTokenPortal), amountOut); + + // Deposit the output asset to the L2 via its portal + return TokenPortal(_outputTokenPortal).depositToAztecPrivate{value: msg.value}( amountOut, + _secretHashForRedeemingMintedNotes, + _canceller, _deadlineForL1ToL2Message, - _secretHashForL1ToL2Message, - _canceller + _secretHashForL1ToL2Message ); } - // docs:end:solidity_uniswap_swap } diff --git a/l1-contracts/test/portals/UniswapPortal.t.sol b/l1-contracts/test/portals/UniswapPortal.t.sol index c554d20fbdf..02318a95c84 100644 --- a/l1-contracts/test/portals/UniswapPortal.t.sol +++ b/l1-contracts/test/portals/UniswapPortal.t.sol @@ -35,11 +35,12 @@ contract UniswapPortalTest is Test { UniswapPortal internal uniswapPortal; uint256 internal amount = 100 ether; - bytes32 internal secretHashForL1ToL2Message = bytes32(0); + bytes32 internal secretHash = bytes32(0); uint24 internal uniswapFeePool = 3000; // 0.3% fee uint256 internal amountOutMinimum = 0; uint32 internal deadlineForL1ToL2Message; // set after fork is activated - bytes32 internal aztecRecipientOrSecretHash = bytes32(uint256(0x3)); + bytes32 internal aztecRecipient = bytes32(uint256(0x3)); + bytes32 internal secretHashForRedeemingMintedNotes = bytes32(uint256(0x4)); function setUp() public { // fork mainnet @@ -68,7 +69,6 @@ contract UniswapPortalTest is Test { /** * L2 to L1 message withdraw to be added to the outbox - * This is the message from L2 to swap the input token * @param _recipient - the L1 address that should receive the funds after withdraw * @param _caller - designated caller on L1 that will call the withdraw function - typically uniswapPortal * Set to address(0) if anyone can call. @@ -90,45 +90,65 @@ contract UniswapPortalTest is Test { /** * L2 to L1 message to be added to the outbox - - * @param _isPrivateFlow - If true, the output assets will be minted privately on Aztec, otherwise publicly - * @param _aztecRecipientOrSecretHashForRedeemingMintedNotes - If public flow, the aztec address to receive the output assets. If private, the hash of the secret to redeem minted notes privately on Aztec. + * @param _aztecRecipient - the recipient on L2 that will receive the output of the swap * @param _caller - designated caller on L1 that will call the swap function - typically address(this) * Set to address(0) if anyone can call. */ - function _createUniswapSwapMessage( - bool _isPrivateFlow, - bytes32 _aztecRecipientOrSecretHashForRedeemingMintedNotes, - address _caller - ) internal view returns (bytes32 entryKey) { - bytes32 contentHash; - { - // prevent stack too deep errors - string memory functionSignature = _isPrivateFlow - ? - "swap_private(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)" - : "swap_public(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)"; - - contentHash = Hash.sha256ToField( + function _createUniswapSwapMessagePublic(bytes32 _aztecRecipient, address _caller) + internal + view + returns (bytes32 entryKey) + { + DataStructures.L2ToL1Msg memory message = DataStructures.L2ToL1Msg({ + sender: DataStructures.L2Actor(l2UniswapAddress, 1), + recipient: DataStructures.L1Actor(address(uniswapPortal), block.chainid), + content: Hash.sha256ToField( abi.encodeWithSignature( - functionSignature, + "swap_public(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)", address(daiTokenPortal), amount, uniswapFeePool, address(wethTokenPortal), amountOutMinimum, - _aztecRecipientOrSecretHashForRedeemingMintedNotes, - secretHashForL1ToL2Message, + _aztecRecipient, + secretHash, deadlineForL1ToL2Message, address(this), _caller ) - ); - } + ) + }); + entryKey = outbox.computeEntryKey(message); + } + /** + * L2 to L1 message to be added to the outbox - + * @param _secretHashForRedeemingMintedNotes - The hash of the secret to redeem minted notes privately on Aztec + * @param _caller - designated caller on L1 that will call the swap function - typically address(this) + * Set to address(0) if anyone can call. + */ + function _createUniswapSwapMessagePrivate( + bytes32 _secretHashForRedeemingMintedNotes, + address _caller + ) internal view returns (bytes32 entryKey) { DataStructures.L2ToL1Msg memory message = DataStructures.L2ToL1Msg({ sender: DataStructures.L2Actor(l2UniswapAddress, 1), recipient: DataStructures.L1Actor(address(uniswapPortal), block.chainid), - content: contentHash + content: Hash.sha256ToField( + abi.encodeWithSignature( + "swap_private(address,uint256,uint24,address,uint256,bytes32,bytes32,uint32,address,address)", + address(daiTokenPortal), + amount, + uniswapFeePool, + address(wethTokenPortal), + amountOutMinimum, + _secretHashForRedeemingMintedNotes, + secretHash, + deadlineForL1ToL2Message, + address(this), + _caller + ) + ) }); entryKey = outbox.computeEntryKey(message); } @@ -145,7 +165,6 @@ contract UniswapPortalTest is Test { // Creates a withdraw transaction without a designated caller. // Should fail when uniswap portal tries to consume it since it tries using a designated caller. function testRevertIfWithdrawMessageHasNoDesignatedCaller() public { - // create message with no designated caller bytes32 entryKey = _createDaiWithdrawMessage(address(uniswapPortal), address(0)); _addMessagesToOutbox(entryKey, bytes32(uint256(0x1))); bytes32 entryKeyPortalChecksAgainst = @@ -153,18 +172,17 @@ contract UniswapPortalTest is Test { vm.expectRevert( abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst) ); - uniswapPortal.swap( + uniswapPortal.swapPublic( address(daiTokenPortal), amount, uniswapFeePool, address(wethTokenPortal), amountOutMinimum, - aztecRecipientOrSecretHash, - secretHashForL1ToL2Message, + aztecRecipient, + secretHash, deadlineForL1ToL2Message, address(this), - true, // say that there is a designated caller - false + true ); } @@ -181,97 +199,63 @@ contract UniswapPortalTest is Test { vm.expectRevert( abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst) ); - uniswapPortal.swap( + uniswapPortal.swapPublic( address(daiTokenPortal), amount, uniswapFeePool, address(wethTokenPortal), amountOutMinimum, - aztecRecipientOrSecretHash, - secretHashForL1ToL2Message, + aztecRecipient, + secretHash, deadlineForL1ToL2Message, address(this), - true, - false + true ); } function testRevertIfSwapParamsDifferentToOutboxMessage() public { bytes32 daiWithdrawMsgKey = _createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal)); - bytes32 swapMsgKey = _createUniswapSwapMessage(false, aztecRecipientOrSecretHash, address(this)); + bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this)); _addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey); bytes32 newAztecRecipient = bytes32(uint256(0x4)); bytes32 entryKeyPortalChecksAgainst = - _createUniswapSwapMessage(false, newAztecRecipient, address(this)); + _createUniswapSwapMessagePublic(newAztecRecipient, address(this)); vm.expectRevert( abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst) ); - uniswapPortal.swap( + uniswapPortal.swapPublic( address(daiTokenPortal), amount, uniswapFeePool, address(wethTokenPortal), amountOutMinimum, newAztecRecipient, // change recipient of swapped token to some other address - secretHashForL1ToL2Message, + secretHash, deadlineForL1ToL2Message, address(this), - true, - false + true ); } - // use bool fuzzer to test for both public and private flows - function testRevertIfSwapMessageWasForDifferentPublicOrPrivateFlow(bool _isPrivateFlow) public { + function testSwapWithDesignatedCaller() public { bytes32 daiWithdrawMsgKey = _createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal)); - // Create message for `_isPrivateFlow`: - bytes32 swapMsgKey = - _createUniswapSwapMessage(_isPrivateFlow, aztecRecipientOrSecretHash, address(this)); + bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this)); _addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey); - bytes32 entryKeyPortalChecksAgainst = - _createUniswapSwapMessage(!_isPrivateFlow, aztecRecipientOrSecretHash, address(this)); - vm.expectRevert( - abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst) - ); - - uniswapPortal.swap( + bytes32 l1ToL2MessageKey = uniswapPortal.swapPublic( address(daiTokenPortal), amount, uniswapFeePool, address(wethTokenPortal), amountOutMinimum, - aztecRecipientOrSecretHash, - secretHashForL1ToL2Message, + aztecRecipient, + secretHash, deadlineForL1ToL2Message, address(this), - true, - !_isPrivateFlow // pass the opposite of `_isPrivateFlow` to the swap function - ); - } - - function testSwapWithDesignatedCaller(bool _isPrivateFlow) public { - bytes32 daiWithdrawMsgKey = - _createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal)); - bytes32 swapMsgKey = - _createUniswapSwapMessage(_isPrivateFlow, aztecRecipientOrSecretHash, address(this)); - _addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey); - - bytes32 l1ToL2MessageKey = uniswapPortal.swap( - address(daiTokenPortal), - amount, - uniswapFeePool, - address(wethTokenPortal), - amountOutMinimum, - aztecRecipientOrSecretHash, - secretHashForL1ToL2Message, - deadlineForL1ToL2Message, - address(this), - true, - _isPrivateFlow + true ); // dai should be taken away from dai portal @@ -285,30 +269,26 @@ contract UniswapPortalTest is Test { assertFalse(outbox.contains(swapMsgKey)); } - function testSwapCalledbyAnyoneIfDesignatedCallerNotSet(address _caller, bool _isPrivateFlow) - public - { + function testSwapCalledbyAnyoneIfDesignatedCallerNotSet(address _caller) public { vm.assume(_caller != address(uniswapPortal)); bytes32 daiWithdrawMsgKey = _createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal)); - // don't set caller on swap() -> so anyone can call this method. - bytes32 swapMsgKey = - _createUniswapSwapMessage(_isPrivateFlow, aztecRecipientOrSecretHash, address(0)); + // don't set caller on swapPublic() -> so anyone can call this method. + bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(0)); _addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey); vm.prank(_caller); - bytes32 l1ToL2MessageKey = uniswapPortal.swap( + bytes32 l1ToL2MessageKey = uniswapPortal.swapPublic( address(daiTokenPortal), amount, uniswapFeePool, address(wethTokenPortal), amountOutMinimum, - aztecRecipientOrSecretHash, - secretHashForL1ToL2Message, + aztecRecipient, + secretHash, deadlineForL1ToL2Message, address(this), - false, - _isPrivateFlow + false ); // check that swap happened: // dai should be taken away from dai portal @@ -322,80 +302,71 @@ contract UniswapPortalTest is Test { assertFalse(outbox.contains(swapMsgKey)); } - function testRevertIfSwapWithDesignatedCallerCalledByWrongCaller( - address _caller, - bool _isPrivateFlow - ) public { + function testRevertIfSwapWithDesignatedCallerCalledByWrongCaller(address _caller) public { vm.assume(_caller != address(this)); bytes32 daiWithdrawMsgKey = _createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal)); - bytes32 swapMsgKey = - _createUniswapSwapMessage(_isPrivateFlow, aztecRecipientOrSecretHash, address(this)); + bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this)); _addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey); vm.startPrank(_caller); - bytes32 entryKeyPortalChecksAgainst = - _createUniswapSwapMessage(_isPrivateFlow, aztecRecipientOrSecretHash, _caller); + bytes32 entryKeyPortalChecksAgainst = _createUniswapSwapMessagePublic(aztecRecipient, _caller); vm.expectRevert( abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst) ); - uniswapPortal.swap( + uniswapPortal.swapPublic( address(daiTokenPortal), amount, uniswapFeePool, address(wethTokenPortal), amountOutMinimum, - aztecRecipientOrSecretHash, - secretHashForL1ToL2Message, + aztecRecipient, + secretHash, deadlineForL1ToL2Message, address(this), - true, - _isPrivateFlow + true ); - entryKeyPortalChecksAgainst = - _createUniswapSwapMessage(_isPrivateFlow, aztecRecipientOrSecretHash, address(0)); + entryKeyPortalChecksAgainst = _createUniswapSwapMessagePublic(aztecRecipient, address(0)); vm.expectRevert( abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst) ); - uniswapPortal.swap( + uniswapPortal.swapPublic( address(daiTokenPortal), amount, uniswapFeePool, address(wethTokenPortal), amountOutMinimum, - aztecRecipientOrSecretHash, - secretHashForL1ToL2Message, + aztecRecipient, + secretHash, deadlineForL1ToL2Message, address(this), - false, - _isPrivateFlow + false ); vm.stopPrank(); } // after the portal does the swap, it adds a L1 to L2 message to the inbox. - // to mint `outputToken` to the `aztecRecipientOrSecretHash` on L2. This test checks that + // to mint `outputToken` to the `aztecRecipient` on L2. This test checks that // if the sequencer doesn't consume the L1->L2 message, then `canceller` can // cancel the message and retrieve the funds (instead of them being stuck on the portal) function testMessageToInboxIsCancellable() public { bytes32 daiWithdrawMsgKey = _createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal)); - bytes32 swapMsgKey = _createUniswapSwapMessage(false, aztecRecipientOrSecretHash, address(this)); + bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this)); _addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey); - bytes32 l1ToL2MessageKey = uniswapPortal.swap{value: 1 ether}( + bytes32 l1ToL2MessageKey = uniswapPortal.swapPublic{value: 1 ether}( address(daiTokenPortal), amount, uniswapFeePool, address(wethTokenPortal), amountOutMinimum, - aztecRecipientOrSecretHash, - secretHashForL1ToL2Message, + aztecRecipient, + secretHash, deadlineForL1ToL2Message, address(this), // this address should be able to cancel - true, - false + true ); uint256 wethAmountOut = WETH9.balanceOf(address(wethTokenPortal)); @@ -408,11 +379,7 @@ contract UniswapPortalTest is Test { // perform op // TODO(2167) - Update UniswapPortal properly with new portal standard. bytes32 entryKey = wethTokenPortal.cancelL1ToAztecMessagePublic( - aztecRecipientOrSecretHash, - wethAmountOut, - deadlineForL1ToL2Message, - secretHashForL1ToL2Message, - 1 ether + wethAmountOut, aztecRecipient, deadlineForL1ToL2Message, secretHash, 1 ether ); assertEq(entryKey, l1ToL2MessageKey, "returned entry key and calculated entryKey should match"); assertFalse(inbox.contains(entryKey), "entry still in inbox"); @@ -424,6 +391,33 @@ contract UniswapPortalTest is Test { assertEq(WETH9.balanceOf(address(wethTokenPortal)), 0, "portal should have no assets"); } + function testRevertIfSwapMessageWasForDifferentPublicOrPrivateFlow() public { + bytes32 daiWithdrawMsgKey = + _createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal)); + + // Create message for `_isPrivateFlow`: + bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this)); + _addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey); + + bytes32 entryKeyPortalChecksAgainst = + _createUniswapSwapMessagePrivate(secretHashForRedeemingMintedNotes, address(this)); + vm.expectRevert( + abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst) + ); + + uniswapPortal.swapPrivate( + address(daiTokenPortal), + amount, + uniswapFeePool, + address(wethTokenPortal), + amountOutMinimum, + secretHashForRedeemingMintedNotes, + secretHash, + deadlineForL1ToL2Message, + address(this), + true + ); + } // TODO(#887) - what if uniswap fails? // TODO(#887) - what happens if uniswap deadline is passed? } 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 97f754f61df..1f6dea97901 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 @@ -278,10 +278,10 @@ describe('uniswap_trade_on_l1_from_l2', () => { logger('Sending messages to L1 portal'); const args = [ wethAmountToBridge, - deadline, - secretHashForMintingWeth.toString(true), secretHashForRedeemingWeth.toString(true), ownerEthAddress.toString(), + deadline, + secretHashForMintingWeth.toString(true), ] as const; const { result: messageKeyHex } = await wethTokenPortal.simulate.depositToAztecPrivate(args, { account: ownerEthAddress.toString(), @@ -369,7 +369,7 @@ describe('uniswap_trade_on_l1_from_l2', () => { const swapArgs = [ wethTokenPortalAddress.toString(), wethAmountToBridge, - 3000, + uniswapFeeTier, daiTokenPortalAddress.toString(), minimumOutputAmount, secretHashForRedeemingDai.toString(true), @@ -377,14 +377,13 @@ describe('uniswap_trade_on_l1_from_l2', () => { deadline, ownerEthAddress.toString(), true, - true, ] as const; - const { result: depositDaiMessageKeyHex } = await uniswapPortal.simulate.swap(swapArgs, { + const { result: depositDaiMessageKeyHex } = await uniswapPortal.simulate.swapPrivate(swapArgs, { account: ownerEthAddress.toString(), } as any); // this should also insert a message into the inbox. - await uniswapPortal.write.swap(swapArgs, {} as any); + await uniswapPortal.write.swapPrivate(swapArgs, {} as any); const depositDaiMessageKey = Fr.fromString(depositDaiMessageKeyHex); // weth was swapped to dai and send to portal 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 558a92797fa..ba14cf7c745 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 @@ -154,11 +154,11 @@ export class CrossChainTestHarness { this.logger('Sending messages to L1 portal to be consumed publicly'); const args = [ - this.ownerAddress.toString(), bridgeAmount, + this.ownerAddress.toString(), + this.ethAccount.toString(), deadline, secretHash.toString(true), - this.ethAccount.toString(), ] as const; const { result: messageKeyHex } = await this.tokenPortal.simulate.depositToAztecPublic(args, { account: this.ethAccount.toString(), @@ -181,10 +181,10 @@ export class CrossChainTestHarness { this.logger('Sending messages to L1 portal to be consumed privately'); const args = [ bridgeAmount, - deadline, - secretHashForL2MessageConsumption.toString(true), secretHashForRedeemingMintedNotes.toString(true), this.ethAccount.toString(), + deadline, + secretHashForL2MessageConsumption.toString(true), ] as const; const { result: messageKeyHex } = await this.tokenPortal.simulate.depositToAztecPrivate(args, { account: this.ethAccount.toString(), diff --git a/yarn-project/end-to-end/src/integration_archiver_l1_to_l2.test.ts b/yarn-project/end-to-end/src/integration_archiver_l1_to_l2.test.ts index 2c021dba910..867acabc95b 100644 --- a/yarn-project/end-to-end/src/integration_archiver_l1_to_l2.test.ts +++ b/yarn-project/end-to-end/src/integration_archiver_l1_to_l2.test.ts @@ -94,7 +94,7 @@ describe('archiver integration with l1 to l2 messages', () => { const mintAmount = 100n; logger('Sending messages to L1 portal'); - const args = [owner.toString(), mintAmount, deadline, secretString, ethAccount.toString()] as const; + const args = [mintAmount, owner.toString(), ethAccount.toString(), deadline, secretString] as const; await tokenPortal.write.depositToAztecPublic(args, {} as any); expect(await underlyingERC20.read.balanceOf([ethAccount.toString()])).toBe(1000000n - mintAmount); @@ -106,7 +106,7 @@ describe('archiver integration with l1 to l2 messages', () => { // cancel the message logger('cancelling the l1 to l2 message'); - const argsCancel = [owner.toString(), 100n, deadline, secretString, 0n] as const; + const argsCancel = [100n, owner.toString(), deadline, secretString, 0n] as const; await tokenPortal.write.cancelL1ToAztecMessagePublic(argsCancel, { gas: 1_000_000n } as any); expect(await underlyingERC20.read.balanceOf([ethAccount.toString()])).toBe(1000000n); // let archiver sync up 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 19def9e72bd..4f25b6bbbc9 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 @@ -230,14 +230,13 @@ describe('uniswap_trade_on_l1_from_l2', () => { deadlineForDepositingSwappedDai, ownerEthAddress.toString(), true, - true, ] as const; - const { result: depositDaiMessageKeyHex } = await uniswapPortal.simulate.swap(swapArgs, { + const { result: depositDaiMessageKeyHex } = await uniswapPortal.simulate.swapPrivate(swapArgs, { account: ownerEthAddress.toString(), } as any); // this should also insert a message into the inbox. - await uniswapPortal.write.swap(swapArgs, {} as any); + await uniswapPortal.write.swapPrivate(swapArgs, {} as any); const depositDaiMessageKey = Fr.fromString(depositDaiMessageKeyHex); // weth was swapped to dai and send to portal