Skip to content

update address when flipping firewall - #277

Open
marco-hp wants to merge 3 commits into
holepunchto:mainfrom
marco-hp:flip-fw
Open

update address when flipping firewall#277
marco-hp wants to merge 3 commits into
holepunchto:mainfrom
marco-hp:flip-fw

Conversation

@marco-hp

@marco-hp marco-hp commented Jul 28, 2026

Copy link
Copy Markdown
Member

This is a race condition

  • Racer A: the node's own bootstrap self-check (dht-rpc's _bootstrap() → the "quick NAT heuristic" → _checkIfFirewalled()). This runs automatically once you start using a freshly-constructed HyperDHT, and it can flip the shared dht.firewalled boolean from true to false within milliseconds.

  • Racer B: that node's own holepunch negotiation for the in-flight connection — specifically, the moment roundPunch() builds round 1's payload, it reads c.puncher.nat.firewall and c.puncher.nat.addresses at that instant and ships them to the peer. That per-connection Nat instance derives its firewall value from the same shared dht.firewalled boolean Racer A is mutating.

-- coworking with Claude

@marco-hp
marco-hp requested a review from a team July 28, 2026 11:37
marco-hp and others added 2 commits July 28, 2026 21:40
`_updateAddresses()` is usually called via `.add()` so has a sample. If
the firewall is `OPEN` there is a lack of addresses, but we don't need
to return the dht's apparent remote address if we have a sample.

Added a test for each with the zero sample case being triggered via
`unfreeze()` as that's the only way it theoretically be set. This may
never be done in practice however.
@lejeunerenard

Copy link
Copy Markdown
Contributor

@marco-hp Thanks for catching this! The Nat should return addresses even if the firewall is OPEN.

I pushed some changes as the solution doesn't account for samples that are available. I also adjusted the existing "zero-sample" test to use .unfreeze() as the path to update the addresses as nat.update() is not called outside of add(), though .unfreeze() itself is also never called, but theoretically could be. Finally I added a new test case to cover the case where there is a sample and the DHT flips to no longer being firewalled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants