Address PR review feedback

Author: ekulkisnek

app/app.rs

@@ -287,7 +287,7 @@ impl App {
         Ok(())
     }
 
-    pub async fn get_new_main_address_async(
+    pub async fn get_new_main_address(
         &self,
     ) -> Result<bitcoin::Address<bitcoin::address::NetworkChecked>, Error> {
         let Some(miner) = self.miner.as_ref() else {
@@ -306,10 +306,10 @@ impl App {
         Ok(res)
     }
 
-    pub fn get_new_main_address(
+    pub fn get_new_main_address_blocking(
         &self,
     ) -> Result<bitcoin::Address<bitcoin::address::NetworkChecked>, Error> {
-        self.runtime.block_on(self.get_new_main_address_async())
+        self.runtime.block_on(self.get_new_main_address())
     }
 
     const EMPTY_BLOCK_BMM_BRIBE: bitcoin::Amount =
@@ -530,7 +530,7 @@ impl App {
         Ok(())
     }
 
-    pub async fn deposit_async(
+    pub async fn deposit(
         &self,
         address: Address,
         amount: bitcoin::Amount,
@@ -548,14 +548,13 @@ impl App {
         Ok(txid)
     }
 
-    pub fn deposit(
+    pub fn deposit_blocking(
         &self,
         address: Address,
         amount: bitcoin::Amount,
         fee: bitcoin::Amount,
     ) -> Result<bitcoin::Txid, Error> {
-        self.runtime
-            .block_on(self.deposit_async(address, amount, fee))
+        self.runtime.block_on(self.deposit(address, amount, fee))
     }
 }
 

app/gui/coins/transfer_receive.rs

@@ -100,7 +100,7 @@ impl Receive {
         };
         let address = app
             .wallet
-            .get_address_or_new()
+            .get_or_generate_last_address()
             .map_err(anyhow::Error::from)
             .inspect_err(|err| tracing::error!("{err:#}"));
         Self {

app/gui/coins/utxo_creator.rs

@@ -93,7 +93,7 @@ impl UtxoCreator {
                     .add_enabled(app.is_some(), Button::new("generate"))
                     .clicked()
                 {
-                    match app.unwrap().get_new_main_address() {
+                    match app.unwrap().get_new_main_address_blocking() {
                         Ok(main_address) => {
                             self.main_address = format!("{main_address}");
                         }

app/gui/parent_chain/transfer.rs

@@ -95,7 +95,7 @@ impl Deposit {
                 Ok(address) => {
                     self.promise =
                         Some(poll_promise::Promise::spawn_async(async move {
-                            app.deposit_async(address, amount, fee)
+                            app.deposit(address, amount, fee)
                                 .await
                                 .map_err(|e| format!("{e:#}"))
                         }));
@@ -191,7 +191,7 @@ impl Withdrawal {
                     let app = app.unwrap().clone();
                     self.generate_promise =
                         Some(poll_promise::Promise::spawn_async(async move {
-                            app.get_new_main_address_async()
+                            app.get_new_main_address()
                                 .await
                                 .map_err(|e| format!("{e:#}"))
                         }));

app/main.rs

@@ -1,4 +1,4 @@
-use std::path::Path;
+use std::{path::Path, sync::Arc};
 
 use clap::Parser as _;
 use mimalloc::MiMalloc;
@@ -229,17 +229,16 @@ fn main() -> anyhow::Result<()> {
     });
 
     if !config.headless {
-        let fallback_rt;
-        let _guard = match &app {
-            Ok(app) => Some(app.runtime.enter()),
+        let rt = match &app {
+            Ok(app) => Arc::clone(&app.runtime),
             Err(_) => {
-                fallback_rt = tokio::runtime::Builder::new_multi_thread()
+                let rt = tokio::runtime::Builder::new_multi_thread()
                     .enable_all()
-                    .build()
-                    .ok();
-                fallback_rt.as_ref().map(|rt| rt.enter())
+                    .build()?;
+                Arc::new(rt)
             }
         };
+        let _rt_guard = rt.enter();
         // For GUI mode we want the GUI to start, even if the app fails to start.
         return run_egui_app(&config, line_buffer, app)
             .map_err(|e| anyhow::anyhow!("failed to run egui app: {e:#}"));

app/rpc_server.rs

@@ -51,7 +51,7 @@ impl RpcServer for RpcServerImpl {
     ) -> RpcResult<bitcoin::Txid> {
         let app = self.app.clone();
         tokio::task::spawn_blocking(move || {
-            app.deposit(
+            app.deposit_blocking(
                 address,
                 bitcoin::Amount::from_sat(value_sats),
                 bitcoin::Amount::from_sat(fee_sats),

lib/wallet.rs

@@ -510,14 +510,17 @@ impl Wallet {
         Ok(address)
     }
 
-    pub fn get_last_address(&self) -> Result<Option<Address>, Error> {
+    /// Gets the latest generated address.
+    pub fn try_get_last_address(&self) -> Result<Option<Address>, Error> {
         let txn = self.env.read_txn().map_err(EnvError::from)?;
         let last = self.index_to_address.last(&txn).map_err(DbError::from)?;
         Ok(last.map(|(_, address)| address))
     }
 
-    pub fn get_address_or_new(&self) -> Result<Address, Error> {
-        if let Some(address) = self.get_last_address()? {
+    /// Gets the latest generated address, or generates a new one if no
+    /// addresses have already been generated.
+    pub fn get_or_generate_last_address(&self) -> Result<Address, Error> {
+        if let Some(address) = self.try_get_last_address()? {
             Ok(address)
         } else {
             self.get_new_address()
@@ -595,14 +598,12 @@ mod tests {
     use super::*;
 
     #[test]
-    fn test_get_address_or_new() -> Result<(), Error> {
-        let test_dir = std::env::temp_dir().join(format!(
-            "thunder_test_wallet_{}",
-            std::time::SystemTime::now()
-                .duration_since(std::time::UNIX_EPOCH)
-                .unwrap()
-                .as_nanos()
-        ));
+    fn test_get_or_generate_last_address() -> anyhow::Result<()> {
+        let nanos = std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)?
+            .as_nanos();
+        let test_dir =
+            std::env::temp_dir().join(format!("thunder_test_wallet_{nanos}"));
 
         // Ensure clean state
         if test_dir.exists() {
@@ -618,30 +619,25 @@ mod tests {
         assert!(wallet.has_seed()?);
 
         // Get last address when none have been generated
-        let last = wallet.get_last_address()?;
+        let last = wallet.try_get_last_address()?;
         assert!(last.is_none());
 
-        // Get address or new should generate the first address
-        let addr1 = wallet.get_address_or_new()?;
+        // The first call should generate the first address.
+        let addr1 = wallet.get_or_generate_last_address()?;
 
-        // Now last address should be addr1
-        let last = wallet.get_last_address()?;
+        let last = wallet.try_get_last_address()?;
         assert_eq!(last, Some(addr1));
 
-        // Subsequent get_address_or_new calls should return the same addr1
-        let addr2 = wallet.get_address_or_new()?;
+        let addr2 = wallet.get_or_generate_last_address()?;
         assert_eq!(addr1, addr2);
 
-        // Generating a new address explicitly should give a new one
         let addr3 = wallet.get_new_address()?;
         assert_ne!(addr1, addr3);
 
-        // Now last address should be addr3
-        let last = wallet.get_last_address()?;
+        let last = wallet.try_get_last_address()?;
         assert_eq!(last, Some(addr3));
 
-        // And get_address_or_new should return addr3
-        let addr4 = wallet.get_address_or_new()?;
+        let addr4 = wallet.get_or_generate_last_address()?;
         assert_eq!(addr3, addr4);
 
         // Clean up