Skip to content

devshardctl: ParseProtocolVersion rejects route v4 (noisy rotation fallback log) #1542

Closed @maria-mitina opened 2026-08-04 15:58 UTC 5 comments Updated 2026-08-30 19:17 UTC
enhancement good first issue Priority: Low devshards

Summary

When the gateway serves DEVSHARD_ROUTE_PREFIX=/devshard/v4, escrow autorotation logs a fallback on every create:

escrow_rotation_protocol_version_fallback route_prefix="/devshard/v4" version="v4" reason=unparseable_protocol error=unknown protocol version "v4"

This is cosmetic: rotation create/settle still succeed. The empty local protocol_version stamp is not used for settlement (settlement uses StateRootAndProtocolVersion from the session SM, e.g. v4).

It clutters testnet/gateway logs and makes health checks harder to read.

Root cause

  • rotationEscrowProtocolVersion() in devshard/cmd/devshardctl/escrow_rotator.go derives a registry protocol stamp from the route version segment.
  • ParseProtocolVersion in devshard/types/domain.go only accepts v1 / v2 / v3.
  • "v4" hits the default branch → log + return "".

Observed impact

  • Does not fail escrow rotation (fallback log is followed by escrow_rotation_created).
  • Does not affect settlement / state roots.
  • Only leaves gateway DB protocol_version empty instead of a parsed enum.

Suggested fix (small)

Either:

  1. Add ProtocolV4 / "v4" to ParseProtocolVersion (and tests), if v4 should be a real protocol stamp; or
  2. Stop treating unknown route majors as an error-level fallback (silent empty stamp / explicit config), if route name and protocol enum are intentionally decoupled.

Also worth aligning compose healthcheck (curl vs image wget) separately — not required for this issue.

Test plan

  • Unit: ParseProtocolVersion("v4") (or chosen behavior) covered
  • With DEVSHARD_ROUTE_PREFIX=/devshard/v4, autorotation creates escrows without unparseable_protocol spam
  • Settlement still carries state_root_and_protocol_version=v4 as today

💬 Comments (5)

@w3lld1 commented 2026-08-05 17:32 UTC

I'd like to take this. I plan to add v4 support to ParseProtocolVersion, extend the focused unit coverage, and verify the escrow rotation parsing path. I can submit the PR within a day. Could you confirm that adding ProtocolV4 is preferred over keeping route majors decoupled, and assign me?

@bonujel commented 2026-08-20 01:50 UTC

ParseProtocolVersion has one caller — escrow_rotator.go:408, the path in the report. Grepping for anything that reads the result turns up nothing:

ParseProtocolVersion    -> devshard/cmd/devshardctl/escrow_rotator.go:408  (only non-test caller)
ProtocolV1/V2/V3        -> no readers outside devshard/types/domain.go and its tests
types.ProtocolVersion   -> no consumers

So the parsed value gets stamped into the gateway DB and is never read back for a decision, which matches the note in the description that settlement goes through StateRootAndProtocolVersion from the session SM.

That points at option 2. Nothing downstream needs the value to be a known enum member, so an unknown route major isn't really an error condition — it's a name the enum hasn't been told about.

Option 1 also has a shelf life: v5 is already in flight (#1584, #1615). Adding ProtocolV4 brings the same log back the day the route prefix moves to /devshard/v5.

@w3lld1 asked the same question on Aug 5 and hasn't had an answer — this is the evidence for it either way. If option 2 is the call, I can put up the patch.

@wrvnnull commented 2026-08-28 17:19 UTC

I'd like to take this.\n\nPlan:\n- add ProtocolV4 to ParseProtocolVersion\n- extend unit test to cover v4\n- keep current v1/v2/v3 behavior unchanged\n\nETA: immediate PR.

@wrvnnull commented 2026-08-30 10:43 UTC

Tracked in #1630 (gateway v4). #1670 was a duplicate and is closed. PR #1678 covers the API-side malformed JSON fix for /v1/participants.


🔄 Auto-synced from Issue #1542 every hour.