Files
meshcore-spec/PYMC_CORE_ISSUES.md
swaitsandClaude Opus 4.6 9db901088e Add comprehensive spec audit results and pyMC_core issue reports
- CHANGELOG.md: Full audit findings organized by severity
- PYMC_CORE_ISSUES.md: 7 evidence-based bug reports for pyMC_core
  (wrong constants, broken path_len decoding, wrong TRACE hash,
  wrong SNR calculation)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add comprehensive spec audit results and pyMC_core issue reports

- CHANGELOG.md: Full audit findings organized by severity
- PYMC_CORE_ISSUES.md: 7 evidence-based bug reports for pyMC_core
  (wrong constants, broken path_len decoding, wrong TRACE hash,
  wrong SNR calculation)
- Remove ERRATA.md: contents already covered by audit findings

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-01 21:37:57 -06:00

13 KiB
Raw Permalink Blame History

pyMC_core Issues for GitHub


Issue 1: constants.py — CIPHER_MAC_SIZE is 32, should be 2

Summary

CIPHER_MAC_SIZE in src/pymc_core/protocol/constants.py is set to 32, but the C++ firmware defines it as 2. This constant controls HMAC-SHA256 truncation length for the encrypt-then-MAC scheme. The correct value of 2 is actually used in crypto.py, creating an internal inconsistency.

Evidence

pyMC_core (src/pymc_core/protocol/constants.py, line 52):

CIPHER_MAC_SIZE = 32  # SHA‑256 HMAC

pyMC_core (src/pymc_core/protocol/crypto.py, line 12):

CIPHER_MAC_SIZE = 2  # matches firmware

C++ firmware (src/MeshCore.h, line 16):

#define CIPHER_MAC_SIZE      2

C++ usage (src/Utils.cpp, line 69):

sha.finalizeHMAC(shared_secret, PUB_KEY_SIZE, dest, CIPHER_MAC_SIZE);

This truncates the HMAC-SHA256 output to 2 bytes.

Impact

Any code importing CIPHER_MAC_SIZE from constants.py (rather than crypto.py) will use 32 instead of 2. This would cause:

  • Incorrect MAC length in packet construction/validation
  • Interoperability failure with C++ firmware nodes
  • crypto.py shadows this with its own correct definition, so the crypto module itself works, but any other module importing from constants.py will get the wrong value.

Fix

Change constants.py line 52 to:

CIPHER_MAC_SIZE = 2  # HMAC-SHA256 truncated to 2 bytes

Issue 2: constants.py — MAX_HASH_SIZE is 32, should be 8

Summary

MAX_HASH_SIZE in constants.py is set to 32, but the C++ firmware defines it as 8. This constant controls the truncation length of SHA-256 packet hashes used for deduplication.

Evidence

pyMC_core (src/pymc_core/protocol/constants.py, line 58):

MAX_HASH_SIZE = 32  # SHA-256 truncated

C++ firmware (src/MeshCore.h, line 6):

#define MAX_HASH_SIZE        8

C++ usage (src/Packet.cpp, line 49):

sha.finalize(hash, MAX_HASH_SIZE);  // Truncates SHA-256 to 8 bytes

pyMC_core usage (src/pymc_core/protocol/packet_utils.py, line 219):

return sha.digest()[:MAX_HASH_SIZE]  # Will take 32 bytes instead of 8

Impact

  • calculate_packet_hash() returns 32 bytes instead of 8
  • calculate_crc() (line 248) extracts the first 4 bytes of the hash, so CRC computation is accidentally correct (first 4 bytes of a 32-byte hash equal first 4 bytes of an 8-byte hash since both are prefixes of the same SHA-256)
  • However, any code comparing full packet hashes for deduplication will fail to match against C++ firmware hashes (32 bytes vs 8 bytes)

Fix

Change constants.py line 58 to:

MAX_HASH_SIZE = 8  # SHA-256 truncated to 8 bytes for deduplication

Issue 3: constants.py — MAX_PACKET_PAYLOAD is 256, should be 184

Summary

MAX_PACKET_PAYLOAD is defined as 256 in constants.py (appears twice, lines 54 and 57), but the C++ firmware defines it as 184.

Evidence

pyMC_core (src/pymc_core/protocol/constants.py, lines 54 and 57):

MAX_PACKET_PAYLOAD = 256  # firmware's default
# ...
MAX_PACKET_PAYLOAD = 256

C++ firmware (src/MeshCore.h, line 19):

#define MAX_PACKET_PAYLOAD  184

C++ firmware (src/MeshCore.h, line 20):

#define MAX_TRANS_UNIT      255

Note: MAX_TRANS_UNIT (255) is the max total packet size on the wire. MAX_PACKET_PAYLOAD (184) is the max payload portion within a packet. The difference accounts for header (1), path_len (1), and up to 64 bytes of path data plus optional 4-byte transport codes: 255 - 1 - 1 - 64 - 4 = 185, rounded down to 184.

Impact

  • validate_payload_size() in packet_utils.py (line 100) accepts payloads up to 256 bytes, which would overflow the C++ firmware's 184-byte payload buffer
  • Packets constructed with payloads between 185-256 bytes will be silently accepted by pyMC_core but will be rejected or cause buffer overflows on firmware nodes

Fix

Remove the duplicate definition and set correctly:

MAX_PACKET_PAYLOAD = 184  # Maximum payload size in bytes

Issue 4: constants.py — MAX_ADVERT_DATA_SIZE is 96, should be 32

Summary

MAX_ADVERT_DATA_SIZE is set to 96 in constants.py, but the C++ firmware defines it as 32.

Evidence

pyMC_core (src/pymc_core/protocol/constants.py, line 48):

MAX_ADVERT_DATA_SIZE = 96

C++ firmware (src/MeshCore.h, line 11):

#define MAX_ADVERT_DATA_SIZE  32

C++ usage (src/Mesh.cpp, line 391):

if (app_data_len > MAX_ADVERT_DATA_SIZE) return NULL;  // Reject adverts > 32 bytes app_data

C++ usage (src/Mesh.cpp, lines 254-255, receive side):

if (app_data_len > MAX_ADVERT_DATA_SIZE) { app_data_len = MAX_ADVERT_DATA_SIZE; }

The receiver truncates app_data to 32 bytes even if more was somehow received.

Impact

  • pyMC_core could construct advertisement packets with up to 96 bytes of app_data
  • C++ firmware will either reject these (on send) or silently truncate to 32 bytes (on receive), losing data
  • Signature verification will fail because the receiver computes the signature over truncated app_data while the sender signed over the full app_data

Fix

Change constants.py line 48 to:

MAX_ADVERT_DATA_SIZE = 32

Issue 5: packet.py — path_len treated as raw byte count, ignoring hash_size/hash_count encoding

Summary

The C++ firmware encodes path_len as a bitfield: the upper 2 bits encode the hash size and the lower 6 bits encode the hash count. The actual number of path bytes on the wire is hash_count * hash_size. pyMC_core treats path_len as a raw byte count, which is only correct when hash_size == 1 (the current V1 default). This will break when multi-byte hashes are used.

Evidence

C++ firmware (src/Packet.h, lines 79-83):

uint8_t getPathHashSize() const { return (path_len >> 6) + 1; }
uint8_t getPathHashCount() const { return path_len & 63; }
uint8_t getPathByteLen() const { return getPathHashCount() * getPathHashSize(); }
void setPathHashCount(uint8_t n) { path_len &= ~63; path_len |= n; }
void setPathHashSizeAndCount(uint8_t sz, uint8_t n) { path_len = ((sz - 1) << 6) | (n & 63); }

C++ wire read (src/Packet.cpp, lines 74-78 in readFrom()):

path_len = src[i++];
if (!isValidPathLen(path_len)) return false;
uint8_t bl = getPathByteLen();        // Computes hash_count * hash_size
memcpy(path, &src[i], bl); i += bl;   // Reads computed byte length, NOT raw path_len

C++ validation (src/Packet.cpp, lines 13-18):

bool Packet::isValidPathLen(uint8_t path_len) {
  uint8_t hash_count = path_len & 63;
  uint8_t hash_size = (path_len >> 6) + 1;
  if (hash_size == 4) return false;  // Reserved for future
  return hash_count*hash_size <= MAX_PATH_SIZE;
}

pyMC_core (src/pymc_core/protocol/packet.py, lines 339-346 in read_from()):

self.path_len = data[idx]
idx += 1
if self.path_len > MAX_PATH_SIZE:          # Wrong check: compares encoded byte, not computed length
    raise ValueError("path_len too large")
self._check_bounds(idx, self.path_len, data_len, "truncated path")  # Uses raw path_len as byte count
self.path = bytearray(data[idx : idx + self.path_len])              # Reads raw path_len bytes
idx += self.path_len

pyMC_core (src/pymc_core/protocol/packet.py, lines 428-429 in get_raw_length()):

base_length = 2 + self.path_len + self.payload_len  # Uses raw path_len, not computed byte length

Example of the bug

Consider a packet with 2-byte hashes and 3 hops. The wire path_len byte would be 0x43 (hash_size code 01 in bits 7-6, hash_count 3 in bits 5-0):

  • C++ decodes: hash_size = (0x43 >> 6) + 1 = 2, hash_count = 0x43 & 63 = 3, path bytes = 2 * 3 = 6 bytes
  • pyMC_core reads: path_len = 0x43 = 67, tries to read 67 bytes of path data

This will either fail with a bounds error or read garbage data.

Impact

  • Currently: Works by accident because V1 uses 1-byte hashes, so the path_len byte equals the path byte count (upper 2 bits are 0)
  • With multi-byte hashes: Completely broken — wrong number of bytes read from wire, wrong get_raw_length(), wrong path data
  • Validation is wrong: path_len > MAX_PATH_SIZE compares the encoded byte (which can be at most 0xBF = 191 for valid packets) against 64. For 1-byte hashes this works, but for 2-byte hashes, a valid path_len of 0x41 (1 hop with 2-byte hash) would be rejected as > 64.

Fix

Add methods to decode the path_len bitfield:

def get_path_hash_size(self) -> int:
    return (self.path_len >> 6) + 1

def get_path_hash_count(self) -> int:
    return self.path_len & 63

def get_path_byte_len(self) -> int:
    return self.get_path_hash_count() * self.get_path_hash_size()

Then update read_from() to use get_path_byte_len() instead of self.path_len for reading path data, and update get_raw_length() similarly.

Also add validation matching C++:

@staticmethod
def is_valid_path_len(path_len: int) -> bool:
    hash_count = path_len & 63
    hash_size = (path_len >> 6) + 1
    if hash_size == 4:  # Reserved
        return False
    return hash_count * hash_size <= MAX_PATH_SIZE

Issue 6: packet_utils.py — calculate_packet_hash() hashes TRACE path_len as 1 byte, should be 2

Summary

For TRACE packets, the C++ firmware hashes path_len as a uint16_t (2 bytes) via sizeof(path_len). pyMC_core hashes it as 1 byte. This produces different hashes, breaking deduplication interop for TRACE packets.

Evidence

C++ firmware (src/Packet.h, line 47):

uint16_t payload_len, path_len;  // path_len is uint16_t in the struct

C++ firmware (src/Packet.cpp, lines 41-49):

void Packet::calculatePacketHash(uint8_t* hash) const {
  SHA256 sha;
  uint8_t t = getPayloadType();
  sha.update(&t, 1);
  if (t == PAYLOAD_TYPE_TRACE) {
    sha.update(&path_len, sizeof(path_len));   // sizeof(uint16_t) = 2 bytes
  }
  sha.update(payload, payload_len);
  sha.finalize(hash, MAX_HASH_SIZE);
}

sizeof(path_len) is sizeof(uint16_t) = 2 bytes. On little-endian platforms (ESP32, STM32, nRF52 — all LE), this hashes the path_len value as a 2-byte little-endian integer.

pyMC_core (src/pymc_core/protocol/packet_utils.py, lines 214-219):

sha = hashlib.sha256()
sha.update(bytes([payload_type]))
if payload_type == PAYLOAD_TYPE_TRACE:
    sha.update(bytes([path_len]))    # Only 1 byte!
sha.update(payload)
return sha.digest()[:MAX_HASH_SIZE]

Example

For a TRACE packet with path_len = 3:

  • C++ hashes: SHA256(0x09 | 0x03 0x00 | payload) (path_len as uint16_t LE = 03 00)
  • pyMC_core hashes: SHA256(0x09 | 0x03 | payload) (path_len as single byte = 03)

These produce completely different hashes.

Impact

  • TRACE packet deduplication will not interoperate between pyMC_core and C++ firmware
  • A pyMC_core node forwarding TRACE packets will compute different hashes than firmware nodes, potentially causing duplicate forwarding or dropped packets

Fix

Change packet_utils.py line 217 from:

sha.update(bytes([path_len]))

to:

sha.update(path_len.to_bytes(2, 'little'))  # uint16_t, matching C++ sizeof(path_len)

Issue 7: packet_utils.py — calculate_snr_db() returns raw value, should divide by 4

Summary

The C++ firmware stores SNR as int8_t scaled by 4x (0.25 dB precision) and divides by 4.0 when returning the dB value. pyMC_core returns the raw integer without dividing.

Evidence

C++ firmware (src/Packet.h, line 92):

float getSNR() const { return ((float)_snr) / 4.0f; }

C++ firmware — SNR is stored scaled (src/Mesh.cpp, line 58, during TRACE forwarding):

pkt->path[pkt->path_len++] = (int8_t) (pkt->getSNR()*4);  // Store as SNR × 4

C++ firmware — SNR is received scaled (src/Dispatcher.cpp, line 205):

pkt->_snr = _radio->getLastSNR() * 4.0f;  // Scale by 4 on receive

pyMC_core (src/pymc_core/protocol/packet_utils.py, lines 149-151):

@staticmethod
def calculate_snr_db(raw_snr: int) -> float:
    """Convert raw SNR value to decibels."""
    return raw_snr if raw_snr is not None else 0.0  # Returns raw value, no /4.0

Impact

  • packet.get_snr() and packet.snr return values 4x too large
  • An SNR of 8.0 dB from firmware (stored as raw 32) will be reported as 32.0 dB by pyMC_core
  • Any signal quality decisions based on SNR will be wrong

Fix

Change packet_utils.py lines 149-151 to:

@staticmethod
def calculate_snr_db(raw_snr: int) -> float:
    """Convert raw SNR value to decibels (raw value is SNR × 4)."""
    return (raw_snr / 4.0) if raw_snr is not None else 0.0