github/core
1
1
Fork 0
mirror of https://github.com/lens-protocol/core.git synced 2026-04-22 03:02:03 -04:00
Code Issues Packages Projects Releases Wiki Activity

feat: Executor permissions validation moved to LensHub when following

Browse Source
This commit is contained in:
donosonaumczuk
2023-01-18 18:30:54 -03:00
parent 16dc0b5a63
commit bcc60005b0
8 changed files with 32 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
contracts/core/FollowNFT.sol
View File

@@ -56,15 +56,11 @@ contract FollowNFT is HubRestricted, LensNFTBase, ERC2981CollectionRoyalties, IF
uint256 followerProfileId,
address executor,
address followerProfileOwner,
bool isExecutorApproved,
uint256 followTokenId
) external override onlyHub returns (uint256) {
if (_followTokenIdByFollowerProfileId[followerProfileId] != 0) {
revert AlreadyFollowing();
}
if (executor != followerProfileOwner && !isExecutorApproved) {
revert DoesNotHavePermissions();
}
uint256 followTokenIdAssigned = followTokenId;
address followTokenOwner;
uint256 currentFollowerProfileId;

3
contracts/interfaces/IFollowNFT.sol
View File

@@ -51,8 +51,6 @@ interface IFollowNFT {
* @param executor The address executing the operation, which is the signer in case of using meta-transactions or
* the sender otherwise.
* @param followerProfileOwner The address holding the follower profile.
* @param isExecutorApproved A boolean indicading whether the executor is an approved delegated executor of the
* follower profile's owner.
* @param followTokenId The ID of the follow token to be used for this follow operation. Zero if a new follow token
* should be minted.
*
@@ -62,7 +60,6 @@ interface IFollowNFT {
uint256 followerProfileId,
address executor,
address followerProfileOwner,
bool isExecutorApproved,
uint256 followTokenId
) external returns (uint256);

11
contracts/libraries/GeneralLib.sol
View File

@@ -162,6 +162,7 @@ library GeneralLib {
uint256[] calldata followTokenIds,
bytes[] calldata followModuleDatas
) external returns (uint256[] memory) {
GeneralHelpers.validateCallerIsOwnerOrDelegatedExecutor(followerProfileId);
return
InteractionHelpers.follow(
followerProfileId,
@@ -184,9 +185,10 @@ library GeneralLib {
returns (uint256[] memory)
{
address followerProfileOwner = GeneralHelpers.ownerOf(vars.followerProfileId);
address signer = vars.delegatedSigner == address(0)
? followerProfileOwner
: vars.delegatedSigner;
address signer = GeneralHelpers.getOriginatorOrDelegatedExecutorSigner(
followerProfileOwner,
vars.delegatedSigner
);
MetaTxHelpers.baseFollowWithSig(signer, vars);
return
InteractionHelpers.follow(
@@ -242,8 +244,7 @@ library GeneralLib {
}
function setBlockStatusWithSig(DataTypes.SetBlockStatusWithSigData calldata vars) external {
// Safe to use the `unsafeOwnerOf` as the signer can not be address zero
address blockerProfileOwner = GeneralHelpers.unsafeOwnerOf(vars.byProfileId);
address blockerProfileOwner = GeneralHelpers.ownerOf(vars.byProfileId);
address signer = GeneralHelpers.getOriginatorOrDelegatedExecutorSigner(
blockerProfileOwner,
vars.delegatedSigner

5
contracts/libraries/helpers/GeneralHelpers.sol
View File

@@ -187,10 +187,7 @@ library GeneralHelpers {
}
function validateCallerIsOwnerOrDelegatedExecutor(uint256 profileId) internal view {
// It's safe to use the `unsafeOwnerOf()` function here because the sender cannot be
// the zero address, the dispatcher is cleared on burn and the zero address cannot approve
// a delegated executor.
address owner = unsafeOwnerOf(profileId);
address owner = ownerOf(profileId);
if (msg.sender != owner) {
validateDelegatedExecutor(owner, msg.sender);
}

4
contracts/libraries/helpers/InteractionHelpers.sol
View File

@@ -45,7 +45,6 @@ library InteractionHelpers {
) {
revert Errors.ArrayMismatch();
}
bool isExecutorApproved = GeneralHelpers.isExecutorApproved(followerProfileOwner, executor);
uint256[] memory followTokenIdsAssigned = new uint256[](idsOfProfilesToFollow.length);
uint256 i;
while (i < idsOfProfilesToFollow.length) {
@@ -61,7 +60,6 @@ library InteractionHelpers {
followerProfileId,
executor,
followerProfileOwner,
isExecutorApproved,
idsOfProfilesToFollow[i],
followTokenIds[i],
followModuleDatas[i]
@@ -339,7 +337,6 @@ library InteractionHelpers {
uint256 followerProfileId,
address executor,
address followerProfileOwner,
bool isExecutorApproved,
uint256 idOfProfileToFollow,
uint256 followTokenId,
bytes calldata followModuleData
@@ -373,7 +370,6 @@ library InteractionHelpers {
followerProfileId,
executor,
followerProfileOwner,
isExecutorApproved,
followTokenId
);

43
test/foundry/FollowNFTTest.t.sol
View File

@@ -72,7 +72,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: MINT_NEW_TOKEN
});
}
@@ -85,7 +84,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: alreadyFollowingProfileId,
executor: alreadyFollowingProfileOwner,
followerProfileOwner: alreadyFollowingProfileOwner,
isExecutorApproved: false,
followTokenId: MINT_NEW_TOKEN
});
}
@@ -104,7 +102,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: unexistentTokenId
});
}
@@ -113,25 +110,7 @@ contract FollowNFTTest is BaseTest, ERC721Test {
// Follow - Minting new token - Negatives
//////////////////////////////////////////////////////////
function testCannotFollowMintingNewTokenIfExecutorIsNotTheProfileOwnerOrHisApprovedExecutor(
address executor
) public {
vm.assume(executor != followerProfileOwner);
vm.assume(executor != address(0));
vm.assume(!hub.isDelegatedExecutorApproved(followerProfileOwner, executor));
vm.prank(address(hub));
vm.expectRevert(IFollowNFT.DoesNotHavePermissions.selector);
followNFT.follow({
followerProfileId: followerProfileId,
executor: executor,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: MINT_NEW_TOKEN
});
}
// No negatives when minting a new token, all the failing cases will occur at LensHub level. See `FollowTest.t.sol`.
//////////////////////////////////////////////////////////
// Follow - Minting new token - Scenarios
@@ -144,7 +123,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: MINT_NEW_TOKEN
});
@@ -158,7 +136,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: MINT_NEW_TOKEN
});
@@ -179,7 +156,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: MINT_NEW_TOKEN
});
@@ -198,7 +174,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: MINT_NEW_TOKEN
});
@@ -231,7 +206,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: executor,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: followTokenId
});
}
@@ -252,7 +226,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: alreadyFollowingProfileOwner,
followerProfileOwner: alreadyFollowingProfileOwner,
isExecutorApproved: false,
followTokenId: followTokenId
});
@@ -273,7 +246,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: alreadyFollowingProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: true,
followTokenId: followTokenId
});
@@ -298,7 +270,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: followTokenId
});
@@ -327,7 +298,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: executorAsApprovedDelegatee,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: true,
followTokenId: followTokenId
});
@@ -362,7 +332,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: executor,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: followTokenId
});
}
@@ -388,7 +357,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: followTokenId
});
@@ -418,7 +386,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: executorAsApprovedDelegatee,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: true,
followTokenId: followTokenId
});
@@ -452,7 +419,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: executorAsApprovedDelegatee,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: true,
followTokenId: followTokenId
});
@@ -479,7 +445,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: followTokenId
});
@@ -510,7 +475,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: executorAsApprovedDelegatee,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: true,
followTokenId: followTokenId
});
@@ -538,7 +502,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: followTokenId
});
@@ -570,7 +533,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: executorAsApprovedDelegatee,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: true,
followTokenId: followTokenId
});
@@ -606,7 +568,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: alreadyFollowingProfileId,
executor: alreadyFollowingProfileOwner,
followerProfileOwner: alreadyFollowingProfileOwner,
isExecutorApproved: false,
followTokenId: followTokenId
});
@@ -890,7 +851,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: MINT_NEW_TOKEN
});
@@ -1025,7 +985,6 @@ contract FollowNFTTest is BaseTest, ERC721Test {
followerProfileId: followerProfileId,
executor: followerProfileOwner,
followerProfileOwner: followerProfileOwner,
isExecutorApproved: false,
followTokenId: MINT_NEW_TOKEN
});

37
test/foundry/FollowTest.t.sol
View File

@@ -95,6 +95,27 @@ contract FollowTest is BaseTest, AssumptionHelpers {
});
}
function testCannotFollowIfExecutorIsNotTheProfileOwnerOrHisApprovedExecutor(uint256 executorPk)
public
{
vm.assume(_isValidPk(executorPk));
address executor = vm.addr(executorPk);
vm.assume(executor != address(0));
vm.assume(executor != followerProfileOwner);
vm.assume(!hub.isDelegatedExecutorApproved(followerProfileOwner, executor));
vm.expectRevert(Errors.ExecutorInvalid.selector);
_follow({
pk: executorPk,
isFollowerProfileOwner: false,
followerProfileId: followerProfileId,
idsOfProfilesToFollow: _toUint256Array(targetProfileId),
followTokenIds: _toUint256Array(MINT_NEW_TOKEN),
datas: _toBytesArray('', '')
});
}
function testCannotFollowIfAmountOfTokenIdsPassedDiffersFromAmountOfProfilesToFollow() public {
vm.expectRevert(Errors.ArrayMismatch.selector);
@@ -235,13 +256,7 @@ contract FollowTest is BaseTest, AssumptionHelpers {
targetFollowNFTAddress,
abi.encodeCall(
followNFT.follow,
(
followerProfileId,
followerProfileOwner,
followerProfileOwner,
false,
MINT_NEW_TOKEN
)
(followerProfileId, followerProfileOwner, followerProfileOwner, MINT_NEW_TOKEN)
)
);
@@ -304,13 +319,7 @@ contract FollowTest is BaseTest, AssumptionHelpers {
targetFollowNFTAddress,
abi.encodeCall(
followNFT.follow,
(
followerProfileId,
approvedDelegatedExecutor,
followerProfileOwner,
true,
MINT_NEW_TOKEN
)
(followerProfileId, approvedDelegatedExecutor, followerProfileOwner, MINT_NEW_TOKEN)
)
);

17
test/foundry/SetBlockStatusTest.t.sol
View File

@@ -62,7 +62,7 @@ contract SetBlockStatusTest is BaseTest, AssumptionHelpers {
vm.prank(statusSetterProfileOwner);
hub.burn(statusSetterProfileId);
vm.expectRevert(Errors.ExecutorInvalid.selector);
vm.expectRevert(Errors.TokenDoesNotExist.selector);
_setBlockStatus({
pk: statusSetterProfileOwnerPk,
@@ -359,21 +359,6 @@ contract SetBlockStatusMetaTxTest is SetBlockStatusTest, MetaTxNegatives {
});
}
function testCannotSetBlockStatusIfSetterProfileDoesNotExist() public override {
vm.prank(statusSetterProfileOwner);
hub.burn(statusSetterProfileId);
vm.expectRevert(Errors.SignatureInvalid.selector);
_setBlockStatus({
pk: statusSetterProfileOwnerPk,
isStatusSetterProfileOwner: true,
byProfileId: statusSetterProfileId,
idsOfProfilesToSetBlockStatus: _toUint256Array(blockeeProfileId),
blockStatus: _toBoolArray(true)
});
}
function testCannotSetBlockStatusIfNotOwnerOrApprovedDelegatedExecutorOfSetterProfile(
uint256 nonOwnerNorDelegatedExecutorPk
) public override {