Nft.upaidRewards reset upon withdrawal. This feels a bit unfair if a user means to collect rewards while withdrawing NFT. If there are not enough rewards in the contract balance, the unpaidRewards will be deleted upon withdrawal with the line delete nftInfo[_nftId];. I'm not sure the user will have visibility into the contract's current balance, whether Native or token, prior to deciding to withdraw. Either way, I think it would be a bad user experience to withdraw and realize all of your pending rewards for a potentially long staking period were erased due to bad timing.
Consider adding an emergencyWithdraw method which explicitly states that pending rewards might be forfeited or keeping track of users' unpaid balances somewhere other than nftInfo[_nftId] which is deleted upon withdrawal. Optional ability for users to claim unpaid rewards AFTER withdrawal following an increase in the contract's balance.
https://github.com/code-423n4/2022-03-biconomy-findings/issues/129#issuecomment-1121127528
Warden: 0xDjango
Nft.upaidRewards reset upon withdrawal. This feels a bit unfair if a user means to collect rewards while withdrawing NFT. If there are not enough rewards in the contract balance, the unpaidRewards will be deleted upon withdrawal with the line
delete nftInfo[_nftId];
. I'm not sure the user will have visibility into the contract's current balance, whether Native or token, prior to deciding to withdraw. Either way, I think it would be a bad user experience to withdraw and realize all of your pending rewards for a potentially long staking period were erased due to bad timing.Link to Code
https://github.com/code-423n4/2022-03-biconomy/blob/04751283f85c9fc94fb644ff2b489ec339cd9ffc/contracts/hyphen/LiquidityFarming.sol#L243-L244
Steps to Fix
Consider adding an emergencyWithdraw method which explicitly states that pending rewards might be forfeited or keeping track of users' unpaid balances somewhere other than nftInfo[_nftId] which is deleted upon withdrawal. Optional ability for users to claim unpaid rewards AFTER withdrawal following an increase in the contract's balance.