Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[addrmgr] Unreachable control paths in addrmgr.updateAddress #356

Open
DanielKrawisz opened this issue Mar 23, 2015 · 2 comments
Open

[addrmgr] Unreachable control paths in addrmgr.updateAddress #356

DanielKrawisz opened this issue Mar 23, 2015 · 2 comments

Comments

@DanielKrawisz
Copy link
Contributor

Say you try to update the same address multiple times with addrmgr.updateAddress. Well in lines 198 - 201 in addrmanager.go there is a condition for the function to return for a certain probability without adding the input address in a new bucket. If the function does not return at that point, we go down to line 213 where the getNewBucket function is called. Then, in lines 216 to 218, the function returns if the given bucket already has that address in it. The problem is that getNewBucket always returns the same number for a given input. Therefore, if you try to add the same address twice, the function always returns without adding the address in a second bucket because getNewBucket always gives it a bucket that already the address in it.

This seems to be a bug because the function would not be written this way if getNewBucket was always expected to return the same number always for any given address. Also, there is a constant called newBucketsPerAddress which is set to four, so clearly it is expected that there could be up to 4 new buckets that could have this address in it. Also, in the Good function, we go through every new bucket to check if a given address is in it, so it seems as if potentially the address might be found in any new bucket. However, there is no way for the new address to get in any bucket but the one whose index is always returned by getNewBucket.

getNewBucket does not have comments explaining what it is supposed to do, so I'm not really sure what the expected behavior is supposed to be.

@davecgh
Copy link
Member

davecgh commented Mar 23, 2015

@dajohi: Can you look into this please.

@dajohi
Copy link
Member

dajohi commented Mar 24, 2015

I am currently looking for this subpkg to be completely rewritten.

@davecgh davecgh changed the title Unreachable control paths in addrmgr.updateAddress [addrmgr] Unreachable control paths in addrmgr.updateAddress Apr 9, 2015
jcvernaleo added a commit to jcvernaleo/btcd that referenced this issue Sep 28, 2016
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

No branches or pull requests

3 participants