From 54ec73df00416426f9d160a3e75ff4a3249e946b Mon Sep 17 00:00:00 2001 From: Tomasz Polaczyk Date: Thu, 21 Nov 2024 16:06:53 +0100 Subject: [PATCH] PR comments --- pallets/external-validator-slashes/src/lib.rs | 4 + .../bridge/src/custom_do_process_message.rs | 140 ++++++++++++ primitives/bridge/src/custom_send_message.rs | 47 ++++ primitives/bridge/src/lib.rs | 208 +----------------- solo-chains/runtime/dancelight/src/lib.rs | 4 - 5 files changed, 198 insertions(+), 205 deletions(-) create mode 100644 primitives/bridge/src/custom_do_process_message.rs create mode 100644 primitives/bridge/src/custom_send_message.rs diff --git a/pallets/external-validator-slashes/src/lib.rs b/pallets/external-validator-slashes/src/lib.rs index f09de4f10..a88b953f7 100644 --- a/pallets/external-validator-slashes/src/lib.rs +++ b/pallets/external-validator-slashes/src/lib.rs @@ -127,7 +127,11 @@ pub mod pallet { /// Invulnerable provider, used to get the invulnerables to know when not to slash type InvulnerablesProvider: InvulnerablesProvider; + + /// Validate a message that will be sent to Ethereum. type ValidateMessage: ValidateMessage; + + /// Send a message to Ethereum. Needs to be validated first. type OutboundQueue: DeliverMessage< Ticket = <::ValidateMessage as ValidateMessage>::Ticket, >; diff --git a/primitives/bridge/src/custom_do_process_message.rs b/primitives/bridge/src/custom_do_process_message.rs new file mode 100644 index 000000000..bb4b724b7 --- /dev/null +++ b/primitives/bridge/src/custom_do_process_message.rs @@ -0,0 +1,140 @@ +use { + super::*, + frame_support::{ + ensure, + traits::{Defensive, ProcessMessage, ProcessMessageError}, + weights::WeightMeter, + }, + snowbridge_pallet_outbound_queue::{ + CommittedMessage, MessageLeaves, Messages, Nonce, ProcessMessageOriginOf, WeightInfo, + }, + sp_runtime::traits::Hash, + sp_std::boxed::Box, +}; + +/// Alternative to [snowbridge_pallet_outbound_queue::Pallet::process_message] using a different +/// [Command] enum. +pub struct CustomProcessSnowbridgeMessage(PhantomData); + +impl CustomProcessSnowbridgeMessage +where + T: snowbridge_pallet_outbound_queue::Config, +{ + /// Process a message delivered by the MessageQueue pallet + pub(crate) fn do_process_message( + _: ProcessMessageOriginOf, + mut message: &[u8], + ) -> Result { + use ProcessMessageError::*; + + // Yield if the maximum number of messages has been processed this block. + // This ensures that the weight of `on_finalize` has a known maximum bound. + ensure!( + MessageLeaves::::decode_len().unwrap_or(0) < T::MaxMessagesPerBlock::get() as usize, + Yield + ); + + // Decode bytes into versioned message + let versioned_queued_message: VersionedQueuedMessage = + VersionedQueuedMessage::decode(&mut message).map_err(|_| Corrupt)?; + + // Convert versioned message into latest supported message version + let queued_message: QueuedMessage = versioned_queued_message + .try_into() + .map_err(|_| Unsupported)?; + + // Obtain next nonce + let nonce = >::try_mutate( + queued_message.channel_id, + |nonce| -> Result { + *nonce = nonce.checked_add(1).ok_or(Unsupported)?; + Ok(*nonce) + }, + )?; + + let pricing_params = T::PricingParameters::get(); + let command = queued_message.command.index(); + let params = queued_message.command.abi_encode(); + let max_dispatch_gas = + ConstantGasMeter::maximum_dispatch_gas_used_at_most(&queued_message.command); + let reward = pricing_params.rewards.remote; + + // Construct the final committed message + let message = CommittedMessage { + channel_id: queued_message.channel_id, + nonce, + command, + params, + max_dispatch_gas, + max_fee_per_gas: pricing_params + .fee_per_gas + .try_into() + .defensive_unwrap_or(u128::MAX), + reward: reward.try_into().defensive_unwrap_or(u128::MAX), + id: queued_message.id, + }; + + // ABI-encode and hash the prepared message + let message_abi_encoded = ethabi::encode(&[message.clone().into()]); + let message_abi_encoded_hash = + ::Hashing::hash(&message_abi_encoded); + + Messages::::append(Box::new(message)); + MessageLeaves::::append(message_abi_encoded_hash); + + snowbridge_pallet_outbound_queue::Pallet::::deposit_event( + snowbridge_pallet_outbound_queue::Event::MessageAccepted { + id: queued_message.id, + nonce, + }, + ); + + Ok(true) + } +} + +impl ProcessMessage for CustomProcessSnowbridgeMessage +where + T: snowbridge_pallet_outbound_queue::Config, +{ + type Origin = T::AggregateMessageOrigin; + + fn process_message( + message: &[u8], + origin: Self::Origin, + meter: &mut WeightMeter, + _id: &mut [u8; 32], + ) -> Result { + // TODO: this weight is from the pallet, should be very similar to the weight of + // Self::do_process_message, but ideally we should benchmark this separately + let weight = T::WeightInfo::do_process_message(); + if meter.try_consume(weight).is_err() { + return Err(ProcessMessageError::Overweight(weight)); + } + + Self::do_process_message(origin.clone(), message) + } +} + +/// A meter that assigns a constant amount of gas for the execution of a command +/// +/// The gas figures are extracted from this report: +/// > forge test --match-path test/Gateway.t.sol --gas-report +/// +/// A healthy buffer is added on top of these figures to account for: +/// * The EIP-150 63/64 rule +/// * Future EVM upgrades that may increase gas cost +pub struct ConstantGasMeter; + +impl ConstantGasMeter { + // The base transaction cost, which includes: + // 21_000 transaction cost, roughly worst case 64_000 for calldata, and 100_000 + // for message verification + pub const MAXIMUM_BASE_GAS: u64 = 185_000; + + fn maximum_dispatch_gas_used_at_most(command: &Command) -> u64 { + match command { + Command::Test { .. } => 60_000, + } + } +} diff --git a/primitives/bridge/src/custom_send_message.rs b/primitives/bridge/src/custom_send_message.rs new file mode 100644 index 000000000..d3d7f6104 --- /dev/null +++ b/primitives/bridge/src/custom_send_message.rs @@ -0,0 +1,47 @@ +use { + super::*, + ethabi::H256, + frame_support::traits::EnqueueMessage, + snowbridge_core::{outbound::SendError, PRIMARY_GOVERNANCE_CHANNEL}, + sp_std::marker::PhantomData, +}; + +/// Alternative to [snowbridge_pallet_outbound_queue::Pallet::deliver] using a different +/// origin. +pub struct CustomSendMessage( + PhantomData<(T, GetAggregateMessageOrigin)>, +); + +impl DeliverMessage + for CustomSendMessage +where + T: snowbridge_pallet_outbound_queue::Config, + GetAggregateMessageOrigin: + Convert::AggregateMessageOrigin>, +{ + type Ticket = Ticket; + + fn deliver(ticket: Self::Ticket) -> Result { + let origin = GetAggregateMessageOrigin::convert(ticket.channel_id); + + if ticket.channel_id != PRIMARY_GOVERNANCE_CHANNEL { + ensure!( + !>::operating_mode().is_halted(), + SendError::Halted + ); + } + + let message = ticket.message.as_bounded_slice(); + + ::MessageQueue::enqueue_message( + message, origin, + ); + snowbridge_pallet_outbound_queue::Pallet::::deposit_event( + snowbridge_pallet_outbound_queue::Event::MessageQueued { + id: ticket.message_id, + }, + ); + + Ok(ticket.message_id) + } +} diff --git a/primitives/bridge/src/lib.rs b/primitives/bridge/src/lib.rs index 84b868cc2..6af386177 100644 --- a/primitives/bridge/src/lib.rs +++ b/primitives/bridge/src/lib.rs @@ -38,14 +38,18 @@ use { sp_runtime::{app_crypto::sp_core, traits::Convert, RuntimeDebug}, sp_std::vec::Vec, }; + +// Separate import as rustfmt wrongly change it to `sp_std::vec::self`, which is the module instead +// of the macro. +use sp_std::vec; + pub use { custom_do_process_message::{ConstantGasMeter, CustomProcessSnowbridgeMessage}, custom_send_message::CustomSendMessage, }; -// Separate import as rustfmt wrongly change it to `sp_std::vec::self`, which is the module instead -// of the macro. -use sp_std::vec; +mod custom_do_process_message; +mod custom_send_message; /// A command which is executable by the Gateway contract on Ethereum #[derive(Clone, Encode, Decode, RuntimeDebug, TypeInfo)] @@ -206,201 +210,3 @@ pub trait DeliverMessage { fn deliver(ticket: Self::Ticket) -> Result; } - -mod custom_do_process_message { - use { - super::*, - frame_support::{ - ensure, - traits::{Defensive, ProcessMessage, ProcessMessageError}, - weights::WeightMeter, - }, - snowbridge_pallet_outbound_queue::{ - CommittedMessage, MessageLeaves, Messages, Nonce, ProcessMessageOriginOf, WeightInfo, - }, - sp_runtime::traits::Hash, - sp_std::boxed::Box, - }; - - /// Alternative to [snowbridge_pallet_outbound_queue::Pallet::process_message] using a different - /// [Command] enum. - pub struct CustomProcessSnowbridgeMessage(PhantomData); - - impl CustomProcessSnowbridgeMessage - where - T: snowbridge_pallet_outbound_queue::Config, - { - /// Process a message delivered by the MessageQueue pallet - pub(crate) fn do_process_message( - _: ProcessMessageOriginOf, - mut message: &[u8], - ) -> Result { - use ProcessMessageError::*; - - // Yield if the maximum number of messages has been processed this block. - // This ensures that the weight of `on_finalize` has a known maximum bound. - ensure!( - MessageLeaves::::decode_len().unwrap_or(0) - < T::MaxMessagesPerBlock::get() as usize, - Yield - ); - - // Decode bytes into versioned message - let versioned_queued_message: VersionedQueuedMessage = - VersionedQueuedMessage::decode(&mut message).map_err(|_| Corrupt)?; - - // Convert versioned message into latest supported message version - let queued_message: QueuedMessage = versioned_queued_message - .try_into() - .map_err(|_| Unsupported)?; - - // Obtain next nonce - let nonce = >::try_mutate( - queued_message.channel_id, - |nonce| -> Result { - *nonce = nonce.checked_add(1).ok_or(Unsupported)?; - Ok(*nonce) - }, - )?; - - let pricing_params = T::PricingParameters::get(); - let command = queued_message.command.index(); - let params = queued_message.command.abi_encode(); - let max_dispatch_gas = - ConstantGasMeter::maximum_dispatch_gas_used_at_most(&queued_message.command); - let reward = pricing_params.rewards.remote; - - // Construct the final committed message - let message = CommittedMessage { - channel_id: queued_message.channel_id, - nonce, - command, - params, - max_dispatch_gas, - max_fee_per_gas: pricing_params - .fee_per_gas - .try_into() - .defensive_unwrap_or(u128::MAX), - reward: reward.try_into().defensive_unwrap_or(u128::MAX), - id: queued_message.id, - }; - - // ABI-encode and hash the prepared message - let message_abi_encoded = ethabi::encode(&[message.clone().into()]); - let message_abi_encoded_hash = - ::Hashing::hash( - &message_abi_encoded, - ); - - Messages::::append(Box::new(message)); - MessageLeaves::::append(message_abi_encoded_hash); - - snowbridge_pallet_outbound_queue::Pallet::::deposit_event( - snowbridge_pallet_outbound_queue::Event::MessageAccepted { - id: queued_message.id, - nonce, - }, - ); - - Ok(true) - } - } - - impl ProcessMessage for CustomProcessSnowbridgeMessage - where - T: snowbridge_pallet_outbound_queue::Config, - { - type Origin = T::AggregateMessageOrigin; - - fn process_message( - message: &[u8], - origin: Self::Origin, - meter: &mut WeightMeter, - _id: &mut [u8; 32], - ) -> Result { - // TODO: this weight is from the pallet, should be very similar to the weight of - // Self::do_process_message, but ideally we should benchmark this separately - let weight = T::WeightInfo::do_process_message(); - if meter.try_consume(weight).is_err() { - return Err(ProcessMessageError::Overweight(weight)); - } - - Self::do_process_message(origin.clone(), message) - } - } - - /// A meter that assigns a constant amount of gas for the execution of a command - /// - /// The gas figures are extracted from this report: - /// > forge test --match-path test/Gateway.t.sol --gas-report - /// - /// A healthy buffer is added on top of these figures to account for: - /// * The EIP-150 63/64 rule - /// * Future EVM upgrades that may increase gas cost - pub struct ConstantGasMeter; - - impl ConstantGasMeter { - // The base transaction cost, which includes: - // 21_000 transaction cost, roughly worst case 64_000 for calldata, and 100_000 - // for message verification - pub const MAXIMUM_BASE_GAS: u64 = 185_000; - - fn maximum_dispatch_gas_used_at_most(command: &Command) -> u64 { - match command { - Command::Test { .. } => 60_000, - } - } - } -} - -mod custom_send_message { - use { - super::*, - ethabi::H256, - frame_support::traits::EnqueueMessage, - snowbridge_core::{outbound::SendError, PRIMARY_GOVERNANCE_CHANNEL}, - sp_std::marker::PhantomData, - }; - - /// Alternative to [snowbridge_pallet_outbound_queue::Pallet::deliver] using a different - /// origin. - pub struct CustomSendMessage( - PhantomData<(T, GetAggregateMessageOrigin)>, - ); - - impl DeliverMessage - for CustomSendMessage - where - T: snowbridge_pallet_outbound_queue::Config, - GetAggregateMessageOrigin: Convert< - ChannelId, - ::AggregateMessageOrigin, - >, - { - type Ticket = Ticket; - - fn deliver(ticket: Self::Ticket) -> Result { - let origin = GetAggregateMessageOrigin::convert(ticket.channel_id); - - if ticket.channel_id != PRIMARY_GOVERNANCE_CHANNEL { - ensure!( - !>::operating_mode().is_halted(), - SendError::Halted - ); - } - - let message = ticket.message.as_bounded_slice(); - - ::MessageQueue::enqueue_message( - message, origin, - ); - snowbridge_pallet_outbound_queue::Pallet::::deposit_event( - snowbridge_pallet_outbound_queue::Event::MessageQueued { - id: ticket.message_id, - }, - ); - - Ok(ticket.message_id) - } - } -} diff --git a/solo-chains/runtime/dancelight/src/lib.rs b/solo-chains/runtime/dancelight/src/lib.rs index 7e2ef0f89..0da2bc9e4 100644 --- a/solo-chains/runtime/dancelight/src/lib.rs +++ b/solo-chains/runtime/dancelight/src/lib.rs @@ -1741,11 +1741,7 @@ construct_runtime! { XcmPallet: pallet_xcm = 90, // Bridging stuff - // https://github.com/paritytech/polkadot-sdk/blob/2ae79be8e028a995b850621ee55f46c041eceefe/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs#L560C1-L560C64 - //EthereumInboundQueue: snowbridge_pallet_inbound_queue = 80, EthereumOutboundQueue: snowbridge_pallet_outbound_queue = 101, - // TODO: already exists, at index 243 - //EthereumBeaconClient: snowbridge_pallet_ethereum_client = 82, EthereumSystem: snowbridge_pallet_system = 103, // Migration stuff