Implement ERC721 standard in both Linear and Pro contracts (#131)

* feat: ERC721

build: install "openzeppelin-contracts" lib
feat: inherit "ERC721" in "SablierV2Linear"
feat: inherit "ERC721" in "SablierV2Pro"
feat: mint NFT in the "_create" function
feat: add "isApprovedOrOwner" function
feat: change the modifier to "isAuthorizedForStream"
feat: add "tokenURI" function
perf: remove the recipient address from the "Stream" struct
refactor: swap the order of the checks in the "withdrawTo" function
test: change the struct recipient with the "users.recipient"
test: make recipient the default caller in the "cancel" function
test: order correctly the branches in the "cancel" tree
test: order correctly the branches in the "cancelAll" tree
test: order correctly the branches in the "withdraw" tree
test: when the caller is an approved third party in the "cancel" function
test: when the caller is an approved third party in the "cancelAll" funtion
test: when the caller is an approved third party in the "withdraw" function
test: when the caller is an approved third party in the "withdrawAll" function
test: when the caller is an approved third party in the "withdrawAllTo" function
test: when the caller is an approved third party in the "withdtawTo" function
test: when the recipient is no longer the owner of the stream in the "cancel" function
test: when the recipient is no longer the owner of the stream in the "withdraw" function
test: when the recipient is no longer the owner of the streams in the "cancelAll" function
test: when the recipient is no longer the owner of the streams in the "withdrawAll" function
test: when the recipient is no longer the owner of the streams in the "withdrawAllTo" function
test: when the recipient is no longer the owner of the stream in the "withrawTo" function
test: remove the local "withdrawAmount" variable from the test functions
test: change the new owner of the streams to "eve" when using "safeTransferFrom" function
test: "isApprovedOrOwner" function in the "SablierV2Linear" contract
test: "isApprovedOrOwner" function in the "SablierV2Pro" contract
test: correct comment about return value for the "isCancelable" function

* refactor: inherit from "IERC721" in "ISablierV2"

build: upgrade to latest "prb-math" lib
chore: add ".solhintignore" file
chore: improve wording in comments
docs: improve wording in NatSpec
refactor: change the ERC-721 name
refactor: name return args in "isApprovedOrOwner" and "tokenURI"
refactor: refer to "to" as "recipient"
refactor: use OpenZeppelin's ERC721 implementation
refactor: use "_isApprovedOrOwner" from OpenZeppelin
test: add dummy function implementations in "AbstractSablierV2"
test: delete "isApprovedOrOwner" tests

* fix: use "_ownerOf" ERC721 function

build: upgrade to "release-v4.8" of "openzeppelin-contracts"

* test: improve wording in testing trees

test: add new "operator" user
test: add new testing branches for un/authorized
test: delete superfluous warps in "withdraw" tests
test: move assertions for recipients at the bottom
test: refer to approved "third-party" as "operator"
test: rename "withdrawAmountZero" to "withdrawAmount"
test: refactor "CallerRecipient" to "OriginalRecipient"
test: use the new operator user in the "approved operator" tests

* refactor: isApprovedOrOwner

chore: add visibility keyword in function separators
refactor: inherit from ERC721 last
test: simplify AbstractSablierV2 contract

* chore: remove unnecessary imports

* style: order imports

chore: remove unnecessary imports

* refactor: burn NFT when stream is ended

test: update tests to check for burned NFT

* refactor: use "safeMint" in the "create" function

* test: correct explanatory comment

Co-authored-by: Paul Razvan Berg <hello@paulrberg.com>

.gitmodules

@@ -1,11 +1,15 @@
 [submodule "lib/forge-std"]
 	path = "lib/forge-std"
 	url = "https://github.com/foundry-rs/forge-std"
+[submodule "lib/openzeppelin-contracts"]
+  branch = "release-v4.8"
+	path = "lib/openzeppelin-contracts"
+	url = "https://github.com/OpenZeppelin/openzeppelin-contracts"
 [submodule "lib/prb-math"]
   branch = "staging"
 	path = "lib/prb-math"
 	url = "https://github.com/paulrberg/prb-math"
 [submodule "lib/prb-contracts"]
-  branch = "refactor/foundry"
+  branch = "main"
 	path = "lib/prb-contracts"
 	url = "https://github.com/paulrberg/prb-contracts"

.solhintignore

@@ -0,0 +1,3 @@
+# directories
+**/lib
+**/node_modules

remappings.txt

@@ -1,4 +1,5 @@
+@openzeppelin/contracts=lib/openzeppelin-contracts/contracts/
 @prb/contracts/=lib/prb-contracts/src/
-@prb/math/=lib/prb-math/contracts/
+@prb/math/=lib/prb-math/src/
 @sablier/v2-core/=src/
 forge-std/=lib/forge-std/src/

src/SablierV2.sol

@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: LGPL-3.0
 pragma solidity >=0.8.13;
 
-import { IERC20 } from "@prb/contracts/token/erc20/IERC20.sol";
-
 import { ISablierV2 } from "./interfaces/ISablierV2.sol";
 
 /// @title SablierV2
@@ -20,9 +18,10 @@ abstract contract SablierV2 is ISablierV2 {
                                       MODIFIERS
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @notice Checks that `msg.sender` is either the sender or the recipient of the stream.
-    modifier onlySenderOrRecipient(uint256 streamId) {
-        if (msg.sender != getSender(streamId) && msg.sender != getRecipient(streamId)) {
+    /// @notice Checks that `msg.sender` is the sender of the stream, an approved operator, or the owner of the
+    /// NFT (also known as the recipient of the stream).
+    modifier isAuthorizedForStream(uint256 streamId) {
+        if (msg.sender != getSender(streamId) && !_isApprovedOrOwner(msg.sender, streamId)) {
             revert SablierV2__Unauthorized(streamId, msg.sender);
         }
         _;
@@ -45,7 +44,7 @@ abstract contract SablierV2 is ISablierV2 {
     }
 
     /*//////////////////////////////////////////////////////////////////////////
-                                 CONSTANT FUNCTIONS
+                              PUBLIC CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @inheritdoc ISablierV2
@@ -58,7 +57,7 @@ abstract contract SablierV2 is ISablierV2 {
     function isCancelable(uint256 streamId) public view virtual override returns (bool cancelable);
 
     /*//////////////////////////////////////////////////////////////////////////
-                               NON-CONSTANT FUNCTIONS
+                            PUBLIC NON-CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @inheritdoc ISablierV2
@@ -110,10 +109,10 @@ abstract contract SablierV2 is ISablierV2 {
     function withdraw(uint256 streamId, uint256 amount)
         external
         streamExists(streamId)
-        onlySenderOrRecipient(streamId)
+        isAuthorizedForStream(streamId)
     {
-        address to = getRecipient(streamId);
-        _withdraw(streamId, to, amount);
+        address recipient = getRecipient(streamId);
+        _withdraw(streamId, recipient, amount);
     }
 
     /// @inheritdoc ISablierV2
@@ -136,8 +135,9 @@ abstract contract SablierV2 is ISablierV2 {
             recipient = getRecipient(streamId);
             sender = getSender(streamId);
             if (sender != address(0)) {
-                // Checks: the `msg.sender` is either the sender or the recipient of the stream.
-                if (msg.sender != sender && msg.sender != recipient) {
+                // Checks: the `msg.sender` is the sender or the stream, an approved operator, or the owner of the NFT
+                // (a.k.a. the recipient of the stream).
+                if (msg.sender != sender && !_isApprovedOrOwner(msg.sender, streamId)) {
                     revert SablierV2__Unauthorized(streamId, msg.sender);
                 }
 
@@ -177,8 +177,9 @@ abstract contract SablierV2 is ISablierV2 {
 
             // If the `streamId` points to a stream that does not exist, skip it.
             if (getSender(streamId) != address(0)) {
-                // Checks: the `msg.sender` is the recipient of the stream.
-                if (msg.sender != getRecipient(streamId)) {
+                // Checks: the `msg.sender` is either an approved operator or the owner of the NFT (a.k.a. the recipient
+                // of the stream).
+                if (!_isApprovedOrOwner(msg.sender, streamId)) {
                     revert SablierV2__Unauthorized(streamId, msg.sender);
                 }
 
@@ -199,23 +200,24 @@ abstract contract SablierV2 is ISablierV2 {
         address to,
         uint256 amount
     ) external streamExists(streamId) {
-        // Checks: the `msg.sender` is the recipient of the stream.
-        if (msg.sender != getRecipient(streamId)) {
-            revert SablierV2__Unauthorized(streamId, msg.sender);
-        }
-
         // Checks: the provided address to withdraw to is not zero.
         if (to == address(0)) {
             revert SablierV2__WithdrawZeroAddress();
         }
+
+        // Checks: the `msg.sender` is either an approved operator or the owner of the NFT (a.k.a. the recipient
+        // of the stream).
+        if (!_isApprovedOrOwner(msg.sender, streamId)) {
+            revert SablierV2__Unauthorized(streamId, msg.sender);
+        }
         _withdraw(streamId, to, amount);
     }
 
     /*//////////////////////////////////////////////////////////////////////////
                              INTERNAL CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @dev Checks the basic requiremenets for the `create` function.
+    /// @dev Checks the basic requirements for the `create` function.
     function _checkCreateArguments(
         address sender,
         address recipient,
@@ -244,6 +246,11 @@ abstract contract SablierV2 is ISablierV2 {
         }
     }
 
+    /// @dev Checks whether the spender is authorized to interact with the stream.
+    /// @param spender The spender to make the query for.
+    /// @param streamId The id of the stream to make the query for.
+    function _isApprovedOrOwner(address spender, uint256 streamId) internal view virtual returns (bool approvedOrOwner);
+
     /*//////////////////////////////////////////////////////////////////////////
                            INTERNAL NON-CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/

src/SablierV2Linear.sol

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: LGPL-3.0
 pragma solidity >=0.8.13;
 
+import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
 import { IERC20 } from "@prb/contracts/token/erc20/IERC20.sol";
 import { SafeERC20 } from "@prb/contracts/token/erc20/SafeERC20.sol";
 import { UD60x18, toUD60x18 } from "@prb/math/UD60x18.sol";
@@ -13,7 +14,8 @@ import { SablierV2 } from "./SablierV2.sol";
 /// @author Sablier Labs Ltd.
 contract SablierV2Linear is
     ISablierV2Linear, // one dependency
-    SablierV2 // one dependency
+    SablierV2, // two dependencies
+    ERC721("Sablier V2 Linear NFT", "SAB-V2-LIN") // six dependencies
 {
     using SafeERC20 for IERC20;
 
@@ -25,7 +27,7 @@ contract SablierV2Linear is
     mapping(uint256 => Stream) internal _streams;
 
     /*//////////////////////////////////////////////////////////////////////////
-                                 CONSTANT FUNCTIONS
+                            PUBLIC CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @inheritdoc ISablierV2Linear
@@ -40,7 +42,7 @@ contract SablierV2Linear is
 
     /// @inheritdoc ISablierV2
     function getRecipient(uint256 streamId) public view override(ISablierV2, SablierV2) returns (address recipient) {
-        recipient = _streams[streamId].recipient;
+        recipient = _ownerOf(streamId);
     }
 
     /// @inheritdoc ISablierV2
@@ -122,8 +124,13 @@ contract SablierV2Linear is
         cancelable = _streams[streamId].cancelable;
     }
 
+    /// @inheritdoc ERC721
+    function tokenURI(uint256 streamId) public view override streamExists(streamId) returns (string memory uri) {
+        uri = "";
+    }
+
     /*//////////////////////////////////////////////////////////////////////////
-                               NON-CONSTANT FUNCTIONS
+                            PUBLIC NON-CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @inheritdoc ISablierV2Linear
@@ -166,12 +173,26 @@ contract SablierV2Linear is
         streamId = _create(sender, recipient, depositAmount, token, startTime, cliffTime, stopTime, cancelable);
     }
 
+    /*//////////////////////////////////////////////////////////////////////////
+                           INTERNAL CONSTANT FUNCTIONS
+    //////////////////////////////////////////////////////////////////////////*/
+
+    /// @inheritdoc SablierV2
+    function _isApprovedOrOwner(address spender, uint256 streamId)
+        internal
+        view
+        override(ERC721, SablierV2)
+        returns (bool approvedOrOwner)
+    {
+        approvedOrOwner = ERC721._isApprovedOrOwner(spender, streamId);
+    }
+
     /*//////////////////////////////////////////////////////////////////////////
                            INTERNAL NON-CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @dev See the documentation for the public functions that call this internal function.
-    function _cancel(uint256 streamId) internal override onlySenderOrRecipient(streamId) {
+    function _cancel(uint256 streamId) internal override isAuthorizedForStream(streamId) {
         Stream memory stream = _streams[streamId];
 
         // Calculate the withdraw and the return amounts.
@@ -181,12 +202,17 @@ contract SablierV2Linear is
             returnAmount = stream.depositAmount - stream.withdrawnAmount - withdrawAmount;
         }
 
+        address recipient = getRecipient(streamId);
+
         // Effects: delete the stream from storage.
         delete _streams[streamId];
 
+        // Effects: burn the NFT.
+        _burn(streamId);
+
         // Interactions: withdraw the tokens to the recipient, if any.
         if (withdrawAmount > 0) {
-            stream.token.safeTransfer(stream.recipient, withdrawAmount);
+            stream.token.safeTransfer(recipient, withdrawAmount);
         }
 
         // Interactions: return the tokens to the sender, if any.
@@ -195,7 +221,7 @@ contract SablierV2Linear is
         }
 
         // Emit an event.
-        emit Cancel(streamId, stream.recipient, withdrawAmount, returnAmount);
+        emit Cancel(streamId, recipient, withdrawAmount, returnAmount);
     }
 
     /// @dev See the documentation for the public functions that call this internal function.
@@ -222,20 +248,22 @@ contract SablierV2Linear is
             revert SablierV2Linear__CliffTimeGreaterThanStopTime(cliffTime, stopTime);
         }
 
-        // Effects: create and store the stream.
+        // Effects: create the stream.
         streamId = nextStreamId;
         _streams[streamId] = Stream({
             cancelable: cancelable,
             cliffTime: cliffTime,
             depositAmount: depositAmount,
-            recipient: recipient,
             sender: sender,
             startTime: startTime,
             stopTime: stopTime,
             token: token,
             withdrawnAmount: 0
         });
 
+        // Effects: mint the NFT for the recipient.
+        _safeMint(recipient, streamId);
+
         // Effects: bump the next stream id.
         // We're using unchecked arithmetic here because this cannot realistically overflow, ever.
         unchecked {
@@ -294,9 +322,10 @@ contract SablierV2Linear is
         // Load the stream in memory, we will need it below.
         Stream memory stream = _streams[streamId];
 
-        // Effects: if this stream is done, save gas by deleting it from storage.
+        // Effects: if this stream is done, delete it from storage and burn the NFT.
         if (stream.depositAmount == stream.withdrawnAmount) {
             delete _streams[streamId];
+            _burn(streamId);
         }
 
         // Interactions: perform the ERC-20 transfer.