ApeWorX / ape

The smart contract development tool for Pythonistas, Data Scientists, and Security Professionals
https://apeworx.io
Apache License 2.0
889 stars 131 forks source link

Tests fail with --coverage while they are 100% pass without --coverage #1777

Closed Doc-Pixel closed 10 months ago

Doc-Pixel commented 10 months ago

Environment information

Installed Plugins:
  infura       0.6.0
  alchemy      0.6.0
  template     0.6.1
  foundry      0.6.11
  solidity     0.6.6
  tokens       0.6.1
  vyper        0.6.11
  hardhat      0.6.13
  polygon      0.6.5
  etherscan    0.6.7
  ens          0.6.1
name: myContract

default_ecosystem: ethereum

# number_of_accounts: 5

plugins:
  - name: infura
  - name: alchemy
  - name: vyper
  - name: ens
  - name: etherscan 
  - name: hardhat

vyper:
  evm_version: istanbul  # bor doesn't support latest forks

geth:
  ethereum:
    mainnet:
      uri: http://myServer:8545
      chain_id: 109

hardhat:
  request_timeout: 30  # Defaults to 30
  fork_request_timeout: 600  # Defaults to 300
  fork:
    ethereum:
      mainnet:
        upstream_provider: geth

test:
  coverage:
    reports:
      terminal:
        verbose: true  # Show verbose coverage information in the terminal.

What went wrong?

When I run my tests without --coverage, I get 100% PASS as a result. When I add --coverage, I get errors. It happens when I call my function that accepts an address as the only input parameter.

So in short:

def myFunction(_address: address):

And I test it with myContract.myFunction(address_to_check, sender=address_in_adminlist)

Ape tells me:

from field must match key's 0x1e59ce931B4CFea3fe4B875411e280e173cB7A9C, but it was 0xc89D42189f0450C2b2c3c61f58Ec5d628176A1E7

Long version below

def test_Profile(my_contract, owner, not_owner):
    my_contract.storeProfile('mango', owner , 1, 'hi Im nobody bla bla bla', sender=owner)
    assert my_contract.getProfile(owner, sender=owner) == ('mango', owner, 1,'hi Im nobody bla bla bla')
    assert my_contract.getProfile(owner, sender=not_owner) == ('mango', owner, 1,'hi Im nobody bla bla bla')

The contract function is literally this: Return a stored struct for the account that got passed. There is no assertion that only the owner address can request their profile.

@view
@external
def getProfile(account: address) -> Profile:
    return self.profileStore[account]

And the error message is

/home/ethbox/Vyper-Main/myContract/tests/test_myContract.py:70: in test_Profile
    assert my_contract.getProfile(owner, sender=not_owner) == ('mango', owner, 1,'hi Im nobody bla bla bla')
E   TypeError: from field must match key's 0x1e59ce931B4CFea3fe4B875411e280e173cB7A9C, but it was 0xc89D42189f0450C2b2c3c61f58Ec5d628176A1E7
        myContract = <myContract 0x274b028b03A250cA03644E6c578D81f019eE1323>
        not_owner  = <TestAccount 0xc89D42189f0450C2b2c3c61f58Ec5d628176A1E7>
        owner      = <TestAccount 0x1e59ce931B4CFea3fe4B875411e280e173cB7A9C>

And the way profiles are stored:

struct Profile:
    # userID: uint256 
    userName: String[64]
    pfpContract: address
    pfpID: uint256
    bio: String[256]

profileStore: HashMap[address, Profile]

So for some reason it doesn't allow not_owner to call the getProfile function and pass not_owner as the sender and requesting the profile created by sender. Which doesn't make sense with the vyper function. Also it should either succeed or fail in both scenarios, wether I test with or without --coverage. So not consistent.

To make things more exciting I fork shibarium with ape-hardhat (even though it's called ethereum in the ape-config. I just flipped the IP) ape test --network ethereum:mainnet-fork:hardhat --coverage

How can it be fixed?

Not entirely sure, seems like the address passed as a parameter (owner) is overwritten with the sender= address (not_owner). This only gets triggered when I use --coverage.

linear[bot] commented 10 months ago

APE-1620 Tests fail with --coverage while they are 100% pass without --coverage

antazoey commented 10 months ago

Can you try again on 0.7 if possible?

We have fixed some things, at least.. failing to get coverage on a tx should no longer cause the test to fail anyway.

Doc-Pixel commented 10 months ago

Can you try again on 0.7 if possible?

We have fixed some things, at least.. failing to get coverage on a tx should no longer cause the test to fail anyway.

I've tested with this setup:

Python 3.10.13 Ape 0.7.1 Vyper 0.3.10

I upgraded the entire toolchain, deleted old python versions / libs like this sudo rm -rf /usr/local/lib/python3.8 and every occurence everywhere because I had some weird interference going on with ape/vyper still trying to pull stuff from those locations.

Uninstalled then reinstalled all the ape plugins. Coverage is working and I no longer get the scenario where test fail with a non-owner/deployer account on a function that doesn't check for msg.sender at all.

:-)

Doc-Pixel commented 10 months ago

Me again, reopening again :-)

getting a did not revert using coverage while the test passes without coverage.

Tried with the 0.7.1 version and also did a pip3 install git+ Also set the version pragma to vyper 0.3.7 and back to 0.3.10 to see if that had any effect which it didn't.

Bit of a peculiar error messages, depending if we have set a revert message or not in the _checkAccess()function

    # with a revert message:
    with ape.reverts():
E   AssertionError: Transaction did not revert.
E   However, an exception of type <class 'OverflowError'> occurred: Python int too large to convert to C ssize_t.

    # without a revert message
    with ape.reverts():
E   AssertionError: Transaction did not revert.
E   However, an exception of type <class 'ape.exceptions.DecodingError'> occurred: Tried to read 32 bytes, only got 0 bytes..

It's checking

denyList: HashMap[address, bool]

@view
@internal
def _denied() -> bool:
    return self.denyList[msg.sender]

@view
@internal
def _checkAccess() -> bool:
    assert not self._denied(), 'optional revert message'
    # rest of checks

I call an external function extFunction that calls _checkAccess() to determine if someone is admin, on allow/deny lists etc depending on contract mode. and one of the checks is to call _denied() to check if someone is on the denylist before the body is executed

Running the tests without --coverage is 100% pass.

Doc-Pixel commented 10 months ago

with -pdb I get some more hints


exc_value = DecodingError('Tried to read 32 bytes, only got 0 bytes.'), traceback = <traceback object at 0x7feb1c98ae00>

    def __exit__(self, exc_type: Type, exc_value: Exception, traceback) -> bool:
        if exc_type is None:
            raise AssertionError("Transaction did not revert.")

        if not isinstance(exc_value, ContractLogicError):
>           raise AssertionError(
                f"Transaction did not revert.\n"
                f"However, an exception of type {type(exc_value)} occurred: {exc_value}."
            ) from exc_value
E           AssertionError: Transaction did not revert.
E           However, an exception of type <class 'ape.exceptions.DecodingError'> occurred: Tried to read 32 bytes, only got 0 bytes..

../../../.local/lib/python3.10/site-packages/ape/pytest/contextmanagers.py:162: AssertionError```
antazoey commented 10 months ago

If you run with -v debug, it will print out Ape's traceback which will really help me. I still haven't reproduced!

Doc-Pixel commented 10 months ago

I think this is the relevant section, let me know if you need more details :-)

with --coverage:

DEBUG: hardhat_metadata
DEBUG: Getting response HTTP. URI: http://127.0.0.1:8545/[hidden] Method: hardhat_metadata, Response: {'jsonrpc': '2.0', 'id': 4, 'result': {'clientVersion': 'HardhatNetwork/2.19.4/@nomicfoundation/ethereumjs-vm/7.0.2', 'chainId': 31337, 'instanceId': '0x54a18089bc9e3a55f490e4b67117ac892f2b4a1b51ac3d1b8ea4f52624cb9bc9', 'latestBlockNumber': 2446853, 'latestBlockHash': '0xf91ca2a5d082a6018eaeee793b51f2b72aa410612727f9a3d6ce332f9af289c0', 'forkedNetwork': {'chainId': 109, 'forkBlockNumber': 2446853, 'forkBlockHash': '0xf91ca2a5d082a6018eaeee793b51f2b72aa410612727f9a3d6ce332f9af289c0'}}}

tests/test_bricker.py .DEBUG: eth_getStorageAt (4)
F
/home/user/Projects/Vyper/shibber/tests/test_bricker.py:115: in test_all_modes
    with ape.reverts():
E   AssertionError: Transaction did not revert.
E   However, an exception of type <class 'ape.exceptions.DecodingError'> occurred: Tried to read 32 bytes, only got 0 bytes..
        allowed_user = <TestAccount 0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc>
        bricker_contract = <bricker 0x5FC8d32690cc91D4c39d9d3abcBD16989F875707>
        denied_user = <TestAccount 0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65>
        mode       = 1
        owner      = <TestAccount 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266>

Starting interactive mode. Type `exit` to halt current test.

In [1]: 

and without --coverage:

DEBUG: hardhat_metadata
DEBUG: Getting response HTTP. URI: http://127.0.0.1:8545/[hidden] Method: hardhat_metadata, Response: {'jsonrpc': '2.0', 'id': 4, 'result': {'clientVersion': 'HardhatNetwork/2.19.4/@nomicfoundation/ethereumjs-vm/7.0.2', 'chainId': 31337, 'instanceId': '0x3e8533557458be475c2f5f93b8e3a8ebf88c301301af3f627a53cc5e7f3cb671', 'latestBlockNumber': 2446967, 'latestBlockHash': '0xce630e41cd1a36bbd68629dbeff2eef47ad243eeaa61ee5c5ec56c9d7c23b472', 'forkedNetwork': {'chainId': 109, 'forkBlockNumber': 2446967, 'forkBlockHash': '0xce630e41cd1a36bbd68629dbeff2eef47ad243eeaa61ee5c5ec56c9d7c23b472'}}}

tests/test_bricker.py DEBUG: Contract call:       <UnrecognizedContract>
DEBUG: From:                0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266
.DEBUG: eth_getStorageAt (4)
DEBUG: Contract call:       <UnrecognizedContract>
DEBUG: From:                0x9965507d1a55bcc2695c58ba16fb37d819b0a4dc
DEBUG: To:                  0x5fc8d32690cc91d4c39d9d3abcbd16989f875707
DEBUG: 
DEBUG: Error: Transaction reverted without a reason string
DEBUG: at <UnrecognizedContract>.<unknown> (0x5fc8d32690cc91d4c39d9d3abcbd16989f875707)
DEBUG: at processTicksAndRejections (node:internal/process/task_queues:95:5)
DEBUG: at HardhatNode.runCall (/home/user/Projects/Vyper/shibber/node_modules/hardhat/src/internal/hardhat-network/provider/node.ts:679:20)
DEBUG: at EthModule._callAction (/home/user/Projects/Vyper/shibber/node_modules/hardhat/src/internal/hardhat-network/provider/modules/eth.ts:360:9)
DEBUG: at HardhatNetworkProvider._sendWithLogging (/home/user/Projects/Vyper/shibber/node_modules/hardhat/src/internal/hardhat-network/provider/provider.ts:145:22)
DEBUG: at HardhatNetworkProvider.request (/home/user/Projects/Vyper/shibber/node_modules/hardhat/src/internal/hardhat-network/provider/provider.ts:122:18)
DEBUG: at JsonRpcHandler._handleRequest (/home/user/Projects/Vyper/shibber/node_modules/hardhat/src/internal/hardhat-network/jsonrpc/handler.ts:191:20)
DEBUG: at JsonRpcHandler._handleSingleRequest (/home/user/Projects/Vyper/shibber/node_modules/hardhat/src/internal/hardhat-network/jsonrpc/handler.ts:152:17)
DEBUG: 
.DEBUG: eth_getStorageAt (4)
.DEBUG: eth_getStorageAt (4)
DEBUG: Contract call:       <UnrecognizedContract>
DEBUG: From:                0x70997970c51812dc3a010c7d01b50e0d17dc79c8
DEBUG: To:                  0x5fc8d32690cc91d4c39d9d3abcbd16989f875707
DEBUG: 
....                                                                                                                                                                 [100%]

======================================================================================== 7 passed in 11.73s =========================================================================================
INFO: Stopping 'Hardhat node' process.
user@laptop:~/Projects/Vyper/shibber$

Interesting enough without coverage there's still (sort of) an error but it seems to pass the test.

antazoey commented 10 months ago

I am jus having trouble reproducing any of this :(

Are you able to possible create a reproduction repo I can work off of?

antazoey commented 10 months ago

here is my example project, piecing together bits here to try and reproduce: https://github.com/antazoey/ape-playground/pull/3/files

but the tests never fail, not even with --coverage, so im not sure

Doc-Pixel commented 10 months ago

The plot thickens:

I am running the tests against shibarium, which is a bor/heimdalld based chain. Pretty much polygon with a different base token.

I switched to my ethereum node and now it forks mainnet and tests with coverage without any issues.

Polygon's bor doesn't support the latest EVM, but (I think) up to the London fork and I know they're working to release an update to bor to support newer EVM versions. Not sure if that is mainnet already.

Currently I'm not in a position to test with polygon if the behaviour is the same. I'll try as soon as I get a chance.

antazoey commented 10 months ago

I just tried polygon mainnet fork and ran the tests but still didnt have an issue, but maybe i should use something already deployed to mainnet before forking?

I pushed my config changes to the repro branch on the PR I commented above.