Skip to content

Conversation

@theuni
Copy link
Member

@theuni theuni commented May 19, 2017

There have been a few PRs lately that have failed checks due to a bug introduced in #10287. The first check happened to work already because the chosen addresses didn't collide. The addrman.Clear() after that check causes addrman's salt to be reset, making all future tests non-deterministic.

When inserting two addresses of the same class, from the same source, there's a 1/64 chance that their hashes will collide. So it's faulty logic to be checking for an exact size. @sipa, mind verifying that?

I'd like to fix this properly by ensuring determinism in the test as a next step, but I'd rather fix the spurious failures asap.

When inserting two addresses of the same class, from the same source, they have
a 1/64 chance of colliding.
@fanquake fanquake added the Tests label May 19, 2017
@laanwj
Copy link
Member

laanwj commented May 19, 2017

utACK 6b51b0a, had noticed this one as well, thanks for fixing

@laanwj laanwj merged commit 6b51b0a into bitcoin:master May 19, 2017
laanwj added a commit that referenced this pull request May 19, 2017
6b51b0a tests: fix spurious addrman test failure (Cory Fields)

Tree-SHA512: 3d41723b1a31ff459d950331ffea7f383e4ef6187990be6a634978bead0c29d7c096f68e7edb6d4dc56069c1fe8a6f12a6daf573cb1e542b15d000eaa54ad288
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 24, 2019
6b51b0a tests: fix spurious addrman test failure (Cory Fields)

Tree-SHA512: 3d41723b1a31ff459d950331ffea7f383e4ef6187990be6a634978bead0c29d7c096f68e7edb6d4dc56069c1fe8a6f12a6daf573cb1e542b15d000eaa54ad288
@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.

3 participants