CHANGELOG.md (1)

34-36: Capitalize “Prometheus” for consistency with prior entries.

Earlier entries use “Prometheus metrics”. Minor wording tweak for consistency.

- - [#6079](https://github.com/ChainSafe/forest/pull/6079) Added prometheus metrics `network_height` and `network_version`.
+ - [#6079](https://github.com/ChainSafe/forest/pull/6079) Added Prometheus metrics `network_height` and `network_version`.

src/shim/version.rs (1)

91-95: Match conversion style used elsewhere in this file.

Other impls use u32::from(...). Align for readability.

 impl From<NetworkVersion> for u32 {
     fn from(value: NetworkVersion) -> Self {
-        value.0.into()
+        u32::from(value.0)
     }
 }

src/daemon/mod.rs (1)

207-213: Avoid shadowing chain_store; reuse the same clone for both closures.

Small readability tweak; same behavior.

-        let chain_store = ctx.state_manager.chain_store().clone();
-        let get_chain_head_height = move || chain_store.heaviest_tipset().epoch();
-        let chain_store = ctx.state_manager.chain_store().clone();
-        let get_chain_head_network_version = move || {
-            let epoch = chain_store.heaviest_tipset().epoch();
-            chain_store.chain_config.network_version(epoch)
-        };
+        let chain_store = ctx.state_manager.chain_store().clone();
+        let get_chain_head_height = {
+            let cs = chain_store.clone();
+            move || cs.heaviest_tipset().epoch()
+        };
+        let get_chain_head_network_version = {
+            let cs = chain_store.clone();
+            move || {
+                let epoch = cs.heaviest_tipset().epoch();
+                cs.chain_config.network_version(epoch)
+            }
+        };

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

src/daemon/mod.rs (1)

207-224: Unify Weak handle, avoid hardcoded V0 fallback, and tighten wording

• Reuse a single Weak for both closures to avoid duplicate downgrades.
• Prefer a fallback derived from chain_config instead of hardcoding NetworkVersion::V0 to reduce misleading scrapes when ChainStore is unavailable.
• Minor: tweak comment phrasing “dead lock” → “deadlock”.

Apply this diff:

-        // Use `Weak` to not dead lock GC.
-        let chain_store = Arc::downgrade(ctx.state_manager.chain_store());
-        let get_chain_head_height = move || {
-            chain_store
-                .upgrade()
-                .map(|cs| cs.heaviest_tipset().epoch())
-                .unwrap_or_default()
-        };
-        // Use `Weak` to not dead lock GC.
-        let chain_store = Arc::downgrade(ctx.state_manager.chain_store());
-        let get_chain_head_network_version = move || {
-            if let Some(cs) = chain_store.upgrade() {
-                let epoch = cs.heaviest_tipset().epoch();
-                cs.chain_config.network_version(epoch)
-            } else {
-                NetworkVersion::V0
-            }
-        };
+        // Use a Weak to avoid deadlocking GC and to not keep ChainStore alive.
+        let chain_store_weak = Arc::downgrade(ctx.state_manager.chain_store());
+        let chain_config = ctx.state_manager.chain_config().clone();
+
+        let get_chain_head_height = {
+            let chain_store_weak = chain_store_weak.clone();
+            move || {
+                chain_store_weak
+                    .upgrade()
+                    .map(|cs| cs.heaviest_tipset().epoch())
+                    .unwrap_or_default()
+            }
+        };
+
+        let get_chain_head_network_version = {
+            let chain_store_weak = chain_store_weak.clone();
+            let chain_config = chain_config.clone();
+            move || {
+                chain_store_weak
+                    .upgrade()
+                    .map(|cs| chain_config.network_version(cs.heaviest_tipset().epoch()))
+                    .unwrap_or_else(|| chain_config.network_version(ChainEpoch::default()))
+            }
+        };

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

src/networks/mod.rs (1)

436-444: Consider caching Height iteration results for performance.

The network_version_revision function iterates through all Heights each time it's called. Since Heights are static and their NetworkVersion mappings don't change, consider caching the first height for each NetworkVersion to avoid repeated iteration in hot paths.

This could be optimized with a lazy static lookup table:

+use std::collections::HashMap;
+
+static FIRST_HEIGHT_BY_VERSION: LazyLock<HashMap<NetworkVersion, Height>> = LazyLock::new(|| {
+    let mut map = HashMap::new();
+    for height in Height::iter() {
+        let nv = NetworkVersion::from(height);
+        map.entry(nv).or_insert(height);
+    }
+    map
+});

 pub fn network_version_revision(&self, epoch: ChainEpoch) -> i64 {
     if let Some(height) = self.network_height(epoch) {
         let nv = NetworkVersion::from(height);
-        if let Some(rev0_height) = Height::iter().find(|h| NetworkVersion::from(*h) == nv) {
+        if let Some(&rev0_height) = FIRST_HEIGHT_BY_VERSION.get(&nv) {
             return (height as i64) - (rev0_height as i64);
         }
     }
     0
 }

src/networks/metrics.rs (1)

53-64: Consider documenting metric units and ranges.

The network_height metric would benefit from documentation about its units (epochs) and expected range in the metric description.

             let metric_encoder = encoder.encode_descriptor(
                 "network_height",
-                "The current network height",
+                "The current network height in epochs (blocks since genesis)",
                 None,
                 network_height.metric_type(),
             )?;

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

src/daemon/mod.rs (2)

210-215: Head-height getter fallback: confirm semantics and observability.

If the Weak can’t upgrade, the metric reports height 0. That could silently skew network_height/network_version during teardown/restart windows. Consider logging once on fallback or documenting this behavior in the metric HELP so operators know 0 can mean “store unavailable.”

---

208-209: Nit: tighten the comment.

Change “dead lock” → “deadlock” and clarify the reason (avoiding keeping the store alive and blocking GC).

-        // Use `Weak` to not dead lock GC.
+        // Use a Weak to avoid keeping the chain store alive and risking GC deadlock.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

src/daemon/mod.rs (1)

234-249: Fix: spawned future must be 'static — use async move and clone captured Arcs

As noted in the earlier review, JoinSet::spawn requires a 'static future. The current async block borrows stack locals and will fail to compile or is fragile.

Apply this diff:

         services.spawn({
             let chain_config = ctx.chain_config().clone();
-            let get_chain_head_height = get_chain_head_height.clone();
-            async {
+            let get_chain_head_height = std::sync::Arc::clone(&get_chain_head_height);
+            let get_chain_head_actor_version =
+                std::sync::Arc::clone(&get_chain_head_actor_version);
+            async move {
                 crate::metrics::init_prometheus(
                     prometheus_listener,
                     db_directory,
                     db,
                     chain_config,
                     get_chain_head_height,
-                    get_chain_head_actor_version,
+                    get_chain_head_actor_version,
                 )
                 .await
                 .context("Failed to initiate prometheus server")
             }
         });

src/shim/state_tree.rs (1)

200-207: Resolve actor bundle metadata efficiently; minor ergonomics

Looks correct and returns a stable reference. Two nits:

• This does a linear scan over ACTOR_BUNDLES_METADATA on every call. If this ends up hot, consider a secondary map keyed by system actor code Cid for O(1) lookup.
• The returned reference is effectively 'static; current signature is fine, but adding a brief doc comment on lifetime/source (static registry) would help future maintainers.

If you expect this method to be called per-scrape for metrics, we can add a cache keyed by the resolved system code. Want a small follow-up patch?

src/daemon/mod.rs (2)

219-233: Avoid per-scrape heavy state reconstruction (optional)

Each scrape rebuilds a StateTree and scans bundle metadata. If scrape frequency is high, this can add load. Consider caching the actor version by epoch (or tipset key) with a small TTL, invalidated on head changes.

I can wire this to the existing head-change publisher and only recompute on Apply. Interested?

---

577-579: Resetting the global Prometheus registry: confirm lifecycle

Overwriting the default registry is fine at process start, but on internal restarts it can drop already-registered custom collectors if any other tasks still hold references. Confirm there’s no concurrent metrics server running when this executes.

If you want a safer pattern, we can guard this behind a “first init” OnceCell or replace it with an explicit “clear” API on the registry.

src/networks/mod.rs (4)

185-224: Validate NV mappings for new heights

Mappings for Teep/Tock/TockFix/GoldenWeek imply NV25–NV27. Ensure these align with lotus/build constants for each network; drift here will misreport network_version.

If helpful, I can add a small test asserting that for each Height the NV matches lotus’ tables.

---

416-423: Off-by-one boundary: upgrade effective at epoch or epoch+1?

network_height uses epoch > info.epoch, which makes the upgrade effective starting at info.epoch + 1. Your calibnet test expects that, but please confirm this matches canonical behavior across networks (Lotus often treats upgrades as effective at the named epoch). If not universally true, we may need a per-network flag or >=.

I can add cross-check tests against known mainnet/calibnet boundaries to lock this down.

---

434-444: Revision math depends on enum order; add a guard

Computing revision as height - first_height_with_same_nv relies on Height variants of the same NV being contiguous and ordered. Add a test to assert contiguity so future insertions don’t silently skew revision numbers.

Apply this test addition:

@@
 #[cfg(test)]
 mod tests {
     use super::*;
+    #[test]
+    fn revisions_are_contiguous_per_nv() {
+        use std::collections::HashMap;
+        let mut groups: HashMap<NetworkVersion, Vec<Height>> = HashMap::new();
+        for h in Height::iter() {
+            groups.entry(NetworkVersion::from(h)).or_default().push(h);
+        }
+        for (_nv, hs) in groups {
+            // Ensure all heights for the NV form a single contiguous run in declaration order
+            let idxs: Vec<_> = hs.iter().map(|h| *h as i64).collect();
+            let min = *idxs.iter().min().unwrap();
+            let max = *idxs.iter().max().unwrap();
+            assert_eq!(idxs.len() as i64, max - min + 1, "non-contiguous NV group");
+        }
+    }

---

47-49: Newest NV constant appears stale

NEWEST_NETWORK_VERSION is V25, but Height includes NV26 and NV27. If this constant is used anywhere, it should be updated to V27.

src/networks/metrics.rs (1)

49-83: Height metrics look correct; consider now() source

Encoding both actual and expected heights is great. Minor: using chrono::Utc::now() is fine; if you already centralize time sources elsewhere (e.g., a Clock trait), consider using it for testability.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

src/networks/metrics.rs (5)

53-64: Optimize gauge allocation by reusing instances.

Creating new Gauge instances on every encode call is inefficient. Consider storing the gauges as fields in the collector struct to avoid repeated allocations during metric collection.

 pub struct NetworkHeightCollector<F>
 where
     F: Fn() -> ChainEpoch,
 {
     block_delay_secs: u32,
     genesis_timestamp: u64,
+    network_height: Gauge,
+    expected_network_height: Gauge,
     #[educe(Debug(ignore))]
     get_chain_head_height: Arc<F>,
 }

Then update the constructor and encode method accordingly:

     pub fn new(
         block_delay_secs: u32,
         genesis_timestamp: u64,
         get_chain_head_height: Arc<F>,
     ) -> Self {
         Self {
             block_delay_secs,
             genesis_timestamp,
+            network_height: Default::default(),
+            expected_network_height: Default::default(),
             get_chain_head_height,
         }
     }

-            let network_height: Gauge = Default::default();
             let epoch = (self.get_chain_head_height)();
-            network_height.set(epoch);
+            self.network_height.set(epoch);
             let metric_encoder = encoder.encode_descriptor(
                 "network_height",
                 "The current network height",
                 None,
-                network_height.metric_type(),
+                self.network_height.metric_type(),
             )?;
-            network_height.encode(metric_encoder)?;
+            self.network_height.encode(metric_encoder)?;

---

127-127: Potential integer overflow when casting network version.

The cast from u32 to gauge value type (likely i64) and then the unnecessary intermediate cast to _ could mask potential issues. Use explicit typing for clarity.

-            nv_gauge.set(u32::from(network_version) as _);
+            nv_gauge.set(i64::from(u32::from(network_version)));

---

151-151: Remove unnecessary cast for actor version.

The actor version is already u64, and the unnecessary cast to _ reduces code clarity.

-            av_gauge.set(actor_version as _);
+            av_gauge.set(actor_version as i64);

---

122-161: Consider adding metric labels for better observability.

Network version metrics could benefit from additional labels like the actual version name (e.g., "v21", "v22") to make dashboards more readable without requiring lookups.

Would you like me to help implement labeled metrics that include both the numeric value and a human-readable version string as a label? This would make Grafana dashboards more intuitive.

---

85-162: Add documentation for the new collector.

The NetworkVersionCollector struct and its public methods lack documentation. Consider adding doc comments to explain the purpose, usage, and the meaning of each metric it exposes.

+/// Collector for network version related metrics.
+/// 
+/// This collector exposes:
+/// - `network_version`: The current network version based on chain epoch
+/// - `network_version_revision`: The revision number within the current network version
+/// - `actor_version`: The actor bundle version at the current chain head
 #[derive(Educe)]
 #[educe(Debug)]
 pub struct NetworkVersionCollector<F1, F2>

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

src/daemon/mod.rs (1)

234-249: Fix: spawned future must be 'static — use async move.

Without move, the async block borrows stack locals (prometheus_listener, db_directory, db, closures), which won’t satisfy JoinSet::spawn’s 'static bound.

Apply this diff:

-        services.spawn({
+        services.spawn({
             let chain_config = ctx.chain_config().clone();
             let get_chain_head_height = get_chain_head_height.clone();
-            async {
+            async move {
                 crate::metrics::init_prometheus(
                     prometheus_listener,
                     db_directory,
                     db,
                     chain_config,
                     get_chain_head_height,
                     get_chain_head_actor_version,
                 )
                 .await
                 .context("Failed to initiate prometheus server")
             }
         });

src/daemon/mod.rs (1)

219-233: Actor-version getter: verify epoch selection and consider lightweight caching.

• Using heaviest_tipset().parent_state() may be off by one at upgrade boundaries; verify against a known upgrade epoch that the reported value matches “actual” actor bundle version for that tipset.
• Building a StateTree every scrape can be expensive; consider memoizing by (tipset_epoch or parent_state_cid) to avoid repeated reads between scrapes.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

docs/docs/users/reference/metrics.md (1)

20-22: Missing network_height doc and unclear distinctions vs. head_epoch/expected_network_height.

• PR title promises network_height, but it's not documented in the table.
• Past review asked to clarify differences; please add a short section explaining how these three relate.

Apply this doc diff to add network_height and a clarifying section:

@@
-| `expected_network_height`      | Gauge     | Count   | The expected network height based on the current time and the genesis block time             |
+| `expected_network_height`      | Gauge     | Count   | The expected network height based on the current time and the genesis block time             |
+| `network_height`               | Gauge     | Count   | Network height derived from the current chain head (see notes below)                         |
 | `network_version`              | Gauge     | Count   | Network version of the current chain head                                                    |
 | `network_version_revision`     | Gauge     | Count   | Network version revision of the current chain head                                           |
 | `actor_version`                | Gauge     | Count   | Actor version of the current chain head                                                      |
@@
 </details>
 
+<details>
+  <summary>Example `network_height` output</summary>
+```
+# HELP network_height Network height derived from the current chain head
+# TYPE network_height gauge
+network_height 3021421
+```
+</details>
+
+### Notes: head_epoch vs. network_height vs. expected_network_height
+- head_epoch: the epoch of the node’s heaviest tipset (what you actually synced to).
+- network_height: height derived from the current head for operational dashboards; intended to track the “current” network height as observed by this node.
+- expected_network_height: height computed from wall‑clock time, genesis timestamp, and block delay; use this to detect lag (expected − head_epoch).
+

head_epoch 3021420
network_height 3021421
expected_network_height 3023095

src/daemon/mod.rs (1)

209-249: Fix: spawn a 'static future with async move (compile blocker).

JoinSet::spawn requires a 'static future. The current async { ... } block borrows stack locals (prometheus_listener, db_directory, db, chain_config, closures), which won’t compile.

Apply:

-            async {
+            async move {
                 crate::metrics::init_prometheus(
                     prometheus_listener,
                     db_directory,
                     db,
                     chain_config,
                     get_chain_head_height,
                     get_chain_head_actor_version,
                 )
                 .await
                 .context("Failed to initiate prometheus server")
             }

docs/docs/users/reference/metrics.md (1)

294-320: Examples look good; add one for network_height to align with the table.

Without an example, users may miss how it renders or differs from head_epoch. See suggested diff above.

src/daemon/mod.rs (1)

221-237: Heavy work per scrape for actor_version; consider caching.

Constructing a StateTree and reading bundle metadata on every Prometheus scrape is expensive and can skew scrape latency. Prefer computing on head changes and publishing a cached atomic value.

If you want, I can draft a small subscriber that updates an AtomicI64 on HeadChange::Apply and have the getter read it.

src/networks/metrics.rs (1)

36-43: Nit: avoid lossy cast from i64 to u64 for timestamps.

Utc::now().timestamp() returns i64. Casting to u64 is fine in practice, but use try_into().unwrap_or_default() to avoid surprising wrap in hypothetical negative times (tests, mocks).

-        let expected_epoch = calculate_expected_epoch(
-            chrono::Utc::now().timestamp() as u64,
+        let now_secs: u64 = chrono::Utc::now().timestamp().try_into().unwrap_or_default();
+        let expected_epoch = calculate_expected_epoch(
-            self.genesis_timestamp,
+            now_secs,
             self.block_delay_secs,
         );

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

src/chain_sync/metrics.rs (2)

31-33: Typo in metric name: "libp2p_messsage_total" (triple “s”)

Consider correcting the metric name (and planning a deprecation path to avoid breaking dashboards).

-            "libp2p_messsage_total",
+            "libp2p_message_total",

If dashboards depend on the old name, register both for one release cycle and mark the old one deprecated in CHANGELOG.

---

23-24: Nit: grammar in metric help text

"validate blocks with no cache hit" → "validates blocks without a cache hit".

-        "Duration of routine which validate blocks with no cache hit",
+        "Duration of routine which validates blocks without a cache hit",

src/networks/metrics.rs (3)

45-83: Optional: inject time source for testability

Directly calling chrono::Utc::now() makes this harder to test. Consider injecting a now() closure.

-    F: Fn() -> ChainEpoch + Send + Sync + 'static,
+    F: Fn() -> ChainEpoch + Send + Sync + 'static,
 {
     fn encode(
         &self,
         mut encoder: prometheus_client::encoding::DescriptorEncoder,
     ) -> Result<(), std::fmt::Error> {
-        {
+        {
             let network_height: Gauge = Default::default();
             let epoch = (self.get_chain_head_height)();
             network_height.set(epoch);
             let metric_encoder = encoder.encode_descriptor(
                 "head_epoch",
                 "Latest epoch synchronized to the node",
                 None,
                 network_height.metric_type(),
             )?;
             network_height.encode(metric_encoder)?;
         }
-        {
+        {
             let expected_network_height: Gauge = Default::default();
-            let expected_epoch = calculate_expected_epoch(
-                chrono::Utc::now().timestamp() as u64,
+            // If adopting injection, replace with: (self.get_now)()
+            let expected_epoch = calculate_expected_epoch(
+                chrono::Utc::now().timestamp() as u64,
                 self.genesis_timestamp,
                 self.block_delay_secs,
             );

If desired, I can provide a full diff adding get_now: Arc<dyn Fn() -> u64 + Send + Sync> to the struct/new().

---

66-79: Naming consistency (epoch vs height)

You expose "head_epoch" but "expected_network_height". Consider "expected_head_epoch" for symmetry.

-                "expected_network_height",
-                "The expected network height based on the current time and the genesis block time",
+                "expected_head_epoch",
+                "The expected head epoch based on current time and genesis time",

If you keep the current name, no blocker—just a consistency note.

---

149-152: Nit: explicit cast for clarity

Be explicit about the gauge’s integer type.

-            av_gauge.set(actor_version as _);
+            av_gauge.set(actor_version as i64);

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.

src/daemon/mod.rs (1)

234-249: Blocker: spawned future must be 'static — use async move.

Without move, the async block borrows stack locals and won’t satisfy JoinSet::spawn’s 'static bound. Also capture get_chain_head_actor_version like height for clarity.

Apply:

-        services.spawn({
+        services.spawn({
             let chain_config = ctx.chain_config().clone();
             let get_chain_head_height = get_chain_head_height.clone();
-            async {
+            let get_chain_head_actor_version = get_chain_head_actor_version.clone();
+            async move {
                 crate::metrics::init_prometheus(
                     prometheus_listener,
                     db_directory,
                     db,
                     chain_config,
                     get_chain_head_height,
                     get_chain_head_actor_version,
                 )
                 .await
                 .context("Failed to initiate prometheus server")
             }
         });

src/daemon/mod.rs (2)

209-218: Height getter: avoid sentinel 0; prefer explicit “unknown”.

Returning 0 on upgrade failure conflates “genesis” with “unknown”. Consider signaling unknown explicitly (e.g., Option) and let the collector decide whether to emit a sample.

Example within current types (minimal churn): add a debug log and a distinct negative epoch for unknown if acceptable in your metric logic.

-            move || {
+            move || {
                 chain_store
                     .upgrade()
                     .map(|cs| cs.heaviest_tipset().epoch())
-                    .unwrap_or_default()
+                    .unwrap_or_else(|| {
+                        tracing::debug!("get_chain_head_height: chain_store dropped; reporting unknown");
+                        0 // consider a distinct sentinel or handle Option upstream
+                    })
             }

---

220-233: Actor version getter may be heavy on each scrape; cache on head changes.

Building a StateTree and reading bundle metadata per /metrics scrape can block the metrics path. Prefer caching the actor version on head changes and having the getter read an Arc<AtomicU64> (or similar).

If helpful, I can sketch a small subscriber that updates an atomic on HeadChange::Apply and wire that into the getter.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: hanabi1224
PR: ChainSafe/forest#5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.