From 343d21f17c9a9560fbab029f3403bcb68b56b23a Mon Sep 17 00:00:00 2001 From: Marcel Moura <5615598+marcelstanley@users.noreply.github.com> Date: Mon, 19 Aug 2024 17:30:21 -0300 Subject: [PATCH 1/8] chore(inspect-server): fix typos --- offchain/inspect-server/src/inspect.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/offchain/inspect-server/src/inspect.rs b/offchain/inspect-server/src/inspect.rs index b2b7bead7..8948d5c0d 100644 --- a/offchain/inspect-server/src/inspect.rs +++ b/offchain/inspect-server/src/inspect.rs @@ -70,7 +70,7 @@ fn respond( } } -/// Loop that answers requests comming from inspect_rx. +/// Loop that answers requests coming from inspect_rx. async fn handle_inspect( address: String, session_id: String, @@ -148,7 +148,7 @@ async fn handle_inspect_error( ) -> String { let mut message = status.message().to_string(); - // If the session was previously tainted, the server-manager replies if with code DataLoss. + // If the session was previously tainted, the server-manager replies it with code DataLoss. // Trying to recover the reason for the session tainted from the session's status. // If not available, we return the original status error message. if status.code() == Code::DataLoss { From 7503f7e99e80a8c58d2d1d902cd15f117f40abdf Mon Sep 17 00:00:00 2001 From: Marcel Moura <5615598+marcelstanley@users.noreply.github.com> Date: Mon, 19 Aug 2024 17:32:31 -0300 Subject: [PATCH 2/8] feat(advance-runner): log tainted session reason --- .../src/server_manager/facade.rs | 89 ++++++++++++++++++- 1 file changed, 87 insertions(+), 2 deletions(-) diff --git a/offchain/advance-runner/src/server_manager/facade.rs b/offchain/advance-runner/src/server_manager/facade.rs index c37375635..ce03dc7ba 100644 --- a/offchain/advance-runner/src/server_manager/facade.rs +++ b/offchain/advance-runner/src/server_manager/facade.rs @@ -180,7 +180,7 @@ impl ServerManagerFacade { ) -> Result> { tracing::trace!("sending advance-state input to server-manager"); - grpc_call!(self, advance_state, { + let response = grpc_call!(self, advance_state, { let input_metadata = InputMetadata { msg_sender: Some(Address { data: (*input_metadata.msg_sender.inner()).into(), @@ -197,7 +197,9 @@ impl ServerManagerFacade { input_metadata: Some(input_metadata), input_payload: input_payload.clone(), } - })?; + }); + + self.handle_advance_state_error(response).await?; tracing::trace!("waiting until the input is processed"); @@ -271,6 +273,77 @@ impl ServerManagerFacade { Ok(outputs) } + /// Handle response to advance-state request + #[tracing::instrument(level = "trace", skip_all)] + async fn handle_advance_state_error( + &mut self, + response: Result, + ) -> Result<()> { + if response.is_ok() { + return Ok(()); + } + + // Capture server-manager error for advance-state request + // to try to get the reason for a possible tainted session error. + let err = response.unwrap_err(); + + let err: ServerManagerError = match err { + ServerManagerError::MethodCallError { + method, + request_id, + source, + } => { + // Original error message to be reported by default. + let mut message = source.message().to_string(); + // The server-manager signals with code Dataloss when the session has been previously tainted. + if source.code() == tonic::Code::DataLoss { + // Recover the tainted session reason from the session's status. + let status_response = grpc_call!( + self, + get_session_status, + GetSessionStatusRequest { + session_id: self.config.session_id.clone(), + } + ); + + if status_response.is_err() { + match status_response.unwrap_err() { + ServerManagerError::MethodCallError { + method: _, + request_id: _, + source, + } => { + tracing::error!( + "get-session-status error: {:?}", + source.message() + ); + } + _ => (), + } + } else { + let status_response = status_response.unwrap(); + let taint_status = status_response.taint_status.clone(); + if let Some(taint_status) = taint_status { + message = format!( + "Server manager session was tainted: {} ({})", + taint_status.error_code, + taint_status.error_message + ); + tracing::error!(message); + } + } + } + ServerManagerError::MethodCallError { + method, + request_id, + source: tonic::Status::new(source.code(), message), + } + } + _ => err, + }; + Err(err) + } + /// Send a finish-epoch request to the server-manager /// Return the epoch claim and the proofs #[tracing::instrument(level = "trace", skip_all)] @@ -369,3 +442,15 @@ impl ServerManagerFacade { Err(ServerManagerError::PendingInputsExceededError {}) } } + +#[tracing::instrument(level = "trace", skip_all)] +fn get_advance_state_error_message(err: &ServerManagerError) -> Option { + match err { + ServerManagerError::MethodCallError { + method: _, + request_id: _, + source, + } => Some(source.message().to_string()), + _ => None, + } +} From b71972bbe320b4fb987a3f218f38aa15a7fed2ee Mon Sep 17 00:00:00 2001 From: Marcel Moura <5615598+marcelstanley@users.noreply.github.com> Date: Fri, 23 Aug 2024 09:49:14 -0300 Subject: [PATCH 3/8] chore(dispatcher): adjust logs While here, move block number info logging to machine drive --- offchain/dispatcher/src/dispatcher.rs | 11 +++++++---- offchain/dispatcher/src/drivers/context.rs | 5 +++++ offchain/dispatcher/src/drivers/machine.rs | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/offchain/dispatcher/src/dispatcher.rs b/offchain/dispatcher/src/dispatcher.rs index 33de10e6a..9246f8b20 100644 --- a/offchain/dispatcher/src/dispatcher.rs +++ b/offchain/dispatcher/src/dispatcher.rs @@ -6,7 +6,7 @@ use eth_state_fold_types::{Block, BlockStreamItem}; use rollups_events::DAppMetadata; use std::sync::Arc; use tokio_stream::StreamExt; -use tracing::{error, info, instrument, trace, warn}; +use tracing::{error, instrument, trace, warn}; use types::foldables::{InputBox, InputBoxInitialState}; use crate::{ @@ -87,13 +87,16 @@ pub async fn start( ) .await?; - trace!("Starting dispatcher..."); loop { match block_subscription.next().await { Some(Ok(BlockStreamItem::NewBlock(b))) => { // Normal operation, react on newest block. - info!("Received block number {}", b.number); - trace!("Block hash {:?}, parent: {:?}", b.hash, b.parent_hash); + trace!( + "Received block number {} and hash {:?}, parent: {:?}", + b.number, + b.hash, + b.parent_hash + ); process_block( &b, &state_server, diff --git a/offchain/dispatcher/src/drivers/context.rs b/offchain/dispatcher/src/drivers/context.rs index 09ff490a2..3ce10f54d 100644 --- a/offchain/dispatcher/src/drivers/context.rs +++ b/offchain/dispatcher/src/drivers/context.rs @@ -7,6 +7,7 @@ use crate::{ }; use rollups_events::DAppMetadata; +use tracing::info; use types::foldables::Input; #[derive(Debug)] @@ -146,6 +147,10 @@ impl Context { .inc(); self.last_finished_epoch = self.last_input_epoch; + info!( + "sent finish_epoch event for epoch {:?}", + self.last_finished_epoch + ); Ok(()) } } diff --git a/offchain/dispatcher/src/drivers/machine.rs b/offchain/dispatcher/src/drivers/machine.rs index 14389694c..c0e9b1213 100644 --- a/offchain/dispatcher/src/drivers/machine.rs +++ b/offchain/dispatcher/src/drivers/machine.rs @@ -37,7 +37,7 @@ impl MachineDriver { }; let block_number = block.number.as_u64(); - tracing::debug!("reacting to standalone block {}", block_number); + tracing::info!("received block {}", block_number); context.finish_epoch_if_needed(block_number, broker).await?; Ok(()) From fdef9192c2598e9c48ea82232ed83e21025ee8de Mon Sep 17 00:00:00 2001 From: Marcel Moura <5615598+marcelstanley@users.noreply.github.com> Date: Fri, 23 Aug 2024 09:50:33 -0300 Subject: [PATCH 4/8] chore(advance-runner): adjust log levels --- offchain/advance-runner/src/runner.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/offchain/advance-runner/src/runner.rs b/offchain/advance-runner/src/runner.rs index 69b7e89c5..f2a72d88c 100644 --- a/offchain/advance-runner/src/runner.rs +++ b/offchain/advance-runner/src/runner.rs @@ -127,14 +127,14 @@ impl Runner { .context(ProduceOutputsSnafu)?; tracing::trace!("produced outputs in broker stream"); - let dapp_address = rollups_claim.dapp_address.clone(); + let claim = rollups_claim.clone(); self.broker .produce_rollups_claim(rollups_claim) .await .context(ProduceClaimSnafu)?; tracing::info!( - "produced epoch claim in broker stream for dapp address {:?}", - dapp_address + "produced epoch claim in broker stream: {:?}", + claim ); } Err(source) => { From e78c86230d0891ce443f17caa7ff1b0848c434c8 Mon Sep 17 00:00:00 2001 From: Marcel Moura <5615598+marcelstanley@users.noreply.github.com> Date: Fri, 23 Aug 2024 09:51:01 -0300 Subject: [PATCH 5/8] chore(authority-claimer): adjust log levels --- offchain/authority-claimer/src/claimer.rs | 4 ++-- offchain/authority-claimer/src/listener.rs | 4 ++-- offchain/authority-claimer/src/sender.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/offchain/authority-claimer/src/claimer.rs b/offchain/authority-claimer/src/claimer.rs index ae9ec7b9c..2119c3ed5 100644 --- a/offchain/authority-claimer/src/claimer.rs +++ b/offchain/authority-claimer/src/claimer.rs @@ -96,7 +96,7 @@ where .listen() .await .context(BrokerListenerSnafu)?; - debug!("Got a claim from the broker: {:?}", rollups_claim); + info!("Received claim from the broker: {:?}", rollups_claim); let is_duplicated_rollups_claim = self .duplicate_checker @@ -108,7 +108,7 @@ where continue; } - info!("Sending a new rollups claim"); + info!("Sending a new rollups claim transaction"); self.transaction_sender = self .transaction_sender .send_rollups_claim_transaction(rollups_claim) diff --git a/offchain/authority-claimer/src/listener.rs b/offchain/authority-claimer/src/listener.rs index 992611908..3d20b4a1f 100644 --- a/offchain/authority-claimer/src/listener.rs +++ b/offchain/authority-claimer/src/listener.rs @@ -41,7 +41,7 @@ impl DefaultBrokerListener { chain_id: u64, dapp_address: Address, ) -> Result { - tracing::trace!("Connecting to the broker ({:?})", broker_config); + tracing::info!("Connecting to the broker ({:?})", broker_config); let broker = Broker::new(broker_config).await?; let dapp_metadata = DAppMetadata { chain_id, @@ -94,7 +94,7 @@ impl MultidappBrokerListener { broker_config: BrokerConfig, chain_id: u64, ) -> Result { - tracing::trace!( + tracing::info!( "Connecting to the broker ({:?}) on multidapp mode", broker_config ); diff --git a/offchain/authority-claimer/src/sender.rs b/offchain/authority-claimer/src/sender.rs index 428e45574..70997f921 100644 --- a/offchain/authority-claimer/src/sender.rs +++ b/offchain/authority-claimer/src/sender.rs @@ -260,7 +260,7 @@ impl TransactionSender for DefaultTransactionSender { dapp_address, }) .inc(); - trace!("Claim transaction confirmed: `{:?}`", receipt); + info!("Claim transaction confirmed: `{:?}`", receipt); Ok(Self { tx_manager, ..self }) } From 545e56e376f03563153c73c895fbb32b63cc2660 Mon Sep 17 00:00:00 2001 From: Renan Santos Date: Fri, 23 Aug 2024 11:33:26 -0300 Subject: [PATCH 6/8] fix(experimental): log only address list changes This applies only to the experimental multi-dapp claimer Co-authored-by: Marcel Moura <5615598+marcelstanley@users.noreply.github.com> --- offchain/authority-claimer/src/listener.rs | 23 ++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/offchain/authority-claimer/src/listener.rs b/offchain/authority-claimer/src/listener.rs index 3d20b4a1f..4ea45482e 100644 --- a/offchain/authority-claimer/src/listener.rs +++ b/offchain/authority-claimer/src/listener.rs @@ -7,7 +7,7 @@ use rollups_events::{ RollupsClaimsStream, INITIAL_ID, }; use snafu::ResultExt; -use std::{collections::HashMap, fmt::Debug}; +use std::{collections::HashMap, collections::HashSet, fmt::Debug}; /// The `BrokerListener` listens for new claims from the broker. #[async_trait] @@ -119,11 +119,22 @@ impl MultidappBrokerListener { // Gets the dapps from the broker. let dapps = self.broker.get_dapps().await.context(BrokerSnafu)?; assert!(!dapps.is_empty()); - tracing::info!( - "Got the following dapps from key \"{}\": {:?}", - rollups_events::DAPPS_KEY, - dapps - ); + { + // Logging if the dapps changed. + let old_dapps: HashSet
= self + .streams + .iter() + .map(|(stream, _)| stream.dapp_address.clone()) + .collect(); + let new_dapps = HashSet::from_iter(dapps.clone()); + if old_dapps != new_dapps { + tracing::info!( + "Updated list of dapp addresses from key \"{}\": {:?}", + rollups_events::DAPPS_KEY, + new_dapps + ); + } + } // Converts dapps to streams. let streams: Vec<_> = dapps From 931e258cb6784c409f6248d34ff2621c429102f6 Mon Sep 17 00:00:00 2001 From: Marcel Moura <5615598+marcelstanley@users.noreply.github.com> Date: Fri, 23 Aug 2024 14:55:20 -0300 Subject: [PATCH 7/8] fix(experimental): log invalid dapp addresses --- offchain/rollups-events/src/broker/mod.rs | 24 +++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/offchain/rollups-events/src/broker/mod.rs b/offchain/rollups-events/src/broker/mod.rs index 529d1d1ee..56e18ab51 100644 --- a/offchain/rollups-events/src/broker/mod.rs +++ b/offchain/rollups-events/src/broker/mod.rs @@ -376,12 +376,24 @@ impl Broker { let mut dapp_addresses: Vec
= vec![]; for value in reply { let normalized = value.to_lowercase(); - let dapp_address = Address::from_str(&normalized).unwrap(); - if dapp_addresses.contains(&dapp_address) { - let _: () = - self.connection.clone().srem(DAPPS_KEY, value).await?; - } else { - dapp_addresses.push(dapp_address); + let dapp_address = Address::from_str(&normalized); + match dapp_address { + Ok(dapp_address) => { + if dapp_addresses.contains(&dapp_address) { + let _: () = self + .connection + .clone() + .srem(DAPPS_KEY, value) + .await?; + } else { + dapp_addresses.push(dapp_address); + } + } + Err(message) => tracing::info!( + "Error while parsing DApp address {:?}: {}", + normalized, + message, + ), } } From 6762c6dcd7ce3e26c5dc86a1f3b8a56d1775201d Mon Sep 17 00:00:00 2001 From: Marcel Moura <5615598+marcelstanley@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:23:06 -0300 Subject: [PATCH 8/8] chore: update CHANGELOG --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 130710ab7..0a343c93f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,12 +12,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added support for the experimental multidapp claimer. - Added the `DAPP_CONTRACT_ADDRESS` environment variable to the `authority-claimer`. If let unset, the service instantiates the MultidappClaimer, that reads dapp addresses from Redis. +- Added `READER_MODE_ENABLED` environment variable to control whether the node generate claims or not. ### Changed - Redacted the contents of `CARTESI_EXPERIMENTAL_SUNODO_VALIDATOR_REDIS_ENDPOINT`. -- server-manager tainted session errors logged on inspect-server. -- Improved dispatcher logs. +- Logged server-manager tainted session errors on advance-runner and inspect-server. +- Adjusted dispatcher, advance-runner and authority-claimer logs. ## [1.5.0] 2024-07-22