diff --git a/src/SablierV2MerkleStreamerFactory.sol b/src/SablierV2MerkleStreamerFactory.sol index 690396c6..acb740a7 100644 --- a/src/SablierV2MerkleStreamerFactory.sol +++ b/src/SablierV2MerkleStreamerFactory.sol @@ -59,15 +59,8 @@ contract SablierV2MerkleStreamerFactory is ISablierV2MerkleStreamerFactory { bytes32 salt = keccak256(abi.encodePacked(initialAdmin, lockupLinear, asset, merkleRoot, expiration)); // Deploy the Merkle streamer with CREATE2. - merkleStreamerLL = new SablierV2MerkleStreamerLL{salt: salt} ( - initialAdmin, - lockupLinear, - asset, - merkleRoot, - expiration, - streamDurations, - cancelable, - transferable + merkleStreamerLL = new SablierV2MerkleStreamerLL{ salt: salt }( + initialAdmin, lockupLinear, asset, merkleRoot, expiration, streamDurations, cancelable, transferable ); // Effects: append the streamer in the admin's list. diff --git a/src/abstracts/SablierV2MerkleStreamer.sol b/src/abstracts/SablierV2MerkleStreamer.sol index 92040d4a..2b8f29fe 100644 --- a/src/abstracts/SablierV2MerkleStreamer.sol +++ b/src/abstracts/SablierV2MerkleStreamer.sol @@ -97,8 +97,8 @@ abstract contract SablierV2MerkleStreamer is // not call other unknown contracts. UD60x18 protocolFee = LOCKUP.comptroller().protocolFees(ASSET); - // Checks: if the protocol fee is not greater than zero, the campaign must be expired. - if (!protocolFee.gt(ud(0)) && !hasExpired()) { + // Checks: the campaign is not expired and the protocol fee is zero. + if (!hasExpired() && !protocolFee.gt(ud(0))) { revert Errors.SablierV2MerkleStreamer_CampaignNotExpired({ currentTime: block.timestamp, expiration: EXPIRATION @@ -118,15 +118,6 @@ abstract contract SablierV2MerkleStreamer is /// @dev Validates the parameters of the `claim` function, which is implemented by child contracts. function _checkClaim(uint256 index, bytes32 leaf, bytes32[] calldata merkleProof) internal view { - // Safe Interactions: query the protocol fee. This is safe because it's a known Sablier contract that does - // not call other unknown contracts. - UD60x18 protocolFee = LOCKUP.comptroller().protocolFees(ASSET); - - // Checks: the protocol fee is zero. - if (protocolFee.gt(ud(0))) { - revert Errors.SablierV2MerkleStreamer_ProtocolFeeNotZero(); - } - // Checks: the campaign has not expired. if (hasExpired()) { revert Errors.SablierV2MerkleStreamer_CampaignExpired({ @@ -140,6 +131,15 @@ abstract contract SablierV2MerkleStreamer is revert Errors.SablierV2MerkleStreamer_StreamClaimed(index); } + // Safe Interactions: query the protocol fee. This is safe because it's a known Sablier contract that does + // not call other unknown contracts. + UD60x18 protocolFee = LOCKUP.comptroller().protocolFees(ASSET); + + // Checks: the protocol fee is zero. + if (protocolFee.gt(ud(0))) { + revert Errors.SablierV2MerkleStreamer_ProtocolFeeNotZero(); + } + // Checks: the input claim is included in the Merkle tree. if (!MerkleProof.verify(merkleProof, MERKLE_ROOT, leaf)) { revert Errors.SablierV2MerkleStreamer_InvalidProof(); diff --git a/src/interfaces/ISablierV2MerkleStreamerLL.sol b/src/interfaces/ISablierV2MerkleStreamerLL.sol index 7a9d373a..794ad944 100644 --- a/src/interfaces/ISablierV2MerkleStreamerLL.sol +++ b/src/interfaces/ISablierV2MerkleStreamerLL.sol @@ -27,9 +27,9 @@ interface ISablierV2MerkleStreamerLL is ISablierV2MerkleStreamer { /// @dev Emits a {Claim} event. /// /// Requirements: - /// - The protocol fee must be zero. /// - The campaign must not have expired. /// - The stream must not have been claimed already. + /// - The protocol fee must be zero. /// - The Merkle proof must be valid. /// /// @param index The index of the recipient in the Merkle tree. diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index 0cf0c8c7..eb896880 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -30,9 +30,6 @@ library Errors { /// @notice Thrown when trying to claim with an invalid Merkle proof. error SablierV2MerkleStreamer_InvalidProof(); - /// @notice Thrown when trying to clawback when the protocol fee is zero. - error SablierV2MerkleStreamer_ProtocolFeeZero(); - /// @notice Thrown when trying to claim when the protocol fee is not zero. error SablierV2MerkleStreamer_ProtocolFeeNotZero(); diff --git a/test/integration/merkle-streamer/ll/claim/claim.t.sol b/test/integration/merkle-streamer/ll/claim/claim.t.sol index 8c7fdbd5..1ceaeaf8 100644 --- a/test/integration/merkle-streamer/ll/claim/claim.t.sol +++ b/test/integration/merkle-streamer/ll/claim/claim.t.sol @@ -13,19 +13,7 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test { MerkleStreamer_Integration_Test.setUp(); } - function test_RevertGiven_ProtocolFeeNotZero() external { - bytes32[] memory merkleProof; - changePrank({ msgSender: users.admin.addr }); - comptroller.setProtocolFee({ asset: asset, newProtocolFee: ud(0.03e18) }); - vm.expectRevert(Errors.SablierV2MerkleStreamer_ProtocolFeeNotZero.selector); - merkleStreamerLL.claim({ index: 1, recipient: users.recipient1.addr, amount: 1, merkleProof: merkleProof }); - } - - modifier givenProtocolFeeZero() { - _; - } - - function test_RevertGiven_CampaignExpired() external givenProtocolFeeZero { + function test_RevertGiven_CampaignExpired() external { uint40 expiration = defaults.EXPIRATION(); uint256 warpTime = expiration + 1 seconds; bytes32[] memory merkleProof; @@ -40,7 +28,7 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test { _; } - function test_RevertGiven_AlreadyClaimed() external givenCampaignNotExpired givenProtocolFeeZero { + function test_RevertGiven_AlreadyClaimed() external givenCampaignNotExpired { claimLL(); uint256 index1 = defaults.INDEX1(); uint128 amount = defaults.CLAIM_AMOUNT(); @@ -53,6 +41,18 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test { _; } + function test_RevertGiven_ProtocolFeeNotZero() external givenCampaignNotExpired givenNotClaimed { + bytes32[] memory merkleProof; + changePrank({ msgSender: users.admin.addr }); + comptroller.setProtocolFee({ asset: asset, newProtocolFee: ud(0.03e18) }); + vm.expectRevert(Errors.SablierV2MerkleStreamer_ProtocolFeeNotZero.selector); + merkleStreamerLL.claim({ index: 1, recipient: users.recipient1.addr, amount: 1, merkleProof: merkleProof }); + } + + modifier givenProtocolFeeZero() { + _; + } + modifier givenNotIncludedInMerkleTree() { _; } @@ -60,8 +60,8 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test { function test_RevertWhen_InvalidIndex() external givenCampaignNotExpired - givenProtocolFeeZero givenNotClaimed + givenProtocolFeeZero givenNotIncludedInMerkleTree { uint256 invalidIndex = 1337; @@ -74,8 +74,8 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test { function test_RevertWhen_InvalidRecipient() external givenCampaignNotExpired - givenProtocolFeeZero givenNotClaimed + givenProtocolFeeZero givenNotIncludedInMerkleTree { uint256 index1 = defaults.INDEX1(); @@ -89,8 +89,8 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test { function test_RevertWhen_InvalidAmount() external givenCampaignNotExpired - givenProtocolFeeZero givenNotClaimed + givenProtocolFeeZero givenNotIncludedInMerkleTree { uint256 index1 = defaults.INDEX1(); @@ -103,8 +103,8 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test { function test_RevertWhen_InvalidMerkleProof() external givenCampaignNotExpired - givenProtocolFeeZero givenNotClaimed + givenProtocolFeeZero givenNotIncludedInMerkleTree { uint256 index1 = defaults.INDEX1(); @@ -121,8 +121,8 @@ contract Claim_Integration_Test is MerkleStreamer_Integration_Test { function test_Claim() external givenCampaignNotExpired - givenProtocolFeeZero givenNotClaimed + givenProtocolFeeZero givenIncludedInMerkleTree { uint256 expectedStreamId = lockupLinear.nextStreamId(); diff --git a/test/integration/merkle-streamer/ll/claim/claim.tree b/test/integration/merkle-streamer/ll/claim/claim.tree index d34016d3..b145b2f6 100644 --- a/test/integration/merkle-streamer/ll/claim/claim.tree +++ b/test/integration/merkle-streamer/ll/claim/claim.tree @@ -1,13 +1,13 @@ claim.t.sol -├── given the protocol fee is greater than zero +├── given the campaign has expired │ └── it should revert -└── given the protocol fee is not greater than zero - ├── given the campaign has expired +└── given the campaign has not expired + ├── given the recipient has claimed │ └── it should revert - └── given the campaign has not expired - ├── given the recipient has claimed + └── given the recipient has not claimed + ├── given the protocol fee is greater than zero │ └── it should revert - └── given the recipient has not claimed + └── given the protocol fee is not greater than zero ├── given the claim is not included in the Merkle tree │ ├── when the index is not valid │ │ └── it should revert diff --git a/test/integration/merkle-streamer/ll/clawback/clawback.tree b/test/integration/merkle-streamer/ll/clawback/clawback.tree index 98263c2c..55f5c04e 100644 --- a/test/integration/merkle-streamer/ll/clawback/clawback.tree +++ b/test/integration/merkle-streamer/ll/clawback/clawback.tree @@ -9,9 +9,9 @@ clawback.t.sol │ ├── it should perform the ERC-20 transfer │ └── it should emit a {Clawback} event └── given the protocol fee is greater than zero - ├── given the campaign has not expired - │ ├── it should perform the ERC-20 transfer - │ └── it should emit a {Clawback} event - └── given the campaign has expired - ├── it should perform the ERC-20 transfer - └── it should emit a {Clawback} event \ No newline at end of file + ├── given the campaign has not expired + │ ├── it should perform the ERC-20 transfer + │ └── it should emit a {Clawback} event + └── given the campaign has expired + ├── it should perform the ERC-20 transfer + └── it should emit a {Clawback} event