ToB: Staking contract implementation is deeply coupled with other Uniswap components

[ewilz]

Staking contract implementation is deeply coupled with other Uniswap components It is worth mentioning that the correct behavior of the staking contract directly depends on core and peripheral components which are outside the scope of this audit. For instance, to stake an incentive, the NFTPosition and the pool are queried:

   /// @dev Stakes a deposited token without doing an ownership check
    function _stakeToken(IncentiveKey memory key, uint256 tokenId) private {
        ...
        (IUniswapV3Pool pool, int24 tickLower, int24 tickUpper, uint128 liquidity) =
            NFTPositionInfo.getPositionInfo(factory, nonfungiblePositionManager, tokenId);
        require(pool == key.pool, 'UniswapV3Staker::stakeToken: token pool is not the incentive pool');
        require(liquidity > 0, 'UniswapV3Staker::stakeToken: cannot stake token with 0 liquidity');
        deposits[tokenId].numberOfStakes++;
        incentives[incentiveId].numberOfStakes++;
        (, uint160 secondsPerLiquidityInsideX128, ) = pool.snapshotCumulativesInside(tickLower, tickUpper);
        …
    }

Figure D.1: Part of the _stakeToken function Additionally, the correct computation of the rewards and the total seconds claimed also require an external call to a uniswap pool:

    /// @inheritdoc IUniswapV3Staker
    function unstakeToken(IncentiveKey memory key, uint256 tokenId) external override {
        ...
        (, uint160 secondsPerLiquidityInsideX128, ) =
            key.pool.snapshotCumulativesInside(deposit.tickLower, deposit.tickUpper);
        (uint256 reward, uint160 secondsInsideX128) =
            RewardMath.computeRewardAmount(
                incentive.totalRewardUnclaimed,
                incentive.totalSecondsClaimedX128,
                key.startTime,
                key.endTime,
                liquidity,
                secondsPerLiquidityInsideInitialX128,
                secondsPerLiquidityInsideX128,
                block.timestamp
            );
         …
    }

Figure D.2: Part of the unstakeToken function As expected, to review the correctness of such interactions without a clear specification of the properties of such components is challenging. To enhance either manual and tool-assisted security review (fuzzing, symbolic execution or even formal verification), consider specifying the properties from these components. Additionally, we strongly recommend performing security reviews including all the mentioned functions to make sure they have the expected properties.

[riat-cloud]

Welcome to v3 staking dilema, how to determine rewardPerToken or rewardPerSecond, if the user can modify his position in the NFT without touching the staking contract, you could be rewarding empty or closed NFT positions if you cannot see how much liquidity the user is providing or if the position moves out of range and is no longer actively contributing to liquidity... The tight coupling...this thing secondsPerLiquidityInsideX128 determines how much liquidity the user has contributed!. the solution here works , BUT i think its rewarding everyone who is in the uniswap pool regardless if they are staking or not,

[uniearn]

We are going to start our fair launch event in a few days. It seems we will be the first project to use the officially deployed V3 Staker contract and also the first to develop a UI for this.

Check us out: www.uniearn.fi