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

Solang contracts will return error data in the future #504

Open
xermicus opened this issue Jul 26, 2023 · 4 comments
Open

Solang contracts will return error data in the future #504

xermicus opened this issue Jul 26, 2023 · 4 comments

Comments

@xermicus
Copy link

xermicus commented Jul 26, 2023

Until now, contracts compiled with Solang in general did not revert, but trap on any runtime problems. I am now working on fixing this, so what is returned if the contract reverted corresponds 1:1 to what a Solidity contract on EVM would return (as much as this is feasible) . There is a series of changes to realize this.

After these PRs are merged, the general process of decoding the output data for Solang Solidity contracts is as follows:

EDIT: The process of decoding this data is documented here

@xermicus
Copy link
Author

(This is different from how it works for ink! contracts, because Solidity has this concept of Error selectors and does bubble up reverts. But it shouldn't be a problem for the UI because the Language is contained in the metadata)

@athei
Copy link
Contributor

athei commented Aug 23, 2023

As long as the returned type is expressed correctly in the metadata it should be decodeable without any change to the UI? The problem is that the message return type doesn't include the error? Can't we just include it there?

@xermicus
Copy link
Author

xermicus commented Aug 23, 2023

For now, the message return type just specifies what will be returned in the non-revert case. If the contract reverts, the error data can be decoded according to the language error field.

While including the error in the message return type is technically possible. That would require the contract to wrap the return data in a result. Which is very orthogonal to the Solidity Language. That type doesn't exist there; instead, Solidity has "error selectors". It'll also break existing contracts, where the 4 error bytes selector might be manually matched. This adds code size and execution overhead to the contract, as well as complexity in the compiler (requiring lots of changes in many parts of the codegen). Resulting in an odd Solidity language dialect, just because of an ink! metadata issue, which I'd rather avoid. On a higher level, the result would be that Solang contracts work around ink! specific metadata things at runtime, which seems sub-optimal. Because the contract output is always just bytes. Without the metadata this can't be decoded anyways (*). So after all, I valued having this single special case in the metadata decoding less terrible than needlessly encoding all contract output into result as the better tradeoff.

* Unless we follow the Solidity model with the error selectors. But because in Solidity, contracts always bubble up uncaught errors, error data can't be trusted, which limits the benefit of this model.

There's also a related issue and discussion in ink!. If the next version of the ink metadata would allow to specify "if the contract execution succeeds, decode type X, if it reverts, decode type Y", the situation would improve a bit. Basically this would allow all languages targeting the contract pallets to re-use the ink! metadata, regardless whether they have a "Result"-like type or not. However, for Solidity error selectors, several non-trivial issues still remain:

  • The enum discriminator is 4 bytes but SCALE has only 1 byte for enum discriminators
  • Returned data can be empty (Solidity has no Option type)
  • Wrapping the error into an Option or some SCALE enum doesn't work because of the error bubbling behavior.

@athei
Copy link
Contributor

athei commented Aug 24, 2023

Thanks for clarifying.

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

No branches or pull requests

2 participants