tests(app): 💎 polish mock consensus test infrastructure

see #3588. follows #4184 and #4181.

this takes a pass through the shared, Penumbra-specific test
infrastructure for mock consensus tests. notably, this decomposes
`init_chain.rs`, which has now become somewhat redundant with the
existence of other more involved tests of e.g. validator uptime
tracking.

this also cleans up some unused imports, guards against future
occurrences of that issue (_sharing code in `tests/` files is awkward_),
and decomposes the `common/mod.rs` file into some distinct standalone
components.

this also belatedly removes the `common::start_test_node()` helper. at
some point (_i was unable to find the link_) it was suggested that we
refrain from a shared setup helper like that. this branch removes that
helper, and updates its call-sites.

this branch is largely code motion, and is intended to be a last bit of
cleanup as we prepare for #3588 to wind down. ❤️

---------

Co-authored-by: Henry de Valence <[email protected]>

crates/core/app/tests/app_can_spend_notes_and_detect_outputs.rs

@@ -1,8 +1,8 @@
-mod common;
-
 use {
+    self::common::BuilderExt,
     anyhow::anyhow,
     cnidarium::TempStorage,
+    penumbra_app::{genesis::AppState, server::consensus::Consensus},
     penumbra_keys::test_keys,
     penumbra_mock_client::MockClient,
     penumbra_mock_consensus::TestNode,
@@ -13,16 +13,27 @@ use {
         memo::MemoPlaintext, plan::MemoPlan, TransactionParameters, TransactionPlan,
     },
     rand_core::OsRng,
-    tap::Tap,
+    tap::{Tap, TapFallible},
     tracing::info,
 };
 
+mod common;
+
 #[tokio::test]
 async fn app_can_spend_notes_and_detect_outputs() -> anyhow::Result<()> {
     // Install a test logger, acquire some temporary storage, and start the test node.
     let guard = common::set_tracing_subscriber();
     let storage = TempStorage::new().await?;
-    let mut test_node = common::start_test_node(&storage).await?;
+    let mut test_node = {
+        let app_state = AppState::default();
+        let consensus = Consensus::new(storage.as_ref().clone());
+        TestNode::builder()
+            .single_validator()
+            .with_penumbra_auto_app_state(app_state)?
+            .init_chain(consensus)
+            .await
+            .tap_ok(|e| tracing::info!(hash = %e.last_app_hash_hex(), "finished init chain"))?
+    };
 
     // Sync the mock client, using the test wallet's spend key, to the latest snapshot.
     let mut client = MockClient::new(test_keys::SPEND_KEY.clone())

crates/core/app/tests/common/mod.rs

@@ -1,134 +1,27 @@
 //! Shared integration testing facilities.
 
-// NB: Allow dead code, and unused imports. these are shared and consumed by files in `tests/`.
-#![allow(dead_code, unused_imports)]
-
-pub use self::test_node_builder_ext::BuilderExt;
-
-use {
-    async_trait::async_trait,
-    cnidarium::TempStorage,
-    penumbra_app::{
-        app::App,
-        genesis::AppState,
-        server::consensus::{Consensus, ConsensusService},
-    },
-    penumbra_mock_consensus::TestNode,
-    std::ops::Deref,
+// NB: these reëxports are shared and consumed by files in `tests/`.
+#[allow(unused_imports)]
+pub use self::{
+    temp_storage_ext::TempStorageExt, test_node_builder_ext::BuilderExt,
+    test_node_ext::TestNodeExt, tracing_subscriber::set_tracing_subscriber,
 };
 
 /// Penumbra-specific extensions to the mock consensus builder.
 ///
 /// See [`BuilderExt`].
 mod test_node_builder_ext;
 
-// Installs a tracing subscriber to log events until the returned guard is dropped.
-pub fn set_tracing_subscriber() -> tracing::subscriber::DefaultGuard {
-    use tracing_subscriber::filter::EnvFilter;
-
-    let filter = "info,penumbra_app=trace,penumbra_mock_consensus=trace";
-    let filter = EnvFilter::try_from_default_env()
-        .or_else(|_| EnvFilter::try_new(filter))
-        .expect("should have a valid filter directive")
-        // Without explicitly disabling the `r1cs` target, the ZK proof implementations
-        // will spend an enormous amount of CPU and memory building useless tracing output.
-        .add_directive(
-            "r1cs=off"
-                .parse()
-                .expect("rics=off is a valid filter directive"),
-        );
-
-    let subscriber = tracing_subscriber::fmt()
-        .with_env_filter(filter)
-        .pretty()
-        .with_test_writer()
-        .finish();
-
-    tracing::subscriber::set_default(subscriber)
-}
-
-/// A [`TestNode`] coupled with Penumbra's [`Consensus`] service.
-pub type PenumbraTestNode = TestNode<ConsensusService>;
-
-/// Returns a new [`PenumbraTestNode`] backed by the given temporary storage.
-pub async fn start_test_node(storage: &TempStorage) -> anyhow::Result<PenumbraTestNode> {
-    use tap::TapFallible;
-    let app_state = AppState::default();
-    let consensus = Consensus::new(storage.as_ref().clone());
-    TestNode::builder()
-        .single_validator()
-        .with_penumbra_auto_app_state(app_state)?
-        .init_chain(consensus)
-        .await
-        .tap_ok(|e| tracing::info!(hash = %e.last_app_hash_hex(), "finished init chain"))
-}
-
-#[async_trait]
-pub trait TempStorageExt: Sized {
-    async fn apply_genesis(self, genesis: AppState) -> anyhow::Result<Self>;
-    async fn apply_default_genesis(self) -> anyhow::Result<Self>;
-}
+/// Extensions to [`TempStorage`][cnidarium::TempStorage].
+mod temp_storage_ext;
 
-#[async_trait]
-impl TempStorageExt for TempStorage {
-    async fn apply_genesis(self, genesis: AppState) -> anyhow::Result<Self> {
-        // Check that we haven't already applied a genesis state:
-        if self.latest_version() != u64::MAX {
-            anyhow::bail!("database already initialized");
-        }
-
-        // Apply the genesis state to the storage
-        let mut app = App::new(self.latest_snapshot()).await?;
-        app.init_chain(&genesis).await;
-        app.commit(self.deref().clone()).await;
-
-        Ok(self)
-    }
-
-    async fn apply_default_genesis(self) -> anyhow::Result<Self> {
-        self.apply_genesis(Default::default()).await
-    }
-}
-
-#[async_trait]
-pub trait TestNodeExt: Sized {
-    async fn fast_forward_to_next_epoch(
-        &mut self,
-        storage: &TempStorage,
-    ) -> anyhow::Result<penumbra_sct::epoch::Epoch>;
-}
-
-#[async_trait]
-impl<C> TestNodeExt for TestNode<C>
-where
-    C: tower::Service<
-            tendermint::v0_37::abci::ConsensusRequest,
-            Response = tendermint::v0_37::abci::ConsensusResponse,
-            Error = tower::BoxError,
-        > + Send
-        + Clone
-        + 'static,
-    C::Future: Send + 'static,
-    C::Error: Sized,
-{
-    async fn fast_forward_to_next_epoch(
-        &mut self,
-        storage: &TempStorage,
-    ) -> Result<penumbra_sct::epoch::Epoch, anyhow::Error> {
-        use {penumbra_sct::component::clock::EpochRead, tap::Tap};
-
-        let get_epoch = || async { storage.latest_snapshot().get_current_epoch().await };
-        let start = get_epoch()
-            .await?
-            .tap(|start| tracing::info!(?start, "fast forwarding to next epoch"));
+/// Penumbra-specific extensions to the mock consensus test node.
+///
+/// See [`TestNodeExt`].
+mod test_node_ext;
 
-        loop {
-            self.block().execute().await?;
-            let current = get_epoch().await?;
-            if current != start {
-                tracing::debug!(end = ?current, ?start, "reached next epoch");
-                return Ok(current);
-            }
-        }
-    }
-}
+/// A pretty [`tracing`] subscriber for use in test cases.
+///
+/// NB: this subscriber makes use of a test writer, that is compatible with `cargo test`'s output
+/// capturing.
+mod tracing_subscriber;

crates/core/app/tests/common/temp_storage_ext.rs

@@ -0,0 +1,33 @@
+use {
+    async_trait::async_trait,
+    cnidarium::TempStorage,
+    penumbra_app::{app::App, genesis::AppState},
+    std::ops::Deref,
+};
+
+#[async_trait]
+pub trait TempStorageExt: Sized {
+    async fn apply_genesis(self, genesis: AppState) -> anyhow::Result<Self>;
+    async fn apply_default_genesis(self) -> anyhow::Result<Self>;
+}
+
+#[async_trait]
+impl TempStorageExt for TempStorage {
+    async fn apply_genesis(self, genesis: AppState) -> anyhow::Result<Self> {
+        // Check that we haven't already applied a genesis state:
+        if self.latest_version() != u64::MAX {
+            anyhow::bail!("database already initialized");
+        }
+
+        // Apply the genesis state to the storage
+        let mut app = App::new(self.latest_snapshot()).await?;
+        app.init_chain(&genesis).await;
+        app.commit(self.deref().clone()).await;
+
+        Ok(self)
+    }
+
+    async fn apply_default_genesis(self) -> anyhow::Result<Self> {
+        self.apply_genesis(Default::default()).await
+    }
+}

crates/core/app/tests/common/test_node_builder_ext.rs

@@ -1,11 +1,16 @@
 use {
+    decaf377_rdsa::VerificationKey,
     penumbra_app::genesis::AppState,
+    penumbra_keys::keys::{SpendKey, SpendKeyBytes},
     penumbra_mock_consensus::builder::Builder,
     penumbra_proto::{
         core::keys::v1::{GovernanceKey, IdentityKey},
         penumbra::core::component::stake::v1::Validator as PenumbraValidator,
     },
     penumbra_shielded_pool::genesis::Allocation,
+    penumbra_stake::DelegationToken,
+    rand::Rng,
+    rand_core::OsRng,
     tracing::trace,
 };
 
@@ -57,12 +62,6 @@ impl BuilderExt for Builder {
 fn generate_penumbra_validator(
     consensus_key: &ed25519_consensus::VerificationKey,
 ) -> (PenumbraValidator, Allocation) {
-    use decaf377_rdsa::VerificationKey;
-    use penumbra_keys::keys::{SpendKey, SpendKeyBytes};
-    use penumbra_stake::DelegationToken;
-    use rand::Rng;
-    use rand_core::OsRng;
-
     let seed = SpendKeyBytes(OsRng.gen());
     let spend_key = SpendKey::from(seed.clone());
     let validator_id_sk = spend_key.spend_auth_key();

crates/core/app/tests/common/test_node_ext.rs

@@ -0,0 +1,45 @@
+use {
+    async_trait::async_trait, cnidarium::TempStorage, penumbra_mock_consensus::TestNode,
+    penumbra_sct::component::clock::EpochRead as _, tap::Tap,
+};
+
+#[async_trait]
+pub trait TestNodeExt: Sized {
+    async fn fast_forward_to_next_epoch(
+        &mut self,
+        storage: &TempStorage,
+    ) -> anyhow::Result<penumbra_sct::epoch::Epoch>;
+}
+
+#[async_trait]
+impl<C> TestNodeExt for TestNode<C>
+where
+    C: tower::Service<
+            tendermint::v0_37::abci::ConsensusRequest,
+            Response = tendermint::v0_37::abci::ConsensusResponse,
+            Error = tower::BoxError,
+        > + Send
+        + Clone
+        + 'static,
+    C::Future: Send + 'static,
+    C::Error: Sized,
+{
+    async fn fast_forward_to_next_epoch(
+        &mut self,
+        storage: &TempStorage,
+    ) -> Result<penumbra_sct::epoch::Epoch, anyhow::Error> {
+        let get_epoch = || async { storage.latest_snapshot().get_current_epoch().await };
+        let start = get_epoch()
+            .await?
+            .tap(|start| tracing::info!(?start, "fast forwarding to next epoch"));
+
+        loop {
+            self.block().execute().await?;
+            let current = get_epoch().await?;
+            if current != start {
+                tracing::debug!(end = ?current, ?start, "reached next epoch");
+                return Ok(current);
+            }
+        }
+    }
+}

crates/core/app/tests/common/tracing_subscriber.rs

@@ -0,0 +1,29 @@
+use {
+    tracing::subscriber::{set_default, DefaultGuard},
+    tracing_subscriber::{filter::EnvFilter, fmt},
+};
+
+/// Installs a tracing subscriber to log events until the returned guard is dropped.
+//  NB: this is marked as "dead code" but it is used by integration tests.
+#[allow(dead_code)]
+pub fn set_tracing_subscriber() -> DefaultGuard {
+    let filter = "info,penumbra_app=trace,penumbra_mock_consensus=trace";
+    let filter = EnvFilter::try_from_default_env()
+        .or_else(|_| EnvFilter::try_new(filter))
+        .expect("should have a valid filter directive")
+        // Without explicitly disabling the `r1cs` target, the ZK proof implementations
+        // will spend an enormous amount of CPU and memory building useless tracing output.
+        .add_directive(
+            "r1cs=off"
+                .parse()
+                .expect("rics=off is a valid filter directive"),
+        );
+
+    let subscriber = fmt()
+        .with_env_filter(filter)
+        .pretty()
+        .with_test_writer()
+        .finish();
+
+    set_default(subscriber)
+}