Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: OverflowError: Python int too large to convert to C ssize_t #1634

Closed
zinootje opened this issue Apr 30, 2020 · 29 comments
Closed

Bug: OverflowError: Python int too large to convert to C ssize_t #1634

zinootje opened this issue Apr 30, 2020 · 29 comments

Comments

@zinootje
Copy link

zinootje commented Apr 30, 2020

  • Version: 5.8.0
  • Python: 3.7.6
  • OS: win
  • pip freeze output
attrs==19.3.0
base58==2.0.0
certifi==2019.11.28
cffi==1.13.2
chardet==3.0.4
Cython==0.29.14
cytoolz==0.10.1
eth-abi==2.1.0
eth-account==0.4.0
eth-hash==0.2.0
eth-keyfile==0.5.1
eth-keys==0.2.4
eth-rlp==0.1.2
eth-typing==2.2.1
eth-utils==1.8.4
gevent==1.4.0
greenlet==0.4.15
hexbytes==0.2.0
idna==2.8
importlib-metadata==1.5.0
ipfshttpclient==0.4.12
jsonschema==3.2.0
lru-dict==1.1.6
multiaddr==0.0.9
mypy-extensions==0.4.3
netaddr==0.7.19
oauthlib==3.1.0
parsimonious==0.8.1
protobuf==3.11.2
pycparser==2.19
pycryptodome==3.9.4
pypiwin32==223
pyrsistent==0.15.7
pywin32==227
requests==2.22.0
requests-oauthlib==1.3.0
rlp==1.2.0
six==1.13.0
toolz==0.10.0
typing-extensions==3.7.4.1
uniswap-python==0.3.4
urllib3==1.25.7
varint==1.0.2
web3==5.8.0
websocket==0.2.1
websocket-client==0.57.0
websockets==8.1
yappi==1.2.3
zipp==2.1.0

What was wrong?

I get a overflow error when calling a specific view function which returns a int256[];
When i call te same function online using a site like https://justsmartcontracts.dev the function works fine

  File "C:/Users/*/PycharmProjects/UltraDex/View.py", line 27, in <module>
    ab = az.getreturn()
  File "C:/Users/*/PycharmProjects/UltraDex/View.py", line 21, in getreturn
    return viewinstance.functions.buysellmultiple(self.addresa,self.addresb,self.am,self.parts,self.flags).call()
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\web3\contract.py", line 959, in call
    **self.kwargs
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\web3\contract.py", line 1498, in call_contract_function
    output_data = web3.codec.decode_abi(output_types, return_data)
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\eth_abi\codec.py", line 181, in decode_abi
    return decoder(stream)
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\eth_abi\decoding.py", line 127, in __call__
    return self.decode(stream)
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\eth_utils\functional.py", line 45, in inner
    return callback(fn(*args, **kwargs))
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\eth_abi\decoding.py", line 173, in decode
    yield decoder(stream)
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\eth_abi\decoding.py", line 127, in __call__
    return self.decode(stream)
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\eth_abi\decoding.py", line 144, in decode
    stream.push_frame(start_pos)
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\eth_abi\decoding.py", line 95, in push_frame
    self.seek_in_frame(0)
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\eth_abi\decoding.py", line 84, in seek_in_frame
    super().seek(self._total_offset + pos, *args, **kwargs)
OverflowError: Python int too large to convert to C ssize_t

I know this isn't enough information. If you tell me what else you need i will try to provide is as quickly as posible

@kclowes
Copy link
Collaborator

kclowes commented Apr 30, 2020

@zinootje Can you provide a minimum reproducible example of what you're trying to do?

@zinootje
Copy link
Author

zinootje commented May 1, 2020

sure , do i also need to provide contract source code or is there another way

@kclowes
Copy link
Collaborator

kclowes commented May 1, 2020

Just the snippet of the contract code that is throwing the error would be good, and any background needed to put it in context.

@zinootje
Copy link
Author

zinootje commented May 1, 2020

Here is my minimal example i also uploaded the source code to ether scan to make it easier. As you will see when you call the function "buysellmultiple" from etherscan it works as expected. I hope i didn't miss something obvious .


ViewContract_Abi = """[
	{
		"inputs": [
			{
				"internalType": "address[]",
				"name": "a",
				"type": "address[]"
			},
			{
				"internalType": "address[]",
				"name": "b",
				"type": "address[]"
			},
			{
				"internalType": "uint256[]",
				"name": "amount",
				"type": "uint256[]"
			},
			{
				"internalType": "uint256",
				"name": "parts",
				"type": "uint256"
			},
			{
				"internalType": "uint256",
				"name": "flags",
				"type": "uint256"
			}
		],
		"name": "buysellmultiple",
		"outputs": [
			{
				"internalType": "uint256[]",
				"name": "",
				"type": "uint256[]"
			}
		],
		"stateMutability": "view",
		"type": "function"
	},
	{
		"inputs": [
			{
				"internalType": "address[]",
				"name": "a",
				"type": "address[]"
			},
			{
				"internalType": "address[]",
				"name": "b",
				"type": "address[]"
			},
			{
				"internalType": "uint256[]",
				"name": "amount",
				"type": "uint256[]"
			},
			{
				"internalType": "uint256",
				"name": "parts",
				"type": "uint256"
			},
			{
				"internalType": "uint256[]",
				"name": "flags",
				"type": "uint256[]"
			}
		],
		"name": "ratemultiple",
		"outputs": [
			{
				"internalType": "uint256[]",
				"name": "",
				"type": "uint256[]"
			}
		],
		"stateMutability": "view",
		"type": "function"
	},
	{
		"inputs": [],
		"name": "split",
		"outputs": [
			{
				"internalType": "address",
				"name": "",
				"type": "address"
			}
		],
		"stateMutability": "view",
		"type": "function"
	}
]"""
ViewContract_Address = "0x4802B5410BBF3dc9284DD63ea02E6Ef588707970"
w3 = Web3(WebsocketProvider(*****))
viewinstance =  w3.eth.contract(address=ViewContract_Address,abi=ViewContract_Abi)
a = viewinstance.functions.buysellmultiple(["0x9f8F72aA9304c8B593d555F12eF6589cC3A579A2"],["0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE"],[10000000000000000000],1,0).call()

@kclowes
Copy link
Collaborator

kclowes commented May 1, 2020

It looks like the amount you're sending is more than sys.maxsize on a 64 bit machine. If you use a smaller amount, does that get around the error?

@pipermerriam
Copy link
Member

@kclowes interestingly this is an eth-abi error, which I believe suggests that the total_offset or the pos from this call in the stacktrace is the value exceeding the integer size limit (which causes an error when that value enters some area of the CPython API that uses C instead of pure-python.

...
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\eth_abi\decoding.py", line 95, in push_frame
    self.seek_in_frame(0)
  File "C:\Users\*\AppData\Local\Programs\Python\Python37\lib\site-packages\eth_abi\decoding.py", line 84, in seek_in_frame
    super().seek(self._total_offset + pos, *args, **kwargs)
OverflowError: Python int too large to convert to C ssize_t

I think that's the place to investigate as it isn't exactly clear to me why or how that is happening.

@pipermerriam
Copy link
Member

If you can reproduce the above error in CI then dropping into a pdb might be the most informative way to try and figure out which value is exceeding the limit and then how eth-abi is encountering a that value.

@fubuloubu
Copy link
Contributor

fubuloubu commented Jun 28, 2020

I'm having a similar sort of issue as well, it seems this comes from an issue with web3.codec.decode_abi (which web3.eth.contract uses) doing things differently from how it would if it were using eth_abi.decode_single directly. The former breaks with an error very similar to the above, the latter works perfectly:

>>> data = HexBytes('0x000000000000000000000000000000000000000000000000000000000000000b00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000d00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000280000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005686f727365000000000000000000000000000000000000000000000000000000')
>>> decode_single('(uint256,int128[7],bytes,int128[3][3],uint256,uint256)', data)                                                    
(11, (0, 0, 13, 0, 0, 0, 0), b'horse', ((0, 0, 0), (0, 0, 0), (0, 0, 0)), 0, 0)

So, it seems part of the issue is that web3.eth.contract needs to be refactored to use the newer API from eth-abi: https://eth-abi.readthedocs.io/en/latest/decoding.html#decoding-abi-values

As a note, this is for decoding a function that returns a struct of the following format:

struct W:
    a: uint256
    b: int128[7]
    c: bytes[100]
    e: int128[3][3]
    f: uint256
    g: uint256

fubuloubu added a commit to fubuloubu/vyper that referenced this issue Jun 28, 2020
NOTE: This works in reality (see
ethereum/web3.py#1634 (comment))
however our test suite currently can't handle it because of a bug in
Web3.py
fubuloubu added a commit to fubuloubu/vyper that referenced this issue Jun 28, 2020
NOTE: This works in reality (see
ethereum/web3.py#1634 (comment))
however our test suite currently can't handle it because of a bug in
Web3.py
@pipermerriam
Copy link
Member

Dumping what I figured out thus far.

Taking the value provided by @fubuloubu it looks like we can correctly perform a round trip encode/decode on the provided value.

>>> abi = '(uint256,int128[7],bytes,int128[3][3],uint256,uint256)'
>>> value = (11, (0, 0, 13, 0, 0, 0, 0), b'horse', ((0, 0, 0), (0, 0, 0), (0, 0, 0)), 0, 0)
>>> data = eth_abi.encode_abi((abi,), (value,))
>>> assert value == eth_abi.decode_abi((abi,), data)

The original example also works if we split the ABI up into it's component parts rather than providing as a tuple-type

>>> data = HexBytes('0x000000000000000000000000000000000000000000000000000000000000000b00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000d00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000280000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005686f727365000000000000000000000000000000000000000000000000000000')
>>> decode_abi(('uint256', 'int128[7]', 'bytes', 'int128[3][3]', 'uint256', 'uint256'), data)
(11, (0, 0, 13, 0, 0, 0, 0), b'horse', ((0, 0, 0), (0, 0, 0), (0, 0, 0)), 0, 0)

So I think the problem here is that when a function returns a singular tuple ABI type we aren't handling it properly.

I think we need to look at doing two things.

  1. How could we provide a better error message. We can at minimum detect when the offset value decoded from the stream is obviously too large, like it is in this case.
  2. Look at how contract ABI definitions specify their tuple return types and update the web3.py decoding logic to properly handle this case.

@ErikBjare
Copy link

Having this issue as well with the Uniswap v2 contracts 🙁

Is it known when/where this regression (I assume) occurred? Is it in web3.py or eth-abi? I have a thing in prod which I can't update until this is fixed :/

@pipermerriam
Copy link
Member

@ErikBjare can you expand a little, which function, what inputs are resulting in this error?

@pipermerriam
Copy link
Member

At present, the code example provided in #1634 (comment) works correctly for me, and brief exploration into the uniswap v5 contract @ 0x7665fecb393ca45061a6338662751d6cf45a854d didn't appear to have any functions that used tuples for output values.

I'm going to close this because we currently don't have a reproducible example that uses web3.py code paths. If someone has a contract call that they know of that causes this bug, please let us know what contract address, ABI, function name, function inputs, and pip freeze information and we'll re-open this and figure out what is wrong.

Currently, it is possible this was a bug in an older version of web3.py/eth-abi but has since been fixed.

@fubuloubu
Copy link
Contributor

@pipermerriam were you able to run the test case mentioned in #1634 (comment)

We currently have an outstanding failure in Vyper that could be a good target to check if this is fixed in the latest version:
https://github.com/vyperlang/vyper/blob/006968f021b0b0001c0d4fa9465160f1dbfbc453/tests/parser/globals/test_getters.py#L25

@wolovim
Copy link
Member

wolovim commented Jul 24, 2020

@fubuloubu I started poking around your example this morning. I'm not well-versed in this part of the code base, but the eth_abi and web3.codec decode methods were doing what I'd expect:

print(eth_abi.decode_abi(['uint256','int128[7]','bytes','int128[3][3]','uint256','uint256'], data))
print(eth_abi.decode_single('(uint256,int128[7],bytes,int128[3][3],uint256,uint256)', data))
print(w3.codec.decode_abi(['uint256','int128[7]','bytes','int128[3][3]','uint256','uint256'], data))
print(w3.codec.decode_single('(uint256,int128[7],bytes,int128[3][3],uint256,uint256)', data))

# all print:
# (11, (0, 0, 13, 0, 0, 0, 0), b'horse', ((0, 0, 0), (0, 0, 0), (0, 0, 0)), 0, 0)

Do you remember one of these failing or have I got the context wrong?

@fubuloubu
Copy link
Contributor

fubuloubu commented Jul 24, 2020

Yeah, one of those was failing before (web3.codec.decode_abi from web3.eth.contract), so I'll give it a shot again and see if our issues are solved by... Some newer release of web3. (Unsure when it was fixed 😬)

@fubuloubu
Copy link
Contributor

Nope, still an issue:

$ python --version
Python 3.8.4
$ pip freeze | grep web3
web3==5.12.0

# I removed the `xfail` for the test case here
$ pytest tests/parser/globals/test_getters.py
...
    def test_getter_code(get_contract_with_gas_estimation_for_constants):
        getter_code = """
    struct W:
        a: uint256
        b: int128[7]
        c: Bytes[100]
        e: int128[3][3]
        f: uint256
        g: uint256
    x: public(uint256)
    y: public(int128[5])
    z: public(Bytes[100])
    w: public(HashMap[int128, W])
    
    @external
    def __init__():
        self.x = as_wei_value(7, "wei")
        self.y[1] = 9
        self.z = b"cow"
        self.w[1].a = 11
        self.w[1].b[2] = 13
        self.w[1].c = b"horse"
        self.w[2].e[1][2] = 17
        self.w[3].f = 750
        self.w[3].g = 751
        """
    
        c = get_contract_with_gas_estimation_for_constants(getter_code)
        assert c.x() == 7
        assert c.y(1) == 9
        assert c.z() == b"cow"
>       assert c.w(1)[0] == 11  # W.a

tests/parser/globals/test_getters.py:54: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/base_conftest.py:24: in __call__
    return self.__prepared_function(*args, **kwargs)
tests/base_conftest.py:42: in __prepared_function
    return getattr(self._function(*args), modifier)(modifier_dict)
site-packages/web3/contract.py:954: in call
    return call_contract_function(
site-packages/web3/contract.py:1507: in call_contract_function
    output_data = web3.codec.decode_abi(output_types, return_data)
site-packages/eth_abi/codec.py:181: in decode_abi
    return decoder(stream)
site-packages/eth_abi/decoding.py:127: in __call__
    return self.decode(stream)
site-packages/eth_utils/functional.py:45: in inner
    return callback(fn(*args, **kwargs))
site-packages/eth_abi/decoding.py:173: in decode
    yield decoder(stream)
site-packages/eth_abi/decoding.py:127: in __call__
    return self.decode(stream)
site-packages/eth_abi/decoding.py:145: in decode
    value = self.tail_decoder(stream)
site-packages/eth_abi/decoding.py:127: in __call__
    return self.decode(stream)
site-packages/eth_utils/functional.py:45: in inner
    return callback(fn(*args, **kwargs))
site-packages/eth_abi/decoding.py:173: in decode
    yield decoder(stream)
site-packages/eth_abi/decoding.py:127: in __call__
    return self.decode(stream)
site-packages/eth_abi/decoding.py:144: in decode
    stream.push_frame(start_pos)
site-packages/eth_abi/decoding.py:95: in push_frame
    self.seek_in_frame(0)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <eth_abi.decoding.ContextFramesBytesIO object at 0x7f1a9cd3a860>, pos = 0, args = (), kwargs = {}

    def seek_in_frame(self, pos, *args, **kwargs):
        """
        Seeks relative to the total offset of the current contextual frames.
        """
>       self.seek(self._total_offset + pos, *args, **kwargs)
E       OverflowError: Python int too large to convert to C ssize_t

site-packages/eth_abi/decoding.py:84: OverflowError

@pipermerriam
Copy link
Member

pipermerriam commented Jul 24, 2020

@fubuloubu did you see my analysis of that issue here: #1634 (comment)

From what I could tell, it looks like the ABI type is incorrect but what I can't tell is whether that is a web3.py problem or the ABI itself is not correct.

I'm curious if we can get the ABI produced by that contract so that we can verify/reproduce this using web3 code.

@fubuloubu
Copy link
Contributor

ABI:

[
  {'outputs': [],
   'inputs': [],
   'stateMutability': 'nonpayable',
   'type': 'constructor'},
  {'name': 'x',
   'outputs': [{'type': 'uint256', 'name': ''}],
   'inputs': [],
   'stateMutability': 'view',
   'type': 'function'},
  {'name': 'y',
   'outputs': [{'type': 'int128', 'name': ''}],
   'inputs': [{'type': 'uint256', 'name': 'arg0'}],
   'stateMutability': 'view',
   'type': 'function'},
  {'name': 'z',
   'outputs': [{'type': 'bytes', 'name': ''}],
   'inputs': [],
   'stateMutability': 'view',
   'type': 'function'},
  {'name': 'w',
   'outputs': [{'type': 'tuple',
     'name': '',
     'components': [{'type': 'uint256', 'name': 'a'},
      {'type': 'int128[7]', 'name': 'b'},
      {'type': 'bytes', 'name': 'c'},
      {'type': 'int128[3][3]', 'name': 'e'},
      {'type': 'uint256', 'name': 'f'},
      {'type': 'uint256', 'name': 'g'}]}],
   'inputs': [{'type': 'int128', 'name': 'arg0'}],
   'stateMutability': 'view',
   'type': 'function'},
]

@pipermerriam
Copy link
Member

pipermerriam commented Jul 24, 2020

Here is what I know so far.....

In [35]: w3.codec.encode_single('uint256', 1234)
Out[35]: b'\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\x04\xd2'

In [36]: w3.codec.encode_abi(['uint256',], [1234,])
Out[36]: b'\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\x04\xd2'

Here we see the behavior for fixed size value encoding. The singular/plural encodings are identical.

Next lets look at variable length encodings.

In [41]: w3.codec.encode_single('uint256[]', [1234, 4321])
Out[41]: b'\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\x02\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\x04\xd2\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\x10\xe1'

In [42]: w3.codec.encode_abi(['uint256[]',], [[1234, 4321]])
Out[42]: b'\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\x02\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\x04\xd2\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\x10\xe1'

If we try to decode the output from the encode_single call with decode_abi things blow up with loud stacktraces.

In [45]: w3.codec.decode_abi(['uint256[]'], w3.codec.encode_single('uint256[]', [1234, 4321]))
---------------------------------------------------------------------------
InsufficientDataBytes                     Traceback (most recent call last)
... <STACKTRACE OMITTED HERE>

So now looking at the tuple example, what I believe we are seeing is an incorrectly encoded output from the smart contract.

If we take a look at a simple example, like the name() function found on most token contracts, I think we can extrapolate what the correct behavior here should be. The name() method on ERC contracts returns a string type. To decode this output we have to do w3.codec.decode_abi(['string'], data). That means our output is not just the string, but an ABI list with a single element of the encoded string.

Now looking at the provided tuple example: The contract ABI does not match the returned data.

  • What the ABI "claims" is being returned:
    • ['(uint256,int128[7],bytes,int128[3][3],uint256,uint256)']
  • What is actually being returned:
    • ['uint256', 'int128[7]', 'bytes', 'int128[3][3]', 'uint256', 'uint256']

So I think that what we are seeing is vyper not correctly encoding the return value for this function.

But maybe I'm incorrect

I did not look to see what solidity does for singular tuple type returns so I think that is the next step needed before a decision can be made on what the correct behavior here should be.

@pipermerriam pipermerriam reopened this Jul 24, 2020
@fubuloubu
Copy link
Contributor

fubuloubu commented Jul 25, 2020

@pipermerriam Turns out you were right, in a way.

Same contract in Solidity:

// SPDX-License-Identifier: GPL-3.0
pragma solidity >=0.4.19 <0.7.0;
pragma experimental ABIEncoderV2;

contract Test {
    struct W {
        uint a;
        int128[7] b;
        bytes c;
        int128[3][3] e;
        uint f;
        uint g;
    }

    uint256 public x;
    int128[5] public y;
    bytes public z;
    mapping(int128 => W) public w;

    constructor() public {
        x = 7 wei;
        y[1] = 9;
        z = "cow";
        w[1].a = 11;
        w[1].b[2] = 13;
        w[1].c = "horse";
        w[2].e[1][2] = 17;
        w[3].f = 750;
        w[3].g = 751;
    }
}

ABI:

[
  {"inputs": [],
   "stateMutability": "nonpayable",
   "type": "constructor"},
  {"inputs": [{"internalType": "int128", "name": "", "type": "int128"}],
   "name": "w",
   "outputs": [
     {"internalType": "uint256", "name": "a", "type": "uint256"},
     {"internalType": "bytes", "name": "c", "type": "bytes"},
     {"internalType": "uint256", "name": "f", "type": "uint256"},
     {"internalType": "uint256", "name": "g", "type": "uint256"}
    ],
    "stateMutability": "view",
    "type": "function"},
   {"inputs": [],
    "name": "x",
    "outputs": [{"internalType": "uint256", "name": "", "type": "uint256"}],
    "stateMutability": "view",
    "type": "function"},
   {"inputs": [{"internalType": "uint256", "name": "", "type": "uint256"}],
    "name": "y",
    "outputs": [{"internalType": "int128", "name": "", "type": "int128"}],
    "stateMutability": "view",
    "type": "function"},
   {"inputs": [],
    "name": "z",
    "outputs": [{"internalType": "bytes", "name": "", "type": "bytes"}],
    "stateMutability": "view",
    "type": "function"}
]

The difference is that Solidity doesn't even use tuple when returning a single struct, and abjectly skips parts of the struct it doesn't like (see missing b and e in the above ABI). It will return a named tuple object if another struct is defined within a struct that is being returned:

struct Y {
    uint a;           
    uint b;
    bytes c;
}
struct X {
    Y y;
}

X public x;

ABI (modified):

"outputs": [
  {"components": [
      {"internalType": "uint256", "name": "a", "type": "uint256"},
      {"internalType": "uint256", "name": "b", "type": "uint256"},
      {"internalType": "bytes", "name": "c", "type": "bytes"}
    ],
    "internalType": "struct Test.Y",
    "name": "y",
    "type": "tuple"}
  ]

I would broadly state that we have found undefined behavior which explains this bug, as Solidity is taking a shortcut of not returning a tuple when a composite type is being returned by itself (struct or tuple), even when a tuple is being explicitly coded in the return of a function:

function return_all_in_w(int128 idx)
    public
    returns (
        uint a,
        int128[7] memory b,
        bytes memory c,
        int128[3][3] memory e,
        uint f,
        uint g,
    )
{
    a = w[idx].a;
    b = w[idx].b;
    c = w[idx].c;
    e = w[idx].e;
    f = w[idx].f;
    g = w[idx].g;
}

NOTE: The ABI for this method correctly returns all of the members of w, unlike the what the public getter would return.


So, for Vyper we indeed have a bug in not showing the proper ABI, but I would argue that it is still legitimate for a contract to return a tuple as that is a valid ABI return type (at least the current ABI specification doesn't explicitly disallow it). That behavior should be tested.


I would also say broadly that the error message for this bug is still an issue that deserves to be solved in it's own right.

@pipermerriam
Copy link
Member

@fubuloubu

I would also say broadly that the error message for this bug is still an issue that deserves to be solved in it's own right.

Definitely, this needs to be fixed regardless of what decision gets made on how to handle singular tuple types.

As for what to do about methods that return singular tuple types with variable size elements, are we in agreement that the output encoding produced by vyper is missing the "wrapper" that other variable size return values exhibit when returned from the EVM?

@fubuloubu
Copy link
Contributor

fubuloubu commented Jul 25, 2020

As for what to do about methods that return singular tuple types with variable size elements, are we in agreement that the output encoding produced by vyper is missing the "wrapper" that other variable size return values exhibit when returned from the EVM?

Actually, I figured out that it is actually the opposite: if the value returned is a complex type (such as a struct or tuple), the convention is not to wrap it. I think we're seeing that our ABI encoder did the right thing, but what we output for the ABI is wrong.


EDIT: Sorry I edited the previous comments one too many times lol

@pipermerriam
Copy link
Member

Issue for better exception messaging in eth-abi: ethereum/eth-abi#142

@pipermerriam
Copy link
Member

It appears this issue is not a problem with web3.py and I am now re-closing this. Happy to re-visit/re-open if someone feels differently or there is something that web3.py could be doing in this case to mitigate.

@fubuloubu
Copy link
Contributor

Would also like to note that a function returning a tuple directly is most likely untested behavior, because no Ethereum smart contract language implements it (unless it is tested separately in eth_abi, which seems likely)

@olaem71
Copy link

olaem71 commented Jun 8, 2021

how to set amount to buy...everything works fine but it stops at approving then preparing bot

@tshirtman
Copy link

Got this error as well while trying to call the name() method of this contract, the value as seen by etherscan is 0x4d616b6572000000000000000000000000000000000000000000000000000000 which can be decoded as ascii to Maker\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, but when when call_contract_function does
output_data = web3.codec.decode_abi(output_types, return_data) (output_types being ['string]') we end up in HeadTailDecoder.decode with start_pos being an impossibly big number (35000214728014336407470934256024915466671168103574785725052718905953538277376) which is not accepted by seek() in the seek_in_frame method a few frames below.

i think the value should be decoded as "Maker", but i'm not very knowledgeable in the whole spec, so not sure what goes wrong in getting us there, or if the contract is just wrong and should be ignored.

@ErikBjare
Copy link

@tshirtman The MKR contract uses a different (old?) version of the ERC20 spec where name() returns a bytes32 and not a string. You need to handle that case separately.

See this PR for how we handle it in uniswap-python: uniswap-python/uniswap-python#191

@tshirtman
Copy link

@ErikBjare Will look into it, thanks for the tip!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants