Gas in `RocketJoeStaking.sol:withdraw()`: `user.amount` should get cached and used for calculation

[code423n4]

Handle

Dravee

Vulnerability details

Impact

SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.

Proof of Concept

The code is as such (see @audit-info comments):

File: RocketJoeStaking.sol
116:     function withdraw(uint256 _amount) external {
117:         UserInfo storage user = userInfo[msg.sender];
118:         require(
119:             user.amount >= _amount, //@audit-info SLOAD
120:             "RocketJoeStaking: withdraw amount exceeds balance"
121:         );
122: 
123:         updatePool();
124: 
125:         uint256 pending = (user.amount * accRJoePerShare) / //@audit-info SLOAD
126:             PRECISION -
127:             user.rewardDebt;
128: 
129:         user.amount = user.amount - _amount;//@audit-info SLOAD
130:         user.rewardDebt = (user.amount * accRJoePerShare) / PRECISION;//@audit-info SLOAD
131: 
132:         _safeRJoeTransfer(msg.sender, pending);
133:         joe.safeTransfer(address(msg.sender), _amount);
134:         emit Withdraw(msg.sender, _amount);
135:     }

Tools Used

VS Code

Recommended Mitigation Steps

Cache user.amount and use it for calculation. I'd suggest:

File: RocketJoeStaking.sol
116:     function withdraw(uint256 _amount) external {
117:         UserInfo storage user = userInfo[msg.sender];
118:         uint256 _userAmount = user.amount;//@audit-info caching
119:         require(
120:             _userAmount >= _amount, //@audit-info MLOAD
121:             "RocketJoeStaking: withdraw amount exceeds balance"
122:         );
123: 
124:         updatePool();
125: 
126:         uint256 pending = (_userAmount * accRJoePerShare) / //@audit-info MLOAD
127:             PRECISION -
128:             user.rewardDebt;
129: 
130:         _userAmount -= _amount;//@audit-info memory calculation
131:         user.amount = _userAmount;//@audit-info MLOAD optimization
132:         user.rewardDebt = (_userAmount * accRJoePerShare) / PRECISION;//@audit-info MLOAD
133: 
134:         _safeRJoeTransfer(msg.sender, pending);
135:         joe.safeTransfer(address(msg.sender), _amount);
136:         emit Withdraw(msg.sender, _amount);
137:     }