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

Add Solidity Custom Error support to Interface #1498

Closed
ricmoo opened this issue Apr 22, 2021 · 7 comments
Closed

Add Solidity Custom Error support to Interface #1498

ricmoo opened this issue Apr 22, 2021 · 7 comments
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published. minor-bump Planned for the next minor version bump.

Comments

@ricmoo
Copy link
Member

ricmoo commented Apr 22, 2021

Solidity has announced and released EIP-838 support for user defined errors in Solidity, so I will be adding support to human-readable ABI, the ABI fragment parser and the Interface in the next minor bump.

@ricmoo
Copy link
Member Author

ricmoo commented Apr 23, 2021

Other Useful Links:

I have this working locally, but have a few other changes I want to roll into this minor bump.

@axic
Copy link

axic commented Apr 27, 2021

Please note that Solidity does not implement EIP-838. That proposed RLP encoding and some prefix, and I can't remember what else was discussed there. Please do not include EIP-838 in the documentation, as that would be misleading users.

The encoding specification is here: https://docs.soliditylang.org/en/v0.8.4/abi-spec.html#errors

@ricmoo
Copy link
Member Author

ricmoo commented Apr 27, 2021

@axic The only mention of RLP in the above EIP-838 spec seems to be a typo, which should have read ABI, no?

Any arguments for the error are RLP encoded in the same way as return values from functions.

Function outputs are ABI encoded, not RLP encoded...

@axic
Copy link

axic commented Apr 27, 2021

It also says the following:

The exact format in which both the selector and the arguments are encoded is to be defined. The Solidity implementation mentioned above leaves room for expansion by prefixing the free-form string with uint256(0).

And that EIP was initially for the reason string, which has been supported in Solidity for quite a bit (revert("string") or require(x, "string")).

I see now that @chriseth commented there, which may be the reason for this confusion 😅

Anyway please just link to the Solidity documentation (and follow the description there), as that discussion never materialised as an EIP.

@ricmoo
Copy link
Member Author

ricmoo commented Apr 27, 2021

Sounds good to me. And I'll stop using the phrase EIP-838 starting... now. :)

Also note, that "Error(string)"has been supported in ethers since it existed, and the changes to fully support the error strings were fairly minor. I've got about 12 features I'm trying to roll into this minor release and am on the last one now, so full support should be out soon. :)

@ricmoo ricmoo changed the title Add EIP-838 support to Interface Add Solidity Custom Error support to Interface Apr 27, 2021
@axic
Copy link

axic commented Apr 27, 2021

Nice!

And thanks for taking the above into consideration, my aim is to avoid potential confusion as that could mean more work for both of us 😅 I wish everything was properly standardised/documented under EIPs, but the process is tedious and was blocked on certain issues regarding the ABI so far.

@frangio
Copy link

frangio commented Apr 29, 2021

@axic, @chriseth implied that Solidity had implemented EIP838 in ethereum/EIPs#838 (comment). Would you mind posting a comment over there explaining that it's not the case?

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels May 30, 2021
@ricmoo ricmoo closed this as completed May 30, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published. minor-bump Planned for the next minor version bump.
Projects
None yet
Development

No branches or pull requests

3 participants