Add more tests and some test performance improvements

* Add a lot more testing around AsyncENS. We can think about splitting AsyncEthereumTesterProvider tests off into their own CI job if they get too big but we should provide ample testing around the async implementation to make sure it's all wired up correctly.
* Cleanup for better readability and consistency across ENS and AsyncENS
* Keep async_ens_setup as module scope with a module-scoped event_loop fixture and add a module-scoped async_w3 for ens module

Author: fselmo

ens/async_ens.py

@@ -118,6 +118,60 @@ def fromWeb3(cls, w3: 'Web3', addr: ChecksumAddress = None) -> 'ENS':
         middlewares = w3.middleware_onion.middlewares
         return cls(provider, addr=addr, middlewares=middlewares)
 
+    def address(self, name: str) -> Optional[ChecksumAddress]:
+        """
+        Look up the Ethereum address that `name` currently points to.
+
+        :param str name: an ENS name to look up
+        :raises InvalidName: if `name` has invalid syntax
+        """
+        return cast(ChecksumAddress, self._resolve(name, 'addr'))
+
+    def setup_address(
+        self,
+        name: str,
+        address: Union[Address, ChecksumAddress, HexAddress] = cast(ChecksumAddress, default),
+        transact: Optional["TxParams"] = None,
+    ) -> Optional[HexBytes]:
+        """
+        Set up the name to point to the supplied address.
+        The sender of the transaction must own the name, or
+        its parent name.
+
+        Example: If the caller owns ``parentname.eth`` with no subdomains
+        and calls this method with ``sub.parentname.eth``,
+        then ``sub`` will be created as part of this call.
+
+        :param str name: ENS name to set up
+        :param str address: name will point to this address, in checksum format. If ``None``,
+            erase the record. If not specified, name will point to the owner's address.
+        :param dict transact: the transaction configuration, like in
+            :meth:`~web3.eth.Eth.send_transaction`
+        :raises InvalidName: if ``name`` has invalid syntax
+        :raises UnauthorizedError: if ``'from'`` in `transact` does not own `name`
+        """
+        if not transact:
+            transact = {}
+        transact = deepcopy(transact)
+        owner = self.setup_owner(name, transact=transact)
+        self._assert_control(owner, name)
+        if is_none_or_zero_address(address):
+            address = None
+        elif address is default:
+            address = owner
+        elif is_binary_address(address):
+            address = to_checksum_address(cast(str, address))
+        elif not is_checksum_address(address):
+            raise ValueError("You must supply the address in checksum format")
+        if self.address(name) == address:
+            return None
+        if address is None:
+            address = EMPTY_ADDR_HEX
+        transact['from'] = owner
+
+        resolver: 'Contract' = self._set_resolver(name, transact=transact)
+        return resolver.functions.setAddr(raw_name_to_hash(name), address).transact(transact)
+
     def name(self, address: ChecksumAddress) -> Optional[str]:
         """
         Look up the name that the address points to, using a
@@ -183,28 +237,68 @@ def setup_name(
                 self.setup_address(name, address, transact=transact)
             return self._setup_reverse(name, address, transact=transact)
 
-    def reverser(self, target_address: ChecksumAddress) -> Optional['Contract']:
-        reversed_domain = address_to_reverse_domain(target_address)
-        return self.resolver(reversed_domain)
+    def owner(self, name: str) -> ChecksumAddress:
+        """
+        Get the owner of a name. Note that this may be different from the
+        deed holder in the '.eth' registrar. Learn more about the difference
+        between deed and name ownership in the ENS `Managing Ownership docs
+        <http://docs.ens.domains/en/latest/userguide.html#managing-ownership>`_
 
-    def _set_resolver(
+        :param str name: ENS name to look up
+        :return: owner address
+        :rtype: str
+        """
+        node = raw_name_to_hash(name)
+        return self.ens.caller.owner(node)
+
+    def setup_owner(
         self,
         name: str,
-        resolver_addr: Optional[ChecksumAddress] = None,
+        new_owner: ChecksumAddress = cast(ChecksumAddress, default),
         transact: Optional["TxParams"] = None,
-    ) -> 'Contract':
+    ) -> Optional[ChecksumAddress]:
+        """
+        Set the owner of the supplied name to `new_owner`.
+
+        For typical scenarios, you'll never need to call this method directly,
+        simply call :meth:`setup_name` or :meth:`setup_address`. This method does *not*
+        set up the name to point to an address.
+
+        If `new_owner` is not supplied, then this will assume you
+        want the same owner as the parent domain.
+
+        If the caller owns ``parentname.eth`` with no subdomains
+        and calls this method with ``sub.parentname.eth``,
+        then ``sub`` will be created as part of this call.
+
+        :param str name: ENS name to set up
+        :param new_owner: account that will own `name`. If ``None``, set owner to empty addr.
+            If not specified, name will point to the parent domain owner's address.
+        :param dict transact: the transaction configuration, like in
+            :meth:`~web3.eth.Eth.send_transaction`
+        :raises InvalidName: if `name` has invalid syntax
+        :raises UnauthorizedError: if ``'from'`` in `transact` does not own `name`
+        :returns: the new owner's address
+        """
         if not transact:
             transact = {}
         transact = deepcopy(transact)
-        if is_none_or_zero_address(resolver_addr):
-            resolver_addr = self.address('resolver.eth')
-        namehash = raw_name_to_hash(name)
-        if self.ens.caller.resolver(namehash) != resolver_addr:
-            self.ens.functions.setResolver(
-                namehash,
-                resolver_addr
-            ).transact(transact)
-        return cast('Contract', self._resolver_contract(address=resolver_addr))
+        (super_owner, unowned, owned) = self._first_owner(name)
+        if new_owner is default:
+            new_owner = super_owner
+        elif not new_owner:
+            new_owner = ChecksumAddress(EMPTY_ADDR_HEX)
+        else:
+            new_owner = to_checksum_address(new_owner)
+        current_owner = self.owner(name)
+        if new_owner == EMPTY_ADDR_HEX and not current_owner:
+            return None
+        elif current_owner == new_owner:
+            return current_owner
+        else:
+            self._assert_control(super_owner, name, owned)
+            self._claim_ownership(new_owner, unowned, owned, super_owner, transact=transact)
+            return new_owner
 
     def resolver(self, name: str) -> Optional['Contract']:
         """
@@ -215,19 +309,9 @@ def resolver(self, name: str) -> Optional['Contract']:
         normal_name = normalize_name(name)
         return self._get_resolver(normal_name)[0]
 
-    def owner(self, name: str) -> ChecksumAddress:
-        """
-        Get the owner of a name. Note that this may be different from the
-        deed holder in the '.eth' registrar. Learn more about the difference
-        between deed and name ownership in the ENS `Managing Ownership docs
-        <http://docs.ens.domains/en/latest/userguide.html#managing-ownership>`_
-
-        :param str name: ENS name to look up
-        :return: owner address
-        :rtype: str
-        """
-        node = raw_name_to_hash(name)
-        return self.ens.caller.owner(node)
+    def reverser(self, target_address: ChecksumAddress) -> Optional['Contract']:
+        reversed_domain = address_to_reverse_domain(target_address)
+        return self.resolver(reversed_domain)
 
     def get_text(self, name: str, key: str) -> str:
         """
@@ -300,100 +384,6 @@ def set_text(
                 "unsupported top level domain (tld)."
             )
 
-    def setup_address(
-        self,
-        name: str,
-        address: Union[Address, ChecksumAddress, HexAddress] = cast(ChecksumAddress, default),
-        transact: Optional["TxParams"] = None,
-    ) -> Optional[HexBytes]:
-        """
-        Set up the name to point to the supplied address.
-        The sender of the transaction must own the name, or
-        its parent name.
-
-        Example: If the caller owns ``parentname.eth`` with no subdomains
-        and calls this method with ``sub.parentname.eth``,
-        then ``sub`` will be created as part of this call.
-
-        :param str name: ENS name to set up
-        :param str address: name will point to this address, in checksum format. If ``None``,
-            erase the record. If not specified, name will point to the owner's address.
-        :param dict transact: the transaction configuration, like in
-            :meth:`~web3.eth.Eth.send_transaction`
-        :raises InvalidName: if ``name`` has invalid syntax
-        :raises UnauthorizedError: if ``'from'`` in `transact` does not own `name`
-        """
-        if not transact:
-            transact = {}
-        transact = deepcopy(transact)
-        owner = self.setup_owner(name, transact=transact)
-        self._assert_control(owner, name)
-        if is_none_or_zero_address(address):
-            address = None
-        elif address is default:
-            address = owner
-        elif is_binary_address(address):
-            address = to_checksum_address(cast(str, address))
-        elif not is_checksum_address(address):
-            raise ValueError("You must supply the address in checksum format")
-        if self.address(name) == address:
-            return None
-        if address is None:
-            address = EMPTY_ADDR_HEX
-        transact['from'] = owner
-
-        resolver: 'Contract' = self._set_resolver(name, transact=transact)
-        return resolver.functions.setAddr(raw_name_to_hash(name), address).transact(transact)
-
-    def setup_owner(
-        self,
-        name: str,
-        new_owner: ChecksumAddress = cast(ChecksumAddress, default),
-        transact: Optional["TxParams"] = None,
-    ) -> Optional[ChecksumAddress]:
-        """
-        Set the owner of the supplied name to `new_owner`.
-
-        For typical scenarios, you'll never need to call this method directly,
-        simply call :meth:`setup_name` or :meth:`setup_address`. This method does *not*
-        set up the name to point to an address.
-
-        If `new_owner` is not supplied, then this will assume you
-        want the same owner as the parent domain.
-
-        If the caller owns ``parentname.eth`` with no subdomains
-        and calls this method with ``sub.parentname.eth``,
-        then ``sub`` will be created as part of this call.
-
-        :param str name: ENS name to set up
-        :param new_owner: account that will own `name`. If ``None``, set owner to empty addr.
-            If not specified, name will point to the parent domain owner's address.
-        :param dict transact: the transaction configuration, like in
-            :meth:`~web3.eth.Eth.send_transaction`
-        :raises InvalidName: if `name` has invalid syntax
-        :raises UnauthorizedError: if ``'from'`` in `transact` does not own `name`
-        :returns: the new owner's address
-        """
-        if not transact:
-            transact = {}
-        transact = deepcopy(transact)
-        (super_owner, unowned, owned) = self._first_owner(name)
-        if new_owner is default:
-            new_owner = super_owner
-        elif not new_owner:
-            new_owner = ChecksumAddress(EMPTY_ADDR_HEX)
-        else:
-            new_owner = to_checksum_address(new_owner)
-        current_owner = self.owner(name)
-        if new_owner == EMPTY_ADDR_HEX and not current_owner:
-            return None
-        elif current_owner == new_owner:
-            return current_owner
-        else:
-            self._assert_control(super_owner, name, owned)
-            self._claim_ownership(new_owner, unowned, owned, super_owner, transact=transact)
-            return new_owner
-
     def _get_resolver(
         self,
         normal_name: str,
@@ -418,14 +408,24 @@ def _get_resolver(
             # set current_name to parent and try again
             current_name = self.parent(current_name)
 
-    def address(self, name: str) -> Optional[ChecksumAddress]:
-        """
-        Look up the Ethereum address that `name` currently points to.
-
-        :param str name: an ENS name to look up
-        :raises InvalidName: if `name` has invalid syntax
-        """
-        return cast(ChecksumAddress, self._resolve(name, 'addr'))
+    def _set_resolver(
+        self,
+        name: str,
+        resolver_addr: Optional[ChecksumAddress] = None,
+        transact: Optional["TxParams"] = None,
+    ) -> 'Contract':
+        if not transact:
+            transact = {}
+        transact = deepcopy(transact)
+        if is_none_or_zero_address(resolver_addr):
+            resolver_addr = self.address('resolver.eth')
+        namehash = raw_name_to_hash(name)
+        if self.ens.caller.resolver(namehash) != resolver_addr:
+            self.ens.functions.setResolver(
+                namehash,
+                resolver_addr
+            ).transact(transact)
+        return cast('Contract', self._resolver_contract(address=resolver_addr))
 
     def _resolve(self, name: str, fn_name: str = 'addr') -> Optional[Union[ChecksumAddress, str]]:
         normal_name = normalize_name(name)
@@ -456,6 +456,34 @@ def _resolve(self, name: str, fn_name: str = 'addr') -> Optional[Union[ChecksumA
             return to_checksum_address(result) if is_address(result) else result
         return None
 
+    def _assert_control(
+        self,
+        account: ChecksumAddress,
+        name: str,
+        parent_owned: Optional[str] = None,
+    ) -> None:
+        if not address_in(account, self.w3.eth.accounts):
+            raise UnauthorizedError(
+                f"in order to modify {name!r}, you must control account"
+                f" {account!r}, which owns {parent_owned or name!r}"
+            )
+
+    def _first_owner(self, name: str) -> Tuple[Optional[ChecksumAddress], Sequence[str], str]:
+        """
+        Takes a name, and returns the owner of the deepest subdomain that has an owner
+
+        :returns: (owner or None, list(unowned_subdomain_labels), first_owned_domain)
+        """
+        owner = None
+        unowned = []
+        pieces = normalize_name(name).split('.')
+        while pieces and is_none_or_zero_address(owner):
+            name = '.'.join(pieces)
+            owner = self.owner(name)
+            if is_none_or_zero_address(owner):
+                unowned.append(pieces.pop(0))
+        return (owner, unowned, name)
+
     def _claim_ownership(
         self,
         owner: ChecksumAddress,
@@ -493,33 +521,6 @@ def _reverse_registrar(self) -> 'Contract':
         addr = self.ens.caller.owner(normal_name_to_hash(REVERSE_REGISTRAR_DOMAIN))
         return self.w3.eth.contract(address=addr, abi=abis.REVERSE_REGISTRAR)
 
-    def _assert_control(
-        self,
-        account: ChecksumAddress,
-        name: str, parent_owned: Optional[str] = None,
-    ) -> None:
-        if not address_in(account, self.w3.eth.accounts):
-            raise UnauthorizedError(
-                f"in order to modify {name!r}, you must control account"
-                f" {account!r}, which owns {parent_owned or name!r}"
-            )
-
-    def _first_owner(self, name: str) -> Tuple[Optional[ChecksumAddress], Sequence[str], str]:
-        """
-        Takes a name, and returns the owner of the deepest subdomain that has an owner
-
-        :returns: (owner or None, list(unowned_subdomain_labels), first_owned_domain)
-        """
-        owner = None
-        unowned = []
-        pieces = normalize_name(name).split('.')
-        while pieces and is_none_or_zero_address(owner):
-            name = '.'.join(pieces)
-            owner = self.owner(name)
-            if is_none_or_zero_address(owner):
-                unowned.append(pieces.pop(0))
-        return (owner, unowned, name)
-
 
 def _resolver_supports_interface(resolver: 'Contract', interface_id: HexStr) -> bool:
     if not any('supportsInterface' in repr(func) for func in resolver.all_functions()):