mirror of
https://github.com/meshcore-dev/meshcore_py.git
synced 2026-08-01 20:56:12 +00:00
fix(anon-req): allow requests to destinations that are not contacts
send_anon_req() refused to send whenever the destination pubkey was absent from the client-side contact cache, returning ERROR contact_not_found. The contact is consulted for one thing only: building the reply-path bytes appended to the request. The companion firmware needs no contact of its own -- since FIRMWARE_VER_CODE 13 its CMD_SEND_ANON_REQ handler synthesises a transient anon contact for an unknown pubkey with out_path_len = 0 (zero-hop direct). Those entries live in a reserved slot ring, are hidden from CMD_GET_CONTACTS and are never persisted, so nothing is polluted by them. The client-side refusal therefore blocked a case the device supports, such as asking a freshly discovered neighbour for its regions before it has ever been added as a contact. Fall back to a zero-hop reply path instead. Two smaller fixes in the same function: - out_path_len is now read once into a local rather than re-read from the contact dict after the await. That dict is a live reference other commands mutate in place (send_msg_with_retry's flood fallback, reset_path); if it flipped to -1 mid-send the suggested_timeout multiplier became 4000 * 0 = 0, registering the binary request with a zero timeout so the response was dropped the moment it arrived. - The value is clamped at 0. update_contact() normally reflects the change back onto the dict, but if it fails the dict stays -1 and the unsigned to_bytes raises OverflowError -- which skipped the reset_path at the end of the method and left the contact pinned to zero-hop on the device. Verified against a companion radio on fw ver 13: without the change all five discovered repeaters were refused client-side in 0.0s with no RF sent; with it all three answered with their region scopes in ~1.1s. test_send_anon_req_contact_not_found is replaced -- it codified the removed limitation -- but its original regression (a TypeError on the NoneType subscript) stays covered.
This commit is contained in:
agessaman
@@ -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)
|
||||
|
||||
@@ -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 ──
|
||||
|
||||
Reference in New Issue
Block a user