I have a function to modify an array, if I comment out the .pop() line, it passes, if not, I get this error:

VM error.revert


function moveShipsToSpace(uint[] memory ships, uint16 fromSpace, uint16 toSpace) public{
     for(uint8 i= 0; i<ships.length; i++) { 
        require(containsInSpace(ships[i],fromSpace), "dont have that ship");            
        // Move the last element into the place to delete
        whichShipsAtSpace[msg.sender][fromSpace][i] = whichShipsAtSpace[msg.sender][fromSpace][whichShipsAtSpace[msg.sender][fromSpace].length - 1];
        // Delete
        // Insert in new location
        emit LogUint(ships[i]);

Also, if I pass a single number, it passes with .pop(), if I pass 2 or more it fails, so not sure what is the issue, please halp!


It actually fails here :

whichShipsAtSpace[msg.sender][fromSpace][i] = ...

because you assume that the index 'i' is a valid index for both the arrays ships and whichShipsAtSpace[msg.sender][fromSpace]. But it's not.

Ships will always stay the same length, but whichShipsAtSpace[msg.sender][fromSpace] length will decrease as you pop items out of it. At some point, the variable i is an out of range index for whichShipsAtSpace[msg.sender][fromSpace].

To fix this, you need to replace :

whichShipsAtSpace[msg.sender][fromSpace][i] = ...

by :

whichShipsAtSpace[msg.sender][fromSpace][shipIndexInFromSpace(ships[i])] = ...

Where shipIndexInFromSpace is a function returning the index of ships[i] inside the whichShipsAtSpace[msg.sender][fromSpace] array.

It's actually a good thing that your code crashed because it's not even doing what you want it to do in its current state. As the index 'i' points to an unknown ship and the order of whichShipsAtSpace[msg.sender][fromSpace][i] elements is being shifted, I beleive your code can potentially silently delete a different ship than the one you intended to.

  • You are the realest mvp of this site. Do you accept a tip in crypto? A coin with low fees like Dash would work. I tried to dm you, but apparently there is no such functionality here. Oct 27 at 22:22
  • Thanks but no need, really, that's what those websites are for.
    – hroussille
    Oct 28 at 7:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.