Skip to content

The node-to-client response enums have no wire-tag pin, while the client-to-node ones now do #102

Description

@sanity

#98 pinned the bincode variant tag of every variant of both delegate message enums, and #95 froze full byte vectors for ClientRequest, ContractRequest and DelegateRequest. The response direction has nothing.

Unpinned today:

  • HostResponse (rust/src/client_api/client_events.rs, 6 variants)
  • ContractResponse
  • QueryResponse (4 variants)
  • the error enums (RequestError, ErrorKind)

Why the asymmetry is worse than uniform absence

A half-pinned protocol reads as a pinned protocol. Someone appending a variant to QueryResponse sees a codebase full of wire-format pin tests, gets a green CI run, and reasonably concludes the guards covered them. Nothing did.

It also happens to be the direction with the incident history. The enums named in the v0.2.11 wire-format break — HostResponse, ContractResponse, RequestError, ErrorKind, QueryResponse — are precisely the unpinned set. The break there was a default encoding change rather than a tag reorder, but the class is the same and the guard would be the same.

And it is about to get more traffic: #5467 Phase 0 adds a NodeQuery variant and a matching QueryResponse variant for delegate diagnostics, with more expected in later phases.

The guard shape

Copy what #98 does for the delegate enums, which fails closed twice over:

  1. An exhaustive match mapping each variant to its frozen tag. #[non_exhaustive] has no effect inside the defining crate, so adding a variant without pinning it is a compile error rather than a silently-uncovered variant.
  2. A probe asserting the tag one past the last known variant does not decode, so the variant-count constant cannot drift. Assert the error names an invalid variant index — not ErrorKind::InvalidTagEncoding, which bincode never produces for an enum tag despite its description string saying "tag for enum is not valid".
  3. A control that the last known tag still decodes from the probe payload, so the probe cannot pass vacuously from the other end.

Two traps worth inheriting from #98's review, both of which produced tests that passed while measuring nothing:

  • Do not build fixtures from defaults. An all-default value often encodes as a run of zero bytes, and an assertion over zeros is satisfied by exactly the append it is meant to catch.
  • A round-trip through this crate's own encoder proves only self-consistency. Freezing a hand-written byte string is what pins a layout; a round-trip would pass just as happily with two fields swapped.

Verifying it

A filtered cargo test matching nothing prints 0 passed and exits 0. Assert a test count, and mutate each guard once to confirm it fails when the thing it guards is broken.

[AI-assisted - Claude]

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions