Skip to content

Commit

Permalink
Merge pull request from GHSA-853p-5678-hv8f
Browse files Browse the repository at this point in the history
* Handle `LangError` from `DelegateCall`

* Add basic delegate E2E test

* Get test working with integration flipper

* Use `incrementer` for test instead

* Add tests to make sure that `LangError`s can be handled

* Get rid of references to the interation Flipper
  • Loading branch information
HCastano committed Jun 14, 2023
1 parent 2eba99e commit f1407ee
Show file tree
Hide file tree
Showing 8 changed files with 269 additions and 20 deletions.
2 changes: 1 addition & 1 deletion crates/env/src/api.rs
Expand Up @@ -300,7 +300,7 @@ where
/// - If the called code execution has trapped.
pub fn invoke_contract_delegate<E, Args, R>(
params: &CallParams<E, DelegateCall<E>, Args, R>,
) -> Result<R>
) -> Result<ink_primitives::MessageResult<R>>
where
E: Environment,
Args: scale::Encode,
Expand Down
2 changes: 1 addition & 1 deletion crates/env/src/backend.rs
Expand Up @@ -433,7 +433,7 @@ pub trait TypedEnvBackend: EnvBackend {
fn invoke_contract_delegate<E, Args, R>(
&mut self,
call_data: &CallParams<E, DelegateCall<E>, Args, R>,
) -> Result<R>
) -> Result<ink_primitives::MessageResult<R>>
where
E: Environment,
Args: scale::Encode,
Expand Down
36 changes: 21 additions & 15 deletions crates/env/src/call/call_builder.rs
Expand Up @@ -151,13 +151,17 @@ where
///
/// # 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.
/// 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_delegate(self).unwrap_or_else(|env_error| {
panic!("Cross-contract call failed with {env_error:?}")
})
crate::invoke_contract_delegate(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:?}")
})
}

/// Invoke the contract using Delegate Call semantics with the given built-up call
Expand All @@ -167,9 +171,10 @@ where
///
/// # Note
///
/// On failure this returns an [`ink::env::Error`][`crate::Error`] which can be
/// On failure this returns an outer [`ink::env::Error`][`crate::Error`] or inner
/// [`ink::primitives::LangError`][`ink_primitives::LangError`], both of which can be
/// handled by the caller.
pub fn try_invoke(&self) -> Result<R, crate::Error> {
pub fn try_invoke(&self) -> Result<ink_primitives::MessageResult<R>, crate::Error> {
crate::invoke_contract_delegate(self)
}
}
Expand Down Expand Up @@ -714,7 +719,7 @@ where
///
/// On failure this an [`ink::env::Error`][`crate::Error`] which can be handled by the
/// caller.
pub fn try_invoke(self) -> Result<(), Error> {
pub fn try_invoke(self) -> Result<ink_primitives::MessageResult<()>, Error> {
self.params().try_invoke()
}
}
Expand Down Expand Up @@ -761,9 +766,9 @@ where
///
/// # Panics
///
/// This method panics if it encounters an [`ink::env::Error`][`crate::Error`]
/// If you want to handle those use the [`try_invoke`][`CallBuilder::try_invoke`]
/// method instead.
/// 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`][`CallBuilder::try_invoke`] method instead.
pub fn invoke(self) -> R {
self.params().invoke()
}
Expand All @@ -773,9 +778,10 @@ where
///
/// # Note
///
/// On failure this an [`ink::env::Error`][`crate::Error`] which can be handled by the
/// caller.
pub fn try_invoke(self) -> Result<R, Error> {
/// On failure this returns an outer [`ink::env::Error`][`crate::Error`] or inner
/// [`ink::primitives::LangError`][`ink_primitives::LangError`], both of which can be
/// handled by the caller.
pub fn try_invoke(self) -> Result<ink_primitives::MessageResult<R>, Error> {
self.params().try_invoke()
}
}
2 changes: 1 addition & 1 deletion crates/env/src/engine/off_chain/impls.rs
Expand Up @@ -453,7 +453,7 @@ impl TypedEnvBackend for EnvInstance {
fn invoke_contract_delegate<E, Args, R>(
&mut self,
params: &CallParams<E, DelegateCall<E>, Args, R>,
) -> Result<R>
) -> Result<ink_primitives::MessageResult<R>>
where
E: Environment,
Args: scale::Encode,
Expand Down
2 changes: 1 addition & 1 deletion crates/env/src/engine/on_chain/impls.rs
Expand Up @@ -444,7 +444,7 @@ impl TypedEnvBackend for EnvInstance {
fn invoke_contract_delegate<E, Args, R>(
&mut self,
params: &CallParams<E, DelegateCall<E>, Args, R>,
) -> Result<R>
) -> Result<ink_primitives::MessageResult<R>>
where
E: Environment,
Args: scale::Encode,
Expand Down
2 changes: 1 addition & 1 deletion crates/ink/src/env_access.rs
Expand Up @@ -631,7 +631,7 @@ where
pub fn invoke_contract_delegate<Args, R>(
self,
params: &CallParams<E, DelegateCall<E>, Args, R>,
) -> Result<R>
) -> Result<ink_primitives::MessageResult<R>>
where
Args: scale::Encode,
R: scale::Decode,
Expand Down
@@ -0,0 +1,32 @@
[package]
name = "call_builder_delegate"
version = "4.2.0"
authors = ["Parity Technologies <admin@parity.io>"]
edition = "2021"
publish = false

[dependencies]
ink = { path = "../../../crates/ink", default-features = false }

scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] }
scale-info = { version = "2.6", default-features = false, features = ["derive"], optional = true }

incrementer = { path = "../../incrementer", default-features = false, features = ["ink-as-dependency"] }

[dev-dependencies]
ink_e2e = { path = "../../../crates/e2e" }

[lib]
path = "lib.rs"

[features]
default = ["std"]
std = [
"ink/std",
"scale/std",
"scale-info/std",

"incrementer/std",
]
ink-as-dependency = []
e2e-tests = []
@@ -0,0 +1,211 @@
//! # Integration Tests for `LangError`
//!
//! This contract is used to ensure that the behavior around `LangError`s works as
//! expected.
//!
//! In particular, it exercises the codepaths that stem from the usage of the
//! [`CallBuilder`](`ink::env::call::CallBuilder`) and
//! [`CreateBuilder`](`ink::env::call::CreateBuilder`) structs.
//!
//! This differs from the codepath used by external tooling, such as `cargo-contract` or
//! the `Contracts-UI` which instead depend on methods from the Contracts pallet which are
//! exposed via RPC.
//!
//! Note that during testing we make use of ink!'s end-to-end testing features, so ensure
//! that you have a node which includes the Contracts pallet running alongside your tests.

#![cfg_attr(not(feature = "std"), no_std, no_main)]

#[ink::contract]
mod call_builder {
use ink::env::{
call::{
build_call,
ExecutionInput,
Selector,
},
DefaultEnvironment,
};

#[ink(storage)]
#[derive(Default)]
pub struct CallBuilderDelegateTest {
/// Since we're going to `DelegateCall` into the `incrementer` contract, we need
/// to make sure our storage layout matches.
value: i32,
}

impl CallBuilderDelegateTest {
#[ink(constructor)]
pub fn new(value: i32) -> Self {
Self { value }
}

/// Call a contract using the `CallBuilder`.
///
/// Since we can't use the `CallBuilder` in a test environment directly we need
/// this wrapper to test things like crafting calls with invalid
/// selectors.
///
/// We also wrap the output in an `Option` since we can't return a `Result`
/// directly from a contract message without erroring out ourselves.
#[ink(message)]
pub fn delegate(
&mut self,
code_hash: Hash,
selector: [u8; 4],
) -> Option<ink::LangError> {
let result = build_call::<DefaultEnvironment>()
.delegate(code_hash)
.exec_input(ExecutionInput::new(Selector::new(selector)))
.returns::<bool>()
.try_invoke()
.expect("Error from the Contracts pallet.");

match result {
Ok(_) => None,
Err(e @ ink::LangError::CouldNotReadInput) => Some(e),
Err(_) => {
unimplemented!("No other `LangError` variants exist at the moment.")
}
}
}

/// Call a contract using the `CallBuilder`.
///
/// Since we can't use the `CallBuilder` in a test environment directly we need
/// this wrapper to test things like crafting calls with invalid
/// selectors.
///
/// This message does not allow the caller to handle any `LangErrors`, for that
/// use the `call` message instead.
#[ink(message)]
pub fn invoke(&mut self, code_hash: Hash, selector: [u8; 4]) -> i32 {
use ink::env::call::build_call;

build_call::<DefaultEnvironment>()
.delegate(code_hash)
.exec_input(ExecutionInput::new(Selector::new(selector)))
.returns::<i32>()
.invoke()
}
}

#[cfg(all(test, feature = "e2e-tests"))]
mod e2e_tests {
use super::*;

type E2EResult<T> = std::result::Result<T, Box<dyn std::error::Error>>;

#[ink_e2e::test]
async fn e2e_call_builder_delegate_returns_correct_value(
mut client: ink_e2e::Client<C, E>,
) -> E2EResult<()> {
let origin = client
.create_and_fund_account(&ink_e2e::alice(), 10_000_000_000_000)
.await;

let expected_value = 42;
let constructor = CallBuilderDelegateTestRef::new(expected_value);
let call_builder = client
.instantiate("call_builder_delegate", &origin, constructor, 0, None)
.await
.expect("instantiate failed");
let mut call_builder_call = call_builder.call::<CallBuilderDelegateTest>();

let code_hash = client
.upload("incrementer", &origin, None)
.await
.expect("upload `incrementer` failed")
.code_hash;

let selector = ink::selector_bytes!("get");
let call = call_builder_call.invoke(code_hash, selector);
let call_result = client
.call(&origin, &call, 0, None)
.await
.expect("Client failed to call `call_builder::invoke`.")
.return_value();

assert_eq!(
call_result, expected_value,
"Decoded an unexpected value from the call."
);

Ok(())
}

#[ink_e2e::test]
async fn e2e_invalid_message_selector_can_be_handled(
mut client: ink_e2e::Client<C, E>,
) -> E2EResult<()> {
let origin = client
.create_and_fund_account(&ink_e2e::bob(), 10_000_000_000_000)
.await;

let constructor = CallBuilderDelegateTestRef::new(Default::default());
let call_builder_contract = client
.instantiate("call_builder_delegate", &origin, constructor, 0, None)
.await
.expect("instantiate failed");
let mut call_builder_call =
call_builder_contract.call::<CallBuilderDelegateTest>();

let code_hash = client
.upload("incrementer", &origin, None)
.await
.expect("upload `incrementer` failed")
.code_hash;

let selector = ink::selector_bytes!("invalid_selector");
let call = call_builder_call.delegate(code_hash, selector);
let call_result = client
.call(&origin, &call, 0, None)
.await
.expect("Calling `call_builder::delegate` failed");

assert!(matches!(
call_result.return_value(),
Some(ink::LangError::CouldNotReadInput)
));

Ok(())
}

#[ink_e2e::test]
async fn e2e_invalid_message_selector_panics_on_invoke(
mut client: ink_e2e::Client<C, E>,
) -> E2EResult<()> {
let origin = client
.create_and_fund_account(&ink_e2e::charlie(), 10_000_000_000_000)
.await;

let constructor = CallBuilderDelegateTestRef::new(Default::default());
let call_builder_contract = client
.instantiate("call_builder_delegate", &origin, constructor, 0, None)
.await
.expect("instantiate failed");
let mut call_builder_call =
call_builder_contract.call::<CallBuilderDelegateTest>();

let code_hash = client
.upload("incrementer", &origin, None)
.await
.expect("upload `incrementer` failed")
.code_hash;

// Since `LangError`s can't be handled by the `CallBuilder::invoke()` method
// we expect this to panic.
let selector = ink::selector_bytes!("invalid_selector");
let call = call_builder_call.invoke(code_hash, selector);
let call_result = client.call_dry_run(&origin, &call, 0, None).await;

assert!(call_result.is_err());
assert!(call_result
.debug_message()
.contains("Cross-contract call failed with CouldNotReadInput"));

Ok(())
}
}
}

0 comments on commit f1407ee

Please sign in to comment.