Concordium / concordium-rust-smart-contracts

Libraries and tools for writing and testing smart contracts on Concordium
https://docs.rs/concordium-std/latest/concordium_std/
Mozilla Public License 2.0
57 stars 35 forks source link

Implement `Reject` trait for `CallContractError` so that they can be returned in entrypoint methods. #408

Open dhruvja opened 7 months ago

dhruvja commented 7 months ago

Description When a cross contract call is made, it returns CallContractError which doesnt seem to implement Reject trait which the entrypoint method expect to return. So a From trait has to be implemented.

Solution Implementing Reject trait on CallContractError itself will help users just return those errors directly from entrypoint without needing to write wrappers.

A From trait for Cis2ClientError which itself is a wrapper for CallContractError looks like below.

impl From<Cis2ClientError<()>> for Error {
    fn from(value: Cis2ClientError<()>) -> Self {
        match value {
            Cis2ClientError::InvokeContractError(e) => match e {
                CallContractError::AmountTooLarge => Self::AmountTooLarge,
                CallContractError::MissingAccount => Self::MissingAccount,
                CallContractError::MissingContract => Self::MissingContract,
                CallContractError::MissingEntrypoint => Self::MissingEntrypoint,
                CallContractError::MessageFailed => Self::MessageFailed,
                CallContractError::LogicReject {
                    reason,
                    return_value,
                } => Self::LogicReject {
                    reason,
                    return_value,
                },
                CallContractError::Trap => Self::Trap,
            },
            Cis2ClientError::ParseResult => Self::ParseResult,
            Cis2ClientError::InvalidResponse => Self::InvalidResponse,
        }
    }
}

Would love to know what you guys think regarding having this trait implemented?

abizjak commented 7 months ago

HI @dhruvja, thanks for the input.

I'm not opposed to implementing this in principle, but there is a general challenge that it's not really possible to have an injective implementation of this trait for InvokeContractError so there would be some loss of information.

This is usually a lot simpler to accept on a case by case basis rather than in a generic implementation.

dhruvja commented 7 months ago

HI @dhruvja, thanks for the input.

I'm not opposed to implementing this in principle, but there is a general challenge that it's not really possible to have an injective implementation of this trait for InvokeContractError so there would be some loss of information.

This is usually a lot simpler to accept on a case by case basis rather than in a generic implementation.

Do you mean its difficult to include the generics when we implement the trait?

abizjak commented 7 months ago

No, I meant that to implement Reject we need to assign codes in the set i32::MIN..-1 to values of the CallContractError and in general it is not possible to do so without losing information, meaning that there would be two values that would map to the same status code. This is unsatisfactory which is why the trait is not implemented at the moment.

abizjak commented 7 months ago

Moving this over to the correct repository.