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-1045] Update timelocks #11

Merged
merged 7 commits into from
Jan 25, 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
42 changes: 22 additions & 20 deletions .gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,29 +1,31 @@
EscrowFactoryTest:testFuzz_DeployCloneForMaker(bytes32,uint56,uint56) (runs: 256, μ: 317514, ~: 321353)
EscrowFactoryTest:testFuzz_DeployCloneForTaker(bytes32,uint56) (runs: 256, μ: 243672, ~: 247826)
EscrowFactoryTest:testFuzz_DeployCloneForMaker(bytes32,uint56,uint56) (runs: 256, μ: 317761, ~: 321353)
EscrowFactoryTest:testFuzz_DeployCloneForTaker(bytes32,uint56) (runs: 256, μ: 243976, ~: 247826)
EscrowFactoryTest:test_NoDeploymentForNotResolver() (gas: 112084)
EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMaker() (gas: 281570)
EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForTaker() (gas: 30637)
EscrowFactoryTest:test_NoInsufficientBalanceNativeDeploymentForMaker() (gas: 241684)
EscrowFactoryTest:test_NoUnsafeDeploymentForTaker() (gas: 38019)
EscrowTest:test_CancelDst() (gas: 230530)
EscrowTest:test_CancelPublicSrc() (gas: 319739)
EscrowTest:test_CancelResolverSrc() (gas: 322898)
EscrowTest:test_NoAnyoneCancelDuringResolverCancelSrc() (gas: 320025)
EscrowTest:test_NoCancelDuringPublicUnlockDst() (gas: 238444)
EscrowTest:test_NoCancelDuringResolverUnlockDst() (gas: 237840)
EscrowTest:test_NoCancelDuringUnlockSrc() (gas: 319525)
EscrowTest:test_NoFailedNativeTokenTransferCancelDst() (gas: 249083)
EscrowTest:test_NoFailedNativeTokenTransferCancelSrc() (gas: 335638)
EscrowTest:test_CancelDst() (gas: 230915)
EscrowTest:test_CancelPublicSrc() (gas: 319805)
EscrowTest:test_CancelResolverSrc() (gas: 322942)
EscrowTest:test_NoAnyoneCancelDuringResolverCancelSrc() (gas: 320091)
EscrowTest:test_NoCancelByAnyoneDst() (gas: 238401)
EscrowTest:test_NoCancelDuringPublicWithdrawalDst() (gas: 238345)
EscrowTest:test_NoCancelDuringResolverWithdrawalDst() (gas: 238071)
EscrowTest:test_NoCancelDuringWithdrawalSrc() (gas: 319437)
EscrowTest:test_NoFailedNativeTokenTransferCancelDst() (gas: 249340)
EscrowTest:test_NoFailedNativeTokenTransferCancelSrc() (gas: 335616)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDst() (gas: 271094)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 352891)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 353692)
EscrowTest:test_NoWithdrawalByAnyone() (gas: 319084)
EscrowTest:test_NoWithdrawalByNonResolverDst() (gas: 238110)
EscrowTest:test_NoWithdrawalDuringFinalityLockDst() (gas: 237218)
EscrowTest:test_NoWithdrawalDuringFinalityLockSrc() (gas: 319286)
EscrowTest:test_NoWithdrawalWithWrongSecretDst() (gas: 238985)
EscrowTest:test_NoWithdrawalWithWrongSecretSrc() (gas: 319936)
EscrowTest:test_WithdrawByAnyoneDst() (gas: 255284)
EscrowTest:test_WithdrawByResolverDst() (gas: 255930)
EscrowTest:test_WithdrawByResolverPublicDst() (gas: 255647)
EscrowTest:test_WithdrawSrc() (gas: 336716)
IntegrationEscrowFactoryTest:testFuzz_DeployCloneForMakerInt(bytes32,uint56,uint56) (runs: 256, μ: 397861, ~: 401576)
EscrowTest:test_NoWithdrawalDuringFinalityLockSrc() (gas: 319954)
EscrowTest:test_NoWithdrawalWithWrongSecretDst() (gas: 239029)
EscrowTest:test_NoWithdrawalWithWrongSecretSrc() (gas: 320533)
EscrowTest:test_WithdrawByAnyoneDst() (gas: 255262)
EscrowTest:test_WithdrawByResolverDst() (gas: 255974)
EscrowTest:test_WithdrawByResolverPublicDst() (gas: 255625)
EscrowTest:test_WithdrawSrc() (gas: 340100)
IntegrationEscrowFactoryTest:testFuzz_DeployCloneForMakerInt(bytes32,uint56,uint56) (runs: 256, μ: 398108, ~: 401576)
IntegrationEscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMakerInt() (gas: 382262)
5 changes: 1 addition & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,8 @@ jobs:
- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1

- name: Check contract sizes
run: forge build --sizes

- name: Run tests
run: yarn test
run: forge test -vvv --gas-report

# - name: Run snapshot
# run: forge snapshot
Expand Down
30 changes: 21 additions & 9 deletions contracts/Escrow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,18 @@ contract Escrow is Clone, IEscrow {

/**
* @notice See {IEscrow-withdrawSrc}.
* @dev The function works on the time interval highlighted with capital letters:
* ---- contract deployed --/-- finality --/-- PRIVATE WITHDRAWAL --/-- private cancel --/-- public cancel ----
*/
function withdrawSrc(bytes32 secret) external {
SrcEscrowImmutables calldata escrowImmutables = srcEscrowImmutables();
if (msg.sender != escrowImmutables.interactionParams.taker) revert InvalidCaller();

uint256 finalisedTimestamp = escrowImmutables.deployedAt + escrowImmutables.extraDataParams.srcTimelocks.finality;
// Check that it's public unlock period.
// Check that it's public withdrawal period.
if (
block.timestamp < finalisedTimestamp ||
block.timestamp >= finalisedTimestamp + escrowImmutables.extraDataParams.srcTimelocks.publicUnlock
block.timestamp >= finalisedTimestamp + escrowImmutables.extraDataParams.srcTimelocks.withdrawal
) revert InvalidWithdrawalTime();

_checkSecretAndTransfer(
Expand All @@ -47,11 +51,13 @@ contract Escrow is Clone, IEscrow {

/**
* @notice See {IEscrow-cancelSrc}.
* @dev The function works on the time intervals highlighted with capital letters:
* ---- contract deployed --/-- finality --/-- private withdrawal --/-- PRIVATE CANCEL --/-- PUBLIC CANCEL ----
*/
function cancelSrc() external {
SrcEscrowImmutables calldata escrowImmutables = srcEscrowImmutables();
uint256 finalisedTimestamp = escrowImmutables.deployedAt + escrowImmutables.extraDataParams.srcTimelocks.finality;
uint256 cancellationTimestamp = finalisedTimestamp + escrowImmutables.extraDataParams.srcTimelocks.publicUnlock;
uint256 cancellationTimestamp = finalisedTimestamp + escrowImmutables.extraDataParams.srcTimelocks.withdrawal;
// Check that it's cancellation period.
if (block.timestamp < cancellationTimestamp) {
revert InvalidCancellationTime();
Expand All @@ -77,19 +83,21 @@ contract Escrow is Clone, IEscrow {

/**
* @notice See {IEscrow-withdrawDst}.
* @dev The function works on the time intervals highlighted with capital letters:
* ---- contract deployed --/-- finality --/-- PRIVATE WITHDRAWAL --/-- PUBLIC WITHDRAWAL --/-- private cancel ----
*/
function withdrawDst(bytes32 secret) external {
DstEscrowImmutables calldata escrowImmutables = dstEscrowImmutables();
uint256 finalisedTimestamp = escrowImmutables.deployedAt + escrowImmutables.timelocks.finality;
uint256 publicUnlockTimestamp = finalisedTimestamp + escrowImmutables.timelocks.unlock;
// Check that it's an unlock period.
uint256 publicWithdrawalTimestamp = finalisedTimestamp + escrowImmutables.timelocks.withdrawal;
// Check that it's an withdrawal period.
if (
block.timestamp < finalisedTimestamp ||
block.timestamp >= publicUnlockTimestamp + escrowImmutables.timelocks.publicUnlock
block.timestamp >= publicWithdrawalTimestamp + escrowImmutables.timelocks.publicWithdrawal
) revert InvalidWithdrawalTime();

// Check that the caller is a taker if it's the private unlock period.
if (block.timestamp < publicUnlockTimestamp && msg.sender != escrowImmutables.taker) revert InvalidCaller();
// Check that the caller is a taker if it's the private withdrawal period.
if (block.timestamp < publicWithdrawalTimestamp && msg.sender != escrowImmutables.taker) revert InvalidCaller();

_checkSecretAndTransfer(
secret,
Expand All @@ -106,14 +114,18 @@ contract Escrow is Clone, IEscrow {

/**
* @notice See {IEscrow-cancelDst}.
* @dev The function works on the time interval highlighted with capital letters:
* ---- contract deployed --/-- finality --/-- private withdrawal --/-- public withdrawal --/-- PRIVATE CANCEL ----
*/
function cancelDst() external {
DstEscrowImmutables calldata escrowImmutables = dstEscrowImmutables();
if (msg.sender != escrowImmutables.taker) revert InvalidCaller();

uint256 finalisedTimestamp = escrowImmutables.deployedAt + escrowImmutables.timelocks.finality;
// Check that it's a cancellation period.
if (
block.timestamp <
finalisedTimestamp + escrowImmutables.timelocks.unlock + escrowImmutables.timelocks.publicUnlock
finalisedTimestamp + escrowImmutables.timelocks.withdrawal + escrowImmutables.timelocks.publicWithdrawal
) {
revert InvalidCancellationTime();
}
Expand Down
8 changes: 4 additions & 4 deletions contracts/EscrowFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ contract EscrowFactory is IEscrowFactory, SimpleSettlementExtension {
if (
block.timestamp +
dstEscrowImmutables.timelocks.finality +
dstEscrowImmutables.timelocks.unlock +
dstEscrowImmutables.timelocks.publicUnlock >
dstEscrowImmutables.timelocks.withdrawal +
dstEscrowImmutables.timelocks.publicWithdrawal >
dstEscrowImmutables.srcCancellationTimestamp
) revert InvalidCreationTime();
bytes memory data = abi.encode(
Expand All @@ -105,8 +105,8 @@ contract EscrowFactory is IEscrowFactory, SimpleSettlementExtension {
dstEscrowImmutables.amount,
dstEscrowImmutables.safetyDeposit,
dstEscrowImmutables.timelocks.finality,
dstEscrowImmutables.timelocks.unlock,
dstEscrowImmutables.timelocks.publicUnlock
dstEscrowImmutables.timelocks.withdrawal,
dstEscrowImmutables.timelocks.publicWithdrawal
);
bytes32 salt = keccak256(abi.encodePacked(data, msg.sender));

Expand Down
24 changes: 12 additions & 12 deletions contracts/interfaces/IEscrow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,25 @@ interface IEscrow {
/**
* Timelocks for the source chain.
* finality: The duration of the chain finality period.
* publicUnlock: The duration of the period when anyone with a secret can withdraw tokens for the taker.
* withdrawal: The duration of the period when only the taker with a secret can withdraw tokens for the taker.
* cancel: The duration of the period when escrow can only be cancelled by the taker.
*/
struct SrcTimelocks {
uint256 finality;
uint256 publicUnlock;
uint256 withdrawal;
uint256 cancel;
}

/**
* Timelocks for the destination chain.
* finality: The duration of the chain finality period.
* unlock: The duration of the period when only the taker with a secret can withdraw tokens for the maker.
* publicUnlock publicUnlock: The duration of the period when anyone with a secret can withdraw tokens for the maker.
* withdrawal: The duration of the period when only the taker with a secret can withdraw tokens for the maker.
* publicWithdrawal: The duration of the period when anyone with a secret can withdraw tokens for the maker.
*/
struct DstTimelocks {
uint256 finality;
uint256 unlock;
uint256 publicUnlock;
uint256 withdrawal;
uint256 publicWithdrawal;
}

// Data for the immutables from the order post interacton.
Expand Down Expand Up @@ -82,9 +82,9 @@ interface IEscrow {

/**
* @notice Withdraws funds to the taker on the source chain.
* @dev Withdrawal can only be made during the public unlock period and with secret
* @dev Withdrawal can only be made by the taker during the withdrawal period and with secret
* with hash matches the hashlock.
* The safety deposit is sent to the caller.
* The safety deposit is sent to the caller (taker).
* @param secret The secret that unlocks the escrow.
*/
function withdrawSrc(bytes32 secret) external;
Expand All @@ -99,17 +99,17 @@ interface IEscrow {

/**
* @notice Withdraws funds to the maker on the destination chain.
* @dev Withdrawal can only be made by taker during the private unlock period or by anyone
* during the public unlock period. In both cases, a secret with hash matching the hashlock must be provided.
* @dev Withdrawal can only be made by taker during the private withdrawal period or by anyone
* during the public withdrawal period. In both cases, a secret with hash matching the hashlock must be provided.
* The safety deposit is sent to the caller.
* @param secret The secret that unlocks the escrow.
*/
function withdrawDst(bytes32 secret) external;

/**
* @notice Cancels the escrow on the destination chain and returns tokens to the taker.
* @dev The escrow can only be cancelled during the cancel period.
* The safety deposit is sent to the caller.
* @dev The escrow can only be cancelled by the taker during the cancel period.
* The safety deposit is sent to the caller (taker).
*/
function cancelDst() external;

Expand Down
Loading