Skip to content

Commit

Permalink
feat: [PRO-823] bind-nodes-executor-to-address (#3987)
Browse files Browse the repository at this point in the history
* feat: ✨ added benchmarks

* fix: 🐛 fixed clippy

* test: 🧪 added tests

* refactor: ♻️ renaming

* docs: 📝 updated comment

---------

Co-authored-by: dandanlen <3168260+dandanlen@users.noreply.github.com>
  • Loading branch information
Janislav and dandanlen committed Sep 18, 2023
1 parent 181de89 commit c82c712
Show file tree
Hide file tree
Showing 6 changed files with 719 additions and 327 deletions.
2 changes: 1 addition & 1 deletion api/lib/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl QueryApi {

Ok(self
.state_chain_client
.storage_map_entry::<pallet_cf_funding::BoundAddress<state_chain_runtime::Runtime>>(
.storage_map_entry::<pallet_cf_funding::BoundRedeemAddress<state_chain_runtime::Runtime>>(
block_hash,
&account_id,
)
Expand Down
9 changes: 8 additions & 1 deletion state-chain/pallets/cf-funding/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ benchmarks! {
let caller: T::AccountId = whitelisted_caller();
}:_(RawOrigin::Signed(caller.clone()), Default::default())
verify {
assert!(BoundAddress::<T>::contains_key(&caller));
assert!(BoundRedeemAddress::<T>::contains_key(&caller));
}

update_restricted_addresses {
Expand All @@ -204,5 +204,12 @@ benchmarks! {
let _ = call.dispatch_bypass_filter(T::EnsureGovernance::try_successful_origin().unwrap());
}

bind_executor_address {
let caller: T::AccountId = whitelisted_caller();
}:_(RawOrigin::Signed(caller.clone()), Default::default())
verify {
assert!(BoundExecutorAddress::<T>::contains_key(&caller));
}

impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test,);
}
63 changes: 58 additions & 5 deletions state-chain/pallets/cf-funding/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ pub mod pallet {
#[pallet::storage]
pub type RedemptionTTLSeconds<T: Config> = StorageValue<_, u64, ValueQuery>;

/// Registered addresses for an executor.
#[pallet::storage]
pub type BoundExecutorAddress<T: Config> =
StorageMap<_, Blake2_128Concat, AccountId<T>, EthereumAddress, OptionQuery>;

/// List of restricted addresses
#[pallet::storage]
pub type RestrictedAddresses<T: Config> =
Expand All @@ -151,7 +156,7 @@ pub mod pallet {

/// Map of bound addresses for accounts.
#[pallet::storage]
pub type BoundAddress<T: Config> =
pub type BoundRedeemAddress<T: Config> =
StorageMap<_, Blake2_128Concat, AccountId<T>, EthereumAddress>;

/// The fee levied for every redemption request. Can be updated by Governance.
Expand Down Expand Up @@ -233,6 +238,9 @@ pub mod pallet {

/// An account has been bound to an address.
BoundRedeemAddress { account_id: AccountId<T>, address: EthereumAddress },

/// An account has been bound to an executor address.
BoundExecutorAddress { account_id: AccountId<T>, address: EthereumAddress },
}

#[pallet::error]
Expand Down Expand Up @@ -288,6 +296,12 @@ pub mod pallet {

/// Stop Bidding is disabled due to Safe Mode.
StopBiddingDisabled,

/// The executor for this account is bound to another address.
ExecutorBindingRestrictionViolated,

/// The account is already bound to an executor address.
ExecutorAddressAlreadyBound,
}

#[pallet::call]
Expand Down Expand Up @@ -354,6 +368,13 @@ pub mod pallet {
) -> DispatchResultWithPostInfo {
let account_id = ensure_signed(origin)?;

if let Some(executor_addr) = BoundExecutorAddress::<T>::get(&account_id) {
ensure!(
executor_addr == executor.unwrap_or_default(),
Error::<T>::ExecutorBindingRestrictionViolated
);
}

ensure!(T::SafeMode::get().redeem_enabled, Error::<T>::RedeemDisabled);

// Not allowed to redeem if we are an active bidder in the auction phase
Expand All @@ -370,7 +391,7 @@ pub mod pallet {
let mut restricted_balances = RestrictedBalances::<T>::get(&account_id);
let redemption_fee = RedemptionTax::<T>::get();

if let Some(bound_address) = BoundAddress::<T>::get(&account_id) {
if let Some(bound_address) = BoundRedeemAddress::<T>::get(&account_id) {
ensure!(
bound_address == address ||
restricted_balances.keys().any(|res_address| res_address == &address),
Expand Down Expand Up @@ -612,7 +633,7 @@ pub mod pallet {
///
/// - [BadOrigin](frame_support::error::BadOrigin)
#[pallet::call_index(7)]
#[pallet::weight(T::WeightInfo::update_restricted_addresses(addresses_to_add.len() as u32, addresses_to_remove.len() as u32))]
#[pallet::weight(T::WeightInfo::update_restricted_addresses(addresses_to_add.len() as u32, addresses_to_remove.len() as u32, 10_u32))]
pub fn update_restricted_addresses(
origin: OriginFor<T>,
addresses_to_add: Vec<EthereumAddress>,
Expand Down Expand Up @@ -651,8 +672,11 @@ pub mod pallet {
address: EthereumAddress,
) -> DispatchResultWithPostInfo {
let account_id = ensure_signed(origin)?;
ensure!(!BoundAddress::<T>::contains_key(&account_id), Error::<T>::AccountAlreadyBound);
BoundAddress::<T>::insert(&account_id, address);
ensure!(
!BoundRedeemAddress::<T>::contains_key(&account_id),
Error::<T>::AccountAlreadyBound
);
BoundRedeemAddress::<T>::insert(&account_id, address);
Self::deposit_event(Event::BoundRedeemAddress { account_id, address });
Ok(().into())
}
Expand All @@ -673,6 +697,35 @@ pub mod pallet {
Self::deposit_event(Event::<T>::RedemptionTaxAmountUpdated { amount });
Ok(())
}

/// Binds executor address to an account.
///
/// ## Events
///
/// - [BoundExecutorAddress](Event::BoundExecutorAddress)
///
/// ## Errors
///
/// - [ExecutorAddressAlreadyBound](Error::ExecutorAddressAlreadyBound)
/// - [BadOrigin](frame_support::error::BadOrigin)
#[pallet::call_index(10)]
#[pallet::weight(T::WeightInfo::bind_executor_address())]
pub fn bind_executor_address(
origin: OriginFor<T>,
executor_address: EthereumAddress,
) -> DispatchResultWithPostInfo {
let account_id = ensure_signed(origin)?;
ensure!(
!BoundExecutorAddress::<T>::contains_key(&account_id),
Error::<T>::ExecutorAddressAlreadyBound,
);
BoundExecutorAddress::<T>::insert(account_id.clone(), executor_address);
Self::deposit_event(Event::BoundExecutorAddress {
account_id,
address: executor_address,
});
Ok(().into())
}
}

#[pallet::genesis_config]
Expand Down
53 changes: 46 additions & 7 deletions state-chain/pallets/cf-funding/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
mock::*, pallet, ActiveBidder, Error, EthereumAddress, PendingRedemptions, RedemptionAmount,
RedemptionTax, RestrictedAddresses, RestrictedBalances,
mock::*, pallet, ActiveBidder, BoundExecutorAddress, Error, EthereumAddress,
PendingRedemptions, RedemptionAmount, RedemptionTax, RestrictedAddresses, RestrictedBalances,
};
use cf_primitives::FlipBalance;
use cf_test_utilities::assert_event_sequence;
Expand All @@ -10,7 +10,7 @@ use cf_traits::{
};
use sp_core::H160;

use crate::BoundAddress;
use crate::BoundRedeemAddress;
use frame_support::{assert_noop, assert_ok};
use pallet_cf_flip::Bonder;
use sp_runtime::{traits::BadOrigin, DispatchError};
Expand Down Expand Up @@ -862,7 +862,7 @@ fn can_only_redeem_funds_to_bound_address() {
const UNRESTRICTED_ADDRESS: EthereumAddress = H160([0x03; 20]);
const AMOUNT: u128 = 100;
RestrictedAddresses::<Test>::insert(RESTRICTED_ADDRESS_1, ());
BoundAddress::<Test>::insert(ALICE, BOUND_ADDRESS);
BoundRedeemAddress::<Test>::insert(ALICE, BOUND_ADDRESS);
assert_ok!(Funding::funded(
RuntimeOrigin::root(),
ALICE,
Expand Down Expand Up @@ -903,7 +903,7 @@ fn redeem_funds_until_restricted_balance_is_zero_and_then_redeem_to_redeem_addre
const UNRESTRICTED_ADDRESS: EthereumAddress = H160([0x03; 20]);
const AMOUNT: u128 = 100;
RestrictedAddresses::<Test>::insert(RESTRICTED_ADDRESS, ());
BoundAddress::<Test>::insert(ALICE, REDEEM_ADDRESS);
BoundRedeemAddress::<Test>::insert(ALICE, REDEEM_ADDRESS);
assert_ok!(Funding::funded(
RuntimeOrigin::root(),
ALICE,
Expand Down Expand Up @@ -1277,8 +1277,8 @@ fn can_bind_redeem_address() {
address,
}) if address == REDEEM_ADDRESS,
);
assert!(BoundAddress::<Test>::contains_key(ALICE));
assert_eq!(BoundAddress::<Test>::get(ALICE).unwrap(), REDEEM_ADDRESS);
assert!(BoundRedeemAddress::<Test>::contains_key(ALICE));
assert_eq!(BoundRedeemAddress::<Test>::get(ALICE).unwrap(), REDEEM_ADDRESS);
});
}

Expand Down Expand Up @@ -1467,3 +1467,42 @@ fn check_restricted_balances_are_getting_removed() {
assert!(RestrictedBalances::<Test>::get(ALICE).get(&RESTRICTED_ADDRESS).is_none());
});
}

#[test]
fn bind_executor_address() {
new_test_ext().execute_with(|| {
const EXECUTOR_ADDRESS: EthereumAddress = H160([0x01; 20]);
assert_ok!(Funding::bind_executor_address(RuntimeOrigin::signed(ALICE), EXECUTOR_ADDRESS));
assert_event_sequence!(
Test,
RuntimeEvent::Funding(crate::Event::BoundExecutorAddress {
account_id: ALICE,
address,
}) if address == EXECUTOR_ADDRESS,
);
assert!(BoundExecutorAddress::<Test>::contains_key(ALICE));
assert_eq!(BoundExecutorAddress::<Test>::get(ALICE).unwrap(), EXECUTOR_ADDRESS);
assert_noop!(
Funding::bind_executor_address(RuntimeOrigin::signed(ALICE), EXECUTOR_ADDRESS),
Error::<Test>::ExecutorAddressAlreadyBound
);
});
}

#[test]
fn detect_wrong_executor_address() {
new_test_ext().execute_with(|| {
const EXECUTOR_ADDRESS: EthereumAddress = H160([0x01; 20]);
const WRONG_EXECUTOR_ADDRESS: EthereumAddress = H160([0x02; 20]);
assert_ok!(Funding::bind_executor_address(RuntimeOrigin::signed(ALICE), EXECUTOR_ADDRESS));
assert_noop!(
Funding::redeem(
RuntimeOrigin::signed(ALICE),
100.into(),
ETH_DUMMY_ADDR,
Some(WRONG_EXECUTOR_ADDRESS)
),
Error::<Test>::ExecutorBindingRestrictionViolated
);
});
}
Loading

0 comments on commit c82c712

Please sign in to comment.