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

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

Closed
cervoneluca opened this issue Feb 25, 2020 · 17 comments · Fixed by #3428
Closed

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

cervoneluca opened this issue Feb 25, 2020 · 17 comments · Fixed by #3428
Labels
1.x 1.0 related issues Bug Addressing a bug

Comments

@cervoneluca
Copy link

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
Copy link
Collaborator

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

@holgerd77
Copy link
Collaborator

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
Copy link
Collaborator

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

@holgerd77
Copy link
Collaborator

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

@holgerd77
Copy link
Collaborator

Ok, will stop here for now.

@cervoneluca
Copy link
Author

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

@holgerd77
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

cgewecke commented Feb 25, 2020

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.

@cgewecke cgewecke added 1.x 1.0 related issues Bug Addressing a bug labels Feb 25, 2020
@cervoneluca
Copy link
Author

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

@holgerd77
Copy link
Collaborator

...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
Copy link
Collaborator

@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
Copy link
Collaborator

@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
Copy link
Collaborator

cgewecke commented Feb 26, 2020

"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:

  • gets the resolver address with Registry.getResolver('luca.cervone.eth'). (It will point to either an old or new resolver type)
  • when calling getContentHash, it's assumed your resolver must be of the new type and will be described by the new ABI.

@holgerd77
Copy link
Collaborator

"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
Copy link
Collaborator

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Bug Addressing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants