From 9d316f28e83aca13cbe1ab869d8b428e7d9accf7 Mon Sep 17 00:00:00 2001 From: byshape Date: Fri, 16 Aug 2024 15:04:56 +0100 Subject: [PATCH] Add access token modifier to public withdraw or cancel functions --- .gas-snapshot | 105 ++++++++++++----------- contracts/BaseEscrow.sol | 11 ++- contracts/EscrowDst.sol | 16 +--- contracts/EscrowFactory.sol | 4 +- contracts/EscrowSrc.sol | 4 +- contracts/zkSync/EscrowDstZkSync.sol | 4 +- contracts/zkSync/EscrowFactoryZkSync.sol | 4 +- contracts/zkSync/EscrowSrcZkSync.sol | 4 +- test/libraries/TimelocksLib.t.sol | 1 + test/unit/Escrow.t.sol | 60 +++++++++++-- test/utils/BaseSetup.sol | 17 ++-- 11 files changed, 143 insertions(+), 87 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index ae0be3b..2990c45 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,76 +1,77 @@ EscrowFactoryTest:test_MultipleFillsInvalidKey() (gas: 474076) EscrowFactoryTest:test_MultipleFillsInvalidSecretsAmount() (gas: 473718) -EscrowFactoryTest:test_NoDeploymentForNotResolver() (gas: 190780) -EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMaker() (gas: 128460) +EscrowFactoryTest:test_NoDeploymentForNotResolver() (gas: 190767) +EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMaker() (gas: 128468) EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForTaker() (gas: 27118) -EscrowFactoryTest:test_NoInsufficientBalanceNativeDeploymentForMaker() (gas: 121696) +EscrowFactoryTest:test_NoInsufficientBalanceNativeDeploymentForMaker() (gas: 121693) EscrowFactoryTest:test_NoInsufficientBalanceNativeDeploymentForTaker() (gas: 27377) EscrowFactoryTest:test_NoUnsafeDeploymentForTaker() (gas: 34495) -EscrowTest:test_CancelDst() (gas: 116126) -EscrowTest:test_CancelDstDifferentTarget() (gas: 143384) -EscrowTest:test_CancelDstWithNativeToken() (gas: 93710) -EscrowTest:test_CancelPublicSrc() (gas: 165464) +EscrowTest:test_CancelDst() (gas: 116076) +EscrowTest:test_CancelDstDifferentTarget() (gas: 143334) +EscrowTest:test_CancelDstWithNativeToken() (gas: 93670) +EscrowTest:test_CancelPublicSrc() (gas: 170885) EscrowTest:test_CancelResolverSrc() (gas: 168607) EscrowTest:test_CancelResolverSrcReceiver() (gas: 179361) -EscrowTest:test_NoAnyoneCancelDuringResolverCancelSrc() (gas: 163856) -EscrowTest:test_NoCallsWithInvalidImmutables() (gas: 286518) +EscrowTest:test_NoAnyoneCancelDuringResolverCancelSrc() (gas: 163836) +EscrowTest:test_NoCallsWithInvalidImmutables() (gas: 286516) EscrowTest:test_NoCancelByAnyoneDst() (gas: 121726) EscrowTest:test_NoCancelDuringWithdrawalDst() (gas: 121486) EscrowTest:test_NoCancelDuringWithdrawalSrc() (gas: 164001) -EscrowTest:test_NoFailedNativeTokenTransferCancelSrc() (gas: 179339) -EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDst() (gas: 154544) -EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 312329) -EscrowTest:test_NoPublicCancelDuringPrivateCancellationSrc() (gas: 163166) -EscrowTest:test_NoPublicWithdrawOutsideOfAllowedPeriodDst() (gas: 126910) -EscrowTest:test_NoPublicWithdrawalOutsideOfAllowedPeriodSrc() (gas: 169085) -EscrowTest:test_NoRescueFundsByAnyoneDst() (gas: 176370) -EscrowTest:test_NoRescueFundsByAnyoneSrc() (gas: 209064) -EscrowTest:test_NoRescueFundsEarlierDst() (gas: 175780) -EscrowTest:test_NoRescueFundsEarlierSrc() (gas: 209041) -EscrowTest:test_NoRevertFailedNativeTokenTransferWithdrawalDstNative() (gas: 91111) -EscrowTest:test_NoWithdrawalByAnyoneSrc() (gas: 160873) +EscrowTest:test_NoFailedNativeTokenTransferCancelSrc() (gas: 216919) +EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDst() (gas: 192056) +EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDstNative() (gas: 84157) +EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 349820) +EscrowTest:test_NoPublicCallsByAnyone() (gas: 287563) +EscrowTest:test_NoPublicCancelDuringPrivateCancellationSrc() (gas: 168592) +EscrowTest:test_NoPublicWithdrawOutsideOfAllowedPeriodDst() (gas: 137737) +EscrowTest:test_NoPublicWithdrawalOutsideOfAllowedPeriodSrc() (gas: 179920) +EscrowTest:test_NoRescueFundsByAnyoneDst() (gas: 176333) +EscrowTest:test_NoRescueFundsByAnyoneSrc() (gas: 209079) +EscrowTest:test_NoRescueFundsEarlierDst() (gas: 175721) +EscrowTest:test_NoRescueFundsEarlierSrc() (gas: 208959) +EscrowTest:test_NoWithdrawalByAnyoneSrc() (gas: 160853) EscrowTest:test_NoWithdrawalByNonResolverDst() (gas: 121439) EscrowTest:test_NoWithdrawalOutsideOfAllowedPeriodDst() (gas: 126326) EscrowTest:test_NoWithdrawalOutsideOfAllowedPeriodSrc() (gas: 169636) -EscrowTest:test_NoWithdrawalWithWrongSecretDst() (gas: 122776) -EscrowTest:test_NoWithdrawalWithWrongSecretSrc() (gas: 164478) -EscrowTest:test_PublicWithdrawSrc() (gas: 181745) -EscrowTest:test_RescueFundsDst() (gas: 158304) -EscrowTest:test_RescueFundsDstNative() (gas: 186735) -EscrowTest:test_RescueFundsSrc() (gas: 195445) +EscrowTest:test_NoWithdrawalWithWrongSecretDst() (gas: 122798) +EscrowTest:test_NoWithdrawalWithWrongSecretSrc() (gas: 164448) +EscrowTest:test_PublicWithdrawSrc() (gas: 187170) +EscrowTest:test_RescueFundsDst() (gas: 158237) +EscrowTest:test_RescueFundsDstNative() (gas: 186700) +EscrowTest:test_RescueFundsSrc() (gas: 195453) EscrowTest:test_RescueFundsSrcNative() (gas: 197742) -EscrowTest:test_WithdrawByAnyoneDst() (gas: 141326) -EscrowTest:test_WithdrawByResolverDst() (gas: 142389) -EscrowTest:test_WithdrawByResolverDstNative() (gas: 97831) -EscrowTest:test_WithdrawByResolverPublicDst() (gas: 141757) -EscrowTest:test_WithdrawSrc() (gas: 186505) -EscrowTest:test_WithdrawSrcTo() (gas: 191335) +EscrowTest:test_WithdrawByAnyoneDst() (gas: 146701) +EscrowTest:test_WithdrawByResolverDst() (gas: 142374) +EscrowTest:test_WithdrawByResolverDstNative() (gas: 97859) +EscrowTest:test_WithdrawByResolverPublicDst() (gas: 141720) +EscrowTest:test_WithdrawSrc() (gas: 186522) +EscrowTest:test_WithdrawSrcTo() (gas: 191352) IntegrationEscrowFactoryTest:test_DeployCloneForMakerNonWhitelistedResolverInt() (gas: 473542) IntegrationEscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMakerInt() (gas: 341264) -IntegrationEscrowFactoryTest:test_NoResolverReentrancy() (gas: 2065187) -IntegrationResolverMockTest:test_MockCancelDst() (gas: 157184) -IntegrationResolverMockTest:test_MockCancelSrc() (gas: 353957) +IntegrationEscrowFactoryTest:test_NoResolverReentrancy() (gas: 2065184) +IntegrationResolverMockTest:test_MockCancelDst() (gas: 157134) +IntegrationResolverMockTest:test_MockCancelSrc() (gas: 353972) IntegrationResolverMockTest:test_MockDeployDst() (gas: 151470) -IntegrationResolverMockTest:test_MockDeploySrc() (gas: 364921) -IntegrationResolverMockTest:test_MockPublicCancelSrc() (gas: 392459) -IntegrationResolverMockTest:test_MockPublicWithdrawDst() (gas: 164841) -IntegrationResolverMockTest:test_MockRescueFundsDst() (gas: 161103) +IntegrationResolverMockTest:test_MockDeploySrc() (gas: 365054) +IntegrationResolverMockTest:test_MockPublicCancelSrc() (gas: 397892) +IntegrationResolverMockTest:test_MockPublicWithdrawDst() (gas: 170216) +IntegrationResolverMockTest:test_MockRescueFundsDst() (gas: 161053) IntegrationResolverMockTest:test_MockRescueFundsSrc() (gas: 382583) -IntegrationResolverMockTest:test_MockWithdrawDst() (gas: 182912) +IntegrationResolverMockTest:test_MockWithdrawDst() (gas: 182875) IntegrationResolverMockTest:test_MockWithdrawToSrc() (gas: 354839) -MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllExtra() (gas: 923576) -MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllFromLast() (gas: 922329) -MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllTwoFills() (gas: 922136) -MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirst() (gas: 707638) -MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirstTwoFills() (gas: 932969) -MerkleStorageInvalidatorIntTest:test_MultipleFillsFillLast() (gas: 707297) -MerkleStorageInvalidatorIntTest:test_MultipleFillsNoDeploymentWithoutValidation() (gas: 301343) +MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllExtra() (gas: 923554) +MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllFromLast() (gas: 922290) +MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllTwoFills() (gas: 922148) +MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirst() (gas: 707614) +MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirstTwoFills() (gas: 932982) +MerkleStorageInvalidatorIntTest:test_MultipleFillsFillLast() (gas: 707218) +MerkleStorageInvalidatorIntTest:test_MultipleFillsNoDeploymentWithoutValidation() (gas: 301456) MerkleStorageInvalidatorIntTest:test_MultipleFillsNoReuseOfSecrets() (gas: 1069228) MerkleStorageInvalidatorIntTest:test_MultipleFillsNoSecondDeploymentWithTheSameIndex() (gas: 786011) MerkleStorageInvalidatorIntTest:test_MultipleFillsOddDivision() (gas: 444783) -MerkleStorageInvalidatorIntTest:test_MultipleFillsOneFill() (gas: 707674) -MerkleStorageInvalidatorIntTest:test_MultipleFillsTwoFills() (gas: 921440) -MerkleStorageInvalidatorTest:test_ShortenedProofIsNotValid() (gas: 230605) -TimelocksLibTest:test_NoTimelocksOverflow() (gas: 156200) +MerkleStorageInvalidatorIntTest:test_MultipleFillsOneFill() (gas: 707645) +MerkleStorageInvalidatorIntTest:test_MultipleFillsTwoFills() (gas: 921433) +MerkleStorageInvalidatorTest:test_ShortenedProofIsNotValid() (gas: 230552) +TimelocksLibTest:test_NoTimelocksOverflow() (gas: 193707) TimelocksLibTest:test_getStartTimestamps() (gas: 16207) TimelocksLibTest:test_setDeployedAt() (gas: 5741) \ No newline at end of file diff --git a/contracts/BaseEscrow.sol b/contracts/BaseEscrow.sol index 6333f6b..f94bf3c 100644 --- a/contracts/BaseEscrow.sol +++ b/contracts/BaseEscrow.sol @@ -22,13 +22,17 @@ abstract contract BaseEscrow is IBaseEscrow { using TimelocksLib for Timelocks; using ImmutablesLib for Immutables; + // Token that is used to access public withdraw or cancel functions. + IERC20 private immutable _ACCESS_TOKEN; + /// @notice See {IBaseEscrow-RESCUE_DELAY}. uint256 public immutable RESCUE_DELAY; /// @notice See {IBaseEscrow-FACTORY}. address public immutable FACTORY = msg.sender; - constructor(uint32 rescueDelay) { + constructor(uint32 rescueDelay, IERC20 accessToken) { RESCUE_DELAY = rescueDelay; + _ACCESS_TOKEN = accessToken; } modifier onlyTaker(Immutables calldata immutables) { @@ -56,6 +60,11 @@ abstract contract BaseEscrow is IBaseEscrow { _; } + modifier onlyAccessTokenHolder() { + if(_ACCESS_TOKEN.balanceOf(msg.sender) == 0) revert InvalidCaller(); + _; + } + /** * @notice See {IBaseEscrow-rescueFunds}. */ diff --git a/contracts/EscrowDst.sol b/contracts/EscrowDst.sol index 6495033..5bfab19 100644 --- a/contracts/EscrowDst.sol +++ b/contracts/EscrowDst.sol @@ -24,7 +24,7 @@ contract EscrowDst is Escrow, IEscrowDst { using AddressLib for Address; using TimelocksLib for Timelocks; - constructor(uint32 rescueDelay) BaseEscrow(rescueDelay) {} + constructor(uint32 rescueDelay, IERC20 accessToken) BaseEscrow(rescueDelay, accessToken) {} /** * @notice See {IBaseEscrow-withdraw}. @@ -47,6 +47,7 @@ contract EscrowDst is Escrow, IEscrowDst { */ function publicWithdraw(bytes32 secret, Immutables calldata immutables) external + onlyAccessTokenHolder() onlyAfter(immutables.timelocks.get(TimelocksLib.Stage.DstPublicWithdrawal)) onlyBefore(immutables.timelocks.get(TimelocksLib.Stage.DstCancellation)) { @@ -78,18 +79,7 @@ contract EscrowDst is Escrow, IEscrowDst { onlyValidImmutables(immutables) onlyValidSecret(secret, immutables) { - address token = immutables.token.get(); - address to = immutables.maker.get(); - if (token == address(0)) { - /** - * @dev The result of the call is not checked intentionally. This is done to ensure that - * even in case of malicious receiver the withdrawal flow can not be blocked and takers - * will be able to get their safety deposit back. - **/ - to.call{ value: immutables.amount }(""); - } else { - IERC20(token).safeTransfer(to, immutables.amount); - } + _uniTransfer(immutables.token.get(), immutables.maker.get(), immutables.amount); _ethTransfer(msg.sender, immutables.safetyDeposit); emit EscrowWithdrawal(secret); } diff --git a/contracts/EscrowFactory.sol b/contracts/EscrowFactory.sol index 7cd04ce..5dc62d3 100644 --- a/contracts/EscrowFactory.sol +++ b/contracts/EscrowFactory.sol @@ -32,8 +32,8 @@ contract EscrowFactory is BaseEscrowFactory { BaseExtension(limitOrderProtocol) ResolverValidationExtension(feeToken, accessToken, owner) MerkleStorageInvalidator(limitOrderProtocol) { - ESCROW_SRC_IMPLEMENTATION = address(new EscrowSrc(rescueDelaySrc)); - ESCROW_DST_IMPLEMENTATION = address(new EscrowDst(rescueDelayDst)); + ESCROW_SRC_IMPLEMENTATION = address(new EscrowSrc(rescueDelaySrc, accessToken)); + ESCROW_DST_IMPLEMENTATION = address(new EscrowDst(rescueDelayDst, accessToken)); _PROXY_SRC_BYTECODE_HASH = ProxyHashLib.computeProxyBytecodeHash(ESCROW_SRC_IMPLEMENTATION); _PROXY_DST_BYTECODE_HASH = ProxyHashLib.computeProxyBytecodeHash(ESCROW_DST_IMPLEMENTATION); } diff --git a/contracts/EscrowSrc.sol b/contracts/EscrowSrc.sol index 32e6778..5bf0d8a 100644 --- a/contracts/EscrowSrc.sol +++ b/contracts/EscrowSrc.sol @@ -27,7 +27,7 @@ contract EscrowSrc is Escrow, IEscrowSrc { using SafeERC20 for IERC20; using TimelocksLib for Timelocks; - constructor(uint32 rescueDelay) BaseEscrow(rescueDelay) {} + constructor(uint32 rescueDelay, IERC20 accessToken) BaseEscrow(rescueDelay, accessToken) {} /** * @notice See {IBaseEscrow-withdraw}. @@ -67,6 +67,7 @@ contract EscrowSrc is Escrow, IEscrowSrc { */ function publicWithdraw(bytes32 secret, Immutables calldata immutables) external + onlyAccessTokenHolder() onlyAfter(immutables.timelocks.get(TimelocksLib.Stage.SrcPublicWithdrawal)) onlyBefore(immutables.timelocks.get(TimelocksLib.Stage.SrcCancellation)) { @@ -95,6 +96,7 @@ contract EscrowSrc is Escrow, IEscrowSrc { */ function publicCancel(Immutables calldata immutables) external + onlyAccessTokenHolder() onlyAfter(immutables.timelocks.get(TimelocksLib.Stage.SrcPublicCancellation)) { _cancel(immutables); diff --git a/contracts/zkSync/EscrowDstZkSync.sol b/contracts/zkSync/EscrowDstZkSync.sol index c075be2..dc87d81 100644 --- a/contracts/zkSync/EscrowDstZkSync.sol +++ b/contracts/zkSync/EscrowDstZkSync.sol @@ -2,12 +2,14 @@ pragma solidity 0.8.23; +import { IERC20 } from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; + import { Escrow, EscrowDst } from "../EscrowDst.sol"; import { EscrowZkSync } from "./EscrowZkSync.sol"; /// @custom:security-contact security@1inch.io contract EscrowDstZkSync is EscrowDst, EscrowZkSync { - constructor(uint32 rescueDelay) EscrowDst(rescueDelay) EscrowZkSync() {} + constructor(uint32 rescueDelay, IERC20 accessToken) EscrowDst(rescueDelay, accessToken) EscrowZkSync() {} function _validateImmutables(Immutables calldata immutables) internal view override(Escrow, EscrowZkSync) { EscrowZkSync._validateImmutables(immutables); diff --git a/contracts/zkSync/EscrowFactoryZkSync.sol b/contracts/zkSync/EscrowFactoryZkSync.sol index 5bb7fca..2c4881f 100644 --- a/contracts/zkSync/EscrowFactoryZkSync.sol +++ b/contracts/zkSync/EscrowFactoryZkSync.sol @@ -38,8 +38,8 @@ contract EscrowFactoryZkSync is BaseEscrowFactory { BaseExtension(limitOrderProtocol) ResolverValidationExtension(feeToken, accessToken, owner) MerkleStorageInvalidator(limitOrderProtocol) { - ESCROW_SRC_IMPLEMENTATION = address(new EscrowSrcZkSync(rescueDelaySrc)); - ESCROW_DST_IMPLEMENTATION = address(new EscrowDstZkSync(rescueDelayDst)); + ESCROW_SRC_IMPLEMENTATION = address(new EscrowSrcZkSync(rescueDelaySrc, accessToken)); + ESCROW_DST_IMPLEMENTATION = address(new EscrowDstZkSync(rescueDelayDst, accessToken)); ESCROW_SRC_INPUT_HASH = keccak256(abi.encode(ESCROW_SRC_IMPLEMENTATION)); ESCROW_DST_INPUT_HASH = keccak256(abi.encode(ESCROW_DST_IMPLEMENTATION)); MinimalProxyZkSync proxySrc = new MinimalProxyZkSync(ESCROW_SRC_IMPLEMENTATION); diff --git a/contracts/zkSync/EscrowSrcZkSync.sol b/contracts/zkSync/EscrowSrcZkSync.sol index 1723679..bddf4b2 100644 --- a/contracts/zkSync/EscrowSrcZkSync.sol +++ b/contracts/zkSync/EscrowSrcZkSync.sol @@ -2,12 +2,14 @@ pragma solidity 0.8.23; +import { IERC20 } from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; + import { Escrow, EscrowSrc } from "../EscrowSrc.sol"; import { EscrowZkSync } from "./EscrowZkSync.sol"; /// @custom:security-contact security@1inch.io contract EscrowSrcZkSync is EscrowSrc, EscrowZkSync { - constructor(uint32 rescueDelay) EscrowSrc(rescueDelay) EscrowZkSync() {} + constructor(uint32 rescueDelay, IERC20 accessToken) EscrowSrc(rescueDelay, accessToken) EscrowZkSync() {} function _validateImmutables(Immutables calldata immutables) internal view override(Escrow, EscrowZkSync) { EscrowZkSync._validateImmutables(immutables); diff --git a/test/libraries/TimelocksLib.t.sol b/test/libraries/TimelocksLib.t.sol index 592cc99..7ddb1c3 100644 --- a/test/libraries/TimelocksLib.t.sol +++ b/test/libraries/TimelocksLib.t.sol @@ -70,6 +70,7 @@ contract TimelocksLibTest is BaseSetup { // withdraw vm.warp(block.timestamp + dstTimelocks.publicWithdrawal); uint256 balanceAlice = dai.balanceOf(alice.addr); + inch.mint(alice.addr, 1); vm.startPrank(alice.addr); dstClone.publicWithdraw(SECRET, immutablesDst); assertEq(dai.balanceOf(address(dstClone)), 0); diff --git a/test/unit/Escrow.t.sol b/test/unit/Escrow.t.sol index 3acf958..f04edd7 100644 --- a/test/unit/Escrow.t.sol +++ b/test/unit/Escrow.t.sol @@ -18,6 +18,7 @@ contract EscrowTest is BaseSetup { function setUp() public virtual override { BaseSetup.setUp(); + inch.mint(address(this), 1); } /* solhint-disable func-name-mixedcase */ @@ -715,6 +716,7 @@ contract EscrowTest is BaseSetup { // withdraw vm.warp(block.timestamp + srcTimelocks.publicWithdrawal + 100); NoReceiveCaller caller = new NoReceiveCaller(); + inch.mint(address(caller), 1); bytes memory data = abi.encodeWithSelector(IEscrowSrc.publicWithdraw.selector, SECRET, swapData.immutables); vm.expectRevert(IBaseEscrow.NativeTokenSendingFailure.selector); caller.arbitraryCall(address(swapData.srcClone), data); @@ -729,12 +731,13 @@ contract EscrowTest is BaseSetup { // withdraw vm.warp(block.timestamp + dstTimelocks.publicWithdrawal + 10); + inch.mint(address(escrowFactory), 1); vm.prank(address(escrowFactory)); vm.expectRevert(IBaseEscrow.NativeTokenSendingFailure.selector); dstClone.publicWithdraw(SECRET, immutables); } - function test_NoRevertFailedNativeTokenTransferWithdrawalDstNative() public { + function test_NoFailedNativeTokenTransferWithdrawalDstNative() public { (IBaseEscrow.Immutables memory immutables, uint256 srcCancellationTimestamp, IBaseEscrow dstClone) = _prepareDataDstCustom( HASHED_SECRET, TAKING_AMOUNT, address(escrowFactory), bob.addr, address(0x00), DST_SAFETY_DEPOSIT ); @@ -744,14 +747,14 @@ contract EscrowTest is BaseSetup { escrowFactory.createDstEscrow{ value: DST_SAFETY_DEPOSIT + TAKING_AMOUNT }(immutables, srcCancellationTimestamp); uint256 balanceBob = bob.addr.balance; - uint256 balanceFactory = address(escrowFactory).balance; + uint256 balanceEscrow = address(dstClone).balance; // withdraw vm.warp(block.timestamp + dstTimelocks.withdrawal + 10); + vm.expectRevert(IBaseEscrow.NativeTokenSendingFailure.selector); dstClone.withdraw(SECRET, immutables); - assertEq(bob.addr.balance, balanceBob + DST_SAFETY_DEPOSIT); - assertEq(address(dstClone).balance, TAKING_AMOUNT); - assertEq(address(escrowFactory).balance, balanceFactory); + assertEq(bob.addr.balance, balanceBob); + assertEq(address(dstClone).balance, balanceEscrow); } function test_NoPublicWithdrawOutsideOfAllowedPeriodDst() public { @@ -1108,6 +1111,7 @@ contract EscrowTest is BaseSetup { // cancel vm.warp(block.timestamp + srcTimelocks.publicCancellation + 100); + inch.mint(address(escrowFactory), 1); vm.prank(address(escrowFactory)); vm.expectRevert(IBaseEscrow.NativeTokenSendingFailure.selector); swapData.srcClone.publicCancel(swapData.immutables); @@ -1172,5 +1176,51 @@ contract EscrowTest is BaseSetup { dstClone.rescueFunds(address(dai), DST_SAFETY_DEPOSIT, immutablesDst); } + function test_NoPublicCallsByAnyone() public { + CrossChainTestLib.SwapData memory swapData = _prepareDataSrc(true, false); + (IBaseEscrow.Immutables memory immutables, uint256 srcCancellationTimestamp, IBaseEscrow dstClone) = _prepareDataDst(); + + (bool success,) = address(swapData.srcClone).call{ value: SRC_SAFETY_DEPOSIT }(""); + assertEq(success, true); + usdc.transfer(address(swapData.srcClone), MAKING_AMOUNT); + + // deploy escrow, source chain + vm.prank(address(limitOrderProtocol)); + escrowFactory.postInteraction( + swapData.order, + "", // extension + swapData.orderHash, + bob.addr, // taker + MAKING_AMOUNT, + TAKING_AMOUNT, + 0, // remainingMakingAmount + swapData.extraData + ); + + // deploy escrow, destination chain + vm.prank(bob.addr); + escrowFactory.createDstEscrow{ value: DST_SAFETY_DEPOSIT }(immutables, srcCancellationTimestamp); + + //--- Source chain ---// + // public withdraw + vm.warp(block.timestamp + srcTimelocks.publicWithdrawal + 100); + vm.prank(alice.addr); + vm.expectRevert(IBaseEscrow.InvalidCaller.selector); + swapData.srcClone.publicWithdraw(SECRET, swapData.immutables); + + // public cancel + vm.warp(block.timestamp + srcTimelocks.publicCancellation + 100); + vm.prank(alice.addr); + vm.expectRevert(IBaseEscrow.InvalidCaller.selector); + swapData.srcClone.publicCancel(swapData.immutables); + + //--- Destination chain ---// + // public withdraw + vm.warp(block.timestamp + dstTimelocks.publicWithdrawal + 100); + vm.prank(alice.addr); + vm.expectRevert(IBaseEscrow.InvalidCaller.selector); + IEscrowDst(address(dstClone)).publicWithdraw(SECRET, immutables); + } + /* solhint-enable func-name-mixedcase */ } diff --git a/test/utils/BaseSetup.sol b/test/utils/BaseSetup.sol index 434e844..ec56781 100644 --- a/test/utils/BaseSetup.sol +++ b/test/utils/BaseSetup.sol @@ -4,7 +4,6 @@ pragma solidity 0.8.23; import { Test } from "forge-std/Test.sol"; import { IWETH, LimitOrderProtocol } from "limit-order-protocol/contracts/LimitOrderProtocol.sol"; -import { WrappedTokenMock } from "limit-order-protocol/contracts/mocks/WrappedTokenMock.sol"; import { IFeeBank } from "limit-order-settlement/contracts/interfaces/IFeeBank.sol"; import { TokenCustomDecimalsMock } from "solidity-utils/contracts/mocks/TokenCustomDecimalsMock.sol"; import { TokenMock } from "solidity-utils/contracts/mocks/TokenMock.sol"; @@ -38,7 +37,6 @@ contract BaseSetup is Test, Utils { TokenMock internal dai; TokenCustomDecimalsMock internal usdc; - WrappedTokenMock internal weth; TokenMock internal inch; LimitOrderProtocol internal limitOrderProtocol; @@ -113,28 +111,29 @@ contract BaseSetup is Test, Utils { vm.label(address(dai), "DAI"); usdc = new TokenCustomDecimalsMock("USDC", "USDC", 1000 ether, 6); vm.label(address(usdc), "USDC"); - weth = new WrappedTokenMock("WETH", "WETH"); - vm.label(address(weth), "WETH"); inch = new TokenMock("1INCH", "1INCH"); vm.label(address(inch), "1INCH"); } function _deployContracts() internal { - limitOrderProtocol = new LimitOrderProtocol(IWETH(weth)); + limitOrderProtocol = new LimitOrderProtocol(IWETH(address(0))); if (isZkSync) { escrowFactory = new EscrowFactoryZkSync(address(limitOrderProtocol), inch, inch, charlie.addr, RESCUE_DELAY, RESCUE_DELAY); } else { escrowFactory = new EscrowFactory(address(limitOrderProtocol), inch, inch, charlie.addr, RESCUE_DELAY, RESCUE_DELAY); } - vm.label(address(escrowFactory), "EscrowFactory"); escrowSrc = EscrowSrc(escrowFactory.ESCROW_SRC_IMPLEMENTATION()); - vm.label(address(escrowSrc), "EscrowSrc"); escrowDst = EscrowDst(escrowFactory.ESCROW_DST_IMPLEMENTATION()); - vm.label(address(escrowDst), "EscrowDst"); feeBank = IFeeBank(escrowFactory.FEE_BANK()); - vm.label(address(feeBank), "FeeBank"); + + if (!isZkSync) { + vm.label(address(escrowFactory), "EscrowFactory"); + vm.label(address(escrowSrc), "EscrowSrc"); + vm.label(address(escrowDst), "EscrowDst"); + vm.label(address(feeBank), "FeeBank"); + } } function _prepareDataSrc(bool fakeOrder, bool allowMultipleFills) internal returns(CrossChainTestLib.SwapData memory) {