From 8bad333ca377b177856f2d330ebdf8d2091d08cd Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Tue, 14 May 2024 12:53:48 +0300 Subject: [PATCH 1/2] Pass FinalityMode by value in executors Derive copy on the enum as well. --- networks/monza/monza-full-node/src/partial.rs | 2 +- networks/suzuka/suzuka-full-node/src/partial.rs | 2 +- .../execution/maptos/util/src/finality_mode.rs | 2 +- protocol-units/execution/monza/executor/src/lib.rs | 2 +- protocol-units/execution/monza/executor/src/v1.rs | 12 ++++++------ protocol-units/execution/suzuka/executor/src/lib.rs | 2 +- protocol-units/execution/suzuka/executor/src/v1.rs | 10 +++++----- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/networks/monza/monza-full-node/src/partial.rs b/networks/monza/monza-full-node/src/partial.rs index 95094b924..7fe1a3ec6 100644 --- a/networks/monza/monza-full-node/src/partial.rs +++ b/networks/monza/monza-full-node/src/partial.rs @@ -166,7 +166,7 @@ impl MonzaPartialNode { ); let block_id = executable_block.block_id; self.executor.execute_block( - &FinalityMode::Opt, + FinalityMode::Opt, executable_block ).await?; diff --git a/networks/suzuka/suzuka-full-node/src/partial.rs b/networks/suzuka/suzuka-full-node/src/partial.rs index 6d880cc49..4cbc4ca11 100644 --- a/networks/suzuka/suzuka-full-node/src/partial.rs +++ b/networks/suzuka/suzuka-full-node/src/partial.rs @@ -166,7 +166,7 @@ impl SuzukaPartialNode { ); let block_id = executable_block.block_id; self.executor.execute_block( - &FinalityMode::Opt, + FinalityMode::Opt, executable_block ).await?; diff --git a/protocol-units/execution/maptos/util/src/finality_mode.rs b/protocol-units/execution/maptos/util/src/finality_mode.rs index c909b1411..d7da735ee 100644 --- a/protocol-units/execution/maptos/util/src/finality_mode.rs +++ b/protocol-units/execution/maptos/util/src/finality_mode.rs @@ -1,4 +1,4 @@ -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum FinalityMode { Dyn, Opt, diff --git a/protocol-units/execution/monza/executor/src/lib.rs b/protocol-units/execution/monza/executor/src/lib.rs index dbd7e9b87..b1ba860bf 100644 --- a/protocol-units/execution/monza/executor/src/lib.rs +++ b/protocol-units/execution/monza/executor/src/lib.rs @@ -24,7 +24,7 @@ pub trait MonzaExecutor { /// Executes a block dynamically async fn execute_block( &self, - mode : &FinalityMode, + mode: FinalityMode, block: ExecutableBlock, ) -> Result<(), anyhow::Error>; diff --git a/protocol-units/execution/monza/executor/src/v1.rs b/protocol-units/execution/monza/executor/src/v1.rs index 43f3f3faf..f1ad41550 100644 --- a/protocol-units/execution/monza/executor/src/v1.rs +++ b/protocol-units/execution/monza/executor/src/v1.rs @@ -43,7 +43,7 @@ impl MonzaExecutor for MonzaExecutorV1 { /// Executes a block dynamically async fn execute_block( &self, - mode: &FinalityMode, + mode: FinalityMode, block: ExecutableBlock, ) -> Result<(), anyhow::Error> { match mode { @@ -131,15 +131,15 @@ mod tests { #[tokio::test] async fn test_execute_opt_block() -> Result<(), anyhow::Error> { - let (tx, rx) = async_channel::unbounded(); - let mut executor = MonzaExecutorV1::try_from_env(tx).await?; + let (tx, _rx) = async_channel::unbounded(); + let executor = MonzaExecutorV1::try_from_env(tx).await?; let block_id = HashValue::random(); let tx = SignatureVerifiedTransaction::Valid(Transaction::UserTransaction( create_signed_transaction(0), )); let txs = ExecutableTransactions::Unsharded(vec![tx]); let block = ExecutableBlock::new(block_id.clone(), txs); - executor.execute_block(&FinalityMode::Opt, block).await?; + executor.execute_block(FinalityMode::Opt, block).await?; Ok(()) } @@ -212,7 +212,7 @@ mod tests { SignatureVerifiedTransaction::Valid(Transaction::UserTransaction(received_transaction)); let txs = ExecutableTransactions::Unsharded(vec![tx]); let block = ExecutableBlock::new(block_id.clone(), txs); - executor.execute_block(&FinalityMode::Opt, block).await?; + executor.execute_block(FinalityMode::Opt, block).await?; services_handle.abort(); background_handle.abort(); @@ -274,7 +274,7 @@ mod tests { )); let txs = ExecutableTransactions::Unsharded(vec![tx]); let block = ExecutableBlock::new(block_id.clone(), txs); - executor.execute_block(&FinalityMode::Opt, block).await?; + executor.execute_block(FinalityMode::Opt, block).await?; blockheight += 1; committed_blocks.insert( diff --git a/protocol-units/execution/suzuka/executor/src/lib.rs b/protocol-units/execution/suzuka/executor/src/lib.rs index affbb9f26..8d3055844 100644 --- a/protocol-units/execution/suzuka/executor/src/lib.rs +++ b/protocol-units/execution/suzuka/executor/src/lib.rs @@ -24,7 +24,7 @@ pub trait SuzukaExecutor { /// Executes a block dynamically async fn execute_block( &self, - mode : &FinalityMode, + mode: FinalityMode, block: ExecutableBlock, ) -> Result<(), anyhow::Error>; diff --git a/protocol-units/execution/suzuka/executor/src/v1.rs b/protocol-units/execution/suzuka/executor/src/v1.rs index fc07e6e3c..59dba8f24 100644 --- a/protocol-units/execution/suzuka/executor/src/v1.rs +++ b/protocol-units/execution/suzuka/executor/src/v1.rs @@ -48,7 +48,7 @@ impl SuzukaExecutor for SuzukaExecutorV1 { /// Executes a block dynamically async fn execute_block( &self, - mode : &FinalityMode, + mode: FinalityMode, block: ExecutableBlock, ) -> Result<(), anyhow::Error> { @@ -137,15 +137,15 @@ mod opt_tests { #[tokio::test] async fn test_execute_opt_block() -> Result<(), anyhow::Error> { - let (tx, rx) = async_channel::unbounded(); - let mut executor = SuzukaExecutorV1::try_from_env(tx).await?; + let (tx, _rx) = async_channel::unbounded(); + let executor = SuzukaExecutorV1::try_from_env(tx).await?; let block_id = HashValue::random(); let tx = SignatureVerifiedTransaction::Valid(Transaction::UserTransaction( create_signed_transaction(0), )); let txs = ExecutableTransactions::Unsharded(vec![tx]); let block = ExecutableBlock::new(block_id.clone(), txs); - executor.execute_block(&FinalityMode::Opt, block).await?; + executor.execute_block(FinalityMode::Opt, block).await?; Ok(()) } @@ -228,7 +228,7 @@ mod opt_tests { )); let txs = ExecutableTransactions::Unsharded(vec![tx]); let block = ExecutableBlock::new(block_id.clone(), txs); - executor.execute_block(&FinalityMode::Opt, block).await?; + executor.execute_block(FinalityMode::Opt, block).await?; services_handle.abort(); background_handle.abort(); From dbc1f9e94cdc48161564146cbca57dbe2851e3b1 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Tue, 14 May 2024 13:26:53 +0300 Subject: [PATCH 2/2] simplify Executor APIs Change signatures of methods that don't need to be async or fallible. --- networks/monza/monza-full-node/src/partial.rs | 24 ++++++++++--------- .../suzuka/suzuka-full-node/src/partial.rs | 24 ++++++++++--------- .../maptos/opt-executor/src/executor.rs | 19 +++++++-------- .../execution/monza/executor/src/lib.rs | 16 ++++++------- .../execution/monza/executor/src/v1.rs | 19 +++++++-------- .../execution/suzuka/executor/src/lib.rs | 16 ++++++------- .../execution/suzuka/executor/src/v1.rs | 19 +++++++-------- 7 files changed, 67 insertions(+), 70 deletions(-) diff --git a/networks/monza/monza-full-node/src/partial.rs b/networks/monza/monza-full-node/src/partial.rs index 7fe1a3ec6..a262d11b2 100644 --- a/networks/monza/monza-full-node/src/partial.rs +++ b/networks/monza/monza-full-node/src/partial.rs @@ -41,16 +41,18 @@ impl MonzaPartialNode { } } - pub async fn bind_transaction_channel(&mut self) -> Result<(), anyhow::Error> { - self.executor.set_tx_channel(self.transaction_sender.clone()).await?; - Ok(()) - } - - pub async fn bound(executor : T, light_node_client: LightNodeServiceClient) -> Result { - let mut node = Self::new(executor, light_node_client); - node.bind_transaction_channel().await?; - Ok(node) - } + fn bind_transaction_channel(&mut self) { + self.executor.set_tx_channel(self.transaction_sender.clone()); + } + + pub fn bound( + executor: T, + light_node_client: LightNodeServiceClient, + ) -> Result { + let mut node = Self::new(executor, light_node_client); + node.bind_transaction_channel(); + Ok(node) + } pub async fn tick_write_transactions_to_da(&self) -> Result<(), anyhow::Error> { @@ -225,7 +227,7 @@ impl MonzaPartialNode { let executor = MonzaExecutorV1::try_from_env(tx).await.context( "Failed to get executor from environment" )?; - Self::bound(executor, light_node_client).await + Self::bound(executor, light_node_client) } } \ No newline at end of file diff --git a/networks/suzuka/suzuka-full-node/src/partial.rs b/networks/suzuka/suzuka-full-node/src/partial.rs index 4cbc4ca11..94534bcd9 100644 --- a/networks/suzuka/suzuka-full-node/src/partial.rs +++ b/networks/suzuka/suzuka-full-node/src/partial.rs @@ -41,16 +41,18 @@ impl SuzukaPartialNode { } } - pub async fn bind_transaction_channel(&mut self) -> Result<(), anyhow::Error> { - self.executor.set_tx_channel(self.transaction_sender.clone()).await?; - Ok(()) - } - - pub async fn bound(executor : T, light_node_client: LightNodeServiceClient) -> Result { - let mut node = Self::new(executor, light_node_client); - node.bind_transaction_channel().await?; - Ok(node) - } + fn bind_transaction_channel(&mut self) { + self.executor.set_tx_channel(self.transaction_sender.clone()); + } + + pub fn bound( + executor: T, + light_node_client: LightNodeServiceClient, + ) -> Result { + let mut node = Self::new(executor, light_node_client); + node.bind_transaction_channel(); + Ok(node) + } pub async fn tick_write_transactions_to_da(&self) -> Result<(), anyhow::Error> { @@ -225,7 +227,7 @@ impl SuzukaPartialNode { let executor = SuzukaExecutorV1::try_from_env(tx).await.context( "Failed to get executor from environment" )?; - Self::bound(executor, light_node_client).await + Self::bound(executor, light_node_client) } } \ No newline at end of file diff --git a/protocol-units/execution/maptos/opt-executor/src/executor.rs b/protocol-units/execution/maptos/opt-executor/src/executor.rs index a85866d24..d60126e8a 100644 --- a/protocol-units/execution/maptos/opt-executor/src/executor.rs +++ b/protocol-units/execution/maptos/opt-executor/src/executor.rs @@ -292,13 +292,12 @@ impl Executor { Ok(()) } - pub async fn try_get_context(&self) -> Result, anyhow::Error> { - Ok(self.context.clone()) + fn context(&self) -> Arc { + self.context.clone() } - pub async fn try_get_apis(&self) -> Result { - let context = self.try_get_context().await?; - Ok(get_apis(context)) + pub fn get_apis(&self) -> Apis { + get_apis(self.context()) } pub async fn run_service(&self) -> Result<(), anyhow::Error> { @@ -313,9 +312,7 @@ impl Executor { } - - let context = self.try_get_context().await?; - let api_service = get_api_service(context).server( + let api_service = get_api_service(self.context()).server( format!("http://{:?}", self.aptos_config.aptos_rest_listen_url) ); @@ -650,7 +647,7 @@ mod tests { executor.execute_block(block).await?; // Retrieve the executor's API interface and fetch the transaction by each hash. - let apis = executor.try_get_apis().await?; + let apis = executor.get_apis(); for hash in transaction_hashes { let _ = apis.transactions.get_transaction_by_hash_inner( &AcceptType::Bcs, @@ -743,7 +740,7 @@ mod tests { Ok(()) as Result<(), anyhow::Error> }); - let api = executor.try_get_apis().await?; + let api = executor.get_apis(); let user_transaction = create_signed_transaction(0, executor.aptos_config.chain_id.clone()); let comparison_user_transaction = user_transaction.clone(); let bcs_user_transaction = bcs::to_bytes(&user_transaction)?; @@ -774,7 +771,7 @@ mod tests { Ok(()) as Result<(), anyhow::Error> }); - let api = executor.try_get_apis().await?; + let api = executor.get_apis(); let mut user_transactions = BTreeSet::new(); let mut comparison_user_transactions = BTreeSet::new(); for _ in 0..25 { diff --git a/protocol-units/execution/monza/executor/src/lib.rs b/protocol-units/execution/monza/executor/src/lib.rs index b1ba860bf..2651799d3 100644 --- a/protocol-units/execution/monza/executor/src/lib.rs +++ b/protocol-units/execution/monza/executor/src/lib.rs @@ -28,14 +28,14 @@ pub trait MonzaExecutor { block: ExecutableBlock, ) -> Result<(), anyhow::Error>; - /// Sets the transaction channel. - async fn set_tx_channel(&mut self, tx_channel: Sender) -> Result<(), anyhow::Error>; - - /// Gets the dyn API. - async fn get_api( - &self, - mode : &FinalityMode, - ) -> Result; + /// Sets the transaction channel. + fn set_tx_channel( + &mut self, + tx_channel: Sender, + ); + + /// Gets the dyn API. + fn get_api(&self, mode: FinalityMode) -> Apis; /// Get block head height. async fn get_block_head_height(&self) -> Result; diff --git a/protocol-units/execution/monza/executor/src/v1.rs b/protocol-units/execution/monza/executor/src/v1.rs index f1ad41550..63210b81c 100644 --- a/protocol-units/execution/monza/executor/src/v1.rs +++ b/protocol-units/execution/monza/executor/src/v1.rs @@ -57,19 +57,18 @@ impl MonzaExecutor for MonzaExecutorV1 { } /// Sets the transaction channel. - async fn set_tx_channel( + fn set_tx_channel( &mut self, tx_channel: Sender, - ) -> Result<(), anyhow::Error> { + ) { self.transaction_channel = tx_channel; - Ok(()) } /// Gets the API. - async fn get_api(&self, _mode: &FinalityMode) -> Result { - match _mode { + fn get_api(&self, mode: FinalityMode) -> Apis { + match mode { FinalityMode::Dyn => unimplemented!(), - FinalityMode::Opt => Ok(self.executor.try_get_apis().await?), + FinalityMode::Opt => self.executor.get_apis(), FinalityMode::Fin => unimplemented!(), } } @@ -166,7 +165,7 @@ mod tests { let bcs_user_transaction = bcs::to_bytes(&user_transaction)?; let request = SubmitTransactionPost::Bcs(aptos_api::bcs_payload::Bcs(bcs_user_transaction)); - let api = executor.get_api(&FinalityMode::Opt).await?; + let api = executor.get_api(FinalityMode::Opt); api.transactions.submit_transaction(AcceptType::Bcs, request).await?; services_handle.abort(); @@ -200,7 +199,7 @@ mod tests { let bcs_user_transaction = bcs::to_bytes(&user_transaction)?; let request = SubmitTransactionPost::Bcs(aptos_api::bcs_payload::Bcs(bcs_user_transaction)); - let api = executor.get_api(&FinalityMode::Opt).await?; + let api = executor.get_api(FinalityMode::Opt); api.transactions.submit_transaction(AcceptType::Bcs, request).await?; let received_transaction = rx.recv().await?; @@ -261,7 +260,7 @@ mod tests { let request = SubmitTransactionPost::Bcs(aptos_api::bcs_payload::Bcs(bcs_user_transaction)); - let api = executor.get_api(&FinalityMode::Opt).await?; + let api = executor.get_api(FinalityMode::Opt); api.transactions.submit_transaction(AcceptType::Bcs, request).await?; let received_transaction = rx.recv().await?; @@ -369,7 +368,7 @@ mod tests { executor.execute_block(block).await?; // Retrieve the executor's API interface and fetch the transaction by each hash. - let apis = executor.try_get_apis().await?; + let apis = executor.get_apis(); for hash in transaction_hashes { let _ = apis .transactions diff --git a/protocol-units/execution/suzuka/executor/src/lib.rs b/protocol-units/execution/suzuka/executor/src/lib.rs index 8d3055844..bee4a7253 100644 --- a/protocol-units/execution/suzuka/executor/src/lib.rs +++ b/protocol-units/execution/suzuka/executor/src/lib.rs @@ -28,14 +28,14 @@ pub trait SuzukaExecutor { block: ExecutableBlock, ) -> Result<(), anyhow::Error>; - /// Sets the transaction channel. - async fn set_tx_channel(&mut self, tx_channel: Sender) -> Result<(), anyhow::Error>; - - /// Gets the dyn API. - async fn get_api( - &self, - mode : &FinalityMode, - ) -> Result; + /// Sets the transaction channel. + fn set_tx_channel( + &mut self, + tx_channel: Sender, + ); + + /// Gets the dyn API. + fn get_api(&self, mode: FinalityMode) -> Apis; /// Get block head height. async fn get_block_head_height(&self) -> Result; diff --git a/protocol-units/execution/suzuka/executor/src/v1.rs b/protocol-units/execution/suzuka/executor/src/v1.rs index 59dba8f24..a1c85b964 100644 --- a/protocol-units/execution/suzuka/executor/src/v1.rs +++ b/protocol-units/execution/suzuka/executor/src/v1.rs @@ -64,21 +64,18 @@ impl SuzukaExecutor for SuzukaExecutorV1 { } /// Sets the transaction channel. - async fn set_tx_channel(&mut self, tx_channel: Sender) -> Result<(), anyhow::Error> { + fn set_tx_channel(&mut self, tx_channel: Sender) { self.transaction_channel = tx_channel; - Ok(()) } /// Gets the API. - async fn get_api( + fn get_api( &self, - _mode : &FinalityMode, - ) -> Result { - match _mode { + mode: FinalityMode, + ) -> Apis { + match mode { FinalityMode::Dyn => unimplemented!(), - FinalityMode::Opt => { - Ok(self.executor.try_get_apis().await?) - }, + FinalityMode::Opt => self.executor.get_apis(), FinalityMode::Fin => unimplemented!(), } } @@ -177,7 +174,7 @@ mod opt_tests { let request = SubmitTransactionPost::Bcs( aptos_api::bcs_payload::Bcs(bcs_user_transaction) ); - let api = executor.get_api(&FinalityMode::Opt).await?; + let api = executor.get_api(FinalityMode::Opt); api.transactions.submit_transaction(AcceptType::Bcs, request).await?; services_handle.abort(); @@ -215,7 +212,7 @@ mod opt_tests { let request = SubmitTransactionPost::Bcs( aptos_api::bcs_payload::Bcs(bcs_user_transaction) ); - let api = executor.get_api(&FinalityMode::Opt).await?; + let api = executor.get_api(FinalityMode::Opt); api.transactions.submit_transaction(AcceptType::Bcs, request).await?; let received_transaction = rx.recv().await?;