Skip to content

Commit

Permalink
feat: integrate ERC-4906
Browse files Browse the repository at this point in the history
docs: polish wording in NatSpec
refactor: move "_nextStreamId" to "SablierV2Lockup"
test: test emit of ERC-4906 events
  • Loading branch information
PaulRBerg committed Jun 23, 2023
1 parent b6e3b44 commit b8a15ef
Show file tree
Hide file tree
Showing 20 changed files with 102 additions and 42 deletions.
3 changes: 0 additions & 3 deletions src/SablierV2LockupDynamic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ contract SablierV2LockupDynamic is
PRIVATE STORAGE
//////////////////////////////////////////////////////////////////////////*/

/// @dev Counter for stream ids, used in the create functions.
uint256 private _nextStreamId;

/// @dev Sablier V2 Lockup Dynamic streams mapped by unsigned integer ids.
mapping(uint256 id => LockupDynamic.Stream stream) private _streams;

Expand Down
3 changes: 0 additions & 3 deletions src/SablierV2LockupLinear.sol
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ contract SablierV2LockupLinear is
PRIVATE STORAGE
//////////////////////////////////////////////////////////////////////////*/

/// @dev Counter for stream ids, used in the create functions.
uint256 private _nextStreamId;

/// @dev Sablier V2 Lockup Linear streams mapped by unsigned integers.
mapping(uint256 id => LockupLinear.Stream stream) private _streams;

Expand Down
31 changes: 27 additions & 4 deletions src/abstracts/SablierV2Lockup.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity >=0.8.19;

import { IERC4906 } from "@openzeppelin/contracts/interfaces/IERC4906.sol";
import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";

Expand All @@ -14,6 +15,7 @@ import { SablierV2Base } from "./SablierV2Base.sol";
/// @title SablierV2Lockup
/// @notice See the documentation in {ISablierV2Lockup}.
abstract contract SablierV2Lockup is
IERC4906, // 2 inherited components
SablierV2Base, // 4 inherited components
ISablierV2Lockup, // 4 inherited components
ERC721 // 6 inherited components
Expand All @@ -22,6 +24,9 @@ abstract contract SablierV2Lockup is
INTERNAL STORAGE
//////////////////////////////////////////////////////////////////////////*/

/// @dev Counter for stream ids, used in the create functions.
uint256 internal _nextStreamId;

/// @dev Contract that generates the non-fungible token URI.
ISablierV2NFTDescriptor internal _nftDescriptor;

Expand Down Expand Up @@ -54,6 +59,12 @@ abstract contract SablierV2Lockup is
_;
}

/// @dev Emits an ERC-4906 event to trigger an update of the NFT metadata.
modifier updateMetadata(uint256 streamId) {
_;
emit MetadataUpdate({ _tokenId: streamId });
}

/*//////////////////////////////////////////////////////////////////////////
USER-FACING CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/
Expand All @@ -79,7 +90,7 @@ abstract contract SablierV2Lockup is
_requireMinted({ tokenId: streamId });

// Generate the URI describing the stream NFT.
uri = _nftDescriptor.tokenURI(this, streamId);
uri = _nftDescriptor.tokenURI({ sablier: this, streamId: streamId });
}

/// @inheritdoc ISablierV2Lockup
Expand Down Expand Up @@ -119,7 +130,7 @@ abstract contract SablierV2Lockup is
}

/// @inheritdoc ISablierV2Lockup
function cancel(uint256 streamId) public override noDelegateCall {
function cancel(uint256 streamId) public override noDelegateCall updateMetadata(streamId) {
// Checks: the stream is neither depleted nor canceled.
if (isDepleted(streamId)) {
revert Errors.SablierV2Lockup_StreamDepleted(streamId);
Expand Down Expand Up @@ -152,7 +163,7 @@ abstract contract SablierV2Lockup is
}

/// @inheritdoc ISablierV2Lockup
function renounce(uint256 streamId) external override noDelegateCall notNull(streamId) {
function renounce(uint256 streamId) external override noDelegateCall notNull(streamId) updateMetadata(streamId) {
// Checks: the stream is not cold.
Lockup.Status status = _statusOf(streamId);
if (status == Lockup.Status.DEPLETED) {
Expand Down Expand Up @@ -184,10 +195,22 @@ abstract contract SablierV2Lockup is
oldNFTDescriptor: oldNftDescriptor,
newNFTDescriptor: newNFTDescriptor
});

// Refresh the NFT metadata for all streams.
emit BatchMetadataUpdate({ _fromTokenId: 1, _toTokenId: _nextStreamId });
}

/// @inheritdoc ISablierV2Lockup
function withdraw(uint256 streamId, address to, uint128 amount) public override noDelegateCall {
function withdraw(
uint256 streamId,
address to,
uint128 amount
)
public
override
noDelegateCall
updateMetadata(streamId)
{
// Checks: the stream is not depleted.
if (isDepleted(streamId)) {
revert Errors.SablierV2Lockup_StreamDepleted(streamId);
Expand Down
14 changes: 7 additions & 7 deletions src/interfaces/ISablierV2Lockup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ interface ISablierV2Lockup is

/// @notice Cancels the stream and refunds any remaining assets to the sender.
///
/// @dev Emits a {CancelLockupStream} event and a {Transfer} event.
/// @dev Emits a {Transfer}, {CancelLockupStream}, and {MetadataUpdate} event.
///
/// Notes:
/// - If there any assets left for the recipient to withdraw, the stream is marked as canceled. Otherwise, the
Expand All @@ -189,7 +189,7 @@ interface ISablierV2Lockup is

/// @notice Cancels multiple streams and refunds any remaining assets to the sender.
///
/// @dev Emits multiple {CancelLockupStream} and {Transfer} events.
/// @dev Emits multiple {Transfer}, {CancelLockupStream}, and {MetadataUpdate} events.
///
/// Notes:
/// - Refer to the notes in {cancel}.
Expand All @@ -202,7 +202,7 @@ interface ISablierV2Lockup is

/// @notice Removes the right of the stream's sender to cancel the stream.
///
/// @dev Emits a {RenounceLockupStream} event.
/// @dev Emits a {RenounceLockupStream} and {MetadataUpdate} event.
///
/// Notes:
/// - This is an irreversible operation.
Expand All @@ -219,7 +219,7 @@ interface ISablierV2Lockup is

/// @notice Sets a new NFT descriptor contract, which produces the URI describing the Sablier stream NFTs.
///
/// @dev Emits a {SetNFTDescriptor} event.
/// @dev Emits a {SetNFTDescriptor} and {BatchMetadataUpdate} event.
///
/// Notes:
/// - Does not revert if the NFT descriptor is the same.
Expand All @@ -232,7 +232,7 @@ interface ISablierV2Lockup is

/// @notice Withdraws the provided amount of assets from the stream to the `to` address.
///
/// @dev Emits a {WithdrawFromLockupStream} and a {Transfer} event.
/// @dev Emits a {Transfer}, {WithdrawFromLockupStream}, and {MetadataUpdate} event.
///
/// Notes:
/// - This function attempts to invoke a hook on the stream's recipient, provided that the recipient is a contract
Expand All @@ -253,7 +253,7 @@ interface ISablierV2Lockup is

/// @notice Withdraws the maximum withdrawable amount from the stream to the `to` address.
///
/// @dev Emits a {WithdrawFromLockupStream} and a {Transfer} event.
/// @dev Emits a {Transfer}, {WithdrawFromLockupStream}, and {MetadataUpdate} event.
///
/// Notes:
/// - Refer to the notes in {withdraw}.
Expand All @@ -267,7 +267,7 @@ interface ISablierV2Lockup is

/// @notice Withdraws assets from streams to the provided address `to`.
///
/// @dev Emits multiple {WithdrawFromLockupStream} and {Transfer} events.
/// @dev Emits multiple {Transfer}, {WithdrawFromLockupStream}, and {MetadataUpdate} events.
///
/// Notes:
/// - This function attempts to call a hook on the recipient of each stream, unless `msg.sender` is the recipient.
Expand Down
6 changes: 3 additions & 3 deletions src/interfaces/ISablierV2LockupDynamic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ interface ISablierV2LockupDynamic is ISablierV2Lockup {
/// @dev This is initialized at construction time and cannot be changed later.
function MAX_SEGMENT_COUNT() external view returns (uint256);

/// @notice Retrieves the stream's range, a struct containing (i) the stream's start time and (ii) end
/// @notice Retrieves the stream's range, which is a struct containing (i) the stream's start time and (ii) end
/// time, both as Unix timestamps.
/// @dev Reverts if `streamId` references a null stream.
/// @param streamId The stream id for the query.
Expand Down Expand Up @@ -93,7 +93,7 @@ interface ISablierV2LockupDynamic is ISablierV2Lockup {
/// `block.timestamp` and all specified time deltas. The segment milestones are derived from these
/// deltas. The stream is funded by `msg.sender` and is wrapped in an ERC-721 NFT.
///
/// @dev Emits a {CreateLockupDynamicStream} and a {Transfer} event.
/// @dev Emits a {Transfer} and {CreateLockupDynamicStream} event.
///
/// Requirements:
/// - All requirements in {createWithMilestones} must be met for the calculated parameters.
Expand All @@ -105,7 +105,7 @@ interface ISablierV2LockupDynamic is ISablierV2Lockup {
/// @notice Creates a stream with the provided segment milestones, implying the end time from the last milestone.
/// The stream is funded by `msg.sender` and is wrapped in an ERC-721 NFT.
///
/// @dev Emits a {CreateLockupDynamicStream} and a {Transfer} event.
/// @dev Emits a {Transfer} and {CreateLockupDynamicStream} event.
///
/// Notes:
/// - As long as the segment milestones are arranged in ascending order, it is not an error for some
Expand Down
6 changes: 3 additions & 3 deletions src/interfaces/ISablierV2LockupLinear.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ interface ISablierV2LockupLinear is ISablierV2Lockup {
/// @param streamId The stream id for the query.
function getCliffTime(uint256 streamId) external view returns (uint40 cliffTime);

/// @notice Retrieves the range of the stream, a struct containing (i) the stream's start time, (ii) cliff
/// @notice Retrieves the stream's range, which is a struct containing (i) the stream's start time, (ii) cliff
/// time, and (iii) end time, all as Unix timestamps.
/// @dev Reverts if `streamId` references a null stream.
/// @param streamId The stream id for the query.
Expand Down Expand Up @@ -87,7 +87,7 @@ interface ISablierV2LockupLinear is ISablierV2Lockup {
/// the sum of `block.timestamp` and `params.durations.total. The stream is funded by `msg.sender` and is wrapped
/// in an ERC-721 NFT.
///
/// @dev Emits a {CreateLockupLinearStream} and a {Transfer} event.
/// @dev Emits a {Transfer} and {CreateLockupLinearStream} event.
///
/// Requirements:
/// - All requirements in {createWithRange} must be met for the calculated parameters.
Expand All @@ -101,7 +101,7 @@ interface ISablierV2LockupLinear is ISablierV2Lockup {
/// @notice Creates a stream with the provided start time and end time as the range. The stream is
/// funded by `msg.sender` and is wrapped in an ERC-721 NFT.
///
/// @dev Emits a {CreateLockupLinearStream} and a {Transfer} event.
/// @dev Emits a {Transfer} and {CreateLockupLinearStream} event.
///
/// Notes:
/// - As long as the times are ordered, it is not an error for the start or the cliff time to be in the past.
Expand Down
8 changes: 6 additions & 2 deletions test/fork/LockupDynamic.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -275,13 +275,15 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
vars.initialLockupDynamicBalance = vars.actualLockupDynamicBalance;
vars.initialRecipientBalance = asset.balanceOf(params.recipient);

// Expect a {WithdrawFromLockupStream} event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupDynamic) });
emit WithdrawFromLockupStream({
streamId: vars.streamId,
to: params.recipient,
amount: params.withdrawAmount
});
vm.expectEmit({ emitter: address(lockupDynamic) });
emit MetadataUpdate({ _tokenId: vars.streamId });

// Make the withdrawal.
changePrank({ msgSender: params.recipient });
Expand Down Expand Up @@ -336,13 +338,15 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
vars.initialSenderBalance = vars.balances[1];
vars.initialRecipientBalance = vars.balances[2];

// Expect a {CancelLockupStream} event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupDynamic) });
vars.senderAmount = lockupDynamic.refundableAmountOf(vars.streamId);
vars.recipientAmount = lockupDynamic.withdrawableAmountOf(vars.streamId);
emit CancelLockupStream(
vars.streamId, params.sender, params.recipient, vars.senderAmount, vars.recipientAmount
);
vm.expectEmit({ emitter: address(lockupDynamic) });
emit MetadataUpdate({ _tokenId: vars.streamId });

// Cancel the stream.
changePrank({ msgSender: params.sender });
Expand Down
8 changes: 6 additions & 2 deletions test/fork/LockupLinear.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,15 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
vars.initialLockupLinearBalance = vars.actualLockupLinearBalance;
vars.initialRecipientBalance = asset.balanceOf(params.recipient);

// Expect a {WithdrawFromLockupStream} event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupLinear) });
emit WithdrawFromLockupStream({
streamId: vars.streamId,
to: params.recipient,
amount: params.withdrawAmount
});
vm.expectEmit({ emitter: address(lockupLinear) });
emit MetadataUpdate({ _tokenId: vars.streamId });

// Make the withdrawal.
changePrank({ msgSender: params.recipient });
Expand Down Expand Up @@ -319,13 +321,15 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
vars.initialSenderBalance = vars.balances[1];
vars.initialRecipientBalance = vars.balances[2];

// Expect a {CancelLockupStream} event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupLinear) });
vars.senderAmount = lockupLinear.refundableAmountOf(vars.streamId);
vars.recipientAmount = lockupLinear.withdrawableAmountOf(vars.streamId);
emit CancelLockupStream(
vars.streamId, params.sender, params.recipient, vars.senderAmount, vars.recipientAmount
);
vm.expectEmit({ emitter: address(lockupLinear) });
emit MetadataUpdate({ _tokenId: vars.streamId });

// Cancel the stream.
changePrank({ msgSender: params.sender });
Expand Down
8 changes: 6 additions & 2 deletions test/integration/basic/lockup/cancel/cancel.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,11 @@ abstract contract Cancel_Integration_Basic_Test is Integration_Test, Cancel_Inte
)
);

// Expect a {CancelLockupStream} event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockup) });
emit CancelLockupStream(streamId, users.sender, address(goodRecipient), senderAmount, recipientAmount);
vm.expectEmit({ emitter: address(lockup) });
emit MetadataUpdate({ _tokenId: streamId });

// Cancel the stream.
lockup.cancel(streamId);
Expand Down Expand Up @@ -456,9 +458,11 @@ abstract contract Cancel_Integration_Basic_Test is Integration_Test, Cancel_Inte
)
);

// Expect a {CancelLockupStream} event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockup) });
emit CancelLockupStream(streamId, address(goodSender), users.recipient, senderAmount, recipientAmount);
vm.expectEmit({ emitter: address(lockup) });
emit MetadataUpdate({ _tokenId: streamId });

// Cancel the stream.
lockup.cancel(streamId);
Expand Down
4 changes: 3 additions & 1 deletion test/integration/basic/lockup/cancel/cancel.tree
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ cancel.t.sol
│ ├── it should update the refunded amount
│ ├── it should refund the sender
│ ├── it should call the recipient hook
│ └── it should emit a {CancelLockupStream} event
│ ├── it should emit a {CancelLockupStream} event
│ └── it should emit a {MetadataUpdate} event
└── when the caller is the recipient
├── when the sender is not a contract
│ ├── it should cancel the stream
Expand Down Expand Up @@ -88,4 +89,5 @@ cancel.t.sol
├── it should update the refunded amount
├── it should refund the sender
├── it should call the sender hook
├── it should emit a {MetadataUpdate} event
└── it should emit a {CancelLockupStream} event
4 changes: 3 additions & 1 deletion test/integration/basic/lockup/renounce/renounce.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,11 @@ abstract contract Renounce_Integration_Basic_Test is Integration_Test, Lockup_In
// Expect a call to hook.
vm.expectCall(address(goodRecipient), abi.encodeCall(ISablierV2LockupRecipient.onStreamRenounced, (streamId)));

// Expect a {RenounceLockupStream} event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockup) });
emit RenounceLockupStream(streamId);
vm.expectEmit({ emitter: address(lockup) });
emit MetadataUpdate({ _tokenId: streamId });

// Renounce the stream.
lockup.renounce(streamId);
Expand Down
3 changes: 2 additions & 1 deletion test/integration/basic/lockup/renounce/renounce.tree
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@ renounce.t.sol
└── when there is no reentrancy
├── it should renounce the stream
├── it should call the recipient hook
└── it should emit a {RenounceLockupStream} event
├── it should emit a {RenounceLockupStream} event
└── it should emit a {MetadataUpdate} event
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ abstract contract SetNFTDescriptor_Integration_Basic_Test is Integration_Test, L
}

function test_SetNFTDescriptor_SameNFTDescriptor() external whenCallerAdmin {
// Expect a {SetNFTDescriptor} event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockup) });
emit SetNFTDescriptor(users.admin, nftDescriptor, nftDescriptor);
vm.expectEmit({ emitter: address(lockup) });
emit BatchMetadataUpdate({ _fromTokenId: 1, _toTokenId: lockup.nextStreamId() });

// Re-set the NFT descriptor.
lockup.setNFTDescriptor(nftDescriptor);
Expand All @@ -44,12 +46,14 @@ abstract contract SetNFTDescriptor_Integration_Basic_Test is Integration_Test, L
}

function test_SetNFTDescriptor_NewNFTDescriptor() external whenCallerAdmin {
// Deploy the new NFT descriptor.
// Deploy another NFT descriptor.
ISablierV2NFTDescriptor newNFTDescriptor = new SablierV2NFTDescriptor();

// Expect a {SetNFTDescriptor} event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockup) });
emit SetNFTDescriptor(users.admin, nftDescriptor, newNFTDescriptor);
vm.expectEmit({ emitter: address(lockup) });
emit BatchMetadataUpdate({ _fromTokenId: 1, _toTokenId: lockup.nextStreamId() });

// Set the new NFT descriptor.
lockup.setNFTDescriptor(newNFTDescriptor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ setNFTDescriptor.t.sol
└── when the caller is the admin
├── when the new NFT descriptor is the same as the current NFT descriptor
│ ├── it should re-set the NFT descriptor
│ └── it should emit a {SetNFTDescriptor} event
│ ├── it should emit a {SetNFTDescriptor} event
│ └── it should emit a {BatchMetadataUpdate} event
└── when the new NFT descriptor is not the same as the current NFT descriptor
├── it should set the new NFT descriptor
└── it should emit a {SetNFTDescriptor} event
├── it should emit a {SetNFTDescriptor} event
└── it should emit a {BatchMetadataUpdate} event
Loading

0 comments on commit b8a15ef

Please sign in to comment.