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

Unexpected behavior when calling web3.eth.ens.getContenthash #2782

Closed
jeluard opened this issue May 6, 2019 · 7 comments
Closed

Unexpected behavior when calling web3.eth.ens.getContenthash #2782

jeluard opened this issue May 6, 2019 · 7 comments
Labels
1.x 1.0 related issues Enhancement Includes improvements or optimizations

Comments

@jeluard
Copy link

jeluard commented May 6, 2019

Description

web3.eth.ens.getContenthash does not return the decoded content as documented.
Not clear if the doc or the behavior is wrong.

Expected behavior

web3.eth.ens.getContenthash('pac-txt.eth').then((result) => {
    console.log(result);
});
> 'QmdTEBPdNxJFFsH1wRE3YeWHREWDiSex8xhgTnqknyxWgu'

Actual behavior

web3.eth.ens.getContenthash('pac-txt.eth').then((result) => {
    console.log(result);
});
> '0xe30101701220e08ea2458249e8f26aee72b95b39c33849a992a3eff40bd06d26c12197adef16'
@nivida
Copy link
Contributor

nivida commented May 6, 2019

This looks like an issue of the AbiCoder from @ricmoo who we are using. I will contact him asap.

@nivida nivida added the Needs Clarification Requires additional input label May 6, 2019
@ricmoo
Copy link
Contributor

ricmoo commented May 6, 2019

The value returned from the Solidity contract is a bytes, so the above value is correct from the ABICoder’s point of view.

If the goal of that function is to return the IPFS multihash, it needs to be parsed by an IPFS multihash library.

See EIP-1577: https://eips.ethereum.org/EIPS/eip-1577

And here is our implementation we use in Meeseeks: https://github.com/ricmoo/meeseeks-app/blob/master/index.html#L958

Hope that helps. :)

@jeluard
Copy link
Author

jeluard commented May 7, 2019

Thanks @ricmoo !

Probably decoding contenthash should be left to end users as it can be hairy if you want full support. So I would say current behavior is fine but doc should be updated?

WDYT @nivida ? If that sounds good I will go ahead and update the doc to fix the example and provide relevant context.

@ricmoo
Copy link
Contributor

ricmoo commented May 7, 2019

I will be adding a small ipfs library to ethers.js in the near future, which could also be wrapped by Web3.js to do this conversion too. But that is a bit further down on my todo list.

@nivida
Copy link
Contributor

nivida commented May 7, 2019

Thanks, @ricmoo for the details.

If the goal of that function is to return the IPFS multihash, it needs to be parsed by an IPFS multihash library.

I could handle this with custom methods instead of using internally the Contract API in the ENS module.

I will be adding a small ipfs library to ethers.js in the near future, which could also be wrapped by Web3.js to do this conversion too. But that is a bit further down on my todo list.

We could probably work together on one module which isn't ethers or Web3 to specific and use it later in both projects?

WDYT @nivida ? If that sounds good I will go ahead and update the doc to fix the example and provide relevant context.

Sounds great! @jeluard

@nivida nivida added Documentation Relates to project wiki or documentation and removed Needs Clarification Requires additional input labels May 7, 2019
@jeluard
Copy link
Author

jeluard commented May 7, 2019

FYI there is an existing library handling content-hash parsing: https://github.com/pldespaigne/content-hash
It's used by e.g. manager.ens.domains. not sure how comprehensive it is.

@nivida nivida added the 1.x 1.0 related issues label Jun 20, 2019
@nivida nivida added Enhancement Includes improvements or optimizations and removed Documentation Relates to project wiki or documentation labels Oct 23, 2019
@ryanio ryanio mentioned this issue May 8, 2020
13 tasks
@cgewecke
Copy link
Collaborator

cgewecke commented Jun 9, 2020

1.2.8 adds contentHash support. Docs here

@cgewecke cgewecke closed this as completed Jun 9, 2020
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 Enhancement Includes improvements or optimizations
Projects
None yet
Development

No branches or pull requests

4 participants