Skip to content

Conversation

@mdheller
Copy link
Member

Motivation

  • Fix cross-language protocol mismatches (Rust used zeroed schema/context) so fixtures remain the single source of truth.
  • Ensure deterministic AEAD AAD/tag computation and enforce full-frame parity (not just tag/payload checks).
  • Add minimal structured decoders and Avro round-trip checks so fixture semantics are verifiable per-language.
  • Improve repo ergonomics with a standard local make verify flow and CI that runs the same checks.

Description

  • Align Rust envelope IDs with canonical constants and use them when building envelopes, and add a Rust envelope decoder that exposes AAD/tag boundaries.
  • Harden Go: add DecodeEnvelope + AADBeforeTag, a TLEB3 decode helper, Avro decoders/encoders for HGRequest/HGResponse, and repack/round-trip checks in fixtures_test.go.
  • Add round-trip Avro decode/encode checks and full-frame decode -> build equality assertions in Rust fixture tests, plus constant-time AEAD tag comparisons (subtle usage).
  • Documentation and tooling updates: parity audit, integration readiness checklist, Makefile with make verify, and a CI workflow that runs format + tests + fixture verification; spec/docs updated to reflect port behavior (XChaCha20-Poly1305 only, AUX opaque, explicit per-frame nonces).

Testing

  • Ran formatting checks (cargo fmt, gofmt) successfully as part of verification steps.
  • Attempted go test ./... but it failed due to network/proxy restrictions when fetching modules (dependency download blocked).
  • Attempted cargo test but it failed due to crates.io index download being blocked by network/proxy restrictions.
  • Ran the Python fixture verifier but it failed locally because the cryptography package (XChaCha20-Poly1305) was not installed in the environment.

Codex Task

@mdheller mdheller self-assigned this Jan 12, 2026
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1da6dde25a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +35 to +38
b := buf[off]
off++
ts, _ := TritUnpack243([]byte{b})
trits = append(trits, ts...)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle TritPack243 tail markers in TLEB3DecodeLen

TLEB3EncodeLen uses TritPack243, which emits a 2‑byte tail marker (243..246 + value) whenever the trit count isn’t a multiple of 5 (e.g., any length with 1–4 base‑9 digits, including 0..6560). TLEB3DecodeLen reads one byte at a time and calls TritUnpack243([]byte{b}) while discarding the error, so if the next byte is a tail marker the decode silently drops trits and then interprets the tail value byte as a full 5‑trit group. That produces the wrong length and causes DecodeEnvelope/splitFields to mis-parse most frames.

Useful? React with 👍 / 👎.

Comment on lines +58 to +60
pack := TritPack243(trits[:used])
usedBytes := len(pack)
return v, offset + usedBytes - 1 + (off - offset), nil

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Fix newOff advance for multi-byte lengths

off already points past the bytes read for this length, but newOff adds usedBytes on top of that (offset + usedBytes - 1 + (off - offset)). For any length that consumes more than one byte, this advances past the actual field boundary (e.g., a two‑byte length returns offset+3 instead of offset+2), so subsequent fields are misaligned and AAD/tag slicing becomes incorrect.

Useful? React with 👍 / 👎.

@mdheller mdheller merged commit 025907e into main Jan 12, 2026
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant