HugoDowsers - Reverting safeTransferFrom functions make the protocol not EIP 721 compliant

[sherlock-admin3]

HugoDowsers

Medium

Reverting safeTransferFrom functions make the protocol not EIP 721 compliant

Summary

The functions safeTransferFrom(address from, address to, uint256 tokenId, bytes memory _data) and safeTransferFrom(address from, address to, uint256 tokenId) are reverting in normal conditions.

These functions are not EIP 721 compliant :
https://eips.ethereum.org/EIPS/eip-721

"The transfer and accept functions’ documentation only specify conditions when the transaction MUST throw. Your implementation MAY also throw in other situations. This allows implementations to achieve interesting results:"

Vulnerability Detail

Add the following test to the MlumStaking.t.sol file

function testTransferPosition() public {
    _rewardToken.mint(address(_pool), 100_000_000);
    _stakingToken.mint(ALICE, 2 ether);

    vm.startPrank(ALICE);
    _stakingToken.approve(address(_pool), 1 ether);
    _pool.createPosition(1 ether, 1 days);
	
    vm.stopPrank();

    assertEq(ERC721(address(_pool)).ownerOf(1), ALICE);

	vm.prank(ALICE);
	_pool.safeTransferFrom(ALICE,BOB,1);
	vm.stopPrank();
}(

The result will the following :
Encountered 1 failing test in test/MlumStaking.t.sol:MlumStakingTest
[FAIL. Reason: ReentrancyGuardReentrantCall()] testTransferPosition() (gas: 559377)

Impact

Medium as VII.2 from :
https://docs.sherlock.xyz/audits/judging/judging#ii.-criteria-for-issue-severity

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/MlumStaking.sol#L336

Tool used

Foundry coverage.

Recommendation

Delete modifier

Duplicate #489