defi-stake-yield-brownie-freecode
defi-stake-yield-brownie-freecode copied to clipboard
Stakers array could contain duplicate entries.
If we have set
uniqueTokensStaked[_user] = uniqueTokensStaked[_user] + 1;
before, and user calls stakeTokens
again with the same token, the
if (uniqueTokensStaked[msg.sender] == 1){
stakers.push(msg.sender);
}
uniqueTokensStaked
for the user will be 1 because it was set in the first staking request. Now msg.sender
will be pushed to the stakers array again as a duplicate.
PS: I have not tested this but it seems like an issue.
https://github.com/PatrickAlphaC/defi-stake-yield-brownie-freecode/blob/0dd62b0be06a7c580a6da047ec018a302b20dd7b/contracts/TokenFarm.sol#L94
Solution would be for the updateUniqueTokensStaked
function to return a boolean when the mapping value is actually set.
i think the code should be somthing like this.
a cleaner version of the code :)
That seems like one possible solution @mehdi077. Me myself I added a boolean return to the updateUniqueTokensStaked
method here.
function updateUniqueTokensStaked(address _user, address _token)
internal
returns (bool)
{
if (stakingBalance[_token][_user] <= 0) {
uniqueTokensStaked[_user] = uniqueTokensStaked[_user] + 1;
return true;
}
return false;
}
I check this boolean before pushing to the stakers array here
I'm not sure which is the better way. Maybe both are ok. I think for your solution @mehdi077, there might be extra gas cost by looking up the initial_amount. But I'm not sure how gas costs are calculated really.
I think you're right!