[audit] #8: _isContract optimization

[stevieraykatz]

This PR removes the assembly method _isContract and instead just uses address.code.legnth. The ~40 gas saved by the assembly does not justify the readability hit.

From Spearbit:
Description
One can avoid extra operations by using a named return variable.

Recommendation
Use a named return variable and let the compiler solc take care of the conversion to bool:

function _isContract(address addr) internal view returns (bool result) {
    assembly {
        result := extcodesize(addr)
    }
}

[wilsoncusack]

What's the context on this check? a contract could later be deployed to this address?

[stevieraykatz]

If a user wants to set the reverse resolver name for a contract that they own, they can call this as the owner instead of needing to add reverse claiming logic to their contract logic.

[wilsoncusack]

Isn't this check superfluous? The owner() call / check will fail if its an EOA?

[stevieraykatz]

This reverts with a generic EVMError which isn't caught by the Try statement. Trace from a targeted test case:

[12293] TryTest::test_noCheck_noOwner()
    ├─ [0] VM::addr(<pk>) [staticcall]
    │   └─ ← [Return] noOwner: [0x28179DE316fe4178155beB9FdE329eE3ddA9C8D8]
    ├─ [0] VM::label(noOwner: [0x28179DE316fe4178155beB9FdE329eE3ddA9C8D8], "noOwner")
    │   └─ ← [Return] 
    ├─ [3101] Try::ownsContractNoCheck(noOwner: [0x28179DE316fe4178155beB9FdE329eE3ddA9C8D8]) [staticcall]
    │   ├─ [0] noOwner::owner() [staticcall]
    │   │   └─ ← [Stop] 
    │   └─ ← [Revert] EvmError: Revert
    └─ ← [Revert] EvmError: Revert

[PASS] test_withCheck_noOwner() (gas: 12733)
Traces:
  [12733] TryTest::test_withCheck_noOwner()
    ├─ [0] VM::addr(<pk>) [staticcall]
    │   └─ ← [Return] noOwner: [0x28179DE316fe4178155beB9FdE329eE3ddA9C8D8]
    ├─ [0] VM::label(noOwner: [0x28179DE316fe4178155beB9FdE329eE3ddA9C8D8], "noOwner")
    │   └─ ← [Return] 
    ├─ [2985] Try::ownsContractWithCheck(noOwner: [0x28179DE316fe4178155beB9FdE329eE3ddA9C8D8]) [staticcall]
    │   └─ ← [Return] false
    ├─ [0] VM::assertFalse(false) [staticcall]
    │   └─ ← [Return] 
    └─ ← [Stop]