ink: Implement error forwarding

You are completely right about 1. This is already implemented. The UIs just don’t make use of it right now.

Events are something that are in the domain of the individual Dapps that build on ink!. A generic UI would just show all events generated by a contract. They can be decoded automatically by using the ABI file. Please keep in mind that the target audience of those generic UIs are contract developers and not users. For developers those the decoded events make sense.

The question here we prefer 2.a or 2.b? And who will add events to UI?=)

Users will interact with contracts through custom Dapps that can make sense of those events and display them in a friendly manner.

Ah I think I finally get what you mean. I will inspect it.

@seanyoung What is the reason you think a dedicated field a la unpacked_result is necessary in the metadata? Shouldn’t the return type Result<…, …> already be enough information for UI’s?

I only wanted to point out that if there is an metadata attribute called revert_on_err then that’s not very descriptive, I think. I agree that if is possible to not have a metadata attribute, then that is even nicer.

I will try to describe the whole flow, how it can be implemented and used, based on the conversations above.

Let’s highlight that we have two types of errors that can be used for error forwarding/debugging: ContractTrapped and ContractReverted. I guess both of them will have a buffer for data in the future based on this comment.

I guess what I am missing is what format I should use; is there a standard encoding for this which e.g. polkadot-js (contract-api) can decode or is up to the client code to manually decode the error?

Based on the type of error you need to decide, how to decode it.

ContractReverted can be returned via return_value function with REVERT flag. It means that the developer can return this error only in the expected execution flow. So I guess it is normal if we introduce an unspoken rule, that the developer must return only the type described in ABI via return_value function(It means that if the method X in ABI returns Result<.., ErrorA>, it must return only Result<.., ErrorA> and not Result<.., ErrorB> or String). In the case of the return_on_err attribute, this restriction will be implemented automatically during compilation time, because ink! will add a code to encode the result(So it passes the encoded data into the buffer of ContractReverted. For encoding ink! and the contract-pallet are using parity-scale-codec, so other applications must use the same algorithm for decoding).

The developer must follow this rule because:

• During cross contract-calls, the ContractReverted will be converted into CalleeReverted error, but it is a positive error, it only means that the child contract didn’t update the state. The parent contract must decide what to do with the result by self. If the developer broke that rule, it means that we can’t decode the data from CalleeReverted, but ink!(And I guess other languages) will try to do it automatically based on the ABI of the child contract and it will cause a DecodeError which already can be emitted via ContractTrapped.
• In the case of direct execution of the method of some contract, UI/DApp will don’t know how to decode the result, when the ABI says that it must be X. It still can represent it like a string in not typed languages(or like another analog of Result with string as error), but it will be not usable and complicated.

The ContractTrapped error must be used only for unexpected behaviors. Like divide by zero, DecodeError and etc. The data from that error can be interpreted only like a string(maybe bytes in some cases), but no more. It is an unexpected error so we can’t unify it.

Based on the above, it is clear how the revert_on_err attribute can be implemented. If the user specifies that attribute, it means that the return type is a Result. ink! can add the code to check that if the result is an error, let’s encode(Based on the return type of the method) it and pass it into return_value with the REVERT flag. If it is a simple call, we will propagate this result to the user. If it is a cross call, we will propagate this result into CalleeReverted, and ink! will interpreter this error like a positive result and will try to decode the data from this error. If the decode is successful, we will return the result to the parent contract, overwise we will panic and emit ContractTrapped.

The developer of the contract can use return_value with the REVERT flag in other cases not related to Result. But I don’t see at the moment a valid use case for that(which will be better than usage with Result)=D Maybe the use case where you can decide to revert the changes but you want to pass some data back to the user/contract… I don’t know) But I think the rule that the type of passed data into return_value must be like in ABI is the core rule.

I know that. I meant let’s imagine, that we already implemented that=) And the question is what will do contract-pallet when a child’s call will return with REVERT flag?

It will revert. However, I am not sure if the planned ink! design for this feature is in a good shape yet.

In a meeting @cmichi and me concluded to put forward with the following design for ink!:

• Introduce a new ink! attribute, called revert_on_err: bool that is optional and which can be applied to both, ink! constructors and messages.
• If applied to ink! constructors or messages ink! includes a compile-time check to guard that the return type of the applied to method is in fact a Result type and furthermore ink! will revert the contract execution upon yielding Result::Err with appropriate return flags to SEAL.
• A new optional metadata field is added to ink! messages and constructors: revert_on_err: Option<bool>. Optional means that it can be missing. Note that it is kind of specific to ink!/Rust and not very useful for other languages like Solang which is why it cannot be mandatory. When the field is missing cargo-contract will assert that the associated ink! constructor or message in fact does NOT return a Result. This means that whenver a user returns a Result type having this new ink! attribute is mandatory to set. If the user wants to revert the execution upon Result::Err the message or constructors should be flagged with either revert_on_err or revert_on_err: true which is both equivalent. If the execution should not be reverted upon Result::Err the user is mandated to flag the ink! constructor or message with revert_on_err: false.
• Besides this the currently generated signatures of call and deploy no longer return a u32 to make the ink! codegen compatible with the most recent SEAL versions again.

This design allows for an efficient design that ink! and cargo-contact can statically verify and which pleasantly fits into the current ink! design from a syntactical point of view.

An example follows:

/// It is still possible to create normal non-Result messages or constructors.
#[ink(message)]
fn foo(&self) -> i32 { ... }

/// Long-form notation, mandates revert upon returning `Result::Err`.
#[ink(message, revert_on_err: true)]
fn try_foo_1(&self) -> Result<i32, MyError> { ... }

/// Short-form notation, mandates revert upon returning `Result::Err`.
///
/// Absolutely the same as `try_foo_1`.
#[ink(message, revert_on_err)]
fn try_foo_2(&self) -> Result<i32, MyError> { ... }

/// Long-form notation, does NOT revert upon returning `Result::Err`.
#[ink(message, revert_on_err: false)]
fn foo_2(&self) -> Result<i32, MyError> { ... }

/// This is invalid ink! code and will yield a compile error via Rust compiler
/// since the message is not returning a Result type.
#[ink(message, revert_on_err: true)]
fn foo_3(&self) -> i32 { ... }

/// This is invalid ink! code and will yield an error via cargo-contract when being compiled
/// since the mandatory revert_on_err for Result type returns is missing.
#[ink(message)]
fn foo_4(&self) -> Result<i32, MyError> { ... }