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.

Sign up for GitHub

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

Jump to bottom

Make CallBuilder and CreateBuilder error handling optional #1602

Merged
merged 11 commits into from
Jan 20, 2023
Merged

Make CallBuilder and CreateBuilder error handling optional #1602

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
947cf6c
Make default behaviour of `fire()` to return "raw" return value
HCastano Jan 19, 2023
5ff7fee
Update delegate call codepath to give option to handle `EnvError`
HCastano Jan 19, 2023
503fddb
Update `CreateBuilder::instantiate()`'s default cleaner as well
HCastano Jan 19, 2023
48023ca
Add a breaking change note in the changelog
HCastano Jan 20, 2023
dbf7ac4
Fix delegator example
HCastano Jan 20, 2023
72add05
Drive-by: rename `fire()` method to `invoke()`
HCastano Jan 20, 2023
9819e76
Fix ERC-1155 example
HCastano Jan 20, 2023
ff759b4
Update `Multisig` example
HCastano Jan 20, 2023
0d37811
Fix ContractRef tests
HCastano Jan 20, 2023
155d416
Revert "Drive-by: rename `fire()` method to `invoke()`"
HCastano Jan 20, 2023
22a924d
Use instantiate instead of `try_instantiate` in `delegator`
ascjones Jan 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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))
Comment on lines -358 to +360
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just call fire?

Copy link
Collaborator

@ascjones ascjones Jan 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the only difference would be that the panic string includes extra info:

let panic_str = format!(
            "encountered error while calling <{} as {}>::{}",
            forwarder_ident, trait_ident, message_ident,
        );

Not sure how useful that is though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think it is useful. If we just fire the panic message will just point to the line within the call builder file. Without backtraces (which we don't have) it will be quite hard to find the line which paniced.

}
)
}
Expand Down
100 changes: 50 additions & 50 deletions crates/ink/tests/ui/trait_def/fail/message_input_non_codec.stderr
View file
Open in desktop
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