Interface-driven coverage measurement (follow-up to #196)

[bkontur]

Motivation

The coverage gate from #196 counts reach via all tests: a patch line covered only by a private unit test passes even if no exposed interface reaches it. The review discussion on #196 (comment) proposed evolving toward interface-driven measurement: a line counts as covered only when it is reachable by entering through the component's own exposed interface.

The property this buys: "the code is wired into the product, not just into a test" — it flags dead code whose only remaining caller is its test.

#196 already ships the diagnostic half (integration-only view in coverage-integration.md, not gated). This issue tracks whether/how to go further.

Current state

scripts/coverage.sh produces two views from one test run:

View	Source	Role
merged	all test targets	gated via diff-cover (MIN_PATCH_COV)
integration-only	--test '*' targets	diagnostic, not gated

The integration-only view approximates "entered via the exposed interface" for provider-node (its tests/ suites drive the axum router and the trait-mocked coordinators), but zeroes out the pallets: pallet tests live in src/ behind mock.rs per polkadot-sdk convention, so they land in the unit bucket even though they enter through extrinsic dispatch — which is the pallet's public surface.

"Exposed interface" is wider than extrinsics + HTTP

Concrete in-repo entry points that a naive definition would mislabel as unreachable:

Entry point	Where	Entered by
Hooks (on_initialize/on_finalize)	pallet/src/lib.rs (challenge slashing runs here)	block execution
migrations (OnRuntimeUpgrade)	pallet/src/migrations.rs, storage-interfaces/file-system/pallet-registry/src/migrations.rs	runtime upgrade
runtime APIs (provider_info, providers, bucket_info, …)	declared in pallet/src/runtime_api.rs, implemented in the runtime crates	node RPC, across the runtime boundary — never an extrinsic
library crates (Rust API, no wire protocol)	provider-negotiation (consumed by both node and client)	pub API

The runtime-API row deserves emphasis: today the backing pallet query functions count as covered only because pallet/src/tests/runtime_api.rs calls them directly, while their real callers (the impl_runtime_apis! blocks) sit in crates the gate skips entirely. That's exactly the "reached by test, not by interface" pattern — except here it is the correct outcome, which a naive interface gate would flag as dead.

dead_code already covers part of this

The dead-code failure mode motivating the proposal is already partially enforced: a private helper whose only non-test caller is removed trips rustc's dead_code lint in the non-cfg(test) compilation, and CI clippy runs with RUSTFLAGS: -D warnings (.github/workflows/check.yml).

What that does not catch is unused pub items — the residual class interface-gating would add. But per the runtime-API row above, for cross-crate pub surface "unused" cannot even be decided inside the measured crate.

Problems any gating design must answer

1. No tool measures call-graph origin. Neither cargo-llvm-cov nor lcov can express "count this line only if the call stack entered via X". The only mechanization Rust offers is visibility: code in tests/ can only call pub items. Interface-gating in practice therefore means "gate on tests/-target coverage", which for pallets means moving mock.rs + tests out of src/ — against SDK-wide convention, and it forbids testing internal invariants directly.

2. Per-component interface definition. Pallet = Call dispatch + Hooks + inherents + migrations + runtime APIs (via mock runtime); provider-node = axum router + coordinator entry points; provider-negotiation = its pub API. This list must live somewhere reviewed, like COV_PACKAGES does today.

3. Defensive code is unreachable by design. defensive_* branches and can't-happen error arms are required by our own guidelines and reachable through no interface. A strict interface gate creates pressure to delete exactly the code we mandate. Needs an explicit carve-out (or stays the reason gating is a bad idea).

4. Name the check after the property. If the gate's purpose becomes dead-code detection rather than a quality metric, the CI job and summary headings should say so — "Rust coverage" reads as "how well tested is this", which invites percentage-chasing. Something like "Dead code: not reachable from an exposed interface" states what a failure means and what the fix is (wire it in or delete it — not "add a unit test"). Cheap: branch protection pins the aggregate basic-checks job, so renaming the coverage job/headings breaks nothing.

Potential directions

A — status quo+

Keep interface-reach as diagnostic column + review convention; rely on dead_code + -D warnings for the private-item case. Cheapest, loses nothing we currently have. Even here, renaming the diagnostic heading (e.g. "Reachable from exposed interfaces" instead of "Integration-only coverage") would make the column's intent self-explanatory.

B — lint the residue

Attack unused-pub directly instead of via coverage — e.g. unreachable_pub where applicable, periodic dead-pub sweeps. Solves the motivating failure mode without touching test layout, but must whitelist cross-crate surface like runtime-API backing functions whose callers live in skipped crates.

C — full interface gating

Restructure pallet tests into tests/, define the per-component interface list, add defensive-code carve-outs, gate diff-cover on lcov-integration.info, and rename the job per problem 4. Highest cost; blocked on problems 1–4 above.

TODO

• Agree which direction (A/B/C) is worth the cost
• Rename job/headings to state the property being checked (applies to any direction)
• If B: pick the lint/tooling and wire it into check.yml
• If C: define the per-component interface list and the defensive-code carve-out first

Related

• ci(coverage): gate on patch coverage, make the job diagnostic and robust #196 — coverage gate + integration-only diagnostic view (this issue's origin)
• Make the Rust coverage gate robust and diagnostic (filename-based exclusion + whole-crate ratio is fragile) #176 — motivating coverage issue
• Formal verification: let's start with the challenge protocol and see #265 — Quint spec for the challenge protocol (correctness assurance; coverage proves reach, not correctness)
• Repo structure: split provider-node, break the storage-client dependency, tidy crate layout #178 — provider split (changes the component list any interface definition hangs off)