Skip to content

Commit

Permalink
address pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
rahul-kothari committed Jun 22, 2023
1 parent fea4db9 commit f758921
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 30 deletions.
4 changes: 2 additions & 2 deletions l1-contracts/test/portals/TokenPortal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ 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 - When true, it ensures only the caller can tell outbox to consume the message
* (otherwise anyone can call this method to release funds to the `_recipient`)
* @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, bool _withCaller)
Expand Down
33 changes: 13 additions & 20 deletions l1-contracts/test/portals/TokenPortal.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ contract TokenPortalTest is Test {
return entryKeys[0];
}

function testWithdrawWithoutDesignatedCaller() public {
function testAnyoneCanCallWithdrawIfNoDesignatedCaller(address _caller) public {
vm.assume(_caller != address(0));
bytes32 expectedEntryKey = _addWithdrawMessageInOutbox(address(0));
assertEq(portalERC20.balanceOf(recipient), 0);

Expand All @@ -208,24 +209,13 @@ contract TokenPortalTest is Test {
tokenPortal.withdraw(withdrawAmount, recipient, false);
}

function testAnyoneCanCallWithdrawIfNoDesignatedCaller() public {
bytes32 expectedEntryKey = _addWithdrawMessageInOutbox(address(0));
assertEq(portalERC20.balanceOf(recipient), 0);

vm.prank(address(0x1));
vm.expectEmit(true, true, true, true);
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);
}

function testWithdrawWithDesignatedCaller() public {
bytes32 expectedEntryKey = _addWithdrawMessageInOutbox(address(this));
function testWithdrawWithDesignatedCallerFailsForOtherCallers(address _caller) public {
vm.assume(_caller != address(this));
// add message with caller as this address
_addWithdrawMessageInOutbox(address(this));

vm.startPrank(address(0x1));
bytes32 entryKeyPortalChecksAgainst = _createWithdrawMessageForOutbox(address(0x1));
vm.startPrank(_caller);
bytes32 entryKeyPortalChecksAgainst = _createWithdrawMessageForOutbox(_caller);
vm.expectRevert(
abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst)
);
Expand All @@ -236,9 +226,12 @@ contract TokenPortalTest is Test {
abi.encodeWithSelector(Errors.Outbox__NothingToConsume.selector, entryKeyPortalChecksAgainst)
);
tokenPortal.withdraw(withdrawAmount, recipient, false);
}

function testWithdrawWithDesignatedCallerSucceedsForDesignatedCaller() public {
// add message with caller as this address
bytes32 expectedEntryKey = _addWithdrawMessageInOutbox(address(this));

// okay, let's consume this for real:
vm.stopPrank();
vm.expectEmit(true, true, true, true);
emit MessageConsumed(expectedEntryKey, address(tokenPortal));
bytes32 actualEntryKey = tokenPortal.withdraw(withdrawAmount, recipient, true);
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/end-to-end/src/cross_chain/test_harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ 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);
// 0xb460af94, selector for "withdraw(uint256,address,address)"
Expand All @@ -167,7 +167,7 @@ export class CrossChainTestHarness {
Buffer.from([0xb4, 0x60, 0xaf, 0x94]),
toBufferBE(withdrawAmount, 32),
this.ethAccount.toBuffer32(),
EthAddress.ZERO.toBuffer32(),
callerOnL1.toBuffer32(),
]),
);
const entryKey = sha256ToField(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -49,7 +49,7 @@ contract Uniswap {
uniswapFeeTier,
outputAsset,
outputAssetPortalAddress,
mminimumOutputAmount,
minimumOutputAmount,
sender.x,
sender.y,
recipient,
Expand All @@ -59,8 +59,8 @@ contract Uniswap {
l1UniswapPortal,
]);

// inputAsset.withdraw(inputAmount, recipient=l1UniswapPortal, callerOnL1=l1UniswapPortal) // only uniswap portal can call this to safegaurd ordering of message consumption.
// if someone else calls it on behalf of uniswap portal and the portal doesn't expect it, then funds might get locked!
// 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;
Expand All @@ -79,7 +79,7 @@ contract Uniswap {
inputAmount,
uniswapFeeTier,
outputAssetPortalAddress,
mminimumOutputAmount,
minimumOutputAmount,
recipient,
secretHash,
deadlineForL1ToL2Message,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@
"visibility": "public"
},
{
"name": "mminimumOutputAmount",
"name": "minimumOutputAmount",
"type": {
"kind": "field"
},
Expand Down

0 comments on commit f758921

Please sign in to comment.