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

Add access token modifier to public withdraw or cancel functions #105

Merged
merged 4 commits into from
Aug 19, 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
97 changes: 49 additions & 48 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: 195267)
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_CancelResolverSrc() (gas: 168607)
EscrowTest:test_CancelDst() (gas: 116076)
EscrowTest:test_CancelDstDifferentTarget() (gas: 143334)
EscrowTest:test_CancelDstWithNativeToken() (gas: 93670)
EscrowTest:test_CancelPublicSrc() (gas: 170900)
EscrowTest:test_CancelResolverSrc() (gas: 168624)
EscrowTest:test_CancelResolverSrcReceiver() (gas: 179361)
EscrowTest:test_NoAnyoneCancelDuringResolverCancelSrc() (gas: 163856)
EscrowTest:test_NoCallsWithInvalidImmutables() (gas: 286518)
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_NoFailedNativeTokenTransferCancelSrc() (gas: 216883)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDst() (gas: 192056)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDstNative() (gas: 84157)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 349789)
EscrowTest:test_NoPublicCallsByAnyone() (gas: 287575)
EscrowTest:test_NoPublicCancelDuringPrivateCancellationSrc() (gas: 168592)
EscrowTest:test_NoPublicWithdrawOutsideOfAllowedPeriodDst() (gas: 137737)
EscrowTest:test_NoPublicWithdrawalOutsideOfAllowedPeriodSrc() (gas: 179922)
EscrowTest:test_NoRescueFundsByAnyoneDst() (gas: 176333)
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_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)
IntegrationEscrowFactoryTest:test_DeployCloneForMakerNonWhitelistedResolverInt() (gas: 473542)
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: 510174)
IntegrationEscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMakerInt() (gas: 341264)
IntegrationEscrowFactoryTest:test_NoResolverReentrancy() (gas: 2065187)
IntegrationResolverMockTest:test_MockCancelDst() (gas: 157184)
IntegrationResolverMockTest:test_MockCancelDst() (gas: 157134)
IntegrationResolverMockTest:test_MockCancelSrc() (gas: 353957)
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_MockPublicCancelSrc() (gas: 397880)
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_MultipleFillsFillAllExtra() (gas: 923554)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllFromLast() (gas: 922336)
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_MultipleFillsFillFirst() (gas: 707614)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirstTwoFills() (gas: 932984)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillLast() (gas: 707218)
MerkleStorageInvalidatorIntTest:test_MultipleFillsNoDeploymentWithoutValidation() (gas: 301456)
MerkleStorageInvalidatorIntTest:test_MultipleFillsNoReuseOfSecrets() (gas: 1069228)
MerkleStorageInvalidatorIntTest:test_MultipleFillsNoSecondDeploymentWithTheSameIndex() (gas: 786011)
MerkleStorageInvalidatorIntTest:test_MultipleFillsNoSecondDeploymentWithTheSameIndex() (gas: 785978)
MerkleStorageInvalidatorIntTest:test_MultipleFillsOddDivision() (gas: 444783)
MerkleStorageInvalidatorIntTest:test_MultipleFillsOneFill() (gas: 707674)
MerkleStorageInvalidatorIntTest:test_MultipleFillsTwoFills() (gas: 921440)
MerkleStorageInvalidatorIntTest:test_MultipleFillsOneFill() (gas: 707615)
MerkleStorageInvalidatorIntTest:test_MultipleFillsTwoFills() (gas: 921433)
MerkleStorageInvalidatorTest:test_ShortenedProofIsNotValid() (gas: 230605)
TimelocksLibTest:test_NoTimelocksOverflow() (gas: 156200)
TimelocksLibTest:test_NoTimelocksOverflow() (gas: 193707)
TimelocksLibTest:test_getStartTimestamps() (gas: 16207)
TimelocksLibTest:test_setDeployedAt() (gas: 5741)
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:
- uses: actions/checkout@v4
with:
repository: matter-labs/foundry-zksync
ref: 'dev'
ref: 'main'
path: 'foundry-zksync'

- name: Install Foundry for zksync
Expand Down Expand Up @@ -79,7 +79,7 @@ jobs:
- uses: actions/checkout@v4
with:
repository: matter-labs/foundry-zksync
ref: 'dev'
ref: 'main'
path: 'foundry-zksync'

- name: Install Foundry for zksync
Expand Down
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 @@
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 @@
_;
}

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 @@
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 @@
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/integration/EscrowFactory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ contract IntegrationEscrowFactoryTest is BaseSetup {

uint256 resolverCredit = feeBank.availableCredit(bob.addr);
inch.mint(charlie.addr, 1000 ether);
accessToken.mint(charlie.addr, 1);

vm.startPrank(charlie.addr);
inch.approve(address(feeBank), 1000 ether);
Expand Down
1 change: 1 addition & 0 deletions test/integration/ResolverMock.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ contract IntegrationResolverMockTest is BaseSetup {
vm.deal(resolverMock, 100 ether);
dai.mint(resolverMock, 1000 ether);
inch.mint(resolverMock, 1000 ether);
accessToken.mint(resolverMock, 1);
vm.startPrank(resolverMock);
inch.approve(address(feeBank), 1000 ether);
feeBank.deposit(10 ether);
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);
accessToken.mint(alice.addr, 1);
vm.startPrank(alice.addr);
dstClone.publicWithdraw(SECRET, immutablesDst);
assertEq(dai.balanceOf(address(dstClone)), 0);
Expand Down
Loading
Loading