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.

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

Return error when call was reverted #121

Merged
merged 4 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ homepage = "https://github.com/Cardinal-Cryptography/drink"
license = "Apache-2.0"
readme = "README.md"
repository = "https://github.com/Cardinal-Cryptography/drink"
version = "0.16.0"
version = "0.17.0"

[workspace.dependencies]
anyhow = { version = "1.0.71" }
Expand Down Expand Up @@ -51,5 +51,5 @@ sp-runtime-interface = { version = "26.0.0" }

# Local dependencies

drink = { version = "=0.16.0", path = "drink" }
drink-test-macro = { version = "=0.16.0", path = "drink/test-macro" }
drink = { version = "=0.17.0", path = "drink" }
drink-test-macro = { version = "=0.17.0", path = "drink/test-macro" }
20 changes: 19 additions & 1 deletion drink/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,22 @@ where
self.call_internal::<_, V>(None, message, args, endowment)
}

/// Calls the last deployed contract. Expect it to be reverted and the message result to be of
/// type `Result<_, E>`.
pub fn call_and_expect_error<S: AsRef<str> + Debug, E: Debug + Decode>(
&mut self,
message: &str,
args: &[S],
endowment: Option<BalanceOf<T::Runtime>>,
) -> E {
self.call_internal::<_, Result<(), E>>(None, message, args, endowment)
.expect_err("Call should fail")
.decode_revert::<Result<(), E>>()
.expect("Call should be reverted")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here we're loosing information - decode_revert returns None if the underlying error is different than CallReverted. So, if it's anything else - we will fail on this expect and not provide any context for the developer to fix it.

Maybe, instead of returning Option from decode_revert, we could return a Result and on the Err side include the original error?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

.expect("Call should return an error")
.expect_err("Call should return an error")
}

/// Calls a contract with a given address. In case of a successful call, returns the encoded
/// result.
pub fn call_with_address<S: AsRef<str> + Debug, V: Decode>(
Expand Down Expand Up @@ -542,7 +558,9 @@ where
});

let ret = match &result.result {
Ok(exec_result) if exec_result.did_revert() => Err(SessionError::CallReverted),
Ok(exec_result) if exec_result.did_revert() => {
Err(SessionError::CallReverted(exec_result.data.clone()))
}
Ok(exec_result) => {
self.record.push_call_return(exec_result.data.clone());
self.record.last_call_return_decoded::<V>()
Expand Down
19 changes: 17 additions & 2 deletions drink/src/session/error.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
//! Module exposing errors and result types for the session API.

use frame_support::sp_runtime::DispatchError;
use parity_scale_codec::Decode;
use thiserror::Error;

use crate::errors::MessageResult;

/// Session specific errors.
#[derive(Error, Debug)]
pub enum SessionError {
Expand All @@ -25,8 +28,8 @@ pub enum SessionError {
#[error("Code upload failed: {0:?}")]
UploadFailed(DispatchError),
/// Call has been reverted by the contract.
#[error("Contract call has been reverted")]
CallReverted,
#[error("Contract call has been reverted. Encoded error: {0:?}")]
CallReverted(Vec<u8>),
/// Contract call failed (aborted by the pallet).
#[error("Contract call failed before execution: {0:?}")]
CallFailed(DispatchError),
Expand All @@ -37,3 +40,15 @@ pub enum SessionError {
#[error("Missing transcoder")]
NoTranscoder,
}

impl SessionError {
/// Check if the error is a revert error and if so, decode the error message.
pub fn decode_revert<T: Decode>(&self) -> Option<MessageResult<T>> {
match self {
SessionError::CallReverted(error) => {
Some(MessageResult::decode(&mut &error[..]).expect("Failed to decode error"))
}
_ => None,
}
}
}
Loading
Loading