Merge pull request #3534 from autonomys/ensure_no_partial_state

ensure no partial state when XDM is being processed

Author: vedhavyas

domains/pallets/messenger/src/lib.rs

@@ -160,6 +160,7 @@ mod pallet {
     use core::cmp::Ordering;
     use frame_support::ensure;
     use frame_support::pallet_prelude::*;
+    use frame_support::storage::with_storage_layer;
     use frame_support::traits::fungible::{Balanced, Inspect, InspectHold, Mutate, MutateHold};
     use frame_support::traits::tokens::{Fortitude, Precision, Preservation};
     use frame_support::weights::WeightToFee;
@@ -973,12 +974,18 @@ mod pallet {
                 if let Some(owner) = &channel.maybe_owner {
                     let hold_id = T::HoldIdentifier::messenger_channel();
                     let locked_amount = channel.channel_reserve_fee;
-                    let amount_to_release = {
+                    let (amount_to_release, maybe_amount_to_burn) = {
                         if channel.state == ChannelState::Open {
-                            locked_amount
+                            (locked_amount, None)
                         } else {
                             let protocol_fee = T::ChannelInitReservePortion::get() * locked_amount;
                             let release_amount = locked_amount.saturating_sub(protocol_fee);
+                            (release_amount, Some(protocol_fee))
+                        }
+                    };
+
+                    with_storage_layer(|| {
+                        if let Some(protocol_fee) = maybe_amount_to_burn {
                             T::Currency::burn_held(
                                 &hold_id,
                                 owner,
@@ -987,11 +994,13 @@ mod pallet {
                                 Fortitude::Force,
                             )?;
                             T::OnXDMRewards::on_chain_protocol_fees(chain_id, protocol_fee);
-                            release_amount
                         }
-                    };
-                    T::Currency::release(&hold_id, owner, amount_to_release, Precision::Exact)
-                        .map_err(|_| Error::<T>::BalanceUnlock)?;
+
+                        T::Currency::release(&hold_id, owner, amount_to_release, Precision::Exact)
+                            .map_err(|_| Error::<T>::BalanceUnlock)?;
+
+                        Ok::<(), DispatchError>(())
+                    })?;
                 }
 
                 channel.state = ChannelState::Closed;

domains/pallets/transporter/src/lib.rs

@@ -482,16 +482,16 @@ impl<T: Config> sp_domains::DomainsTransfersTracker<BalanceOf<T>> for Pallet<T>
     ) -> Result<(), Self::Error> {
         Self::ensure_consensus_chain()?;
 
-        if let Some(domain_id) = from_chain_id.maybe_domain_chain() {
-            DomainBalances::<T>::try_mutate(domain_id, |current_balance| {
-                *current_balance = current_balance
-                    .checked_sub(&amount)
-                    .ok_or(Error::LowBalanceOnDomain)?;
-                Ok(())
-            })?;
-        }
-
         UnconfirmedTransfers::<T>::try_mutate(from_chain_id, to_chain_id, |total_amount| {
+            if let Some(domain_id) = from_chain_id.maybe_domain_chain() {
+                DomainBalances::<T>::try_mutate(domain_id, |current_balance| {
+                    *current_balance = current_balance
+                        .checked_sub(&amount)
+                        .ok_or(Error::LowBalanceOnDomain)?;
+                    Ok(())
+                })?;
+            }
+
             *total_amount = total_amount
                 .checked_add(&amount)
                 .ok_or(Error::BalanceOverflow)?;
@@ -511,18 +511,19 @@ impl<T: Config> sp_domains::DomainsTransfersTracker<BalanceOf<T>> for Pallet<T>
             *total_amount = total_amount
                 .checked_sub(&amount)
                 .ok_or(Error::BalanceUnderflow)?;
+
+            if let Some(domain_id) = to_chain_id.maybe_domain_chain() {
+                DomainBalances::<T>::try_mutate(domain_id, |current_balance| {
+                    *current_balance = current_balance
+                        .checked_add(&amount)
+                        .ok_or(Error::BalanceOverflow)?;
+                    Ok(())
+                })?;
+            }
+
             Ok(())
         })?;
 
-        if let Some(domain_id) = to_chain_id.maybe_domain_chain() {
-            DomainBalances::<T>::try_mutate(domain_id, |current_balance| {
-                *current_balance = current_balance
-                    .checked_add(&amount)
-                    .ok_or(Error::BalanceOverflow)?;
-                Ok(())
-            })?;
-        }
-
         Ok(())
     }
 
@@ -536,17 +537,19 @@ impl<T: Config> sp_domains::DomainsTransfersTracker<BalanceOf<T>> for Pallet<T>
             *total_amount = total_amount
                 .checked_sub(&amount)
                 .ok_or(Error::BalanceUnderflow)?;
+
+            if let Some(domain_id) = from_chain_id.maybe_domain_chain() {
+                DomainBalances::<T>::try_mutate(domain_id, |current_balance| {
+                    *current_balance = current_balance
+                        .checked_add(&amount)
+                        .ok_or(Error::BalanceOverflow)?;
+                    Ok(())
+                })?;
+            }
+
             Ok(())
         })?;
 
-        if let Some(domain_id) = from_chain_id.maybe_domain_chain() {
-            DomainBalances::<T>::try_mutate(domain_id, |current_balance| {
-                *current_balance = current_balance
-                    .checked_add(&amount)
-                    .ok_or(Error::BalanceOverflow)?;
-                Ok(())
-            })?;
-        }
         Ok(())
     }
 
@@ -560,15 +563,17 @@ impl<T: Config> sp_domains::DomainsTransfersTracker<BalanceOf<T>> for Pallet<T>
             *total_amount = total_amount
                 .checked_sub(&amount)
                 .ok_or(Error::BalanceUnderflow)?;
-            Ok(())
-        })?;
 
-        CancelledTransfers::<T>::try_mutate(from_chain_id, to_chain_id, |total_amount| {
-            *total_amount = total_amount
-                .checked_add(&amount)
-                .ok_or(Error::BalanceOverflow)?;
+            CancelledTransfers::<T>::try_mutate(from_chain_id, to_chain_id, |total_amount| {
+                *total_amount = total_amount
+                    .checked_add(&amount)
+                    .ok_or(Error::BalanceOverflow)?;
+                Ok(())
+            })?;
+
             Ok(())
         })?;
+
         Ok(())
     }