Remove peer exchange from main; it was merged unreviewed - #27
Merged
Conversation
PR #25 was opened, reviewed and approved as a single-file change to net.cpp: the .btf peer cache, +169/-0. I then pushed peer exchange and a /port option onto that same branch while the PR was open, which silently widened it to six files. The merge carried all of it in. Nothing about the peer exchange had been validated. Its two-node test never completed -- node B could not dial node A at all -- so what landed on main is unproven network protocol code that no one agreed to take. This restores main to what #25 actually described. The peer cache stays, with its runtime numbers (36.3s to 2.1s to first peer). #26 stays. What comes out: - the "btfpeers" message, both directions - BtfPexCollect / BtfPexAccept and the per-message caps - CNode::nLastPexRecv - the `desc` field on the cached peer record - BtfLocalDescriptor / BtfSecpContext - /port Peer exchange returns as its own pull request once its test passes. The /port option is worth having on its own -- two nodes cannot share a machine without it, which is what blocked the test -- and should go up separately rather than ride along again. Release v1.2.2 is unaffected: it was built from 3caee27, before either merge.
Vic-Nas
added a commit
to Vic-Nas/bitflash-mod
that referenced
this pull request
Jul 29, 2026
…sh-sh#25) Discovery otherwise depends entirely on the Nostr relays -- a node that's been up for hours and knows exactly which peers respond throws all of it away on exit, and the next start walks the same mostly-dead descriptor list again. Now persists (address, meeting node, encryption key) for peers that actually connected, and a new thread dials up to 8 of them before relay discovery has said anything. Upstream measured this as 36.3s to 2.1s to first peer. Ported only the part of Bitflash-sh#25 that's still actually on their main: the peer-exchange protocol and desc-field additions that rode along on the same PR were reverted by them shortly after (Bitflash-sh#27, merged unreviewed, its own two-node test never completed) -- not carrying that part over. main_gui: implement the port command-line option, which net.cpp already documented (Bitflash-sh#28) nListenPort was described as tunable via a command-line option since it was written; nothing ever parsed it, so a second node couldn't start on a machine already running one. Sets addrLocalHost.port too, or the node announces a port it never bound. util: stop debug.log dropping lines under concurrency (Bitflash-sh#29) strDebugFile's path resolution was outside any lock while the write itself (plus our rotation logic, added independently) was already inside one -- so one thread could read the path while another was still assigning it. On a race, fopen got a torn path, returned NULL, and the line was dropped silently: a missing line then reads as "this thread never ran," which is unsound. Path resolution now shares the same lock as the write/rotate. nostr: cap reassembled WebSocket message size (rest of Bitflash-sh#17) Only fixed the j[0]-type-check half of this earlier and dismissed the frame-size half as moot post-libwebsockets -- that was wrong. lws bounds a single fragment, but m_curFrame reassembly across fragments had no ceiling of its own, so a relay sending one message in many small fragments could still make us allocate unbounded memory. Capped at 4 MiB, matching the number used in the original hand-rolled-reader fix this project shipped once already.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What happened
#25 was opened, reviewed and approved as one file: the
.btfpeer cache innet.cpp, +169/-0. While it was open I pushed peer exchange and a/portoption onto the same branch, which widened it to six files without saying so.
The merge took all of it.
None of the peer exchange had been validated. Its two-node test never
completed — node B could not dial node A — and I said as much at the time
("code that has not passed an end-to-end test should not be a PR yet") while
the code was already sitting in a PR I had asked to be merged. That was my
mistake, not a review failure.
What this does
Restores
mainto what #25 described. Removed:btfpeersmessage, send and receiveBtfPexCollect/BtfPexAcceptand the per-message capsCNode::nLastPexRecvdescfield on the cached peer recordBtfLocalDescriptor/BtfSecpContext/portKept, untouched: the peer cache from #25 (36.3s → 2.1s to first peer, measured)
and the rendezvous descriptor fix from #26.
Build green on MSYS2 UCRT64.
Not affected
Release v1.2.2 was built from
3caee27, before either merge. Nothingpublished contains this code.
What comes back, separately
Peer exchange returns as its own PR once its test passes. The design holds
up — signed self-certifying descriptors, so the sender is trusted for nothing —
it simply has not been shown to work end to end.
/portdeserves its own PR.net.cpphas claimednListenPortis "tunablevia /port" since it was written and nothing ever parsed it, so two nodes cannot
share a machine — which is exactly what blocked the peer exchange test.