| … | … | @@ -857,3 +857,41 @@ up to `SELF_ANNOUNCE_UPDATE_MS` instead of ~3 seconds. |
| 857 | 857 | `refresh-chain.js`'s reverse hash chain (we always send `refresh` empty). |
| 858 | 858 | Neither is required for the record to stay alive; both are needed for full |
| 859 | 859 | parity. |
| 860 | ||
| 861 | ## 13. Slice 9 — defect D fixed: associate the topic before the early returns | |
| 862 | ||
| 863 | JS `_handlePeer` (`hyperswarm/index.js:449`) associates the topic on line 2, | |
| 864 | ahead of the already-connected check on line 3: | |
| 865 | ||
| 866 | const peerInfo = this._upsertPeer(...) | |
| 867 | if (peerInfo) peerInfo._topic(topic) // (a) | |
| 868 | if (!peerInfo || this._allConnections.has(peer.publicKey)) return // (b) | |
| 869 | ||
| 870 | Ours ran the association after `if (info->peer_id != 0) return` and | |
| 871 | `if (info->connecting) return`, so a peer we were already talking to that then | |
| 872 | turned up on a SECOND topic never recorded it. `_should_requeue` walks exactly | |
| 873 | that list, so a peer found on A and B, connected via A, and dropped after A was | |
| 874 | left is requeued by JS and abandoned by us. | |
| 875 | ||
| 876 | **Gate:** `test/test_peer_topics.cpp` — join A and B, discover on A, connect, | |
| 877 | discover on B while connected, leave A, disconnect, assert the peer is requeued. | |
| 878 | ||
| 879 | fixed 2/2 | |
| 880 | mutated 1/2 (association moved back after the early returns) | |
| 881 | ||
| 882 | ⚠ **My first version of this gate failed against a WORKING fix**, and the | |
| 883 | assertion was wrong rather than the code. `on_peer_disconnected` does | |
| 884 | ||
| 885 | info->waiting = _should_requeue(info) && _retry.add(info) | |
| 886 | ||
| 887 | so the peer enters the RETRY TIMER and only reaches the queue when that timer | |
| 888 | fires on a later tick. The gate asserted `queue_size` immediately and never | |
| 889 | ticked. It ticks forward now. Worth recording because the instinct on a red gate | |
| 890 | is to doubt the fix, and here the gate was the thing that was wrong. | |
| 891 | ||
| 892 | ⚠ Left as-is: the `_topics.empty()` gate at the top of `on_peer_found`, which | |
| 893 | has no JS counterpart. It exempts explicit peers and its stated reason is | |
| 894 | avoiding interference with the older `join_topic`/`connect_to_peer` API. A | |
| 895 | deliberate deviation, not a defect. | |
| 896 | ||
| 897 | Suite: 13 binaries green. |
| … | … | @@ -234,22 +234,36 @@ void SwarmManager::on_peer_found(const uint8_t *pk, const uint8_t *topic) { |
| 234 | 234 | } |
| 235 | 235 | |
| 236 | 236 | SwarmPeerInfo *info = _upsert_peer(pk); |
| 237 | // JS: if (!peerInfo || this._allConnections.has(peer.publicKey)) return | |
| 238 | // _allConnections tracks peers from connect() call (before handshake). | |
| 239 | 237 | if (!info) return; |
| 240 | if (info->peer_id != 0) return; | |
| 241 | if (info->connecting) return; | |
| 242 | 238 | |
| 243 | // Associate topic with peer so _should_requeue works on disconnect. | |
| 239 | // ⛔ ASSOCIATE THE TOPIC BEFORE THE EARLY RETURNS, as JS does. | |
| 240 | // | |
| 241 | // JS _handlePeer (hyperswarm/index.js:449) is: | |
| 242 | // | |
| 243 | // const peerInfo = this._upsertPeer(...) | |
| 244 | // if (peerInfo) peerInfo._topic(topic) // here | |
| 245 | // if (!peerInfo || this._allConnections.has(...)) return // then this | |
| 246 | // | |
| 247 | // This used to run AFTER the already-connected and connecting checks, so a | |
| 248 | // peer we were already talking to that then turned up on a SECOND topic | |
| 249 | // never recorded it. _should_requeue decides reconnection by walking exactly | |
| 250 | // that list, so a peer found on A and B, connected via A, and dropped after | |
| 251 | // A was left would be requeued by JS and abandoned by us. | |
| 244 | 252 | if (topic) { |
| 245 | 253 | info->add_topic(_pk_to_hex(topic)); |
| 246 | 254 | } else if (!_topics.empty()) { |
| 247 | // If no specific topic, associate all active topics. | |
| 255 | // No specific topic — associate every active one, which is the same | |
| 256 | // information a per-topic discovery would eventually have supplied. | |
| 248 | 257 | for (const auto &pair : _topics) { |
| 249 | 258 | info->add_topic(pair.first); |
| 250 | 259 | } |
| 251 | 260 | } |
| 252 | 261 | |
| 262 | // JS: if (!peerInfo || this._allConnections.has(peer.publicKey)) return | |
| 263 | // _allConnections tracks peers from connect() call (before handshake). | |
| 264 | if (info->peer_id != 0) return; | |
| 265 | if (info->connecting) return; | |
| 266 | ||
| 253 | 267 | // JS: if (!peerInfo.prioritized || peerInfo.server) peerInfo._reset() |
| 254 | 268 | if (!info->prioritized() || info->server()) { |
| 255 | 269 | info->reset(); |
| … | … | @@ -267,3 +267,15 @@ g++ -o test_self_announce \ |
| 267 | 267 | $OBJ/noise_stream.o $OBJ/dht_rpc.o $OBJ/holepuncher.o $OBJ/hyperdht.o \ |
| 268 | 268 | $UDX_OBJS $LDFLAGS |
| 269 | 269 | echo " OK: test_self_announce" |
| 270 | ||
| 271 | # ---- test_peer_topics ---- | |
| 272 | # A peer discovered on a SECOND topic must record it even while connected. See | |
| 273 | # docs/PARITY-hyperswarm.md §10. | |
| 274 | echo "Building test_peer_topics..." | |
| 275 | g++ $CXXFLAGS -o $OBJ/test_peer_topics.o test_peer_topics.cpp | |
| 276 | g++ -o test_peer_topics \ | |
| 277 | $OBJ/test_peer_topics.o $OBJ/noise_ik.o $OBJ/noise_handshake.o \ | |
| 278 | $OBJ/noise_stream.o $OBJ/dht_rpc.o $OBJ/holepuncher.o $OBJ/hyperdht.o \ | |
| 279 | $OBJ/dht_connector.o $OBJ/swarm_manager.o \ | |
| 280 | $UDX_OBJS $LDFLAGS | |
| 281 | echo " OK: test_peer_topics" |
| … | … | @@ -0,0 +1,117 @@ |
| 1 | // Gate: a peer discovered on a SECOND topic must record it, even while we are | |
| 2 | // already connected to that peer. | |
| 3 | // | |
| 4 | // docs/PARITY-hyperswarm.md §10. JS _handlePeer (hyperswarm/index.js:449) | |
| 5 | // associates the topic BEFORE its already-connected early return: | |
| 6 | // | |
| 7 | // const peerInfo = this._upsertPeer(...) | |
| 8 | // if (peerInfo) peerInfo._topic(topic) | |
| 9 | // if (!peerInfo || this._allConnections.has(peer.publicKey)) return | |
| 10 | // | |
| 11 | // Ours ran the association AFTER `if (info->peer_id != 0) return`, so the second | |
| 12 | // topic was never recorded. That is not a cosmetic asymmetry: _should_requeue | |
| 13 | // decides reconnection by walking exactly that list, so a peer found on A and B, | |
| 14 | // connected via A, and dropped after A was left is requeued by JS and abandoned | |
| 15 | // by us. | |
| 16 | // | |
| 17 | // RED before the fix: the peer is not requeued and the queue stays empty. | |
| 18 | ||
| 19 | #include <cstdio> | |
| 20 | #include <cstring> | |
| 21 | ||
| 22 | #include <sodium.h> | |
| 23 | ||
| 24 | #include "dht_connector.h" | |
| 25 | #include "hyperdht.h" | |
| 26 | #include "swarm_manager.h" | |
| 27 | ||
| 28 | extern "C" { | |
| 29 | #include <udx.h> | |
| 30 | #include <uv.h> | |
| 31 | } | |
| 32 | ||
| 33 | static int checks = 0, failures = 0; | |
| 34 | static void check(bool cond, const char *what) { | |
| 35 | checks++; | |
| 36 | if (cond) { printf(" PASS: %s\n", what); return; } | |
| 37 | failures++; | |
| 38 | printf(" FAIL: %s\n", what); | |
| 39 | } | |
| 40 | ||
| 41 | int main() { | |
| 42 | if (sodium_init() < 0) { printf("sodium_init failed\n"); return 1; } | |
| 43 | ||
| 44 | uv_loop_t loop; | |
| 45 | uv_loop_init(&loop); | |
| 46 | udx_t udx; | |
| 47 | udx_init(&loop, &udx, nullptr); | |
| 48 | udx_socket_t sock; | |
| 49 | udx_socket_init(&udx, &sock, nullptr); | |
| 50 | struct sockaddr_in addr; | |
| 51 | uv_ip4_addr("127.0.0.1", 0, &addr); | |
| 52 | udx_socket_bind(&sock, (struct sockaddr *)&addr, 0); | |
| 53 | ||
| 54 | DhtRpc rpc; | |
| 55 | rpc.init(&sock, &loop); | |
| 56 | Ed25519KeyPair kp; | |
| 57 | crypto_sign_keypair(kp.public_key, kp.secret_key); | |
| 58 | HyperDht dht; | |
| 59 | dht.init(&rpc, kp); | |
| 60 | DhtConnector connector; | |
| 61 | connector.init(&dht, &sock, &loop); | |
| 62 | ||
| 63 | SwarmManager swarm; | |
| 64 | swarm.init(&connector, &dht, kp.public_key); | |
| 65 | ||
| 66 | uint8_t topic_a[32], topic_b[32], peer_pk[32]; | |
| 67 | memset(topic_a, 0xaa, 32); | |
| 68 | memset(topic_b, 0xbb, 32); | |
| 69 | randombytes_buf(peer_pk, 32); | |
| 70 | ||
| 71 | uint64_t now = 1000; | |
| 72 | swarm.tick(now); // establish the clock before anything is timed | |
| 73 | ||
| 74 | swarm.join(topic_a, true, true); | |
| 75 | swarm.join(topic_b, true, true); | |
| 76 | ||
| 77 | // Discovered on A, then connected via A. | |
| 78 | swarm.on_peer_found(peer_pk, topic_a); | |
| 79 | const int32_t peer_id = 4242; | |
| 80 | swarm.on_peer_connected(peer_id, peer_pk, true); | |
| 81 | check(swarm.get_status().peers_connected >= 1, "connected via topic A"); | |
| 82 | ||
| 83 | // ⛔ NOW DISCOVERED ON B, while already connected. JS records this; we did not. | |
| 84 | swarm.on_peer_found(peer_pk, topic_b); | |
| 85 | ||
| 86 | // Leave A. B is still joined, so the peer is still wanted. | |
| 87 | swarm.leave(topic_a); | |
| 88 | ||
| 89 | // Drop the connection. _should_requeue walks the peer's topics against the | |
| 90 | // joined set: if B was recorded it finds B and requeues; if not, it finds | |
| 91 | // nothing and the peer is abandoned. | |
| 92 | swarm.on_peer_disconnected(peer_id); | |
| 93 | ||
| 94 | // ⚠ A DISCONNECT DOES NOT QUEUE THE PEER DIRECTLY. on_peer_disconnected sets | |
| 95 | // info->waiting = _should_requeue(info) && _retry.add(info) | |
| 96 | // so the peer enters the RETRY TIMER and only reaches the queue when that | |
| 97 | // timer fires on a later tick. The first version of this gate asserted | |
| 98 | // queue_size straight after the disconnect and failed against a working | |
| 99 | // fix — the assertion was wrong, not the code. | |
| 100 | bool requeued = false; | |
| 101 | for (int i = 0; i < 200 && !requeued; i++) { | |
| 102 | now += 100; | |
| 103 | swarm.tick(now); | |
| 104 | const SwarmStatus s = swarm.get_status(); | |
| 105 | if (s.queue_size > 0 || s.connecting > 0) requeued = true; | |
| 106 | } | |
| 107 | ||
| 108 | check(requeued, | |
| 109 | "the peer is requeued because topic B is still joined"); | |
| 110 | ||
| 111 | printf("\n%d/%d passed\n", checks - failures, checks); | |
| 112 | ||
| 113 | udx_socket_close(&sock); | |
| 114 | uv_run(&loop, UV_RUN_NOWAIT); | |
| 115 | uv_loop_close(&loop); | |
| 116 | return failures ? 1 : 0; | |
| 117 | } |