-
Notifications
You must be signed in to change notification settings - Fork 207
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
Polkadot: Reverts return encoded error data #1449
Conversation
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff just some minor things
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still comments open when you requested a review from me
Signed-off-by: xermicus <cyrill@parity.io>
Co-authored-by: Lucas Steuernagel <38472950+LucasSte@users.noreply.github.com> Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please
docs/targets/polkadot.rst
Outdated
SCALE encoding does not allow discriminators larger than 1 byte, the hex-encoded error selector | ||
is provided as the enum variant name in the metadata (the selector could also be calculated by | ||
reconstructing and hashing the error signature based on the enum variant types). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So where does the name go? E.g. Panic
, Error
, or CustomError5
?
I do think this this paragraph is inscrutable. I still don't have a good idea of what is going on here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes me think, do you think it'd be better if the Selector bytes goes into the path of the Variant Type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rewritten it and added an example.
I also removed that the selector can be reconstructed. We don't guarantee this for messages selectors, so I doubt whether we can guarantee this for errors once we introduce custom errors. And including the selector directly is better anyways and makes this unnecessary anyways.
Co-authored-by: Lucas Steuernagel <38472950+LucasSte@users.noreply.github.com> Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
…tadata Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
This is a continuation of #1415.
require()
,assert()
andrevert()
now return error data, according to the Ethereum Solidity documentation. Additionally, many reverts inserted by the compiler now return the correspondingPanic(uint256)
error data, to align Solang closer withsolc
.The error types known to the contract are added in the metadata
lang_error
field. At the moment there are onlyError
andPanic
because we don't support custom errors yet.Refactored revert-related code into a dedicated
codegen
module to. Refactored thepolkadot::errors
into distinct tests, made them less brittle and added assertions for the execution output.I'm now working on a follow-up PR for bubbling up uncaught exceptions (this is why it's already included that in the documentation).