fix: propagate errors in system_tables

Author: stuarttimwhite

pallets/system_tables/src/lib.rs

@@ -241,11 +241,11 @@ pub mod pallet {
 
                         Ok(())
                     }
-                    _ => Err(Error::<T>::MissingExpectedField.into()),
+                    _ => emit_missing_expected_field_error::<T>(),
                 }
             })
-            .for_each(emit_for_error::<T>);
-        Ok(())
+            .collect::<Result<Vec<_>, _>>()
+            .map(|_| ())
     }
 
     /// Process a Nominate SystemRequest
@@ -272,9 +272,10 @@ pub mod pallet {
                                 errors
                             );
 
-                            errors
-                                .into_iter()
-                                .for_each(|e| emit_for_error::<T>(Result::<(), _>::Err(e)));
+                            for error in errors.iter().copied() {
+                                emit_error::<T>(error);
+                            }
+                            Err(*errors.first().expect("There is at least one error"))?;
                         }
 
                         if nominations.is_empty() {
@@ -289,12 +290,11 @@ pub mod pallet {
                         pallet_staking::Pallet::<T>::nominate(nominator_signer, nominations)?;
                         Ok(())
                     }
-                    _ => Err(Error::<T>::MissingExpectedField.into()),
+                    _ => emit_missing_expected_field_error::<T>(),
                 }
             })
-            .for_each(emit_for_error::<T>);
-
-        Ok(())
+            .collect::<Result<Vec<_>, _>>()
+            .map(|_| ())
     }
 
     /// Processes the actual withdrawal once the Unstaked event is emitted from the Ethereum
@@ -325,12 +325,11 @@ pub mod pallet {
 
                         Ok(())
                     }
-                    _ => Err(Error::<T>::MissingExpectedField.into()),
+                    _ => emit_missing_expected_field_error::<T>(),
                 }
             })
-            .for_each(emit_for_error::<T>);
-
-        Ok(())
+            .collect::<Result<Vec<_>, _>>()
+            .map(|_| ())
     }
 
     /// Processes an unstake claimed event and updates chain state appropriately
@@ -383,11 +382,11 @@ pub mod pallet {
 
                         Ok(())
                     }
-                    _ => Err(Error::<T>::MissingExpectedField.into()),
+                    _ => emit_missing_expected_field_error::<T>(),
                 }
             })
-            .for_each(emit_for_error::<T>);
-        Ok(())
+            .collect::<Result<Vec<_>, _>>()
+            .map(|_| ())
     }
 
     /// Parse a system request to initiate unstaking
@@ -420,12 +419,11 @@ pub mod pallet {
 
                         Ok(())
                     }
-                    _ => Err(Error::<T>::MissingExpectedField.into()),
+                    _ => emit_missing_expected_field_error::<T>(),
                 }
             })
-            .for_each(emit_for_error::<T>);
-
-        Ok(())
+            .collect::<Result<Vec<_>, _>>()
+            .map(|_| ())
     }
 
     /// Process a request to cancel unstakng
@@ -448,18 +446,21 @@ pub mod pallet {
                             .map_err(|e| e.error)?;
                         Ok(())
                     }
-                    _ => Err(Error::<T>::MissingExpectedField.into()),
+                    _ => emit_missing_expected_field_error::<T>(),
                 }
             })
-            .for_each(emit_for_error::<T>);
-        Ok(())
+            .collect::<Result<Vec<_>, _>>()
+            .map(|_| ())
     }
 
-    fn emit_for_error<T: Config>(r: DispatchResult) {
-        if let Err(error) = r {
-            // Emit an event for any errors
-            Pallet::<T>::deposit_event(Event::<T>::MessageProcessingError { error });
-        }
+    fn emit_error<T: Config>(error: DispatchError) {
+        Pallet::<T>::deposit_event(Event::<T>::MessageProcessingError { error });
+    }
+
+    fn emit_missing_expected_field_error<T: Config>() -> DispatchResult {
+        let error = Error::<T>::MissingExpectedField.into();
+        emit_error::<T>(error);
+        Err(error)
     }
 
     #[pallet::storage]
@@ -502,12 +503,11 @@ pub mod pallet {
                         messages::handle_message::<T>(eth_sender, body.to_vec())?;
                         Ok(())
                     }
-                    _ => Err(Error::<T>::MissingExpectedField.into()),
+                    _ => emit_missing_expected_field_error::<T>(),
                 }
             })
-            .for_each(emit_for_error::<T>);
-
-        Ok(())
+            .collect::<Result<Vec<_>, _>>()
+            .map(|_| ())
     }
 
     /// Process a funded message received from our EVM contract
@@ -541,12 +541,11 @@ pub mod pallet {
                         )?;
                         Ok(())
                     }
-                    _ => Err(Error::<T>::MissingExpectedField.into()),
+                    _ => emit_missing_expected_field_error::<T>(),
                 }
             })
-            .for_each(emit_for_error::<T>);
-
-        Ok(())
+            .collect::<Result<Vec<_>, _>>()
+            .map(|_| ())
     }
 
     /// A custom offence handler that chills validators when they offend