fix: Intermediate nodes for GET/PUT/UPDATE have stats: None, blind to health tracking

[sanity]

Problem

PR #3526 fixed the stats: None gap for subscribe operations at intermediate forwarding nodes. The same gap exists in GET, PUT, and UPDATE operations:

• get.rs:1056 — intermediate GET forward sets stats: None
• put.rs:256 — intermediate PUT forward sets stats: None
• update.rs:312 — intermediate UPDATE forward sets stats: None

When an intermediate node forwards one of these operations and it times out, the failure is invisible to both PeerHealthTracker and the router's isotonic failure estimator. This means black-hole peers are never penalized for dropping these operations.

Subscribe was hit hardest because of its multi-hop chain topology, but GET/PUT/UPDATE have the same structural blind spot.

Approach

Follow the same pattern as the subscribe fix in PR #3526: set stats: Some(...) with the target peer and contract location when forwarding at intermediate nodes.

Context

Discovered during investigation of #3523 (black-hole peers). The subscribe fix is in PR #3526.

[AI-assisted - Claude]

[github-actions]

🤖 Auto-labeled

Applied labels:

• T-bug (98% confidence)
• P-high (85% confidence)
• E-easy (90% confidence)
• A-networking (95% confidence)

Reasoning: The report describes incorrect behavior (intermediate GET/PUT/UPDATE forwards set stats: None, causing failures to be invisible to PeerHealthTracker and failure estimators), so it's a bug. It affects core routing/peer-health and could allow black-hole peers to avoid penalty, so priority is high. The fix follows an existing pattern (subscribe fix in PR #3526) and appears to be a small change in three locations, so effort is likely easy. The issue pertains to networking/routing and touches contract location metadata when setting stats, so A-networking and A-contracts are relevant.

Previous labels: none

If these labels are incorrect, please update them. This helps improve the auto-labeling system.

[sanity]

Issue Priority Assessment

Generated: 2026-03-12T22:25:02.392710838+00:00
Issue hash: 9cf7329e

Value Assessment

Value Assessment

The issue is still very much relevant: GET, PUT, and UPDATE operations continue to set stats: None when an intermediate node instantiates the operation from a forwarded request (crates/core/src/operations/get.rs:1056, crates/core/src/operations/put.rs:256, crates/core/src/operations/update.rs:312). Because no stats are captured at that point, those forwarders never populate the next_peer/contract_location pair that GetOp::outcome, PutOp::outcome, and UpdateOp::outcome require in order to turn a timeout into ContractOpFailure instead of OpOutcome::Incomplete (crates/core/src/operations/get.rs:553, crates/core/src/operations/put.rs:49, crates/core/src/operations/update.rs:133). In contrast, the publish/subscribe fix in PR #3526 already updates self.stats before retrying a forward (crates/core/src/operations/subscribe.rs:702), so the pattern is known and simply needs to be copied over to the other operations.

Because the OpOutcome machinery is what drives routing telemetry, the lack of stats leaves operators blind to misbehaving peers. node.rs only emits a RouteEvent when OpOutcome is a success or a failure, and it drops the event entirely if the outcome is Incomplete (crates/core/src/node.rs:588). Those RouteEvents are what feed the router’s failure estimator, as demonstrated by the test_subscribe_outcome_feeds_router/test_get_partial_timing_feeds_router cases that check router.failure_estimator.len() increments only when an event exists (crates/core/src/router.rs:1291 and crates/core/src/router.rs:1320). Without route events, the PeerHealthTracker in ring/connection_manager.rs never records failures or successes (crates/core/src/ring/connection_manager.rs:134-171), so a peer that black-holes GET/PUT/UPDATE traffic will keep looking healthy, remain in the ring, and continue diverting critical requests.

Fixing this has high user impact because it restores visibility into every hop’s success/failure history, which is core to the project’s reliability goals and the high-priority work around black-hole peers (see issue #3523). It doesn’t block other work directly, but it prevents downstream monitoring and eviction logic from functioning correctly for three major operation types, so the networking stack cannot accurately adapt to failing peers. Given the existing findings in #3523 and the fact that the subscribe code already uses the desired pattern (stats: Some) for forwarding, this is a logical, relatively easy follow-up that should be addressed soon while the surrounding health-tracking investigation is still fresh.

Cost Assessment

The gap is still there: load_or_init for GET, PUT, and UPDATE forwarding nodes still initializes stats to None (crates/core/src/operations/get.rs:1056, crates/core/src/operations/put.rs:256, crates/core/src/operations/update.rs:312), so an intermediate hop never records which peer it forwarded to and nothing ever hits PeerHealthTracker or the isotonic failure estimator if the forward times out. Nothing in the surrounding state machines has been reworked since PR #3526—forwarders still call helpers like try_forward_or_return and the existing GetStats/PutStats/UpdateStats structures are the right place to store the target_peer/contract_location pair—so the prescribed fix (set stats: Some(...) with the target info when you hand the request off) still applies directly and the issue remains actionable.

Scope is very limited to those three operation modules plus the stats structs that they already populate for originators. The forwarding branches in each handler are clearly defined: GET’s try_forward_or_return returns an OpEnum::Get with whatever stats value it inherited from the incoming message, so one change there and one where the forwarder prepares the stats prior to sending the GetMsg::Request should be enough. PUT’s forwarding case around crates/core/src/operations/put.rs:387‑540 already sets stats when the originator forwards, and UPDATE’s forwarding block does the same for the originator path—the missing piece is that intermediate nodes never set those fields when they themselves forward. Touching these spots won’t cascade into other subsystems, and GetStats/PutStats/UpdateStats are already consumed for OpOutcome and timeout tracking so no new plumbing is required.

The implementation complexity is low: you only need to populate the existing stats objects with the next hop and contract location before emitting the forwarded Request and ensure they remain Some for the remainder of the op. Because it mirrors the subscribe fix, there are no new external dependencies, approvals, or long waits—just code changes within crates/core/src/operations. Risk is minimal if you respect the current orthogonal handling for originators (don’t override their existing stats data) and keep the telemetry/time-sampling invariants, but you should add or update unit tests to assert that forwarded nodes now produce a ContractOpFailure/ContractOpSuccess outcome that includes the target peer so PeerHealthTracker sees failures.

---

Generated by issue-prioritizer