Skip to content

Commit

Permalink
Make CallBuilder and CreateBuilder error handling optional (#1602)
Browse files Browse the repository at this point in the history
* Make default behaviour of `fire()` to return "raw" return value

* Update delegate call codepath to give option to handle `EnvError`

* Update `CreateBuilder::instantiate()`'s default cleaner as well

* Add a breaking change note in the changelog

* Fix delegator example

* Drive-by: rename `fire()` method to `invoke()`

This keeps this more consistent since we're wrapping a few methods with
`invoke` in the name anyways.

* Fix ERC-1155 example

* Update `Multisig` example

* Fix ContractRef tests

* Revert "Drive-by: rename `fire()` method to `invoke()`"

This reverts commit 72add05.

Decided it's best to do this in a standalone PR.

* Use instantiate instead of `try_instantiate` in `delegator`

Co-authored-by: Andrew Jones <[email protected]>
  • Loading branch information
HCastano and ascjones committed Jan 20, 2023
1 parent de17d10 commit a83c937
Show file tree
Hide file tree
Showing 12 changed files with 216 additions and 121 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## Unreleased
- Add E2E testing framework MVP ‒ [#1395](https://github.com/paritytech/ink/pull/1395)
- Add E2E tests for `Mapping` functions - [#1492](https://github.com/paritytech/ink/pull/1492)
- Make CallBuilder and CreateBuilder error handling optional - [#1602](https://github.com/paritytech/ink/pull/1602)

### Breaking Changes
With the addition of [#1602](https://github.com/paritytech/ink/pull/1602),
the `CallBuilder::fire()`, `CallParams::invoke()`, and `CreateBuilder::instantiate()`
methods now unwrap the `Result` from `pallet-contracts` under the hood.

If you wish to handle the error use the new `try_` variants of those methods instead.

## Version 4.0.0-beta

Expand Down
107 changes: 79 additions & 28 deletions crates/env/src/call/call_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,17 @@ where
///
/// # Panics
///
/// This method panics if it encounters an [`ink_primitives::LangError`]. If you want to handle
/// those use the [`try_invoke`][`CallParams::try_invoke`] method instead.
pub fn invoke(&self) -> Result<R, crate::Error> {
crate::invoke_contract(self).map(|inner| {
inner.unwrap_or_else(|lang_error| {
/// This method panics if it encounters an [`ink::env::Error`][`crate::Error`] or an
/// [`ink::primitives::LangError`][`ink_primitives::LangError`]. If you want to handle those
/// use the [`try_invoke`][`CallParams::try_invoke`] method instead.
pub fn invoke(&self) -> R {
crate::invoke_contract(self)
.unwrap_or_else(|env_error| {
panic!("Cross-contract call failed with {:?}", env_error)
})
.unwrap_or_else(|lang_error| {
panic!("Cross-contract call failed with {:?}", lang_error)
})
})
}

/// Invokes the contract with the given built-up call parameters.
Expand All @@ -128,7 +131,8 @@ where
///
/// # Note
///
/// On failure this returns an [`ink_primitives::LangError`] which can be handled by the caller.
/// On failure this returns an outer [`ink::env::Error`][`crate::Error`] or inner
/// [`ink_primitives::LangError`], both of which can be handled by the caller.
pub fn try_invoke(&self) -> Result<ink_primitives::MessageResult<R>, crate::Error> {
crate::invoke_contract(self)
}
Expand All @@ -140,11 +144,29 @@ where
Args: scale::Encode,
R: scale::Decode,
{
/// Invokes the contract via delegated call with the given
/// built-up call parameters.
/// Invoke the contract using Delegate Call semantics with the given built-up call parameters.
///
/// Returns the result of the contract execution.
pub fn invoke(&self) -> Result<R, crate::Error> {
///
/// # Panics
///
/// This method panics if it encounters an [`ink::env::Error`][`crate::Error`] If you want to
/// handle those use the [`try_invoke`][`CallParams::try_invoke`] method instead.
pub fn invoke(&self) -> R {
crate::invoke_contract_delegate(self).unwrap_or_else(|env_error| {
panic!("Cross-contract call failed with {:?}", env_error)
})
}

/// Invoke the contract using Delegate Call semantics with the given built-up call parameters.
///
/// Returns the result of the contract execution.
///
/// # Note
///
/// On failure this returns an [`ink::env::Error`][`crate::Error`] which can be handled by the
/// caller.
pub fn try_invoke(&self) -> Result<R, crate::Error> {
crate::invoke_contract_delegate(self)
}
}
Expand Down Expand Up @@ -192,8 +214,7 @@ where
/// .push_arg(&[0x10u8; 32])
/// )
/// .returns::<()>()
/// .fire()
/// .expect("Got an error from the Contract's pallet.");
/// .fire();
/// ```
///
/// ## Example 2: With Return Value
Expand Down Expand Up @@ -228,8 +249,7 @@ where
/// .push_arg(&[0x10u8; 32])
/// )
/// .returns::<i32>()
/// .fire()
/// .expect("Got an error from the Contract's pallet.");
/// .fire();
/// ```
///
/// ## Example 3: Delegate call
Expand All @@ -256,8 +276,7 @@ where
/// .push_arg(&[0x10u8; 32])
/// )
/// .returns::<i32>()
/// .fire()
/// .expect("Got an error from the Contract's pallet.");
/// .fire();
/// ```
///
/// # Handling `LangError`s
Expand Down Expand Up @@ -660,17 +679,19 @@ where
///
/// # Panics
///
/// This method panics if it encounters an [`ink_primitives::LangError`]. If you want to handle
/// those use the [`try_fire`][`CallBuilder::try_fire`] method instead.
pub fn fire(self) -> Result<(), Error> {
/// This method panics if it encounters an [`ink::env::Error`][`crate::Error`] or an
/// [`ink::primitives::LangError`][`ink_primitives::LangError`]. If you want to handle those
/// use the [`try_fire`][`CallBuilder::try_fire`] method instead.
pub fn fire(self) {
self.params().invoke()
}

/// Invokes the cross-chain function call.
///
/// # Note
///
/// On failure this returns an [`ink_primitives::LangError`] which can be handled by the caller.
/// On failure this returns an outer [`ink::env::Error`][`crate::Error`] or inner
/// [`ink_primitives::LangError`], both of which can be handled by the caller.
pub fn try_fire(self) -> Result<ink_primitives::MessageResult<()>, Error> {
self.params().try_invoke()
}
Expand All @@ -686,10 +707,24 @@ impl<E>
where
E: Environment,
{
/// Invokes the cross-chain function call.
pub fn fire(self) -> Result<(), Error> {
/// Invokes the cross-chain function call using Delegate Call semantics.
///
/// # Panics
///
/// This method panics if it encounters an [`ink::env::Error`][`crate::Error`]
/// If you want to handle those use the [`try_fire`][`CallBuilder::try_fire`] method instead.
pub fn fire(self) {
self.params().invoke()
}

/// Invokes the cross-chain function call using Delegate Call semantics.
///
/// # Note
///
/// On failure this an [`ink::env::Error`][`crate::Error`] which can be handled by the caller.
pub fn try_fire(self) -> Result<(), Error> {
self.params().try_invoke()
}
}

impl<E, Args, R>
Expand All @@ -703,17 +738,19 @@ where
///
/// # Panics
///
/// This method panics if it encounters an [`ink_primitives::LangError`]. If you want to handle
/// those use the [`try_fire`][`CallBuilder::try_fire`] method instead.
pub fn fire(self) -> Result<R, Error> {
/// This method panics if it encounters an [`ink::env::Error`][`crate::Error`] or an
/// [`ink::primitives::LangError`][`ink_primitives::LangError`]. If you want to handle those
/// use the [`try_fire`][`CallBuilder::try_fire`] method instead.
pub fn fire(self) -> R {
self.params().invoke()
}

/// Invokes the cross-chain function call and returns the result.
///
/// # Note
///
/// On failure this returns an [`ink_primitives::LangError`] which can be handled by the caller.
/// On failure this returns an outer [`ink::env::Error`][`crate::Error`] or inner
/// [`ink_primitives::LangError`], both of which can be handled by the caller.
pub fn try_fire(self) -> Result<ink_primitives::MessageResult<R>, Error> {
self.params().try_invoke()
}
Expand All @@ -726,8 +763,22 @@ where
Args: scale::Encode,
R: scale::Decode,
{
/// Invokes the cross-chain function call and returns the result.
pub fn fire(self) -> Result<R, Error> {
/// Invokes the cross-chain function call using Delegate Call semantics and returns the result.
///
/// # Panics
///
/// This method panics if it encounters an [`ink::env::Error`][`crate::Error`]
/// If you want to handle those use the [`try_fire`][`CallBuilder::try_fire`] method instead.
pub fn fire(self) -> R {
self.params().invoke()
}

/// Invokes the cross-chain function call using Delegate Call semantics and returns the result.
///
/// # Note
///
/// On failure this an [`ink::env::Error`][`crate::Error`] which can be handled by the caller.
pub fn try_fire(self) -> Result<R, Error> {
self.params().try_invoke()
}
}
45 changes: 40 additions & 5 deletions crates/env/src/call/create_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,28 @@ where
R: FromAccountId<E>,
{
/// Instantiates the contract and returns its account ID back to the caller.
///
/// # Panics
///
/// This method panics if it encounters an [`ink::env::Error`][`crate::Error`]. If you want to
/// handle those use the [`try_instantiate`][`CreateParams::try_instantiate`] method instead.
#[inline]
pub fn instantiate(&self) -> Result<R, crate::Error> {
pub fn instantiate(&self) -> R {
crate::instantiate_contract(self)
.map(FromAccountId::from_account_id)
.unwrap_or_else(|env_error| {
panic!("Cross-contract instantiation failed with {:?}", env_error)
})
}

/// Instantiates the contract and returns its account ID back to the caller.
///
/// # Note
///
/// On failure this returns an [`ink::env::Error`][`crate::Error`] which can be handled by the
/// caller.
#[inline]
pub fn try_instantiate(&self) -> Result<R, crate::Error> {
crate::instantiate_contract(self).map(FromAccountId::from_account_id)
}
}
Expand Down Expand Up @@ -180,8 +200,7 @@ where
/// )
/// .salt_bytes(&[0xDE, 0xAD, 0xBE, 0xEF])
/// .params()
/// .instantiate()
/// .unwrap();
/// .instantiate();
/// ```
///
/// **Note:** The shown example panics because there is currently no cross-calling
Expand Down Expand Up @@ -384,9 +403,25 @@ where
Salt: AsRef<[u8]>,
R: FromAccountId<E>,
{
/// Instantiates the contract using the given instantiation parameters.
/// Instantiates the contract and returns its account ID back to the caller.
///
/// # Panics
///
/// This method panics if it encounters an [`ink::env::Error`][`crate::Error`]. If you want to
/// handle those use the [`try_instantiate`][`CreateBuilder::try_instantiate`] method instead.
#[inline]
pub fn instantiate(self) -> Result<R, Error> {
pub fn instantiate(self) -> R {
self.params().instantiate()
}

/// Instantiates the contract and returns its account ID back to the caller.
///
/// # Note
///
/// On failure this returns an [`ink::env::Error`][`crate::Error`] which can be handled by the
/// caller.
#[inline]
pub fn try_instantiate(self) -> Result<R, Error> {
self.params().try_instantiate()
}
}
5 changes: 3 additions & 2 deletions crates/ink/codegen/src/generator/trait_def/call_forwarder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,9 @@ impl CallForwarder<'_> {
, #input_bindings
)*
)
.fire()
.unwrap_or_else(|err| ::core::panic!("{}: {:?}", #panic_str, err))
.try_fire()
.unwrap_or_else(|env_err| ::core::panic!("{}: {:?}", #panic_str, env_err))
.unwrap_or_else(|lang_err| ::core::panic!("{}: {:?}", #panic_str, lang_err))
}
)
}
Expand Down
100 changes: 50 additions & 50 deletions crates/ink/tests/ui/trait_def/fail/message_input_non_codec.stderr
Original file line number Diff line number Diff line change
@@ -1,56 +1,56 @@
error[E0277]: the trait bound `NonCodec: WrapperTypeDecode` is not satisfied
--> tests/ui/trait_def/fail/message_input_non_codec.rs:6:23
|
6 | fn message(&self, input: NonCodec);
| ^^^^^ the trait `WrapperTypeDecode` is not implemented for `NonCodec`
|
= help: the following other types implement trait `WrapperTypeDecode`:
Arc<T>
Box<T>
Rc<T>
= note: required for `NonCodec` to implement `parity_scale_codec::Decode`
--> tests/ui/trait_def/fail/message_input_non_codec.rs:6:23
|
6 | fn message(&self, input: NonCodec);
| ^^^^^ the trait `WrapperTypeDecode` is not implemented for `NonCodec`
|
= help: the following other types implement trait `WrapperTypeDecode`:
Arc<T>
Box<T>
Rc<T>
= note: required for `NonCodec` to implement `parity_scale_codec::Decode`
note: required by a bound in `DispatchInput`
--> src/codegen/dispatch/type_check.rs
|
| T: scale::Decode + 'static;
| ^^^^^^^^^^^^^ required by this bound in `DispatchInput`
--> src/codegen/dispatch/type_check.rs
|
| T: scale::Decode + 'static;
| ^^^^^^^^^^^^^ required by this bound in `DispatchInput`

error[E0277]: the trait bound `NonCodec: WrapperTypeEncode` is not satisfied
--> tests/ui/trait_def/fail/message_input_non_codec.rs:3:1
|
3 | #[ink::trait_definition]
| ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `WrapperTypeEncode` is not implemented for `NonCodec`
4 | pub trait TraitDefinition {
5 | #[ink(message)]
| - required by a bound introduced by this call
|
= help: the following other types implement trait `WrapperTypeEncode`:
&T
&mut T
Arc<T>
Box<T>
Cow<'a, T>
Rc<T>
String
Vec<T>
parity_scale_codec::Ref<'a, T, U>
= note: required for `NonCodec` to implement `Encode`
--> tests/ui/trait_def/fail/message_input_non_codec.rs:3:1
|
3 | #[ink::trait_definition]
| ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `WrapperTypeEncode` is not implemented for `NonCodec`
4 | pub trait TraitDefinition {
5 | #[ink(message)]
| - required by a bound introduced by this call
|
= help: the following other types implement trait `WrapperTypeEncode`:
&T
&mut T
Arc<T>
Box<T>
Cow<'a, T>
Rc<T>
String
Vec<T>
parity_scale_codec::Ref<'a, T, U>
= note: required for `NonCodec` to implement `Encode`
note: required by a bound in `ExecutionInput::<ArgumentList<ArgumentListEnd, ArgumentListEnd>>::push_arg`
--> $WORKSPACE/crates/env/src/call/execution_input.rs
|
| T: scale::Encode,
| ^^^^^^^^^^^^^ required by this bound in `ExecutionInput::<ArgumentList<ArgumentListEnd, ArgumentListEnd>>::push_arg`
--> $WORKSPACE/crates/env/src/call/execution_input.rs
|
| T: scale::Encode,
| ^^^^^^^^^^^^^ required by this bound in `ExecutionInput::<ArgumentList<ArgumentListEnd, ArgumentListEnd>>::push_arg`

error[E0599]: the method `fire` exists for struct `CallBuilder<E, Set<Call<E>>, Set<ExecutionInput<ArgumentList<ink::ink_env::call::utils::Argument<NonCodec>, ArgumentList<ArgumentListEnd, ArgumentListEnd>>>>, Set<ReturnType<()>>>`, but its trait bounds were not satisfied
--> tests/ui/trait_def/fail/message_input_non_codec.rs:5:5
|
5 | #[ink(message)]
| ^ method cannot be called on `CallBuilder<E, Set<Call<E>>, Set<ExecutionInput<ArgumentList<ink::ink_env::call::utils::Argument<NonCodec>, ArgumentList<ArgumentListEnd, ArgumentListEnd>>>>, Set<ReturnType<()>>>` due to unsatisfied trait bounds
|
::: $WORKSPACE/crates/env/src/call/execution_input.rs
|
| pub struct ArgumentList<Head, Rest> {
| ----------------------------------- doesn't satisfy `_: Encode`
|
= note: the following trait bounds were not satisfied:
`ArgumentList<ink::ink_env::call::utils::Argument<NonCodec>, ArgumentList<ArgumentListEnd, ArgumentListEnd>>: Encode`
error[E0599]: the method `try_fire` exists for struct `CallBuilder<E, Set<Call<E>>, Set<ExecutionInput<ArgumentList<ink::ink_env::call::utils::Argument<NonCodec>, ArgumentList<ArgumentListEnd, ArgumentListEnd>>>>, Set<ReturnType<()>>>`, but its trait bounds were not satisfied
--> tests/ui/trait_def/fail/message_input_non_codec.rs:5:5
|
5 | #[ink(message)]
| ^ method cannot be called on `CallBuilder<E, Set<Call<E>>, Set<ExecutionInput<ArgumentList<ink::ink_env::call::utils::Argument<NonCodec>, ArgumentList<ArgumentListEnd, ArgumentListEnd>>>>, Set<ReturnType<()>>>` due to unsatisfied trait bounds
|
::: $WORKSPACE/crates/env/src/call/execution_input.rs
|
| pub struct ArgumentList<Head, Rest> {
| ----------------------------------- doesn't satisfy `_: Encode`
|
= note: the following trait bounds were not satisfied:
`ArgumentList<ink::ink_env::call::utils::Argument<NonCodec>, ArgumentList<ArgumentListEnd, ArgumentListEnd>>: Encode`
Loading

0 comments on commit a83c937

Please sign in to comment.