Internal _withdraw, reading from storage twice.

[code423n4]

Handle

evertkors

Vulnerability details

Impact

userAmountStaked[token][msg.sender] is read from storage twice on Staker.sol:925

Recommended Mitigation Steps

uint256 amountUser = userAmountStaked[token][msg.sender]
require(amountUser > 0, "nothing to withdraw")
_mintAccumulatedFloat(marketIndex, msg.sender);
userAmountStaked[token][msg.sender] = amountUser - amount

[JasoonS]

Unfortunately this is still needed. _mintAccumulatedFloatAndExecuteOutstandingShifts(marketIndex, msg.sender); modifies the userAmountStaked[token][msg.sender], so you do need to read that value again.

See below image:

[JasoonS]

Verdict: given the code this optimization introduces a bug. Final code indeed only reads userAmountStaked once, but that is due to other optimizations.

[JasoonS]

Not an exact duplicate of #25, #24 and #18 (different variable - but same optimization suggested -closing)

[0xean]

I am not sure where the divergence in code is here but the line that was reference by the warden is

require(userAmountStaked[token][msg.sender] > 0, "nothing to withdraw");
    _mintAccumulatedFloat(marketIndex, msg.sender);

    userAmountStaked[token][msg.sender] = userAmountStaked[token][msg.sender] - amount;

where the _mintAccumulatedFloat does not modify the userAmountStaked. Given this information, I am reopening the issue.