Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SC-1052] rawETH flag on withdrawal #12

Merged
merged 1 commit into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 35 additions & 31 deletions .gas-snapshot
Original file line number Diff line number Diff line change
@@ -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)
9 changes: 7 additions & 2 deletions contracts/Escrow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}
}
21 changes: 15 additions & 6 deletions contracts/EscrowFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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) >
Expand All @@ -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
);
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion contracts/interfaces/IEscrowFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
63 changes: 51 additions & 12 deletions test/unit/Escrow.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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
(
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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
(
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
25 changes: 20 additions & 5 deletions test/unit/EscrowFactory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 */
Expand Down
Loading