Only return an error if the extrinsic failed.

paritytech · Aug 30, 2020 · 9b54c3a · 9b54c3a
1 parent 3ea9d3b
commit 9b54c3a
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 36 deletions.
diff --git a/src/error.rs b/src/error.rs
@@ -62,13 +62,7 @@ pub enum Error {
     TypeSizeUnavailable(String),
     /// Runtime error.
     #[error("Runtime error: {0}")]
-    Runtime(RuntimeError),
-    /// Bad origin.
-    #[error("Bad origin: throw by ensure_signed, ensure_root or ensure_none.")]
-    BadOrigin,
-    /// Cannot lookup.
-    #[error("Cannot lookup some information required to validate the transaction.")]
-    CannotLookup,
+    Runtime(#[from] RuntimeError),
     /// Other error.
     #[error("Other error: {0}")]
     Other(String),
@@ -98,9 +92,29 @@ impl From<String> for Error {
     }
 }
 
-impl Error {
+/// Runtime error.
+#[derive(Clone, Debug, Eq, Error, PartialEq)]
+pub enum RuntimeError {
+    /// Module error.
+    #[error("Runtime module error: {0}")]
+    Module(ModuleError),
+    /// Bad origin.
+    #[error("Bad origin: throw by ensure_signed, ensure_root or ensure_none.")]
+    BadOrigin,
+    /// Cannot lookup.
+    #[error("Cannot lookup some information required to validate the transaction.")]
+    CannotLookup,
+    /// Other error.
+    #[error("Other error: {0}")]
+    Other(String),
+}
+
+impl RuntimeError {
     /// Converts a `DispatchError` into a subxt error.
-    pub fn from_dispatch(metadata: &Metadata, error: DispatchError) -> Result<(), Self> {
+    pub fn from_dispatch(
+        metadata: &Metadata,
+        error: DispatchError,
+    ) -> Result<Self, Error> {
         match error {
             DispatchError::Module {
                 index,
@@ -109,22 +123,22 @@ impl Error {
             } => {
                 let module = metadata.module_with_errors(index)?;
                 let error = module.error(error)?;
-                Err(Error::Runtime(RuntimeError {
+                Ok(Self::Module(ModuleError {
                     module: module.name().to_string(),
                     error: error.to_string(),
                 }))
             }
-            DispatchError::BadOrigin => Err(Error::BadOrigin),
-            DispatchError::CannotLookup => Err(Error::CannotLookup),
-            DispatchError::Other(msg) => Err(Error::Other(msg.into())),
+            DispatchError::BadOrigin => Ok(Self::BadOrigin),
+            DispatchError::CannotLookup => Ok(Self::CannotLookup),
+            DispatchError::Other(msg) => Ok(Self::Other(msg.into())),
         }
     }
 }
 
-/// Runtime errors.
+/// Module error.
 #[derive(Clone, Debug, Eq, Error, PartialEq)]
 #[error("{error} from {module}")]
-pub struct RuntimeError {
+pub struct ModuleError {
     pub module: String,
     pub error: String,
 }
diff --git a/src/events.rs b/src/events.rs
@@ -39,7 +39,10 @@ use std::{
 };
 
 use crate::{
-    error::Error,
+    error::{
+        Error,
+        RuntimeError,
+    },
     metadata::{
         EventArg,
         Metadata,
@@ -168,7 +171,9 @@ impl<T: System> EventsDecoder<T> {
                         }
                     };
                     if let Err(error) = result {
-                        Error::from_dispatch(&self.metadata, error)?;
+                        return Err(
+                            RuntimeError::from_dispatch(&self.metadata, error)?.into()
+                        )
                     }
                 }
             }
@@ -177,10 +182,7 @@ impl<T: System> EventsDecoder<T> {
     }
 
     /// Decode events.
-    pub fn decode_events(
-        &self,
-        input: &mut &[u8],
-    ) -> Result<Vec<(Phase, RawEvent)>, Error> {
+    pub fn decode_events(&self, input: &mut &[u8]) -> Result<Vec<(Phase, Raw)>, Error> {
         let compact_len = <Compact<u32>>::decode(input)?;
         let len = compact_len.0 as usize;
 
@@ -201,20 +203,36 @@ impl<T: System> EventsDecoder<T> {
             );
 
             let mut event_data = Vec::<u8>::new();
-            self.decode_raw_bytes(&event_metadata.arguments(), input, &mut event_data)?;
-
-            log::debug!("raw bytes: {}", hex::encode(&event_data),);
+            let result = self.decode_raw_bytes(
+                &event_metadata.arguments(),
+                input,
+                &mut event_data,
+            );
+            let raw = match result {
+                Ok(()) => {
+                    log::debug!("raw bytes: {}", hex::encode(&event_data),);
+
+                    let event = RawEvent {
+                        module: module.name().to_string(),
+                        variant: event_metadata.name.clone(),
+                        data: event_data,
+                    };
 
-            let event = RawEvent {
-                module: module.name().to_string(),
-                variant: event_metadata.name.clone(),
-                data: event_data,
+                    // topics come after the event data in EventRecord
+                    let _topics = Vec::<T::Hash>::decode(input)?;
+                    Raw::Event(event)
+                }
+                Err(Error::Runtime(err)) => Raw::Error(err),
+                Err(err) => return Err(err),
             };
 
-            // topics come after the event data in EventRecord
-            let _topics = Vec::<T::Hash>::decode(input)?;
-            r.push((phase, event));
+            r.push((phase, raw));
         }
         Ok(r)
     }
 }
+
+pub enum Raw {
+    Event(RawEvent),
+    Error(RuntimeError),
+}
diff --git a/src/frame/balances.rs b/src/frame/balances.rs
@@ -112,6 +112,7 @@ mod tests {
     use crate::{
         error::{
             Error,
+            ModuleError,
             RuntimeError,
         },
         events::EventsDecoder,
@@ -193,8 +194,8 @@ mod tests {
         let res = client
             .transfer_and_watch(&hans, alice.account_id(), 100_000_000_000)
             .await;
-        if let Err(Error::Runtime(error)) = res {
-            let error2 = RuntimeError {
+        if let Err(Error::Runtime(RuntimeError::Module(error))) = res {
+            let error2 = ModuleError {
                 module: "Balances".into(),
                 error: "InsufficientBalance".into(),
             };

diff --git a/src/frame/sudo.rs b/src/frame/sudo.rs
@@ -43,7 +43,7 @@ pub struct SudoCall<'a, T: Sudo> {
 mod tests {
     use super::*;
     use crate::{
-        error::Error,
+        error::{Error, RuntimeError},
         extrinsic::PairSigner,
         frame::balances::TransferCall,
         tests::{
@@ -68,7 +68,7 @@ mod tests {
 
         let res = client.sudo_and_watch(&alice, &call).await;
         assert!(
-            if let Err(Error::BadOrigin) = res {
+            if let Err(Error::Runtime(RuntimeError::BadOrigin)) = res {
                 true
             } else {
                 false

diff --git a/src/subscription.rs b/src/subscription.rs
@@ -22,6 +22,7 @@ use crate::{
     error::Error,
     events::{
         EventsDecoder,
+        Raw,
         RawEvent,
     },
     frame::{
@@ -99,13 +100,17 @@ impl<T: Runtime> EventSubscription<T> {
                         Ok(events) => events,
                         Err(error) => return Some(Err(error)),
                     };
-                    for (phase, event) in raw_events {
+                    for (phase, raw) in raw_events {
                         if let Phase::ApplyExtrinsic(i) = phase {
                             if let Some(ext_index) = self.extrinsic {
                                 if i as usize != ext_index {
                                     continue
                                 }
                             }
+                            let event = match raw {
+                                Raw::Event(event) => event,
+                                Raw::Error(err) => return Some(Err(err.into())),
+                            };
                             if let Some((module, variant)) = self.event {
                                 if event.module != module || event.variant != variant {
                                     continue