fix: asb tracing, remove disable-timespamps option

swap/src/asb/command.rs

@@ -19,7 +19,6 @@ where
     let args = RawArguments::from_clap(&matches);
 
     let json = args.json;
-    let disable_timestamp = args.disable_timestamp;
     let testnet = args.testnet;
     let config = args.config;
     let command: RawCommand = args.cmd;
@@ -28,7 +27,6 @@ where
         RawCommand::Start { resume_only } => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::Start { resume_only },
@@ -36,7 +34,6 @@ where
         RawCommand::History => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::History,
@@ -49,7 +46,6 @@ where
         } => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::Logs {
@@ -62,7 +58,6 @@ where
         RawCommand::WithdrawBtc { amount, address } => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::WithdrawBtc {
@@ -73,7 +68,6 @@ where
         RawCommand::Balance => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::Balance,
@@ -81,7 +75,6 @@ where
         RawCommand::Config => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::Config,
@@ -89,7 +82,6 @@ where
         RawCommand::ExportBitcoinWallet => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::ExportBitcoinWallet,
@@ -100,7 +92,6 @@ where
         }) => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::Redeem {
@@ -114,7 +105,6 @@ where
         }) => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::Cancel { swap_id },
@@ -124,7 +114,6 @@ where
         }) => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::Refund { swap_id },
@@ -134,7 +123,6 @@ where
         }) => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::Punish { swap_id },
@@ -142,7 +130,6 @@ where
         RawCommand::ManualRecovery(ManualRecovery::SafelyAbort { swap_id }) => Arguments {
             testnet,
             json,
-            disable_timestamp,
             config_path: config_path(config, testnet)?,
             env_config: env_config(testnet),
             cmd: Command::SafelyAbort { swap_id },
@@ -202,7 +189,6 @@ pub struct BitcoinAddressNetworkMismatch {
 pub struct Arguments {
     pub testnet: bool,
     pub json: bool,
-    pub disable_timestamp: bool,
     pub config_path: PathBuf,
     pub env_config: env::Config,
     pub cmd: Command,
@@ -414,7 +400,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: false,
             json: false,
-            disable_timestamp: false,
             config_path: default_mainnet_conf_path,
             env_config: mainnet_env_config,
             cmd: Command::Start { resume_only: false },
@@ -432,7 +417,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: false,
             json: false,
-            disable_timestamp: false,
             config_path: default_mainnet_conf_path,
             env_config: mainnet_env_config,
             cmd: Command::History,
@@ -450,7 +434,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: false,
             json: false,
-            disable_timestamp: false,
             config_path: default_mainnet_conf_path,
             env_config: mainnet_env_config,
             cmd: Command::Balance,
@@ -472,7 +455,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: false,
             json: false,
-            disable_timestamp: false,
             config_path: default_mainnet_conf_path,
             env_config: mainnet_env_config,
             cmd: Command::WithdrawBtc {
@@ -499,7 +481,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: false,
             json: false,
-            disable_timestamp: false,
             config_path: default_mainnet_conf_path,
             env_config: mainnet_env_config,
             cmd: Command::Cancel {
@@ -525,7 +506,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: false,
             json: false,
-            disable_timestamp: false,
             config_path: default_mainnet_conf_path,
             env_config: mainnet_env_config,
             cmd: Command::Refund {
@@ -551,7 +531,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: false,
             json: false,
-            disable_timestamp: false,
             config_path: default_mainnet_conf_path,
             env_config: mainnet_env_config,
             cmd: Command::Punish {
@@ -577,7 +556,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: false,
             json: false,
-            disable_timestamp: false,
             config_path: default_mainnet_conf_path,
             env_config: mainnet_env_config,
             cmd: Command::SafelyAbort {
@@ -597,7 +575,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: true,
             json: false,
-            disable_timestamp: false,
             config_path: default_testnet_conf_path,
             env_config: testnet_env_config,
             cmd: Command::Start { resume_only: false },
@@ -615,7 +592,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: true,
             json: false,
-            disable_timestamp: false,
             config_path: default_testnet_conf_path,
             env_config: testnet_env_config,
             cmd: Command::History,
@@ -633,7 +609,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: true,
             json: false,
-            disable_timestamp: false,
             config_path: default_testnet_conf_path,
             env_config: testnet_env_config,
             cmd: Command::Balance,
@@ -657,7 +632,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: true,
             json: false,
-            disable_timestamp: false,
             config_path: default_testnet_conf_path,
             env_config: testnet_env_config,
             cmd: Command::WithdrawBtc {
@@ -684,7 +658,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: true,
             json: false,
-            disable_timestamp: false,
             config_path: default_testnet_conf_path,
             env_config: testnet_env_config,
             cmd: Command::Cancel {
@@ -711,7 +684,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: true,
             json: false,
-            disable_timestamp: false,
             config_path: default_testnet_conf_path,
             env_config: testnet_env_config,
             cmd: Command::Refund {
@@ -738,7 +710,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: true,
             json: false,
-            disable_timestamp: false,
             config_path: default_testnet_conf_path,
             env_config: testnet_env_config,
             cmd: Command::Punish {
@@ -765,7 +736,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: true,
             json: false,
-            disable_timestamp: false,
             config_path: default_testnet_conf_path,
             env_config: testnet_env_config,
             cmd: Command::SafelyAbort {
@@ -785,7 +755,6 @@ mod tests {
         let expected_args = Arguments {
             testnet: false,
             json: false,
-            disable_timestamp: true,
             config_path: default_mainnet_conf_path,
             env_config: mainnet_env_config,
             cmd: Command::Start { resume_only: false },

swap/src/asb/tracing.rs

@@ -5,75 +5,57 @@ use tracing_subscriber::filter::LevelFilter;
 use tracing_subscriber::fmt::time::UtcTime;
 use tracing_subscriber::layer::SubscriberExt;
 use tracing_subscriber::fmt;
+use tracing_subscriber::util::SubscriberInitExt;
 
-
+/// Output formats for logging messages.
+pub enum Format {
+    /// Standard, human readable format.
+    Raw,
+    /// Machine readable format.
+    Json,
+}
 
 pub fn init(
     level: LevelFilter,
-    json_format: bool,
-    timestamp: bool,
+    format: Format,
     dir: impl AsRef<Path>,
 ) -> Result<()> {
     if level == LevelFilter::OFF {
         return Ok(());
     }
 
-    let is_terminal = atty::is(atty::Stream::Stderr);
-
-    // File logger will always write in JSON format and with timestamps
+    // file logger will always write in JSON format and with timestamps
     let file_appender = tracing_appender::rolling::never(dir.as_ref(), "swap-all.log");
 
     let file_layer = fmt::layer()
-    .with_writer(std::io::stdout)
-    .with_ansi(false)
-    .with_timer(UtcTime::rfc_3339())
-    .with_target(false)
-    .json();
+        .with_writer(file_appender)
+        .with_ansi(false)
+        .with_timer(UtcTime::rfc_3339())
+        .with_target(false)
+        .json();
 
-    // Terminal logger
-    let terminal_layer_base = fmt::layer() 
+    // terminal logger
+    let is_terminal = atty::is(atty::Stream::Stderr);
+    let terminal_layer = fmt::layer() 
         .with_writer(std::io::stdout)
         .with_ansi(is_terminal)
         .with_timer(UtcTime::rfc_3339())
         .with_target(false);
 
-    // Since tracing is stupid, and makes each option return a different type
-    // but also doesn't allow dynamic dispatch we have to use this beauty
-    let (
-        a,
-        b, 
-        c, 
-        d
-    ) = match (json_format, timestamp) {
-        (true, true) => (Some(terminal_layer_base.json()), None, None, None),
-        (true, false) => (None, Some(terminal_layer_base.json().without_time()), None, None),
-        (false, true) => (None, None, Some(terminal_layer_base), None),
-        (false, false) => (None, None, None, Some(terminal_layer_base.without_time())),
-    };
-
-    let combined_subscriber = tracing_subscriber::registry()
-        .with(file_layer)
-        .with(a);
-
-    combined_subscriber.init();
+    // combine the layers and start logging, format with json if specified 
+    if let Format::Json = format {
+        tracing_subscriber::registry()
+            .with(file_layer)
+            .with(terminal_layer.json())
+            .init();
+    } else {
+        tracing_subscriber::registry()
+            .with(file_layer)
+            .with(terminal_layer)
+            .init();
+    }
     
-    // let builder = FmtSubscriber::builder()
-    //     .with_env_filter(format!("asb={},swap={}", level, level))
-    //     .with_writer(async_file_appender.and(std::io::stderr))
-    //     .with_ansi(is_terminal)
-    //     .with_timer(UtcTime::rfc_3339())
-    //     .with_target(false);
-
-    
-
-
-    // match (json_format, timestamp) {
-    //     (true, true) => builder.json().init(),
-    //     (true, false) => builder.json().without_time().init(),
-    //     (false, true) => builder.init(),
-    //     (false, false) => builder.without_time().init(),
-    // }
-
+    // now we can use the tracing macros to log messages
     tracing::info!(%level, "Initialized tracing");
 
     Ok(())

swap/src/bin/asb.rs

@@ -18,6 +18,7 @@ use libp2p::core::multiaddr::Protocol;
 use libp2p::core::Multiaddr;
 use libp2p::swarm::AddressScore;
 use libp2p::Swarm;
+use swap::asb::tracing::Format;
 use std::convert::TryInto;
 use std::fs::read_dir;
 use std::net::{IpAddr, Ipv4Addr, SocketAddr};
@@ -47,39 +48,39 @@ const DEFAULT_WALLET_NAME: &str = "asb-wallet";
 
 #[tokio::main]
 async fn main() -> Result<()> {
+    // parse cli arguments
     let Arguments {
         testnet,
         json,
-        disable_timestamp,
         config_path,
         env_config,
         cmd,
     } = match parse_args(env::args_os()) {
         Ok(args) => args,
         Err(e) => {
+            // make sure to display the clap error message it exists
             if let Some(clap_err) = e.downcast_ref::<clap::Error>() {
-                match clap_err.kind {
-                    ErrorKind::HelpDisplayed | ErrorKind::VersionDisplayed => {
-                        println!("{}", clap_err.message);
-                        std::process::exit(0);
-                    }
-                    _ => {
-                        bail!(e);
-                    }
+                if let ErrorKind::HelpDisplayed | ErrorKind::VersionDisplayed = clap_err.kind {
+                    println!("{}", clap_err.message);
+                    std::process::exit(0);
                 }
             }
             bail!(e);
         }
     };
 
+    // warn if we're not on the latest version
     if let Err(e) = check_latest_version(env!("CARGO_PKG_VERSION")).await {
         eprintln!("{}", e);
     }
 
+    // initialize tracing
+    let format = if json { Format::Json } else { Format::Raw };
     let log_dir = system_data_dir()?.join("logs");
-    asb::tracing::init(LevelFilter::DEBUG, json, !disable_timestamp, log_dir)
+    asb::tracing::init(LevelFilter::DEBUG, format, log_dir)
         .expect("initialize tracing");
 
+    // read config from the specified path
     let config = match read_config(config_path.clone())? {
         Ok(config) => config,
         Err(ConfigNotInitialized {}) => {
@@ -88,6 +89,7 @@ async fn main() -> Result<()> {
         }
     };
 
+    // check for conflicting env / config values
     if config.monero.network != env_config.monero_network {
         bail!(format!(
             "Expected monero network in config file to be {:?} but was {:?}",
@@ -114,6 +116,7 @@ async fn main() -> Result<()> {
             rendezvous_addrs.sort();
             rendezvous_addrs.dedup();
             let new_len = rendezvous_addrs.len();
+
             if new_len < prev_len {
                 tracing::warn!(
                     "`rendezvous_point` config has {} duplicate entries, they are being ignored.",
@@ -121,9 +124,12 @@ async fn main() -> Result<()> {
                 );
             }
 
+            // initialize monero wallet
             let monero_wallet = init_monero_wallet(&config, env_config).await?;
             let monero_address = monero_wallet.get_main_address();
             tracing::info!(%monero_address, "Monero wallet address");
+
+            // check monero balance
             let monero = monero_wallet.get_balance().await?;
             match (monero.balance, monero.unlocked_balance) {
                 (0, _) => {
@@ -146,6 +152,7 @@ async fn main() -> Result<()> {
                 }
             }
 
+            // init bitcoin wallet
             let bitcoin_wallet = init_bitcoin_wallet(&config, &seed, env_config).await?;
             let bitcoin_balance = bitcoin_wallet.balance().await?;
             tracing::info!(%bitcoin_balance, "Bitcoin wallet balance");

swap/src/common.rs

@@ -44,7 +44,7 @@ fn is_latest_version(current: &str, latest: &str) -> bool {
 macro_rules! regex_find_placeholders {
     ($pattern:expr, $create_placeholder:expr, $replacements:expr, $input:expr) => {{
         // compile the regex pattern
-        let regex = once_cell::sync::Lazy::new(|| {
+        static REGEX: once_cell::sync::Lazy<regex::Regex> = once_cell::sync::Lazy::new(|| {
             regex::Regex::new($pattern).expect("invalid regex pattern")
         });
 
@@ -53,7 +53,7 @@ macro_rules! regex_find_placeholders {
 
         // for every matched address check whether we already found it
         // and if we didn't, generate a placeholder for it
-        for address in regex.find_iter($input) {
+        for address in REGEX.find_iter($input) {
             if !$replacements.contains_key(address.as_str()) {
                 $replacements.insert(address.as_str().to_owned(), $create_placeholder(counter));
                 counter += 1;