net: retire ThreadGetMyExternalIP + HTTP path (after #7 lands) #5
Labels
No labels
enhancement
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
SubGeniusFinance/Offerings-to-Cthulhu#5
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Background
PR #4 (merged in
1439f4f) fixed the immediate breakage inGetMyExternalIPby swapping the dead hardcoded IPs for three modern services and switching to HTTP/1.0. That's the correct minimal fix for making it work.The longer-term direction is to delete the HTTP path entirely — what upstream Bitcoin Core did in bitcoin/bitcoin#7028 (2016). Reasons:
-externalip=anyway).AddLocalattack surface (a hostile response on the HTTP path gets fed toAddLocal(addrLocalHost, LOCAL_HTTP)and gossipped viaaddr).GetMyExternalIP2— small piece of attack surface in its own right.ThreadGetMyExternalIP) and the address-discovery delay at startup.Prerequisite: issue #7 must land first
This issue is now scoped to HTTP-path deletion only. The
SeenLocal → AddLocal(LOCAL_PEER)swap that lets peer votes seedmapLocalHost— which is what makes deleting the HTTP path safe — has been split off to #7. Land #7 first; let peer-vote scores accumulate on a live node in-debug=netlogs to confirm the replacement path is working; then do this deletion.Proposed change (after #7)
Delete:
GetMyExternalIP(src/net.cpp:346-403)GetMyExternalIP2(src/net.cpp:300-344)ThreadGetMyExternalIP(src/net.cpp:405-412)src/net.cpp:1711(threadGroup.create_thread(... &ThreadGetMyExternalIP))src/net.hUntouched: UPnP path (
src/net.cpp:1104), local-interface enumeration (src/net.cpp:1655-1701),LOCAL_HTTPenum value (left insrc/net.heven though no path produces it anymore — leaving the slot avoids reshuffling other enum values' integer scores, which could affect on-disk peers.dat compatibility).Net delta: roughly -110 lines, +0 lines, no new dependencies.
Trade-offs worth knowing
pfrom->fInboundguard atmain.cpp:3895means only inbound peers feedaddrMein. A node behind NAT with no port forwarding gets nothing from peer votes. Acceptable because such a node is unreachable anyway — nothing useful to advertise. Same trade-off upstream accepted.addrMe-from-peer requires a completed inbound handshake. On a small chain like OFF this may take meaningfully longer, especially for a fresh-syncing node that's mostly making outbound connections during IBD. Cosmetic, but real.-externalip=or are reachable by known IP; this code path is firing into the void today and will fire into the void tomorrow either way.References
@9019x — same offer as before, no obligation. If #7 is the friendlier first contribution and this is the larger follow-up, that's an even cleaner split than I originally framed it.
Closed by PR #9 (merge commit
24196aa).PR #9 deleted all five items from this issue's proposal:
GetMyExternalIPGetMyExternalIP2ThreadGetMyExternalIPDiscover()net.hOne small divergence from what this issue specified: this issue suggested leaving the
LOCAL_HTTPenum slot in place to preservepeers.datinteger-score compat. PR #9 actually removed it, shiftingLOCAL_MANUALfrom 6 → 5. Acceptable on OFF —peers.datscores are advisory (not consensus), and post-Restoration most node operators are starting fresh anyway. Flagging for the record, not blocking.Thanks @9019x.
Self-correction on the comment above: I flagged the
LOCAL_HTTPenum removal as apeers.datcompatibility risk. That's wrong —mapLocalHostis runtime-only memory (rebuilt every startup from-bind=/-externalip=/ interface enum / UPnP), not persisted topeers.dator anywhere else. The enum integer values are internal-runtime-only. PR #9's removal ofLOCAL_HTTPhas zero cross-version footprint.The "preserve the slot" line in the original issue text was also wrong on the same point. No-op risk; correct decision in the PR to just remove it cleanly.