From 4df0bf6bcb0effa5f6b1982d6f3634f63517f285 Mon Sep 17 00:00:00 2001 From: b00ste Date: Fri, 2 Aug 2024 12:10:22 +0300 Subject: [PATCH] chore: update contracts, readme and tests --- packages/lsp26-contracts/README.md | 50 +++---------------- packages/lsp26-contracts/constants.ts | 11 ++-- .../contracts/ILSP26FollowingSystem.sol | 8 +-- .../lsp26-contracts/contracts/LSP26Errors.sol | 2 - .../contracts/LSP26FollowingSystem.sol | 41 ++++++++------- .../contracts/mock/RevertOnFollow.sol | 20 ++++++++ .../tests/LSP26FollowingSystem.test.ts | 39 +++++++++++++-- 7 files changed, 95 insertions(+), 76 deletions(-) create mode 100644 packages/lsp26-contracts/contracts/mock/RevertOnFollow.sol diff --git a/packages/lsp26-contracts/README.md b/packages/lsp26-contracts/README.md index 44c90d86b..1a17db82a 100644 --- a/packages/lsp26-contracts/README.md +++ b/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... diff --git a/packages/lsp26-contracts/constants.ts b/packages/lsp26-contracts/constants.ts index 7ae3bf471..eba63c6bd 100644 --- a/packages/lsp26-contracts/constants.ts +++ b/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', +}; diff --git a/packages/lsp26-contracts/contracts/ILSP26FollowingSystem.sol b/packages/lsp26-contracts/contracts/ILSP26FollowingSystem.sol index ceed49ff2..32d91f509 100644 --- a/packages/lsp26-contracts/contracts/ILSP26FollowingSystem.sol +++ b/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. diff --git a/packages/lsp26-contracts/contracts/LSP26Errors.sol b/packages/lsp26-contracts/contracts/LSP26Errors.sol index 93c39e4aa..7c404adb7 100644 --- a/packages/lsp26-contracts/contracts/LSP26Errors.sol +++ b/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); diff --git a/packages/lsp26-contracts/contracts/LSP26FollowingSystem.sol b/packages/lsp26-contracts/contracts/LSP26FollowingSystem.sol index 3ede7fa01..748ca0921 100644 --- a/packages/lsp26-contracts/contracts/LSP26FollowingSystem.sol +++ b/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); } } diff --git a/packages/lsp26-contracts/contracts/mock/RevertOnFollow.sol b/packages/lsp26-contracts/contracts/mock/RevertOnFollow.sol new file mode 100644 index 000000000..3670a4894 --- /dev/null +++ b/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"); + } +} diff --git a/packages/lsp26-contracts/tests/LSP26FollowingSystem.test.ts b/packages/lsp26-contracts/tests/LSP26FollowingSystem.test.ts index 33c563232..c8e897a9a 100644 --- a/packages/lsp26-contracts/tests/LSP26FollowingSystem.test.ts +++ b/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,6 +38,9 @@ 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); @@ -41,6 +48,8 @@ describe('testing `LSP26FollowingSystem`', () => { 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;