Skip to content

Commit

Permalink
take the protocol fee when the channel being closed is in init state
Browse files Browse the repository at this point in the history
  • Loading branch information
@vedhavyas
vedhavyas committed Jun 6, 2024
1 parent bb7e1f6 commit d18a82f
Show file tree
Hide file tree
Showing 2 changed files with 150 additions and 34 deletions.
62 changes: 43 additions & 19 deletions domains/pallets/messenger/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ mod pallet {
use sp_messenger::{
InherentError, InherentType, OnXDMRewards, StorageKeys, INHERENT_IDENTIFIER,
};
use sp_runtime::{ArithmeticError, Perbill};
use sp_runtime::{ArithmeticError, Perbill, Saturating};
use sp_subspace_mmr::MmrProofVerifier;
#[cfg(feature = "std")]
use std::collections::BTreeSet;
Expand Down Expand Up @@ -566,7 +566,8 @@ mod pallet {
let owner = ensure_signed(origin)?;

// initiate the channel config
let channel_id = Self::do_init_channel(dst_chain_id, params, Some(owner.clone()))?;
let channel_id =
Self::do_init_channel(dst_chain_id, params, Some(owner.clone()), true)?;

// reserve channel open fees
let hold_id = T::HoldIdentifier::messenger_channel(dst_chain_id, channel_id);
Expand Down Expand Up @@ -848,7 +849,7 @@ mod pallet {
max_outgoing_messages: 100,
fee_model,
};
let channel_id = Self::do_init_channel(dst_chain_id, init_params, None)?;
let channel_id = Self::do_init_channel(dst_chain_id, init_params, None, true)?;
Self::do_open_channel(dst_chain_id, channel_id)?;
Ok(())
}
Expand Down Expand Up @@ -928,7 +929,7 @@ mod pallet {
let channel = maybe_channel.as_mut().ok_or(Error::<T>::MissingChannel)?;

ensure!(
channel.state == ChannelState::Open,
channel.state != ChannelState::Closed,
Error::<T>::InvalidChannelState
);

Expand All @@ -939,7 +940,24 @@ mod pallet {
if let Some(owner) = &channel.maybe_owner {
let hold_id = T::HoldIdentifier::messenger_channel(chain_id, channel_id);
let locked_amount = T::Currency::balance_on_hold(&hold_id, owner);
T::Currency::release(&hold_id, owner, locked_amount, Precision::Exact)
let amount_to_release = {
if channel.state == ChannelState::Open {
locked_amount
} else {
let protocol_fee = T::ChannelInitReservePortion::get() * locked_amount;
let release_amount = locked_amount.saturating_sub(protocol_fee);
T::Currency::burn_held(
&hold_id,
owner,
protocol_fee,
Precision::Exact,
Fortitude::Force,
)?;
T::OnXDMRewards::on_xdm_rewards(protocol_fee);
release_amount
}
};
T::Currency::release(&hold_id, owner, amount_to_release, Precision::Exact)
.map_err(|_| Error::<T>::BalanceUnlock)?;
}

Expand All @@ -959,17 +977,20 @@ mod pallet {
dst_chain_id: ChainId,
init_params: InitiateChannelParams<BalanceOf<T>>,
maybe_owner: Option<T::AccountId>,
check_allowlist: bool,
) -> Result<ChannelId, DispatchError> {
ensure!(
T::SelfChainId::get() != dst_chain_id,
Error::<T>::InvalidChain,
);

let chain_allowlist = ChainAllowlist::<T>::get();
ensure!(
chain_allowlist.contains(&dst_chain_id),
Error::<T>::ChainNotAllowed
);
if check_allowlist {
let chain_allowlist = ChainAllowlist::<T>::get();
ensure!(
chain_allowlist.contains(&dst_chain_id),
Error::<T>::ChainNotAllowed
);
}

let channel_id = NextChannelId::<T>::get(dst_chain_id);
let next_channel_id = channel_id
Expand Down Expand Up @@ -1073,15 +1094,18 @@ mod pallet {
{
// channel is being opened without an owner since this is a relay message
// from other chain
Self::do_init_channel(msg.src_chain_id, params, None).map_err(|err| {
log::error!(
"Error initiating channel: {:?} with chain: {:?}: {:?}",
msg.channel_id,
msg.src_chain_id,
err
);
InvalidTransaction::Call
})?;
// we do not check the allowlist to finish the end to end flow
Self::do_init_channel(msg.src_chain_id, params, None, false).map_err(
|err| {
log::error!(
"Error initiating channel: {:?} with chain: {:?}: {:?}",
msg.channel_id,
msg.src_chain_id,
err
);
InvalidTransaction::Call
},
)?;
} else {
log::error!("Unexpected call instead of channel open request: {:?}", msg,);
return Err(InvalidTransaction::Call.into());
Expand Down
122 changes: 107 additions & 15 deletions domains/pallets/messenger/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
OutboxResponses, Pallet, U256,
};
use frame_support::traits::fungible::Inspect;
use frame_support::traits::tokens::{Fortitude, Preservation};
use frame_support::{assert_err, assert_ok};
use pallet_transporter::Location;
use sp_core::storage::StorageKey;
Expand Down Expand Up @@ -160,19 +161,6 @@ fn test_close_missing_channel() {
});
}

#[test]
fn test_close_not_open_channel() {
new_chain_a_ext().execute_with(|| {
let chain_id = 2.into();
let channel_id = U256::zero();
create_channel(chain_id, channel_id, Default::default());
assert_err!(
Messenger::close_channel(RuntimeOrigin::root(), chain_id, channel_id,),
Error::<Runtime>::InvalidChannelState
);
});
}

#[test]
fn test_close_open_channel() {
new_chain_a_ext().execute_with(|| {
Expand Down Expand Up @@ -286,6 +274,7 @@ fn open_channel_between_chains(
true,
MessageWeightTag::ProtocolChannelOpen,
None,
true,
);

// check channel state be open on chain_b
Expand Down Expand Up @@ -372,6 +361,7 @@ fn send_message_between_chains(
false,
Default::default(),
Some(Endpoint::Id(0)),
true,
);

// check state on chain_b
Expand Down Expand Up @@ -422,6 +412,7 @@ fn close_channel_between_chains(
true,
MessageWeightTag::ProtocolChannelClose,
None,
true,
);

// check channel state be close on chain_b
Expand Down Expand Up @@ -483,6 +474,7 @@ fn force_toggle_channel_state<Runtime: crate::Config>(
dst_chain_id: ChainId,
channel_id: ChannelId,
toggle: bool,
add_to_allow_list: bool,
) {
let fee_model = FeeModel {
relay_fee: Default::default(),
Expand All @@ -494,8 +486,11 @@ fn force_toggle_channel_state<Runtime: crate::Config>(

let channel = Pallet::<Runtime>::channels(dst_chain_id, channel_id).unwrap_or_else(|| {
let list = BTreeSet::from([dst_chain_id]);
ChainAllowlist::<Runtime>::put(list);
Pallet::<Runtime>::do_init_channel(dst_chain_id, init_params, None).unwrap();
if add_to_allow_list {
ChainAllowlist::<Runtime>::put(list);
}
Pallet::<Runtime>::do_init_channel(dst_chain_id, init_params, None, add_to_allow_list)
.unwrap();
Pallet::<Runtime>::channels(dst_chain_id, channel_id).unwrap()
});

Expand All @@ -521,6 +516,7 @@ fn channel_relay_request_and_response(
toggle_channel_state: bool,
weight_tag: MessageWeightTag,
maybe_endpoint: Option<Endpoint>,
add_to_allowlist: bool,
) {
let chain_a_id = chain_a::SelfChainId::get();
let chain_b_id = chain_b::SelfChainId::get();
Expand Down Expand Up @@ -555,6 +551,7 @@ fn channel_relay_request_and_response(
chain_a_id,
channel_id,
toggle_channel_state,
add_to_allowlist,
);
Inbox::<chain_b::Runtime>::set(Some(msg));

Expand Down Expand Up @@ -611,6 +608,7 @@ fn channel_relay_request_and_response(
chain_b_id,
channel_id,
toggle_channel_state,
true,
);
OutboxResponses::<chain_a::Runtime>::set(Some(msg));

Expand Down Expand Up @@ -666,6 +664,97 @@ fn test_close_channel_between_chains() {
close_channel_between_chains(&mut chain_a_test_ext, &mut chain_b_test_ext, channel_id)
}

#[test]
fn close_init_channels_between_chains() {
let mut chain_a_test_ext = chain_a::new_test_ext();
let mut chain_b_test_ext = chain_b::new_test_ext();

let chain_a_id = chain_a::SelfChainId::get();
let chain_b_id = chain_b::SelfChainId::get();

let fee_model = FeeModel {
relay_fee: Default::default(),
};

let pre_user_account_balance = chain_a_test_ext.execute_with(|| {
<chain_a::Balances as Inspect<BalanceOf<chain_a::Runtime>>>::reducible_balance(
&USER_ACCOUNT,
Preservation::Protect,
Fortitude::Polite,
)
});

// initiate channel open on chain_a
let channel_id = chain_a_test_ext.execute_with(|| -> ChannelId {
let channel_id = U256::zero();
create_channel(chain_b_id, channel_id, fee_model);
channel_id
});

chain_a_test_ext.execute_with(|| {
let channel = Channels::<chain_a::Runtime>::get(chain_b_id, channel_id).unwrap();
assert_eq!(channel.state, ChannelState::Initiated)
});

let post_channel_init_balance = chain_a_test_ext.execute_with(|| {
<chain_a::Balances as Inspect<BalanceOf<chain_a::Runtime>>>::reducible_balance(
&USER_ACCOUNT,
Preservation::Protect,
Fortitude::Polite,
)
});

assert_eq!(
post_channel_init_balance,
pre_user_account_balance - chain_a::ChannelReserveFee::get()
);

channel_relay_request_and_response(
&mut chain_a_test_ext,
&mut chain_b_test_ext,
channel_id,
Nonce::zero(),
false,
MessageWeightTag::ProtocolChannelOpen,
None,
false,
);

chain_a_test_ext.execute_with(|| {
let channel = Channels::<chain_a::Runtime>::get(chain_b_id, channel_id).unwrap();
assert_eq!(channel.state, ChannelState::Initiated)
});

chain_b_test_ext.execute_with(|| {
let channel = Channels::<chain_b::Runtime>::get(chain_a_id, channel_id).unwrap();
assert_eq!(channel.state, ChannelState::Initiated)
});

// close channel
chain_a_test_ext.execute_with(|| close_channel(chain_b_id, channel_id, Some(Nonce::zero())));

chain_a_test_ext.execute_with(|| {
let channel = Channels::<chain_a::Runtime>::get(chain_b_id, channel_id).unwrap();
assert_eq!(channel.state, ChannelState::Closed)
});

let post_channel_close_balance = chain_a_test_ext.execute_with(|| {
<chain_a::Balances as Inspect<BalanceOf<chain_a::Runtime>>>::reducible_balance(
&USER_ACCOUNT,
Preservation::Protect,
Fortitude::Polite,
)
});

// user will only get 80% of reserve since 20% is taken by the protocol
let protocol_fee =
chain_a::ChannelInitReservePortion::get() * chain_a::ChannelReserveFee::get();
assert_eq!(
post_channel_close_balance,
pre_user_account_balance - protocol_fee
);
}

#[test]
fn test_update_consensus_channel_allowlist() {
let mut consensus_chain_test_ext = consensus_chain::new_test_ext();
Expand Down Expand Up @@ -860,6 +949,7 @@ fn test_transport_funds_between_chains() {
false,
Default::default(),
Some(Endpoint::Id(100)),
true,
);

// post check
Expand Down Expand Up @@ -894,6 +984,7 @@ fn test_transport_funds_between_chains_if_src_chain_disallows_after_message_is_s
false,
Default::default(),
Some(Endpoint::Id(100)),
true,
);

// post check should be successful since the chain_a already initiated the
Expand Down Expand Up @@ -940,6 +1031,7 @@ fn test_transport_funds_between_chains_if_dst_chain_disallows_after_message_is_s
false,
Default::default(),
Some(Endpoint::Id(100)),
true,
);

// post check should be not be successful since the chain_b rejected the transfer
Expand Down

0 comments on commit d18a82f

Please sign in to comment.