BUG - StackingERC721.unstake

[pldespaigne]

The Bug

I found a bug in the unstake() function of the StackingERC721 contract. The bug cause the VM to crash every time with an invalid opcode when executing unstake().

The code is like that :

for(uint i = tokens.length - 1; i >= 0; i--) {
  uint256 id = tokens[i]; // <-- This line crash every time !
  tokens.pop();
  if(isTokenA)
    tokenA.transferFrom(address(this), msg.sender, id);
  else
     tokenB.transferFrom(address(this), msg.sender, id);
}

The error is caused by i going out of the bounds of the tokens array. We can easily verify it like this:

import "hardhat/console.sol";

// ...

for(uint i = tokens.length - 1; i >= 0; i--) {
  console.log(i); // 3, 2, 1, 0, 115792089237316195423570985008687907853269984665640564039457584007913129639935
  uint256 id = tokens[i]; // <-- This line crash every time !
  tokens.pop();
  if(isTokenA)
    tokenA.transferFrom(address(this), msg.sender, id);
  else
     tokenB.transferFrom(address(this), msg.sender, id);
}

Proposed Fix

Simply patch the for loop by removing the = sign, like that : for(uint i = tokens.length - 1; i > 0; i--) {

[stalker474]

This is correct and the loop is indeed invalid, thanks for the fix!