OverviewFilesCommitsIssuesDocsReleasesPeers

hyperswarm-peer commits / e6a0fbd

liboverlay: cancel() and progress(), and the landmine the obvious cancel lays
swarmd's content plane needs CONTENT_CANCEL and CONTENT_PROGRESS, and Protocol had neither — HyperOverlay::cancel answered "cancel is not implemented yet" and its content_progress signal has never fired, because nothing produces a progress event. Both are now real: cancel(hash, discard) pauses or discards, and progress(hash, ...) reports bytes and chunks, distinguishing "not fetching" from "0 of 0" because a caller needs that difference. ⛔ THE FIRST cancel() SENT chunkCancel FOR EVERY OUTSTANDING CHUNK, which is the obvious thing and is wrong. chunkCancel is a HOTSWAP primitive: the holder skips that (path, index) exactly once and forgets it, because the need it is racing is already in flight. Cancelling a whole download therefore arms one landmine per chunk, and the next request for the same content walks into all of them — every chunk skipped once, nothing reported by either side, the resumed fetch stalled for ever. The new resume assertion caught it. The fix that suggests itself — "a fresh need supersedes an older cancel" — was tried and is also wrong: it takes test_backpressure's "the cancelled chunks are skipped" from 18/18 to 17/18, because that IS the race chunkCancel exists to win. So cancel() stays silent, and pays a bounded cost stated at the call site: the holders finish the serve jobs they had, and our on_chunk_data drops bytes for a download that is gone. Also corrected the README's "not started: attach the binding to live peers" — that landed some time ago; HyperOverlay::udx_add_peer does exactly it. Suite: 368 assertions across eight runners, all PASS (was 344). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
e6a0fbde04cc10ea88ebdf9e807921112af0b0d4
by plankatron <fleeky@gmail.com> · 2026-09-10T19:11:05.000Z

Changed files (3) +168 -12

liboverlay/README.md +21 -12
……@@ -94,11 +94,15 @@ running it prints a confident green for code that does not exist.
9494 liboverlay/test/test_protocol.cpp $(pkg-config --libs libsodium) \
9595 && HO_CONFORMANCE=$CONF /tmp/ho_proto
9696
97 g++ $FLAGS -I src $(pkg-config --cflags libsodium) -o /tmp/ho_adapter \
97 # The adapter reaches into src/, and src/compact_encoding.h now forwards to
98 # libcompact — so this one needs those headers and objects. test/build_tests.sh
99 # compiles them into /tmp/hsp-obj.
100 g++ $FLAGS -I src -I thirdparty/libcompact/include -I thirdparty/libutf/include \
101 $(pkg-config --cflags libsodium) -o /tmp/ho_adapter \
98102 liboverlay/test/test_transport_adapter.cpp src/protomux.cpp \
99 $(pkg-config --libs libsodium) && /tmp/ho_adapter
103 /tmp/hsp-obj/compact_*.o $(pkg-config --libs libsodium) && /tmp/ho_adapter
100104
101Current: **42 + 61 + 34 + 52 + 72 + 18 + 54 + 11 = 344 assertions, PASS**, and
105Current: **42 + 61 + 34 + 52 + 72 + 18 + 78 + 11 = 368 assertions, PASS**, and
102106clean under `-fsanitize=address,undefined`.
103107
104108## Status
……@@ -120,7 +124,8 @@ Done:
120124 stalled holder, reassignment on peer loss, idle watchdog (**52 assertions**)
121125- protocol driver — multi-source fetch/serve, neither side holding a whole file,
122126 paged chunk lists both ways with the F2 accumulator cap, conduct asserted
123 against the JS transcripts (**54 assertions**)
127 against the JS transcripts, plus `cancel()` (pause vs discard) and `progress()`
128 (**78 assertions**)
124129- serve pump — single-flight per peer, parks on a full write buffer and resumes
125130 where it stopped, hotswap cancels honoured (**18 assertions**)
126131- Protomux transport adapter, end to end over real framing, including
……@@ -196,14 +201,18 @@ skipping ONE registration (shifting every later slot by one) takes the adapter
196201test from 7/7 to 0/7 — no error is raised anywhere, the transfer simply moves 21
197202bytes instead of 206,484. That is what misrouting looks like in the field.
198203
199Not started, in dependency order (see `DESIGN-transfer-manager.md` for the scope
200decision behind each):
2014. **Attach the binding to live peers.** `HyperOverlay` (src/overlay_node.*) is
202 registered, runs its worker thread and drives the library correctly, but
203 nothing calls `Protocol::add_peer` — the OverlayTransport instances are never
204 created from `UdxTransport`'s connections. Until that lands, a fetch over the
205 real swarm finds no holders and correctly gives up on the watchdog, which is
206 exactly what the headless smoke test asserts today.
204**Live peers are attached** (this section said otherwise until 2026-09-10, and
205was wrong): `HyperOverlay::attach` installs `UdxTransport::set_mux_callbacks`, and
206`udx_add_peer` builds an `OverlayTransport` per connection whose ready callback
207calls `Protocol::add_peer`. swarmd does the same thing without Godot — and it has
208to do it itself rather than embed `HyperOverlay`, because `set_mux_callbacks`
209holds ONE pair of callbacks and whoever sets it last owns the peer lifecycle.
210
211⛔ `cancel()` deliberately does NOT send chunkCancel to the holders. chunkCancel
212is a hotswap primitive the holder consumes ONCE per (path, index); cancelling
213every outstanding chunk arms one skip per chunk, and the next fetch of the same
214content walks into all of them and stalls in silence. Measured. See the comment
215at `Protocol::cancel`.
207216
208217**Rebuilding after a transport change.** Step 6 changed `src/protomux.cpp` and
209218`src/udx_transport.cpp`, so the extension must be rebuilt for any of it to matter
liboverlay/include/overlay/protocol.h +62 -0
……@@ -338,6 +338,68 @@ public:
338338
339339 // Callers name a content hash; the map is keyed by its synthetic path.
340340 bool has_download(const std::string &h) const { return downloads_.count(content_path(h)) != 0; }
341
342 // Stop a fetch. `discard` destroys the partial; otherwise it is kept and the
343 // next request() for the same hash resumes it — that distinction is the whole
344 // reason a partial exists, and transfer.h implements both halves already.
345 //
346 // ⛔ IT DOES NOT SEND chunkCancel, AND THAT IS NOT AN OVERSIGHT.
347 //
348 // The obvious version tells the holders to stop, so they are not pumping
349 // bytes at a transfer that has stopped reading. It cannot: chunkCancel is a
350 // HOTSWAP primitive with one-shot semantics — the holder skips that
351 // (path, index) exactly ONCE and forgets it, because the need it is racing is
352 // already in flight. Cancelling every outstanding chunk therefore arms one
353 // landmine per chunk, and the next request for the same content walks into
354 // all of them: every chunk skipped once, nothing reported by either side, the
355 // resumed fetch stalled. Measured — that is what the resume assertion below
356 // caught, and it is why `test_backpressure`'s "the cancelled chunks are
357 // skipped" is not a rule that can be relaxed to fix it.
358 //
359 // The cost of staying silent is bounded and stated: the holders finish the
360 // serve jobs they already had, and our on_chunk_data drops the bytes because
361 // the download is gone. For a cancelled transfer that is the tail of one
362 // file. The alternative is a protocol frame that does not exist, and
363 // inventing one would stop us talking to the JS peers in production.
364 bool cancel(const std::string &contentHash, bool discard) {
365 const std::string path = content_path(contentHash);
366 auto it = downloads_.find(path);
367 if (it == downloads_.end()) return false;
368 Download &d = *it->second;
369
370 if (d.started) {
371 if (discard) d.transfer.cancel();
372 else d.transfer.pause();
373 }
374
375 drop_pages(path);
376 downloads_.erase(it);
377 HO_LOG("cancelled %s (%s)\n", contentHash.substr(0, 12).c_str(),
378 discard ? "discarded" : "partial kept");
379 return true;
380 }
381
382 // What a caller can see of a running fetch. False when nothing is running for
383 // that hash — which is a different answer from "0 of 0" and callers need it.
384 bool progress(const std::string &contentHash, uint64_t &received_bytes,
385 size_t &received, size_t &total) const {
386 auto it = downloads_.find(content_path(contentHash));
387 if (it == downloads_.end()) return false;
388 const Download &d = *it->second;
389 received_bytes = d.transfer.received_bytes();
390 received = d.transfer.received();
391 total = d.transfer.total();
392 return true;
393 }
394
395 // The hashes of every fetch currently running, for a caller that reports on
396 // all of them rather than polling one it already knows about.
397 std::vector<std::string> active_downloads() const {
398 std::vector<std::string> out;
399 out.reserve(downloads_.size());
400 for (const auto &kv : downloads_) out.push_back(kv.second->contentHash);
401 return out;
402 }
341403 const Download *download(const std::string &h) const {
342404 auto it = downloads_.find(content_path(h));
343405 return it == downloads_.end() ? nullptr : it->second.get();
liboverlay/test/test_protocol.cpp +85 -0
……@@ -158,6 +158,17 @@ static void pump() {
158158 }
159159}
160160
161// Deliver at most `n` frames, so a test can stop a transfer half-way. pump()
162// drains to quiet, which is the wrong instrument for anything that happens
163// DURING a transfer.
164static void pump_n(size_t n) {
165 for (size_t i = 0; i < n && !Loopback::queue.empty(); i++) {
166 auto item = Loopback::queue.front();
167 Loopback::queue.erase(Loopback::queue.begin());
168 item.target->on_message(item.from, item.id, item.frame);
169 }
170}
171
161172static std::vector<uint8_t> make_bytes(size_t n, uint32_t seed) {
162173 std::vector<uint8_t> v(n);
163174 uint32_t s = seed;
……@@ -884,6 +895,80 @@ int main() {
884895 check(received == content, "and the bytes are the original");
885896 }
886897
898 // ---- cancel, and what a caller can see while a transfer runs -----------
899 //
900 // A fetch that cannot be stopped is a fetch that owns the disk until it
901 // finishes, and a caller with no view of it can only report "downloading".
902 // swarmd's CONTENT_CANCEL and CONTENT_PROGRESS are these two methods.
903 //
904 // The distinction the partial exists for: pausing KEEPS it so the next
905 // request resumes; discarding removes it.
906 {
907 printf("\n-- cancel and progress\n");
908 reset_wire();
909
910 const std::vector<uint8_t> content = make_bytes(200 * 1024, 23);
911 const std::string oid = hash_of(content);
912 const std::string src = write_temp("ho_p_src_cancel.bin", content);
913 const std::string dest = tmp_path("ho_p_dst_cancel.bin");
914 std::remove(dest.c_str());
915 std::remove(xfer::partial_path_for(dest).c_str());
916
917 index::FileIndex ia, ib;
918 std::remove(tmp_path("ho_p_ca.log").c_str()); std::remove(tmp_path("ho_p_cb.log").c_str());
919 ia.open(tmp_path("ho_p_ca.log"));
920 ib.open(tmp_path("ho_p_cb.log"));
921
922 Protocol alice(&ia, holder_of(oid, src));
923 Protocol bob(&ib, holds_nothing);
924 Loopback la, lb;
925 la.label = "alice"; lb.label = "bob";
926 connect(la, alice, lb, bob);
927
928 bool done = false;
929 bob.on_complete = [&](const std::string &, const std::string &) { done = true; };
930
931 uint64_t bytes = 0; size_t got = 0, total = 0;
932 check(!bob.progress(oid, bytes, got, total),
933 "progress on a hash nobody is fetching is not an answer");
934
935 bob.request(oid, dest);
936 pump_n(4); // request -> chunkHashes -> chunkNeed -> some chunkData
937
938 check(bob.progress(oid, bytes, got, total), "progress is readable mid-transfer");
939 check(total > 1, "and knows how many chunks the file has (" + std::to_string(total) + ")");
940 check(got > 0 && got < total,
941 "and reports a transfer in progress, not 0 and not finished (" +
942 std::to_string(got) + "/" + std::to_string(total) + ")");
943
944 check(bob.cancel(oid, /*discard=*/false), "cancel finds the download");
945 check(!bob.has_download(oid), "and the download is gone");
946 check(io::file_exists(xfer::partial_path_for(dest)),
947 "a PAUSED cancel keeps the partial — that is what it is for");
948
949 pump(); // frames still in flight must land nowhere
950 check(!done, "nothing completes after a cancel");
951 check(!io::file_exists(dest), "and no destination file appears");
952
953 // ---- and the same fetch resumes rather than restarting ----
954 bob.request(oid, dest);
955 pump();
956 check(done, "the fetch resumes and completes");
957 check(read_file(dest) == content, "and the resumed bytes are the original");
958
959 // ---- discard removes the partial ----
960 const std::string dest2 = tmp_path("ho_p_dst_cancel2.bin");
961 std::remove(dest2.c_str());
962 std::remove(xfer::partial_path_for(dest2).c_str());
963 bob.request(oid, dest2);
964 pump_n(4);
965 check(bob.cancel(oid, /*discard=*/true), "cancel finds the second download");
966 check(!io::file_exists(xfer::partial_path_for(dest2)),
967 "a DISCARDING cancel takes the partial with it");
968
969 check(!bob.cancel(oid, false), "cancelling what is not running is false, not a crash");
970 }
971
887972 printf("\n %d assertions pass, %d fail\n", passed, failed);
888973 printf("%s\n", failed == 0 ? "PASS" : "FAIL");
889974 return failed == 0 ? 0 : 1;