From 2a7e8c6fb39bbcc29d93b51ffa23ee4a0f44229b Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 9 May 2023 11:43:36 +0300 Subject: [PATCH] temporarily remove balance guard (#2121) --- bridges/relays/client-substrate/src/client.rs | 24 +-- bridges/relays/client-substrate/src/guard.rs | 185 +----------------- .../src/finality/guards.rs | 48 ----- .../lib-substrate-relay/src/finality/mod.rs | 1 - 4 files changed, 5 insertions(+), 253 deletions(-) delete mode 100644 bridges/relays/lib-substrate-relay/src/finality/guards.rs diff --git a/bridges/relays/client-substrate/src/client.rs b/bridges/relays/client-substrate/src/client.rs index 91ff7b9a214d4..a1c8a22be19d0 100644 --- a/bridges/relays/client-substrate/src/client.rs +++ b/bridges/relays/client-substrate/src/client.rs @@ -17,7 +17,7 @@ //! Substrate node client. use crate::{ - chain::{Chain, ChainWithBalances, ChainWithTransactions}, + chain::{Chain, ChainWithTransactions}, rpc::{ SubstrateAuthorClient, SubstrateChainClient, SubstrateFinalityClient, SubstrateFrameSystemClient, SubstrateStateClient, SubstrateSystemClient, @@ -31,14 +31,12 @@ use async_trait::async_trait; use bp_runtime::{HeaderIdProvider, StorageDoubleMapKeyProvider, StorageMapKeyProvider}; use codec::{Decode, Encode}; use frame_support::weights::Weight; -use frame_system::AccountInfo; use futures::{SinkExt, StreamExt}; use jsonrpsee::{ core::DeserializeOwned, ws_client::{WsClient as RpcClient, WsClientBuilder as RpcClientBuilder}, }; use num_traits::{Saturating, Zero}; -use pallet_balances::AccountData; use pallet_transaction_payment::RuntimeDispatchInfo; use relay_utils::{relay_loop::RECONNECT_DELAY, STALL_TIMEOUT}; use sp_core::{ @@ -424,26 +422,6 @@ impl Client { }) } - /// Return native tokens balance of the account. - pub async fn free_native_balance(&self, account: C::AccountId) -> Result - where - C: ChainWithBalances, - { - self.jsonrpsee_execute(move |client| async move { - let storage_key = C::account_info_storage_key(&account); - let encoded_account_data = - SubstrateStateClient::::storage(&*client, storage_key, None) - .await? - .ok_or(Error::AccountDoesNotExist)?; - let decoded_account_data = AccountInfo::>::decode( - &mut &encoded_account_data.0[..], - ) - .map_err(Error::ResponseParseFailed)?; - Ok(decoded_account_data.data.free) - }) - .await - } - /// Get the nonce of the given Substrate account. /// /// Note: It's the caller's responsibility to make sure `account` is a valid SS58 address. diff --git a/bridges/relays/client-substrate/src/guard.rs b/bridges/relays/client-substrate/src/guard.rs index 1afbf0d3d17ed..a4ed47424e818 100644 --- a/bridges/relays/client-substrate/src/guard.rs +++ b/bridges/relays/client-substrate/src/guard.rs @@ -20,10 +20,8 @@ use crate::{error::Error, Chain, ChainWithBalances, Client}; use async_trait::async_trait; -use num_traits::CheckedSub; use sp_version::RuntimeVersion; use std::{ - collections::VecDeque, fmt::Display, time::{Duration, Instant}, }; @@ -36,11 +34,6 @@ pub trait Environment: Send + Sync + 'static { /// Return current runtime version. async fn runtime_version(&mut self) -> Result; - /// Return free native balance of the account on the chain. - async fn free_native_balance( - &mut self, - account: C::AccountId, - ) -> Result; /// Return current time. fn now(&self) -> Instant { @@ -99,80 +92,6 @@ pub fn abort_on_spec_version_change( }); } -/// Abort if, during 24 hours, free balance of given account is decreased at least by given value. -/// Other components may increase (or decrease) balance of account and it WILL affect logic of the -/// guard. -pub fn abort_when_account_balance_decreased( - mut env: impl Environment, - account_id: C::AccountId, - maximal_decrease: C::Balance, -) { - const DAY: Duration = Duration::from_secs(60 * 60 * 24); - - async_std::task::spawn(async move { - log::info!( - target: "bridge-guard", - "Starting balance guard for {}/{:?}. Maximal decrease: {:?}", - C::NAME, - account_id, - maximal_decrease, - ); - - let mut balances = VecDeque::new(); - - loop { - let current_time = env.now(); - - // remember balances that are beyound 24h border - if let Some(time_border) = current_time.checked_sub(DAY) { - while balances.front().map(|(time, _)| *time < time_border).unwrap_or(false) { - balances.pop_front(); - } - } - - // read balance of the account - let current_balance = env.free_native_balance(account_id.clone()).await; - - // remember balance and check difference - match current_balance { - Ok(current_balance) => { - // remember balance - balances.push_back((current_time, current_balance)); - - // check if difference between current and oldest balance is too large - let (oldest_time, oldest_balance) = - balances.front().expect("pushed to queue couple of lines above; qed"); - let balances_difference = oldest_balance.checked_sub(¤t_balance); - if balances_difference > Some(maximal_decrease) { - log::error!( - target: "bridge-guard", - "Balance of {} account {:?} has decreased from {:?} to {:?} in {} minutes. Aborting relay", - C::NAME, - account_id, - oldest_balance, - current_balance, - current_time.duration_since(*oldest_time).as_secs() / 60, - ); - - env.abort().await; - } - }, - Err(error) => { - log::warn!( - target: "bridge-guard", - "Failed to read {} account {:?} balance: {}. Relay may need to be stopped manually", - C::NAME, - account_id, - error, - ); - }, - }; - - env.sleep(conditions_check_delay::()).await; - } - }); -} - /// Delay between conditions check. fn conditions_check_delay() -> Duration { C::AVERAGE_BLOCK_INTERVAL * (10 + rand::random::() % 10) @@ -185,13 +104,6 @@ impl Environment for Client { async fn runtime_version(&mut self) -> Result { Client::::runtime_version(self).await } - - async fn free_native_balance( - &mut self, - account: C::AccountId, - ) -> Result { - Client::::free_native_balance(self, account).await - } } #[cfg(test)] @@ -207,7 +119,6 @@ mod tests { struct TestEnvironment { runtime_version_rx: UnboundedReceiver, - free_native_balance_rx: UnboundedReceiver, slept_tx: UnboundedSender<()>, aborted_tx: UnboundedSender<()>, } @@ -220,10 +131,6 @@ mod tests { Ok(self.runtime_version_rx.next().await.unwrap_or_default()) } - async fn free_native_balance(&mut self, _account: u32) -> Result { - Ok(self.free_native_balance_rx.next().await.unwrap_or_default()) - } - async fn sleep(&mut self, _duration: Duration) { let _ = self.slept_tx.send(()).await; } @@ -240,17 +147,11 @@ mod tests { async_std::task::block_on(async { let ( (mut runtime_version_tx, runtime_version_rx), - (_free_native_balance_tx, free_native_balance_rx), (slept_tx, mut slept_rx), (aborted_tx, mut aborted_rx), - ) = (unbounded(), unbounded(), unbounded(), unbounded()); + ) = (unbounded(), unbounded(), unbounded()); abort_on_spec_version_change( - TestEnvironment { - runtime_version_rx, - free_native_balance_rx, - slept_tx, - aborted_tx, - }, + TestEnvironment { runtime_version_rx, slept_tx, aborted_tx }, 0, ); @@ -272,17 +173,11 @@ mod tests { async_std::task::block_on(async { let ( (mut runtime_version_tx, runtime_version_rx), - (_free_native_balance_tx, free_native_balance_rx), (slept_tx, mut slept_rx), (aborted_tx, mut aborted_rx), - ) = (unbounded(), unbounded(), unbounded(), unbounded()); + ) = (unbounded(), unbounded(), unbounded()); abort_on_spec_version_change( - TestEnvironment { - runtime_version_rx, - free_native_balance_rx, - slept_tx, - aborted_tx, - }, + TestEnvironment { runtime_version_rx, slept_tx, aborted_tx }, 42, ); @@ -298,76 +193,4 @@ mod tests { assert!(aborted_rx.next().now_or_never().is_none()); }); } - - #[test] - fn aborts_when_balance_is_too_low() { - async_std::task::block_on(async { - let ( - (_runtime_version_tx, runtime_version_rx), - (mut free_native_balance_tx, free_native_balance_rx), - (slept_tx, mut slept_rx), - (aborted_tx, mut aborted_rx), - ) = (unbounded(), unbounded(), unbounded(), unbounded()); - abort_when_account_balance_decreased( - TestEnvironment { - runtime_version_rx, - free_native_balance_rx, - slept_tx, - aborted_tx, - }, - 0, - 100, - ); - - // client responds with initial balance - free_native_balance_tx.send(1000).await.unwrap(); - - // then the guard sleeps - slept_rx.next().await; - - // and then client responds with updated balance, which is too low - free_native_balance_tx.send(899).await.unwrap(); - - // then the `abort` function is called - aborted_rx.next().await; - // and we do not reach next `sleep` function call - assert!(slept_rx.next().now_or_never().is_none()); - }); - } - - #[test] - fn does_not_aborts_when_balance_is_enough() { - async_std::task::block_on(async { - let ( - (_runtime_version_tx, runtime_version_rx), - (mut free_native_balance_tx, free_native_balance_rx), - (slept_tx, mut slept_rx), - (aborted_tx, mut aborted_rx), - ) = (unbounded(), unbounded(), unbounded(), unbounded()); - abort_when_account_balance_decreased( - TestEnvironment { - runtime_version_rx, - free_native_balance_rx, - slept_tx, - aborted_tx, - }, - 0, - 100, - ); - - // client responds with initial balance - free_native_balance_tx.send(1000).await.unwrap(); - - // then the guard sleeps - slept_rx.next().await; - - // and then client responds with updated balance, which is enough - free_native_balance_tx.send(950).await.unwrap(); - - // then the `sleep` function is called - slept_rx.next().await; - // and `abort` is not called - assert!(aborted_rx.next().now_or_never().is_none()); - }); - } } diff --git a/bridges/relays/lib-substrate-relay/src/finality/guards.rs b/bridges/relays/lib-substrate-relay/src/finality/guards.rs deleted file mode 100644 index 188a03733a382..0000000000000 --- a/bridges/relays/lib-substrate-relay/src/finality/guards.rs +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2019-2021 Parity Technologies (UK) Ltd. -// This file is part of Parity Bridges Common. - -// Parity Bridges Common is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Parity Bridges Common is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Parity Bridges Common. If not, see . - -//! Tools for starting guards of finality relays. - -use crate::TransactionParams; - -use relay_substrate_client::{ - AccountIdOf, AccountKeyPairOf, ChainWithBalances, ChainWithTransactions, -}; -use sp_core::Pair; - -/// Start finality relay guards. -pub async fn start( - target_client: &relay_substrate_client::Client, - transaction_params: &TransactionParams>, - enable_version_guard: bool, - maximal_balance_decrease_per_day: C::Balance, -) -> relay_substrate_client::Result<()> -where - AccountIdOf: From< as Pair>::Public>, -{ - if enable_version_guard { - relay_substrate_client::guard::abort_on_spec_version_change( - target_client.clone(), - target_client.simple_runtime_version().await?.spec_version, - ); - } - relay_substrate_client::guard::abort_when_account_balance_decreased( - target_client.clone(), - transaction_params.signer.public().into(), - maximal_balance_decrease_per_day, - ); - Ok(()) -} diff --git a/bridges/relays/lib-substrate-relay/src/finality/mod.rs b/bridges/relays/lib-substrate-relay/src/finality/mod.rs index b0f0ee4e52cc7..37ee61b48f1bd 100644 --- a/bridges/relays/lib-substrate-relay/src/finality/mod.rs +++ b/bridges/relays/lib-substrate-relay/src/finality/mod.rs @@ -39,7 +39,6 @@ use sp_core::Pair; use std::{fmt::Debug, marker::PhantomData}; pub mod engine; -pub mod guards; pub mod initialize; pub mod source; pub mod target;