diff --git a/src/helpers/BaseChatMesh.cpp b/src/helpers/BaseChatMesh.cpp index d8a32f3f1..813d3287e 100644 --- a/src/helpers/BaseChatMesh.cpp +++ b/src/helpers/BaseChatMesh.cpp @@ -495,12 +495,13 @@ void BaseChatMesh::onAckRecv(mesh::Packet* packet, uint32_t ack_crc) { ContactInfo* from; if (processAck((uint8_t *)&ack_crc, from)) { txt_send_timeout = 0; // matched one we're waiting for, cancel timeout timer - packet->markDoNotRetransmit(); // ACK was for this node, so don't retransmit if (from != NULL && packet->isRouteFlood() && from->out_path_len != OUT_PATH_UNKNOWN) { // we have direct path, but other node is still sending flood, so maybe they didn't receive reciprocal path properly(?) handleReturnPathRetry(*from, packet->path, packet->path_len); } + // Check the received route before marking, which replaces the header. + packet->markDoNotRetransmit(); // ACK was for this node, so don't retransmit } } diff --git a/test/fixtures/companion_contact_lifetime/test.cpp b/test/fixtures/companion_contact_lifetime/test.cpp index f116a7659..89ea650d8 100644 --- a/test/fixtures/companion_contact_lifetime/test.cpp +++ b/test/fixtures/companion_contact_lifetime/test.cpp @@ -116,6 +116,8 @@ public: uint8_t max_hops = 0; unsigned discoveries = 0, full_notices = 0, path_updates = 0; std::vector path_notices; + std::vector path_notice_lengths; + std::vector> path_notice_bytes; BaseChatMesh() { #include "production_base_constructor.inc" @@ -142,8 +144,12 @@ public: void onContactsFull() { ++full_notices; } void onContactPathUpdated(const ContactInfo&) { ++path_updates; } void onContactResponse(const ContactInfo&, const uint8_t*, uint8_t) {} - void handleReturnPathRetry(const ContactInfo& peer, const uint8_t*, uint8_t) { + void handleReturnPathRetry(const ContactInfo& peer, const uint8_t* path, uint8_t path_len) { + uint8_t bytes[MAX_PATH_SIZE]; + const size_t size = mesh::Packet::writePath(bytes, path, path_len); path_notices.push_back(peer.id.pub_key[0]); + path_notice_lengths.push_back(path_len); + path_notice_bytes.emplace_back(bytes, bytes + size); } void resetContacts(); static void resetContactValue(ContactInfo&); @@ -291,15 +297,21 @@ struct Fixture { bool ack(uint32_t crc, ContactInfo*& peer) { return node.processAck(reinterpret_cast(&crc), peer); } - void received(uint32_t crc) { + void received(uint32_t crc, uint8_t expected_peer = 0, uint8_t route = ROUTE_TYPE_FLOOD) { mesh::Packet packet; - packet.header = ROUTE_TYPE_FLOOD; + packet.header = route | (PAYLOAD_TYPE_ACK << PH_TYPE_SHIFT); + packet.setPathHashSizeAndCount(2, 2); + const uint8_t path[] = {0xa1, 0xa2, 0xb1, 0xb2}; + memcpy(packet.path, path, sizeof(path)); + const auto notices = node.path_notices.size(); node.txt_send_timeout = 9000; node.onAckRecv(&packet, crc); assert(packet.isMarkedDoNotRetransmit() && node.txt_send_timeout == 0); - // Actual Packet marking replaces the route header. Preserve the current - // (separately audited) no-return-path behavior rather than a fake Packet. - assert(node.path_notices.empty()); + if (expected_peer != 0) { + assert(node.path_notices.size() == notices + 1 && node.path_notices.back() == expected_peer); + assert(node.path_notice_lengths.back() == packet.path_len); + assert(node.path_notice_bytes.back() == std::vector(path, path + sizeof(path))); + } else assert(node.path_notices.size() == notices); } void drain() { usb_stream.write_capacity = other_stream.write_capacity = 4096; @@ -382,7 +394,7 @@ int main() { uint8_t path[MAX_PATH_SIZE]; assert(first.contact->copyPathTo(path) && path[0] == 2); assert(second.contact->copyPathTo(path) && path[0] == 3); - f.received(first.ack); f.received(second.ack); f.received(same_peer.ack); + f.received(first.ack, 2); f.received(second.ack, 3); f.received(same_peer.ack, 2); if (MESH_ENABLE_ONE_KEY_DM) assert((f.node.learned == std::vector{2, 3, 2})); else assert(f.node.learned.empty()); ++checks; @@ -640,10 +652,44 @@ int main() { else assert(f.node.learned.empty()); ++checks; } + const struct { uint8_t route, expected_peer; } routes[] = { + {ROUTE_TYPE_FLOOD, 1}, {ROUTE_TYPE_TRANSPORT_FLOOD, 1}, + {ROUTE_TYPE_DIRECT, 0}, {ROUTE_TYPE_TRANSPORT_DIRECT, 0}, + }; + for (const auto& route : routes) { + Fixture f; auto* peer = f.add(1); auto& entry = f.pending(peer); + f.received(entry.ack, route.expected_peer, route.route); + assert(entry.ack == 0 && f.node.cancellations == 1); + ++checks; + } + { + Fixture f; auto* peer = f.add(9); auto& entry = f.pending(peer); + assert(peer->out_path_len == OUT_PATH_UNKNOWN); + f.received(entry.ack); + assert(entry.ack == 0 && f.node.cancellations == 1); + ++checks; + } + { + Fixture f; auto* peer = f.add(1); auto& entry = f.pending(peer); f.blockUsb(); + f.received(entry.ack, 1); + assert(entry.confirmed && f.node.path_notices.size() == 1); + const auto prior = entry; + for (uint32_t crc : {entry.ack, uint32_t(0xdeadbeef)}) { + mesh::Packet packet; + packet.header = ROUTE_TYPE_FLOOD | (PAYLOAD_TYPE_ACK << PH_TYPE_SHIFT); + f.node.txt_send_timeout = 9000; + f.node.onAckRecv(&packet, crc); + assert(!packet.isMarkedDoNotRetransmit() && packet.isRouteFlood()); + assert(f.node.txt_send_timeout == 9000 && f.node.path_notices.size() == 1); + assert(f.node.cancellations == 1); + unchanged(entry, prior); + } + ++checks; + } #if defined(ESP32_PLATFORM) && defined(BOARD_HAS_PSRAM) - assert(checks == 31); + assert(checks == 37); #else - assert(checks == 28); + assert(checks == 34); #endif assert(outstanding_allocations.empty()); printf("PASS: %u production contact lifetime checks\n", checks); diff --git a/test/test_companion_contact_lifetime.py b/test/test_companion_contact_lifetime.py index e12ed7418..310c767c9 100644 --- a/test/test_companion_contact_lifetime.py +++ b/test/test_companion_contact_lifetime.py @@ -134,7 +134,7 @@ class CompanionContactLifetimeTests(unittest.TestCase): self.assertEqual(compiled.returncode, 0, compiled.stdout + compiled.stderr) checked = subprocess.run([str(binary)], capture_output=True, text=True, timeout=10) self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) - expected_checks = 31 if platform == "psram" else 28 + expected_checks = 37 if platform == "psram" else 34 self.assertIn(f"PASS: {expected_checks} production contact lifetime checks", checked.stdout)