chore: update contracts, readme and tests

packages/lsp26-contracts/README.md

@@ -1,51 +1,17 @@
-# LSP Package Template
+# LSP26 Following System · [![npm version](https://img.shields.io/npm/v/@lukso/lsp26-contracts.svg?style=flat)](https://www.npmjs.com/package/@lukso/lsp26-contracts)
 
-This project can be used as a skeleton to build a package for a LSP implementation in Solidity (LUKSO Standard Proposal)
+Package for the LSP26 Following System standard.
 
-It is based on Hardhat.
-
-## How to setup a LSP as a package?
-
-1. Copy the `template/` folder and paste it under the `packages/` folder. Then rename this `template/` folder that you copied with the LSP name.
-
-You can do so either:
-
-- manually, by copying the folder and pasting it inside `packages` and then renaming it.
-  or
-- by running the following command from the root of the repository.
+## Installation
 
 ```bash
-cp -r template packages/lsp-name
+npm i @lukso/lsp26-contracts
 ```
 
-2. Update the `"name"` and `"description"` field inside the `package.json` for this LSP package you just created.
-
-3. Setup the dependencies
-
-If this LSP uses external dependencies like `@openzeppelin/contracts`, put them under `dependencies` with the version number.
-
-```json
-"@openzeppelin/contracts": "^4.9.3"
-```
+## Available Constants & Types
 
-If this LSP uses other LSP as dependencies, put each LSP dependency as shown below. This will use the current code in the package:
+The `@lukso/lsp26-contracts` npm package contains useful constants such as InterfaceIds, and specific constants related to the LSP26 Standard. You can import and access them as follow:
 
-```json
-"@lsp2": "*"
+```js
+import { INTERFACE_ID_LSP26, LSP26_TYPE_IDS } from "@lukso/lsp26-contracts";
 ```
-
-4. Setup the npm commands for linting, building, testing, etc... under the `"scripts"` in the `package.json`
-
-5. Test that all commands you setup run successfully
-
-By running the commands below, your LSP package should come up in the list of packages that Turbo is running this command for.
-
-```bash
-turbo build
-turbo lint
-turbo lint:solidity
-turbo test
-turbo test:foundry
-```
-
-6. Finally update the relevant information in the `README.md` file in the folder of the newly created package, such as the title at the top, some description, etc...

packages/lsp26-contracts/constants.ts

@@ -1,4 +1,9 @@
-// Define your constants to be exported here
+export const INTERFACE_ID_LSP26 = '0xc52d6008';
 
-// example
-export const LSPN_CONSTANT_VALUE = 123;
+export const LSP26_TYPE_IDS = {
+  // keccak256('LSP26FollowerSystem_FollowNotification')
+  _TYPEID_LSP26_FOLLOW: '0x71e02f9f05bcd5816ec4f3134aa2e5a916669537ec6c77fe66ea595fabc2d51a',
+
+  // keccak256('LSP26FollowerSystem_UnfollowNotification')
+  _TYPEID_LSP26_UNFOLLOW: '0x9d3c0b4012b69658977b099bdaa51eff0f0460f421fba96d15669506c00d1c4f',
+};

packages/lsp26-contracts/contracts/ILSP26FollowingSystem.sol

@@ -13,23 +13,23 @@ interface ILSP26FollowingSystem {
     event Unfollow(address unfollower, address addr);
 
     /// @notice Follow an specific address.
-    /// @custom:events {Follow} event when following an address.
     /// @param addr The address to start following.
+    /// @custom:events {Follow} event when following an address.
     function follow(address addr) external;
 
     /// @notice Follow a list of addresses.
-    /// @custom:events {Follow} event when following each address in the list.
     /// @param addresses The list of addresses to follow.
+    /// @custom:events {Follow} event when following each address in the list.
     function followBatch(address[] memory addresses) external;
 
     /// @notice Unfollow a specific address.
-    /// @custom:events {Unfollow} event when unfollowing an address.
     /// @param addr The address to stop following.
+    /// @custom:events {Unfollow} event when unfollowing an address.
     function unfollow(address addr) external;
 
     /// @notice Unfollow a list of addresses.
-    /// @custom:events {Follow} event when unfollowing each address in the list.
     /// @param addresses The list of addresses to unfollow.
+    /// @custom:events {Follow} event when unfollowing each address in the list.
     function unfollowBatch(address[] memory addresses) external;
 
     /// @notice Check if an address is following a specific address.

packages/lsp26-contracts/contracts/LSP26Errors.sol

@@ -3,8 +3,6 @@ pragma solidity ^0.8.17;
 
 error LSP26CannotSelfFollow();
 
-error LSP26CannotSelfUnfollow();
-
 error LSP26AlreadyFollowing(address addr);
 
 error LSP26NotFollowing(address addr);

packages/lsp26-contracts/contracts/LSP26FollowingSystem.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
 pragma solidity ^0.8.17;
 
-// interafces
+// interfaces
 import {ILSP26FollowingSystem} from "./ILSP26FollowingSystem.sol";
 import {
     ILSP1UniversalReceiver
@@ -27,7 +27,6 @@ import {
 // errors
 import {
     LSP26CannotSelfFollow,
-    LSP26CannotSelfUnfollow,
     LSP26AlreadyFollowing,
     LSP26NotFollowing
 } from "./LSP26Errors.sol";
@@ -46,7 +45,7 @@ contract LSP26FollowingSystem is ILSP26FollowingSystem {
 
     // @inheritdoc ILSP26FollowingSystem
     function followBatch(address[] memory addresses) public {
-        for (uint256 index = 0; index < addresses.length; index++) {
+        for (uint256 index = 0; index < addresses.length; ++index) {
             _follow(addresses[index]);
         }
     }
@@ -58,7 +57,7 @@ contract LSP26FollowingSystem is ILSP26FollowingSystem {
 
     // @inheritdoc ILSP26FollowingSystem
     function unfollowBatch(address[] memory addresses) public {
-        for (uint256 index = 0; index < addresses.length; index++) {
+        for (uint256 index = 0; index < addresses.length; ++index) {
             _unfollow(addresses[index]);
         }
     }
@@ -87,9 +86,11 @@ contract LSP26FollowingSystem is ILSP26FollowingSystem {
         uint256 startIndex,
         uint256 endIndex
     ) public view returns (address[] memory) {
-        address[] memory followings = new address[](endIndex - startIndex);
+        uint256 sliceLength = endIndex - startIndex;
 
-        for (uint256 index = 0; index < endIndex - startIndex; index++) {
+        address[] memory followings = new address[](sliceLength);
+
+        for (uint256 index = 0; index < sliceLength; ++index) {
             followings[index] = _followingsOf[addr].at(startIndex + index);
         }
 
@@ -102,9 +103,11 @@ contract LSP26FollowingSystem is ILSP26FollowingSystem {
         uint256 startIndex,
         uint256 endIndex
     ) public view returns (address[] memory) {
-        address[] memory followers = new address[](endIndex - startIndex);
+        uint256 sliceLength = endIndex - startIndex;
+
+        address[] memory followers = new address[](sliceLength);
 
-        for (uint256 index = 0; index < endIndex - startIndex; index++) {
+        for (uint256 index = 0; index < sliceLength; ++index) {
             followers[index] = _followersOf[addr].at(startIndex + index);
         }
 
@@ -116,13 +119,16 @@ contract LSP26FollowingSystem is ILSP26FollowingSystem {
             revert LSP26CannotSelfFollow();
         }
 
-        if (_followingsOf[msg.sender].contains(addr)) {
+        bool isAdded = _followingsOf[msg.sender].add(addr);
+
+        if (!isAdded) {
             revert LSP26AlreadyFollowing(addr);
         }
 
-        _followingsOf[msg.sender].add(addr);
         _followersOf[addr].add(msg.sender);
 
+        emit Follow(msg.sender, addr);
+
         if (addr.supportsERC165InterfaceUnchecked(_INTERFACEID_LSP1)) {
             // solhint-disable no-empty-blocks
             try
@@ -131,24 +137,20 @@ contract LSP26FollowingSystem is ILSP26FollowingSystem {
                     abi.encodePacked(msg.sender)
                 )
             {} catch {}
-            // returns (bytes memory data) {} catch {}
         }
-
-        emit Follow(msg.sender, addr);
     }
 
     function _unfollow(address addr) internal {
-        if (msg.sender == addr) {
-            revert LSP26CannotSelfUnfollow();
-        }
+        bool isRemoved = _followingsOf[msg.sender].remove(addr);
 
-        if (!_followingsOf[msg.sender].contains(addr)) {
+        if (!isRemoved) {
             revert LSP26NotFollowing(addr);
         }
 
-        _followingsOf[msg.sender].remove(addr);
         _followersOf[addr].remove(msg.sender);
 
+        emit Unfollow(msg.sender, addr);
+
         if (addr.supportsERC165InterfaceUnchecked(_INTERFACEID_LSP1)) {
             // solhint-disable no-empty-blocks
             try
@@ -157,9 +159,6 @@ contract LSP26FollowingSystem is ILSP26FollowingSystem {
                     abi.encodePacked(msg.sender)
                 )
             {} catch {}
-            // returns (bytes memory data) {} catch {}
         }
-
-        emit Unfollow(msg.sender, addr);
     }
 }

packages/lsp26-contracts/contracts/mock/RevertOnFollow.sol

@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: Apache-2.0
+pragma solidity ^0.8.17;
+
+// interfaces
+import {
+    ILSP1UniversalReceiver
+} from "@lukso/lsp1-contracts/contracts/ILSP1UniversalReceiver.sol";
+
+contract RevertOnFollow is ILSP1UniversalReceiver {
+    function supportsInterface(bytes4) external pure returns (bool) {
+        return true;
+    }
+
+    function universalReceiver(
+        bytes32,
+        bytes memory
+    ) external payable returns (bytes memory) {
+        revert("Block LSP1 notifications");
+    }
+}

packages/lsp26-contracts/tests/LSP26FollowingSystem.test.ts

@@ -13,12 +13,16 @@ import {
   LSP26FollowingSystem__factory,
   LSP0ERC725Account,
   LSP0ERC725Account__factory,
+  RevertOnFollow__factory,
+  RevertOnFollow,
 } from '../types';
 
 describe('testing `LSP26FollowingSystem`', () => {
   let context: {
     followerSystem: LSP26FollowingSystem;
     followerSystemAddress: string;
+    revertOnFollow: RevertOnFollow;
+    revertOnFollowAddress: string;
     universalProfile: LSP0ERC725Account;
     owner: SignerWithAddress;
     singleFollowSigner: SignerWithAddress;
@@ -34,13 +38,18 @@ describe('testing `LSP26FollowingSystem`', () => {
     const followerSystemAddress = await followerSystem.getAddress();
     const universalProfile = await new LSP0ERC725Account__factory(owner).deploy(owner.address);
 
+    const revertOnFollow = await new RevertOnFollow__factory(owner).deploy();
+    const revertOnFollowAddress = await revertOnFollow.getAddress();
+
     const executeBatchFollowSigners = signers.slice(2, 12);
     const batchFollowSigners = signers.slice(12, 22);
     const multiFollowSigners = signers.slice(22, 10_022);
 
     context = {
       followerSystem,
       followerSystemAddress,
+      revertOnFollow,
+      revertOnFollowAddress,
       universalProfile,
       owner,
       singleFollowSigner,
@@ -66,13 +75,24 @@ describe('testing `LSP26FollowingSystem`', () => {
         .to.be.revertedWithCustomError(context.followerSystem, 'LSP26AlreadyFollowing')
         .withArgs(randomAddress);
     });
+
+    it('should not revert if follow recipient reverts inside the LSP1 hook', async () => {
+      await context.followerSystem.connect(context.owner).follow(context.revertOnFollowAddress);
+
+      expect(
+        await context.followerSystem.isFollowing(
+          context.owner.address,
+          context.revertOnFollowAddress,
+        ),
+      ).to.be.true;
+    });
   });
 
   describe('testing `unfollow(address)`', () => {
     it('should revert when unfollowing your own address', async () => {
-      await expect(
-        context.followerSystem.connect(context.owner).unfollow(context.owner.address),
-      ).to.be.revertedWithCustomError(context.followerSystem, 'LSP26CannotSelfUnfollow');
+      await expect(context.followerSystem.connect(context.owner).unfollow(context.owner.address))
+        .to.be.revertedWithCustomError(context.followerSystem, 'LSP26NotFollowing')
+        .withArgs(context.owner.address);
     });
 
     it('should revert when unfollowing an address that is not followed', async () => {
@@ -82,9 +102,20 @@ describe('testing `LSP26FollowingSystem`', () => {
         .to.be.revertedWithCustomError(context.followerSystem, 'LSP26NotFollowing')
         .withArgs(randomAddress);
     });
+
+    it('should not revert if unfollow recipient reverts inside the LSP1 hook', async () => {
+      await context.followerSystem.connect(context.owner).unfollow(context.revertOnFollowAddress);
+
+      expect(
+        await context.followerSystem.isFollowing(
+          context.owner.address,
+          context.revertOnFollowAddress,
+        ),
+      ).to.be.false;
+    });
   });
 
-  describe('gas tests', () => {
+  describe.skip('gas tests', () => {
     const gasCostResult: {
       followingGasCost?: number[];
       followCost?: number;