From 93ba640febd054ea67c0801f8b0654ed18d9056f Mon Sep 17 00:00:00 2001 From: Stan Drozd Date: Wed, 4 Jan 2023 17:16:55 +0100 Subject: [PATCH 1/7] wormhole-attester: port the v1.3.0 healthcheck hotfix This PR fixes a missing assignment to HealthCheckState's enable flag. This would cause the attester to ignore all settings (including defaults) and keep the enable flag set to false because we would never bother setting it from attestation_cfg that came at runtime. The bug is fixed and the healthcheck behavior is hardened. Currently, the lazy_static block will explicitly initialize the global with default values from attestation_cfg (enable is true, window size is 100). This prevents a similar human error from disabling the healthcheck. Secondly, main.rs overwrites the default values using the HealthCheckState::new() constructor, instead of individual struct member assignments. This ensures that all necessary values or their defaults are passed as the desired healthcheck state. --- wormhole-attester/client/src/healthcheck.rs | 3 ++- wormhole-attester/client/src/main.rs | 26 ++++++++++++--------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/wormhole-attester/client/src/healthcheck.rs b/wormhole-attester/client/src/healthcheck.rs index 696b21c2c8..46888772d8 100644 --- a/wormhole-attester/client/src/healthcheck.rs +++ b/wormhole-attester/client/src/healthcheck.rs @@ -1,4 +1,5 @@ use { + crate::attestation_cfg, std::{ collections::VecDeque, sync::Arc, @@ -7,7 +8,7 @@ use { }; lazy_static::lazy_static! { - pub static ref HEALTHCHECK_STATE: Arc> = Arc::new(Mutex::new(HealthCheckState::new(1, false))); + pub static ref HEALTHCHECK_STATE: Arc> = Arc::new(Mutex::new(HealthCheckState::new(attestation_cfg::default_healthcheck_window_size as usize, attestation_cfg::default_enable_healthcheck()))); } /// Helper structure for deciding service health diff --git a/wormhole-attester/client/src/main.rs b/wormhole-attester/client/src/main.rs index 4c321f0ca5..7df3e03a4d 100644 --- a/wormhole-attester/client/src/main.rs +++ b/wormhole-attester/client/src/main.rs @@ -41,6 +41,7 @@ use { gen_set_config_tx, gen_set_is_active_tx, get_config_account, + healthcheck::HealthCheckState, start_metrics_server, AttestationConfig, BatchState, @@ -304,17 +305,20 @@ async fn handle_attest_daemon_mode( metrics_bind_addr: SocketAddr, ) -> Result<(), ErrBox> { // Update healthcheck window size from config - if attestation_cfg.enable_healthcheck { - if attestation_cfg.healthcheck_window_size == 0 { - return Err(format!( - "{} must be above 0", - stringify!(attestation_cfg.healthcheck_window_size) - ) - .into()); - } - let mut hc = HEALTHCHECK_STATE.lock().await; - hc.max_window_size = attestation_cfg.healthcheck_window_size as usize; - } else { + if attestation_cfg.healthcheck_window_size == 0 { + return Err(format!( + "{} must be above 0", + stringify!(attestation_cfg.healthcheck_window_size) + ) + .into()); + } + let mut hc = HEALTHCHECK_STATE.lock().await; + *hc = HealthCheckState::new( + attestation_cfg.healthcheck_window_size as usize, + attestation_cfg.enable_healthcheck, + ); + + if !attestation_cfg.enable_healthcheck { warn!("WARNING: Healthcheck is disabled"); } From b1f35319930ee049b9a8c3b3c081911a2c26cc92 Mon Sep 17 00:00:00 2001 From: Stan Drozd Date: Thu, 5 Jan 2023 13:38:53 +0100 Subject: [PATCH 2/7] p2w_autoattest.py: Get debug output from first attestation --- third_party/pyth/p2w_autoattest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/third_party/pyth/p2w_autoattest.py b/third_party/pyth/p2w_autoattest.py index ebbc72aaf2..f8a1839f36 100755 --- a/third_party/pyth/p2w_autoattest.py +++ b/third_party/pyth/p2w_autoattest.py @@ -187,6 +187,7 @@ P2W_RPC_TIMEOUT_SECS, ], capture_output=True, + debug = True, ) logging.info("p2w_autoattest ready to roll!") From c01371b635ebfc7a2585518b912ec5db0b82a893 Mon Sep 17 00:00:00 2001 From: Stan Drozd Date: Thu, 5 Jan 2023 14:29:37 +0100 Subject: [PATCH 3/7] wormhole-attester: fix ()-less function bug, harden usize convertion This change fixes a bug that would cause a gigantic allocation to happen, after the attestation_cfg::default_healthcheck_window_size function is taken without being called and converted to a very large usize value. This would result in a big allocation which would crash the attester. At the core it's a misunderstanding about "Fn() as usize" conversion which unfortunately is safe Rust. --- wormhole-attester/client/src/healthcheck.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wormhole-attester/client/src/healthcheck.rs b/wormhole-attester/client/src/healthcheck.rs index 46888772d8..f069c708c1 100644 --- a/wormhole-attester/client/src/healthcheck.rs +++ b/wormhole-attester/client/src/healthcheck.rs @@ -2,13 +2,14 @@ use { crate::attestation_cfg, std::{ collections::VecDeque, + convert::TryInto, sync::Arc, }, tokio::sync::Mutex, }; lazy_static::lazy_static! { - pub static ref HEALTHCHECK_STATE: Arc> = Arc::new(Mutex::new(HealthCheckState::new(attestation_cfg::default_healthcheck_window_size as usize, attestation_cfg::default_enable_healthcheck()))); + pub static ref HEALTHCHECK_STATE: Arc> = Arc::new(Mutex::new(HealthCheckState::new(attestation_cfg::default_healthcheck_window_size().try_into().expect("could not convert window size to usize"), attestation_cfg::default_enable_healthcheck()))); } /// Helper structure for deciding service health From 8513c284ae0b8e11b819ca3aee6f0b74ff01b89b Mon Sep 17 00:00:00 2001 From: Stan Drozd Date: Thu, 5 Jan 2023 14:39:37 +0100 Subject: [PATCH 4/7] Trigger build From dcdc26463cc935b9685d2867f054fb8bf6ac9c81 Mon Sep 17 00:00:00 2001 From: Stan Drozd Date: Thu, 5 Jan 2023 15:12:09 +0100 Subject: [PATCH 5/7] wormhole-attester: Fix a misuse of a mutex guard This commit changes the `let hc = ` into a direct assignment to the locked mutex guard. Rust mutices are typically scope-based which makes using them in variables more error prone. --- wormhole-attester/client/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wormhole-attester/client/src/main.rs b/wormhole-attester/client/src/main.rs index 7df3e03a4d..57ccab304c 100644 --- a/wormhole-attester/client/src/main.rs +++ b/wormhole-attester/client/src/main.rs @@ -312,8 +312,8 @@ async fn handle_attest_daemon_mode( ) .into()); } - let mut hc = HEALTHCHECK_STATE.lock().await; - *hc = HealthCheckState::new( + + *HEALTHCHECK_STATE.lock().await = HealthCheckState::new( attestation_cfg.healthcheck_window_size as usize, attestation_cfg.enable_healthcheck, ); From b9641dded645a7f2579a64e158fd0c7010ac806e Mon Sep 17 00:00:00 2001 From: Stan Drozd Date: Thu, 5 Jan 2023 15:15:43 +0100 Subject: [PATCH 6/7] Dockerfile.client: Remove unused ADD wormhole-attester. This speeds up the build of the container, and prevents useless triggers from the attester codebase --- tilt-devnet/docker-images/Dockerfile.client | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tilt-devnet/docker-images/Dockerfile.client b/tilt-devnet/docker-images/Dockerfile.client index 42fdc4c962..1db6746ee7 100644 --- a/tilt-devnet/docker-images/Dockerfile.client +++ b/tilt-devnet/docker-images/Dockerfile.client @@ -9,12 +9,11 @@ RUN apt-get update && apt-get install -yq libudev-dev ncat RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - && apt-get install -y nodejs ADD rust-toolchain /rust-toolchain -ADD wormhole-attester/ /usr/src/wormhole-attester -WORKDIR /usr/src/wormhole-attester +WORKDIR /usr/src/bridge-client RUN --mount=type=cache,target=/root/.cache \ - --mount=type=cache,target=/usr/local/cargo/registry,id=cargo_registry \ - --mount=type=cache,target=/usr/src/solana/pyth2wormhole/target,id=p2w_cargo_build \ + --mount=type=cache,target=/usr/local/cargo/registry \ + --mount=type=cache,target=target \ cargo install --version =2.0.12 --locked spl-token-cli --target-dir target @@ -26,7 +25,7 @@ ENV BRIDGE_ADDRESS="Bridge1p5gheXUvJ6jGWGeCsgPKgnE3YgdGKRVCMY9o" RUN --mount=type=cache,target=/root/.cache \ --mount=type=cache,target=/usr/local/cargo/registry \ - --mount=type=cache,target=/usr/src/solana/pyth2wormhole/target \ + --mount=type=cache,target=target \ set -xe && \ cargo install bridge_client --git https://github.com/wormhole-foundation/wormhole --tag $WORMHOLE_TAG --locked --root /usr/local --target-dir target && \ cargo install token_bridge_client --git https://github.com/wormhole-foundation/wormhole --tag $WORMHOLE_TAG --locked --root /usr/local --target-dir target From ca2082b54a7896b103e12e15ff4d1244cf5ab4b3 Mon Sep 17 00:00:00 2001 From: Stan Drozd Date: Thu, 5 Jan 2023 15:17:23 +0100 Subject: [PATCH 7/7] p2w_autoattest.py: simplify log filtering to just info --- third_party/pyth/p2w_autoattest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/pyth/p2w_autoattest.py b/third_party/pyth/p2w_autoattest.py index f8a1839f36..8f8ed40a4d 100755 --- a/third_party/pyth/p2w_autoattest.py +++ b/third_party/pyth/p2w_autoattest.py @@ -166,7 +166,7 @@ # Set helpfully chatty logging default, filtering especially annoying # modules like async HTTP requests and tokio runtime logs -os.environ["RUST_LOG"] = os.environ.get("RUST_LOG", "pyth_wormhole_attester_client,solana_client,main,pyth_sdk_solana=trace") +os.environ["RUST_LOG"] = os.environ.get("RUST_LOG", "info") # Send the first attestation in one-shot mode for testing first_attest_result = run_or_die(