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

Unable to get rpc data #42

Open
antazoey opened this issue Oct 4, 2021 · 2 comments
Open

Unable to get rpc data #42

antazoey opened this issue Oct 4, 2021 · 2 comments

Comments

@antazoey
Copy link
Contributor

antazoey commented Oct 4, 2021

Steps to reproduce

Using python,

self._client = Client()
self._client.eth.block_number()

>>> Network might be unstable, try again later.
 Reason: b'cannot use a string pattern on a bytes-like object'

Digging in, it looks like it is trying to use json.loads() on a bytes value of a boolean.rpc_api.py, line 142 returns the bytes boolean from response.value and runtime.py trys to call json.loads() on it. I am not sure by observing the data what is supposed to be happening.

Expected behavior

I should get a dict response representing the json-rpc data

Actual behavior

It errors

System configuration

Incubed version

3.3.2

Request

I have tried block_numer() and gas_price()

Incubed Configuration

default

Chain

mainnet and goerli

@leonardotc
Copy link
Contributor

leonardotc commented Oct 25, 2021

Hello @unparalleled-js

Thank you for reporting (and debugging). I will try to run some tests here and see what that is about.

@leonardotc
Copy link
Contributor

Hello again @unparalleled-js

So what was happening was that in the python transport we were getting the method (http verb) to fill in the dictionary as a byte array (or uint8*).

            request_params = {
                'url': str(in3_request.url_at(i), 'utf8'),
                'method': str(in3_request.method(), 'utf8'),
                'data': in3_request.payload(),
                'headers': {'Content-type': 'application/json', 'Accept': 'application/json'},
            }

The problem is that internally urllib expect that to be a string. There is a method validator somewhere within the package that uses a regex to validate it (possibly to check if it is a valid http verb). Since it was an array of byte, itd blow up on the invoke of urllib.request.urlopen(request, timeout=timeout). The solution for that was changing the code above to

            request_params = {
                'url': str(in3_request.url_at(i), 'utf8'),
                'method': str(in3_request.method(), 'utf8'),
                'data': str(in3_request.payload(), 'utf8'),
                'headers': {'Content-type': 'application/json', 'Accept': 'application/json'},
            }

In general, id prefer to have it as a const char* from the c but changing those native types can be challenge on the binding because of garbage collection, pointer disposal etc. So yea, if you are relying on that fix you could use the transport class as:

import urllib.parse
import urllib.request

from in3.exception import TransportException
from in3.libin3.transport import In3Request, In3Response


def https_transport(in3_request: In3Request, in3_response: In3Response):
    for i in range(0, in3_request.urls_len()):
        try:
            request_params = {
                'url': str(in3_request.url_at(i), 'utf8'),
                'method': str(in3_request.method(), 'utf8'),
                'data': in3_request.payload(),
                'headers': {'Content-type': 'application/json', 'Accept': 'application/json'},
            }
            request = urllib.request.Request(**request_params)
            timeout = 180000
            with urllib.request.urlopen(request, timeout=timeout) as response:
                if not response.status == 200:
                    raise TransportException('Request failed with status: {}'.format(str(response.status)))
                msg = response.read()
                in3_response.success(i, msg)
        except Exception as err:
            in3_response.failure(i, str(err).encode('utf8'))
    return 0

and set it as the transport function in the Client creation. Since we are wrapping up the btc signers, the fix will be pushed in the same release. I believe this regression was introduced as a consequence of the zksync wallet.

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

2 participants