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

refactor: dry-fy functions #683

Merged
merged 1 commit into from
Sep 26, 2023
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
34 changes: 0 additions & 34 deletions src/SablierV2LockupDynamic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -630,26 +630,10 @@ contract SablierV2LockupDynamic is

// Effects: renounce the stream by making it not cancelable.
_streams[streamId].isCancelable = false;

// Interactions: if the recipient is a contract, try to invoke the renounce hook on the recipient without
// reverting if the hook is not implemented, and also without bubbling up any potential revert.
address recipient = _ownerOf(streamId);
if (recipient.code.length > 0) {
try ISablierV2LockupRecipient(recipient).onStreamRenounced(streamId) { } catch { }
}

// Log the renouncement.
emit ISablierV2Lockup.RenounceLockupStream(streamId);
}

/// @dev See the documentation for the user-facing functions that call this internal function.
function _withdraw(uint256 streamId, address to, uint128 amount) internal override {
// Checks: the withdraw amount is not greater than the withdrawable amount.
uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
if (amount > withdrawableAmount) {
revert Errors.SablierV2Lockup_Overdraw(streamId, amount, withdrawableAmount);
}

// Effects: update the withdrawn amount.
_streams[streamId].amounts.withdrawn = _streams[streamId].amounts.withdrawn + amount;

Expand All @@ -668,23 +652,5 @@ contract SablierV2LockupDynamic is

// Interactions: perform the ERC-20 transfer.
_streams[streamId].asset.safeTransfer({ to: to, value: amount });

// Retrieve the recipient from storage.
address recipient = _ownerOf(streamId);

// Interactions: if `msg.sender` is not the recipient and the recipient is a contract, try to invoke the
// withdraw hook on it without reverting if the hook is not implemented, and also without bubbling up
// any potential revert.
if (msg.sender != recipient && recipient.code.length > 0) {
try ISablierV2LockupRecipient(recipient).onStreamWithdrawn({
streamId: streamId,
caller: msg.sender,
to: to,
amount: amount
}) { } catch { }
}

// Log the withdrawal.
emit ISablierV2Lockup.WithdrawFromLockupStream(streamId, to, amount);
}
}
34 changes: 0 additions & 34 deletions src/SablierV2LockupLinear.sol
Original file line number Diff line number Diff line change
Expand Up @@ -536,26 +536,10 @@ contract SablierV2LockupLinear is

// Effects: renounce the stream by making it not cancelable.
_streams[streamId].isCancelable = false;

// Interactions: if the recipient is a contract, try to invoke the renounce hook on the recipient without
// reverting if the hook is not implemented, and also without bubbling up any potential revert.
address recipient = _ownerOf(streamId);
if (recipient.code.length > 0) {
try ISablierV2LockupRecipient(recipient).onStreamRenounced(streamId) { } catch { }
}

// Log the renouncement.
emit ISablierV2Lockup.RenounceLockupStream(streamId);
}

/// @dev See the documentation for the user-facing functions that call this internal function.
function _withdraw(uint256 streamId, address to, uint128 amount) internal override {
// Checks: the withdraw amount is not greater than the withdrawable amount.
uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
if (amount > withdrawableAmount) {
revert Errors.SablierV2Lockup_Overdraw(streamId, amount, withdrawableAmount);
}

// Effects: update the withdrawn amount.
_streams[streamId].amounts.withdrawn = _streams[streamId].amounts.withdrawn + amount;

Expand All @@ -574,23 +558,5 @@ contract SablierV2LockupLinear is

// Interactions: perform the ERC-20 transfer.
_streams[streamId].asset.safeTransfer({ to: to, value: amount });

// Retrieve the recipient from storage.
address recipient = _ownerOf(streamId);

// Interactions: if `msg.sender` is not the recipient and the recipient is a contract, try to invoke the
// withdraw hook on it without reverting if the hook is not implemented, and also without bubbling up
// any potential revert.
if (msg.sender != recipient && recipient.code.length > 0) {
try ISablierV2LockupRecipient(recipient).onStreamWithdrawn({
streamId: streamId,
caller: msg.sender,
to: to,
amount: amount
}) { } catch { }
}

// Log the withdrawal.
emit ISablierV2Lockup.WithdrawFromLockupStream(streamId, to, amount);
}
}
41 changes: 38 additions & 3 deletions src/abstracts/SablierV2Lockup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions
import { ISablierV2Comptroller } from "../interfaces/ISablierV2Comptroller.sol";
import { ISablierV2Lockup } from "../interfaces/ISablierV2Lockup.sol";
import { ISablierV2NFTDescriptor } from "../interfaces/ISablierV2NFTDescriptor.sol";
import { ISablierV2LockupRecipient } from "../interfaces/hooks/ISablierV2LockupRecipient.sol";
import { Errors } from "../libraries/Errors.sol";
import { Lockup } from "../types/DataTypes.sol";
import { SablierV2Base } from "./SablierV2Base.sol";
Expand Down Expand Up @@ -198,8 +199,18 @@ abstract contract SablierV2Lockup is
revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
}

// Effects: renounce the stream.
// Checks and Effects: renounce the stream.
_renounce(streamId);

// Interactions: if the recipient is a contract, try to invoke the renounce hook on the recipient without
// reverting if the hook is not implemented, and also without bubbling up any potential revert.
address recipient = _ownerOf(streamId);
if (recipient.code.length > 0) {
try ISablierV2LockupRecipient(recipient).onStreamRenounced(streamId) { } catch { }
}

// Log the renouncement.
emit ISablierV2Lockup.RenounceLockupStream(streamId);
}

/// @inheritdoc ISablierV2Lockup
Expand Down Expand Up @@ -255,8 +266,32 @@ abstract contract SablierV2Lockup is
revert Errors.SablierV2Lockup_WithdrawAmountZero(streamId);
}

// Checks, Effects and Interactions: make the withdrawal.
// Checks: the withdraw amount is not greater than the withdrawable amount.
uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
if (amount > withdrawableAmount) {
revert Errors.SablierV2Lockup_Overdraw(streamId, amount, withdrawableAmount);
}

// Effects and Interactions: make the withdrawal.
_withdraw(streamId, to, amount);

// Retrieve the recipient from storage.
address recipient = _ownerOf(streamId);

// Interactions: if `msg.sender` is not the recipient and the recipient is a contract, try to invoke the
// withdraw hook on it without reverting if the hook is not implemented, and also without bubbling up
// any potential revert.
if (msg.sender != recipient && recipient.code.length > 0) {
try ISablierV2LockupRecipient(recipient).onStreamWithdrawn({
streamId: streamId,
caller: msg.sender,
to: to,
amount: amount
}) { } catch { }
}

// Log the withdrawal.
emit ISablierV2Lockup.WithdrawFromLockupStream(streamId, to, amount);
}

/// @inheritdoc ISablierV2Lockup
Expand Down Expand Up @@ -284,7 +319,7 @@ abstract contract SablierV2Lockup is
// Skip the withdrawal if the withdrawable amount is zero.
uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
if (withdrawableAmount > 0) {
_withdraw({ streamId: streamId, to: currentRecipient, amount: withdrawableAmount });
withdraw({ streamId: streamId, to: currentRecipient, amount: withdrawableAmount });
}

// Checks and Effects: transfer the NFT.
Expand Down
4 changes: 2 additions & 2 deletions test/utils/Precompiles.sol

Large diffs are not rendered by default.