From 9cd112fbe250b1c066029793934e799a444ece78 Mon Sep 17 00:00:00 2001 From: Roy Yang Date: Fri, 3 Nov 2023 03:51:26 +1300 Subject: [PATCH] feat: implement dry-run (#4155) Co-authored-by: Daniel Co-authored-by: dandanlen <3168260+dandanlen@users.noreply.github.com> --- api/bin/chainflip-cli/src/main.rs | 2 +- api/lib/src/lib.rs | 52 +++++----------- bouncer/shared/lp_api_test.ts | 5 +- .../client/base_rpc_api.rs | 6 ++ .../client/extrinsic_api/signed.rs | 42 +++++++++++++ .../signed/submission_watcher.rs | 62 ++++++++++++++++++- engine/src/state_chain_observer/client/mod.rs | 25 ++++++++ .../pallets/cf-account-roles/src/lib.rs | 1 + 8 files changed, 153 insertions(+), 42 deletions(-) diff --git a/api/bin/chainflip-cli/src/main.rs b/api/bin/chainflip-cli/src/main.rs index 1e8c61ff8d..b7f8e2ec84 100644 --- a/api/bin/chainflip-cli/src/main.rs +++ b/api/bin/chainflip-cli/src/main.rs @@ -27,7 +27,7 @@ async fn main() { std::process::exit(match run_cli().await { Ok(_) => 0, Err(err) => { - eprintln!("Error: {err:?}"); + eprintln!("Error: {err:#}"); 1 }, }) diff --git a/api/lib/src/lib.rs b/api/lib/src/lib.rs index 6b174b9226..3745b69376 100644 --- a/api/lib/src/lib.rs +++ b/api/lib/src/lib.rs @@ -139,26 +139,12 @@ impl StateChainApi { } } -#[async_trait] -impl< - SignedExtrinsicClient: Send + Sync + 'static + SignedExtrinsicApi, - RawRpcClient: Send + Sync + 'static + RawRpcApi, - > OperatorApi for StateChainClient> -{ - async fn dry_run( - &self, - _call: RuntimeCall, - _at: Option, - ) -> Result { - // TODO: PRO-917 fix dry run - Ok(Bytes::from(vec![])) - } -} - #[async_trait] impl GovernanceApi for StateChainClient {} #[async_trait] impl BrokerApi for StateChainClient {} +#[async_trait] +impl OperatorApi for StateChainClient {} #[async_trait] pub trait OperatorApi: SignedExtrinsicApi + RotateSessionKeysApi + AuctionPhaseApi { @@ -169,9 +155,9 @@ pub trait OperatorApi: SignedExtrinsicApi + RotateSessionKeysApi + AuctionPhaseA executor: Option, ) -> Result { let call = RuntimeCall::from(pallet_cf_funding::Call::redeem { amount, address, executor }); - self.dry_run(call.clone(), None).await?; - let (tx_hash, ..) = self.submit_signed_extrinsic(call).await.until_in_block().await?; + let (tx_hash, ..) = + self.submit_signed_extrinsic_with_dry_run(call).await?.until_in_block().await?; Ok(tx_hash) } @@ -209,11 +195,9 @@ pub trait OperatorApi: SignedExtrinsicApi + RotateSessionKeysApi + AuctionPhaseA AccountRole::None => bail!("Cannot register account role None"), }; - let _ = self.dry_run(call.clone(), None).await?; - let (tx_hash, ..) = self - .submit_signed_extrinsic(call) - .await + .submit_signed_extrinsic_with_dry_run(call) + .await? .until_in_block() .await .context("Could not register account role for account")?; @@ -282,12 +266,6 @@ pub trait OperatorApi: SignedExtrinsicApi + RotateSessionKeysApi + AuctionPhaseA println!("Vanity name set at tx {tx_hash:#x}."); Ok(()) } - - async fn dry_run( - &self, - call: RuntimeCall, - at: Option, - ) -> Result; } #[async_trait] @@ -327,14 +305,16 @@ pub trait BrokerApi: SignedExtrinsicApi { channel_metadata: Option, ) -> Result { let (_tx_hash, events, header, ..) = self - .submit_signed_extrinsic(pallet_cf_swapping::Call::request_swap_deposit_address { - source_asset, - destination_asset, - destination_address, - broker_commission_bps, - channel_metadata, - }) - .await + .submit_signed_extrinsic_with_dry_run( + pallet_cf_swapping::Call::request_swap_deposit_address { + source_asset, + destination_asset, + destination_address, + broker_commission_bps, + channel_metadata, + }, + ) + .await? .until_in_block() .await?; diff --git a/bouncer/shared/lp_api_test.ts b/bouncer/shared/lp_api_test.ts index 99c9695964..ece2edae03 100644 --- a/bouncer/shared/lp_api_test.ts +++ b/bouncer/shared/lp_api_test.ts @@ -125,8 +125,9 @@ async function testRegisterWithExistingLpAccount() { // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error: any) { // This account is already registered, so the command will fail. - if (!error.message.includes('Could not register account role for account')) { - throw new Error(`Unexpected lp_register_account error: ${JSON.stringify(error)}`); + // This message is from the `AccountRoleAlreadyRegistered` pallet error. + if (!error.message.includes('The account already has a registered role')) { + throw new Error(`Unexpected lp_register_account error: ${error}`); } } } diff --git a/engine/src/state_chain_observer/client/base_rpc_api.rs b/engine/src/state_chain_observer/client/base_rpc_api.rs index 87afc4f830..4b4b9b5bb1 100644 --- a/engine/src/state_chain_observer/client/base_rpc_api.rs +++ b/engine/src/state_chain_observer/client/base_rpc_api.rs @@ -122,6 +122,8 @@ pub trait BaseRpcApi { async fn latest_finalized_block_hash(&self) -> RpcResult; + async fn latest_unfinalized_block_hash(&self) -> RpcResult; + async fn subscribe_finalized_block_headers( &self, ) -> RpcResult>>; @@ -217,6 +219,10 @@ impl BaseRpcApi for BaseRpcClient RpcResult { + Ok(unwrap_value(self.raw_rpc_client.block_hash(None).await?).expect(SUBSTRATE_BEHAVIOUR)) + } + async fn latest_finalized_block_hash(&self) -> RpcResult { self.raw_rpc_client.finalized_head().await } diff --git a/engine/src/state_chain_observer/client/extrinsic_api/signed.rs b/engine/src/state_chain_observer/client/extrinsic_api/signed.rs index 4d1b6e9f87..d235711f46 100644 --- a/engine/src/state_chain_observer/client/extrinsic_api/signed.rs +++ b/engine/src/state_chain_observer/client/extrinsic_api/signed.rs @@ -94,6 +94,18 @@ pub trait SignedExtrinsicApi { + Sync + 'static; + async fn submit_signed_extrinsic_with_dry_run( + &self, + call: Call, + ) -> Result<(H256, (Self::UntilInBlockFuture, Self::UntilFinalizedFuture))> + where + Call: Into + + Clone + + std::fmt::Debug + + Send + + Sync + + 'static; + async fn finalize_signed_extrinsic( &self, call: Call, @@ -115,6 +127,7 @@ pub struct SignedExtrinsicClient { oneshot::Sender, submission_watcher::RequestStrategy, )>, + dry_run_sender: mpsc::Sender<(state_chain_runtime::RuntimeCall, oneshot::Sender>)>, _task_handle: ScopedJoinHandle<()>, } @@ -136,6 +149,7 @@ impl SignedExtrinsicClient { const REQUEST_BUFFER: usize = 16; let (request_sender, mut request_receiver) = mpsc::channel(REQUEST_BUFFER); + let (dry_run_sender, mut dry_run_receiver) = mpsc::channel(REQUEST_BUFFER); let account_nonce = { loop { @@ -177,6 +191,7 @@ impl SignedExtrinsicClient { Ok(Self { account_id: signer.account_id.clone(), request_sender, + dry_run_sender, _task_handle: scope.spawn_with_handle({ let mut state_chain_stream = state_chain_stream.clone(); @@ -199,6 +214,9 @@ impl SignedExtrinsicClient { if let Some((call, until_in_block_sender, until_finalized_sender, strategy)) = request_receiver.recv() => { submission_watcher.new_request(&mut requests, call, until_in_block_sender, until_finalized_sender, strategy).await?; } else break Ok(()), + if let Some((call, result_sender)) = dry_run_receiver.recv() => { + let _ = result_sender.send(submission_watcher.dry_run_extrinsic(call).await.map_err(Into::into)); + } else break Ok(()), let submission_details = submission_watcher.watch_for_submission_in_block() => { submission_watcher.on_submission_in_block(&mut requests, submission_details).await?; }, @@ -258,6 +276,30 @@ impl SignedExtrinsicApi for SignedExtrinsicClient { ) } + /// Dry run the call, and only submit the extrinsic onto the chain + /// if dry-run returns Ok(()) + async fn submit_signed_extrinsic_with_dry_run( + &self, + call: Call, + ) -> Result<(H256, (Self::UntilInBlockFuture, Self::UntilFinalizedFuture))> + where + Call: Into + + Clone + + std::fmt::Debug + + Send + + Sync + + 'static, + { + let _ = send_request(&self.dry_run_sender, |result_sender| { + (call.clone().into(), result_sender) + }) + .await + .await + .expect(OR_CANCEL)?; + + Ok(self.submit_signed_extrinsic(call.into()).await) + } + async fn finalize_signed_extrinsic( &self, call: Call, diff --git a/engine/src/state_chain_observer/client/extrinsic_api/signed/submission_watcher.rs b/engine/src/state_chain_observer/client/extrinsic_api/signed/submission_watcher.rs index c998093519..3e16dc184d 100644 --- a/engine/src/state_chain_observer/client/extrinsic_api/signed/submission_watcher.rs +++ b/engine/src/state_chain_observer/client/extrinsic_api/signed/submission_watcher.rs @@ -5,11 +5,15 @@ use std::{ use anyhow::{anyhow, Result}; use cf_primitives::SemVer; +use codec::{Decode, Encode}; use frame_support::{dispatch::DispatchInfo, pallet_prelude::InvalidTransaction}; use itertools::Itertools; use sc_transaction_pool_api::TransactionStatus; use sp_core::H256; -use sp_runtime::{traits::Hash, MultiAddress}; +use sp_runtime::{ + traits::Hash, transaction_validity::TransactionValidityError, ApplyExtrinsicResult, + MultiAddress, +}; use state_chain_runtime::{BlockNumber, Nonce, UncheckedExtrinsic}; use thiserror::Error; use tokio::sync::oneshot; @@ -43,6 +47,18 @@ pub enum ExtrinsicError { Dispatch(DispatchError), } +#[derive(Error, Debug)] +pub enum DryRunError { + #[error(transparent)] + RpcCallError(#[from] jsonrpsee::core::Error), + #[error("Unable to decode dry_run RPC result: {0}")] + CannotDecodeReply(#[from] codec::Error), + #[error("The transaction is invalid: {0}")] + InvalidTransaction(#[from] TransactionValidityError), + #[error("The transaction failed: {0}")] + Dispatch(#[from] DispatchError), +} + pub type ExtrinsicResult = Result< (H256, Vec, state_chain_runtime::Header, DispatchInfo), ExtrinsicError, @@ -166,6 +182,24 @@ impl<'a, 'env, BaseRpcClient: base_rpc_api::BaseRpcApi + Send + Sync + 'static> ) } + fn build_and_sign_extrinsic( + &self, + call: state_chain_runtime::RuntimeCall, + nonce: Nonce, + ) -> state_chain_runtime::UncheckedExtrinsic { + self.signer + .new_signed_extrinsic( + call.clone(), + &self.runtime_version, + self.genesis_hash, + self.finalized_block_hash, + self.finalized_block_number, + self.extrinsic_lifetime, + nonce, + ) + .0 + } + async fn submit_extrinsic_at_nonce( &mut self, request: &mut Request, @@ -184,9 +218,8 @@ impl<'a, 'env, BaseRpcClient: base_rpc_api::BaseRpcApi + Send + Sync + 'static> assert!(lifetime.contains(&(self.finalized_block_number + 1))); let tx_hash: H256 = { - use sp_core::{blake2_256, Encode}; let encoded = signed_extrinsic.encode(); - blake2_256(&encoded).into() + sp_core::blake2_256(&encoded).into() }; match self.base_rpc_client.submit_and_watch_extrinsic(signed_extrinsic).await { @@ -271,6 +304,29 @@ impl<'a, 'env, BaseRpcClient: base_rpc_api::BaseRpcApi + Send + Sync + 'static> }) } + pub async fn dry_run_extrinsic( + &mut self, + call: state_chain_runtime::RuntimeCall, + ) -> Result<(), DryRunError> { + // Use the nonce from the latest unfinalized block. + let hash = self.base_rpc_client.latest_unfinalized_block_hash().await?; + let nonce = self + .base_rpc_client + .storage_map_entry::>( + hash, + &self.signer.account_id, + ) + .await? + .nonce; + let uxt = self.build_and_sign_extrinsic(call.clone(), nonce); + let result_bytes = self.base_rpc_client.dry_run(Encode::encode(&uxt).into(), None).await?; + let dry_run_result: ApplyExtrinsicResult = Decode::decode(&mut &*result_bytes)?; + + debug!(target: "state_chain_client", "Dry run completed. Result: {:?}", &dry_run_result); + + Ok(dry_run_result?.map_err(|e| self.error_decoder.decode_dispatch_error(e))?) + } + pub async fn new_request( &mut self, requests: &mut BTreeMap, diff --git a/engine/src/state_chain_observer/client/mod.rs b/engine/src/state_chain_observer/client/mod.rs index 2e9a25b905..7ef05c3818 100644 --- a/engine/src/state_chain_observer/client/mod.rs +++ b/engine/src/state_chain_observer/client/mod.rs @@ -543,6 +543,22 @@ impl< self.signed_extrinsic_client.account_id() } + /// Do a dry run of the extrinsic first, only submit if Ok(()) + async fn submit_signed_extrinsic_with_dry_run( + &self, + call: Call, + ) -> anyhow::Result<(H256, (Self::UntilInBlockFuture, Self::UntilFinalizedFuture))> + where + Call: Into + + Clone + + std::fmt::Debug + + Send + + Sync + + 'static, + { + self.signed_extrinsic_client.submit_signed_extrinsic_with_dry_run(call).await + } + /// Submit an signed extrinsic, returning the hash of the submission async fn submit_signed_extrinsic( &self, @@ -647,6 +663,15 @@ pub mod mocks { + Sync + 'static; + async fn submit_signed_extrinsic_with_dry_run(&self, call: Call) -> anyhow::Result<(H256, (::UntilInBlockFuture, ::UntilFinalizedFuture))> + where + Call: Into + + Clone + + std::fmt::Debug + + Send + + Sync + + 'static; + async fn finalize_signed_extrinsic(&self, call: Call) -> (::UntilInBlockFuture, ::UntilFinalizedFuture) where Call: Into diff --git a/state-chain/pallets/cf-account-roles/src/lib.rs b/state-chain/pallets/cf-account-roles/src/lib.rs index 4d82f2e902..919fd4ae82 100644 --- a/state-chain/pallets/cf-account-roles/src/lib.rs +++ b/state-chain/pallets/cf-account-roles/src/lib.rs @@ -54,6 +54,7 @@ pub mod pallet { pub enum Error { UnknownAccount, AccountNotInitialised, + /// The account already has a registered role. AccountRoleAlreadyRegistered, /// Initially when swapping features are deployed to the chain, they will be disabled. SwappingDisabled,