fix bug in how eth_tester_middleware filled default fields

[pacrob]

What was wrong?

function async_default_transaction_fields_middleware was using some non-asynced helper functions, so it wouldn't fill properly

How was it fixed?

added async versions of the helper functions

Todo:

• Add entry to the release notes

Cute Animal Picture

[fselmo]

I haven't looked too deep here yet but right away I returned all of the code in web3/providers/eth_tester/middleware.py to what it is in master and all the tests pass, including the ones added in this PR.

Let's try to create a test (or a few) that fail(s) without the changes in the PR but passes with the changes.

[pacrob]

Updated tests so they now fail on the old code. I tried various ways of patching the w3 instance but couldn't make anything work, so went with Mock instead.

One question/point for discussion - the guess_from functions try to fill the from field, but don't make any noise if they can't. Should they?

[fselmo]

Nice catch and fix 👌🏼 ... I had a few nits, mostly just on conventions. I'll approve since the fix is in there though 👍🏼. I'm good with the mocking too.

As far as not making any noise when it cannot fill the fields... it looks like we purposefully return None if that's the case. I'd leave it for now as this seems more of a helper middleware, to make communicating with eth-tester a bit easier, than it seems a validation type middleware.

[fselmo]

nit: w3 -> async_w3 for consistency

[pacrob]

done

[fselmo]

nit: w3 -> async_w3 for consistency

[pacrob]

done

[fselmo]

nit: I've been separating async from sync tests with:

def test_last_sync_test_here():
    ...
    

# -- async -- #


@pytest.mark.asyncio
async def test_async_first_async_test_here():
    ...

With sync and async tests that exist in the same file I'd like to organize them somehow for quicker / easier readability. Since that convention has been used in quite a few places already I'm partial to it 😄 .

[pacrob]

check

[fselmo]

nit: Let's have these as capitalized and near the top of the file just to stick with python convention, especially since sync and async both use these values.

from some_module import last_import_statement

SAMPLE_ADDRESS_1 = "0x0000000000000000000000000000000000000001"
SAMPLE_ADDRESS_2 = "0x0000000000000000000000000000000000000002"

[pacrob]

check

[fselmo]

Instead of re-defining the variable here, I think we should either make the mock return the list with a variable more appropriately named w3_account (singular):

mock_w3.eth.accounts = [w3_account]  # and the variable is renamed to w3_account (singular)

# async:
async def mock_async_accounts():
    return [w3_account]  # singular

or... we can just pass in a list of accounts into the tests themselves and keep the variable as w3_accounts (plural). I vote this option and we can change one of the tests to return more than one account in the list to make sure that works as intended as well... I believe it always returns accounts[0].

[pacrob]

Makes sense to have the mock return what the function normally would. I've updated the sample address constants accordingly. It will still only ever choose index 0 of the list of addresses though, as that's how the guess_from function is written.

[kclowes]

LGTM! 🚢