ensdomains / ens-contracts

The core contracts of the ENS protocol
https://ens.domains/
MIT License
565 stars 384 forks source link

ENS Contracts relying on older Openzeppelin version #338

Closed pikonha closed 6 months ago

pikonha commented 6 months ago

According to the package.json the ENS contracts rely on Openzeppelin ^v4.1.0.

The problem is that OffchainDNSResolver, LowLevelCallUtils and UniversalResolve call the isContract function from an address which isn't supported on the Openzepplin contract since version 2.x.x (currently on 5.x.x), see docs.

Error (9582): Member "isContract" not found or not visible after argument-dependent lookup in address.
  --> lib/ens-contracts/contracts/utils/LowLevelCallUtils.sol:38:13:
   |
38 |             target.isContract(),
   |             ^^^^^^^^^^^^^^^^^

I've tried installing version 4.x.x but it also doesn't expose this function, which led me to monkey patch the latest version of Openzeppelin to be able to deploy the following code locally:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Script, console} from "forge-std/Script.sol";

import "../src/Helper.sol";
import "@ens-contracts/registry/ENSRegistry.sol";
import "@ens-contracts/reverseRegistrar/ReverseRegistrar.sol";
import "@ens-contracts/utils/UniversalResolver.sol";
import {PublicResolver, INameWrapper} from "@ens-contracts/resolvers/PublicResolver.sol";

contract PublicResolverScript is Script, ENSHelper {
    function run() external {
        uint256 privateKey = vm.envUint("PRIVATE_KEY");
        address publicKey = vm.addr(privateKey);
        vm.startBroadcast(privateKey);

        ENSRegistry registry = new ENSRegistry();
        string[] memory urls = new string[](1);
        urls[0] = "localhost:8080";
        new UniversalResolver(address(registry), urls);
        ReverseRegistrar registrar = new ReverseRegistrar(registry);

        // .reverse
        registry.setSubnodeOwner(rootNode, labelhash("reverse"), publicKey);
        // addr.reverse
        registry.setSubnodeOwner(namehash("reverse"), labelhash("addr"), address(registrar));

        PublicResolver resolver = new PublicResolver(registry, INameWrapper(publicKey), publicKey, address(registrar));
        registrar.setDefaultResolver(address(resolver));

        // .eth
        registry.setSubnodeRecord(rootNode, labelhash("eth"), publicKey, address(resolver), 100000);
        // public.eth
        registry.setSubnodeRecord(namehash("eth"), labelhash("public"), publicKey, address(resolver), 100000);

        // inital properties
        resolver.setAddr(namehash("public.eth"), address(1));
        resolver.setName(namehash("public.eth"), "blockful");
        resolver.setText(namehash("public.eth"), "avatar", "ipfs://QmdzG4h3KZjcyLsDaVxuFGAjYi7MYN4xxGpU9hwSj1c3CQ"); // blockful.jpeg

        vm.stopBroadcast();
    }
}

Is there any official workaround for it or am I doing something wrong?

The monkey patch I had to do was to add the following function on the Openzeppelin Address library:

/**
  * @dev This method relies on extcodesize/address.code.length, which returns 0
  * for contracts in construction, since the code is only stored at the end
  * of the constructor execution.
*/
function isContract(address account) internal view returns (bool) {
  return account.code.length > 0;
}
pikonha commented 6 months ago

Installing the Openzepellin v2.5.1 causes other errors such as the following caused by the Ownable contract been placed in a different directory.

Error:
failed to resolve file: "/Users/lpicollo/dev/blockful/external-resolver/packages/contracts/lib/openzeppelin-contracts/access/Ownable.sol": No such file or directory (os error 2); check configured remappings.
    --> "/Users/lpicollo/dev/blockful/external-resolver/packages/contracts/lib/ens-contracts/contracts/reverseRegistrar/ReverseRegistrar.sol"
        "@openzeppelin/contracts/access/Ownable.sol"
pikonha commented 6 months ago

fixed in [this issue](fixed in this issue)