From f4b6dba04744f6c31a589fe7ccb83cdc3be886f5 Mon Sep 17 00:00:00 2001 From: ron Date: Thu, 25 Apr 2024 13:18:03 +0800 Subject: [PATCH] AgentId as user input --- .../primitives/core/src/outbound.rs | 45 +++++----- .../primitives/router/src/outbound/mod.rs | 77 +++++++++------- .../primitives/router/src/outbound/tests.rs | 87 +++++++++++-------- .../bridge-hub-rococo/src/tests/snowbridge.rs | 12 +-- .../penpal/src/pallets/transact_helper.rs | 7 +- 5 files changed, 131 insertions(+), 97 deletions(-) diff --git a/bridges/snowbridge/primitives/core/src/outbound.rs b/bridges/snowbridge/primitives/core/src/outbound.rs index 557ffe77a017..988d3eadad37 100644 --- a/bridges/snowbridge/primitives/core/src/outbound.rs +++ b/bridges/snowbridge/primitives/core/src/outbound.rs @@ -140,6 +140,19 @@ mod v1 { // Fee multiplier multiplier: UD60x18, }, + /// Execute a contract on the target chain + Transact { + /// ID of the agent + agent_id: H256, + /// Target contract address + target: H160, + /// The call data to the contract + payload: Vec, + /// The dynamic gas component that needs to be specified when executing the contract + gas_limit: u64, + /// The fee to cover the delivery cost + fee: u128, + }, } impl Command { @@ -155,6 +168,7 @@ mod v1 { Command::TransferNativeFromAgent { .. } => 6, Command::SetTokenTransferFees { .. } => 7, Command::SetPricingParameters { .. } => 8, + Command::Transact { .. } => 9, } } @@ -212,6 +226,13 @@ mod v1 { Token::Uint(U256::from(*delivery_cost)), Token::Uint(multiplier.clone().into_inner()), ])]), + Command::Transact { agent_id, target, payload, gas_limit, .. } => + ethabi::encode(&[Token::Tuple(vec![ + Token::FixedBytes(agent_id.as_bytes().to_owned()), + Token::Address(*target), + Token::Bytes(payload.clone()), + Token::Uint(U256::from(*gas_limit)), + ])]), } } } @@ -239,24 +260,12 @@ mod v1 { /// The amount of tokens to transfer amount: u128, }, - /// Execute a contract on the target chain - Transact { - /// Target contract address - target: H160, - /// The call data to the contract - payload: Vec, - /// The dynamic gas component that needs to be specified when executing the contract - gas_limit: u64, - /// The fee to cover the delivery cost - fee: u128, - }, } impl AgentExecuteCommand { fn index(&self) -> u8 { match self { AgentExecuteCommand::TransferToken { .. } => 0, - AgentExecuteCommand::Transact { .. } => 1, } } @@ -272,15 +281,6 @@ mod v1 { Token::Uint(U256::from(*amount)), ])), ]), - AgentExecuteCommand::Transact { target, payload, gas_limit, .. } => - ethabi::encode(&[ - Token::Uint(self.index().into()), - Token::Bytes(ethabi::encode(&[ - Token::Address(*target), - Token::Bytes(payload.clone()), - Token::Uint(U256::from(*gas_limit)), - ])), - ]), } } } @@ -413,7 +413,6 @@ impl GasMeter for ConstantGasMeter { // * Assume dest account in ERC20 contract does not yet have a storage slot // * ERC20.transferFrom possibly does other business logic besides updating balances AgentExecuteCommand::TransferToken { .. } => 100_000, - AgentExecuteCommand::Transact { gas_limit, .. } => *gas_limit, }, Command::Upgrade { initializer, .. } => { let initializer_max_gas = match *initializer { @@ -426,6 +425,7 @@ impl GasMeter for ConstantGasMeter { }, Command::SetTokenTransferFees { .. } => 60_000, Command::SetPricingParameters { .. } => 60_000, + Command::Transact { gas_limit, .. } => *gas_limit, } } } @@ -442,6 +442,7 @@ pub const ETHER_DECIMALS: u8 = 18; #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] pub struct TransactInfo { + pub agent_id: H256, pub target: H160, pub call: Vec, pub gas_limit: u64, diff --git a/bridges/snowbridge/primitives/router/src/outbound/mod.rs b/bridges/snowbridge/primitives/router/src/outbound/mod.rs index 6c3b8c71ff2d..f5f28cedb903 100644 --- a/bridges/snowbridge/primitives/router/src/outbound/mod.rs +++ b/bridges/snowbridge/primitives/router/src/outbound/mod.rs @@ -88,36 +88,28 @@ where SendError::MissingArgument })?; - let mut converter = XcmConverter::new(&message, &expected_network); - let (agent_execute_command, message_id) = converter.convert().map_err(|err|{ + let mut converter = XcmConverter::::new( + &message, + expected_network, + para_id.into(), + ); + let (command, message_id) = converter.convert().map_err(|err|{ log::error!(target: "xcm::ethereum_blob_exporter", "unroutable due to pattern matching error '{err:?}'."); SendError::Unroutable })?; - let source_location = Location::new(1, local_sub.clone()); - let agent_id = match AgentHashedDescription::convert_location(&source_location) { - Some(id) => id, - None => { - log::error!(target: "xcm::ethereum_blob_exporter", "unroutable due to not being able to create agent id. '{source_location:?}'"); - return Err(SendError::Unroutable) - }, - }; - let channel_id: ChannelId = ParaId::from(para_id).into(); - let outbound_message = Message { - id: Some(message_id.into()), - channel_id, - command: Command::AgentExecute { agent_id, command: agent_execute_command.clone() }, - }; + let outbound_message = + Message { id: Some(message_id.into()), channel_id, command: command.clone() }; // validate the message let (ticket, fees) = OutboundQueue::validate(&outbound_message).map_err(|err| { log::error!(target: "xcm::ethereum_blob_exporter", "OutboundQueue validation of message failed. {err:?}"); SendError::Unroutable })?; - let fee = match agent_execute_command { - AgentExecuteCommand::Transact { fee, .. } => { + let fee = match command { + Command::Transact { fee, .. } => { ensure!(fee > fees.remote, SendError::Fees); Ok::(Asset::from((Location::parent(), fees.local)).into()) }, @@ -163,6 +155,7 @@ enum XcmConverterError { TransactSovereignAccountExpected, TransactDecodeFailed, UnexpectedInstruction, + Unroutable, } macro_rules! match_expression { @@ -174,17 +167,28 @@ macro_rules! match_expression { }; } -struct XcmConverter<'a, Call> { +struct XcmConverter<'a, AgentHashedDescription, Call> { iter: Peekable>>, - ethereum_network: &'a NetworkId, + ethereum_network: NetworkId, + para_id: ParaId, + _marker: PhantomData, } -impl<'a, Call> XcmConverter<'a, Call> { - fn new(message: &'a Xcm, ethereum_network: &'a NetworkId) -> Self { - Self { iter: message.inner().iter().peekable(), ethereum_network } +impl<'a, AgentHashedDescription, Call> XcmConverter<'a, AgentHashedDescription, Call> +where + AgentHashedDescription: ConvertLocation, +{ + fn new(message: &'a Xcm, ethereum_network: NetworkId, para_id: ParaId) -> Self { + Self { + iter: message.inner().iter().peekable(), + ethereum_network, + para_id, + _marker: Default::default(), + } } - fn convert(&mut self) -> Result<(AgentExecuteCommand, [u8; 32]), XcmConverterError> { + fn convert(&mut self) -> Result<(Command, [u8; 32]), XcmConverterError> { let result = match self.peek() { + // Transact message Ok(Transact { .. }) => self.transact_message(), // Get withdraw/deposit and make native tokens create message. Ok(WithdrawAsset { .. }) => self.native_tokens_unlock_message(), @@ -200,7 +204,7 @@ impl<'a, Call> XcmConverter<'a, Call> { Ok(result) } - fn transact_message(&mut self) -> Result<(AgentExecuteCommand, [u8; 32]), XcmConverterError> { + fn transact_message(&mut self) -> Result<(Command, [u8; 32]), XcmConverterError> { let call_data = if let Transact { origin_kind, call, .. } = self.next()? { ensure!( *origin_kind == OriginKind::SovereignAccount, @@ -216,7 +220,8 @@ impl<'a, Call> XcmConverter<'a, Call> { let topic_id = match_expression!(self.next()?, SetTopic(id), id).ok_or(SetTopicExpected)?; Ok(( - AgentExecuteCommand::Transact { + Command::Transact { + agent_id: message.agent_id, target: message.target, payload: message.call, gas_limit: message.gas_limit, @@ -226,9 +231,7 @@ impl<'a, Call> XcmConverter<'a, Call> { )) } - fn native_tokens_unlock_message( - &mut self, - ) -> Result<(AgentExecuteCommand, [u8; 32]), XcmConverterError> { + fn native_tokens_unlock_message(&mut self) -> Result<(Command, [u8; 32]), XcmConverterError> { use XcmConverterError::*; // Get the reserve assets from WithdrawAsset. @@ -302,7 +305,19 @@ impl<'a, Call> XcmConverter<'a, Call> { // Check if there is a SetTopic and skip over it if found. let topic_id = match_expression!(self.next()?, SetTopic(id), id).ok_or(SetTopicExpected)?; - Ok((AgentExecuteCommand::TransferToken { token, recipient, amount }, *topic_id)) + let agent_id = AgentHashedDescription::convert_location(&Location::new( + 1, + Parachain(self.para_id.into()), + )) + .ok_or(Unroutable)?; + + Ok(( + Command::AgentExecute { + agent_id, + command: AgentExecuteCommand::TransferToken { token, recipient, amount }, + }, + *topic_id, + )) } fn next(&mut self) -> Result<&'a Instruction, XcmConverterError> { @@ -315,7 +330,7 @@ impl<'a, Call> XcmConverter<'a, Call> { fn network_matches(&self, network: &Option) -> bool { if let Some(network) = network { - network == self.ethereum_network + network.clone() == self.ethereum_network } else { true } diff --git a/bridges/snowbridge/primitives/router/src/outbound/tests.rs b/bridges/snowbridge/primitives/router/src/outbound/tests.rs index 63a241ebf291..a861453ce059 100644 --- a/bridges/snowbridge/primitives/router/src/outbound/tests.rs +++ b/bridges/snowbridge/primitives/router/src/outbound/tests.rs @@ -410,11 +410,14 @@ fn xcm_converter_convert_success() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); - let expected_payload = AgentExecuteCommand::TransferToken { - token: token_address.into(), - recipient: beneficiary_address.into(), - amount: 1000, + let mut converter = XcmConverter::::new(&message, network, 1000.into()); + let expected_payload = Command::AgentExecute { + agent_id: hex!("81c5ab2571199e3188135178f3c2c8e2d268be1313d029b30f534fa579b69b79").into(), + command: AgentExecuteCommand::TransferToken { + token: token_address.into(), + recipient: beneficiary_address.into(), + amount: 1000, + }, }; let result = converter.convert(); assert_eq!(result, Ok((expected_payload, [0; 32]))); @@ -443,11 +446,14 @@ fn xcm_converter_convert_without_buy_execution_yields_success() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); - let expected_payload = AgentExecuteCommand::TransferToken { - token: token_address.into(), - recipient: beneficiary_address.into(), - amount: 1000, + let mut converter = XcmConverter::::new(&message, network, 1000.into()); + let expected_payload = Command::AgentExecute { + agent_id: hex!("81c5ab2571199e3188135178f3c2c8e2d268be1313d029b30f534fa579b69b79").into(), + command: AgentExecuteCommand::TransferToken { + token: token_address.into(), + recipient: beneficiary_address.into(), + amount: 1000, + }, }; let result = converter.convert(); assert_eq!(result, Ok((expected_payload, [0; 32]))); @@ -478,11 +484,14 @@ fn xcm_converter_convert_with_wildcard_all_asset_filter_succeeds() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); - let expected_payload = AgentExecuteCommand::TransferToken { - token: token_address.into(), - recipient: beneficiary_address.into(), - amount: 1000, + let mut converter = XcmConverter::::new(&message, network, 1000.into()); + let expected_payload = Command::AgentExecute { + agent_id: hex!("81c5ab2571199e3188135178f3c2c8e2d268be1313d029b30f534fa579b69b79").into(), + command: AgentExecuteCommand::TransferToken { + token: token_address.into(), + recipient: beneficiary_address.into(), + amount: 1000, + }, }; let result = converter.convert(); assert_eq!(result, Ok((expected_payload, [0; 32]))); @@ -513,11 +522,14 @@ fn xcm_converter_convert_with_fees_less_than_reserve_yields_success() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); - let expected_payload = AgentExecuteCommand::TransferToken { - token: token_address.into(), - recipient: beneficiary_address.into(), - amount: 1000, + let mut converter = XcmConverter::::new(&message, network, 1000.into()); + let expected_payload = Command::AgentExecute { + agent_id: hex!("81c5ab2571199e3188135178f3c2c8e2d268be1313d029b30f534fa579b69b79").into(), + command: AgentExecuteCommand::TransferToken { + token: token_address.into(), + recipient: beneficiary_address.into(), + amount: 1000, + }, }; let result = converter.convert(); assert_eq!(result, Ok((expected_payload, [0; 32]))); @@ -547,7 +559,7 @@ fn xcm_converter_convert_without_set_topic_yields_set_topic_expected() { ClearTopic, ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::SetTopicExpected)); } @@ -564,7 +576,8 @@ fn xcm_converter_convert_with_partial_message_yields_unexpected_end_of_xcm() { .into(); let message: Xcm<()> = vec![WithdrawAsset(assets)].into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); + let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::UnexpectedEndOfXcm)); } @@ -595,7 +608,7 @@ fn xcm_converter_with_different_fee_asset_fails() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::InvalidFeeAsset)); } @@ -625,7 +638,7 @@ fn xcm_converter_with_fees_greater_than_reserve_fails() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::InvalidFeeAsset)); } @@ -636,7 +649,7 @@ fn xcm_converter_convert_with_empty_xcm_yields_unexpected_instruction() { let message: Xcm<()> = vec![].into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::UnexpectedEndOfXcm)); @@ -668,7 +681,7 @@ fn xcm_converter_convert_with_extra_instructions_yields_end_of_xcm_message_expec ClearError, ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::EndOfXcmMessageExpected)); @@ -698,7 +711,7 @@ fn xcm_converter_convert_without_withdraw_asset_yields_unexpected_instruction() SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::UnexpectedInstruction)); @@ -723,7 +736,7 @@ fn xcm_converter_convert_without_withdraw_asset_yields_deposit_expected() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::DepositAssetExpected)); @@ -756,7 +769,7 @@ fn xcm_converter_convert_without_assets_yields_no_reserve_assets() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::NoReserveAssets)); @@ -794,7 +807,7 @@ fn xcm_converter_convert_with_two_assets_yields_too_many_assets() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::TooManyAssets)); @@ -825,7 +838,7 @@ fn xcm_converter_convert_without_consuming_filter_yields_filter_does_not_consume SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::FilterDoesNotConsumeAllAssets)); @@ -856,7 +869,7 @@ fn xcm_converter_convert_with_zero_amount_asset_yields_zero_asset_transfer() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::ZeroAssetTransfer)); @@ -886,7 +899,7 @@ fn xcm_converter_convert_non_ethereum_asset_yields_asset_resolution_failed() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::AssetResolutionFailed)); @@ -919,7 +932,7 @@ fn xcm_converter_convert_non_ethereum_chain_asset_yields_asset_resolution_failed SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::AssetResolutionFailed)); @@ -952,7 +965,7 @@ fn xcm_converter_convert_non_ethereum_chain_yields_asset_resolution_failed() { SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::AssetResolutionFailed)); @@ -989,7 +1002,7 @@ fn xcm_converter_convert_with_non_ethereum_beneficiary_yields_beneficiary_resolu SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::BeneficiaryResolutionFailed)); @@ -1025,7 +1038,7 @@ fn xcm_converter_convert_with_non_ethereum_chain_beneficiary_yields_beneficiary_ SetTopic([0; 32]), ] .into(); - let mut converter = XcmConverter::new(&message, &network); + let mut converter = XcmConverter::::new(&message, network, 1000.into()); let result = converter.convert(); assert_eq!(result.err(), Some(XcmConverterError::BeneficiaryResolutionFailed)); diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs index b2fd0db3264e..e63e98496807 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/snowbridge.rs @@ -567,14 +567,15 @@ fn register_weth_token_in_asset_hub_fail_for_insufficient_fee() { fn transact_from_penpal_to_ethereum() { BridgeHubRococo::fund_para_sovereign(PenpalA::para_id().into(), INITIAL_FUND); + // Agent as PalletInstance + let agent_id = snowbridge_pallet_system::agent_id_of::<::Runtime>( + &Location::new(1, [Parachain(PenpalA::para_id().into()), PalletInstance(52)]), + ) + .unwrap(); + // Create agent and channel BridgeHubRococo::execute_with(|| { type Runtime = ::Runtime; - let agent_id = snowbridge_pallet_system::agent_id_of::(&Location::new( - 1, - [Parachain(PenpalA::para_id().into())], - )) - .unwrap(); snowbridge_pallet_system::Agents::::insert(agent_id, ()); let channel_id: ChannelId = PenpalA::para_id().into(); snowbridge_pallet_system::Channels::::insert( @@ -596,6 +597,7 @@ fn transact_from_penpal_to_ethereum() { let remote_cost = 2_750_872_500_000; assert_ok!(::TransactHelper::transact_to_ethereum( RuntimeOrigin::signed(sender.clone()), + agent_id, //contract hex!("ee9170abfbf9421ad6dd07f6bdec9d89f2b581e0").into(), //call diff --git a/cumulus/parachains/runtimes/testing/penpal/src/pallets/transact_helper.rs b/cumulus/parachains/runtimes/testing/penpal/src/pallets/transact_helper.rs index 6ca0011eb84d..487d788de370 100644 --- a/cumulus/parachains/runtimes/testing/penpal/src/pallets/transact_helper.rs +++ b/cumulus/parachains/runtimes/testing/penpal/src/pallets/transact_helper.rs @@ -20,7 +20,7 @@ pub mod pallet { use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; - use sp_core::H160; + use sp_core::{H160, H256}; use sp_std::{vec, vec::Vec}; use xcm::prelude::*; use xcm_executor::traits::XcmAssetTransfers; @@ -51,6 +51,7 @@ pub mod pallet { #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] pub struct TransactInfo { + pub agent_id: H256, pub target: H160, pub call: Vec, pub gas_limit: u64, @@ -69,6 +70,7 @@ pub mod pallet { #[pallet::weight(Weight::from_parts(100_000_000, 0))] pub fn transact_to_ethereum( origin: OriginFor, + agent_id: H256, target: H160, call: Vec, fee: u128, @@ -80,7 +82,8 @@ pub mod pallet { parents: 2, interior: Junctions::from([GlobalConsensus(Ethereum { chain_id: 11155111 })]), }; - let transact = TransactInfo { target, call, gas_limit, fee }; + + let transact = TransactInfo { agent_id, target, call, gas_limit, fee }; let inner_message = Xcm(vec![ Transact {