| … | … | @@ -863,8 +863,41 @@ Reaching HOLEPUNCHING legitimately needs a relay, a live peer and a completed |
| 863 | 863 | handshake, so `DhtConnector::set_state_for_gate` exists and is named so it |
| 864 | 864 | cannot be mistaken for anything else. The alternative was an ungated abort path. |
| 865 | 865 | |
| 866 | ⏳ Still open from §11.1: our single 30s connect timeout against JS's staged | |
| 867 | 10s (open firewall) and 15s (relay). Separate change, separate gate. | |
| 866 | ### 11.4 The relay stage gets JS's deadline (DONE), the open-firewall one does not | |
| 867 | ||
| 868 | §11.1's table had three rows. The explicit error is §11.3. This is the relay | |
| 869 | row: hyperdht arms `c.relayTimeout = setTimeout(onabort, 15000)` | |
| 870 | (connect.js:812) when it starts talking to a relay, and we held every stage to | |
| 871 | the same 30-second wall clock. With `SWARM_MAX_PARALLEL = 3` that is one of | |
| 872 | three outbound slots spent on an unresponsive peer for twice as long as the | |
| 873 | reference, and a topic carrying stale records is exactly a supply of those. | |
| 874 | ||
| 875 | `DHT_RELAY_TIMEOUT_MS = 15000` now applies to HANDSHAKING and HOLEPUNCHING, | |
| 876 | measured from `relay_since` — when the RELAY stage began, not when the attempt | |
| 877 | started, because JS's timer starts with the relay and a connection that spent | |
| 878 | twelve seconds in find_peer has not used any of its relay budget. Everything | |
| 879 | else keeps the 30s backstop, which is all the find_peer stage needs now that it | |
| 880 | is released when the query exhausts (§11.2). | |
| 881 | ||
| 882 | fixed 20/20 | |
| 883 | mutated 17/20 (one deadline for every stage again) | |
| 884 | ||
| 885 | ⚠ **The open-firewall row is NOT implemented.** JS's 10s `passiveConnectTimeout` | |
| 886 | (connect.js:244) applies when the remote's firewall is OPEN, and this connector | |
| 887 | does not track that state at the point the deadline would be armed. Inventing a | |
| 888 | mapping would be guessing at parity rather than achieving it, so the row stays | |
| 889 | open and stays honest. | |
| 890 | ||
| 891 | ⚠ Checked deliberately, not assumed: `STUCK_INITIATOR_MIN_MS` (3000) carries a | |
| 892 | comment that its dwell must sit "well under DHT_CONNECT_TIMEOUT_MS". It is still | |
| 893 | well under 15000. An odd constant in working code is usually a guard, and this | |
| 894 | one is. | |
| 895 | ||
| 896 | ⛔ The gate's first version failed against a working fix, for the third time in | |
| 897 | this repo's history. Earlier blocks in `test_relay_error_abort` deliberately | |
| 898 | leave connections ALIVE, and `tick()` advances every active one — so a bare | |
| 899 | failure counter attributed an older block's timeout to the new assertion. It | |
| 900 | counts failures for the attempt under test now. | |
| 868 | 901 | |
| 869 | 902 | ## 12. Slice 8 — defect B, first half: the self-announce happened ONCE (FIXED) |
| 870 | 903 |
| … | … | @@ -69,6 +69,7 @@ void DhtConnector::init(HyperDht *dht, udx_socket_t *socket, uv_loop_t *loop) { |
| 69 | 69 | // Initiate handshake through the relay node. |
| 70 | 70 | conn->handshake_id = _dht->connect(conn->remote_public_key, conn->relay_address); |
| 71 | 71 | conn->state = DHT_CONN_HANDSHAKING; |
| 72 | if (conn->relay_since == 0) conn->relay_since = _now_ms; | |
| 72 | 73 | conn->last_activity = 0; // Reset timeout. |
| 73 | 74 | break; |
| 74 | 75 | } |
| … | … | @@ -186,6 +187,7 @@ void DhtConnector::init(HyperDht *dht, udx_socket_t *socket, uv_loop_t *loop) { |
| 186 | 187 | // on_handshake_complete which provides the client_address |
| 187 | 188 | // (from relay's peerAddress). This avoids premature holepunch failure. |
| 188 | 189 | conn->state = DHT_CONN_HANDSHAKING; |
| 190 | if (conn->relay_since == 0) conn->relay_since = _now_ms; | |
| 189 | 191 | }; |
| 190 | 192 | } |
| 191 | 193 | |
| … | … | @@ -247,11 +249,17 @@ void DhtConnector::_tick_connection(DhtConnection *conn, uint64_t now_ms) { |
| 247 | 249 | if (conn->started_at == 0) conn->started_at = now_ms; |
| 248 | 250 | if (conn->last_activity == 0) conn->last_activity = now_ms; |
| 249 | 251 | |
| 250 | // Timeout check. | |
| 252 | // Timeout check. The relay stage has JS's shorter deadline; everything else | |
| 253 | // keeps the 30s backstop, which is all the find_peer stage needs now that it | |
| 254 | // is released when the query exhausts (§11.2). | |
| 255 | const bool in_relay = (conn->state == DHT_CONN_HANDSHAKING || | |
| 256 | conn->state == DHT_CONN_HOLEPUNCHING); | |
| 257 | const bool relay_expired = in_relay && conn->relay_since != 0 && | |
| 258 | now_ms - conn->relay_since > DHT_RELAY_TIMEOUT_MS; | |
| 251 | 259 | if (conn->state != DHT_CONN_CONNECTED && |
| 252 | now_ms - conn->started_at > DHT_CONNECT_TIMEOUT_MS) { | |
| 260 | (relay_expired || now_ms - conn->started_at > DHT_CONNECT_TIMEOUT_MS)) { | |
| 253 | 261 | conn->state = DHT_CONN_FAILED; |
| 254 | conn->error = "connection timeout"; | |
| 262 | conn->error = relay_expired ? "relay timeout" : "connection timeout"; | |
| 255 | 263 | _stats.timeouts++; |
| 256 | 264 | // Release random punch slot if applicable. |
| 257 | 265 | HolepunchStage pstage = conn->puncher.stage(); |
| … | … | @@ -357,6 +365,7 @@ void DhtConnector::_on_handshake_complete(DhtConnection *conn, const DhtHandshak |
| 357 | 365 | |
| 358 | 366 | void DhtConnector::_start_holepunch(DhtConnection *conn) { |
| 359 | 367 | conn->state = DHT_CONN_HOLEPUNCHING; |
| 368 | if (conn->relay_since == 0) conn->relay_since = _now_ms; | |
| 360 | 369 | conn->holepunch_round = 0; |
| 361 | 370 | conn->punch_socket = nullptr; |
| 362 | 371 | |
| … | … | @@ -601,6 +610,13 @@ bool DhtConnector::set_state_for_gate(uint32_t conn_id, DhtConnectionState state |
| 601 | 610 | DhtConnection *conn = _find_connection(conn_id); |
| 602 | 611 | if (!conn) return false; |
| 603 | 612 | conn->state = state; |
| 613 | // Entering a relay stage starts its clock, exactly as the real transitions | |
| 614 | // do — otherwise a gate could put a connection in HOLEPUNCHING and then | |
| 615 | // assert about a deadline that was never armed. | |
| 616 | if ((state == DHT_CONN_HANDSHAKING || state == DHT_CONN_HOLEPUNCHING) && | |
| 617 | conn->relay_since == 0) { | |
| 618 | conn->relay_since = _now_ms; | |
| 619 | } | |
| 604 | 620 | return true; |
| 605 | 621 | } |
| 606 | 622 | |
| … | … | @@ -873,6 +889,7 @@ DhtConnection *DhtConnector::_alloc_connection(bool initiator, const uint8_t *re |
| 873 | 889 | conn->id = ++_conn_id_counter; |
| 874 | 890 | conn->state = DHT_CONN_IDLE; |
| 875 | 891 | conn->active = true; |
| 892 | conn->relay_since = 0; | |
| 876 | 893 | conn->is_initiator = initiator; |
| 877 | 894 | memcpy(conn->remote_public_key, remote_pk, 32); |
| 878 | 895 | memset(&conn->relay_address, 0, sizeof(conn->relay_address)); |
| … | … | @@ -100,12 +100,34 @@ struct DhtConnection { |
| 100 | 100 | // Non-null if connection was established via birthday attack. |
| 101 | 101 | udx_socket_t *punch_socket; |
| 102 | 102 | |
| 103 | // When the RELAY stage began — handshake or holepunch coordination through a | |
| 104 | // DHT node. 0 until it does. See DHT_RELAY_TIMEOUT_MS. | |
| 105 | uint64_t relay_since; | |
| 106 | ||
| 103 | 107 | // Error info. |
| 104 | 108 | const char *error; |
| 105 | 109 | }; |
| 106 | 110 | |
| 107 | 111 | static constexpr int DHT_MAX_CONNECTIONS = 16; |
| 108 | 112 | static constexpr uint64_t DHT_CONNECT_TIMEOUT_MS = 30000; // 30 seconds. |
| 113 | // ⛔ THE RELAY STAGE GETS JS'S DEADLINE, NOT THE BACKSTOP. | |
| 114 | // | |
| 115 | // hyperdht arms `c.relayTimeout = setTimeout(onabort, 15000)` (connect.js:812) | |
| 116 | // when it starts talking to a relay, and abandons the attempt there. Ours held | |
| 117 | // every stage to the same 30s wall clock, so a peer that answered the DHT and | |
| 118 | // then went quiet occupied one of SWARM_MAX_PARALLEL = 3 outbound slots for | |
| 119 | // twice as long as the reference — and a topic carrying stale announce records | |
| 120 | // is exactly a supply of those. | |
| 121 | // | |
| 122 | // Measured from when the RELAY stage began, not from when the attempt started: | |
| 123 | // JS's timer starts with the relay, and a connection that spent twelve seconds | |
| 124 | // in find_peer has not used any of its relay budget. | |
| 125 | // | |
| 126 | // ⚠ The OTHER row of PARITY §11.1's table — JS's 10s passiveConnectTimeout for | |
| 127 | // a peer whose firewall is OPEN — is NOT implemented here. It needs a firewall | |
| 128 | // state this connector does not track at that point, and inventing a mapping | |
| 129 | // for it would be guessing at parity rather than achieving it. | |
| 130 | static constexpr uint64_t DHT_RELAY_TIMEOUT_MS = 15000; // JS connect.js:812. | |
| 109 | 131 | static constexpr uint64_t DHT_KEEPALIVE_INTERVAL_MS = 10000; // 10 seconds. |
| 110 | 132 | static constexpr uint64_t DHT_KEEPALIVE_TIMEOUT_MS = 30000; // 30 seconds. |
| 111 | 133 | static constexpr uint64_t RANDOM_PUNCH_COOLDOWN_MS = 20000; // 20 seconds between random punches. |
| … | … | @@ -57,12 +57,22 @@ int main() { |
| 57 | 57 | DhtConnector connector; |
| 58 | 58 | connector.init(&dht, &sock, &loop); |
| 59 | 59 | |
| 60 | // ⛔ COUNT FAILURES FOR THE ATTEMPT UNDER TEST, NOT ALL OF THEM. | |
| 61 | // | |
| 62 | // Blocks below deliberately leave connections ALIVE — that is what several | |
| 63 | // of them assert — and tick() advances every active one. A bare counter | |
| 64 | // therefore attributes an older block's timeout to the current block's | |
| 65 | // assertion, which is how the first version of the staged-timeout case | |
| 66 | // reported "failed one second before its deadline" for a failure that | |
| 67 | // belonged to a connection created two blocks earlier. | |
| 68 | uint32_t watch_id = 0; | |
| 60 | 69 | int failed_count = 0; |
| 61 | 70 | std::string last_error; |
| 62 | connector.on_failed = [&](uint32_t, const uint8_t *, const char *err) { | |
| 71 | connector.on_failed = [&](uint32_t id, const uint8_t *, const char *err) { | |
| 72 | printf(" (on_failed conn=%u: %s)\n", id, err ? err : "?"); | |
| 73 | if (watch_id != 0 && id != watch_id) return; | |
| 63 | 74 | failed_count++; |
| 64 | 75 | last_error = err ? err : ""; |
| 65 | printf(" (on_failed: %s)\n", last_error.c_str()); | |
| 66 | 76 | }; |
| 67 | 77 | |
| 68 | 78 | IPv4Address relay; |
| … | … | @@ -75,6 +85,7 @@ int main() { |
| 75 | 85 | // ---- 1. garbage, for an attempt that IS waiting on a relay ---- |
| 76 | 86 | { |
| 77 | 87 | const uint32_t id = connector.connect(remote_pk); |
| 88 | watch_id = id; | |
| 78 | 89 | check(id != 0, "connect() started an attempt"); |
| 79 | 90 | check(connector.set_state_for_gate(id, DHT_CONN_HOLEPUNCHING), |
| 80 | 91 | "the attempt is waiting on its relay"); |
| … | … | @@ -99,6 +110,7 @@ int main() { |
| 99 | 110 | failed_count = 0; |
| 100 | 111 | randombytes_buf(remote_pk, 32); |
| 101 | 112 | const uint32_t id = connector.connect(remote_pk); |
| 113 | watch_id = id; | |
| 102 | 114 | connector.set_state_for_gate(id, DHT_CONN_HOLEPUNCHING); |
| 103 | 115 | |
| 104 | 116 | connector.on_holepunch_response(0x10000 | id, nullptr, 0, relay); |
| … | … | @@ -111,6 +123,7 @@ int main() { |
| 111 | 123 | failed_count = 0; |
| 112 | 124 | randombytes_buf(remote_pk, 32); |
| 113 | 125 | const uint32_t id = connector.connect(remote_pk); // still FINDING |
| 126 | watch_id = id; | |
| 114 | 127 | const uint8_t garbage[8] = { 1, 2, 3, 4, 5, 6, 7, 8 }; |
| 115 | 128 | |
| 116 | 129 | connector.on_holepunch_response(0x10000 | id, garbage, sizeof(garbage), relay); |
| … | … | @@ -138,6 +151,54 @@ int main() { |
| 138 | 151 | check(failed_count == 0, "and fails nothing"); |
| 139 | 152 | } |
| 140 | 153 | |
| 154 | // ---- 6. the relay stage has JS's deadline, not the 30s backstop ---- | |
| 155 | // | |
| 156 | // PARITY §11.1: hyperdht arms relayTimeout at 15s (connect.js:812) when it | |
| 157 | // starts talking to a relay. Ours held every stage to the same 30s wall | |
| 158 | // clock, so an unresponsive peer occupied one of three outbound slots for | |
| 159 | // twice as long as the reference. | |
| 160 | { | |
| 161 | failed_count = 0; | |
| 162 | randombytes_buf(remote_pk, 32); | |
| 163 | const uint32_t id = connector.connect(remote_pk); | |
| 164 | watch_id = id; | |
| 165 | ||
| 166 | connector.tick(1000); // the connector's clock starts here | |
| 167 | check(connector.set_state_for_gate(id, DHT_CONN_HOLEPUNCHING), | |
| 168 | "an attempt reached the relay stage"); | |
| 169 | ||
| 170 | connector.tick(1000 + DHT_RELAY_TIMEOUT_MS - 1000); | |
| 171 | check(failed_count == 0 && connector.find_connection(id) != nullptr, | |
| 172 | "it is still alive one second before the relay deadline"); | |
| 173 | ||
| 174 | connector.tick(1000 + DHT_RELAY_TIMEOUT_MS + 1000); | |
| 175 | check(failed_count == 1, "and fails one second after it"); | |
| 176 | check(last_error == "relay timeout", | |
| 177 | "and says the relay timed out, not that the connection did"); | |
| 178 | check(connector.find_connection(id) == nullptr, "and the slot is given back"); | |
| 179 | } | |
| 180 | ||
| 181 | // ---- 7. and the OTHER stages keep the backstop ---- | |
| 182 | // | |
| 183 | // find_peer is released when the query exhausts (§11.2), so its 30s is a | |
| 184 | // backstop rather than the deadline. Shortening it here would abandon | |
| 185 | // attempts the DHT has not finished answering for. | |
| 186 | { | |
| 187 | failed_count = 0; | |
| 188 | randombytes_buf(remote_pk, 32); | |
| 189 | const uint32_t id = connector.connect(remote_pk); // stays in FINDING | |
| 190 | watch_id = id; | |
| 191 | ||
| 192 | connector.tick(1000); | |
| 193 | connector.tick(1000 + DHT_RELAY_TIMEOUT_MS + 5000); | |
| 194 | check(failed_count == 0 && connector.find_connection(id) != nullptr, | |
| 195 | "an attempt still in FINDING outlives the RELAY deadline"); | |
| 196 | ||
| 197 | connector.tick(1000 + DHT_CONNECT_TIMEOUT_MS + 1000); | |
| 198 | check(failed_count == 1, "and is only cut off by the 30s backstop"); | |
| 199 | check(last_error == "connection timeout", "which names itself differently"); | |
| 200 | } | |
| 201 | ||
| 141 | 202 | printf("\n%d/%d passed\n", checks - failures, checks); |
| 142 | 203 | udx_socket_close(&sock); |
| 143 | 204 | uv_run(&loop, UV_RUN_NOWAIT); |