Add check for invalid target. Closes #24

[gbalabasquer]

[gbalabasquer]

@rainbreak what do you think about this change? apparently if you delegatecall to an address that doesn't have code it doesn't revert, just passes. I'm not sure if that is ok or not, but as we are checking if the address != 0 we might also check if the target is actually a contract. cc @levity

[rainbreak]

With constantinople you can also consider extcodehash

[gbalabasquer]

With constantinople you can also consider extcodehash

Yeah, I was planning to take care of it in another PR.

[gbalabasquer]

ahh @rainbreak you were meaning for this specific check, I thought you were talking about the possibility of simplifying the cache with the new opcode. Anyway I think using extcodesize is quite clear for this case.

[gbalabasquer]

@rainbreak can we merge this?

[rainbreak]

With constantinople you can also consider extcodehash

Yeah, I was planning to take care of it in another PR.

Are you still considering extcodehash?

[gbalabasquer]

With constantinople you can also consider extcodehash

Yeah, I was planning to take care of it in another PR.

Are you still considering extcodehash?

In the beginning I thought you were suggesting extcodehash for the cache replacement, but then I realized that it was to replace this extcodesize. The only benefit I see using the new opcode is the 300 gas saving. In contraposition the if condition will need check if the result is != 0 and != the empty bytecode hash. I think is clearer for understanding the case using the size opcode. Anyway if you prefer the other, we can wait for Constantinople (if this time succeeds :P).

[PaulRBerg]

As per this other discussion in the OpenZeppelin repo, the gas cost for EXTCODEHASH got set to 700 in EIP-1884. It is not cheaper to use it instead of EXTCODESIZE now.