Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Remove overusage of DomainAddress #1979

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions libs/types/src/domain_address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ 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 {
pub fn truncate_into_eth_address(address: AccountId32) -> H160 {
let bytes: [u8; 32] = address.into();
H160::from(
*(bytes)
Expand Down Expand Up @@ -99,7 +99,7 @@ impl DomainAddress {
match domain {
Domain::Centrifuge => DomainAddress::Centrifuge(address.into()),
Domain::Evm(chain_id) => {
DomainAddress::Evm(chain_id, account_to_eth_address(address.into()))
DomainAddress::Evm(chain_id, truncate_into_eth_address(address.into()))
}
}
}
Expand All @@ -122,7 +122,7 @@ impl DomainAddress {
/// clamping the inner address if needed.
pub fn h160(&self) -> H160 {
match self.clone() {
Self::Centrifuge(x) => account_to_eth_address(x),
Self::Centrifuge(x) => truncate_into_eth_address(x),
Self::Evm(_, x) => x,
}
}
Expand Down
82 changes: 74 additions & 8 deletions pallets/axelar-router/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ use cfg_traits::{
liquidity_pools::{MessageReceiver, MessageSender},
PreConditions,
};
use cfg_types::{domain_address::DomainAddress, EVMChainId};
use cfg_types::{
domain_address::{truncate_into_eth_address, Domain, DomainAddress},
EVMChainId,
};
use ethabi::{Contract, Function, Param, ParamType, Token};
use fp_evm::PrecompileHandle;
use frame_support::{
Expand All @@ -32,7 +35,7 @@ use frame_system::pallet_prelude::*;
pub use pallet::*;
use precompile_utils::prelude::*;
use scale_info::prelude::{format, string::String};
use sp_core::{H160, H256, U256};
use sp_core::{crypto::AccountId32, H160, H256, U256};
use sp_std::{boxed::Box, collections::btree_map::BTreeMap, vec, vec::Vec};

#[cfg(test)]
Expand Down Expand Up @@ -128,7 +131,7 @@ pub mod pallet {
/// The target of the messages coming from other chains
type Receiver: MessageReceiver<
Middleware = Self::Middleware,
Origin = DomainAddress,
Origin = Domain,
Message = Vec<u8>,
>;

Expand All @@ -148,13 +151,26 @@ pub mod pallet {
#[pallet::storage]
pub type ChainNameById<T: Config> = StorageMap<_, Twox64Concat, AxelarId, ChainName>;

/// Storage that contains a limited number of whitelisted domain instances.
///
/// This can only be modified by an admin.
#[pallet::storage]
#[pallet::getter(fn allowlist)]
pub type Allowlist<T: Config> = StorageMap<_, Blake2_128Concat, DomainAddress, ()>;

#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
/// An instance was added to a domain.
ConfigSet {
name: ChainName,
config: Box<AxelarConfig>,
},
/// An instance was added to a domain.
InstanceAdded { instance: DomainAddress },

/// An instance was removed from a domain.
InstanceRemoved { instance: DomainAddress },
}

#[pallet::error]
Expand All @@ -173,6 +189,12 @@ pub mod pallet {

/// Emit when a message is received from a non LP caller
ContractCallerMismatch,

/// Instance was already added to the domain.
InstanceAlreadyAdded,

/// Unknown instance.
UnknownInstance,
}

#[pallet::call]
Expand Down Expand Up @@ -212,6 +234,42 @@ pub mod pallet {

Ok(())
}

/// Add a known instance for a specific domain.
#[pallet::weight(Weight::from_parts(50_000_000, 512).saturating_add(RocksDbWeight::get().writes(2)))]
#[pallet::call_index(1)]
pub fn add_instance(origin: OriginFor<T>, instance: DomainAddress) -> DispatchResult {
T::AdminOrigin::ensure_origin(origin)?;

ensure!(
!Allowlist::<T>::contains_key(&instance),
Error::<T>::InstanceAlreadyAdded,
);

Allowlist::<T>::insert(instance.clone(), ());

Self::deposit_event(Event::InstanceAdded { instance });

Ok(())
}

/// Remove an instance from a specific domain.
#[pallet::weight(Weight::from_parts(50_000_000, 512).saturating_add(RocksDbWeight::get().writes(2)))]
#[pallet::call_index(2)]
pub fn remove_instance(origin: OriginFor<T>, instance: DomainAddress) -> DispatchResult {
T::AdminOrigin::ensure_origin(origin.clone())?;

ensure!(
Allowlist::<T>::contains_key(&instance),
Error::<T>::UnknownInstance,
);

Allowlist::<T>::remove(instance.clone());

Self::deposit_event(Event::InstanceRemoved { instance });

Ok(())
}
}

impl<T: Config> Pallet<T> {
Expand All @@ -236,12 +294,20 @@ pub mod pallet {

match config.domain {
DomainConfig::Evm(EvmConfig { chain_id, .. }) => {
let source_address_bytes = decode_var_source::<EVM_ADDRESS_LEN>(source_address)
.ok_or(Error::<T>::InvalidSourceAddress)?;
let source_address: H160 = decode_var_source::<EVM_ADDRESS_LEN>(source_address)
.ok_or(Error::<T>::InvalidSourceAddress)?
.into();

let domain_address = DomainAddress::Evm(chain_id, source_address);

ensure!(
Allowlist::<T>::contains_key(domain_address),
Error::<T>::UnknownInstance,
);

T::Receiver::receive(
AxelarId::Evm(chain_id).into(),
DomainAddress::Evm(chain_id, source_address_bytes.into()),
Domain::Evm(chain_id),
payload.to_vec(),
)
}
Expand Down Expand Up @@ -318,7 +384,7 @@ pub mod pallet {
impl<T: Config> MessageSender for Pallet<T> {
type Message = Vec<u8>;
type Middleware = AxelarId;
type Origin = DomainAddress;
type Origin = AccountId32;

fn send(
axelar_id: AxelarId,
Expand All @@ -340,7 +406,7 @@ pub mod pallet {
.map_err(DispatchError::Other)?;

T::Transactor::call(
origin.h160(),
truncate_into_eth_address(origin),
evm_config.target_contract_address,
message.as_slice(),
evm_config.fee_values.value,
Expand Down
4 changes: 2 additions & 2 deletions pallets/axelar-router/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(AccountId32::new([0; 32]));
const SENDER: AccountId32 = AccountId32::new([0; 32]);
const MESSAGE: &[u8] = &[1, 2, 3];
const FEE_VALUE: U256 = U256::zero();
const GAS_LIMIT: U256 = U256::one();
Expand Down Expand Up @@ -91,7 +91,7 @@ mod send {
correct_configuration();

Transactor::mock_call(move |from, to, data, value, gas_price, gas_limit| {
assert_eq!(from, SENDER.h160());
assert_eq!(from, truncate_into_eth_address(SENDER));
assert_eq!(to, AXELAR_CONTRACT_ADDRESS);
assert_eq!(data, &wrap_message(MESSAGE.to_vec()));
assert_eq!(value, FEE_VALUE);
Expand Down
63 changes: 18 additions & 45 deletions pallets/liquidity-pools-forwarder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ mod tests;
use core::fmt::Debug;

use cfg_traits::liquidity_pools::{LpMessageForwarded, MessageReceiver, MessageSender};
use cfg_types::domain_address::{Domain, DomainAddress};
use cfg_types::domain_address::Domain;
use frame_support::{dispatch::DispatchResult, pallet_prelude::*};
use frame_system::pallet_prelude::OriginFor;
pub use pallet::*;
Expand All @@ -46,11 +46,6 @@ use sp_std::convert::TryInto;

#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
pub struct ForwardInfo {
/// Refers to the chain from which the message originates.
///
/// Example: Assume a three-hop A -> B -> C, then this refers to the domain
/// of A.
pub(crate) source_domain: Domain,
/// Refers to contract on forwarding chain.
///
/// Example: Assume A -> B -> C, then this refers to the forwarding
Expand Down Expand Up @@ -87,16 +82,12 @@ pub mod pallet {
+ FullCodec;

/// The entity of the messages coming from this chain.
type MessageSender: MessageSender<
Middleware = Self::RouterId,
Origin = DomainAddress,
Message = Self::Message,
>;
type MessageSender: MessageSender<Middleware = Self::RouterId, Message = Self::Message>;

/// The entity which acts on unwrapped messages.
type MessageReceiver: MessageReceiver<
Middleware = Self::RouterId,
Origin = DomainAddress,
Origin = Domain,
Message = Self::Message,
>;

Expand All @@ -110,13 +101,11 @@ pub mod pallet {
/// Forwarding info was set
ForwarderSet {
router_id: T::RouterId,
source_domain: Domain,
forwarding_contract: H160,
},
/// Forwarding info was removed
ForwarderRemoved {
router_id: T::RouterId,
source_domain: Domain,
forwarding_contract: H160,
},
}
Expand Down Expand Up @@ -155,22 +144,19 @@ pub mod pallet {
pub fn set_forwarder(
origin: OriginFor<T>,
router_id: T::RouterId,
source_domain: Domain,
forwarding_contract: H160,
) -> DispatchResult {
T::AdminOrigin::ensure_origin(origin)?;

RouterForwarding::<T>::insert(
&router_id,
ForwardInfo {
source_domain,
contract: forwarding_contract,
},
);

Self::deposit_event(Event::<T>::ForwarderSet {
router_id,
source_domain,
forwarding_contract,
});

Expand All @@ -191,7 +177,6 @@ pub mod pallet {
.map(|info| {
Self::deposit_event(Event::<T>::ForwarderRemoved {
router_id,
source_domain: info.source_domain,
forwarding_contract: info.contract,
});
})
Expand All @@ -202,16 +187,16 @@ pub mod pallet {
impl<T: Config> MessageSender for Pallet<T> {
type Message = T::Message;
type Middleware = T::RouterId;
type Origin = DomainAddress;
type Origin = <T::MessageSender as MessageSender>::Origin;

fn send(
router_id: T::RouterId,
origin: DomainAddress,
origin: Self::Origin,
message: T::Message,
) -> DispatchResult {
let msg = RouterForwarding::<T>::get(&router_id)
.map(|info| {
T::Message::try_wrap_forward(info.source_domain, info.contract, message.clone())
T::Message::try_wrap_forward(Domain::Centrifuge, info.contract, message.clone())
})
.unwrap_or_else(|| {
ensure!(!message.is_forwarded(), Error::<T>::ForwardInfoNotFound);
Expand All @@ -225,42 +210,30 @@ pub mod pallet {
impl<T: Config> MessageReceiver for Pallet<T> {
type Message = T::Message;
type Middleware = T::RouterId;
type Origin = DomainAddress;
type Origin = Domain;

fn receive(
router_id: T::RouterId,
forwarding_domain_address: DomainAddress,
forwarding_domain: Domain,
message: T::Message,
) -> DispatchResult {
// Message can be unwrapped iff it was forwarded
//
// NOTE: Contract address irrelevant here because it is only necessary for
// outbound forwarded messages
let (lp_message, domain_address) = match (
RouterForwarding::<T>::get(&router_id),
let (lp_message, domain) = match (
RouterForwarding::<T>::get(&router_id).is_some(),
message.clone().unwrap_forwarded(),
) {
(Some(info), Some((source_domain, _contract, lp_message))) => {
ensure!(
info.source_domain == source_domain,
Error::<T>::SourceDomainMismatch
);

let domain_address = DomainAddress::Evm(
info.source_domain
.get_evm_chain_id()
.expect("Domain not Centrifuge; qed"),
info.contract,
);
Ok((lp_message, domain_address))
// NOTE: Contract address irrelevant here because it is only necessary for
// outbound forwarded messages
(true, Some((source_domain, _contract, lp_message))) => {
Ok((lp_message, source_domain))
}
(Some(_), None) => Err(Error::<T>::UnwrappingFailed),
(None, None) => Ok((message, forwarding_domain_address)),
(None, Some((_, _, _))) => Err(Error::<T>::ForwardInfoNotFound),
(true, None) => Err(Error::<T>::UnwrappingFailed),
(false, None) => Ok((message, forwarding_domain)),
(false, Some((_, _, _))) => Err(Error::<T>::ForwardInfoNotFound),
}
.map_err(|e: Error<T>| e)?;

T::MessageReceiver::receive(router_id, domain_address, lp_message)
T::MessageReceiver::receive(router_id, domain, lp_message)
}
}
}
Loading