Skip to content

fixed issue with content hash encoding being undefined#130

Open
Abhimanyu121 wants to merge 1 commit into
ensdomains:masterfrom
Abhimanyu121:origin/master
Open

fixed issue with content hash encoding being undefined#130
Abhimanyu121 wants to merge 1 commit into
ensdomains:masterfrom
Abhimanyu121:origin/master

Conversation

@Abhimanyu121
Copy link
Copy Markdown
Contributor

Encoded content hash was under a key named encodedContenthash .encoded but encodedContenthash was being passed directly.

@makoto
Copy link
Copy Markdown
Member

makoto commented Sep 28, 2021

Thank you for the PR. can you let me know how to replicate the existing issue so that I can test whether this change has fixed it or not.

@Abhimanyu121
Copy link
Copy Markdown
Contributor Author

when we call setContentWithResolver it consequently calls setContenthashWithResolver in which encodedContenthash was being directly passed to setContenthash instead of encodedContentHash. encoded

@rekpero
Copy link
Copy Markdown

rekpero commented Sep 29, 2021

The issue was that when we are calling the setContenthash(...), it should pass the encoded content hash to the contract. But instead of that, the encoding function is returning an object of {encoded: "something", error: null} that is getting passed to the contract which is giving us the error Argument value is not correct from the contract.

This PR fixes this issue

@makoto
Copy link
Copy Markdown
Member

makoto commented Oct 1, 2021

I used your branch and getting this
Screenshot 2021-10-01 at 20 21 32

@Abhimanyu121
Copy link
Copy Markdown
Contributor Author

I used your branch and getting this Screenshot 2021-10-01 at 20 21 32

can you send me what you used as content hash?

@Abhimanyu121
Copy link
Copy Markdown
Contributor Author

Its working fine for me, I used ipfs and arweave hashes
Here are the screenshots
Screenshot 2021-10-02 at 11 03 16 PM

Screenshot 2021-10-02 at 11 04 22 PM

Screenshot 2021-10-02 at 11 03 04 PM

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

Successfully merging this pull request may close these issues.

3 participants