Merge pull request #3496 from autonomys/xdm-fee-bookkeeping

Correctly count the cross-chain XDM fee transfer and ensure the balance bookkeeping match the domain total issuance

Author: NingLin-P

crates/pallet-domains/src/lib.rs

@@ -10,7 +10,7 @@ mod benchmarking;
 mod tests;
 
 pub mod block_tree;
-mod bundle_storage_fund;
+pub mod bundle_storage_fund;
 pub mod domain_registry;
 pub mod extensions;
 pub mod migrations;

crates/subspace-runtime/src/lib.rs

@@ -767,6 +767,7 @@ impl pallet_messenger::Config for Runtime {
     type MaxOutgoingMessages = MaxOutgoingMessages;
     type MessengerOrigin = pallet_messenger::EnsureMessengerOrigin;
     type MessageVersion = MessageVersion;
+    type NoteChainTransfer = Transporter;
 }
 
 impl<C> frame_system::offchain::CreateTransactionBase<C> for Runtime
@@ -1800,10 +1801,22 @@ impl_runtime_apis! {
 #[cfg(test)]
 mod tests {
     use crate::{Runtime, SubspaceBlockWeights as BlockWeights};
+    use pallet_domains::bundle_storage_fund::AccountType;
+    use sp_domains::OperatorId;
+    use sp_runtime::traits::AccountIdConversion;
     use subspace_runtime_primitives::tests_utils::FeeMultiplierUtils;
 
     #[test]
     fn multiplier_can_grow_from_zero() {
         FeeMultiplierUtils::<Runtime, BlockWeights>::multiplier_can_grow_from_zero()
     }
+
+    #[test]
+    fn test_bundle_storage_fund_account_uniqueness() {
+        let _: <Runtime as frame_system::Config>::AccountId = <Runtime as pallet_domains::Config>::PalletId::get()
+            .try_into_sub_account((AccountType::StorageFund, OperatorId::MAX))
+            .expect(
+                "The `AccountId` type must be large enough to fit the seed of the bundle storage fund account",
+            );
+    }
 }

domains/client/domain-operator/src/tests.rs

@@ -75,7 +75,7 @@ use std::time::Duration;
 use subspace_core_primitives::pot::PotOutput;
 use subspace_runtime_primitives::opaque::Block as CBlock;
 use subspace_runtime_primitives::{Balance, SSC};
-use subspace_test_primitives::DOMAINS_BLOCK_PRUNING_DEPTH;
+use subspace_test_primitives::{OnchainStateApi as _, DOMAINS_BLOCK_PRUNING_DEPTH};
 use subspace_test_runtime::Runtime;
 use subspace_test_service::{
     produce_block_with, produce_blocks, produce_blocks_until, MockConsensusNode,
@@ -8426,6 +8426,112 @@ async fn test_invalid_chain_reward_receipt() {
     }
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn test_domain_total_issuance_match_consensus_chain_bookkeeping_with_xdm() {
+    let directory = TempDir::new().expect("Must be able to create temporary directory");
+
+    let mut builder = sc_cli::LoggerBuilder::new("");
+    builder.with_colors(false);
+    let _ = builder.init();
+
+    let tokio_handle = tokio::runtime::Handle::current();
+
+    // Start Ferdie
+    let mut ferdie = MockConsensusNode::run(
+        tokio_handle.clone(),
+        Sr25519Alice,
+        BasePath::new(directory.path().join("ferdie")),
+    );
+
+    // Run Alice (a system domain authority node)
+    let mut alice = domain_test_service::DomainNodeBuilder::new(
+        tokio_handle.clone(),
+        BasePath::new(directory.path().join("alice")),
+    )
+    .build_evm_node(Role::Authority, Alice, &mut ferdie)
+    .await;
+
+    // Run the cross domain gossip message worker
+    ferdie.start_cross_domain_gossip_message_worker();
+
+    produce_blocks!(ferdie, alice, 3).await.unwrap();
+
+    // Open XDM channel between the consensus chain and the EVM domain
+    open_xdm_channel(&mut ferdie, &mut alice).await;
+
+    for i in 0..10 {
+        let transfer_amount = 123456789987654321 * i;
+        let domain_to_consensus = i % 2 == 0;
+
+        // Transfer balance from
+        let pre_alice_free_balance = alice.free_balance(alice.key.to_account_id());
+        let pre_ferdie_free_balance = ferdie.free_balance(ferdie.key.to_account_id());
+
+        if domain_to_consensus {
+            alice
+                .construct_and_send_extrinsic(pallet_transporter::Call::transfer {
+                    dst_location: pallet_transporter::Location {
+                        chain_id: ChainId::Consensus,
+                        account_id: AccountIdConverter::convert(Sr25519Alice.into()),
+                    },
+                    amount: transfer_amount,
+                })
+                .await
+                .expect("Failed to construct and send extrinsic");
+        } else {
+            ferdie
+                .construct_and_send_extrinsic_with(pallet_transporter::Call::transfer {
+                    dst_location: pallet_transporter::Location {
+                        chain_id: ChainId::Domain(EVM_DOMAIN_ID),
+                        account_id: AccountId20Converter::convert(Alice.to_account_id()),
+                    },
+                    amount: transfer_amount,
+                })
+                .await
+                .expect("Failed to construct and send extrinsic");
+        }
+
+        // Wait until transfer succeed
+        produce_blocks_until!(ferdie, alice, {
+            let post_alice_free_balance = alice.free_balance(alice.key.to_account_id());
+            let post_ferdie_free_balance = ferdie.free_balance(ferdie.key.to_account_id());
+
+            if domain_to_consensus {
+                post_alice_free_balance <= pre_alice_free_balance - transfer_amount
+                    && post_ferdie_free_balance == pre_ferdie_free_balance + transfer_amount
+            } else {
+                post_ferdie_free_balance <= pre_ferdie_free_balance - transfer_amount
+                    && post_alice_free_balance == pre_alice_free_balance + transfer_amount
+            }
+        })
+        .await
+        .unwrap();
+
+        let ferdie_best_hash = ferdie.client.info().best_hash;
+        let (_, confirmed_domain_hash) = ferdie
+            .client
+            .runtime_api()
+            .latest_confirmed_domain_block(ferdie_best_hash, EVM_DOMAIN_ID)
+            .unwrap()
+            .unwrap();
+        let domain_total_balance = ferdie
+            .client
+            .runtime_api()
+            .domain_balance(ferdie_best_hash, EVM_DOMAIN_ID)
+            .unwrap();
+        let domain_total_issuance = alice
+            .client
+            .runtime_api()
+            .total_issuance(confirmed_domain_hash)
+            .unwrap();
+        assert_eq!(domain_total_balance, domain_total_issuance);
+
+        ferdie.clear_tx_pool().await.unwrap();
+        alice.clear_tx_pool().await;
+        produce_blocks!(ferdie, alice, 5).await.unwrap();
+    }
+}
+
 fn bundle_to_tx(
     ferdie: &MockConsensusNode,
     opaque_bundle: OpaqueBundleOf<Runtime>,

domains/pallets/messenger/src/fees.rs

@@ -1,15 +1,20 @@
 #[cfg(not(feature = "std"))]
 extern crate alloc;
-use crate::pallet::{InboxFee, InboxResponseMessageWeightTags, InboxResponses, OutboxFee};
+use crate::pallet::{
+    InboxFee, InboxFeesOnHold, InboxFeesOnHoldStartAt, InboxResponseMessageWeightTags,
+    InboxResponses, OutboxFee, OutboxFeesOnHold, OutboxFeesOnHoldStartAt,
+};
 use crate::{BalanceOf, Config, Error, Pallet};
-use frame_support::traits::fungible::Mutate;
+#[cfg(not(feature = "std"))]
+use alloc::vec;
+use frame_support::traits::fungible::{Balanced, Mutate};
 use frame_support::traits::tokens::{Fortitude, Precision, Preservation};
 use frame_support::weights::WeightToFee;
 use sp_core::Get;
 use sp_messenger::endpoint::{CollectedFee, Endpoint};
 use sp_messenger::messages::{ChainId, ChannelId, FeeModel, MessageId, Nonce};
 use sp_messenger::OnXDMRewards;
-use sp_runtime::traits::{CheckedAdd, CheckedMul, Zero};
+use sp_runtime::traits::{CheckedAdd, CheckedMul, CheckedSub, Zero};
 use sp_runtime::{DispatchError, DispatchResult, Saturating};
 
 impl<T: Config> Pallet<T> {
@@ -136,6 +141,9 @@ impl<T: Config> Pallet<T> {
 
         let mut current_nonce = latest_confirmed_nonce;
         let mut inbox_fees = BalanceOf::<T>::zero();
+        let on_hold_start_at_nonce =
+            InboxFeesOnHoldStartAt::<T>::get(channel_id).unwrap_or(Nonce::MAX);
+        let mut on_hold_inbox_fees = BalanceOf::<T>::zero();
         while let Some(nonce) = current_nonce {
             if InboxResponses::<T>::take((dst_chain_id, channel_id, nonce)).is_none() {
                 // Make the loop efficient, by breaking as soon as there are no more responses.
@@ -150,12 +158,30 @@ impl<T: Config> Pallet<T> {
             // For every inbox response we take, distribute the reward to the operators.
             if let Some(inbox_fee) = InboxFee::<T>::take((dst_chain_id, (channel_id, nonce))) {
                 inbox_fees = inbox_fees.saturating_add(inbox_fee);
+                if on_hold_start_at_nonce <= nonce {
+                    on_hold_inbox_fees = on_hold_inbox_fees.saturating_add(inbox_fee);
+                }
             }
 
             current_nonce = nonce.checked_sub(Nonce::one())
         }
 
         if !inbox_fees.is_zero() {
+            if !on_hold_inbox_fees.is_zero() {
+                InboxFeesOnHold::<T>::mutate(|inbox_fees_on_hold| {
+                    *inbox_fees_on_hold = inbox_fees_on_hold
+                        .checked_sub(&on_hold_inbox_fees)
+                        .ok_or(Error::<T>::BalanceUnderflow)?;
+
+                    // If the `imbalance` is dropped without consuming it will increase the total issuance by
+                    // the same amount as we rescinded here, thus we need to manually `mem::forget` it.
+                    let imbalance = T::Currency::rescind(on_hold_inbox_fees);
+                    core::mem::forget(imbalance);
+
+                    Ok::<(), Error<T>>(())
+                })?;
+            }
+
             Self::reward_operators(inbox_fees);
         }
 
@@ -165,10 +191,29 @@ impl<T: Config> Pallet<T> {
     pub(crate) fn reward_operators_for_outbox_execution(
         dst_chain_id: ChainId,
         message_id: MessageId,
-    ) {
+    ) -> DispatchResult {
         if let Some(fee) = OutboxFee::<T>::take((dst_chain_id, message_id)) {
+            let update_on_hold = OutboxFeesOnHoldStartAt::<T>::get(message_id.0)
+                .map(|start_at_nonce| start_at_nonce <= message_id.1)
+                .unwrap_or(false);
+            if update_on_hold {
+                OutboxFeesOnHold::<T>::mutate(|outbox_fees_on_hold| {
+                    *outbox_fees_on_hold = outbox_fees_on_hold
+                        .checked_sub(&fee)
+                        .ok_or(Error::<T>::BalanceUnderflow)?;
+
+                    // If the `imbalance` is dropped without consuming it will increase the total issuance by
+                    // the same amount as we rescinded here, thus we need to manually `mem::forget` it.
+                    let imbalance = T::Currency::rescind(fee);
+                    core::mem::forget(imbalance);
+
+                    Ok::<(), Error<T>>(())
+                })?;
+            }
+
             Self::reward_operators(fee);
         }
+        Ok(())
     }
 
     /// Increments the current block's relayer rewards.