Skip to content

Commit

Permalink
test: add integration tests for https functionality to binaries (#4983)
Browse files Browse the repository at this point in the history
## Describe your changes

While working on the tonic version bumps in #4980, we discovered that
the HTTPS client support can break without failing CI. This PR builds on
the new tests in #4979, trying to establish a baseline sanity check that
"yes, the programs can talk to https endpoints" so that during a large
refactor, we can easily confirm no regressions.

## Issue ticket number and link

Refs 

* #4980
* #4979
* #4400

## Testing and review

Check out this branch, run `just integration-testnet` and confirm all
checks pass. I also tacked on a commit enabling these new tests in CI,
which I consider temporary, but useful for the immediate near-term.

It'd also be helpful to point out any places that might use HTTPS
clients that aren't covered yet.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

  > tests/CI only, no app code changes
  • Loading branch information
conorsch authored Jan 14, 2025
1 parent 539cf4a commit 94d6812
Show file tree
Hide file tree
Showing 9 changed files with 279 additions and 1 deletion.
29 changes: 29 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,32 @@ jobs:
# Build each crate separately, to validate that the feature-gating is working.
# This is a lighter-weight version of `cargo check-all-features --workspace --release`.
- run: ./deployments/scripts/check-crate-feature-sets

# Integration tests that run against the public testnet endpoints.
# Temporarily enabling these in CI, to provide assurance during refactoring.
testnet-integration:
runs-on: buildjet-16vcpu-ubuntu-2204
steps:
- uses: actions/checkout@v4
with:
lfs: true

- name: install nix
uses: nixbuild/nix-quick-install-action@v28

- name: setup nix cache
uses: nix-community/cache-nix-action@v5
with:
primary-key: nix-${{ runner.os }}-${{ hashFiles('**/*.nix') }}
restore-prefixes-first-match: nix-${{ runner.os }}-
backend: buildjet

- name: Load rust cache
uses: astriaorg/buildjet-rust-cache@v2.5.1

# Confirm that the nix devshell is buildable and runs at all.
- name: validate nix env
run: nix develop --command echo hello

- name: run the testnet integration tests
run: nix develop --command just integration-testnet
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/bin/pclientd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ edition = {workspace = true}
default = ["std", "download-proving-keys"]
std = ["ibc-types/std"]
sct-divergence-check = ["penumbra-view/sct-divergence-check"]
integration-testnet = []
# Enable to use rayon parallelism for crypto operations
parallel = ["penumbra-transaction/parallel"]
download-proving-keys = ["penumbra-proof-params/download-proving-keys"]
Expand Down
90 changes: 90 additions & 0 deletions crates/bin/pclientd/tests/testnet.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
#![cfg(feature = "integration-testnet")]
//! Performs read-only operations against the public Penumbra testnet.
//! Useful to validate that HTTPS support is working.
use std::process::Command as StdCommand;

use anyhow::Context;
use assert_cmd::cargo::CommandCargoExt;
use futures::StreamExt;
use tempfile::tempdir;
use tokio::process::Command as TokioCommand;

use pclientd::PclientdConfig;
use penumbra_keys::test_keys;
use penumbra_proto::view::v1::view_service_client::ViewServiceClient;
use penumbra_view::ViewClient;

const NODE_URL: &str = "https://testnet.plinfra.net";

const PCLIENTD_BIND_ADDR: &str = "127.0.0.1:9081";

/// Build config for pclientd, supporting only read operations.
/// The custody portion is intentionally unset, to avoid side-effects
/// on the public testnet.
fn generate_readonly_config() -> anyhow::Result<PclientdConfig> {
Ok(PclientdConfig {
full_viewing_key: test_keys::FULL_VIEWING_KEY.clone(),
grpc_url: NODE_URL.parse()?,
bind_addr: PCLIENTD_BIND_ADDR.parse()?,
// No custody, so operations are read-only.
kms_config: None,
})
}

#[tokio::test]
/// Start a pclientd process for the testnet wallet, and sync to current height
/// on the testnet. We don't perform any write actions, so there will be no on-chain
/// side-effects. Instead, we simply confirm that pclientd can talk to a remote
/// endpoint and understand the blocks it receives.
async fn pclientd_sync_against_testnet() -> anyhow::Result<()> {
tracing_subscriber::fmt::try_init().ok();
// Create a tempdir for the pclientd instance to run in.
let data_dir = tempdir().unwrap();

// 1. Construct a config for the `pclientd` instance:
let config = generate_readonly_config()?;

let config_file_path = data_dir.path().to_owned().join("config.toml");
config.save(&config_file_path)?;

// 2. Run a `pclientd` instance in the background as a subprocess.
let home_dir = data_dir.path().to_owned();
// Use a std Command so we can use the cargo-specific extensions from assert_cmd
let mut pclientd_cmd = StdCommand::cargo_bin("pclientd")?;
pclientd_cmd.args(["--home", home_dir.as_path().to_str().unwrap(), "start"]);

// Convert to an async-aware Tokio command so we can spawn it in the background.
let mut pclientd_cmd = TokioCommand::from(pclientd_cmd);
// Important: without this, we could accidentally leave the pclientd instance running.
pclientd_cmd.kill_on_drop(true);

let mut pclientd = pclientd_cmd.spawn()?;

// Wait for the newly spawned daemon to come up.
tokio::time::sleep(std::time::Duration::from_millis(2500)).await;
if let Some(status) = pclientd.try_wait()? {
// An error occurred during startup, probably.
anyhow::bail!("pclientd exited early: {status:?}");
}

// 3. Build a client for the daemon we just started.
let pclientd_url = format!("http://{}", PCLIENTD_BIND_ADDR);
let channel = tonic::transport::Channel::from_shared(pclientd_url)?
.connect()
.await
.context("failed to open channel to test instance of pclientd")?;
let mut view_client = ViewServiceClient::new(channel.clone());
// let mut custody_client = CustodyServiceClient::new(channel.clone());

// 4. Use the view protocol to wait for it to sync.
let mut status_stream = (&mut view_client as &mut dyn ViewClient)
.status_stream()
.await?;
while let Some(item) = status_stream.as_mut().next().await.transpose()? {
tracing::debug!(?item);
}
// We exit here, dropping the pclientd process handle.
// We've verified that we can sync the wallet.
Ok(())
}
3 changes: 2 additions & 1 deletion crates/bin/pmonitor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ homepage = { workspace = true }
license = { workspace = true }
publish = false

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[features]
integration-testnet = []

[dependencies]
anyhow = {workspace = true}
Expand Down
119 changes: 119 additions & 0 deletions crates/bin/pmonitor/tests/testnet.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
#![cfg(feature = "integration-testnet")]
//! Integration tests for pmonitor against the public Penumbra testnet.
//! Mostly useful for verifying that HTTPS connections for gRPC
//! are well supported.
use anyhow::Result;
use assert_cmd::Command as AssertCommand;
use pcli::config::PcliConfig;

use std::fs::File;
use std::io::{BufWriter, Write};
use std::path::{Path, PathBuf};
use tempfile::{tempdir, NamedTempFile, TempDir};

const NODE_URL: &str = "https://testnet.plinfra.net";

/// Initialize a new pcli wallet at the target directory.
/// Discards the generated seed phrase.
fn pcli_init_softkms(pcli_home: &TempDir) -> Result<()> {
let mut cmd = AssertCommand::cargo_bin("pcli")?;
cmd.args([
"--home",
pcli_home
.path()
.to_str()
.expect("can convert wallet path to string"),
"init",
"--grpc-url",
NODE_URL,
"soft-kms",
"generate",
])
// send empty string to accept the interstitial seed phrase display
.write_stdin("");
cmd.assert().success();
Ok(())
}

/// Retrieve a FullViewingKey from a pcli home dir.
fn get_fvk_from_wallet_dir(pcli_home: &TempDir) -> Result<String> {
let pcli_config_path = pcli_home.path().join("config.toml");
let pcli_config = PcliConfig::load(
pcli_config_path
.to_str()
.expect("failed to convert pcli wallet path to str"),
)?;
Ok(pcli_config.full_viewing_key.to_string())
}

/// Given a list of FVKs, formatted as Strings, write a JSON file
/// containing those FVKs, for use with pmonitor via the `--fvks` CLI flag.
fn write_fvks_json(fvks: Vec<String>, dest_filepath: &File) -> Result<()> {
let mut w = BufWriter::new(dest_filepath);
serde_json::to_writer(&mut w, &fvks)?;
w.flush()?;
Ok(())
}

#[test]
// Initialize an empty (i.e. random) wallet. We don't care about prior balances,
// because we're not exercising misbehavior: all we care about is that pmonitor
// can talk to an HTTPS endpoint and understand the blocks it pulls.
pub fn pmonitor_passes_with_empty_wallet_on_testnet() -> Result<()> {
tracing_subscriber::fmt::try_init().ok();
let pcli_home = tempdir().unwrap();
pcli_init_softkms(&pcli_home)?;

let fvks = vec![get_fvk_from_wallet_dir(&pcli_home)?];
let fvks_json = NamedTempFile::new()?;
write_fvks_json(fvks, fvks_json.as_file())?;
let pmonitor_pardir = tempfile::tempdir()?;
let pmonitor_home = initialize_pmonitor(&pmonitor_pardir, fvks_json.path())?;

// Run `pmonitor audit` based on the pcli wallets and associated FVKs.
let cmd = AssertCommand::cargo_bin("pmonitor")?
.args([
"--home",
pmonitor_home
.as_path()
.to_str()
.expect("failed to parse pmonitor tempdir as directory"),
"audit",
])
.ok();

if cmd.is_ok() {
Ok(())
} else {
anyhow::bail!("failed during 'pmonitor audit'")
}
}

/// Generate a config directory for `pmonitor`, based on input FVKs.
fn initialize_pmonitor(tmpdir: &TempDir, fvks_json: &Path) -> anyhow::Result<PathBuf> {
// pmonitor doesn't like pre-existing homedirs so we'll nest this one.
let pmonitor_home = tmpdir.path().join("pmonitor");

let cmd = AssertCommand::cargo_bin("pmonitor")?
.args([
"--home",
pmonitor_home
.as_path()
.to_str()
.expect("failed to parse pmonitor tempdir as dir"),
"init",
"--grpc-url",
NODE_URL,
"--fvks",
fvks_json
.to_str()
.expect("failed to parse fvk json tempfile as filepath"),
])
.output();

assert!(
cmd.unwrap().status.success(),
"failed to initialize pmonitor"
);
Ok(pmonitor_home)
}
8 changes: 8 additions & 0 deletions crates/misc/measure/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,24 @@ dist = false
name = "measure"
path = "src/main.rs"

[features]
integration-testnet = []

[dependencies]
anyhow = {workspace = true}
bytesize = "1.2"
clap = {workspace = true, features = ["derive", "env"]}
indicatif = {workspace = true}
penumbra-compact-block = {workspace = true, default-features = false}
penumbra-proto = {workspace = true, features = ["rpc"], default-features = true}
penumbra-view = {workspace = true}
serde_json = {workspace = true}
tokio = {workspace = true, features = ["full"]}
tonic = {workspace = true, features = ["tls"]}
tracing = {workspace = true}
tracing-subscriber = {workspace = true, features = ["env-filter"]}
url = {workspace = true}

[dev-dependencies]
assert_cmd = {workspace = true}
predicates = "2.1"
23 changes: 23 additions & 0 deletions crates/misc/measure/tests/testnet.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#![cfg(feature = "integration-testnet")]
//! Integration tests for communicating with the public Penumbra testnet.
//! These tests are off by default, given that they contact remote services,
//! but are useful to verify functionality for e.g. HTTPS connectivity.
use assert_cmd::Command;

/// The URL for the public testnet pd endpoint.
const NODE_URL: &str = "https://testnet.plinfra.net";

// Raise the command timeout because the long-lived testnet will have more blocks to sync.
// Syncing ~1,000,000 blocks on a mostly-empty wallet should not take ~10 minutes!
// See GH#4970 for a report of a recent slowdown in pcli syncing operations.
const TESTNET_TIMEOUT_COMMAND_SECONDS: u64 = 900;

#[test]
fn stream_blocks_on_testnet() {
let mut cmd = Command::cargo_bin("measure").unwrap();
cmd.args(["--node", NODE_URL, "stream-blocks"])
.timeout(std::time::Duration::from_secs(
TESTNET_TIMEOUT_COMMAND_SECONDS,
));
cmd.assert().success();
}
4 changes: 4 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ metrics:
rustdocs:
./deployments/scripts/rust-docs

# Run integration tests against the testnet, for validating HTTPS support
integration-testnet:
cargo nextest run --release --features integration-testnet -E 'test(/_testnet$/)'

# Run smoke test suite, via process-compose config.
smoke:
./deployments/scripts/warn-about-pd-state
Expand Down

0 comments on commit 94d6812

Please sign in to comment.