Consensys / Tokens

Ethereum Token Contracts
MIT License
2.06k stars 1.19k forks source link

EIP20Factory.sol: function verifyEIP20 is broken #119

Closed MosheStauber closed 6 years ago

MosheStauber commented 6 years ago

`//verifies if a contract that has been deployed is a Human Standard Token. //NOTE: This is a very expensive function, and should only be used in an eth_call. ~800k gas function verifyEIP20(address _tokenContract) public view returns (bool) { bytes memory fetchedTokenByteCode = codeAt(_tokenContract);

    if (fetchedTokenByteCode.length != EIP20ByteCode.length) {
        return false; //clear mismatch
    }

  //starting iterating through it if lengths match
    for (uint i = 0; i < fetchedTokenByteCode.length; i++) {
        if (fetchedTokenByteCode[i] != EIP20ByteCode[i]) {
            return false;
        }
        return true;
    }
}`

Shouldn't the return true; statement be outside of the for-loop? The function will return true on the first match, even if the rest dont match.

simondlr commented 6 years ago

You are correct @notACodeMonkey1. It was unexpectedly introduced after linting & wasn't properly rechecked.

Here's the PR that initially took out the bracket. https://github.com/ConsenSys/Tokens/pull/106

The factory got faster because the linting screwed up the brackets. @maurelian @skmgoldin. :)

Thanks for the catch & keen eye.

skmgoldin commented 6 years ago

Shoot. Thanks for the catch indeed.