diff --git a/src/meshcore/commands/base.py b/src/meshcore/commands/base.py index 543460a..1c315eb 100644 --- a/src/meshcore/commands/base.py +++ b/src/meshcore/commands/base.py @@ -299,34 +299,61 @@ class CommandHandlerBase: return result async def send_anon_req(self, dst: DestinationType, request_type: AnonReqType, data: Optional[bytes] = None, context={}, timeout=None, min_timeout=0) -> Event: + """Send an anonymous request to *dst*. + + *dst* need not be a known contact. When it is, that contact's out path is + used as the reply path; otherwise a zero-hop direct reply path is + requested (see the comment below). + + Note: *data* is currently ignored -- the request body is the reply path, + which is derived here rather than supplied by the caller. + """ dst_bytes = _validate_destination(dst, prefix_length=32) pubkey_prefix = _validate_destination(dst, prefix_length=6) logger.debug(f"Anon Binary request to {dst_bytes.hex()}") - contact = self._get_contact_by_prefix(dst_bytes.hex()) # need a contact for return path - if contact is None: - logger.error("No contact found") - return Event(EventType.ERROR, {"reason": "contact_not_found"}) + # The contact is consulted only to build the reply path appended to the + # request; it is not required to send one. Companion firmware from + # FIRMWARE_VER_CODE 13 synthesises a transient anon contact for an unknown + # pubkey (out_path_len = 0, zero-hop direct), so an unknown destination is + # reachable as long as we ask it to reply zero-hop. Refusing here would + # block probing any node the client has not already added - for instance + # asking a freshly discovered neighbour for its regions. + contact = self._get_contact_by_prefix(dst_bytes.hex()) zero_hop = False - if contact["out_path_len"] == -1: - logger.info("No path set trying zero hop") - zero_hop = True - await self.change_contact_path(contact, "") + if contact is None: + logger.debug("No contact found, requesting a zero-hop direct reply path") + out_path_len = 0 + out_path = b"" + else: + if contact["out_path_len"] == -1: + logger.info("No path set trying zero hop") + zero_hop = True + await self.change_contact_path(contact, "") + # update_contact() normally reflects the change back onto the dict, so + # out_path_len reads 0 here. Clamp anyway: if that call failed (e.g. the + # device query inside it errored) the dict is still -1, and the unsigned + # to_bytes below would raise OverflowError -- which would skip the + # reset_path at the end of this method and leave the contact pinned to + # zero-hop on the device. Zero is the right value to send regardless, + # since zero-hop is exactly what we just asked for. + out_path_len = max(contact["out_path_len"], 0) + out_path = bytes.fromhex(contact["out_path"])[::-1] - data = contact["out_path_len"].to_bytes(1, "little") + bytes.fromhex(contact["out_path"])[::-1] + data = out_path_len.to_bytes(1, "little") + out_path data = b"\x39" + dst_bytes + request_type.value.to_bytes(1, "little", signed=False) + (data if data else b"") result = await self.send(data, [EventType.MSG_SENT, EventType.ERROR]) - + # Register the request with the reader if we have both reader and request_type - if (result.type == EventType.MSG_SENT and - self._reader is not None and + if (result.type == EventType.MSG_SENT and + self._reader is not None and request_type is not None): - + exp_tag = result.payload["expected_ack"].hex() # Use provided timeout or fallback to suggested timeout (with 5s default) - result.payload["suggested_timeout"] = result.payload.get("suggested_timeout", 4000) * (contact["out_path_len"] + 1) # update timeout from path_len + result.payload["suggested_timeout"] = result.payload.get("suggested_timeout", 4000) * (out_path_len + 1) # update timeout from path_len actual_timeout = timeout if timeout is not None and timeout > 0 else result.payload.get("suggested_timeout", 4000) / 800.0 actual_timeout = min_timeout if actual_timeout < min_timeout else actual_timeout self._reader.register_binary_request(pubkey_prefix.hex(), exp_tag, request_type, actual_timeout, context=context, is_anon=True) diff --git a/tests/unit/test_error_handling.py b/tests/unit/test_error_handling.py index 7f88e28..8be06d1 100644 --- a/tests/unit/test_error_handling.py +++ b/tests/unit/test_error_handling.py @@ -203,21 +203,199 @@ async def test_set_multi_acks_error( assert result.is_error() -# ── send_anon_req returns ERROR on contact not found ───────── +# ── send_anon_req falls back to zero-hop when no contact exists ───────── -async def test_send_anon_req_contact_not_found( - command_handler, mock_dispatcher +async def test_send_anon_req_without_contact_sends_zero_hop( + command_handler, mock_connection, mock_dispatcher ): - """send_anon_req returns ERROR event when contact prefix not found, - instead of raising TypeError on NoneType subscript.""" + """An unknown destination is sent with a zero-hop reply path. + + The contact is only used to build the reply path. Companion firmware from + FIRMWARE_VER_CODE 13 synthesises a transient anon contact for an unknown + pubkey, so refusing to send here would needlessly block probing any node + the client has not already added. Must still not raise TypeError on the + NoneType subscript that this test originally guarded. + """ command_handler._get_contact_by_prefix = MagicMock(return_value=None) + command_handler.change_contact_path = AsyncMock() + command_handler.reset_path = AsyncMock() + setup_event_response( + mock_dispatcher, EventType.MSG_SENT, + {"expected_ack": b"\x01\x02\x03\x04", "suggested_timeout": 4000}, + ) result = await command_handler.send_anon_req( VALID_PUBKEY_HEX, MagicMock(value=1) ) - assert result.is_error() - assert result.payload["reason"] == "contact_not_found" + assert not result.is_error() + sent = mock_connection.send.await_args.args[0] + # \x39 | 32-byte pubkey | request type | reply-path-len 0 (no path bytes) + assert sent == b"\x39" + bytes.fromhex(VALID_PUBKEY_HEX) + b"\x01" + b"\x00" + + # No contact to mutate, so no device round-trips for path changes. + command_handler.change_contact_path.assert_not_awaited() + command_handler.reset_path.assert_not_awaited() + + +async def test_send_anon_req_without_contact_does_not_scale_timeout( + command_handler, mock_dispatcher +): + """suggested_timeout must not be multiplied by a path length we don't have.""" + command_handler._get_contact_by_prefix = MagicMock(return_value=None) + reader = MagicMock() + reader.register_binary_request = MagicMock() + command_handler._reader = reader + setup_event_response( + mock_dispatcher, EventType.MSG_SENT, + {"expected_ack": b"\x01\x02\x03\x04", "suggested_timeout": 4000}, + ) + + result = await command_handler.send_anon_req( + VALID_PUBKEY_HEX, MagicMock(value=1) + ) + + # out_path_len 0 -> (0 + 1) -> unchanged from the device's own estimate. + assert result.payload["suggested_timeout"] == 4000 + reader.register_binary_request.assert_called_once() + + +async def test_send_anon_req_with_contact_still_uses_its_path( + command_handler, mock_connection, mock_dispatcher +): + """A known contact's reply path and timeout scaling are both unchanged. + + _reader must be set: the out_path_len-based timeout scaling is gated behind + it, and that scaling is the only behaviour the no-contact refactor touches on + this path -- without a reader it is never executed. + """ + command_handler._get_contact_by_prefix = MagicMock(return_value={ + "public_key": VALID_PUBKEY_HEX, + "out_path_len": 3, + "out_path": "aabbcc", + }) + command_handler.change_contact_path = AsyncMock() + command_handler.reset_path = AsyncMock() + reader = MagicMock() + command_handler._reader = reader + setup_event_response( + mock_dispatcher, EventType.MSG_SENT, + {"expected_ack": b"\x01\x02\x03\x04", "suggested_timeout": 4000}, + ) + + result = await command_handler.send_anon_req(VALID_PUBKEY_HEX, MagicMock(value=1)) + + sent = mock_connection.send.await_args.args[0] + # \x39 | pubkey | request type | reply-path-len 3 | path reversed + assert sent == ( + b"\x39" + bytes.fromhex(VALID_PUBKEY_HEX) + b"\x01" + + b"\x03" + bytes.fromhex("ccbbaa") + ) + # Scaled by out_path_len + 1 = 4. + assert result.payload["suggested_timeout"] == 16000 + reader.register_binary_request.assert_called_once() + assert reader.register_binary_request.call_args.args[3] == 16000 / 800.0 + command_handler.change_contact_path.assert_not_awaited() + command_handler.reset_path.assert_not_awaited() + + +async def test_send_anon_req_timeout_uses_path_len_as_sent( + command_handler, mock_connection, mock_dispatcher +): + """The timeout multiplier must match the path length actually transmitted. + + The contact dict is a live reference that other commands mutate in place, so + re-reading it after the await could scale the timeout by a path length that + was never sent. + """ + contact = {"public_key": VALID_PUBKEY_HEX, "out_path_len": 3, "out_path": "aabbcc"} + command_handler._get_contact_by_prefix = MagicMock(return_value=contact) + command_handler._reader = MagicMock() + + def fake_subscribe(evt_type, handler, attr_filters=None): + sub = MagicMock(spec=Subscription) + sub.unsubscribe = MagicMock() + if evt_type == EventType.MSG_SENT: + # Simulate another command flipping the contact to flood mid-send. + contact["out_path_len"] = -1 + contact["out_path"] = "" + asyncio.get_event_loop().call_soon( + handler, + Event(EventType.MSG_SENT, + {"expected_ack": b"\x01\x02\x03\x04", "suggested_timeout": 4000}), + ) + return sub + + mock_dispatcher.subscribe = MagicMock(side_effect=fake_subscribe) + + result = await command_handler.send_anon_req(VALID_PUBKEY_HEX, MagicMock(value=1)) + + sent = mock_connection.send.await_args.args[0] + assert sent.endswith(b"\x03" + bytes.fromhex("ccbbaa")) + # 4 x 4000, matching the 3-hop path in the frame -- not 0 from the mutated -1. + assert result.payload["suggested_timeout"] == 16000 + + +async def test_send_anon_req_flood_contact_still_forces_and_restores_zero_hop( + command_handler, mock_connection, mock_dispatcher +): + """out_path_len == -1 keeps its existing force-zero-hop-then-restore path.""" + contact = { + "public_key": VALID_PUBKEY_HEX, + "out_path_len": -1, + "out_path": "", + } + + async def fake_change_path(c, path): + # update_contact() reflects the change onto the dict; -1 would otherwise + # raise OverflowError on the unsigned to_bytes below. + c["out_path_len"] = 0 + c["out_path"] = "" + + command_handler._get_contact_by_prefix = MagicMock(return_value=contact) + command_handler.change_contact_path = AsyncMock(side_effect=fake_change_path) + command_handler.reset_path = AsyncMock() + setup_event_response( + mock_dispatcher, EventType.MSG_SENT, + {"expected_ack": b"\x01\x02\x03\x04", "suggested_timeout": 4000}, + ) + + await command_handler.send_anon_req(VALID_PUBKEY_HEX, MagicMock(value=1)) + + command_handler.change_contact_path.assert_awaited_once() + command_handler.reset_path.assert_awaited_once() + sent = mock_connection.send.await_args.args[0] + assert sent == b"\x39" + bytes.fromhex(VALID_PUBKEY_HEX) + b"\x01" + b"\x00" + + +async def test_send_anon_req_survives_change_contact_path_not_mutating( + command_handler, mock_connection, mock_dispatcher +): + """A failed change_contact_path must not crash and must still restore the path. + + update_contact() normally writes out_path_len back onto the dict, but if it + errors the value stays -1. Unclamped, the unsigned to_bytes raises + OverflowError, which skips reset_path and leaves the contact pinned to + zero-hop on the device. + """ + contact = {"public_key": VALID_PUBKEY_HEX, "out_path_len": -1, "out_path": ""} + command_handler._get_contact_by_prefix = MagicMock(return_value=contact) + # Returns an error without reflecting the change onto the dict. + command_handler.change_contact_path = AsyncMock( + return_value=Event(EventType.ERROR, {"reason": "device_query_failed"}) + ) + command_handler.reset_path = AsyncMock() + setup_event_response( + mock_dispatcher, EventType.MSG_SENT, + {"expected_ack": b"\x01\x02\x03\x04", "suggested_timeout": 4000}, + ) + + result = await command_handler.send_anon_req(VALID_PUBKEY_HEX, MagicMock(value=1)) + + assert not result.is_error() + sent = mock_connection.send.await_args.args[0] + assert sent == b"\x39" + bytes.fromhex(VALID_PUBKEY_HEX) + b"\x01" + b"\x00" + command_handler.reset_path.assert_awaited_once() # ── send_trace handles unknown path_hash_len without NameError ──