From cefe8387caa78c00ce4599b7120d6566382cefe1 Mon Sep 17 00:00:00 2001 From: byshape Date: Tue, 30 Jan 2024 11:24:22 +0000 Subject: [PATCH] Added ability to withdraw native currency for the maker --- .gas-snapshot | 66 +++++++++++++------------ contracts/Escrow.sol | 9 +++- contracts/EscrowFactory.sol | 21 +++++--- contracts/interfaces/IEscrowFactory.sol | 2 +- test/unit/Escrow.t.sol | 63 ++++++++++++++++++----- test/unit/EscrowFactory.t.sol | 25 ++++++++-- 6 files changed, 129 insertions(+), 57 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 9db71ab..4ff575b 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,32 +1,36 @@ -EscrowFactoryTest:testFuzz_DeployCloneForMaker(bytes32,uint56,uint56) (runs: 256, μ: 234194, ~: 237909) -EscrowFactoryTest:testFuzz_DeployCloneForTaker(bytes32,uint56) (runs: 256, μ: 180412, ~: 183236) -EscrowFactoryTest:test_NoDeploymentForNotResolver() (gas: 103456) -EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMaker() (gas: 185356) -EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForTaker() (gas: 27715) -EscrowFactoryTest:test_NoInsufficientBalanceNativeDeploymentForMaker() (gas: 151006) -EscrowFactoryTest:test_NoUnsafeDeploymentForTaker() (gas: 34980) -EscrowTest:test_CancelDst() (gas: 165076) -EscrowTest:test_CancelPublicSrc() (gas: 229537) -EscrowTest:test_CancelResolverSrc() (gas: 230646) -EscrowTest:test_NoAnyoneCancelDuringResolverCancelSrc() (gas: 227896) -EscrowTest:test_NoCancelByAnyoneDst() (gas: 172822) -EscrowTest:test_NoCancelDuringPublicWithdrawalDst() (gas: 170625) -EscrowTest:test_NoCancelDuringResolverWithdrawalDst() (gas: 168351) -EscrowTest:test_NoCancelDuringWithdrawalSrc() (gas: 225333) -EscrowTest:test_NoFailedNativeTokenTransferCancelDst() (gas: 183501) -EscrowTest:test_NoFailedNativeTokenTransferCancelSrc() (gas: 245348) -EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDst() (gas: 203410) -EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 259494) -EscrowTest:test_NoWithdrawalByAnyone() (gas: 223044) -EscrowTest:test_NoWithdrawalByNonResolverDst() (gas: 168503) -EscrowTest:test_NoWithdrawalDuringFinalityLockDst() (gas: 165563) -EscrowTest:test_NoWithdrawalDuringFinalityLockSrc() (gas: 223873) -EscrowTest:test_NoWithdrawalWithWrongSecretDst() (gas: 169323) -EscrowTest:test_NoWithdrawalWithWrongSecretSrc() (gas: 226329) -EscrowTest:test_WithdrawByAnyoneDst() (gas: 187575) -EscrowTest:test_WithdrawByResolverDst() (gas: 186265) -EscrowTest:test_WithdrawByResolverPublicDst() (gas: 187938) -EscrowTest:test_WithdrawSrc() (gas: 245902) -IntegrationEscrowFactoryTest:testFuzz_DeployCloneForMakerInt(bytes32,uint56,uint56) (runs: 256, μ: 296280, ~: 299872) -IntegrationEscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMakerInt() (gas: 281514) +EscrowFactoryTest:testFuzz_DeployCloneForMaker(bytes32,uint56,uint56) (runs: 256, μ: 234241, ~: 237956) +EscrowFactoryTest:testFuzz_DeployCloneForTaker(bytes32,uint56) (runs: 256, μ: 180570, ~: 183216) +EscrowFactoryTest:test_NoDeploymentForNotResolver() (gas: 103525) +EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMaker() (gas: 185403) +EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForTaker() (gas: 27693) +EscrowFactoryTest:test_NoInsufficientBalanceNativeDeploymentForMaker() (gas: 151053) +EscrowFactoryTest:test_NoInsufficientBalanceNativeDeploymentForTaker() (gas: 32357) +EscrowFactoryTest:test_NoUnsafeDeploymentForTaker() (gas: 34955) +EscrowTest:test_CancelDst() (gas: 165089) +EscrowTest:test_CancelPublicSrc() (gas: 229612) +EscrowTest:test_CancelResolverSrc() (gas: 230726) +EscrowTest:test_NoAnyoneCancelDuringResolverCancelSrc() (gas: 227976) +EscrowTest:test_NoCancelByAnyoneDst() (gas: 172835) +EscrowTest:test_NoCancelDuringPublicWithdrawalDst() (gas: 170638) +EscrowTest:test_NoCancelDuringResolverWithdrawalDst() (gas: 168342) +EscrowTest:test_NoCancelDuringWithdrawalSrc() (gas: 225391) +EscrowTest:test_NoFailedNativeTokenTransferCancelDst() (gas: 183492) +EscrowTest:test_NoFailedNativeTokenTransferCancelSrc() (gas: 245406) +EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDst() (gas: 203418) +EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDstNative() (gas: 134412) +EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 259569) +EscrowTest:test_NoWithdrawalByAnyone() (gas: 223124) +EscrowTest:test_NoWithdrawalByNonResolverDst() (gas: 168494) +EscrowTest:test_NoWithdrawalDuringFinalityLockDst() (gas: 165554) +EscrowTest:test_NoWithdrawalDuringFinalityLockSrc() (gas: 223953) +EscrowTest:test_NoWithdrawalWithWrongSecretDst() (gas: 169358) +EscrowTest:test_NoWithdrawalWithWrongSecretSrc() (gas: 226387) +EscrowTest:test_WithdrawByAnyoneDst() (gas: 187578) +EscrowTest:test_WithdrawByResolverDst() (gas: 186317) +EscrowTest:test_WithdrawByResolverDstNative() (gas: 146388) +EscrowTest:test_WithdrawByResolverPublicDst() (gas: 187946) +EscrowTest:test_WithdrawByResolverPublicDstNative() (gas: 148068) +EscrowTest:test_WithdrawSrc() (gas: 245999) +IntegrationEscrowFactoryTest:testFuzz_DeployCloneForMakerInt(bytes32,uint56,uint56) (runs: 256, μ: 296657, ~: 300125) +IntegrationEscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMakerInt() (gas: 281711) TimelocksLibTest:test_getStartTimestamps() (gas: 15326) \ No newline at end of file diff --git a/contracts/Escrow.sol b/contracts/Escrow.sol index ce96fc8..d30ec03 100644 --- a/contracts/Escrow.sol +++ b/contracts/Escrow.sol @@ -16,7 +16,7 @@ import { IEscrow } from "./interfaces/IEscrow.sol"; * @notice Contract to initially lock funds on both chains and then unlock with verification of the secret presented. * @dev Funds are locked in at the time of contract deployment. On both chains this is done by calling `EscrowFactory` * functions. On the source chain Limit Order Protocol calls the `postInteraction` function and on the destination - * chain taker calls the `createEscrowDst` function. + * chain taker calls the `createDstEscrow` function. * Withdrawal and cancellation functions for the source and destination chains are implemented separately. */ contract Escrow is Clone, IEscrow { @@ -198,6 +198,11 @@ contract Escrow is Clone, IEscrow { uint256 amount ) internal { if (!_isValidSecret(secret, hashlock)) revert InvalidSecret(); - IERC20(token).safeTransfer(recipient, amount); + if (token == address(0)) { + (bool success, ) = recipient.call{value: amount}(""); + if (!success) revert NativeTokenSendingFailure(); + } else { + IERC20(token).safeTransfer(recipient, amount); + } } } diff --git a/contracts/EscrowFactory.sol b/contracts/EscrowFactory.sol index 057132f..bbb8325 100644 --- a/contracts/EscrowFactory.sol +++ b/contracts/EscrowFactory.sol @@ -92,10 +92,17 @@ contract EscrowFactory is IEscrowFactory, SimpleSettlementExtension { } /** - * @notice See {IEscrowFactory-createEscrowDst}. + * @notice See {IEscrowFactory-createDstEscrow}. */ - function createEscrowDst(DstEscrowImmutablesCreation calldata dstImmutables) external payable { - if (msg.value < dstImmutables.args.safetyDeposit) revert InsufficientEscrowBalance(); + function createDstEscrow(DstEscrowImmutablesCreation calldata dstImmutables) external payable { + uint256 nativeAmount = dstImmutables.args.safetyDeposit; + address token = dstImmutables.args.packedAddresses.token(); + // If the destination token is native, add its amount to the safety deposit. + if (token == address(0)) { + nativeAmount += dstImmutables.args.amount; + } + if (msg.value < nativeAmount) revert InsufficientEscrowBalance(); + // Check that the escrow cancellation will start not later than the cancellation time on the source chain. if ( dstImmutables.args.timelocks.dstCancellationStart(block.timestamp) > @@ -113,9 +120,11 @@ contract EscrowFactory is IEscrowFactory, SimpleSettlementExtension { } address escrow = _createEscrow(data, msg.value); - IERC20(dstImmutables.args.packedAddresses.token()).safeTransferFrom( - msg.sender, escrow, dstImmutables.args.amount - ); + if (token != address(0)) { + IERC20(dstImmutables.args.packedAddresses.token()).safeTransferFrom( + msg.sender, escrow, dstImmutables.args.amount + ); + } } /** diff --git a/contracts/interfaces/IEscrowFactory.sol b/contracts/interfaces/IEscrowFactory.sol index 1054e48..6deddd0 100644 --- a/contracts/interfaces/IEscrowFactory.sol +++ b/contracts/interfaces/IEscrowFactory.sol @@ -23,7 +23,7 @@ interface IEscrowFactory { * and approve the destination token to be transferred to the created escrow. * @param dstEscrowImmutables The immutables of the escrow contract that are used in deployment. */ - function createEscrowDst(DstEscrowImmutablesCreation calldata dstEscrowImmutables) external payable; + function createDstEscrow(DstEscrowImmutablesCreation calldata dstEscrowImmutables) external payable; /** * @notice Returns the deterministic address of the escrow based on the salt. diff --git a/test/unit/Escrow.t.sol b/test/unit/Escrow.t.sol index 6e9b06c..8b3d30e 100644 --- a/test/unit/Escrow.t.sol +++ b/test/unit/Escrow.t.sol @@ -87,7 +87,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.startPrank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); // withdraw vm.expectRevert(IEscrow.InvalidWithdrawalTime.selector); @@ -142,7 +142,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.startPrank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); uint256 balanceAlice = dai.balanceOf(alice.addr); uint256 balanceBob = bob.addr.balance; @@ -159,6 +159,29 @@ contract EscrowTest is BaseSetup { assertEq(address(dstClone).balance, balanceEscrowNative - DST_SAFETY_DEPOSIT); } + function test_WithdrawByResolverDstNative() public { + ( + IEscrowFactory.DstEscrowImmutablesCreation memory immutables, + Escrow dstClone + ) = _prepareDataDst(SECRET, TAKING_AMOUNT, alice.addr, bob.addr, address(0x00)); + + // deploy escrow + vm.startPrank(bob.addr); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT + TAKING_AMOUNT}(immutables); + + uint256 balanceAlice = alice.addr.balance; + uint256 balanceBob = bob.addr.balance; + uint256 balanceEscrow = address(dstClone).balance; + + // withdraw + vm.warp(block.timestamp + dstTimelocks.finality + 10); + dstClone.withdrawDst(SECRET); + + assertEq(alice.addr.balance, balanceAlice + TAKING_AMOUNT); + assertEq(bob.addr.balance, balanceBob + DST_SAFETY_DEPOSIT); + assertEq(address(dstClone).balance, balanceEscrow - DST_SAFETY_DEPOSIT - TAKING_AMOUNT); + } + function test_NoWithdrawalWithWrongSecretSrc() public { // deploy escrow ( @@ -200,7 +223,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.startPrank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); // withdraw vm.warp(block.timestamp + dstTimelocks.finality + 100); @@ -217,7 +240,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.prank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); // withdraw vm.warp(block.timestamp + dstTimelocks.finality + 100); @@ -234,7 +257,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.prank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); uint256 balanceAlice = dai.balanceOf(alice.addr); uint256 balanceThis = address(this).balance; @@ -260,7 +283,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.startPrank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); uint256 balanceAlice = dai.balanceOf(alice.addr); uint256 balanceBob = bob.addr.balance; @@ -319,7 +342,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.prank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); // withdraw vm.warp(block.timestamp + dstTimelocks.finality + dstTimelocks.withdrawal + 10); @@ -328,6 +351,22 @@ contract EscrowTest is BaseSetup { dstClone.withdrawDst(SECRET); } + function test_NoFailedNativeTokenTransferWithdrawalDstNative() public { + ( + IEscrowFactory.DstEscrowImmutablesCreation memory immutables, + Escrow dstClone + ) = _prepareDataDst(SECRET, TAKING_AMOUNT, address(escrowFactory), bob.addr, address(0x00)); + + // deploy escrow + vm.startPrank(bob.addr); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT + TAKING_AMOUNT}(immutables); + + // withdraw + vm.warp(block.timestamp + dstTimelocks.finality + 10); + vm.expectRevert(IEscrow.NativeTokenSendingFailure.selector); + dstClone.withdrawDst(SECRET); + } + function test_CancelResolverSrc() public { // deploy escrow ( @@ -480,7 +519,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.startPrank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); uint256 balanceBob = dai.balanceOf(bob.addr); uint256 balanceBobNative = bob.addr.balance; @@ -506,7 +545,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.prank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); // cancel vm.warp(block.timestamp + dstTimelocks.finality + dstTimelocks.withdrawal + dstTimelocks.publicWithdrawal + 100); @@ -522,7 +561,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.startPrank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); // cancel vm.warp(block.timestamp + dstTimelocks.finality + 100); @@ -538,7 +577,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.startPrank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); // cancel vm.warp(block.timestamp + dstTimelocks.finality + dstTimelocks.withdrawal + 100); @@ -587,7 +626,7 @@ contract EscrowTest is BaseSetup { // deploy escrow vm.startPrank(bob.addr); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); // cancel vm.warp(block.timestamp + dstTimelocks.finality + dstTimelocks.withdrawal + dstTimelocks.publicWithdrawal + 100); diff --git a/test/unit/EscrowFactory.t.sol b/test/unit/EscrowFactory.t.sol index fac8ae7..8fe75e7 100644 --- a/test/unit/EscrowFactory.t.sol +++ b/test/unit/EscrowFactory.t.sol @@ -76,7 +76,7 @@ contract EscrowFactoryTest is BaseSetup { uint256 safetyDeposit = uint64(amount) * 10 / 100; // deploy escrow vm.prank(bob.addr); - escrowFactory.createEscrowDst{value: safetyDeposit}(immutables); + escrowFactory.createDstEscrow{value: safetyDeposit}(immutables); assertEq(bob.addr.balance, balanceBobNative - immutables.args.safetyDeposit); assertEq(dai.balanceOf(bob.addr), balanceBob - amount); @@ -174,23 +174,38 @@ contract EscrowFactoryTest is BaseSetup { } function test_NoUnsafeDeploymentForTaker() public { - (IEscrowFactory.DstEscrowImmutablesCreation memory immutables,) = _prepareDataDst(SECRET, TAKING_AMOUNT, alice.addr, bob.addr, address(dai)); + (IEscrowFactory.DstEscrowImmutablesCreation memory immutables,) = _prepareDataDst( + SECRET, TAKING_AMOUNT, alice.addr, bob.addr, address(dai) + ); vm.warp(immutables.srcCancellationTimestamp + 1); // deploy escrow vm.prank(bob.addr); vm.expectRevert(IEscrowFactory.InvalidCreationTime.selector); - escrowFactory.createEscrowDst{value: DST_SAFETY_DEPOSIT}(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); } function test_NoInsufficientBalanceDeploymentForTaker() public { - (IEscrowFactory.DstEscrowImmutablesCreation memory immutables,) = _prepareDataDst(SECRET, TAKING_AMOUNT, alice.addr, bob.addr, address(dai)); + (IEscrowFactory.DstEscrowImmutablesCreation memory immutables,) = _prepareDataDst( + SECRET, TAKING_AMOUNT, alice.addr, bob.addr, address(dai) + ); + + // deploy escrow + vm.prank(bob.addr); + vm.expectRevert(IEscrowFactory.InsufficientEscrowBalance.selector); + escrowFactory.createDstEscrow(immutables); + } + + function test_NoInsufficientBalanceNativeDeploymentForTaker() public { + (IEscrowFactory.DstEscrowImmutablesCreation memory immutables,) = _prepareDataDst( + SECRET, TAKING_AMOUNT, alice.addr, bob.addr, address(0x00) + ); // deploy escrow vm.prank(bob.addr); vm.expectRevert(IEscrowFactory.InsufficientEscrowBalance.selector); - escrowFactory.createEscrowDst(immutables); + escrowFactory.createDstEscrow{value: DST_SAFETY_DEPOSIT}(immutables); } /* solhint-enable func-name-mixedcase */