From 00846ab073a0f60e82326a10f5b3e28c3038582d Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 18 Jan 2023 09:56:53 +0300 Subject: [PATCH] Relayer reward metric (#1742) * use StorageDoubleMapKeyProvider in RelayerRewards * add metrics * clippy * fixed alerts that have caused missing dashboards * fix metric name * fix metric name again * add new metrics to the RialtoParachain <> Millau maintenance dashboard * remove obsolete dashboard --- bridges/modules/relayers/Cargo.toml | 2 + bridges/modules/relayers/src/lib.rs | 17 ++++-- bridges/primitives/relayers/Cargo.toml | 2 + bridges/primitives/relayers/src/lib.rs | 22 ++++++- .../src/cli/relay_headers_and_messages/mod.rs | 26 +++++--- .../client-bridge-hub-rococo/src/lib.rs | 1 + .../client-bridge-hub-wococo/src/lib.rs | 1 + bridges/relays/client-millau/src/lib.rs | 2 + .../relays/client-rialto-parachain/src/lib.rs | 2 + bridges/relays/client-rialto/src/lib.rs | 2 + bridges/relays/client-substrate/src/chain.rs | 10 ++++ bridges/relays/lib-substrate-relay/Cargo.toml | 2 + .../src/messages_metrics.rs | 59 +++++++++++++++++-- 13 files changed, 127 insertions(+), 21 deletions(-) diff --git a/bridges/modules/relayers/Cargo.toml b/bridges/modules/relayers/Cargo.toml index 9c07123836381..732adbf4faa6c 100644 --- a/bridges/modules/relayers/Cargo.toml +++ b/bridges/modules/relayers/Cargo.toml @@ -15,6 +15,7 @@ scale-info = { version = "2.1.1", default-features = false, features = ["derive" bp-messages = { path = "../../primitives/messages", default-features = false } bp-relayers = { path = "../../primitives/relayers", default-features = false } +bp-runtime = { path = "../../primitives/runtime", default-features = false } # Substrate Dependencies @@ -37,6 +38,7 @@ default = ["std"] std = [ "bp-messages/std", "bp-relayers/std", + "bp-runtime/std", "codec/std", "frame-support/std", "frame-system/std", diff --git a/bridges/modules/relayers/src/lib.rs b/bridges/modules/relayers/src/lib.rs index b28d17cf5a4fd..7132914a4ae85 100644 --- a/bridges/modules/relayers/src/lib.rs +++ b/bridges/modules/relayers/src/lib.rs @@ -21,7 +21,8 @@ #![warn(missing_docs)] use bp_messages::LaneId; -use bp_relayers::PaymentProcedure; +use bp_relayers::{PaymentProcedure, RelayerRewardsKeyProvider}; +use bp_runtime::StorageDoubleMapKeyProvider; use frame_support::sp_runtime::Saturating; use sp_arithmetic::traits::{AtLeast32BitUnsigned, Zero}; use sp_std::marker::PhantomData; @@ -46,6 +47,10 @@ pub mod pallet { use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; + /// `RelayerRewardsKeyProvider` for given configuration. + type RelayerRewardsKeyProviderOf = + RelayerRewardsKeyProvider<::AccountId, ::Reward>; + #[pallet::config] pub trait Config: frame_system::Config { /// The overarching event type. @@ -146,11 +151,11 @@ pub mod pallet { #[pallet::getter(fn relayer_reward)] pub type RelayerRewards = StorageDoubleMap< _, - Blake2_128Concat, - T::AccountId, - Identity, - LaneId, - T::Reward, + as StorageDoubleMapKeyProvider>::Hasher1, + as StorageDoubleMapKeyProvider>::Key1, + as StorageDoubleMapKeyProvider>::Hasher2, + as StorageDoubleMapKeyProvider>::Key2, + as StorageDoubleMapKeyProvider>::Value, OptionQuery, >; } diff --git a/bridges/primitives/relayers/Cargo.toml b/bridges/primitives/relayers/Cargo.toml index 4f893f9f83ebc..acede813995d1 100644 --- a/bridges/primitives/relayers/Cargo.toml +++ b/bridges/primitives/relayers/Cargo.toml @@ -11,6 +11,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0" # Bridge Dependencies bp-messages = { path = "../messages", default-features = false } +bp-runtime = { path = "../runtime", default-features = false } # Substrate Dependencies @@ -27,6 +28,7 @@ hex-literal = "0.3" default = ["std"] std = [ "bp-messages/std", + "bp-runtime/std", "frame-support/std", "sp-runtime/std", "sp-std/std", diff --git a/bridges/primitives/relayers/src/lib.rs b/bridges/primitives/relayers/src/lib.rs index d00b5f626e4ab..207908296cbf5 100644 --- a/bridges/primitives/relayers/src/lib.rs +++ b/bridges/primitives/relayers/src/lib.rs @@ -20,8 +20,10 @@ #![cfg_attr(not(feature = "std"), no_std)] use bp_messages::LaneId; +use bp_runtime::StorageDoubleMapKeyProvider; +use frame_support::{Blake2_128Concat, Identity}; use sp_runtime::{ - codec::{Decode, Encode}, + codec::{Codec, Decode, Encode, EncodeLike}, traits::AccountIdConversion, }; use sp_std::{fmt::Debug, marker::PhantomData}; @@ -65,6 +67,24 @@ where } } +/// Can be use to access the runtime storage key within the `RelayerRewards` map of the relayers +/// pallet. +pub struct RelayerRewardsKeyProvider(PhantomData<(AccountId, Reward)>); + +impl StorageDoubleMapKeyProvider for RelayerRewardsKeyProvider +where + AccountId: Codec + EncodeLike, + Reward: Codec + EncodeLike, +{ + const MAP_NAME: &'static str = "RelayerRewards"; + + type Hasher1 = Blake2_128Concat; + type Key1 = AccountId; + type Hasher2 = Identity; + type Key2 = LaneId; + type Value = Reward; +} + #[cfg(test)] mod tests { use super::*; diff --git a/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs b/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs index ea6d6ad9f51a2..193632c28b427 100644 --- a/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs +++ b/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages/mod.rs @@ -61,8 +61,8 @@ use crate::{ use bp_messages::LaneId; use bp_runtime::BalanceOf; use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, Chain, ChainWithBalances, ChainWithTransactions, Client, - Parachain, + AccountIdOf, AccountKeyPairOf, Chain, ChainWithBalances, ChainWithMessages, + ChainWithTransactions, Client, Parachain, }; use relay_utils::metrics::MetricsParams; use sp_core::Pair; @@ -259,9 +259,9 @@ where type Base: Full2WayBridgeBase; /// The left relay chain. - type Left: ChainWithTransactions + ChainWithBalances + CliChain; + type Left: ChainWithTransactions + ChainWithBalances + ChainWithMessages + CliChain; /// The right relay chain. - type Right: ChainWithTransactions + ChainWithBalances + CliChain; + type Right: ChainWithTransactions + ChainWithBalances + ChainWithMessages + CliChain; /// Left to Right bridge. type L2R: MessagesCliBridge; @@ -317,28 +317,36 @@ where self.mut_base().start_on_demand_headers_relayers().await?; // add balance-related metrics + let lanes = self + .base() + .common() + .shared + .lane + .iter() + .cloned() + .map(Into::into) + .collect::>(); { let common = self.mut_base().mut_common(); - substrate_relay_helper::messages_metrics::add_relay_balances_metrics( + substrate_relay_helper::messages_metrics::add_relay_balances_metrics::<_, Self::Right>( common.left.client.clone(), &mut common.metrics_params, &common.left.accounts, + &lanes, ) .await?; - substrate_relay_helper::messages_metrics::add_relay_balances_metrics( + substrate_relay_helper::messages_metrics::add_relay_balances_metrics::<_, Self::Left>( common.right.client.clone(), &mut common.metrics_params, &common.right.accounts, + &lanes, ) .await?; } - let lanes = self.base().common().shared.lane.clone(); // Need 2x capacity since we consider both directions for each lane let mut message_relays = Vec::with_capacity(lanes.len() * 2); for lane in lanes { - let lane = lane.into(); - let left_to_right_messages = substrate_relay_helper::messages_lane::run::< ::MessagesLane, >(self.left_to_right().messages_relay_params( diff --git a/bridges/relays/client-bridge-hub-rococo/src/lib.rs b/bridges/relays/client-bridge-hub-rococo/src/lib.rs index 8e6e971292599..806a492b7f8ee 100644 --- a/bridges/relays/client-bridge-hub-rococo/src/lib.rs +++ b/bridges/relays/client-bridge-hub-rococo/src/lib.rs @@ -110,6 +110,7 @@ impl ChainWithTransactions for BridgeHubRococo { impl ChainWithMessages for BridgeHubRococo { const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str = bp_bridge_hub_rococo::WITH_BRIDGE_HUB_ROCOCO_MESSAGES_PALLET_NAME; + const WITH_CHAIN_RELAYERS_PALLET_NAME: Option<&'static str> = None; const TO_CHAIN_MESSAGE_DETAILS_METHOD: &'static str = bp_bridge_hub_rococo::TO_BRIDGE_HUB_ROCOCO_MESSAGE_DETAILS_METHOD; diff --git a/bridges/relays/client-bridge-hub-wococo/src/lib.rs b/bridges/relays/client-bridge-hub-wococo/src/lib.rs index 3fd8187fa1fea..40f50ce1e3862 100644 --- a/bridges/relays/client-bridge-hub-wococo/src/lib.rs +++ b/bridges/relays/client-bridge-hub-wococo/src/lib.rs @@ -110,6 +110,7 @@ impl ChainWithTransactions for BridgeHubWococo { impl ChainWithMessages for BridgeHubWococo { const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str = bp_bridge_hub_wococo::WITH_BRIDGE_HUB_WOCOCO_MESSAGES_PALLET_NAME; + const WITH_CHAIN_RELAYERS_PALLET_NAME: Option<&'static str> = None; const TO_CHAIN_MESSAGE_DETAILS_METHOD: &'static str = bp_bridge_hub_wococo::TO_BRIDGE_HUB_WOCOCO_MESSAGE_DETAILS_METHOD; diff --git a/bridges/relays/client-millau/src/lib.rs b/bridges/relays/client-millau/src/lib.rs index fb901a4b2de39..34bbea92d57e9 100644 --- a/bridges/relays/client-millau/src/lib.rs +++ b/bridges/relays/client-millau/src/lib.rs @@ -45,6 +45,8 @@ impl ChainWithGrandpa for Millau { impl ChainWithMessages for Millau { const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str = bp_millau::WITH_MILLAU_MESSAGES_PALLET_NAME; + // TODO (https://github.com/paritytech/parity-bridges-common/issues/1692): change the name + const WITH_CHAIN_RELAYERS_PALLET_NAME: Option<&'static str> = Some("BridgeRelayers"); const TO_CHAIN_MESSAGE_DETAILS_METHOD: &'static str = bp_millau::TO_MILLAU_MESSAGE_DETAILS_METHOD; const FROM_CHAIN_MESSAGE_DETAILS_METHOD: &'static str = diff --git a/bridges/relays/client-rialto-parachain/src/lib.rs b/bridges/relays/client-rialto-parachain/src/lib.rs index ebaac73e03600..739b33838b5be 100644 --- a/bridges/relays/client-rialto-parachain/src/lib.rs +++ b/bridges/relays/client-rialto-parachain/src/lib.rs @@ -63,6 +63,8 @@ impl ChainWithBalances for RialtoParachain { impl ChainWithMessages for RialtoParachain { const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str = bp_rialto_parachain::WITH_RIALTO_PARACHAIN_MESSAGES_PALLET_NAME; + // TODO (https://github.com/paritytech/parity-bridges-common/issues/1692): change the name + const WITH_CHAIN_RELAYERS_PALLET_NAME: Option<&'static str> = Some("BridgeRelayers"); const TO_CHAIN_MESSAGE_DETAILS_METHOD: &'static str = bp_rialto_parachain::TO_RIALTO_PARACHAIN_MESSAGE_DETAILS_METHOD; const FROM_CHAIN_MESSAGE_DETAILS_METHOD: &'static str = diff --git a/bridges/relays/client-rialto/src/lib.rs b/bridges/relays/client-rialto/src/lib.rs index 4c3a9d1e18eaa..8ad31de4d583a 100644 --- a/bridges/relays/client-rialto/src/lib.rs +++ b/bridges/relays/client-rialto/src/lib.rs @@ -63,6 +63,8 @@ impl ChainWithGrandpa for Rialto { impl ChainWithMessages for Rialto { const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str = bp_rialto::WITH_RIALTO_MESSAGES_PALLET_NAME; + // TODO (https://github.com/paritytech/parity-bridges-common/issues/1692): change the name + const WITH_CHAIN_RELAYERS_PALLET_NAME: Option<&'static str> = Some("BridgeRelayers"); const TO_CHAIN_MESSAGE_DETAILS_METHOD: &'static str = bp_rialto::TO_RIALTO_MESSAGE_DETAILS_METHOD; const FROM_CHAIN_MESSAGE_DETAILS_METHOD: &'static str = diff --git a/bridges/relays/client-substrate/src/chain.rs b/bridges/relays/client-substrate/src/chain.rs index db08fc1f98ce1..4ec5edfc41bcf 100644 --- a/bridges/relays/client-substrate/src/chain.rs +++ b/bridges/relays/client-substrate/src/chain.rs @@ -101,6 +101,16 @@ pub trait ChainWithMessages: Chain { /// the same name. const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str; + // TODO (https://github.com/paritytech/parity-bridges-common/issues/1692): check all the names + // after the issue is fixed - all names must be changed + + /// Name of the bridge relayers pallet (used in `construct_runtime` macro call) that is deployed + /// at some other chain to bridge with this `ChainWithMessages`. + /// + /// We assume that all chains that are bridging with this `ChainWithMessages` are using + /// the same name. + const WITH_CHAIN_RELAYERS_PALLET_NAME: Option<&'static str>; + /// Name of the `ToOutboundLaneApi::message_details` runtime API method. /// The method is provided by the runtime that is bridged with this `ChainWithMessages`. const TO_CHAIN_MESSAGE_DETAILS_METHOD: &'static str; diff --git a/bridges/relays/lib-substrate-relay/Cargo.toml b/bridges/relays/lib-substrate-relay/Cargo.toml index bdf49d42eab44..e044be0957c74 100644 --- a/bridges/relays/lib-substrate-relay/Cargo.toml +++ b/bridges/relays/lib-substrate-relay/Cargo.toml @@ -12,6 +12,7 @@ async-std = "1.9.0" async-trait = "0.1" codec = { package = "parity-scale-codec", version = "3.1.5" } futures = "0.3.12" +hex = "0.4" num-traits = "0.2" log = "0.4.17" @@ -20,6 +21,7 @@ log = "0.4.17" bp-header-chain = { path = "../../primitives/header-chain" } bp-parachains = { path = "../../primitives/parachains" } bp-polkadot-core = { path = "../../primitives/polkadot-core" } +bp-relayers = { path = "../../primitives/relayers" } bridge-runtime-common = { path = "../../bin/runtime-common" } finality-grandpa = { version = "0.16.0" } diff --git a/bridges/relays/lib-substrate-relay/src/messages_metrics.rs b/bridges/relays/lib-substrate-relay/src/messages_metrics.rs index 37a6d67baae43..943f3b7c3694f 100644 --- a/bridges/relays/lib-substrate-relay/src/messages_metrics.rs +++ b/bridges/relays/lib-substrate-relay/src/messages_metrics.rs @@ -18,12 +18,15 @@ use crate::TaggedAccount; +use bp_messages::LaneId; +use bp_runtime::StorageDoubleMapKeyProvider; use codec::Decode; use frame_system::AccountInfo; use pallet_balances::AccountData; use relay_substrate_client::{ metrics::{FloatStorageValue, FloatStorageValueMetric}, - AccountIdOf, BalanceOf, Chain, ChainWithBalances, Client, Error as SubstrateError, IndexOf, + AccountIdOf, BalanceOf, Chain, ChainWithBalances, ChainWithMessages, Client, + Error as SubstrateError, IndexOf, }; use relay_utils::metrics::{MetricsParams, StandaloneMetric}; use sp_core::storage::StorageData; @@ -31,10 +34,11 @@ use sp_runtime::{FixedPointNumber, FixedU128}; use std::{convert::TryFrom, fmt::Debug, marker::PhantomData}; /// Add relay accounts balance metrics. -pub async fn add_relay_balances_metrics( +pub async fn add_relay_balances_metrics( client: Client, metrics: &mut MetricsParams, relay_accounts: &Vec>>, + lanes: &[LaneId], ) -> anyhow::Result<()> where BalanceOf: Into + std::fmt::Debug, @@ -68,13 +72,30 @@ where for account in relay_accounts { let relay_account_balance_metric = FloatStorageValueMetric::new( - FreeAccountBalance:: { token_decimals, _phantom: Default::default() }, + AccountBalanceFromAccountInfo:: { token_decimals, _phantom: Default::default() }, client.clone(), C::account_info_storage_key(account.id()), format!("at_{}_relay_{}_balance", C::NAME, account.tag()), format!("Balance of the {} relay account at the {}", account.tag(), C::NAME), )?; relay_account_balance_metric.register_and_spawn(&metrics.registry)?; + + if let Some(relayers_pallet_name) = BC::WITH_CHAIN_RELAYERS_PALLET_NAME { + for lane in lanes { + let relay_account_reward_metric = FloatStorageValueMetric::new( + AccountBalance:: { token_decimals, _phantom: Default::default() }, + client.clone(), + bp_relayers::RelayerRewardsKeyProvider::, BalanceOf>::final_key( + relayers_pallet_name, + account.id(), + lane, + ), + format!("at_{}_relay_{}_reward_for_lane_{}_with_{}", C::NAME, account.tag(), hex::encode(lane.as_ref()), BC::NAME), + format!("Reward of the {} relay account for serving lane {:?} with {} at the {}", account.tag(), lane, BC::NAME, C::NAME), + )?; + relay_account_reward_metric.register_and_spawn(&metrics.registry)?; + } + } } Ok(()) @@ -82,12 +103,12 @@ where /// Adapter for `FloatStorageValueMetric` to decode account free balance. #[derive(Clone, Debug)] -struct FreeAccountBalance { +struct AccountBalanceFromAccountInfo { token_decimals: u32, _phantom: PhantomData, } -impl FloatStorageValue for FreeAccountBalance +impl FloatStorageValue for AccountBalanceFromAccountInfo where C: Chain, BalanceOf: Into, @@ -110,6 +131,34 @@ where } } +/// Adapter for `FloatStorageValueMetric` to decode account free balance. +#[derive(Clone, Debug)] +struct AccountBalance { + token_decimals: u32, + _phantom: PhantomData, +} + +impl FloatStorageValue for AccountBalance +where + C: Chain, + BalanceOf: Into, +{ + type Value = FixedU128; + + fn decode( + &self, + maybe_raw_value: Option, + ) -> Result, SubstrateError> { + maybe_raw_value + .map(|raw_value| { + BalanceOf::::decode(&mut &raw_value.0[..]) + .map_err(SubstrateError::ResponseParseFailed) + .map(|balance| convert_to_token_balance(balance.into(), self.token_decimals)) + }) + .transpose() + } +} + /// Convert from raw `u128` balance (nominated in smallest chain token units) to the float regular /// tokens value. fn convert_to_token_balance(balance: u128, token_decimals: u32) -> FixedU128 {