INFURA / infura

Official Public Repository for INFURA
https://infura.io
381 stars 62 forks source link

cannot unmarshal non-string into Go value of type common.Address #189

Open olegabr opened 4 years ago

olegabr commented 4 years ago

Request

$ curl -X POST -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","id":1,"method":"eth_call","params":[{"to":"0x3E0371bcb61283c036A48274AbDe0Ab3DA107a50","data":"0x98445e6f000000000000000000000000c35fe412f77f581d973fc5ca1912fef8082c108d000000000000000000000000000000000000000000000000000000000000cb95","from":null},"latest"]}' https://mainnet.infura.io/v3/MY-PROJECT-ID-HERE

Response

{"jsonrpc":"2.0","id":1,"error":{"code":-32602,"message":"invalid argument 0: json: cannot unmarshal non-string into Go value of type common.Address"}}

It looks like the same request worked just some hours ago.

averyonghub commented 4 years ago

Try the eth_call without from: null in the params payload! Let us know if that works for you!

olegabr commented 4 years ago

Yes! That works:

$ curl -X POST -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","id":1,"method":"eth_call","params":[{"to":"0x3E0371bcb61283c036A48274AbDe0Ab3DA107a50","data":"0x98445e6f000000000000000000000000c35fe412f77f581d973fc5ca1912fef8082c108d000000000000000000000000000000000000000000000000000000000000cb95"},"latest"]}' https://mainnet.infura.io/v3/MY-PROJECT-ID-HERE
{"jsonrpc":"2.0","id":1,"result":"0x0000000000000000000000000000000000000000000000000000000000000000"}
olegabr commented 4 years ago

However, I can not remove the "from":null field from my code, since the web3js lib adds it. I'm using the previous stable web3js version (0.20.X).

ryanschneider commented 4 years ago

Unfortunately, this isn't something we can resolve immediately on our end, as the issue is that geth expects the from field, if present, to be non-null. Later versions of geth have changed to allow an explicit "from":null but we aren't running those releases yet. We are in the process of validating them for upgrade but don't have a schedule yet when the upgrade itself will take place.

In the meantime, if it's truly caused by 0.20.x your only recourse might be to use 1.x or 2.x versions of web3js.

olegabr commented 4 years ago

Thank you for a responce! I just wonder why it worked during two previous years and stopped a day ago :-)

olegabr commented 4 years ago

I've made a patch for web3.js 0.20.7 branch, but it can not be applied since MetaMask injects it's own version which is the vanilla 0.20.7:

Screenshot from 2019-09-24 17-49-38

my patch:

s$ git diff dist/web3.js
diff --git a/dist/web3.js b/dist/web3.js
index 841d09bf..c20fd669 100644
--- a/dist/web3.js
+++ b/dist/web3.js
@@ -3735,6 +3735,9 @@ var inputCallFormatter = function (options){
     if (options.from) {
         options.from = inputAddressFormatter(options.from);
     }
+    if (null === options.from) {
+        delete options.from;
+    }

     if (options.to) { // it might be contract creation
         options.to = inputAddressFormatter(options.to);
ryanschneider commented 4 years ago

I just wonder why it worked during two previous years and stopped a day ago :-)

We haven't changed anything on our end, is it possible the issue you're having is with MetaMask?

egalano commented 4 years ago

@olegabr thanks for the report. I was able to reproduce this issue with a newer version of web3.js as well (1.2.1). I also was able to reproduce this issue against the Cloudflare Ethereum gateway which uses Geth 1.8.x.

The etherscan API also returns the same error so this isn't just an Infura issue this is something that has been around in geth-1.8.x for a while.

Were there any changes in your app that would be surfacing this now?

olegabr commented 4 years ago

I've beat the system! I'm doing

    window.epg.web3.eth.defaultAccount = "0x0000000000000000000000000000000000000000";

before each contract call now and it works like a charm!

I've tried to do it once after the web3 object creation, but MM has replaced it with null by timer in it's internals. That is why it must be set before each contract method call. Since browser javascript engine is guaranteed to be synchronous, MM has no chance to break it in between.

The important detail is that this issue rises only if MM account is not logged in. When it is logged in, the defaultAccount field is initialized with a proper non-null value and geth is happy.

I've faced this issue just now because I have not tested this non-logged in mode for about a year. About a year ago there were not such an issue and the non-logged in MM mode worked well with infura and geth behind it. So, you were right that this problem is originated by geth and is here for some time already.

I have a workaround now and will fix all my plugins, but I still consider this situation as an infura.io fault, since it promised to abstract users from ethereum nodes complexities, essentially, encapsulate it behind API layer and this issue is exactly the incapsulation failure issue. The right infura behaviour should be to sanitize the request before passing it to geth according to the current used geth version requirements.

ryanschneider commented 4 years ago

@olegabr great work debugging the root cause! We agree with sanitizing the request, in fact we were discussing that very idea internally yesterday, so it's on our roadmap now.

But then of course the issue is deciding which items are valid and which aren't. Luckily there's EIP-1474 which makes an attempt at a standardize the RPC request/response payloads, but it has somewhat stalled out:

https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1474.md#eth_call

Although coincidentally the EIP does not mark from as optional. :)

If you have time, please comment on the discussion here https://ethereum-magicians.org/t/eip-remote-procedure-call-specification/1537 as I think it shows the value of this EIP: if it was being strictly followed by all the clients there wouldn't be any discrepancies and your time wouldn't have been wasted chasing down this issue!

olegabr commented 4 years ago

You are an umbrella API for all these clients. De facto, you have the power to establish a standard all dapps would use, hiding all these client discrepancies behind your API layer. Correct me if I'm wrong, but providing a stable API for dapp developers is an essense of the infura.io promise and mission. There is only one way to achieve that: specify such API and hide all client discrepancies under this API interface.

egalano commented 4 years ago

You’re not wrong that we do have an influence. We feel it’s important to approach this collaboratively rather than using our power to assert a standard. We try to do what we can to improve the developer experience. We work with client developers on standards like EIP-1474 and EIP-1767. We can’t force client developers to implement any particular standard though. We are working towards it but can’t be too aggressive without forcing interface changes on users which are not compatible with a local node.

olegabr commented 4 years ago

I've worked directly with geth and parity. They are not compatible. My point is that infura.io is a compatibility layer for dapps and it is a win-win for all:

  1. dapp developers are isolated from client discrepancies
  2. client developers are free to experiment

You should be more aggressive I believe. Not in forsing clients to become compatible, but in positioning yourself as the compatibility layer on top of them.

egalano commented 4 years ago

Yeah as you say, since they aren’t compatible we end up trying to find the least common denominator between them instead of being a superset which would be in the developers best interest. I’d be interested in hearing more of your thoughts on this as I know you’ve been around the ecosystem a long time and have been an active part of our community. Please feel free to email me if you’re willing to discuss this further outside of the scope of this issue. eg@infura.io

ryanschneider commented 4 years ago

Not in forsing clients to become compatible, but in positioning yourself as the compatibility layer on top of them.

While I agree 100% with your intent, we have to be careful: if we work around too many client compatibility issues, then we end up building an implicit lock-in to Infura, which is something we want to avoid. In our opinion if your DApp works w/ Infura it should work with a vanilla client, and vice-versa.

While I understand the current behavior is frustrating, it would be possibly more frustrating if you moved from Infura to your own nodes and ran into lots of little compatibility issues that we were hiding, so IMO it's important for us to work with the community on fixing issues like this for all clients, so actually I think us "forcing the clients to be compatible" is something we should try to do. :)

In your specific case, it's a small change, but for us it's a larger philosophical one: in the past we've been reluctant to make any changes to the incoming RPC requests so as not to lead to possible lock-in. All of this is to say that we are taking your issue seriously but want to make sure we make the right choice for the ecosystem and your and other clients' long-term success.

olegabr commented 4 years ago

Looks like I'm completely wrong here, sorry. The web3.js should be a compatibility layer on top of all nodes and node as a service systems like infura.io. The problem with web3.js is that it's previous and current stable versions can not work with new geth, and they do not release maintainance versions for these 0.20.X and 1.2.X branches.

egalano commented 4 years ago

Have you tried ethers.js? My understanding is that it does a much better job at being what you hoped web3js would be doing here. We also plan to contribute to efforts at this client side compatibility layer.