Decide a DHT values peer's family by transport, not by blob length

CHANGELOG.md

@@ -1,6 +1,6 @@
 # Changelog
 
-## 0.6.7 - 2026-09-12
+## 0.6.7 - 2026-09-14
 
 A measurement release. Every fix here is a case where the engine was doing
 something wrong *and the instruments said it was fine* — a tracker retried
@@ -51,6 +51,16 @@ speed of 0 B/s reported on a torrent moving at 100 KB/s.
   oscillates, and a torrent slower than one piece per window reads 0 until its
   first piece lands.
 
+- A packed `values` string in a `get_peers` response is no longer mistaken for an
+  IPv6 peer. **18 is a multiple of 6**, so a string carrying three compact IPv4
+  peers has exactly the size of one compact IPv6 peer, and the decoder tested the
+  IPv6 stride first — inventing an IPv6 address out of three IPv4 ones. A phantom
+  endpoint is not harmless: it consumes a dial slot, can never connect, and its
+  failure is recorded against the **IPv6** family, which is what drives the
+  per-family dial throttle — so the guess teaches the engine that IPv6 does not
+  work. BEP 32 runs the IPv6 DHT as a separate DHT, which makes the transport the
+  authority: the family the response arrived on now decides, and length remains
+  the tie-breaker only where that family's own unit does not divide the string.
 - A disk error while serving a BEP 52 hash request returns an error instead of
   raising. `Merkle.leaf_range_response_from_disk/7` documents
   `{:error, term()}` and handles a failed `:file.open/2` that way, but the

lib/elixir_torrent/dht.ex

@@ -584,7 +584,9 @@ defmodule DHT do
 
     tables = RoutingTables.mark_good(state.routing_tables, contact, from_query: true)
     state = %{state | routing_tables: tables}
-    peers = KRPC.response_peers(response)
+    # The family the answer arrived on disambiguates a packed `values` string —
+    # see `KRPC.response_peers/2`.
+    peers = KRPC.response_peers(response, transport_family(ip))
 
     nodes =
       response
@@ -1499,10 +1501,13 @@ defmodule DHT do
 
   @spec socket_for_dest(t(), :inet.ip_address()) :: port()
   defp socket_for_dest(state, ip) do
-    family = if tuple_size(ip) == 8, do: :inet6, else: :inet
-    select_socket(state, family)
+    select_socket(state, transport_family(ip))
   end
 
+  @spec transport_family(:inet.ip_address()) :: :inet | :inet6
+  defp transport_family(ip) when tuple_size(ip) == 8, do: :inet6
+  defp transport_family(_ip), do: :inet
+
   @spec socket_family(t(), port()) :: :inet | :inet6
   defp socket_family(%__MODULE__{socket_v6: socket_v6}, socket) when socket == socket_v6,
     do: :inet6

lib/elixir_torrent/dht/krpc.ex

@@ -310,38 +310,59 @@ defmodule DHT.KRPC do
     (v4 ++ v6) |> Enum.uniq_by(& &1.id)
   end
 
-  @doc "Decode compact peers from a get_peers response body."
-  @spec response_peers(response()) :: [Peer.t()]
-  def response_peers(%{values: values}) do
+  @doc """
+  Decode compact peers from a get_peers response body.
+
+  `family` is the address family the response arrived on. It is needed because
+  **18 is a multiple of 6**: BEP 5 nominally puts one compact peer in each
+  `values` string, but implementations pack several into one, so a string carrying
+  three IPv4 peers is indistinguishable by length alone from a single IPv6 peer.
+  BEP 32 runs the IPv6 DHT as a *separate* DHT, which makes the transport the
+  authority — a response that came back over IPv6 carries IPv6 peers. Length stays
+  the tie-breaker only when the transport's own unit does not divide the string,
+  and for callers that do not know the family.
+  """
+  @spec response_peers(response(), :inet | :inet6 | nil) :: [Peer.t()]
+  def response_peers(response, family \\ nil)
+
+  def response_peers(%{values: values}, family) do
     values
     |> normalize_values()
-    |> Enum.flat_map(&decode_value_peers/1)
+    |> Enum.flat_map(&decode_value_peers(&1, family))
     |> Enum.uniq_by(&{&1.ip, &1.port})
   end
 
-  def response_peers(_), do: []
+  def response_peers(_, _), do: []
 
   @spec normalize_values(term()) :: [binary()]
   defp normalize_values(values) when is_binary(values), do: [values]
   defp normalize_values(values) when is_list(values), do: Enum.filter(values, &is_binary/1)
   defp normalize_values(_), do: []
 
-  @spec decode_value_peers(binary()) :: [Peer.t()]
-  defp decode_value_peers(blob) when is_binary(blob) do
-    size = byte_size(blob)
-
-    cond do
-      size == 0 ->
-        []
-
-      rem(size, @ipv6_peer_info_size) == 0 ->
-        Compact.decode_ipv6_peers(blob)
-
-      rem(size, @peer_info_size) == 0 ->
-        Compact.decode_peers(blob)
-
-      true ->
-        []
+  @spec decode_value_peers(binary(), :inet | :inet6 | nil) :: [Peer.t()]
+  defp decode_value_peers(blob, family) when is_binary(blob) do
+    case value_peer_family(byte_size(blob), family) do
+      :inet6 -> Compact.decode_ipv6_peers(blob)
+      :inet -> Compact.decode_peers(blob)
+      nil -> []
     end
   end
+
+  # The transport family goes first, so a packed 18-byte string arriving over IPv4
+  # is read as the three IPv4 peers it is rather than one invented IPv6 address.
+  # A phantom endpoint is not harmless: it consumes a dial slot, can never connect,
+  # and its failure is recorded against the IPv6 family — which is what drives the
+  # per-family dial throttle, so guessing wrong here teaches the engine that IPv6
+  # does not work.
+  @spec value_peer_family(non_neg_integer(), :inet | :inet6 | nil) :: :inet | :inet6 | nil
+  defp value_peer_family(0, _family), do: nil
+
+  defp value_peer_family(size, :inet6) when rem(size, @ipv6_peer_info_size) == 0, do: :inet6
+  defp value_peer_family(size, :inet) when rem(size, @peer_info_size) == 0, do: :inet
+
+  # Family unknown, or known but its unit does not divide the string — a v4 peer
+  # answered over the v6 DHT, say. Fall back to length, preferring IPv6 as before.
+  defp value_peer_family(size, _family) when rem(size, @ipv6_peer_info_size) == 0, do: :inet6
+  defp value_peer_family(size, _family) when rem(size, @peer_info_size) == 0, do: :inet
+  defp value_peer_family(_size, _family), do: nil
 end

test/ipv6_dial_test.exs

@@ -107,6 +107,57 @@ defmodule IPv6DialTest do
       assert length(peers) == 2
       assert Enum.all?(peers, fn %Peer{ip: ip} -> tuple_size(ip) == 8 end)
     end
+
+    # 18 is a multiple of 6, so a `values` string packing three IPv4 peers has
+    # exactly the size of one IPv6 peer. Length alone cannot tell them apart; BEP 32
+    # runs the IPv6 DHT as a separate DHT, so the transport the answer arrived on is
+    # the authority.
+    test "a packed 18-byte blob over IPv4 is three IPv4 peers, not one IPv6 peer" do
+      blob =
+        Compact.encode_peer({1, 2, 3, 4}, 6881) <>
+          Compact.encode_peer({5, 6, 7, 8}, 6882) <>
+          Compact.encode_peer({9, 10, 11, 12}, 6883)
+
+      assert byte_size(blob) == 18
+
+      peers = KRPC.response_peers(%{values: [blob]}, :inet)
+
+      assert length(peers) == 3
+      assert Enum.all?(peers, fn %Peer{ip: ip} -> tuple_size(ip) == 4 end)
+      assert %Peer{ip: {9, 10, 11, 12}, port: 6883} in peers
+    end
+
+    test "the same 18-byte blob over IPv6 is one IPv6 peer" do
+      blob =
+        <<0x26, 0x02, 0x00, 0x2D, 0x40, 0x00, 0x00, 0x01, 0, 0, 0, 0, 0, 0, 0, 0x42, 0x1A, 0xE1>>
+
+      peers = KRPC.response_peers(%{values: [blob]}, :inet6)
+
+      assert length(peers) == 1
+      assert Enum.all?(peers, fn %Peer{ip: ip} -> tuple_size(ip) == 8 end)
+    end
+
+    test "falls back to length when the family's own unit does not divide the blob" do
+      # A single IPv4 peer answered over the IPv6 DHT: 6 is not a multiple of 18, so
+      # the length has to decide, and it says IPv4.
+      blob = Compact.encode_peer({1, 2, 3, 4}, 6881)
+
+      assert KRPC.response_peers(%{values: [blob]}, :inet6) == [
+               %Peer{ip: {1, 2, 3, 4}, port: 6881}
+             ]
+    end
+
+    test "with no family the old length-only preference for IPv6 is unchanged" do
+      blob =
+        Compact.encode_peer({1, 2, 3, 4}, 6881) <>
+          Compact.encode_peer({5, 6, 7, 8}, 6882) <>
+          Compact.encode_peer({9, 10, 11, 12}, 6883)
+
+      peers = KRPC.response_peers(%{values: [blob]})
+
+      assert length(peers) == 1
+      assert Enum.all?(peers, fn %Peer{ip: ip} -> tuple_size(ip) == 8 end)
+    end
   end
 
   describe "DHT.cap_lookup_peers/2" do