Refactor: Unify DomainAddress and account conversions (r2) (#1969)

* runtime-common compiling

* runtimes compiling

* add IT required changes

* add id_eth

* rename id_eth to in_eth

* fix lp UTs, still pending the inbounds

* finish lp UTs

* fix IT last tests

* fix clippy and benchmarking

* fix issues after rebasing

* fix IT

* base refactor

* fix ids

* fix weird compilation issue

* fix clippy

* minor change

libs/types/src/domain_address.rs

@@ -10,87 +10,96 @@
 // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 // GNU General Public License for more details.
 
-use cfg_utils::vec_to_fixed_array;
 use frame_support::pallet_prelude::RuntimeDebug;
 use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
 use scale_info::TypeInfo;
-use sp_runtime::traits::AccountIdConversion;
+use sp_core::{crypto::AccountId32, H160};
+use sp_runtime::{traits::AccountIdConversion, TypeId};
 
 use crate::EVMChainId;
 
+const MAX_ADDRESS_SIZE: usize = 32;
+
+/// By just clamping the value to a smaller address
+pub fn account_to_eth_address(address: AccountId32) -> H160 {
+	let bytes: [u8; 32] = address.into();
+	H160::from(
+		*(bytes)
+			.split_first_chunk::<20>()
+			.expect("always fit, qed")
+			.0,
+	)
+}
+
+/// By adding chain information to the new added bytes
+pub fn eth_address_to_account(chain_id: u64, address: H160) -> AccountId32 {
+	// We use a custom encoding here rather than relying on
+	// `AccountIdConversion` for a couple of reasons:
+	// 1. We have very few bytes to spare, so choosing our own fields is nice
+	// 2. AccountIdConversion puts the tag first, which can unbalance the storage
+	//    trees if users create many H160-derived accounts. We put the tag last
+	//    here.
+	let tag = b"EVM";
+	let mut bytes = [0; 32];
+	bytes[0..20].copy_from_slice(&address.0);
+	bytes[20..28].copy_from_slice(&chain_id.to_be_bytes());
+	bytes[28..31].copy_from_slice(tag);
+	AccountId32::new(bytes)
+}
+
 /// A Domain is a chain or network we can send a message to.
-#[derive(Encode, Decode, Clone, Eq, MaxEncodedLen, PartialEq, RuntimeDebug, TypeInfo)]
+#[derive(Encode, Decode, Clone, Copy, Eq, MaxEncodedLen, PartialEq, RuntimeDebug, TypeInfo)]
 pub enum Domain {
-	/// Referring to the Centrifuge Parachain. Will be used for handling
-	/// incoming messages.
-	///
-	/// NOTE: messages CAN NOT be sent directly from the Centrifuge chain to the
-	/// Centrifuge chain itself.
+	/// Referring to the Centrifuge Chain.
 	Centrifuge,
 	/// An EVM domain, identified by its EVM Chain Id
-	EVM(EVMChainId),
+	Evm(EVMChainId),
+}
+
+impl TypeId for Domain {
+	const TYPE_ID: [u8; 4] = crate::ids::DOMAIN_ID;
 }
 
 impl Domain {
 	pub fn into_account<AccountId: Encode + Decode>(&self) -> AccountId {
-		DomainLocator {
-			domain: self.clone(),
-		}
-		.into_account_truncating()
+		self.into_account_truncating()
 	}
 }
 
-#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
-pub struct DomainLocator<Domain> {
-	pub domain: Domain,
-}
-
 #[derive(Encode, Decode, Clone, Eq, MaxEncodedLen, PartialEq, RuntimeDebug, TypeInfo)]
 pub enum DomainAddress {
-	/// A Centrifuge-Chain based account address, 32-bytes long
-	Centrifuge([u8; 32]),
-	/// An EVM chain address, 20-bytes long
-	EVM(EVMChainId, [u8; 20]),
-}
-
-impl Default for DomainAddress {
-	fn default() -> Self {
-		DomainAddress::Centrifuge([0; 32])
-	}
+	/// A centrifuge based account
+	Centrifuge(AccountId32),
+	/// An EVM chain address
+	Evm(EVMChainId, H160),
 }
 
-impl DomainAddress {
-	pub fn evm(chain_id: EVMChainId, address: [u8; 20]) -> Self {
-		Self::EVM(chain_id, address)
-	}
-
-	pub fn centrifuge(address: [u8; 32]) -> Self {
-		Self::Centrifuge(address)
-	}
+impl TypeId for DomainAddress {
+	const TYPE_ID: [u8; 4] = crate::ids::DOMAIN_ADDRESS_ID;
 }
 
-impl From<(EVMChainId, [u8; 20])> for DomainAddress {
-	fn from((chain_id, address): (EVMChainId, [u8; 20])) -> Self {
-		Self::evm(chain_id, address)
+impl Default for DomainAddress {
+	fn default() -> Self {
+		DomainAddress::Centrifuge(AccountId32::new([0; 32]))
 	}
 }
 
 impl From<DomainAddress> for Domain {
 	fn from(x: DomainAddress) -> Self {
 		match x {
 			DomainAddress::Centrifuge(_) => Domain::Centrifuge,
-			DomainAddress::EVM(chain_id, _) => Domain::EVM(chain_id),
+			DomainAddress::Evm(chain_id, _) => Domain::Evm(chain_id),
 		}
 	}
 }
 
 impl DomainAddress {
-	/// Get the address in a 32-byte long representation.
-	/// For EVM addresses, append 12 zeros.
-	pub fn address(&self) -> [u8; 32] {
-		match self.clone() {
-			Self::Centrifuge(x) => x,
-			Self::EVM(_, x) => vec_to_fixed_array(x),
+	pub fn new(domain: Domain, address: [u8; MAX_ADDRESS_SIZE]) -> Self {
+		match domain {
+			Domain::Centrifuge => DomainAddress::Centrifuge(address.into()),
+			Domain::Evm(chain_id) => {
+				DomainAddress::Evm(chain_id, account_to_eth_address(address.into()))
+			}
 		}
 	}
 
@@ -99,24 +108,26 @@ impl DomainAddress {
 	}
 }
 
-#[cfg(test)]
-mod tests {
-	use parity_scale_codec::{Decode, Encode};
-
-	use super::*;
-
-	#[test]
-	fn test_domain_encode_decode() {
-		test_domain_identity(Domain::Centrifuge);
-		test_domain_identity(Domain::EVM(1284));
-		test_domain_identity(Domain::EVM(1));
+impl DomainAddress {
+	/// Returns the current address as an centrifuge address
+	pub fn account(&self) -> AccountId32 {
+		match self.clone() {
+			Self::Centrifuge(x) => x,
+			Self::Evm(chain_id, x) => eth_address_to_account(chain_id, x),
+		}
 	}
 
-	/// Test that (decode . encode) results in the original value
-	fn test_domain_identity(domain: Domain) {
-		let encoded = domain.encode();
-		let decoded = Domain::decode(&mut encoded.as_slice()).unwrap();
+	/// Returns the current address as an ethrerum address,
+	/// clamping the inner address if needed.
+	pub fn h160(&self) -> H160 {
+		match self.clone() {
+			Self::Centrifuge(x) => account_to_eth_address(x),
+			Self::Evm(_, x) => x,
+		}
+	}
 
-		assert_eq!(domain, decoded);
+	/// Returns the current address as plain bytes
+	pub fn bytes(&self) -> [u8; MAX_ADDRESS_SIZE] {
+		self.account().into()
 	}
 }

libs/types/src/ids.rs

@@ -15,10 +15,7 @@
 use frame_support::PalletId;
 use sp_runtime::TypeId;
 
-use crate::{
-	domain_address::{DomainAddress, DomainLocator},
-	investments::InvestmentAccount,
-};
+use crate::investments::InvestmentAccount;
 
 // The TypeId impl we derive pool-accounts from
 impl<InvestmentId> TypeId for InvestmentAccount<InvestmentId> {
@@ -45,10 +42,5 @@ pub const CHAIN_BRIDGE_NATIVE_TOKEN_ID: [u8; 4] = *b"xCFG";
 /// The identifier of the group eligible to receive block rewards.
 pub const COLLATOR_GROUP_ID: u32 = 1;
 
-impl TypeId for DomainAddress {
-	const TYPE_ID: [u8; 4] = *b"dadr";
-}
-
-impl<Domain> TypeId for DomainLocator<Domain> {
-	const TYPE_ID: [u8; 4] = *b"domn";
-}
+pub const DOMAIN_ID: [u8; 4] = *b"domn";
+pub const DOMAIN_ADDRESS_ID: [u8; 4] = *b"dadr";

node/src/chain_spec.rs

@@ -28,13 +28,13 @@ use cfg_primitives::{
 	SAFE_XCM_VERSION,
 };
 use cfg_types::{
+	domain_address::DomainAddress,
 	fee_keys::FeeKey,
 	tokens::{usdc, AssetMetadata, CrossChainTransferability, CurrencyId, CustomMetadata},
 };
 use cfg_utils::vec_to_fixed_array;
 use cumulus_primitives_core::ParaId;
 use hex_literal::hex;
-use runtime_common::account_conversion::AccountConverter;
 use sc_chain_spec::{ChainSpecExtension, ChainSpecGroup};
 use sc_service::{ChainType, Properties};
 use serde::{Deserialize, Serialize};
@@ -271,8 +271,8 @@ fn centrifuge_genesis(
 	let chain_id: u32 = id.into();
 
 	endowed_accounts.extend(endowed_evm_accounts.into_iter().map(|(addr, id)| {
-		let chain_id = id.unwrap_or_else(|| chain_id.into());
-		AccountConverter::convert_evm_address(chain_id, addr)
+		let chain_id = id.unwrap_or(chain_id.into());
+		DomainAddress::Evm(chain_id, addr.into()).account()
 	}));
 
 	let num_endowed_accounts = endowed_accounts.len();
@@ -371,8 +371,8 @@ fn altair_genesis(
 	let chain_id: u32 = id.into();
 
 	endowed_accounts.extend(endowed_evm_accounts.into_iter().map(|(addr, id)| {
-		let chain_id = id.unwrap_or_else(|| chain_id.into());
-		AccountConverter::convert_evm_address(chain_id, addr)
+		let chain_id = id.unwrap_or(chain_id.into());
+		DomainAddress::Evm(chain_id, addr.into()).account()
 	}));
 
 	let num_endowed_accounts = endowed_accounts.len();
@@ -472,8 +472,8 @@ fn development_genesis(
 	let chain_id: u32 = id.into();
 
 	endowed_accounts.extend(endowed_evm_accounts.into_iter().map(|(addr, id)| {
-		let chain_id = id.unwrap_or_else(|| chain_id.into());
-		AccountConverter::convert_evm_address(chain_id, addr)
+		let chain_id = id.unwrap_or(chain_id.into());
+		DomainAddress::Evm(chain_id, addr.into()).account()
 	}));
 
 	let num_endowed_accounts = endowed_accounts.len();

pallets/axelar-router/src/lib.rs

@@ -244,7 +244,7 @@ pub mod pallet {
 
 					T::Receiver::receive(
 						AxelarId::Evm(chain_id).into(),
-						DomainAddress::EVM(chain_id, source_address_bytes),
+						DomainAddress::Evm(chain_id, source_address_bytes.into()),
 						payload.to_vec(),
 					)
 				}
@@ -330,8 +330,6 @@ pub mod pallet {
 
 			match config.domain {
 				DomainConfig::Evm(evm_config) => {
-					let sender_evm_address = H160::from_slice(&origin.address()[0..20]);
-
 					let message = wrap_into_axelar_msg(
 						message,
 						chain_name.into_inner(),
@@ -340,7 +338,7 @@ pub mod pallet {
 					.map_err(DispatchError::Other)?;
 
 					T::Transactor::call(
-						sender_evm_address,
+						origin.h160(),
 						evm_config.target_contract_address,
 						message.as_slice(),
 						evm_config.fee_values.value,

pallets/axelar-router/src/tests.rs

@@ -1,5 +1,5 @@
 use frame_support::{assert_err, assert_noop, assert_ok};
-use sp_core::U256;
+use sp_core::{crypto::AccountId32, U256};
 
 use crate::{mock::*, *};
 
@@ -9,7 +9,7 @@ const LP_CONTRACT_ADDRESS: H160 = H160::repeat_byte(1);
 const AXELAR_CONTRACT_ADDRESS: H160 = H160::repeat_byte(2);
 const SOURCE_ADDRESS: H160 = H160::repeat_byte(3);
 const AXELAR_CONTRACT_HASH: H256 = H256::repeat_byte(42);
-const SENDER: DomainAddress = DomainAddress::Centrifuge([0; 32]);
+const SENDER: DomainAddress = DomainAddress::Centrifuge(AccountId32::new([0; 32]));
 const MESSAGE: &[u8] = &[1, 2, 3];
 const FEE_VALUE: U256 = U256::zero();
 const GAS_LIMIT: U256 = U256::one();
@@ -91,7 +91,7 @@ mod send {
 			correct_configuration();
 
 			Transactor::mock_call(move |from, to, data, value, gas_price, gas_limit| {
-				assert_eq!(from, H160::from_slice(&SENDER.address()[0..20]));
+				assert_eq!(from, SENDER.h160());
 				assert_eq!(to, AXELAR_CONTRACT_ADDRESS);
 				assert_eq!(data, &wrap_message(MESSAGE.to_vec()));
 				assert_eq!(value, FEE_VALUE);
@@ -143,7 +143,7 @@ mod receive {
 
 			Receiver::mock_receive(|middleware, origin, message| {
 				assert_eq!(middleware, Middleware(AxelarId::Evm(CHAIN_ID)));
-				assert_eq!(origin, DomainAddress::EVM(CHAIN_ID, SOURCE_ADDRESS.0));
+				assert_eq!(origin, DomainAddress::Evm(CHAIN_ID, SOURCE_ADDRESS));
 				assert_eq!(&message, MESSAGE);
 				Ok(())
 			});

pallets/liquidity-pools-gateway/src/lib.rs

@@ -380,7 +380,7 @@ pub mod pallet {
 			T::AdminOrigin::ensure_origin(origin)?;
 
 			ensure!(domain != Domain::Centrifuge, Error::<T>::DomainNotSupported);
-			DomainHookAddress::<T>::insert(domain.clone(), hook_address);
+			DomainHookAddress::<T>::insert(domain, hook_address);
 
 			Self::deposit_event(Event::DomainHookAddressSet {
 				domain,
@@ -495,7 +495,7 @@ pub mod pallet {
 				Error::<T>::DomainNotSupported
 			);
 
-			PackedMessage::<T>::mutate((&from, destination.clone()), |batch| match batch {
+			PackedMessage::<T>::mutate((&from, destination), |batch| match batch {
 				Some(batch) => batch.pack_with(message),
 				None => Self::queue_outbound_message(destination, message),
 			})

pallets/liquidity-pools-gateway/src/mock.rs

@@ -10,14 +10,15 @@ use frame_support::{derive_impl, weights::constants::RocksDbWeight};
 use frame_system::EnsureRoot;
 use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
 use scale_info::TypeInfo;
-use sp_core::{crypto::AccountId32, H256};
+use sp_core::{crypto::AccountId32, H160};
 use sp_runtime::{traits::IdentityLookup, DispatchError, DispatchResult};
 
 use crate::{pallet as pallet_liquidity_pools_gateway, EnsureLocal, GatewayMessage};
 
 pub const TEST_SESSION_ID: u32 = 1;
 pub const TEST_EVM_CHAIN: EVMChainId = 1;
-pub const TEST_DOMAIN_ADDRESS: DomainAddress = DomainAddress::EVM(TEST_EVM_CHAIN, [1; 20]);
+pub const TEST_DOMAIN_ADDRESS: DomainAddress =
+	DomainAddress::Evm(TEST_EVM_CHAIN, H160::repeat_byte(1));
 
 pub const ROUTER_ID_1: RouterId = RouterId(1);
 pub const ROUTER_ID_2: RouterId = RouterId(2);
@@ -123,7 +124,7 @@ impl RouterProvider<Domain> for TestRouterProvider {
 	fn routers_for_domain(domain: Domain) -> Vec<Self::RouterId> {
 		match domain {
 			Domain::Centrifuge => vec![],
-			Domain::EVM(_) => vec![ROUTER_ID_1, ROUTER_ID_2, ROUTER_ID_3],
+			Domain::Evm(_) => vec![ROUTER_ID_1, ROUTER_ID_2, ROUTER_ID_3],
 		}
 	}
 }
@@ -161,7 +162,7 @@ impl cfg_mocks::router_message::pallet::Config for Runtime {
 }
 
 frame_support::parameter_types! {
-	pub Sender: DomainAddress = DomainAddress::Centrifuge(AccountId32::from(H256::from_low_u64_be(123).to_fixed_bytes()).into());
+	pub Sender: DomainAddress = DomainAddress::Centrifuge(AccountId32::from([1; 32]));
 	pub const MaxIncomingMessageSize: u32 = 1024;
 	pub const LpAdminAccount: AccountId32 = LP_ADMIN_ACCOUNT;
 	pub const MaxRouterCount: u32 = 8;

pallets/liquidity-pools-gateway/src/origin.rs

@@ -14,8 +14,6 @@ use cfg_types::domain_address::DomainAddress;
 use frame_support::traits::EnsureOrigin;
 use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
 use scale_info::TypeInfo;
-#[cfg(feature = "runtime-benchmarks")]
-use sp_core::H160;
 use sp_runtime::RuntimeDebug;
 
 #[derive(Clone, Eq, PartialEq, RuntimeDebug, Encode, Decode, MaxEncodedLen, TypeInfo)]
@@ -34,9 +32,6 @@ impl<O: Into<Result<GatewayOrigin, O>> + From<GatewayOrigin>> EnsureOrigin<O> fo
 
 	#[cfg(feature = "runtime-benchmarks")]
 	fn try_successful_origin() -> Result<O, ()> {
-		Ok(O::from(GatewayOrigin::Domain(DomainAddress::EVM(
-			1,
-			H160::from_low_u64_be(1).into(),
-		))))
+		unimplemented!()
 	}
 }