vyperlang / titanoboa

a vyper interpreter
https://titanoboa.readthedocs.io
Other
247 stars 49 forks source link

Empty, nested revert messages contain unexpected characters #143

Closed antazoey closed 3 months ago

antazoey commented 6 months ago

When a revert happens in a sub-call, it trickles back in a way causing the b from the b"" and a / to appear, which can be difficult to parse.

Revert('Revert("Revert(b\'\')")')

repro

  1. Go to this repo: https://github.com/ApeWorX/SafeDrop
  2. Go to test tests/functional/test_transfer.py::test_mint_not_minter
  3. The part that says:
    with boa.reverts():
        nft.mint(hacker.address, sender=hacker.address)

change to

    nft.mint(hacker.address, sender=hacker.address)

so the revert is no caught.

Run the test:

pytest tests/functional/test_transfer.py::test_mint_not_minter

should see output like:

FAILED tests/functional/test_transfer.py::test_mint_not_minter - boa.contracts.base_evm_contract.BoaError: Revert('Revert(\'Revert("Revert(b\\\'\\\')")\')')

the end of such output is Revert('Revert(\'Revert("Revert(b\\\'\\\')")\')') which i find has much room for improvement regarding its display

charles-cooper commented 5 months ago

can you provide a repro? repro is eluding me for some reason

antazoey commented 5 months ago

can you provide a repro? repro is eluding me for some reason

ok i just added some

DanielSchiavini commented 3 months ago

@antazoey the given repository seems to be empty

DanielSchiavini commented 3 months ago

I tested with the following code and cannot reproduce (the test passes):

def test_error_str():
    contract = boa.loads("""
@external
def bar(x: uint256):
    assert x + 1 == 6  # @rekt x is not 5
    """)
    nested_contract = boa.loads("""
contract: address

interface Contract:
    def bar(x: uint256): nonpayable

@external
def __init__(contract: address):
    self.contract = contract

@external
def ext_call():
    Contract(self.contract).bar(11)
""", contract)
    multinested_contract = boa.loads(
        """
contract: address

interface Contract:
    def ext_call(): nonpayable

@external
def __init__(contract: address):
    self.contract = contract

@external
def ext_call():
    Contract(self.contract).ext_call()
    """, nested_contract
    )
    with pytest.raises(BoaError) as error_context:
        multinested_contract.ext_call()
    assert str(error_context.value).startswith("Revert(b'')\n")
antazoey commented 3 months ago
  1. https://github.com/ApeWorX/SafeDrop

Sorry, I meant here: https://github.com/antazoey/SafeDrop/tree/feat/merkle-cli

DanielSchiavini commented 3 months ago

@antazoey I'm struggling to run this repository. There's no requirements list that I could locate. Also no explanation on how to run the tests. After installing ape and boa I am getting the following error with either ape test or pytest tests:

ERROR tests/functional/test_transfer.py::test_mint - ape.exceptions.ApeAttributeError: ProjectManager has no attribute or contract named 'Claim'. However, there is a source file named 'Claim.vy', did you mean to reference a contract name from this source file? Else...
ERROR tests/functional/test_transfer.py::test_mint_not_minter - ape.exceptions.ApeAttributeError: ProjectManager has no attribute or contract named 'Claim'. However, there is a source file named 'Claim.vy', did you mean to reference a contract name from this source file? Else...
ERROR tests/integration/test_safe_drop.py::test_integration - ape.exceptions.ApeAttributeError: ProjectManager has no attribute or contract named 'Claim'. However, there is a source file named 'Claim.vy', did you mean to reference a contract name from this source file? Else...
antazoey commented 3 months ago

Apologies! Im not sure why im getting reverts like this. If i have time, ill make a better repro. Feel free to close.

antazoey commented 3 months ago

Trying to reproduce again, but suddenly getting this error:

E   TypeError: titanoboa_computation.apply_computation() got an unexpected keyword argument 'parent_computation'

edit: appears to be this https://github.com/vyperlang/titanoboa/issues/193

antazoey commented 3 months ago

OK, got it. Downgraded to 0.1.8 to get around #193

Here is a screenshot to prove what I am seeing:

Screenshot 2024-05-17 at 10 03 47

(running ape test tests/functional) btw, your tests are not running probably because you dont have ape-vyper installed..

but nonetheless I can try to get a better repro

DanielSchiavini commented 3 months ago

Please test with boa master. It might have been fixed already

antazoey commented 3 months ago

OK, easier repro here: https://github.com/antazoey/repro-boabug

I tried from main but am getting this error:

ModuleNotFoundError: No module named 'boa.test.plugin'
DanielSchiavini commented 3 months ago

@antazoey thanks for the reproduction repository, that's very helpful. I copied the scenario to our repo and ran it. See #230 the error I get is

    def marshal_to_python(self, computation, vyper_typ):
        self._computation = computation  # for further inspection

        if computation.is_error:
>           self.handle_error(computation)
E           boa.contracts.base_evm_contract.BoaError: Revert(b'')
E           
E           <boa/tests/unitary/fixtures/Claim.vy at 0x0880cf17Bd263d3d3a5c09D2D86cCecA3CcbD97c, compiled with vyper-0.3.10+9136169>
E           <storage: baseURI=ipfs://3120242557696478518, totalSupply=0, balanceOf={}, ownerOf={}, idToApprovals={}, isApprovedForAll={}, is_minter={}, owner=0x00dE89C733555886f785b0C32b498300297e481F, name=XBridgeHack Claim 2024-06-26, symbol=XBridgeHack_20240626, description=<truncated>, artist=0x0000000000000000000000000000000000000000, nonces={}, DOMAIN_SEPARATOR=b'\xf0\x10\xf9\x1e\x9f)\xe9\x92\xe7\xda\xe5f\xe5\xdb\xdd\x0cD\xe7\xaaz\xe9nH0\x85\xfbx\x08\x00y\xbd!\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'>
E            <compiler: user assert>
E             contract "boa/tests/unitary/fixtures/Claim.vy:439", function "setMinter", line 439:4 
E                  438     """
E             ---> 439     assert msg.sender == self.owner
E             -------------^
E                  440     self.is_minter[minter] = True
E            <locals: minter=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266>

../../boa/contracts/vyper/vyper_contract.py:715: BoaError
DanielSchiavini commented 3 months ago

OK scrap that, in CI the result is FAILED tests/unitary/test_reverts.py::test_repro - boa.contracts.base_evm_contract.BoaError: Revert("Revert(b'')")

So I cannot reproduce locally for some reason. But I have a bug to fix :laughing:

antazoey commented 3 months ago

Yay, I am glad CI was able to repro! Let me know if there is anything else I can do.

antazoey commented 3 months ago

More info: This does not seem to happen in my Python 3.10 env but it does happen in my Python 3.11 env.

DanielSchiavini commented 3 months ago

OK I found the issue. The __str__ implementation was patching the error every time it was called. So it was a matter of calling str() multiple times to see the error get more and more nested.