chore: fire shutdown signal on anvil node handle drop (#8947)

* chore: add anvil NodeHandle.fire_shutdown_signal

* Remove DAPP remappings from env vars from cli tests.

* Unwrap fire shutdown

* Fix clippy

* track_caller on fire shutdown

* fire shutdown signal on drop

crates/anvil/src/lib.rs

@@ -255,32 +255,41 @@ pub async fn try_spawn(mut config: NodeConfig) -> io::Result<(EthApi, NodeHandle
 
 type IpcTask = JoinHandle<()>;
 
-/// A handle to the spawned node and server tasks
+/// A handle to the spawned node and server tasks.
 ///
 /// This future will resolve if either the node or server task resolve/fail.
 pub struct NodeHandle {
     config: NodeConfig,
-    /// The address of the running rpc server
+    /// The address of the running rpc server.
     addresses: Vec<SocketAddr>,
-    /// Join handle for the Node Service
+    /// Join handle for the Node Service.
     pub node_service: JoinHandle<Result<(), NodeError>>,
     /// Join handles (one per socket) for the Anvil server.
     pub servers: Vec<JoinHandle<Result<(), NodeError>>>,
-    // The future that joins the ipc server, if any
+    /// The future that joins the ipc server, if any.
     ipc_task: Option<IpcTask>,
     /// A signal that fires the shutdown, fired on drop.
     _signal: Option<Signal>,
-    /// A task manager that can be used to spawn additional tasks
+    /// A task manager that can be used to spawn additional tasks.
     task_manager: TaskManager,
 }
 
+impl Drop for NodeHandle {
+    fn drop(&mut self) {
+        // Fire shutdown signal to make sure anvil instance is terminated.
+        if let Some(signal) = self._signal.take() {
+            signal.fire().unwrap()
+        }
+    }
+}
+
 impl NodeHandle {
-    /// The [NodeConfig] the node was launched with
+    /// The [NodeConfig] the node was launched with.
     pub fn config(&self) -> &NodeConfig {
         &self.config
     }
 
-    /// Prints the launch info
+    /// Prints the launch info.
     pub(crate) fn print(&self, fork: Option<&ClientFork>) {
         self.config.print(fork);
         if !self.config.silent {
@@ -298,25 +307,25 @@ impl NodeHandle {
         }
     }
 
-    /// The address of the launched server
+    /// The address of the launched server.
     ///
     /// **N.B.** this may not necessarily be the same `host + port` as configured in the
-    /// `NodeConfig`, if port was set to 0, then the OS auto picks an available port
+    /// `NodeConfig`, if port was set to 0, then the OS auto picks an available port.
     pub fn socket_address(&self) -> &SocketAddr {
         &self.addresses[0]
     }
 
-    /// Returns the http endpoint
+    /// Returns the http endpoint.
     pub fn http_endpoint(&self) -> String {
         format!("http://{}", self.socket_address())
     }
 
-    /// Returns the websocket endpoint
+    /// Returns the websocket endpoint.
     pub fn ws_endpoint(&self) -> String {
         format!("ws://{}", self.socket_address())
     }
 
-    /// Returns the path of the launched ipc server, if any
+    /// Returns the path of the launched ipc server, if any.
     pub fn ipc_path(&self) -> Option<String> {
         self.config.get_ipc_path()
     }
@@ -336,44 +345,44 @@ impl NodeHandle {
         ProviderBuilder::new(&self.config.get_ipc_path()?).build().ok()
     }
 
-    /// Signer accounts that can sign messages/transactions from the EVM node
+    /// Signer accounts that can sign messages/transactions from the EVM node.
     pub fn dev_accounts(&self) -> impl Iterator<Item = Address> + '_ {
         self.config.signer_accounts.iter().map(|wallet| wallet.address())
     }
 
-    /// Signer accounts that can sign messages/transactions from the EVM node
+    /// Signer accounts that can sign messages/transactions from the EVM node.
     pub fn dev_wallets(&self) -> impl Iterator<Item = PrivateKeySigner> + '_ {
         self.config.signer_accounts.iter().cloned()
     }
 
-    /// Accounts that will be initialised with `genesis_balance` in the genesis block
+    /// Accounts that will be initialised with `genesis_balance` in the genesis block.
     pub fn genesis_accounts(&self) -> impl Iterator<Item = Address> + '_ {
         self.config.genesis_accounts.iter().map(|w| w.address())
     }
 
-    /// Native token balance of every genesis account in the genesis block
+    /// Native token balance of every genesis account in the genesis block.
     pub fn genesis_balance(&self) -> U256 {
         self.config.genesis_balance
     }
 
-    /// Default gas price for all txs
+    /// Default gas price for all txs.
     pub fn gas_price(&self) -> u128 {
         self.config.get_gas_price()
     }
 
-    /// Returns the shutdown signal
+    /// Returns the shutdown signal.
     pub fn shutdown_signal(&self) -> &Option<Signal> {
         &self._signal
     }
 
-    /// Returns mutable access to the shutdown signal
+    /// Returns mutable access to the shutdown signal.
     ///
-    /// This can be used to extract the Signal
+    /// This can be used to extract the Signal.
     pub fn shutdown_signal_mut(&mut self) -> &mut Option<Signal> {
         &mut self._signal
     }
 
-    /// Returns the task manager that can be used to spawn new tasks
+    /// Returns the task manager that can be used to spawn new tasks.
     ///
     /// ```
     /// use anvil::NodeHandle;

crates/anvil/tests/it/anvil_api.rs

@@ -667,7 +667,7 @@ async fn can_remove_pool_transactions() {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn test_reorg() {
-    let (api, mut handle) = spawn(NodeConfig::test()).await;
+    let (api, handle) = spawn(NodeConfig::test()).await;
     let provider = handle.ws_provider();
 
     let accounts = handle.dev_wallets().collect::<Vec<_>>();
@@ -792,8 +792,4 @@ async fn test_reorg() {
         })
         .await;
     assert!(res.is_err());
-
-    if let Some(signal) = handle.shutdown_signal_mut().take() {
-        signal.fire().unwrap();
-    }
 }

crates/anvil/tests/it/gas.rs

@@ -190,7 +190,7 @@ async fn test_tip_above_fee_cap() {
 #[tokio::test(flavor = "multi_thread")]
 async fn test_can_use_fee_history() {
     let base_fee = 50u128;
-    let (_api, mut handle) = spawn(NodeConfig::test().with_base_fee(Some(base_fee))).await;
+    let (_api, handle) = spawn(NodeConfig::test().with_base_fee(Some(base_fee))).await;
     let provider = handle.http_provider();
 
     for _ in 0..10 {
@@ -215,8 +215,4 @@ async fn test_can_use_fee_history() {
         assert_eq!(latest_block.header.base_fee_per_gas.unwrap(), *latest_fee_history_fee);
         assert_eq!(latest_fee_history_fee, next_base_fee);
     }
-
-    if let Some(signal) = handle.shutdown_signal_mut().take() {
-        signal.fire().unwrap();
-    }
 }

crates/anvil/tests/it/proof.rs

@@ -122,16 +122,12 @@ async fn test_storage_proof() {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn can_get_random_account_proofs() {
-    let (api, mut handle) = spawn(NodeConfig::test()).await;
+    let (api, _handle) = spawn(NodeConfig::test()).await;
 
     for acc in std::iter::repeat_with(Address::random).take(10) {
         let _ = api
             .get_proof(acc, Vec::new(), None)
             .await
             .unwrap_or_else(|_| panic!("Failed to get proof for {acc:?}"));
     }
-
-    if let Some(signal) = handle.shutdown_signal_mut().take() {
-        signal.fire().unwrap();
-    }
 }

crates/anvil/tests/it/traces.rs

@@ -722,7 +722,7 @@ async fn test_trace_address_fork2() {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn test_trace_filter() {
-    let (api, mut handle) = spawn(NodeConfig::test()).await;
+    let (api, handle) = spawn(NodeConfig::test()).await;
     let provider = handle.ws_provider();
 
     let accounts = handle.dev_wallets().collect::<Vec<_>>();
@@ -859,8 +859,4 @@ async fn test_trace_filter() {
 
     let traces = api.trace_filter(tracer).await.unwrap();
     assert_eq!(traces.len(), 5);
-
-    if let Some(signal) = handle.shutdown_signal_mut().take() {
-        signal.fire().unwrap();
-    }
 }

crates/forge/tests/cli/config.rs

@@ -204,18 +204,6 @@ forgetest_init!(can_override_config, |prj, cmd| {
         Remapping::from(config.remappings[0].clone()).to_string()
     );
 
-    // env vars work
-    std::env::remove_var("DAPP_REMAPPINGS");
-    std::env::set_var("DAPP_REMAPPINGS", "ds-test/=lib/forge-std/lib/ds-test/from-env/");
-    let config = forge_utils::load_config_with_root(Some(prj.root()));
-    assert_eq!(
-        format!(
-            "ds-test/={}/",
-            prj.root().join("lib/forge-std/lib/ds-test/from-env").to_slash_lossy()
-        ),
-        Remapping::from(config.remappings[0].clone()).to_string()
-    );
-
     let config =
         prj.config_from_output(["--remappings", "ds-test/=lib/forge-std/lib/ds-test/from-cli"]);
     assert_eq!(
@@ -234,7 +222,6 @@ forgetest_init!(can_override_config, |prj, cmd| {
         Remapping::from(config.remappings[0].clone()).to_string()
     );
 
-    std::env::remove_var("DAPP_REMAPPINGS");
     pretty_err(&remappings_txt, fs::remove_file(&remappings_txt));
 
     let expected = profile.into_basic().to_string_pretty().unwrap().trim().to_string();
@@ -506,7 +493,6 @@ forgetest!(can_set_gas_price, |prj, cmd| {
 
 // test that we can detect remappings from foundry.toml
 forgetest_init!(can_detect_lib_foundry_toml, |prj, cmd| {
-    std::env::remove_var("DAPP_REMAPPINGS");
     let config = cmd.config();
     let remappings = config.remappings.iter().cloned().map(Remapping::from).collect::<Vec<_>>();
     similar_asserts::assert_eq!(
@@ -518,15 +504,13 @@ forgetest_init!(can_detect_lib_foundry_toml, |prj, cmd| {
     );
 
     // create a new lib directly in the `lib` folder with a remapping
-    std::env::remove_var("DAPP_REMAPPINGS");
     let mut config = config;
     config.remappings = vec![Remapping::from_str("nested/=lib/nested").unwrap().into()];
     let nested = prj.paths().libraries[0].join("nested-lib");
     pretty_err(&nested, fs::create_dir_all(&nested));
     let toml_file = nested.join("foundry.toml");
     pretty_err(&toml_file, fs::write(&toml_file, config.to_string_pretty().unwrap()));
 
-    std::env::remove_var("DAPP_REMAPPINGS");
     let config = cmd.config();
     let remappings = config.remappings.iter().cloned().map(Remapping::from).collect::<Vec<_>>();
     similar_asserts::assert_eq!(
@@ -549,7 +533,6 @@ forgetest_init!(can_detect_lib_foundry_toml, |prj, cmd| {
     let toml_file = nested.join("foundry.toml");
     pretty_err(&toml_file, fs::write(&toml_file, config.to_string_pretty().unwrap()));
 
-    std::env::remove_var("DAPP_REMAPPINGS");
     let another_config = cmd.config();
     let remappings =
         another_config.remappings.iter().cloned().map(Remapping::from).collect::<Vec<_>>();
@@ -569,7 +552,6 @@ forgetest_init!(can_detect_lib_foundry_toml, |prj, cmd| {
 
     config.src = "custom-source-dir".into();
     pretty_err(&toml_file, fs::write(&toml_file, config.to_string_pretty().unwrap()));
-    std::env::remove_var("DAPP_REMAPPINGS");
     let config = cmd.config();
     let remappings = config.remappings.iter().cloned().map(Remapping::from).collect::<Vec<_>>();
     similar_asserts::assert_eq!(