From 9612f08e460e9986b8750ff7fe28831cb70d0d3f Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Sat, 6 Jun 2026 21:05:50 -0700 Subject: [PATCH] fix(#1610): decode firmware 1.16.0 extended ACK (5/6-byte payloads) (#1618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Firmware 1.16.0 (`companion-v1.16.0`) ships variable-length `PAYLOAD_TYPE_ACK` payloads: 4 bytes (legacy) → 5 bytes (4-byte CRC + 1-byte attempt, commit `f6e6fdaa`) → 6 bytes (+ 1-byte RNG, commit `a130a95a`). CoreScope's decoder previously truncated past the 4-byte CRC and discarded the attempt + RNG bytes. This PR teaches `cmd/ingestor/decoder.go` to surface the extended bytes on the decoded payload so the DB/UI can distinguish v1.15 vs v1.16 senders, with no schema or wire-compat changes. Partial fix for #1610 — top-level ACK + multipart-inner ACK are covered. PATH-extra ACK parsing (`decodePathPayload`) is deferred to #1612 per triage. ## Changes - `decodeAck` reads 4/5/6-byte payloads. Keeps `extraHash` (4-byte CRC) for compat; adds optional `ackLen`, `ackAttempt`, `ackRand` JSON fields. Legacy 4-byte ACKs leave attempt/rand `nil`. - `decodeMultipart` ACK branch relaxes the `len >= 5` floor so the inner blob can be 4/5/6 bytes (multipart `payload_len` 5/6/7). Adds `innerAckLen`, `innerAckAttempt`, `innerAckRand`. - All additions are `omitempty` — backwards-compatible JSON only. No DB column, no schema migration, no frontend change. ## Out of scope (per issue triage) - `decodePathPayload` PATH-extra parsing — tracked separately in #1612. - Frontend rendering of attempt counter — leave for a follow-up if the DB/UI eventually wants to display it. ## TDD - **Red commit `3fce0465`** adds `cmd/ingestor/issue1610_test.go` with 6 new assertions (legacy 4-byte, extended 5/6-byte, multipart variants of each). New fields are declared on `Payload` so the test compiles, but no decoder populates them yet — tests fail on `ackLen= want 4` etc. Verified isolation with `git stash` of decoder.go + re-run. - **Green commit `5165c202`** implements the decoder changes. `go test ./...` in `cmd/ingestor` passes. ## Fixtures Synthetic wire vectors built by hand against the firmware spec — the issue did not provide real captures. Each test cites the firmware ref + commit it derives from (`BaseChatMesh.cpp:218-234`, commits `f6e6fdaa` and `a130a95a`). ## References - Issue #1610 - Firmware tag `companion-v1.16.0` @ `07a3ca9e` - Upstream PR meshcore-dev/MeshCore#2594 - Blog: https://blog.meshcore.io/2026/06/06/release-1-16-0 --------- Co-authored-by: corescope-bot --- cmd/ingestor/decoder.go | 49 +++++++++++- cmd/ingestor/issue1610_test.go | 134 +++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 cmd/ingestor/issue1610_test.go diff --git a/cmd/ingestor/decoder.go b/cmd/ingestor/decoder.go index 1c8abc5b..cba3fe15 100644 --- a/cmd/ingestor/decoder.go +++ b/cmd/ingestor/decoder.go @@ -109,6 +109,15 @@ type Payload struct { MAC string `json:"mac,omitempty"` EncryptedData string `json:"encryptedData,omitempty"` ExtraHash string `json:"extraHash,omitempty"` + // Extended ACK fields per firmware 1.16.0 (issue #1610) — + // firmware/src/helpers/BaseChatMesh.cpp:218-234. ACK payloads grew from + // always-4 bytes to 4/5/6 (4-byte truncated sha256 CRC, optional 1-byte + // attempt counter, optional 1-byte RNG byte added in commit a130a95a). + // AckLen is the wire payload length; AckAttempt/AckRand are surfaced + // only when the sender included them (legacy 4-byte ACKs leave them nil). + AckLen *int `json:"ackLen,omitempty"` + AckAttempt *int `json:"ackAttempt,omitempty"` + AckRand *int `json:"ackRand,omitempty"` PubKey string `json:"pubKey,omitempty"` Timestamp uint32 `json:"timestamp,omitempty"` TimestampISO string `json:"timestampISO,omitempty"` @@ -148,6 +157,12 @@ type Payload struct { InnerType *int `json:"innerType,omitempty"` InnerTypeName string `json:"innerTypeName,omitempty"` InnerAckCrc string `json:"innerAckCrc,omitempty"` + // Extended ACK inner fields (issue #1610) — when the multipart inner + // blob is a v1.16+ extended ACK (5 or 6 bytes after the byte0 header), + // surface the same attempt/rand bytes as the top-level decoder. + InnerAckLen *int `json:"innerAckLen,omitempty"` + InnerAckAttempt *int `json:"innerAckAttempt,omitempty"` + InnerAckRand *int `json:"innerAckRand,omitempty"` InnerPayload string `json:"innerPayload,omitempty"` // CONTROL (PAYLOAD_TYPE_CONTROL=0x0B) byte0 flags, per // firmware/src/Mesh.cpp:69 — byte0 high-bit marks zero-hop direct subset. @@ -266,10 +281,27 @@ func decodeAck(buf []byte) Payload { return Payload{Type: "ACK", Error: "too short", RawHex: hex.EncodeToString(buf)} } checksum := binary.LittleEndian.Uint32(buf[0:4]) - return Payload{ + ackLen := len(buf) + if ackLen > 6 { + ackLen = 6 + } + p := Payload{ Type: "ACK", ExtraHash: fmt.Sprintf("%08x", checksum), + AckLen: &ackLen, } + // Firmware 1.16.0 extended ACK (issue #1610): 5th byte is the attempt + // counter (commit f6e6fdaa), 6th byte is a random byte added so identical + // attempts still hash uniquely (commit a130a95a). + if len(buf) >= 5 { + attempt := int(buf[4]) + p.AckAttempt = &attempt + } + if len(buf) >= 6 { + rnd := int(buf[5]) + p.AckRand = &rnd + } + return p } func decodeAdvert(buf []byte, validateSignatures bool) Payload { @@ -664,6 +696,21 @@ func decodeMultipart(buf []byte) Payload { // to match decodeAck's extraHash convention. crc := binary.LittleEndian.Uint32(buf[1:5]) p.InnerAckCrc = fmt.Sprintf("%08x", crc) + // Firmware 1.16.0 extended ACK (issue #1610): inner ACK blob may be + // 5 or 6 bytes (payload_len = 1 + ack_len) instead of always 4. + ackLen := len(buf) - 1 + if ackLen > 6 { + ackLen = 6 + } + p.InnerAckLen = &ackLen + if len(buf) >= 6 { + attempt := int(buf[5]) + p.InnerAckAttempt = &attempt + } + if len(buf) >= 7 { + rnd := int(buf[6]) + p.InnerAckRand = &rnd + } } else if len(buf) > 1 { p.InnerPayload = hex.EncodeToString(buf[1:]) } diff --git a/cmd/ingestor/issue1610_test.go b/cmd/ingestor/issue1610_test.go new file mode 100644 index 00000000..74689617 --- /dev/null +++ b/cmd/ingestor/issue1610_test.go @@ -0,0 +1,134 @@ +package main + +// Tests for issue #1610: firmware 1.16.0 extended ACK support. +// +// Wire vectors are synthetic, derived by hand from the firmware spec: +// - Variable-length ACK on the wire: +// firmware/src/Mesh.cpp:545-575 createAck/createMultiAck (commit f6e6fdaa) +// - 5-byte ACK = 4-byte truncated sha256 CRC + 1-byte attempt counter: +// firmware/src/helpers/BaseChatMesh.cpp:218-232 (commit f6e6fdaa) +// - 6-byte ACK = 5-byte + 1-byte RNG (so identical attempts get unique hash): +// firmware/src/helpers/BaseChatMesh.cpp:219-234 (commit a130a95a) +// - Multipart ACK inner blob: firmware/src/Mesh.cpp:292-307 — byte0 then +// ack bytes, payload_len = 1 + ack_len. + +import ( + "testing" +) + +// --- top-level ACK (decodeAck) --- + +func TestDecodeAckLegacy4Byte(t *testing.T) { + // Backwards-compat: 4-byte ACK leaves the new optional fields nil. + buf := []byte{0xAA, 0xBB, 0xCC, 0xDD} + p := decodeAck(buf) + if p.ExtraHash != "ddccbbaa" { + t.Errorf("extraHash=%q want ddccbbaa", p.ExtraHash) + } + if p.AckLen == nil || *p.AckLen != 4 { + t.Errorf("ackLen=%v want 4", p.AckLen) + } + if p.AckAttempt != nil { + t.Errorf("ackAttempt=%v want nil for legacy 4-byte ACK", *p.AckAttempt) + } + if p.AckRand != nil { + t.Errorf("ackRand=%v want nil for legacy 4-byte ACK", *p.AckRand) + } +} + +func TestDecodeAck5ByteExtended(t *testing.T) { + // v1.16 sender (commit f6e6fdaa): 4-byte CRC + 1-byte attempt. + buf := []byte{0xAA, 0xBB, 0xCC, 0xDD, 0x07} + p := decodeAck(buf) + if p.ExtraHash != "ddccbbaa" { + t.Errorf("extraHash=%q want ddccbbaa", p.ExtraHash) + } + if p.AckLen == nil || *p.AckLen != 5 { + t.Errorf("ackLen=%v want 5", p.AckLen) + } + if p.AckAttempt == nil || *p.AckAttempt != 7 { + t.Errorf("ackAttempt=%v want 7", p.AckAttempt) + } + if p.AckRand != nil { + t.Errorf("ackRand=%v want nil for 5-byte ACK", *p.AckRand) + } +} + +func TestDecodeAck6ByteExtended(t *testing.T) { + // v1.16 sender (commit a130a95a): 4-byte CRC + 1-byte attempt + 1-byte RNG. + buf := []byte{0xAA, 0xBB, 0xCC, 0xDD, 0x02, 0x5A} + p := decodeAck(buf) + if p.ExtraHash != "ddccbbaa" { + t.Errorf("extraHash=%q want ddccbbaa", p.ExtraHash) + } + if p.AckLen == nil || *p.AckLen != 6 { + t.Errorf("ackLen=%v want 6", p.AckLen) + } + if p.AckAttempt == nil || *p.AckAttempt != 2 { + t.Errorf("ackAttempt=%v want 2", p.AckAttempt) + } + if p.AckRand == nil || *p.AckRand != 0x5A { + t.Errorf("ackRand=%v want 90", p.AckRand) + } +} + +// --- multipart-with-ACK (decodeMultipart) --- + +// buildMultipartAckByte0: remaining<<4 | PayloadACK (0x02). +func buildMultipartAckByte0(remaining int) byte { + return byte((remaining<<4)&0xF0) | byte(PayloadACK&0x0F) +} + +func TestDecodeMultipartAck4ByteLegacy(t *testing.T) { + // Pre-1.16 inner ACK is 4 bytes → ackLen=4, attempt/rand nil. + buf := []byte{buildMultipartAckByte0(3), 0xAA, 0xBB, 0xCC, 0xDD} + p := decodeMultipart(buf) + if p.InnerAckCrc != "ddccbbaa" { + t.Errorf("innerAckCrc=%q want ddccbbaa", p.InnerAckCrc) + } + if p.InnerAckLen == nil || *p.InnerAckLen != 4 { + t.Errorf("innerAckLen=%v want 4", p.InnerAckLen) + } + if p.InnerAckAttempt != nil { + t.Errorf("innerAckAttempt=%v want nil", *p.InnerAckAttempt) + } + if p.InnerAckRand != nil { + t.Errorf("innerAckRand=%v want nil", *p.InnerAckRand) + } +} + +func TestDecodeMultipartAck5Byte(t *testing.T) { + // v1.16: byte0 + 4-byte CRC + 1-byte attempt → payload_len = 6. + buf := []byte{buildMultipartAckByte0(1), 0xAA, 0xBB, 0xCC, 0xDD, 0x09} + p := decodeMultipart(buf) + if p.InnerAckCrc != "ddccbbaa" { + t.Errorf("innerAckCrc=%q want ddccbbaa", p.InnerAckCrc) + } + if p.InnerAckLen == nil || *p.InnerAckLen != 5 { + t.Errorf("innerAckLen=%v want 5", p.InnerAckLen) + } + if p.InnerAckAttempt == nil || *p.InnerAckAttempt != 9 { + t.Errorf("innerAckAttempt=%v want 9", p.InnerAckAttempt) + } + if p.InnerAckRand != nil { + t.Errorf("innerAckRand=%v want nil for 5-byte inner ACK", *p.InnerAckRand) + } +} + +func TestDecodeMultipartAck6Byte(t *testing.T) { + // v1.16: byte0 + 4-byte CRC + 1-byte attempt + 1-byte RNG → payload_len = 7. + buf := []byte{buildMultipartAckByte0(0), 0xAA, 0xBB, 0xCC, 0xDD, 0x04, 0xC3} + p := decodeMultipart(buf) + if p.InnerAckCrc != "ddccbbaa" { + t.Errorf("innerAckCrc=%q want ddccbbaa", p.InnerAckCrc) + } + if p.InnerAckLen == nil || *p.InnerAckLen != 6 { + t.Errorf("innerAckLen=%v want 6", p.InnerAckLen) + } + if p.InnerAckAttempt == nil || *p.InnerAckAttempt != 4 { + t.Errorf("innerAckAttempt=%v want 4", p.InnerAckAttempt) + } + if p.InnerAckRand == nil || *p.InnerAckRand != 0xC3 { + t.Errorf("innerAckRand=%v want 195", p.InnerAckRand) + } +}