Argo Bridge: limit revert_outbound_transfer rationale

runtime-modules/argo-bridge/src/benchmarking.rs

@@ -130,6 +130,39 @@ benchmarks! {
             RawEvent::OutboundTransferRequested(transfer_id, sender, dest_account, transfer_amount, fee).into());
     }
 
+    // Worse case scenario
+    // - rationale of the maximum size
+    revert_outbound_transfer{
+        let fee: BalanceOf<T> = 10u32.into();
+        let pauser_acount = T::AccountId::create_account_id(1u32);
+        let operator_account = T::AccountId::create_account_id(1u32);
+        let remote_chains: Vec<u32> = (0..MAX_REMOTE_CHAINS).collect();
+        let parameters = BridgeConstraints {
+            operator_account: Some(operator_account.clone()),
+            pauser_accounts: Some(vec![pauser_acount.clone()]),
+            bridging_fee: Some(fee),
+            thawn_duration: Some(1u32.into()),
+            remote_chains: Some(BoundedVec::try_from(remote_chains).unwrap())
+        };
+
+        ArgoBridge::<T>::update_bridge_constrains(
+            RawOrigin::Root.into(),
+            parameters
+        ).unwrap();
+        activate_bridge::<T>(&pauser_acount, &operator_account);
+
+        let revert_amount: u32 = 1030u32;
+        set_bridge_mint_allowance::<T>(revert_amount.into(), fee);
+
+        let transfer_id = 1u64;
+        let rationale = vec![0u8; (MAX_BYTES_RATIONALE) as usize];
+        let revert_account = T::AccountId::create_account_id(2u32);
+    }: _(RawOrigin::Signed(operator_account), transfer_id, revert_account.clone(), revert_amount.into(), rationale.clone().try_into().unwrap())
+    verify {
+        assert_last_event::<T>(
+            RawEvent::OutboundTransferReverted(transfer_id, revert_account, revert_amount.into(), rationale.try_into().unwrap()).into());
+    }
+
 
     // Worst case scenario:
     // - max number of remote chains being use
@@ -295,6 +328,13 @@ mod tests {
         });
     }
 
+    #[test]
+    fn test_revert_outbound_transfer() {
+        with_test_externalities(|| {
+            assert_ok!(ArgoBridge::test_benchmark_revert_outbound_transfer());
+        });
+    }
+
     #[test]
     fn test_finalize_inbound_transfer() {
         with_test_externalities(|| {

runtime-modules/argo-bridge/src/events.rs

@@ -3,10 +3,12 @@
 use frame_support::decl_event;
 
 use crate::{RemoteAccount, RemoteTransfer, TransferId};
-use sp_std::vec::Vec;
 
 use crate::types::*;
 
+use frame_support::storage::bounded_vec::BoundedVec;
+use frame_support::traits::ConstU32;
+
 // Balance type alias
 type BalanceOf<T> = <T as balances::Config>::Balance;
 
@@ -20,7 +22,12 @@ decl_event!(
     {
         OutboundTransferRequested(TransferId, AccountId, RemoteAccount, Balance, Balance),
         InboundTransferFinalized(RemoteTransfer, AccountId, Balance),
-        OutboundTransferReverted(TransferId, AccountId, Balance, Vec<u8>),
+        OutboundTransferReverted(
+            TransferId,
+            AccountId,
+            Balance,
+            BoundedVec<u8, ConstU32<MAX_BYTES_RATIONALE>>,
+        ),
         BridgePaused(AccountId),
         BridgeThawnStarted(AccountId, BlockNumber),
         BridgeThawnFinished(),

runtime-modules/argo-bridge/src/lib.rs

@@ -137,14 +137,13 @@ decl_module! {
             Ok(())
         }
 
-        // TODO: add weight for revert_outbound_transfer
-        #[weight = WeightInfoArgo::<T>::finalize_inbound_transfer()]
+        #[weight = WeightInfoArgo::<T>::revert_outbound_transfer()]
         pub fn revert_outbound_transfer(
             origin,
             transfer_id: TransferId,
             revert_account: T::AccountId,
             revert_amount: BalanceOf<T>,
-            rationale: vec::Vec<u8>,
+            rationale: BoundedVec<u8, ConstU32<MAX_BYTES_RATIONALE>>,
         ) -> DispatchResult {
             ensure!(Self::operator_account().is_some(), Error::<T>::OperatorAccountNotSet);
             let caller = ensure_signed(origin)?;

runtime-modules/argo-bridge/src/tests/mod.rs

@@ -1,6 +1,6 @@
 #![cfg(test)]
 
-use core::convert::TryFrom;
+use core::convert::{TryFrom, TryInto};
 
 use crate::tests::mock::*;
 use frame_support::{assert_err, assert_ok};
@@ -338,15 +338,15 @@ fn revert_outbound_transfer_success() {
             transfer_id,
             revert_account,
             revert_amount,
-            rationale.clone(),
+            rationale.clone().try_into().unwrap(),
         );
         assert_ok!(result);
         assert_eq!(Balances::free_balance(revert_account), revert_amount);
         last_event_eq!(RawEvent::OutboundTransferReverted(
             transfer_id,
             revert_account,
             revert_amount,
-            rationale,
+            rationale.try_into().unwrap(),
         ));
     });
 }
@@ -359,7 +359,7 @@ fn revert_outbound_transfer_with_no_operator_account() {
             1u64,
             account!(2),
             joy!(123),
-            vec![],
+            vec![].try_into().unwrap(),
         );
         assert_err!(result, Error::<Test>::OperatorAccountNotSet);
     });
@@ -386,7 +386,7 @@ fn revert_outbound_transfer_with_unauthorized_account() {
             1u64,
             account!(2),
             joy!(123),
-            vec![],
+            vec![].try_into().unwrap(),
         );
         assert_err!(result, Error::<Test>::NotOperatorAccount);
     });
@@ -413,7 +413,7 @@ fn revert_outbound_transfer_with_insufficient_bridge_mint() {
             1u64,
             account!(2),
             joy!(100),
-            vec![],
+            vec![].try_into().unwrap(),
         );
         assert_err!(result, Error::<Test>::InsufficientBridgeMintAllowance);
     });

runtime-modules/argo-bridge/src/types.rs

@@ -14,6 +14,8 @@ pub type TransferId = u64;
 
 pub const MAX_REMOTE_CHAINS: u32 = 10;
 
+pub const MAX_BYTES_RATIONALE: u32 = 200;
+
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
 #[derive(Encode, Decode, Clone, PartialEq, Eq, Debug, TypeInfo, MaxEncodedLen)]
 pub struct BridgeConstraints<AccountId, Balance, BlockNumber> {

runtime-modules/argo-bridge/src/weights.rs

@@ -18,7 +18,7 @@
 //! Autogenerated weights for argo_bridge
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
-//! DATE: 2024-05-31, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! DATE: 2024-06-04, STEPS: `50`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
 //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("prod-test"), DB CACHE: 1024
 
 // Executed Command:
@@ -45,6 +45,7 @@ use sp_std::marker::PhantomData;
 /// Weight functions needed for argo_bridge.
 pub trait WeightInfo {
 	fn request_outbound_transfer() -> Weight;
+	fn revert_outbound_transfer() -> Weight;
 	fn finalize_inbound_transfer() -> Weight;
 	fn pause_bridge() -> Weight;
 	fn init_unpause_bridge() -> Weight;
@@ -71,8 +72,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 		// Proof Size summary in bytes:
 		//  Measured:  `430`
 		//  Estimated: `11104`
-		// Minimum execution time: 998_107 nanoseconds.
-		Weight::from_parts(1_038_838_000, 0u64)
+		// Minimum execution time: 1_128_198 nanoseconds.
+		Weight::from_parts(1_308_617_000, 0u64)
 			.saturating_add(Weight::from_parts(0, 11104))
 			.saturating_add(T::DbWeight::get().reads(6_u64))
 			.saturating_add(T::DbWeight::get().writes(3_u64))
@@ -83,6 +84,24 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 	// Proof: ArgoBridge Status (max_values: Some(1), max_size: Some(5), added: 500, mode: MaxEncodedLen)
 	// Storage: ArgoBridge MintAllowance (r:1 w:1)
 	// Proof: ArgoBridge MintAllowance (max_values: Some(1), max_size: Some(16), added: 511, mode: MaxEncodedLen)
+	// Storage: System Account (r:1 w:0)
+	// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen)
+	fn revert_outbound_transfer() -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `321`
+		//  Estimated: `8101`
+		// Minimum execution time: 858_538 nanoseconds.
+		Weight::from_parts(937_558_000, 0u64)
+			.saturating_add(Weight::from_parts(0, 8101))
+			.saturating_add(T::DbWeight::get().reads(4_u64))
+			.saturating_add(T::DbWeight::get().writes(1_u64))
+	}
+	// Storage: ArgoBridge OperatorAccount (r:1 w:0)
+	// Proof: ArgoBridge OperatorAccount (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen)
+	// Storage: ArgoBridge Status (r:1 w:0)
+	// Proof: ArgoBridge Status (max_values: Some(1), max_size: Some(5), added: 500, mode: MaxEncodedLen)
+	// Storage: ArgoBridge MintAllowance (r:1 w:1)
+	// Proof: ArgoBridge MintAllowance (max_values: Some(1), max_size: Some(16), added: 511, mode: MaxEncodedLen)
 	// Storage: ArgoBridge RemoteChains (r:1 w:0)
 	// Proof: ArgoBridge RemoteChains (max_values: Some(1), max_size: Some(41), added: 536, mode: MaxEncodedLen)
 	// Storage: System Account (r:1 w:1)
@@ -91,8 +110,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 		// Proof Size summary in bytes:
 		//  Measured:  `473`
 		//  Estimated: `9627`
-		// Minimum execution time: 883_138 nanoseconds.
-		Weight::from_parts(900_668_000, 0u64)
+		// Minimum execution time: 967_528 nanoseconds.
+		Weight::from_parts(1_171_918_000, 0u64)
 			.saturating_add(Weight::from_parts(0, 9627))
 			.saturating_add(T::DbWeight::get().reads(5_u64))
 			.saturating_add(T::DbWeight::get().writes(2_u64))
@@ -105,8 +124,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 		// Proof Size summary in bytes:
 		//  Measured:  `498`
 		//  Estimated: `1806`
-		// Minimum execution time: 371_039 nanoseconds.
-		Weight::from_parts(382_129_000, 0u64)
+		// Minimum execution time: 414_149 nanoseconds.
+		Weight::from_parts(424_310_000, 0u64)
 			.saturating_add(Weight::from_parts(0, 1806))
 			.saturating_add(T::DbWeight::get().reads(1_u64))
 			.saturating_add(T::DbWeight::get().writes(1_u64))
@@ -121,8 +140,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 		// Proof Size summary in bytes:
 		//  Measured:  `498`
 		//  Estimated: `3295`
-		// Minimum execution time: 402_659 nanoseconds.
-		Weight::from_parts(420_319_000, 0u64)
+		// Minimum execution time: 426_939 nanoseconds.
+		Weight::from_parts(467_489_000, 0u64)
 			.saturating_add(Weight::from_parts(0, 3295))
 			.saturating_add(T::DbWeight::get().reads(2_u64))
 			.saturating_add(T::DbWeight::get().writes(1_u64))
@@ -135,8 +154,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 		// Proof Size summary in bytes:
 		//  Measured:  `173`
 		//  Estimated: `3007`
-		// Minimum execution time: 412_269 nanoseconds.
-		Weight::from_parts(422_459_000, 0u64)
+		// Minimum execution time: 613_939 nanoseconds.
+		Weight::from_parts(791_438_000, 0u64)
 			.saturating_add(Weight::from_parts(0, 3007))
 			.saturating_add(T::DbWeight::get().reads(2_u64))
 			.saturating_add(T::DbWeight::get().writes(1_u64))
@@ -155,8 +174,8 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 		// Proof Size summary in bytes:
 		//  Measured:  `0`
 		//  Estimated: `0`
-		// Minimum execution time: 293_519 nanoseconds.
-		Weight::from_parts(298_009_000, 0u64)
+		// Minimum execution time: 355_860 nanoseconds.
+		Weight::from_parts(454_089_000, 0u64)
 			.saturating_add(Weight::from_parts(0, 0))
 			.saturating_add(T::DbWeight::get().writes(5_u64))
 	}
@@ -167,6 +186,9 @@ impl WeightInfo for () {
 	fn request_outbound_transfer() -> Weight {
 		Weight::from_parts(0, 0)
 	}
+	fn revert_outbound_transfer() -> Weight {
+		Weight::from_parts(0, 0)
+	}
 	fn finalize_inbound_transfer() -> Weight {
 		Weight::from_parts(0, 0)
 	}