Skip to content

Commit

Permalink
Add access token modifier to public withdraw or cancel functions
Browse files Browse the repository at this point in the history
  • Loading branch information
byshape committed Aug 16, 2024
1 parent 7b8d3f7 commit 9d316f2
Show file tree
Hide file tree
Showing 11 changed files with 143 additions and 87 deletions.
105 changes: 53 additions & 52 deletions .gas-snapshot
Original file line number Diff line number Diff line change
@@ -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)
11 changes: 10 additions & 1 deletion contracts/BaseEscrow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Check warning on line 35 in contracts/BaseEscrow.sol

View check run for this annotation

Codecov / codecov/patch

contracts/BaseEscrow.sol#L35

Added line #L35 was not covered by tests
}

modifier onlyTaker(Immutables calldata immutables) {
Expand Down Expand Up @@ -56,6 +60,11 @@ abstract contract BaseEscrow is IBaseEscrow {
_;
}

modifier onlyAccessTokenHolder() {
if(_ACCESS_TOKEN.balanceOf(msg.sender) == 0) revert InvalidCaller();
_;
}

/**
* @notice See {IBaseEscrow-rescueFunds}.
*/
Expand Down
16 changes: 3 additions & 13 deletions contracts/EscrowDst.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand All @@ -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))
{
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions contracts/EscrowFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Check warning on line 36 in contracts/EscrowFactory.sol

View check run for this annotation

Codecov / codecov/patch

contracts/EscrowFactory.sol#L35-L36

Added lines #L35 - L36 were not covered by tests
_PROXY_SRC_BYTECODE_HASH = ProxyHashLib.computeProxyBytecodeHash(ESCROW_SRC_IMPLEMENTATION);
_PROXY_DST_BYTECODE_HASH = ProxyHashLib.computeProxyBytecodeHash(ESCROW_DST_IMPLEMENTATION);
}
Expand Down
4 changes: 3 additions & 1 deletion contracts/EscrowSrc.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion contracts/zkSync/EscrowDstZkSync.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 [email protected]
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);
Expand Down
4 changes: 2 additions & 2 deletions contracts/zkSync/EscrowFactoryZkSync.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Check warning on line 42 in contracts/zkSync/EscrowFactoryZkSync.sol

View check run for this annotation

Codecov / codecov/patch

contracts/zkSync/EscrowFactoryZkSync.sol#L41-L42

Added lines #L41 - L42 were not covered by tests
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);
Expand Down
4 changes: 3 additions & 1 deletion contracts/zkSync/EscrowSrcZkSync.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 [email protected]
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);
Expand Down
1 change: 1 addition & 0 deletions test/libraries/TimelocksLib.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 9d316f2

Please sign in to comment.