Skip to content

Commit

Permalink
refactor: move private channel storage into cf-swapping
Browse files Browse the repository at this point in the history
  • Loading branch information
@msgmaxim
msgmaxim committed Nov 5, 2024
1 parent f352aa1 commit d6ced21
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 192 deletions.
48 changes: 7 additions & 41 deletions state-chain/pallets/cf-ingress-egress/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,10 @@ use cf_primitives::{
use cf_runtime_utilities::log_or_panic;
use cf_traits::{
impl_pallet_safe_mode, AccountRoleRegistry, AdjustedFeeEstimationApi, AssetConverter,
AssetWithholding, BalanceApi, BoostApi, Broadcaster, Chainflip, DepositApi, EgressApi,
EpochInfo, FeePayment, FetchesTransfersLimitProvider, GetBlockHeight, IngressEgressFeeApi,
IngressSink, IngressSource, NetworkEnvironmentProvider, OnDeposit, PoolApi,
PrivateChannelManager, ScheduledEgressDetails, SwapLimitsProvider, SwapRequestHandler,
SwapRequestType,
AssetWithholding, BalanceApi, BoostApi, Broadcaster, Chainflip, ChannelIdAllocator, DepositApi,
EgressApi, EpochInfo, FeePayment, FetchesTransfersLimitProvider, GetBlockHeight,
IngressEgressFeeApi, IngressSink, IngressSource, NetworkEnvironmentProvider, OnDeposit,
PoolApi, ScheduledEgressDetails, SwapLimitsProvider, SwapRequestHandler, SwapRequestType,
};
use frame_support::{
pallet_prelude::{OptionQuery, *},
Expand Down Expand Up @@ -590,10 +589,6 @@ pub mod pallet {
pub(crate) type FailedRejections<T: Config<I>, I: 'static = ()> =
StorageValue<_, Vec<TaintedTransactionDetails<T, I>>, ValueQuery>;

#[pallet::storage]
pub(crate) type BrokerPrivateChannels<T: Config<I>, I: 'static = ()> =
StorageMap<_, Identity, T::AccountId, ChannelId, OptionQuery>;

#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config<I>, I: 'static = ()> {
Expand Down Expand Up @@ -790,10 +785,6 @@ pub mod pallet {
UnsupportedChain,
/// Transaction cannot be reported after being pre-witnessed or boosted.
TransactionAlreadyPrewitnessed,
/// Cannot open a private channel for a broker because one already exists.
PrivateChannelExistsForBroker,
/// Cannot close a private channel for a broker because it does not exist.
NoPrivateChannelExistsForBroker,
}

#[pallet::hooks]
Expand Down Expand Up @@ -2503,34 +2494,9 @@ impl<T: Config<I>, I: 'static> EgressApi<T::TargetChain> for Pallet<T, I> {
}
}

/// Private channels are only expected to be used for bitcoin.
impl<T: Config<I>, I: 'static> PrivateChannelManager for Pallet<T, I> {
type AccountId = T::AccountId;

fn open_private_channel(broker_id: &Self::AccountId) -> Result<ChannelId, DispatchError> {
ensure!(
!BrokerPrivateChannels::<T, I>::contains_key(broker_id),
Error::<T, I>::PrivateChannelExistsForBroker
);

// TODO: burn fee for opening a channel?
let next_channel_id = Self::allocate_next_channel_id()?;

BrokerPrivateChannels::<T, I>::insert(broker_id.clone(), next_channel_id);

Ok(next_channel_id)
}

fn close_private_channel(broker_id: &Self::AccountId) -> Result<ChannelId, DispatchError> {
let Some(channel_id) = BrokerPrivateChannels::<T, I>::take(broker_id) else {
return Err(Error::<T, I>::NoPrivateChannelExistsForBroker.into())
};

Ok(channel_id)
}

fn private_channel_lookup(broker_id: &Self::AccountId) -> Option<ChannelId> {
BrokerPrivateChannels::<T, I>::get(broker_id)
impl<T: Config<I>, I: 'static> ChannelIdAllocator for Pallet<T, I> {
fn allocate_private_channel_id() -> Result<ChannelId, DispatchError> {
Ok(Self::allocate_next_channel_id()?)
}
}

Expand Down
104 changes: 26 additions & 78 deletions state-chain/pallets/cf-ingress-egress/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1980,87 +1980,35 @@ fn failed_ccm_deposit_can_deposit_event() {
});
}

mod private_broker_channels {

use super::*;

use cf_traits::PrivateChannelManager;

#[test]
fn opening_new_channel() {
new_test_ext().execute_with(|| {
const REGULAR_CHANNEL_ID_1: u64 = 1;
const PRIVATE_CHANNEL_ID: u64 = 2;
const REGULAR_CHANNEL_ID_2: u64 = 3;

let open_regular_channel_expecting_id = |expected_channel_id: u64| {
let (channel_id, ..) = IngressEgress::open_channel(
&ALICE,
EthAsset::Eth,
ChannelAction::LiquidityProvision { lp_account: 0, refund_address: None },
0,
)
.unwrap();

assert_eq!(channel_id, expected_channel_id);
};

// Open a regular channel first to check that ids of regular
// and private channels do not overlap:
open_regular_channel_expecting_id(REGULAR_CHANNEL_ID_1);

assert_eq!(crate::BrokerPrivateChannels::<Test, ()>::get(BROKER), None);

// Opening a private channel should succeed:
{
assert_eq!(IngressEgress::open_private_channel(&BROKER), Ok(PRIVATE_CHANNEL_ID));

assert_eq!(
crate::BrokerPrivateChannels::<Test, ()>::get(BROKER),
Some(PRIVATE_CHANNEL_ID)
);
assert_eq!(
IngressEgress::private_channel_lookup(&BROKER),
Some(PRIVATE_CHANNEL_ID)
);
}

// Open a regular channel again to check that opening a private channel
// updates the channel id counter:
open_regular_channel_expecting_id(REGULAR_CHANNEL_ID_2);

// The same broker should not be able to open another private channel:
{
assert_noop!(
IngressEgress::open_private_channel(&BROKER),
crate::Error::<Test, ()>::PrivateChannelExistsForBroker
);
}
});
}

#[test]
fn closing_channel() {
new_test_ext().execute_with(|| {
// Channel not opened yet:
assert_noop!(
IngressEgress::close_private_channel(&BROKER),
crate::Error::<Test, ()>::NoPrivateChannelExistsForBroker
);
#[test]
fn private_and_regular_channel_ids_do_not_overlap() {
new_test_ext().execute_with(|| {
const REGULAR_CHANNEL_ID_1: u64 = 1;
const PRIVATE_CHANNEL_ID: u64 = 2;
const REGULAR_CHANNEL_ID_2: u64 = 3;

assert_eq!(IngressEgress::private_channel_lookup(&BROKER), None);
let open_regular_channel_expecting_id = |expected_channel_id: u64| {
let (channel_id, ..) = IngressEgress::open_channel(
&ALICE,
EthAsset::Eth,
ChannelAction::LiquidityProvision { lp_account: 0, refund_address: None },
0,
)
.unwrap();

let channel_id = IngressEgress::open_private_channel(&BROKER)
.expect("should be able to open a private channel");
assert_eq!(channel_id, expected_channel_id);
};

assert!(crate::BrokerPrivateChannels::<Test, ()>::get(BROKER).is_some());
assert_eq!(IngressEgress::private_channel_lookup(&BROKER), Some(channel_id));
// Open a regular channel first to check that ids of regular
// and private channels do not overlap:
open_regular_channel_expecting_id(REGULAR_CHANNEL_ID_1);

// Should succeed now that the channel has been opened:
assert_eq!(IngressEgress::close_private_channel(&BROKER), Ok(channel_id));
// This method is used, for example, by the swapping pallet when requesting
// a channel id for private broker channels:
assert_eq!(IngressEgress::allocate_next_channel_id(), Ok(PRIVATE_CHANNEL_ID));

assert_eq!(crate::BrokerPrivateChannels::<Test, ()>::get(BROKER), None);
assert_eq!(IngressEgress::private_channel_lookup(&BROKER), None);
});
}
// Open a regular channel again to check that opening a private channel
// updates the channel id counter:
open_regular_channel_expecting_id(REGULAR_CHANNEL_ID_2);
});
}
6 changes: 3 additions & 3 deletions state-chain/pallets/cf-swapping/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ mod benchmarks {
}

assert!(
T::PrivateChannelManager::private_channel_lookup(&broker_id).is_some(),
BrokerPrivateBtcChannels::<T>::contains_key(&broker_id),
"Private channel must have been opened"
);
}
Expand All @@ -155,7 +155,7 @@ mod benchmarks {
assert_ok!(Pallet::<T>::open_private_btc_channel(caller.clone()));

assert!(
T::PrivateChannelManager::private_channel_lookup(&broker_id).is_some(),
BrokerPrivateBtcChannels::<T>::contains_key(&broker_id),
"Private channel must have been opened"
);

Expand All @@ -165,7 +165,7 @@ mod benchmarks {
}

assert!(
T::PrivateChannelManager::private_channel_lookup(&broker_id).is_none(),
!BrokerPrivateBtcChannels::<T>::contains_key(&broker_id),
"Private channel must have been closed"
);
}
Expand Down
36 changes: 25 additions & 11 deletions state-chain/pallets/cf-swapping/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use cf_primitives::{
};
use cf_runtime_utilities::log_or_panic;
use cf_traits::{
impl_pallet_safe_mode, BalanceApi, DepositApi, ExecutionCondition, IngressEgressFeeApi,
PrivateChannelManager, SwapLimitsProvider, SwapRequestHandler, SwapRequestType,
impl_pallet_safe_mode, BalanceApi, ChannelIdAllocator, DepositApi, ExecutionCondition,
IngressEgressFeeApi, SwapLimitsProvider, SwapRequestHandler, SwapRequestType,
SwapRequestTypeEncoded, SwapType, SwappingApi,
};
use frame_support::{
Expand Down Expand Up @@ -442,9 +442,7 @@ pub mod pallet {
/// The balance API for interacting with the asset-balance pallet.
type BalanceApi: BalanceApi<AccountId = <Self as frame_system::Config>::AccountId>;

type PrivateChannelManager: PrivateChannelManager<
AccountId = <Self as frame_system::Config>::AccountId,
>;
type ChannelIdAllocator: ChannelIdAllocator;
}

#[pallet::pallet]
Expand Down Expand Up @@ -518,6 +516,10 @@ pub mod pallet {
pub type MinimumChunkSize<T: Config> =
StorageMap<_, Twox64Concat, Asset, AssetAmount, ValueQuery>;

#[pallet::storage]
pub(crate) type BrokerPrivateBtcChannels<T: Config> =
StorageMap<_, Identity, T::AccountId, ChannelId, OptionQuery>;

#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
Expand Down Expand Up @@ -695,10 +697,12 @@ pub mod pallet {
SwapRequestDurationTooLong,
/// Invalid DCA parameters.
InvalidDcaParameters,
/// Broker must close their private channels before deregistering
PrivateChannelNotClosed,
/// The provided Refund address cannot be decoded into ForeignChainAddress.
InvalidRefundAddress,
/// Broker cannot deregister or open a new private channel because one already exists.
PrivateChannelExistsForBroker,
/// Cannot close a private channel for a broker because it does not exist.
NoPrivateChannelExistsForBroker,
}

#[pallet::genesis_config]
Expand Down Expand Up @@ -955,8 +959,8 @@ pub mod pallet {
let account_id = T::AccountRoleRegistry::ensure_broker(who)?;

ensure!(
T::PrivateChannelManager::private_channel_lookup(&account_id).is_none(),
Error::<T>::PrivateChannelNotClosed
!BrokerPrivateBtcChannels::<T>::contains_key(&account_id),
Error::<T>::PrivateChannelExistsForBroker
);

ensure!(
Expand Down Expand Up @@ -1079,7 +1083,15 @@ pub mod pallet {
pub fn open_private_btc_channel(origin: OriginFor<T>) -> DispatchResult {
let broker_id = T::AccountRoleRegistry::ensure_broker(origin)?;

let channel_id = T::PrivateChannelManager::open_private_channel(&broker_id)?;
ensure!(
!BrokerPrivateBtcChannels::<T>::contains_key(&broker_id),
Error::<T>::PrivateChannelExistsForBroker
);

// TODO: burn fee for opening a channel?
let channel_id = T::ChannelIdAllocator::allocate_private_channel_id()?;

BrokerPrivateBtcChannels::<T>::insert(broker_id.clone(), channel_id);

Self::deposit_event(Event::<T>::PrivateBrokerChannelOpened { broker_id, channel_id });

Expand All @@ -1091,7 +1103,9 @@ pub mod pallet {
pub fn close_private_btc_channel(origin: OriginFor<T>) -> DispatchResult {
let broker_id = T::AccountRoleRegistry::ensure_broker(origin)?;

let channel_id = T::PrivateChannelManager::close_private_channel(&broker_id)?;
let Some(channel_id) = BrokerPrivateBtcChannels::<T>::take(&broker_id) else {
return Err(Error::<T>::NoPrivateChannelExistsForBroker.into())
};

Self::deposit_event(Event::<T>::PrivateBrokerChannelClosed { broker_id, channel_id });

Expand Down
47 changes: 8 additions & 39 deletions state-chain/pallets/cf-swapping/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use cf_traits::{
deposit_handler::MockDepositHandler, egress_handler::MockEgressHandler,
ingress_egress_fee_handler::MockIngressEgressFeeHandler,
},
AccountRoleRegistry, PrivateChannelManager, SwappingApi,
AccountRoleRegistry, ChannelIdAllocator, SwappingApi,
};
use frame_support::{derive_impl, pallet_prelude::DispatchError, parameter_types, weights::Weight};
use sp_core::ConstU32;
Expand Down Expand Up @@ -44,7 +44,7 @@ parameter_types! {
pub static Swaps: Vec<(Asset, Asset, AssetAmount)> = vec![];
pub static SwapRate: f64 = DEFAULT_SWAP_RATE as f64;
pub storage Liquidity: BoundedBTreeMap<Asset, AssetAmount, ConstU32<100>> = Default::default();
pub storage PrivateChannels: BoundedBTreeMap<u64, ChannelId, ConstU32<100>> = Default::default();
pub storage NextChannelId: u64 = 0;
}

thread_local! {
Expand Down Expand Up @@ -150,45 +150,14 @@ impl WeightInfo for MockWeightInfo {
pub struct AlwaysValid;
impl CcmValidityCheck for AlwaysValid {}

pub struct MockPrivateChannelManager {}

impl PrivateChannelManager for MockPrivateChannelManager {
type AccountId = u64;

fn open_private_channel(broker_id: &Self::AccountId) -> Result<ChannelId, DispatchError> {
let private_channels = PrivateChannels::get();

// The id assignment isn't quite right (doesn't take deletions into account), but works for
// the purposes of our tests. Future tests can improve this if required.
let next_channel_id = private_channels.len() as u64 + 1;

let private_channels = private_channels
.try_mutate(|liquidity| {
liquidity.insert(*broker_id, next_channel_id);
})
.unwrap();

PrivateChannels::set(&private_channels);
pub struct MockChannelIdAllocator {}

impl ChannelIdAllocator for MockChannelIdAllocator {
fn allocate_private_channel_id() -> Result<ChannelId, DispatchError> {
let next_channel_id = NextChannelId::get();
NextChannelId::set(&(next_channel_id + 1));
Ok(next_channel_id)
}

fn close_private_channel(broker_id: &Self::AccountId) -> Result<ChannelId, DispatchError> {
let mut removed_channel = None;
let private_channels = PrivateChannels::get()
.try_mutate(|liquidity| {
removed_channel = liquidity.remove(broker_id);
})
.unwrap();

PrivateChannels::set(&private_channels);

removed_channel.ok_or(DispatchError::Other("no channel found"))
}

fn private_channel_lookup(broker_id: &Self::AccountId) -> Option<ChannelId> {
PrivateChannels::get().get(broker_id).copied()
}
}

impl pallet_cf_swapping::Config for Test {
Expand All @@ -205,7 +174,7 @@ impl pallet_cf_swapping::Config for Test {
type BalanceApi = MockBalance;
type CcmValidityChecker = AlwaysValid;
type NetworkFee = NetworkFee;
type PrivateChannelManager = MockPrivateChannelManager;
type ChannelIdAllocator = MockChannelIdAllocator;
}

pub const ALICE: <Test as frame_system::Config>::AccountId = 123u64;
Expand Down
Loading

0 comments on commit d6ced21

Please sign in to comment.