StakeTransfererFeature Static Analysis Findings

STF-01S: Inapplicacy of Checks-Effects-Interactions Pattern

Description:

The linked statements conduct external calls with values retrieved from storage that are zeroed out after they have been utilized by the external call.

Example:

Copy
38StakeReceiver(transferTo).delegateStake(msg.sender, user.amountStaked);
39
40totalStaked = totalStaked.sub(user.amountStaked);
41user.amountStaked = 0;
42
43for (uint256 i = 0; i < rewardsTokens.length; i++) {
44    user.tokensOwed[i] = 0;
45    user.rewardDebt[i] = 0;
46}

Recommendation:

As external calls can lead to a re-entrancy, it is always best practice to apply the Checks-Effects-Interactions pattern. In the linked case, the code should store the variable to a temporary in-memory slot, zero it out and then perform the external call it is meant to accompany. The severity of this finding has been labelled as "minor" due to the transferTo being whitelisted by an owner-based ACL, however, the pattern should be applied regardless to ensure security by design.

Alleviation:

The Checks-Effects-Interactions pattern was applied by first zeroing out the amount that is being delegated before it is used by the external call.