refactor: Refactor AddressMapping in pallet-cosmos

frame/cosmos/src/lib.rs

@@ -50,13 +50,12 @@ use frame_support::{
 	weights::Weight,
 };
 use frame_system::{pallet_prelude::OriginFor, CheckWeight};
-use pallet_cosmos_types::events::traits::EventManager;
 use pallet_cosmos_types::{
 	address::acc_address_from_bech32,
 	context,
 	context::traits::Context,
 	errors::{CosmosError, RootError},
-	events::CosmosEvent,
+	events::{traits::EventManager, CosmosEvent},
 	gas::{traits::GasMeter, Gas},
 	handler::AnteDecorator,
 	msgservice::MsgServiceRouter,
@@ -103,18 +102,20 @@ where
 	pub fn check_self_contained(&self) -> Option<Result<H160, TransactionValidityError>> {
 		if let Call::transact { tx_bytes } = self {
 			let check = || {
-				let (_hrp, address) = Tx::decode(&mut &tx_bytes[..])
+				let (_hrp, address_raw) = Tx::decode(&mut &tx_bytes[..])
 					.map(|tx| T::SigVerifiableTx::fee_payer(&tx))
 					.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Call))?
 					.map(|fee_payer| acc_address_from_bech32(&fee_payer))
-					.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Call))?
-					.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Call))?;
+					.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::BadSigner))?
+					.map_err(|_| {
+						TransactionValidityError::Invalid(InvalidTransaction::BadSigner)
+					})?;
 
-				if address.len() != 20 {
-					return Err(TransactionValidityError::Invalid(InvalidTransaction::Call));
+				if address_raw.len() != 20 {
+					return Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner));
 				}
 
-				Ok(H160::from_slice(&address))
+				Ok(H160::from_slice(&address_raw))
 			};
 
 			Some(check())
@@ -159,8 +160,7 @@ where
 }
 
 pub trait AddressMapping<A> {
-	fn from_address_raw(address: H160) -> A;
-	fn from_bech32(address: &str) -> Option<A>;
+	fn into_account_id(address: H160) -> A;
 }
 
 #[frame_support::pallet]
@@ -500,7 +500,7 @@ impl<T: Config> Pallet<T> {
 	}
 
 	pub fn sequence_of(address: &H160) -> u64 {
-		let account_id = T::AddressMapping::from_address_raw(*address);
+		let account_id = T::AddressMapping::into_account_id(*address);
 		frame_system::Pallet::<T>::account_nonce(&account_id).saturated_into()
 	}
 }

frame/cosmos/src/mock.rs

@@ -138,7 +138,7 @@ where
 	T::AccountId: From<CosmosSigner> + EcdsaExt,
 	H: Hasher<Out = H256>,
 {
-	fn from_address_raw(address: H160) -> T::AccountId {
+	fn into_account_id(address: H160) -> T::AccountId {
 		if let Some(x) = pallet_cosmos_accounts::Connections::<T>::get(address) {
 			return x;
 		}
@@ -152,15 +152,6 @@ where
 
 		CosmosSigner(ecdsa::Public(interim)).into()
 	}
-
-	fn from_bech32(address: &str) -> Option<T::AccountId> {
-		let (_hrp, address_raw) = acc_address_from_bech32(address).ok()?;
-		if address_raw.len() != 20 {
-			return None;
-		}
-
-		Some(Self::from_address_raw(H160::from_slice(&address_raw)))
-	}
 }
 
 type AnteHandler<T> = pallet_cosmos_x_auth::AnteDecorators<T>;
@@ -239,11 +230,11 @@ impl<T> Convert<Vec<u8>, Result<AccountId, ()>> for AccountToAddr<T>
 where
 	T: pallet_cosmos::Config<AccountId = CosmosSigner>,
 {
-	fn convert(address_raw: Vec<u8>) -> Result<AccountId, ()> {
+	fn convert(address: Vec<u8>) -> Result<AccountId, ()> {
 		// Cosmos address length is 20, contract address is 32.
-		let account = match address_raw.len() {
-			20 => T::AddressMapping::from_address_raw(H160::from_slice(&address_raw)),
-			32 => AccountId::unchecked_from(H256::from_slice(&address_raw)),
+		let account = match address.len() {
+			20 => T::AddressMapping::into_account_id(H160::from_slice(&address)),
+			32 => AccountId::unchecked_from(H256::from_slice(&address)),
 			_ => return Err(()),
 		};
 
@@ -314,7 +305,7 @@ impl fp_self_contained::SelfContainedCall for RuntimeCall {
 		match self {
 			RuntimeCall::Cosmos(call) => match call.check_self_contained()? {
 				Ok(address) => Some(Ok(
-					<Test as pallet_cosmos::Config>::AddressMapping::from_address_raw(address),
+					<Test as pallet_cosmos::Config>::AddressMapping::into_account_id(address),
 				)),
 				Err(e) => Some(Err(e)),
 			},

frame/cosmos/x/auth/src/fee.rs

@@ -29,14 +29,15 @@ use frame_support::{
 };
 use pallet_cosmos::AddressMapping;
 use pallet_cosmos_types::{
+	address::acc_address_from_bech32,
 	coin::amount_to_string,
 	events::{
 		CosmosEvent, EventAttribute, ATTRIBUTE_KEY_FEE, ATTRIBUTE_KEY_FEE_PAYER, EVENT_TYPE_TX,
 	},
 	handler::AnteDecorator,
 };
 use pallet_cosmos_x_auth_signing::sign_verifiable_tx::traits::SigVerifiableTx;
-use sp_core::Get;
+use sp_core::{Get, H160};
 use sp_runtime::{
 	traits::{Convert, Zero},
 	transaction_validity::{TransactionValidity, TransactionValidityError, ValidTransaction},
@@ -90,8 +91,12 @@ where
 			return Err(TransactionValidityError::Invalid(InvalidTransaction::Call));
 		}
 
-		let deduct_fees_from = T::AddressMapping::from_bech32(&fee_payer)
-			.ok_or(TransactionValidityError::Invalid(InvalidTransaction::Call))?;
+		let (_hrp, address_raw) = acc_address_from_bech32(&fee_payer)
+			.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::BadSigner))?;
+		if address_raw.len() != 20 {
+			return Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner));
+		}
+		let deduct_fees_from = T::AddressMapping::into_account_id(H160::from_slice(&address_raw));
 
 		// TODO: Check fee is zero
 		if !fee.amount.is_empty() {

frame/cosmos/x/auth/src/sigverify.rs

@@ -72,15 +72,15 @@ where
 				.get(i)
 				.ok_or(TransactionValidityError::Invalid(InvalidTransaction::BadSigner))?;
 
-			let (_hrp, signer_addr) = acc_address_from_bech32(signer)
+			let (_hrp, signer_addr_raw) = acc_address_from_bech32(signer)
 				.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::BadSigner))?;
 
-			if signer_addr.len() != 20 {
+			if signer_addr_raw.len() != 20 {
 				return Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner));
 			}
 
-			let signer_addr = H160::from_slice(&signer_addr);
-			let sequence = pallet_cosmos::Pallet::<T>::sequence_of(&signer_addr);
+			let sequence =
+				pallet_cosmos::Pallet::<T>::sequence_of(&H160::from_slice(&signer_addr_raw));
 			if signer_info.sequence > sequence {
 				return Err(TransactionValidityError::Invalid(InvalidTransaction::Future));
 			} else if signer_info.sequence < sequence {
@@ -134,17 +134,16 @@ where
 					hasher.update(sha2_256(&public_key.key));
 					let address = H160::from_slice(&hasher.finalize());
 
-					let (_hrp, signer_addr) =
+					let (_hrp, signer_addr_raw) =
 						acc_address_from_bech32(&signer_data.address).map_err(|_| {
 							TransactionValidityError::Invalid(InvalidTransaction::BadSigner)
 						})?;
 
-					if signer_addr.len() != 20 {
+					if signer_addr_raw.len() != 20 {
 						return Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner));
 					}
 
-					let signer_addr = H160::from_slice(&signer_addr);
-					if signer_addr != address {
+					if  H160::from_slice(&signer_addr_raw) != address {
 						return Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner));
 					}
 
@@ -210,10 +209,15 @@ where
 {
 	fn ante_handle(tx: &Tx, _simulate: bool) -> TransactionValidity {
 		let signers = T::SigVerifiableTx::get_signers(tx)
-			.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::BadSigner))?;
+			.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Call))?;
 		for signer in signers.iter() {
-			let account = T::AddressMapping::from_bech32(signer)
-				.ok_or(TransactionValidityError::Invalid(InvalidTransaction::BadSigner))?;
+			let (_hrp, address_raw) = acc_address_from_bech32(signer)
+				.map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::BadSigner))?;
+			if address_raw.len() != 20 {
+				return Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner));
+			}
+
+			let account = T::AddressMapping::into_account_id(H160::from_slice(&address_raw));
 			frame_system::pallet::Pallet::<T>::inc_account_nonce(account);
 		}
 

frame/cosmos/x/bank/src/msgs.rs

@@ -26,12 +26,15 @@ use pallet_assets::WeightInfo as _;
 use pallet_balances::WeightInfo as _;
 use pallet_cosmos::AddressMapping;
 use pallet_cosmos_types::{
+	address::acc_address_from_bech32,
 	coin::amount_to_string,
 	context,
 	errors::{CosmosError, RootError},
-	events::{traits::EventManager, EventAttribute, ATTRIBUTE_KEY_AMOUNT, ATTRIBUTE_KEY_SENDER}, gas::traits::GasMeter,
+	events::{traits::EventManager, EventAttribute, ATTRIBUTE_KEY_AMOUNT, ATTRIBUTE_KEY_SENDER},
+	gas::traits::GasMeter,
 };
 use pallet_cosmos_x_bank_types::events::{ATTRIBUTE_KEY_RECIPIENT, EVENT_TYPE_TRANSFER};
+use sp_core::H160;
 use sp_runtime::{traits::Convert, SaturatedConversion};
 
 pub struct MsgSendHandler<T>(PhantomData<T>);
@@ -51,10 +54,19 @@ where
 		let MsgSend { from_address, to_address, amount } =
 			MsgSend::decode(&mut &*msg.value).map_err(|_| RootError::UnpackAnyError)?;
 
-		let from_account =
-			T::AddressMapping::from_bech32(&from_address).ok_or(RootError::InvalidAddress)?;
-		let to_account =
-			T::AddressMapping::from_bech32(&to_address).ok_or(RootError::InvalidAddress)?;
+		let (_hrp, from_address_raw) =
+			acc_address_from_bech32(&from_address).map_err(|_| RootError::InvalidAddress)?;
+		let (_hrp, to_address_raw) =
+			acc_address_from_bech32(&to_address).map_err(|_| RootError::InvalidAddress)?;
+
+		if from_address_raw.len() != 20 {
+			return Err(RootError::InvalidAddress.into());
+		}
+		if to_address_raw.len() != 20 {
+			return Err(RootError::InvalidAddress.into());
+		}
+		let from_account = T::AddressMapping::into_account_id(H160::from_slice(&from_address_raw));
+		let to_account = T::AddressMapping::into_account_id(H160::from_slice(&to_address_raw));
 
 		ctx.gas_meter()
 			.consume_gas(T::DbWeight::get().reads(2).ref_time(), "")

frame/cosmos/x/wasm/Cargo.toml

@@ -19,6 +19,7 @@ log = { workspace = true, default-features = false }
 libflate = { workspace = true, default-features = false }
 
 frame-support = { workspace = true, default-features = false }
+sp-core = { workspace = true, default-features = false }
 sp-runtime = { workspace = true, default-features = false }
 
 hp-crypto = { workspace = true, default-features = false }
@@ -37,6 +38,7 @@ std = [
 	"log/std",
 	"libflate/std",
 	"frame-support/std",
+	"sp-core/std",
 	"sp-runtime/std",
 	"hp-crypto/std",
 	"pallet-cosmos/std",

frame/cosmos/x/wasm/src/msgs.rs

@@ -32,6 +32,7 @@ use hp_crypto::EcdsaExt;
 use libflate::gzip::Decoder;
 use pallet_cosmos::AddressMapping;
 use pallet_cosmos_types::{
+	address::acc_address_from_bech32,
 	context,
 	errors::{CosmosError, RootError},
 	events::{traits::EventManager, CosmosEvent, EventAttribute},
@@ -52,6 +53,7 @@ use pallet_cosmwasm::{
 		CodeIdentifier, ContractCodeOf, ContractLabelOf, ContractMessageOf, ContractSaltOf, FundsOf,
 	},
 };
+use sp_core::H160;
 use sp_runtime::{traits::Convert, SaturatedConversion};
 
 pub struct MsgStoreCodeHandler<T>(PhantomData<T>);
@@ -72,7 +74,12 @@ where
 		let MsgStoreCode { sender, wasm_byte_code, instantiate_permission: _ } =
 			MsgStoreCode::decode(&mut &*msg.value).map_err(|_| RootError::TxDecodeError)?;
 
-		let who = T::AddressMapping::from_bech32(&sender).ok_or(RootError::TxDecodeError)?;
+		let (_hrp, address_raw) =
+			acc_address_from_bech32(&sender).map_err(|_| RootError::InvalidAddress)?;
+		if address_raw.len() != 20 {
+			return Err(RootError::InvalidAddress.into());
+		}
+		let who = T::AddressMapping::into_account_id(H160::from_slice(&address_raw));
 		let mut decoder = Decoder::new(&wasm_byte_code[..]).map_err(|_| WasmError::CreateFailed)?;
 		let mut decoded_code = Vec::new();
 		decoder.read_to_end(&mut decoded_code).map_err(|_| WasmError::CreateFailed)?;
@@ -128,7 +135,12 @@ where
 		if sender.is_empty() {
 			return Err(WasmError::Empty.into());
 		}
-		let who = T::AddressMapping::from_bech32(&sender).ok_or(RootError::TxDecodeError)?;
+		let (_hrp, address_raw) =
+			acc_address_from_bech32(&sender).map_err(|_| RootError::InvalidAddress)?;
+		if address_raw.len() != 20 {
+			return Err(RootError::InvalidAddress.into());
+		}
+		let who = T::AddressMapping::into_account_id(H160::from_slice(&address_raw));
 		let gas = ctx.gas_meter().gas_remaining();
 		let mut shared = pallet_cosmwasm::Pallet::<T>::do_create_vm_shared(
 			gas,
@@ -203,7 +215,12 @@ where
 		if sender.is_empty() {
 			return Err(WasmError::Empty.into());
 		}
-		let who = T::AddressMapping::from_bech32(&sender).ok_or(RootError::TxDecodeError)?;
+		let (_hrp, address_raw) =
+			acc_address_from_bech32(&sender).map_err(|_| RootError::InvalidAddress)?;
+		if address_raw.len() != 20 {
+			return Err(RootError::InvalidAddress.into());
+		}
+		let who = T::AddressMapping::into_account_id(H160::from_slice(&address_raw));
 		let gas = ctx.gas_meter().gas_remaining();
 		let mut shared = pallet_cosmwasm::Pallet::<T>::do_create_vm_shared(
 			gas,
@@ -259,7 +276,12 @@ where
 		if sender.is_empty() {
 			return Err(WasmError::Empty.into());
 		}
-		let who = T::AddressMapping::from_bech32(&sender).ok_or(RootError::TxDecodeError)?;
+		let (_hrp, address_raw) =
+			acc_address_from_bech32(&sender).map_err(|_| RootError::InvalidAddress)?;
+		if address_raw.len() != 20 {
+			return Err(RootError::InvalidAddress.into());
+		}
+		let who = T::AddressMapping::into_account_id(H160::from_slice(&address_raw));
 		let gas = ctx.gas_meter().gas_remaining();
 		let mut shared = pallet_cosmwasm::Pallet::<T>::do_create_vm_shared(
 			gas,
@@ -318,7 +340,12 @@ where
 		if sender.is_empty() {
 			return Err(WasmError::Empty.into());
 		}
-		let who = T::AddressMapping::from_bech32(&sender).ok_or(RootError::TxDecodeError)?;
+		let (_hrp, address_raw) =
+			acc_address_from_bech32(&sender).map_err(|_| RootError::InvalidAddress)?;
+		if address_raw.len() != 20 {
+			return Err(RootError::InvalidAddress.into());
+		}
+		let who = T::AddressMapping::into_account_id(H160::from_slice(&address_raw));
 		let gas = ctx.gas_meter().gas_remaining();
 		let mut shared = pallet_cosmwasm::Pallet::<T>::do_create_vm_shared(
 			gas,

template/runtime/src/accounts.rs

@@ -60,11 +60,11 @@ impl<T> Convert<Vec<u8>, Result<AccountId, ()>> for AccountToAddr<T>
 where
 	T: pallet_cosmos::Config<AccountId = CosmosSigner>,
 {
-	fn convert(address_raw: Vec<u8>) -> Result<AccountId, ()> {
+	fn convert(address: Vec<u8>) -> Result<AccountId, ()> {
 		// Cosmos address length is 20, contract address is 32.
-		let account = match address_raw.len() {
-			20 => T::AddressMapping::from_address_raw(H160::from_slice(&address_raw)),
-			32 => AccountId::unchecked_from(H256::from_slice(&address_raw)),
+		let account = match address.len() {
+			20 => T::AddressMapping::into_account_id(H160::from_slice(&address)),
+			32 => AccountId::unchecked_from(H256::from_slice(&address)),
 			_ => return Err(()),
 		};
 

template/runtime/src/compat/cosmos.rs

@@ -22,7 +22,6 @@ use core::marker::PhantomData;
 use hp_account::CosmosSigner;
 use hp_crypto::EcdsaExt;
 use pallet_cosmos::AddressMapping;
-use pallet_cosmos_types::address::acc_address_from_bech32;
 use sp_core::{ecdsa, Hasher, H160, H256};
 
 /// Hashed address mapping.
@@ -34,7 +33,7 @@ where
 	T::AccountId: From<CosmosSigner> + EcdsaExt,
 	H: Hasher<Out = H256>,
 {
-	fn from_address_raw(address: H160) -> T::AccountId {
+	fn into_account_id(address: H160) -> T::AccountId {
 		if let Some(x) = pallet_cosmos_accounts::Connections::<T>::get(address) {
 			return x;
 		}
@@ -48,13 +47,4 @@ where
 
 		CosmosSigner(ecdsa::Public(interim)).into()
 	}
-
-	fn from_bech32(address: &str) -> Option<T::AccountId> {
-		let (_hrp, address_raw) = acc_address_from_bech32(address).ok()?;
-		if address_raw.len() != 20 {
-			return None;
-		}
-
-		Some(Self::from_address_raw(H160::from_slice(&address_raw)))
-	}
 }