feat: more information added to AllBatchError. (PRO-1171)

state-chain/chains/src/btc/api.rs

@@ -67,7 +67,7 @@ where
 		transfer_params: Vec<TransferAssetParams<Bitcoin>>,
 	) -> Result<Self, AllBatchError> {
 		let agg_key @ AggKey { current, .. } =
-			<E as ChainEnvironment<(), AggKey>>::lookup(()).ok_or(AllBatchError::Other)?;
+			<E as ChainEnvironment<(), AggKey>>::lookup(()).ok_or(AllBatchError::AggKeyNotSet)?;
 		let bitcoin_change_script =
 			DepositAddress::new(current, CHANGE_ADDRESS_SALT).script_pubkey();
 		let mut total_output_amount: u64 = 0;
@@ -89,7 +89,7 @@ where
 				.ok_or(AllBatchError::NotRequired)?,
 			number_of_outputs: (btc_outputs.len() + 1) as u64, // +1 for the change output
 		})
-		.ok_or(AllBatchError::Other)?;
+		.ok_or(AllBatchError::UtxoSelectionFailed)?;
 		if change_amount >= BITCOIN_DUST_LIMIT {
 			btc_outputs.push(BitcoinOutput {
 				amount: change_amount,

state-chain/chains/src/dot/api.rs

@@ -66,7 +66,7 @@ where
 			E::replay_protection(false),
 			fetch_params,
 			transfer_params,
-			E::try_vault_account().ok_or(AllBatchError::Other)?,
+			E::try_vault_account().ok_or(AllBatchError::VaultAccountNotSet)?,
 		)))
 	}
 }

state-chain/chains/src/eth/api.rs

@@ -232,7 +232,7 @@ where
 					EvmFetchId::NotRequired => (),
 				};
 			} else {
-				return Err(AllBatchError::Other)
+				return Err(AllBatchError::UnsupportedToken)
 			}
 		}
 		if fetch_only_params.is_empty() &&
@@ -255,7 +255,7 @@ where
 									amount,
 									asset: address,
 								})
-								.ok_or(AllBatchError::Other)
+								.ok_or(AllBatchError::UnsupportedToken)
 						})
 						.collect::<Result<Vec<_>, _>>()?,
 				),

state-chain/chains/src/lib.rs

@@ -341,10 +341,31 @@ pub trait RegisterRedemption: ApiCall<<Ethereum as Chain>::ChainCrypto> {
 	fn amount(&self) -> u128;
 }
 
-#[derive(Debug)]
+#[derive(Debug, Encode, Decode, Clone, PartialEq, Eq, TypeInfo)]
 pub enum AllBatchError {
+	/// Empty transaction - the call is not required.
 	NotRequired,
-	Other,
+
+	/// The token address lookup failed. The token is not supported on the target chain.
+	UnsupportedToken,
+
+	/// The vault account is not set.
+	VaultAccountNotSet,
+
+	/// The Aggregate key lookup failed
+	AggKeyNotSet,
+
+	/// Unable to select Utxos.
+	UtxoSelectionFailed,
+
+	/// Some other DispatchError occurred.
+	DispatchError(DispatchError),
+}
+
+impl From<DispatchError> for AllBatchError {
+	fn from(e: DispatchError) -> Self {
+		AllBatchError::DispatchError(e)
+	}
 }
 
 #[derive(Debug)]

state-chain/pallets/cf-ingress-egress/src/lib.rs

@@ -36,7 +36,7 @@ use cf_traits::{
 };
 use frame_support::{
 	pallet_prelude::*,
-	sp_runtime::{traits::Zero, DispatchError, Saturating, TransactionOutcome},
+	sp_runtime::{traits::Zero, DispatchError, Saturating},
 };
 use frame_system::pallet_prelude::*;
 pub use pallet::*;
@@ -140,7 +140,6 @@ pub mod pallet {
 	use cf_traits::LpDepositHandler;
 	use core::marker::PhantomData;
 	use frame_support::{
-		storage::with_transaction,
 		traits::{ConstU128, EnsureOrigin, IsType},
 		DefaultNoBound,
 	};
@@ -533,6 +532,9 @@ pub mod pallet {
 		UtxoConsolidation {
 			broadcast_id: BroadcastId,
 		},
+		FailedToBuildAllBatchCall {
+			error: AllBatchError,
+		},
 		ChannelOpeningFeePaid {
 			fee: T::Amount,
 		},
@@ -598,8 +600,9 @@ pub mod pallet {
 		/// Take all scheduled Egress and send them out
 		fn on_finalize(_n: BlockNumberFor<T>) {
 			// Send all fetch/transfer requests as a batch. Revert storage if failed.
-			if let Err(e) = with_transaction(|| Self::do_egress_scheduled_fetch_transfer()) {
-				log::error!("Ingress-Egress failed to send BatchAll. Error: {e:?}");
+			if let Err(error) = Self::do_egress_scheduled_fetch_transfer() {
+				log::error!("Ingress-Egress failed to send BatchAll. Error: {error:?}");
+				Self::deposit_event(Event::<T, I>::FailedToBuildAllBatchCall { error });
 			}
 
 			if let Ok(egress_transaction) =
@@ -885,7 +888,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	/// Take all scheduled egress requests and send them out in an `AllBatch` call.
 	///
 	/// Note: Egress transactions with Blacklisted assets are not sent, and kept in storage.
-	fn do_egress_scheduled_fetch_transfer() -> TransactionOutcome<DispatchResult> {
+	#[transactional]
+	fn do_egress_scheduled_fetch_transfer() -> Result<(), AllBatchError> {
 		let batch_to_send: Vec<_> =
 			ScheduledEgressFetchOrTransfer::<T, I>::mutate(|requests: &mut Vec<_>| {
 				// Filter out disabled assets and requests that are not ready to be egressed.
@@ -927,7 +931,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			});
 
 		if batch_to_send.is_empty() {
-			return TransactionOutcome::Commit(Ok(()))
+			return Ok(())
 		}
 
 		let mut fetch_params = vec![];
@@ -983,12 +987,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 					broadcast_id,
 					egress_ids,
 				});
-				TransactionOutcome::Commit(Ok(()))
+				Ok(())
 			},
-			Err(AllBatchError::NotRequired) => TransactionOutcome::Commit(Ok(())),
-			Err(AllBatchError::Other) => TransactionOutcome::Rollback(Err(DispatchError::Other(
-				"AllBatch ApiCall creation failed, rolled back storage",
-			))),
+			Err(AllBatchError::NotRequired) => Ok(()),
+			Err(other) => Err(other),
 		}
 	}
 

state-chain/pallets/cf-ingress-egress/src/tests.rs

@@ -1489,6 +1489,31 @@ fn consolidation_tx_gets_broadcasted_on_finalize() {
 	});
 }
 
+#[test]
+fn all_batch_errors_are_logged_as_event() {
+	new_test_ext()
+		.execute_with(|| {
+			ScheduledEgressFetchOrTransfer::<Test>::set(vec![
+				FetchOrTransfer::<Ethereum>::Transfer {
+					asset: ETH_ETH,
+					amount: 1_000,
+					destination_address: ALICE_ETH_ADDRESS,
+					egress_id: (ForeignChain::Ethereum, 1),
+				},
+			]);
+			MockEthAllBatch::set_success(false);
+		})
+		.then_execute_at_next_block(|_| {})
+		.then_execute_with(|_| {
+			System::assert_last_event(RuntimeEvent::IngressEgress(
+				crate::Event::<Test>::FailedToBuildAllBatchCall {
+					error: "Other(\"Failed to build AllBatch call. Error: UnsupportedToken\")"
+						.to_string(),
+				},
+			));
+		});
+}
+
 #[test]
 fn broker_pays_a_fee_for_each_deposit_address() {
 	new_test_ext().execute_with(|| {

state-chain/traits/src/mocks/api_call.rs

@@ -90,7 +90,7 @@ impl AllBatch<Ethereum> for MockEthereumApiCall<MockEthEnvironment> {
 				_phantom: PhantomData,
 			}))
 		} else {
-			Err(AllBatchError::Other)
+			Err(AllBatchError::UnsupportedToken)
 		}
 	}
 }
@@ -294,7 +294,7 @@ impl AllBatch<Bitcoin> for MockBitcoinApiCall<MockBtcEnvironment> {
 				_phantom: PhantomData,
 			}))
 		} else {
-			Err(AllBatchError::Other)
+			Err(AllBatchError::UnsupportedToken)
 		}
 	}
 }