web3 / web3.js

Collection of comprehensive TypeScript libraries for Interaction with the Ethereum JSON RPC API and utility functions.
https://web3js.org/
Other
19.25k stars 4.94k forks source link

web3.eth.ens.getContent seems to do not work properly #3392

Closed cervoneluca closed 4 years ago

cervoneluca commented 4 years ago

Expected behavior

Given a ENS name that has an associated content, I should be able to receive the content hash when I call the web3.eth.ens.getContent('myName');

Actual behavior

I run this code (in typescript)

const web3 = new Web3(new Web3.providers.HttpProvider(
   "https://mainnet.infura.io/v3/my-infura-key"
));
web3.eth.ens.getContent('luca.cervone.eth')
   .then((result: String) => {
      console.log(result);
   }).catch((error: Error) => {
      console.log(error);
   });

The above code returns the following error:

Uncaught (in promise) Error: Returned values aren't valid, did it run Out of Gas? You might also see this error if you are not using the correct ABI for the contract you are retrieving data from, requesting data from a block number that does not exist, or querying a node which is not fully synced.
    at ABICoder.decodeParameters (index.js?bbaf:239)
    at Contract._decodeMethodReturn (index.js?d100:557)
    at Method.outputFormatter (index.js?d100:910)
    at Method.formatOutput (index.js?6248:167)
    at sendTxCallback (index.js?6248:596)
    at eval (index.js?176c:147)
    at XMLHttpRequest.request.onreadystatechange (index.js?8148:110)

I attach a screenshot proving that the name as an associated content

Schermata 2020-02-25 alle 11 11 45
holgerd77 commented 4 years ago

Just tested this locally, can confirm the error message posted.

holgerd77 commented 4 years ago

Not such an ENS-pro, just jumping through the RPC calls by adding some console.log statements here within the HTTP provider code to get payload and result from the calls done (I think) in the Resolver.

Following results:

  1. eth_getBlockByNumber -> result seems fine
  2. net_version -> seems fine
  3. eth_getBlockByNumber -> seems fine
  4. eth_call -> seems fine
  5. eth_call-> eventually/likely broken, not understanding the underlying logic though yet

Payload and results:

{
  jsonrpc: '2.0',
  id: 5,
  method: 'eth_call',
  params: [
    {
      data: '0x2dff6941a1f73e526ac891049ab614b93d732548e97a6d5ad36911f27f2655373eb13c15',
      from: undefined,
      gasPrice: undefined,
      gas: undefined,
      to: '0x4976fb03c32e5b8cfe2b6ccb31c09ba78ebaba41'
    },
    'latest'
  ]
}
{ jsonrpc: '2.0', id: 5, result: '0x' }
holgerd77 commented 4 years ago

Error is thrown here in the ABI code with bytes being an empty string.

holgerd77 commented 4 years ago

Latest call goes to the ENS contract: https://etherscan.io/address/0x4976fb03c32e5b8cfe2b6ccb31c09ba78ebaba41

holgerd77 commented 4 years ago

Ok, will stop here for now.

cervoneluca commented 4 years ago

So, it seems to be actually a Web3.js bug, I'm I right?

holgerd77 commented 4 years ago

No, I wouldn't confirm this yet.

Have some more time, will do some further analysis, this needs eventually to be continued.

The address from above (0x49...) isn't actually the main (new, post bug fix) ENS address, which is: 0x00000000000c2e074ec69a0dfb2997ba6c7d2e1e

Instead this address is derived from the 4th eth_call, which provides the address as a result:

{
  jsonrpc: '2.0',
  id: 4,
  method: 'eth_call',
  params: [
    {
      data: '0x0178b8bfa1f73e526ac891049ab614b93d732548e97a6d5ad36911f27f2655373eb13c15',
      from: undefined,
      gasPrice: undefined,
      gas: undefined,
      to: '0x00000000000c2e074ec69a0dfb2997ba6c7d2e1e'
    },
    'latest'
  ]
}
{
  jsonrpc: '2.0',
  id: 4,
  result: '0x0000000000000000000000004976fb03c32e5b8cfe2b6ccb31c09ba78ebaba41'
}

Just stating for the moment, not sure what this means yet.

holgerd77 commented 4 years ago

I wonder if the 0x49 address is the address of the old public resolver and this might be wrong and it should rather be the new resolver address (see docs) returned? Not confirmed though.

cervoneluca commented 4 years ago

I have successfully changed the resolver to the new one, as it is possible to see in the screenshoot and also confirmed by executing this line of code (that returns now 0xDaaF96c344f63131acadD0Ea35170E7892d3dfBA):

web3.eth.ens.resolver('luca.cervone.eth').then(console.log);

However, the error is always there

Schermata 2020-02-25 alle 14 57 54
cgewecke commented 4 years ago

Looked at this a bit and think it is a Web3 bug.

There's been an API change at ENS for this method...

If you:

...you'll receive a bytes32 hash for luca.cervone.eth that looks like this:

0xe3010170122059948439065f29619ef41280cbb932be52c56d99c5966b65e0111239f098bbef

That value has to be parsed using a utility like decodeContenthash at @ensdomains/ui to produce an output like this:

{
 "protocolType": "ipfs",
 "decoded": "QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn"
}

TLDR; there are lots of changes.

It might make sense to look at the ongoing work at @ensdomains/ui and think about how it might be integrated here - maybe by wrapping it more directly and pulling all resources from it.

cervoneluca commented 4 years ago

Thank you @cgewecke ! I'll try to solve it with the steps you described!

holgerd77 commented 4 years ago

...you'll receive a bytes32 hash for luca.cervone.eth that looks like this:

0xe3010170122059948439065f29619ef41280cbb932be52c56d99c5966b65e0111239f098bbef

Short correction: this is actually longer than 32 bytes, in this case 38 bytes.

Dropped on this to count by skimming through this article ("The New ENS Manager Now Supports EIP1577 contenthash") by the ENS team giving some context on the contenthash introduction.

There it is stated:

we have a functionsetContent(bytes32 node, bytes32 hash) to set content, but we haven’t standardised the format for this field, and it’s limited to 32 bytes — rendering it unsuitable for use with IPFS

So they are explicitly mentioning IPFS not working with the old content method.

As stated in EIP-1577 content will further co-exist along contenthash but is now deprecated. So it seems to be the correct way for implementing this here in the library is to add additional contenthash methods to the ENS API?

@cgewecke I am not completely grasping the resolver part yet. Is it correct that the resolver is not hard-coded into Web3 but received on a per-domain basis via call? Can we switch here on a per-case basis to select the correct ABI?

cgewecke commented 4 years ago

@holgerd77 Thanks for finding that blog post! That's the missing puzzle piece :)

Is it correct that the resolver is not hard-coded into Web3 but received on a per-domain basis via call? Can we switch here on a per-case basis to select the correct ABI?

Yes, I think the resolver address is obtained per ens name, and the resolver contract instance is instantiated in contracts/Registry.js like this:

https://github.com/ethereum/web3.js/blob/ccc229e03edf805498d67ecbacbf69b0cec82e56/packages/web3-eth-ens/src/contracts/Registry.js#L525-L532

So, following your analysis, perhaps a fix would be to extend the API here with something like:

web3.eth.ens.getContentHash(name: string) : Promise<string> 

...and switch abis on the basis of whether getContent or getContentHash is being called?

holgerd77 commented 4 years ago

@cgewecke hmm, not sure about the ABI switch via method, "getContent" is also present on the new resolver, wouldn't this then be the wrong ABI and shouldn't therefore the ABI taken depending on the resolver?

Or, haven't looked how this is implemented, but can't we just switch completely to the new resolver and take the new ABI accordingly?

cgewecke commented 4 years ago

"getContent" is also present on the new resolver

Ah weird - the ABI I copied from etherscan for the resolver at 0xDaaF96c344f63131acadD0Ea35170E7892d3dfBA doesn't have the old method, called "content". It only has "contenthash". (ABI is in a gist here).

There's a contract marked "ENS Old Resolver" on Etherscan at 0x1da022710df5002339274aadee8d58218e9d6ab5 for comparison. It looks like it only has "content".

Are you seeing both in the same contract somewhere?

Was assuming the way this works is that Web3:

holgerd77 commented 4 years ago

"getContent" is also present on the new resolver

Are you seeing both in the same contract somewhere?

Ah no, sorry, that was an unproven assumption by me, thought it wouldn't be removed if in a deprecated state, but eventually deprecated in this context means: it can still (and only) be used via the old resolver contract. Sorry for the confusion.

To decide on the resolver ABI by function name doesn't seem like a robust solution to me, this would lead to erratic decisions when a shared (between new and old resolver) functionality is called (or am I missing something here? Still very cautious on this whole ENS topic.).

The (static) resolver ABI is integrated here:

https://github.com/ethereum/web3.js/blob/ccc229e03edf805498d67ecbacbf69b0cec82e56/packages/web3-eth-ens/src/contracts/Registry.js#L531

I think it should rather be decided on which ABI to be selected by the resolver address returned? Should the resolver addresses eventually be added to the config.js file?

There is also an ENS get contract ABI functionality - don't have the whole picture yet - is it eventually possible with this to dynamically fetch the matching ABI? Or otherwise request the resolver version number, if something like this exists and decide upon the ABI to choose by the resolver version?

cgewecke commented 4 years ago

To decide on the resolver ABI by function name doesn't seem like a robust solution to me, this would lead to erratic decisions when a shared (between new and old resolver) functionality is called

This is a good point. Also looks like the resolver implements EIP 165 (interface detection) supportsInterface - there might be several options here to see if the call is valid...