feat: Restricted address should override bound restrictions (#4159)

Co-authored-by: Daniel <daniel@chainflip.io>
Co-authored-by: dandanlen <3168260+dandanlen@users.noreply.github.com>

api/bin/chainflip-cli/src/main.rs

@@ -1,5 +1,5 @@
 #![feature(absolute_path)]
-use anyhow::{bail, Context, Result};
+use anyhow::{Context, Result};
 use clap::Parser;
 use futures::FutureExt;
 use serde::Serialize;
@@ -11,7 +11,7 @@ use crate::settings::{
 };
 use api::{
 	lp::LpApi, primitives::RedemptionAmount, queries::QueryApi, AccountId32, BrokerApi,
-	GovernanceApi, KeyPair, OperatorApi, SignedExtrinsicApi, StateChainApi, SwapDepositAddress,
+	GovernanceApi, KeyPair, OperatorApi, StateChainApi, SwapDepositAddress,
 };
 use cf_chains::eth::Address as EthereumAddress;
 use chainflip_api as api;
@@ -140,65 +140,23 @@ async fn run_cli() -> Result<()> {
 async fn request_redemption(
 	api: StateChainApi,
 	amount: Option<f64>,
-	supplied_redeem_address: Option<String>,
+	supplied_redeem_address: String,
 	supplied_executor_address: Option<String>,
 ) -> Result<()> {
-	let account_id = api.state_chain_client.account_id();
-
-	// Check the bound redeem address for this account
-	let supplied_redeem_address = if let Some(address) = supplied_redeem_address {
-		Some(EthereumAddress::from(
-			clean_hex_address::<[u8; 20]>(&address).context("Invalid ETH address supplied")?,
-		))
-	} else {
-		None
-	};
-	let bound_redeem_address =
-		api.query_api().get_bound_redeem_address(None, Some(account_id.clone())).await?;
-
-	let redeem_address = match (supplied_redeem_address, bound_redeem_address) {
-		(Some(supplied_address), Some(bound_address)) =>
-			if supplied_address != bound_address {
-				bail!("Supplied ETH address `{supplied_address:?}` does not match bound address for this account `{bound_address:?}`.");
-			} else {
-				bound_address
-			},
-		(Some(supplied_address), None) => supplied_address,
-		(None, Some(bound_address)) => {
-			println!("Using bound redeem address.");
-			bound_address
-		},
-		(None, None) =>
-			bail!("No redeem address supplied and no bound redeem address found for your account {account_id}."),
-	};
+	// Check validity of the redeem address
+	let redeem_address = EthereumAddress::from(
+		clean_hex_address::<[u8; 20]>(&supplied_redeem_address)
+			.context("Invalid redeem address")?,
+	);
 
-	// Check the bound executor address for this account
-	let supplied_executor_address = if let Some(address) = supplied_executor_address {
+	// Check the validity of the executor address
+	let executor_address = if let Some(address) = supplied_executor_address {
 		Some(EthereumAddress::from(
-			clean_hex_address::<[u8; 20]>(&address).context("Invalid ETH address supplied")?,
+			clean_hex_address::<[u8; 20]>(&address).context("Invalid executor address")?,
 		))
 	} else {
 		None
 	};
-	let bound_executor_address = api
-		.query_api()
-		.get_bound_executor_address(None, Some(account_id.clone()))
-		.await?;
-
-	let executor_address = match (bound_executor_address, supplied_executor_address) {
-		(Some(bound_address), Some(supplied_address)) =>
-			if bound_address != supplied_address {
-				bail!("Supplied executor address `{supplied_address:?}` does not match bound address for this account `{bound_address:?}`.");
-			} else {
-				Some(supplied_address)
-			},
-		(Some(bound_address), None) => {
-			println!("Using bound executor address {bound_address}.");
-			Some(bound_address)
-		},
-		(None, Some(executor)) => Some(executor),
-		(None, None) => None,
-	};
 
 	// Calculate the redemption amount
 	let amount = match amount {

api/bin/chainflip-cli/src/settings.rs

@@ -104,10 +104,8 @@ pub enum CliCommand {
 			long = "exact"
 		)]
 		amount: Option<f64>,
-		#[clap(
-			help = "The Ethereum address you wish to redeem your FLIP to. If not specified, the redeem address bound to your account will be used"
-		)]
-		eth_address: Option<String>,
+		#[clap(help = "The Ethereum address you wish to redeem your FLIP to.")]
+		eth_address: String,
 		#[clap(
 			help = "Optional executor address. If specified, only this address will be able to execute the redemption."
 		)]

api/lib/src/queries.rs

@@ -1,6 +1,6 @@
 use super::*;
 use cf_chains::{address::ToHumanreadableAddress, Chain};
-use cf_primitives::{chains::assets::any, AssetAmount};
+use cf_primitives::{chains::assets::any, AssetAmount, FlipBalance};
 use chainflip_engine::state_chain_observer::client::{
 	chain_api::ChainApi, storage_api::StorageApi,
 };
@@ -155,6 +155,24 @@ impl QueryApi {
 			.await?)
 	}
 
+	pub async fn get_restricted_balances(
+		&self,
+		block_hash: Option<state_chain_runtime::Hash>,
+		account_id: Option<state_chain_runtime::AccountId>,
+	) -> Result<BTreeMap<EthereumAddress, FlipBalance>> {
+		let block_hash =
+			block_hash.unwrap_or_else(|| self.state_chain_client.latest_finalized_hash());
+		let account_id = account_id.unwrap_or_else(|| self.state_chain_client.account_id());
+
+		Ok(self
+			.state_chain_client
+			.storage_map_entry::<pallet_cf_funding::RestrictedBalances<state_chain_runtime::Runtime>>(
+				block_hash,
+				&account_id,
+			)
+			.await?)
+	}
+
 	pub async fn pre_update_check(
 		&self,
 		block_hash: Option<state_chain_runtime::Hash>,

state-chain/pallets/cf-funding/src/lib.rs

@@ -361,11 +361,6 @@ pub mod pallet {
 		) -> DispatchResultWithPostInfo {
 			let account_id = ensure_signed(origin)?;
 
-			if let Some(executor_addr) = BoundExecutorAddress::<T>::get(&account_id) {
-				let executor = executor.ok_or(Error::<T>::ExecutorBindingRestrictionViolated)?;
-				ensure!(executor_addr == executor, 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
@@ -380,12 +375,22 @@ pub mod pallet {
 			);
 
 			let mut restricted_balances = RestrictedBalances::<T>::get(&account_id);
+
+			// Ignore executor binding restrictions for withdrawals of restricted funds.
+			if !restricted_balances.contains_key(&address) {
+				if let Some(bound_executor) = BoundExecutorAddress::<T>::get(&account_id) {
+					ensure!(
+						executor == Some(bound_executor),
+						Error::<T>::ExecutorBindingRestrictionViolated
+					);
+				}
+			}
+
 			let redemption_fee = RedemptionTax::<T>::get();
 
 			if let Some(bound_address) = BoundRedeemAddress::<T>::get(&account_id) {
 				ensure!(
-					bound_address == address ||
-						restricted_balances.keys().any(|res_address| res_address == &address),
+					bound_address == address || restricted_balances.contains_key(&address),
 					Error::<T>::AccountBindingRestrictionViolated
 				);
 			}

state-chain/pallets/cf-funding/src/tests.rs

@@ -946,6 +946,58 @@ fn redeem_funds_until_restricted_balance_is_zero_and_then_redeem_to_redeem_addre
 	});
 }
 
+#[test]
+fn redeem_funds_to_restricted_address_overrides_bound_and_executor_restrictions() {
+	new_test_ext().execute_with(|| {
+		const RESTRICTED_ADDRESS: EthereumAddress = H160([0x01; 20]);
+		const REDEEM_ADDRESS: EthereumAddress = H160([0x02; 20]);
+		const EXECUTOR_ADDRESS: EthereumAddress = H160([0x04; 20]);
+		const RANDOM_ADDRESS: EthereumAddress = H160([0x12; 20]);
+		const AMOUNT: u128 = 100;
+		RestrictedAddresses::<Test>::insert(RESTRICTED_ADDRESS, ());
+		BoundRedeemAddress::<Test>::insert(ALICE, REDEEM_ADDRESS);
+		BoundExecutorAddress::<Test>::insert(ALICE, EXECUTOR_ADDRESS);
+
+		assert_ok!(Funding::funded(RuntimeOrigin::root(), ALICE, AMOUNT, REDEEM_ADDRESS, TX_HASH));
+		assert_ok!(Funding::funded(RuntimeOrigin::root(), ALICE, AMOUNT, REDEEM_ADDRESS, TX_HASH));
+		assert_ok!(Funding::funded(
+			RuntimeOrigin::root(),
+			ALICE,
+			AMOUNT,
+			RESTRICTED_ADDRESS,
+			TX_HASH
+		));
+
+		// Redeem using a wrong executor should fail because we have bounded executor address
+		assert_noop!(
+			Funding::redeem(
+				RuntimeOrigin::signed(ALICE),
+				(AMOUNT).into(),
+				REDEEM_ADDRESS,
+				Default::default()
+			),
+			Error::<Test>::ExecutorBindingRestrictionViolated
+		);
+		// Redeem using correct redeem and executor should complete succesfully
+		assert_ok!(Funding::redeem(
+			RuntimeOrigin::signed(ALICE),
+			(AMOUNT).into(),
+			REDEEM_ADDRESS,
+			Some(EXECUTOR_ADDRESS)
+		));
+		assert_ok!(Funding::redeemed(RuntimeOrigin::root(), ALICE, AMOUNT, TX_HASH));
+		// Redeem using restricted address should complete even with wrong executor and bound redeem
+		// address
+		assert_ok!(Funding::redeem(
+			RuntimeOrigin::signed(ALICE),
+			(AMOUNT).into(),
+			RESTRICTED_ADDRESS,
+			Some(RANDOM_ADDRESS)
+		));
+		assert_ok!(Funding::redeemed(RuntimeOrigin::root(), ALICE, AMOUNT, TX_HASH));
+	});
+}
+
 #[cfg(test)]
 mod test_restricted_balances {
 	use sp_core::H160;