Call contract function using block hash makes two requests inside

[F4ever]

Hi!

All call to contract functions with block identifier as block hash makes two requests:

1. Fetch block by block hash.
2. Send call request to contract with block number from previous response.

https://github.com/ethereum/web3.py/blob/master/web3/contract/contract.py#L406

How can it be fixed?

I propose to replace this part

web3.py/web3/_utils/contracts.py

Lines 419 to 421 in a35351d

	elif isinstance(block_identifier, bytes) or is_hex_encoded_block_hash(
	block_identifier
	):

with

elif is_hex_encoded_block_hash(block_identifier):
    return block_identifier
elif isinstance(block_identifier, bytes):
    return '0x' + block_identifier.hex()

[reedsa]

@fselmo the latest on main returns w3.eth.get_block(block_identifier)["number"]. I'm curious to know if this is still required?

[fselmo]

Yeah the suggestion here seems to be a better one. Remove the call to w3.eth.get_block() altogether. I'm not sure this is actually breaking though. We could chalk it up to a performance newsfragment since it shouldn't alter what the return value is and just gets rid of an extra call. Thoughts there?

[reedsa]

The return value should be the block_number rather than the block_identifier. The tests assert that the block number 0 is returned when passed the HexStr identifier. https://github.com/ethereum/web3.py/blob/main/tests/core/contracts/test_contract_util_functions.py#L103

I also found a Contracts.parse_block_identifier_no_extra_call function exists. I think it'd be good to just update parse_block_identifier and remove parse_block_identifier_no_extra_call. Is that possible, given this comment?

[fselmo]

The return value should be the block_number rather than the block_identifier

You're right, I don't think eth_call accepts the block hash as the block identifier. Only the block number as an integer OR the latest, earliest, pending, etc keywords. I looked briefly and I'm pretty sure we can remove this entirely from the contract caller init - both in the sync and async cases - since we use it again in ContractFunction.call().

That might be the answer.

[fselmo]

@reedsa I'm pretty sure all of this can go and we can just pass block_identifier directly through in line 595 since that sets it up for using self.call_function which returns fn.call (AsyncFunction.call()) which calls the correct method here. We should make sure there's a test in place to test that.

If the above is true, we can also remove the equivalent line from the __init__ of the ContractCaller (non-async) here since that will also get called here. We should also make sure there's an equivalent sync version of this same test.