heartbeat_interval_hours is declared with no value_parser range, so every u64 including 0 parses:
crates/gitlawb-node/src/config.rs:129
#[arg(long, env = "GITLAWB_HEARTBEAT_INTERVAL_HOURS", default_value_t = 20)]
pub heartbeat_interval_hours: u64,
Every other duration knob in that file carries range(1..) (lines 182, 193, 203, 215, 225, 234). This one does not.
The value reaches a timer through two hops. crates/gitlawb-node/src/main.rs:845 builds Duration::from_secs(config.heartbeat_interval_hours * 3600), and crates/gitlawb-node/src/operator.rs:161 passes that Duration to tokio::time::interval.
The failure
With the value set to 0, tokio::time::interval(Duration::ZERO) panics with "period must be non-zero". Verified against tokio 1.50.0, the pinned version, in a release build.
The panic is inside the tokio::spawn at operator.rs:160, and the workspace release profile does not set panic = "abort", so it takes down only the heartbeat task. The node keeps serving. Worse, operator.rs:155 emits "operator heartbeat loop starting" at INFO before the spawn, so the startup logs read as healthy while no heartbeat is ever posted. The node then falls outside the contract's 24h HEARTBEAT_WINDOW and goes inactive on-chain.
So the observable symptom is not a crash. It is a node that looks up, serves traffic, logs a started heartbeat loop, and quietly loses operator status a day later.
Reachability
This affects only nodes that opted into on-chain staking. main.rs:502 gates the whole path on both GITLAWB_CONTRACT_NODE_STAKING and GITLAWB_OPERATOR_PRIVATE_KEY being non-empty; otherwise no timer is built at all and the node logs "on-chain PoS disabled".
Two related gaps on the same knob
The doc comment at config.rs:127 says the value "Must be less than the contract's HEARTBEAT_WINDOW (24h) with headroom", and nothing enforces it. GITLAWB_HEARTBEAT_INTERVAL_HOURS=25 parses and produces the same silent on-chain inactivity, just after the first missed window rather than immediately.
The * 3600 at main.rs:845 is unchecked. It overflows u64 above 5,124,095,576,030,431 hours, and in release that wraps rather than panicking, producing an arbitrary interval that the interval_hours = interval_secs / 3600 log at operator.rs:156 would then misreport. Worth noting for completeness, but that threshold is about 584 billion years and no plausible typo reaches it: an extra digit on the default 20 gives 200. This is an argument for fixing at the parser rather than at the arithmetic site, not a defect on its own.
Suggested fix
Add a value_parser range on the field so the node fails at boot rather than at spawn:
value_parser = clap::value_parser!(u64).range(1..=23)
The lower bound of 1 removes the panic. The upper bound of 23 encodes the protocol constraint the help text already promises, and that choice is deliberate: a purely arithmetic ceiling would accept values such as 25 that parse and run but are guaranteed to miss the contract window, which is the actual failure here rather than the overflow. Reject rather than clamp, so the running configuration keeps matching what the operator wrote.
Optionally make the multiply saturating as defense in depth for construction paths the parser does not cover, since tests build Config by direct field mutation, with a comment saying that is why it is there.
heartbeat_interval_hoursis declared with novalue_parserrange, so everyu64including0parses:Every other duration knob in that file carries
range(1..)(lines 182, 193, 203, 215, 225, 234). This one does not.The value reaches a timer through two hops.
crates/gitlawb-node/src/main.rs:845buildsDuration::from_secs(config.heartbeat_interval_hours * 3600), andcrates/gitlawb-node/src/operator.rs:161passes that Duration totokio::time::interval.The failure
With the value set to 0,
tokio::time::interval(Duration::ZERO)panics with "periodmust be non-zero". Verified against tokio 1.50.0, the pinned version, in a release build.The panic is inside the
tokio::spawnatoperator.rs:160, and the workspace release profile does not setpanic = "abort", so it takes down only the heartbeat task. The node keeps serving. Worse,operator.rs:155emits "operator heartbeat loop starting" at INFO before the spawn, so the startup logs read as healthy while no heartbeat is ever posted. The node then falls outside the contract's 24h HEARTBEAT_WINDOW and goes inactive on-chain.So the observable symptom is not a crash. It is a node that looks up, serves traffic, logs a started heartbeat loop, and quietly loses operator status a day later.
Reachability
This affects only nodes that opted into on-chain staking.
main.rs:502gates the whole path on bothGITLAWB_CONTRACT_NODE_STAKINGandGITLAWB_OPERATOR_PRIVATE_KEYbeing non-empty; otherwise no timer is built at all and the node logs "on-chain PoS disabled".Two related gaps on the same knob
The doc comment at
config.rs:127says the value "Must be less than the contract's HEARTBEAT_WINDOW (24h) with headroom", and nothing enforces it.GITLAWB_HEARTBEAT_INTERVAL_HOURS=25parses and produces the same silent on-chain inactivity, just after the first missed window rather than immediately.The
* 3600atmain.rs:845is unchecked. It overflowsu64above 5,124,095,576,030,431 hours, and in release that wraps rather than panicking, producing an arbitrary interval that theinterval_hours = interval_secs / 3600log atoperator.rs:156would then misreport. Worth noting for completeness, but that threshold is about 584 billion years and no plausible typo reaches it: an extra digit on the default 20 gives 200. This is an argument for fixing at the parser rather than at the arithmetic site, not a defect on its own.Suggested fix
Add a
value_parserrange on the field so the node fails at boot rather than at spawn:The lower bound of 1 removes the panic. The upper bound of 23 encodes the protocol constraint the help text already promises, and that choice is deliberate: a purely arithmetic ceiling would accept values such as 25 that parse and run but are guaranteed to miss the contract window, which is the actual failure here rather than the overflow. Reject rather than clamp, so the running configuration keeps matching what the operator wrote.
Optionally make the multiply saturating as defense in depth for construction paths the parser does not cover, since tests build
Configby direct field mutation, with a comment saying that is why it is there.