Replace inline assembly with Solidity in ERC165Checker

[frangio]

One of the few assembly blocks we have is in ERC165Checker.

https://github.com/OpenZeppelin/openzeppelin-solidity/blob/2ccc12b328e6d5d1d3faa5b99bec8d1de8b92fea/contracts/introspection/ERC165Checker.sol#L103-L120

I think we used assembly back then because Solidity didn't have .staticcall yet. It should be possible to replace it with .staticcall and abi.decode now.

[yopereir]

Would it be something like this?

function _callERC165SupportsInterface(address account, bytes4 interfaceId)
        private
        view
        returns (bool,bool)
    {
        bytes memory encodedParams = abi.encodeWithSelector(_INTERFACE_ID_ERC165, interfaceId);
        (bool success,bytes memory result) = account.staticcall(encodedParams);
        return (success,abi.decode(result,(bool)));
    }

Don't have a good grasp on the assembly code yet..

[frangio]

@RCYP Looks like it, yeah! Go for it.

[frangio]

Oh, actually, you're missing the gas limit. It should be account.staticcall.gas(30000)(encodedParams).

[yopereir]

After putting the gas limit it fails to compile if the function is declared as view. I get the following error:
TypeError: Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable

So should I remove the view declaration from the function (which will require removing view from all the calling functions as well)? It compiles successfully without adding the gas limit.

[frangio]

Hm, that seems like a bug? I can't see how a static call would modify state. I'll ask the solc developers.

[frangio]

Reported in ethereum/solidity#6901.

[frangio]

The Solidity bug above has been fixed in 0.5.10. However, bumping the minimum solc required for this contract to 0.5.10 is technically a breaking change, because we currently support 0.5.0.

@nventuro What do you think? I think it would be okay to bump the pragma for this specific contract. Removing the hand written assembly is definitely desirable.

[nventuro]

Technically our API guarantees explicitly exclude the Solidity version.

I think bumping the pragma on this one is fine. We should also update the truffle-config file to upgrade the one we use in our builds, since we're not running with 0.5.10 yet.

[yopereir]

Should i make theses changes to this file and truffle-config and make a PR then?

[nventuro]

@yopereir that'd be great! 😁

[yopereir]

ok, i'll do it over this weekend.