{"action":"created","comment":{"url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/comments/48060871","id":48060871,"diff_hunk":"@@ -123,26 +193,26 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions)\n     // Set addrman addr placement to be deterministic.\n     addrman.MakeDeterministic();\n \n-    CNetAddr source = CNetAddr(\"252.2.2.2:8333\");\n+    CNetAddr source = CNetAddr(\"252.2.2.2\");\n \n     BOOST_CHECK(addrman.size() == 0);\n \n-    for (unsigned int i = 1; i < 4; i++){\n-        CService addr = CService(\"250.1.1.\"+boost::to_string(i));\n+    for (unsigned int i = 1; i < 18; i++) {","path":"src/test/addrman_tests.cpp","position":184,"original_position":184,"commit_id":"8e05d6e81f92dc47f77540242f2cf7d23d78fb6f","original_commit_id":"8e05d6e81f92dc47f77540242f2cf7d23d78fb6f","user":{"login":"EthanHeilman","id":274814,"avatar_url":"https://avatars.githubusercontent.com/u/274814?v=3","gravatar_id":"","url":"https://api.github.com/users/EthanHeilman","html_url":"https://github.com/EthanHeilman","followers_url":"https://api.github.com/users/EthanHeilman/followers","following_url":"https://api.github.com/users/EthanHeilman/following{/other_user}","gists_url":"https://api.github.com/users/EthanHeilman/gists{/gist_id}","starred_url":"https://api.github.com/users/EthanHeilman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/EthanHeilman/subscriptions","organizations_url":"https://api.github.com/users/EthanHeilman/orgs","repos_url":"https://api.github.com/users/EthanHeilman/repos","events_url":"https://api.github.com/users/EthanHeilman/events{/privacy}","received_events_url":"https://api.github.com/users/EthanHeilman/received_events","type":"User","site_admin":false},"body":"The test is testing if collisions are handled property, thus we need a collision to occur. However the round at which a collision occurs is dependent on the input values. One of the input values is a CNetAddr, so fixing the issue with CNetAddr changed the round in which a collision occurs from the 18th round rather than the 4th round. ","created_at":"2015-12-18T19:33:40Z","updated_at":"2015-12-18T19:33:40Z","html_url":"https://github.com/bitcoin/bitcoin/pull/7212#discussion_r48060871","pull_request_url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/7212","_links":{"self":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/comments/48060871"},"html":{"href":"https://github.com/bitcoin/bitcoin/pull/7212#discussion_r48060871"},"pull_request":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/7212"}}},"pull_request":{"url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/7212","id":53583551,"html_url":"https://github.com/bitcoin/bitcoin/pull/7212","diff_url":"https://github.com/bitcoin/bitcoin/pull/7212.diff","patch_url":"https://github.com/bitcoin/bitcoin/pull/7212.patch","issue_url":"https://api.github.com/repos/bitcoin/bitcoin/issues/7212","number":7212,"state":"open","locked":false,"title":"Adds unittests for CAddrMan and CAddrinfo, removes source of non-determinism.","user":{"login":"EthanHeilman","id":274814,"avatar_url":"https://avatars.githubusercontent.com/u/274814?v=3","gravatar_id":"","url":"https://api.github.com/users/EthanHeilman","html_url":"https://github.com/EthanHeilman","followers_url":"https://api.github.com/users/EthanHeilman/followers","following_url":"https://api.github.com/users/EthanHeilman/following{/other_user}","gists_url":"https://api.github.com/users/EthanHeilman/gists{/gist_id}","starred_url":"https://api.github.com/users/EthanHeilman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/EthanHeilman/subscriptions","organizations_url":"https://api.github.com/users/EthanHeilman/orgs","repos_url":"https://api.github.com/users/EthanHeilman/repos","events_url":"https://api.github.com/users/EthanHeilman/events{/privacy}","received_events_url":"https://api.github.com/users/EthanHeilman/received_events","type":"User","site_admin":false},"body":"* Adds unittests for several methods in CAddrMan which are not currently tested.\r\n\r\n* Creates unittests for CAddrinfo which had no test coverage. These tests validate security critical features.\r\n\r\n* Refactors addrman so that tests can overrides the random number generator to allow for deterministic tests of CAddrman.select that involve tests of more than one addr in tried or new. See justification below.\r\n\r\n* We move the MakeDeterministic method from addrman into a test class to reduce lines of code in addrman and to prevent non-test calls.\r\n\r\nDetails on GetRandInt Wrapper\r\n========\r\n\r\nTo allow for deterministic tests of the select method in CAddrMan we wrap the GetRandInt function with a method RandomInt which can be overridden in a test class CAddrManTest. The RandomInt wrapper name was used so that it would not be a prefix substring of GetRandInt to avoid find and replace mistakes. Changes to random number generators as always a concern, this approach presents no risks because only subclasses of addrman can redirect calls to GetRandInt.\r\n\r\n\r\nIPv4 IPv6 confusion\r\n=======\r\n\r\n~~Bitcoin addr constructors assume any IP string that contains a semicolon is a IPv6 address. Thus\r\nCService(\"250.1.2.1:9999\") will create an IPv6 address with a default port number 8333~~.  I have discovered this behavior only applies to CNetAddr constructors since CNetAddr does not take a port number.","created_at":"2015-12-14T16:01:11Z","updated_at":"2015-12-18T19:33:40Z","closed_at":null,"merged_at":null,"merge_commit_sha":"97bf192943aa2ffea4666490e652f893c9fed251","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/7212/commits","review_comments_url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/7212/comments","review_comment_url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/comments{/number}","comments_url":"https://api.github.com/repos/bitcoin/bitcoin/issues/7212/comments","statuses_url":"https://api.github.com/repos/bitcoin/bitcoin/statuses/8e05d6e81f92dc47f77540242f2cf7d23d78fb6f","head":{"label":"EthanHeilman:unittest","ref":"unittest","sha":"8e05d6e81f92dc47f77540242f2cf7d23d78fb6f","user":{"login":"EthanHeilman","id":274814,"avatar_url":"https://avatars.githubusercontent.com/u/274814?v=3","gravatar_id":"","url":"https://api.github.com/users/EthanHeilman","html_url":"https://github.com/EthanHeilman","followers_url":"https://api.github.com/users/EthanHeilman/followers","following_url":"https://api.github.com/users/EthanHeilman/following{/other_user}","gists_url":"https://api.github.com/users/EthanHeilman/gists{/gist_id}","starred_url":"https://api.github.com/users/EthanHeilman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/EthanHeilman/subscriptions","organizations_url":"https://api.github.com/users/EthanHeilman/orgs","repos_url":"https://api.github.com/users/EthanHeilman/repos","events_url":"https://api.github.com/users/EthanHeilman/events{/privacy}","received_events_url":"https://api.github.com/users/EthanHeilman/received_events","type":"User","site_admin":false},"repo":{"id":32641671,"name":"bitcoin","full_name":"EthanHeilman/bitcoin","owner":{"login":"EthanHeilman","id":274814,"avatar_url":"https://avatars.githubusercontent.com/u/274814?v=3","gravatar_id":"","url":"https://api.github.com/users/EthanHeilman","html_url":"https://github.com/EthanHeilman","followers_url":"https://api.github.com/users/EthanHeilman/followers","following_url":"https://api.github.com/users/EthanHeilman/following{/other_user}","gists_url":"https://api.github.com/users/EthanHeilman/gists{/gist_id}","starred_url":"https://api.github.com/users/EthanHeilman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/EthanHeilman/subscriptions","organizations_url":"https://api.github.com/users/EthanHeilman/orgs","repos_url":"https://api.github.com/users/EthanHeilman/repos","events_url":"https://api.github.com/users/EthanHeilman/events{/privacy}","received_events_url":"https://api.github.com/users/EthanHeilman/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/EthanHeilman/bitcoin","description":"Bitcoin Core integration/staging tree","fork":true,"url":"https://api.github.com/repos/EthanHeilman/bitcoin","forks_url":"https://api.github.com/repos/EthanHeilman/bitcoin/forks","keys_url":"https://api.github.com/repos/EthanHeilman/bitcoin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/EthanHeilman/bitcoin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/EthanHeilman/bitcoin/teams","hooks_url":"https://api.github.com/repos/EthanHeilman/bitcoin/hooks","issue_events_url":"https://api.github.com/repos/EthanHeilman/bitcoin/issues/events{/number}","events_url":"https://api.github.com/repos/EthanHeilman/bitcoin/events","assignees_url":"https://api.github.com/repos/EthanHeilman/bitcoin/assignees{/user}","branches_url":"https://api.github.com/repos/EthanHeilman/bitcoin/branches{/branch}","tags_url":"https://api.github.com/repos/EthanHeilman/bitcoin/tags","blobs_url":"https://api.github.com/repos/EthanHeilman/bitcoin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/EthanHeilman/bitcoin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/EthanHeilman/bitcoin/git/refs{/sha}","trees_url":"https://api.github.com/repos/EthanHeilman/bitcoin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/EthanHeilman/bitcoin/statuses/{sha}","languages_url":"https://api.github.com/repos/EthanHeilman/bitcoin/languages","stargazers_url":"https://api.github.com/repos/EthanHeilman/bitcoin/stargazers","contributors_url":"https://api.github.com/repos/EthanHeilman/bitcoin/contributors","subscribers_url":"https://api.github.com/repos/EthanHeilman/bitcoin/subscribers","subscription_url":"https://api.github.com/repos/EthanHeilman/bitcoin/subscription","commits_url":"https://api.github.com/repos/EthanHeilman/bitcoin/commits{/sha}","git_commits_url":"https://api.github.com/repos/EthanHeilman/bitcoin/git/commits{/sha}","comments_url":"https://api.github.com/repos/EthanHeilman/bitcoin/comments{/number}","issue_comment_url":"https://api.github.com/repos/EthanHeilman/bitcoin/issues/comments{/number}","contents_url":"https://api.github.com/repos/EthanHeilman/bitcoin/contents/{+path}","compare_url":"https://api.github.com/repos/EthanHeilman/bitcoin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/EthanHeilman/bitcoin/merges","archive_url":"https://api.github.com/repos/EthanHeilman/bitcoin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/EthanHeilman/bitcoin/downloads","issues_url":"https://api.github.com/repos/EthanHeilman/bitcoin/issues{/number}","pulls_url":"https://api.github.com/repos/EthanHeilman/bitcoin/pulls{/number}","milestones_url":"https://api.github.com/repos/EthanHeilman/bitcoin/milestones{/number}","notifications_url":"https://api.github.com/repos/EthanHeilman/bitcoin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/EthanHeilman/bitcoin/labels{/name}","releases_url":"https://api.github.com/repos/EthanHeilman/bitcoin/releases{/id}","created_at":"2015-03-21T17:13:38Z","updated_at":"2015-05-05T15:09:41Z","pushed_at":"2015-12-15T22:56:11Z","git_url":"git://github.com/EthanHeilman/bitcoin.git","ssh_url":"git@github.com:EthanHeilman/bitcoin.git","clone_url":"https://github.com/EthanHeilman/bitcoin.git","svn_url":"https://github.com/EthanHeilman/bitcoin","homepage":"https://bitcoin.org/en/download","size":58516,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"bitcoin:master","ref":"master","sha":"7a5040155ed59f8c9c51734bb2ee29f1593eaa6a","user":{"login":"bitcoin","id":528860,"avatar_url":"https://avatars.githubusercontent.com/u/528860?v=3","gravatar_id":"","url":"https://api.github.com/users/bitcoin","html_url":"https://github.com/bitcoin","followers_url":"https://api.github.com/users/bitcoin/followers","following_url":"https://api.github.com/users/bitcoin/following{/other_user}","gists_url":"https://api.github.com/users/bitcoin/gists{/gist_id}","starred_url":"https://api.github.com/users/bitcoin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bitcoin/subscriptions","organizations_url":"https://api.github.com/users/bitcoin/orgs","repos_url":"https://api.github.com/users/bitcoin/repos","events_url":"https://api.github.com/users/bitcoin/events{/privacy}","received_events_url":"https://api.github.com/users/bitcoin/received_events","type":"Organization","site_admin":false},"repo":{"id":1181927,"name":"bitcoin","full_name":"bitcoin/bitcoin","owner":{"login":"bitcoin","id":528860,"avatar_url":"https://avatars.githubusercontent.com/u/528860?v=3","gravatar_id":"","url":"https://api.github.com/users/bitcoin","html_url":"https://github.com/bitcoin","followers_url":"https://api.github.com/users/bitcoin/followers","following_url":"https://api.github.com/users/bitcoin/following{/other_user}","gists_url":"https://api.github.com/users/bitcoin/gists{/gist_id}","starred_url":"https://api.github.com/users/bitcoin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bitcoin/subscriptions","organizations_url":"https://api.github.com/users/bitcoin/orgs","repos_url":"https://api.github.com/users/bitcoin/repos","events_url":"https://api.github.com/users/bitcoin/events{/privacy}","received_events_url":"https://api.github.com/users/bitcoin/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/bitcoin/bitcoin","description":"Bitcoin Core integration/staging tree","fork":false,"url":"https://api.github.com/repos/bitcoin/bitcoin","forks_url":"https://api.github.com/repos/bitcoin/bitcoin/forks","keys_url":"https://api.github.com/repos/bitcoin/bitcoin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/bitcoin/bitcoin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/bitcoin/bitcoin/teams","hooks_url":"https://api.github.com/repos/bitcoin/bitcoin/hooks","issue_events_url":"https://api.github.com/repos/bitcoin/bitcoin/issues/events{/number}","events_url":"https://api.github.com/repos/bitcoin/bitcoin/events","assignees_url":"https://api.github.com/repos/bitcoin/bitcoin/assignees{/user}","branches_url":"https://api.github.com/repos/bitcoin/bitcoin/branches{/branch}","tags_url":"https://api.github.com/repos/bitcoin/bitcoin/tags","blobs_url":"https://api.github.com/repos/bitcoin/bitcoin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/bitcoin/bitcoin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/bitcoin/bitcoin/git/refs{/sha}","trees_url":"https://api.github.com/repos/bitcoin/bitcoin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/bitcoin/bitcoin/statuses/{sha}","languages_url":"https://api.github.com/repos/bitcoin/bitcoin/languages","stargazers_url":"https://api.github.com/repos/bitcoin/bitcoin/stargazers","contributors_url":"https://api.github.com/repos/bitcoin/bitcoin/contributors","subscribers_url":"https://api.github.com/repos/bitcoin/bitcoin/subscribers","subscription_url":"https://api.github.com/repos/bitcoin/bitcoin/subscription","commits_url":"https://api.github.com/repos/bitcoin/bitcoin/commits{/sha}","git_commits_url":"https://api.github.com/repos/bitcoin/bitcoin/git/commits{/sha}","comments_url":"https://api.github.com/repos/bitcoin/bitcoin/comments{/number}","issue_comment_url":"https://api.github.com/repos/bitcoin/bitcoin/issues/comments{/number}","contents_url":"https://api.github.com/repos/bitcoin/bitcoin/contents/{+path}","compare_url":"https://api.github.com/repos/bitcoin/bitcoin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/bitcoin/bitcoin/merges","archive_url":"https://api.github.com/repos/bitcoin/bitcoin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/bitcoin/bitcoin/downloads","issues_url":"https://api.github.com/repos/bitcoin/bitcoin/issues{/number}","pulls_url":"https://api.github.com/repos/bitcoin/bitcoin/pulls{/number}","milestones_url":"https://api.github.com/repos/bitcoin/bitcoin/milestones{/number}","notifications_url":"https://api.github.com/repos/bitcoin/bitcoin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/bitcoin/bitcoin/labels{/name}","releases_url":"https://api.github.com/repos/bitcoin/bitcoin/releases{/id}","created_at":"2010-12-19T15:16:43Z","updated_at":"2015-12-18T19:03:51Z","pushed_at":"2015-12-18T19:09:40Z","git_url":"git://github.com/bitcoin/bitcoin.git","ssh_url":"git@github.com:bitcoin/bitcoin.git","clone_url":"https://github.com/bitcoin/bitcoin.git","svn_url":"https://github.com/bitcoin/bitcoin","homepage":"https://bitcoin.org/en/download","size":101198,"stargazers_count":8043,"watchers_count":8043,"language":"C++","has_issues":true,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":5493,"mirror_url":null,"open_issues_count":421,"forks":5493,"open_issues":421,"watchers":8043,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/7212"},"html":{"href":"https://github.com/bitcoin/bitcoin/pull/7212"},"issue":{"href":"https://api.github.com/repos/bitcoin/bitcoin/issues/7212"},"comments":{"href":"https://api.github.com/repos/bitcoin/bitcoin/issues/7212/comments"},"review_comments":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/7212/comments"},"review_comment":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/7212/commits"},"statuses":{"href":"https://api.github.com/repos/bitcoin/bitcoin/statuses/8e05d6e81f92dc47f77540242f2cf7d23d78fb6f"}}}}