Skip to content

Commit

Permalink
Merge pull request #5162 from freakstatic/argo-limit-revert-rationale
Browse files Browse the repository at this point in the history 
Argo Bridge: limit revert_outbound_transfer rationale
  • Loading branch information
@kdembler
kdembler authored Jun 4, 2024
2 parents e193904 + 7ba4ffb commit 03c80bb
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 24 deletions.
40 changes: 40 additions & 0 deletions runtime-modules/argo-bridge/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(|| {
Expand Down
11 changes: 9 additions & 2 deletions runtime-modules/argo-bridge/src/events.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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(),
Expand Down
5 changes: 2 additions & 3 deletions runtime-modules/argo-bridge/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
12 changes: 6 additions & 6 deletions runtime-modules/argo-bridge/src/tests/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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(),
));
});
}
Expand All @@ -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);
});
Expand All @@ -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);
});
Expand All @@ -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);
});
Expand Down
2 changes: 2 additions & 0 deletions runtime-modules/argo-bridge/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
48 changes: 35 additions & 13 deletions runtime-modules/argo-bridge/src/weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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;
Expand All @@ -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))
Expand All @@ -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)
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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))
}
Expand All @@ -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)
}
Expand Down

0 comments on commit 03c80bb

Please sign in to comment.