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

Avoid internal modifiers to simplify logic #75

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
100 changes: 50 additions & 50 deletions .gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -6,67 +6,67 @@ EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForTaker() (gas: 22658)
EscrowFactoryTest:test_NoInsufficientBalanceNativeDeploymentForMaker() (gas: 112896)
EscrowFactoryTest:test_NoInsufficientBalanceNativeDeploymentForTaker() (gas: 27394)
EscrowFactoryTest:test_NoUnsafeDeploymentForTaker() (gas: 30108)
EscrowTest:test_CancelDst() (gas: 111454)
EscrowTest:test_CancelDstDifferentTarget() (gas: 143281)
EscrowTest:test_CancelDstWithNativeToken() (gas: 93611)
EscrowTest:test_CancelPublicSrc() (gas: 156600)
EscrowTest:test_CancelResolverSrc() (gas: 159757)
EscrowTest:test_CancelResolverSrcReceiver() (gas: 170942)
EscrowTest:test_NoAnyoneCancelDuringResolverCancelSrc() (gas: 154958)
EscrowTest:test_NoCallsWithInvalidImmutables() (gas: 277046)
EscrowTest:test_NoCancelByAnyoneDst() (gas: 117101)
EscrowTest:test_NoCancelDuringWithdrawalDst() (gas: 116861)
EscrowTest:test_NoCancelDuringWithdrawalSrc() (gas: 155092)
EscrowTest:test_NoFailedNativeTokenTransferCancelSrc() (gas: 170449)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDst() (gas: 149884)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDstNative() (gas: 83354)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 303443)
EscrowTest:test_NoPublicCancelDuringPrivateCancellationSrc() (gas: 154273)
EscrowTest:test_NoPublicWithdrawOutsideOfAllowedPeriodDst() (gas: 122285)
EscrowTest:test_NoPublicWithdrawalOutsideOfAllowedPeriodSrc() (gas: 160156)
EscrowTest:test_NoRescueFundsByAnyoneDst() (gas: 171804)
EscrowTest:test_NoRescueFundsByAnyoneSrc() (gas: 200175)
EscrowTest:test_NoRescueFundsEarlierDst() (gas: 171247)
EscrowTest:test_NoRescueFundsEarlierSrc() (gas: 200092)
EscrowTest:test_NoWithdrawalByAnyoneSrc() (gas: 151984)
EscrowTest:test_NoWithdrawalByNonResolverDst() (gas: 116801)
EscrowTest:test_NoWithdrawalOutsideOfAllowedPeriodDst() (gas: 121701)
EscrowTest:test_NoWithdrawalOutsideOfAllowedPeriodSrc() (gas: 160717)
EscrowTest:test_NoWithdrawalWithWrongSecretDst() (gas: 118166)
EscrowTest:test_NoWithdrawalWithWrongSecretSrc() (gas: 155589)
EscrowTest:test_PublicWithdrawSrc() (gas: 172891)
EscrowTest:test_RescueFundsDst() (gas: 153614)
EscrowTest:test_RescueFundsDstNative() (gas: 182171)
EscrowTest:test_RescueFundsSrc() (gas: 186570)
EscrowTest:test_RescueFundsSrcNative() (gas: 188836)
EscrowTest:test_WithdrawByAnyoneDst() (gas: 136666)
EscrowTest:test_WithdrawByResolverDst() (gas: 137751)
EscrowTest:test_WithdrawByResolverDstNative() (gas: 97799)
EscrowTest:test_WithdrawByResolverPublicDst() (gas: 137097)
EscrowTest:test_WithdrawSrc() (gas: 177670)
EscrowTest:test_WithdrawSrcTo() (gas: 182509)
EscrowTest:test_CancelDst() (gas: 111446)
EscrowTest:test_CancelDstDifferentTarget() (gas: 143273)
EscrowTest:test_CancelDstWithNativeToken() (gas: 93603)
EscrowTest:test_CancelPublicSrc() (gas: 156592)
EscrowTest:test_CancelResolverSrc() (gas: 159743)
EscrowTest:test_CancelResolverSrcReceiver() (gas: 170928)
EscrowTest:test_NoAnyoneCancelDuringResolverCancelSrc() (gas: 154950)
EscrowTest:test_NoCallsWithInvalidImmutables() (gas: 276429)
EscrowTest:test_NoCancelByAnyoneDst() (gas: 117093)
EscrowTest:test_NoCancelDuringWithdrawalDst() (gas: 116853)
EscrowTest:test_NoCancelDuringWithdrawalSrc() (gas: 155390)
EscrowTest:test_NoFailedNativeTokenTransferCancelSrc() (gas: 170441)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDst() (gas: 149893)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDstNative() (gas: 83358)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 303438)
EscrowTest:test_NoPublicCancelDuringPrivateCancellationSrc() (gas: 154571)
EscrowTest:test_NoPublicWithdrawOutsideOfAllowedPeriodDst() (gas: 123054)
EscrowTest:test_NoPublicWithdrawalOutsideOfAllowedPeriodSrc() (gas: 160897)
EscrowTest:test_NoRescueFundsByAnyoneDst() (gas: 171803)
EscrowTest:test_NoRescueFundsByAnyoneSrc() (gas: 200183)
EscrowTest:test_NoRescueFundsEarlierDst() (gas: 171246)
EscrowTest:test_NoRescueFundsEarlierSrc() (gas: 200100)
EscrowTest:test_NoWithdrawalByAnyoneSrc() (gas: 151970)
EscrowTest:test_NoWithdrawalByNonResolverDst() (gas: 116793)
EscrowTest:test_NoWithdrawalOutsideOfAllowedPeriodDst() (gas: 122457)
EscrowTest:test_NoWithdrawalOutsideOfAllowedPeriodSrc() (gas: 161459)
EscrowTest:test_NoWithdrawalWithWrongSecretDst() (gas: 117949)
EscrowTest:test_NoWithdrawalWithWrongSecretSrc() (gas: 155348)
EscrowTest:test_PublicWithdrawSrc() (gas: 172898)
EscrowTest:test_RescueFundsDst() (gas: 153613)
EscrowTest:test_RescueFundsDstNative() (gas: 182170)
EscrowTest:test_RescueFundsSrc() (gas: 186578)
EscrowTest:test_RescueFundsSrcNative() (gas: 188844)
EscrowTest:test_WithdrawByAnyoneDst() (gas: 136678)
EscrowTest:test_WithdrawByResolverDst() (gas: 137758)
EscrowTest:test_WithdrawByResolverDstNative() (gas: 97806)
EscrowTest:test_WithdrawByResolverPublicDst() (gas: 137104)
EscrowTest:test_WithdrawSrc() (gas: 177686)
EscrowTest:test_WithdrawSrcTo() (gas: 182501)
IntegrationEscrowFactoryTest:test_DeployCloneForMakerNonWhitelistedResolverInt() (gas: 470984)
IntegrationEscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMakerInt() (gas: 338739)
IntegrationResolverMockTest:test_MockCancelDst() (gas: 152609)
IntegrationResolverMockTest:test_MockCancelSrc() (gas: 351046)
IntegrationResolverMockTest:test_MockCancelDst() (gas: 152601)
IntegrationResolverMockTest:test_MockCancelSrc() (gas: 351032)
IntegrationResolverMockTest:test_MockDeployDst() (gas: 146974)
IntegrationResolverMockTest:test_MockDeploySrc() (gas: 361519)
IntegrationResolverMockTest:test_MockPublicCancelSrc() (gas: 360087)
IntegrationResolverMockTest:test_MockPublicWithdrawDst() (gas: 156924)
IntegrationResolverMockTest:test_MockRescueFundsDst() (gas: 156496)
IntegrationResolverMockTest:test_MockRescueFundsSrc() (gas: 379602)
IntegrationResolverMockTest:test_MockWithdrawDst() (gas: 178381)
IntegrationResolverMockTest:test_MockWithdrawToSrc() (gas: 352052)
IntegrationResolverMockTest:test_MockPublicCancelSrc() (gas: 360071)
IntegrationResolverMockTest:test_MockPublicWithdrawDst() (gas: 156928)
IntegrationResolverMockTest:test_MockRescueFundsDst() (gas: 156480)
IntegrationResolverMockTest:test_MockRescueFundsSrc() (gas: 379586)
IntegrationResolverMockTest:test_MockWithdrawDst() (gas: 178388)
IntegrationResolverMockTest:test_MockWithdrawToSrc() (gas: 352044)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllExtra() (gas: 921492)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllTwoFills() (gas: 920173)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirst() (gas: 704406)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirst() (gas: 704307)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirstTwoFills() (gas: 930326)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillLast() (gas: 704142)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillLast() (gas: 704033)
MerkleStorageInvalidatorIntTest:test_MultipleFillsNoDeploymentWithoutValidation() (gas: 298851)
MerkleStorageInvalidatorIntTest:test_MultipleFillsNoSecondDeploymentWithTheSameIndex() (gas: 776050)
MerkleStorageInvalidatorIntTest:test_MultipleFillsOddDivision() (gas: 437381)
MerkleStorageInvalidatorIntTest:test_MultipleFillsOneFill() (gas: 704285)
MerkleStorageInvalidatorIntTest:test_MultipleFillsTwoFills() (gas: 918885)
TimelocksLibTest:test_NoTimelocksOverflow() (gas: 128740)
TimelocksLibTest:test_NoTimelocksOverflow() (gas: 129129)
TimelocksLibTest:test_getStartTimestamps() (gas: 16180)
TimelocksLibTest:test_setDeployedAt() (gas: 5731)
10 changes: 5 additions & 5 deletions contracts/EscrowDst.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ contract EscrowDst is Escrow, IEscrowDst {
function withdraw(bytes32 secret, Immutables calldata immutables)
external
onlyTaker(immutables)
onlyValidImmutables(immutables)
onlyValidSecret(secret, immutables)
onlyAfter(immutables.timelocks.get(TimelocksLib.Stage.DstWithdrawal))
onlyBefore(immutables.timelocks.get(TimelocksLib.Stage.DstCancellation))
{
Expand All @@ -46,6 +48,8 @@ contract EscrowDst is Escrow, IEscrowDst {
*/
function publicWithdraw(bytes32 secret, Immutables calldata immutables)
external
onlyValidImmutables(immutables)
onlyValidSecret(secret, immutables)
onlyAfter(immutables.timelocks.get(TimelocksLib.Stage.DstPublicWithdrawal))
onlyBefore(immutables.timelocks.get(TimelocksLib.Stage.DstCancellation))
{
Expand All @@ -72,11 +76,7 @@ contract EscrowDst is Escrow, IEscrowDst {
* @dev Transfers ERC20 (or native) tokens to the maker and native tokens to the caller.
* @param immutables The immutable values used to deploy the clone contract.
*/
function _withdraw(bytes32 secret, Immutables calldata immutables)
internal
onlyValidImmutables(immutables)
onlyValidSecret(secret, immutables)
{
function _withdraw(bytes32 secret, Immutables calldata immutables) internal {
_uniTransfer(immutables.token.get(), immutables.maker.get(), immutables.amount);
_ethTransfer(msg.sender, immutables.safetyDeposit);
emit Withdrawal(secret);
Expand Down
16 changes: 10 additions & 6 deletions contracts/EscrowSrc.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ contract EscrowSrc is Escrow, IEscrowSrc {
function withdraw(bytes32 secret, Immutables calldata immutables)
external
onlyTaker(immutables)
onlyValidImmutables(immutables)
onlyValidSecret(secret, immutables)
onlyAfter(immutables.timelocks.get(TimelocksLib.Stage.SrcWithdrawal))
onlyBefore(immutables.timelocks.get(TimelocksLib.Stage.SrcCancellation))
{
Expand All @@ -52,6 +54,8 @@ contract EscrowSrc is Escrow, IEscrowSrc {
function withdrawTo(bytes32 secret, address target, Immutables calldata immutables)
external
onlyTaker(immutables)
onlyValidImmutables(immutables)
onlyValidSecret(secret, immutables)
onlyAfter(immutables.timelocks.get(TimelocksLib.Stage.SrcWithdrawal))
onlyBefore(immutables.timelocks.get(TimelocksLib.Stage.SrcCancellation))
{
Expand All @@ -66,6 +70,8 @@ contract EscrowSrc is Escrow, IEscrowSrc {
*/
function publicWithdraw(bytes32 secret, Immutables calldata immutables)
external
onlyValidImmutables(immutables)
onlyValidSecret(secret, immutables)
onlyAfter(immutables.timelocks.get(TimelocksLib.Stage.SrcPublicWithdrawal))
onlyBefore(immutables.timelocks.get(TimelocksLib.Stage.SrcCancellation))
{
Expand All @@ -81,6 +87,7 @@ contract EscrowSrc is Escrow, IEscrowSrc {
function cancel(Immutables calldata immutables)
external
onlyTaker(immutables)
onlyValidImmutables(immutables)
onlyAfter(immutables.timelocks.get(TimelocksLib.Stage.SrcCancellation))
{
_cancel(immutables);
Expand All @@ -94,6 +101,7 @@ contract EscrowSrc is Escrow, IEscrowSrc {
*/
function publicCancel(Immutables calldata immutables)
external
onlyValidImmutables(immutables)
onlyAfter(immutables.timelocks.get(TimelocksLib.Stage.SrcPublicCancellation))
{
_cancel(immutables);
Expand All @@ -105,11 +113,7 @@ contract EscrowSrc is Escrow, IEscrowSrc {
* @param target The address to transfer ERC20 tokens to.
* @param immutables The immutable values used to deploy the clone contract.
*/
function _withdrawTo(bytes32 secret, address target, Immutables calldata immutables)
internal
onlyValidImmutables(immutables)
onlyValidSecret(secret, immutables)
{
function _withdrawTo(bytes32 secret, address target, Immutables calldata immutables) internal {
IERC20(immutables.token.get()).safeTransfer(target, immutables.amount);
_ethTransfer(msg.sender, immutables.safetyDeposit);
emit Withdrawal(secret);
Expand All @@ -119,7 +123,7 @@ contract EscrowSrc is Escrow, IEscrowSrc {
* @dev Transfers ERC20 tokens to the maker and native tokens to the caller.
* @param immutables The immutable values used to deploy the clone contract.
*/
function _cancel(Immutables calldata immutables) internal onlyValidImmutables(immutables) {
function _cancel(Immutables calldata immutables) internal {
IERC20(immutables.token.get()).safeTransfer(immutables.maker.get(), immutables.amount);
_ethTransfer(msg.sender, immutables.safetyDeposit);
emit EscrowCancelled();
Expand Down