Skip to content

Net: Remove "Address refresh broadcast" logic.#6745

Closed
pstratem wants to merge 1 commit into
bitcoin:masterfrom
pstratem:addr_known_reset
Closed

Net: Remove "Address refresh broadcast" logic.#6745
pstratem wants to merge 1 commit into
bitcoin:masterfrom
pstratem:addr_known_reset

Conversation

@pstratem

@pstratem pstratem commented Oct 2, 2015

Copy link
Copy Markdown

addrKnown is now a CRollingBloomFilter which forgets about entries every 5000 inserts

addrKnown is now a CRollingBloomFilter which forgets about entries every 5000 inserts.
@jgarzik

jgarzik commented Oct 2, 2015

Copy link
Copy Markdown
Contributor

This seems like it would degrade service on networks like testnet with a low level of addresses / address traffic, where the local address would be broadcast far less frequently (versus current code).

A time based rotation (clearing) is not equivalent to a traffic based rotation.

@pstratem

pstratem commented Oct 2, 2015

Copy link
Copy Markdown
Author

@jgarzik The local address is broadcast to all outbound connected peers on connect.

@laanwj laanwj added the P2P label Oct 5, 2015
@laanwj

laanwj commented Dec 3, 2015

Copy link
Copy Markdown
Member

Agree with @jgarzik. I'm not convinced that this is a good idea, specially rebroadcasting our own address once in a while is useful, I don't think we should rely on the implementation details of the rolling bloom filter to do this as a side-effect.

@laanwj laanwj closed this Dec 3, 2015
@sipa

sipa commented Dec 3, 2015

Copy link
Copy Markdown
Member

I guess I'll remove this part from #7125 as well then.

EDIT: #7125 only removes the wipe-the-filter logic, not the rebroadcast logic.

@laanwj

laanwj commented Dec 3, 2015

Copy link
Copy Markdown
Member

@sipa Well unless you think our reasoning is wrong? Is there some privacy compromise here?
(at some point we might replace the rolling bloom filters with something else again, and we may have forgotten the decision to remove this, hence I'm somewhat in favor of doing it explicitly)

@laanwj laanwj reopened this Dec 3, 2015
@sipa

sipa commented Dec 3, 2015

Copy link
Copy Markdown
Member

I guess there are a few ways forward:

  • Keep as-is, drop the entire filter every 24 hours + rebroadcast.
  • Don't drop the filter, but still rebroadcast your own address periodically in a forceful way (bypassing the filter).
  • Don't drop the filter, but still rebroadcast periodically (subject to the filter, which means no rebroadcast if there was not enough noise to hide the data in).
  • This PR (no drop, no rebroadcast).

I think I prefer 3.

@pstratem

pstratem commented Dec 4, 2015

Copy link
Copy Markdown
Author

@sipa how about randomly settings bits in the filter to 0 instead of completely clearing it?

@pstratem pstratem closed this Dec 8, 2015
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants