{"action":"created","comment":{"url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/comments/141537878","pull_request_review_id":65765072,"id":141537878,"diff_hunk":"@@ -312,6 +312,17 @@ public override void ContextualCheckBlockHeader(ContextInformation context)\n                 ConsensusErrors.BlockTimestampTooEarly.Throw();\n             }\n \n+            int lastCheckpointHeight = this.checkpoints.GetLastCheckpointHeight();","path":"Stratis.Bitcoin.Features.Consensus/PosConsensusValidator.cs","position":4,"original_position":4,"commit_id":"e2d8cc991f507ea3c07f5db633f517e2ade64b2b","original_commit_id":"e2d8cc991f507ea3c07f5db633f517e2ade64b2b","user":{"login":"Aprogiena","id":30052221,"avatar_url":"https://avatars0.githubusercontent.com/u/30052221?v=4","gravatar_id":"","url":"https://api.github.com/users/Aprogiena","html_url":"https://github.com/Aprogiena","followers_url":"https://api.github.com/users/Aprogiena/followers","following_url":"https://api.github.com/users/Aprogiena/following{/other_user}","gists_url":"https://api.github.com/users/Aprogiena/gists{/gist_id}","starred_url":"https://api.github.com/users/Aprogiena/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Aprogiena/subscriptions","organizations_url":"https://api.github.com/users/Aprogiena/orgs","repos_url":"https://api.github.com/users/Aprogiena/repos","events_url":"https://api.github.com/users/Aprogiena/events{/privacy}","received_events_url":"https://api.github.com/users/Aprogiena/received_events","type":"User","site_admin":false},"body":"I've already created an issue for such a test - https://github.com/stratisproject/StratisBitcoinFullNode/issues/493 - and frankly, once this test exists, I should be able to find a place where to but the check. I was expecting what you just confirmed, so I need to rewrite this. If you could help with the test, that would help a lot.\r\n\r\nI'm not sure that this is attacker only scenario though. A poorly connected big staker (say 40 % of the network) will have some chance to achieve this by mistake. Well it is a pretty big mistake, but I would argue that it is possible without any malicious code being present on that particular node. I would the normal consensus temporary ban as it is usual is the right reaction. But since we don't implement these bans yet, this is to be decided later anyway.\r\n\r\nOK, so to progress here - if you could help with the test, this will really help me in getting that extra bit of knowledge that is needed to find the spot. And certainly I would then start with those places you've mentioned.","created_at":"2017-09-28T06:49:36Z","updated_at":"2017-09-28T06:49:36Z","html_url":"https://github.com/stratisproject/StratisBitcoinFullNode/pull/492#discussion_r141537878","pull_request_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/492","author_association":"COLLABORATOR","_links":{"self":{"href":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/comments/141537878"},"html":{"href":"https://github.com/stratisproject/StratisBitcoinFullNode/pull/492#discussion_r141537878"},"pull_request":{"href":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/492"}},"in_reply_to_id":141489458},"pull_request":{"url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/492","id":142761195,"html_url":"https://github.com/stratisproject/StratisBitcoinFullNode/pull/492","diff_url":"https://github.com/stratisproject/StratisBitcoinFullNode/pull/492.diff","patch_url":"https://github.com/stratisproject/StratisBitcoinFullNode/pull/492.patch","issue_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/issues/492","number":492,"state":"open","locked":false,"title":"Feature/checksync issue 413","user":{"login":"Aprogiena","id":30052221,"avatar_url":"https://avatars0.githubusercontent.com/u/30052221?v=4","gravatar_id":"","url":"https://api.github.com/users/Aprogiena","html_url":"https://github.com/Aprogiena","followers_url":"https://api.github.com/users/Aprogiena/followers","following_url":"https://api.github.com/users/Aprogiena/following{/other_user}","gists_url":"https://api.github.com/users/Aprogiena/gists{/gist_id}","starred_url":"https://api.github.com/users/Aprogiena/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Aprogiena/subscriptions","organizations_url":"https://api.github.com/users/Aprogiena/orgs","repos_url":"https://api.github.com/users/Aprogiena/repos","events_url":"https://api.github.com/users/Aprogiena/events{/privacy}","received_events_url":"https://api.github.com/users/Aprogiena/received_events","type":"User","site_admin":false},"body":"This is an **ATTEMPT** to implement what StratisX knows as \"synchronized checkpoints\". It is a mechanism that should prevent reorgs longer than 500 blocks. \r\n\r\nHowever, I'm completely unsure that I've got it right - esp. the check is in the right place and checking the right thing. I believe we construct our chain differently than StratisX, so that is why I'm unsure.\r\n\r\nI assume the ConcurrentChain's Tip is just the headers we've seen on the network, but not necessarily a valid chain. And I consider ConsensusLoop's Tip as the block that is at the end of our fully validated longest chain. \r\n\r\nSo, in `ContextualCheckBlockHeader` of PosConsensusValidator, I implement the mechanism ported from StratisX in their `Checkpoints::CheckSync`. I've used different naming as I don't really consider this as a checkpoint thing. \r\n\r\nAnyway, what I'm unsure if such a block that would not pass the check would enter our `AcceptBlock` method in ConsensusLoop. If so, then it should be alright.\r\n\r\nI'm bit afraid of the rewinding inside ConsensusLoop's `ExecuteNextBlock`. I don't really understands when it does take place.\r\n\r\n**So I would like the reviewer to check whether the placement of this code is actually correct and does what is intended - i.e. that if we are fully synced and then suddenly we connect to a new peer who claims a longer chain, for which we would need to reorganize more than 500 blocks then it would be rejected by this.** \r\n\r\nFix #413 ","created_at":"2017-09-24T16:44:49Z","updated_at":"2017-09-28T06:49:36Z","closed_at":null,"merged_at":null,"merge_commit_sha":"45c7ce7e8c6158b9027a19ebcdbd163c7c2666a1","assignee":null,"assignees":[],"requested_reviewers":[],"milestone":null,"commits_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/492/commits","review_comments_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/492/comments","review_comment_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/comments{/number}","comments_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/issues/492/comments","statuses_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/statuses/e2d8cc991f507ea3c07f5db633f517e2ade64b2b","head":{"label":"Aprogiena:feature/checksync-issue-413","ref":"feature/checksync-issue-413","sha":"e2d8cc991f507ea3c07f5db633f517e2ade64b2b","user":{"login":"Aprogiena","id":30052221,"avatar_url":"https://avatars0.githubusercontent.com/u/30052221?v=4","gravatar_id":"","url":"https://api.github.com/users/Aprogiena","html_url":"https://github.com/Aprogiena","followers_url":"https://api.github.com/users/Aprogiena/followers","following_url":"https://api.github.com/users/Aprogiena/following{/other_user}","gists_url":"https://api.github.com/users/Aprogiena/gists{/gist_id}","starred_url":"https://api.github.com/users/Aprogiena/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Aprogiena/subscriptions","organizations_url":"https://api.github.com/users/Aprogiena/orgs","repos_url":"https://api.github.com/users/Aprogiena/repos","events_url":"https://api.github.com/users/Aprogiena/events{/privacy}","received_events_url":"https://api.github.com/users/Aprogiena/received_events","type":"User","site_admin":false},"repo":{"id":96870892,"name":"StratisBitcoinFullNode","full_name":"Aprogiena/StratisBitcoinFullNode","owner":{"login":"Aprogiena","id":30052221,"avatar_url":"https://avatars0.githubusercontent.com/u/30052221?v=4","gravatar_id":"","url":"https://api.github.com/users/Aprogiena","html_url":"https://github.com/Aprogiena","followers_url":"https://api.github.com/users/Aprogiena/followers","following_url":"https://api.github.com/users/Aprogiena/following{/other_user}","gists_url":"https://api.github.com/users/Aprogiena/gists{/gist_id}","starred_url":"https://api.github.com/users/Aprogiena/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Aprogiena/subscriptions","organizations_url":"https://api.github.com/users/Aprogiena/orgs","repos_url":"https://api.github.com/users/Aprogiena/repos","events_url":"https://api.github.com/users/Aprogiena/events{/privacy}","received_events_url":"https://api.github.com/users/Aprogiena/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Aprogiena/StratisBitcoinFullNode","description":"Bitcoin full node in C#","fork":true,"url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode","forks_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/forks","keys_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/teams","hooks_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/hooks","issue_events_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/issues/events{/number}","events_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/events","assignees_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/assignees{/user}","branches_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/branches{/branch}","tags_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/tags","blobs_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/git/refs{/sha}","trees_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/statuses/{sha}","languages_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/languages","stargazers_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/stargazers","contributors_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/contributors","subscribers_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/subscribers","subscription_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/subscription","commits_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/commits{/sha}","git_commits_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/git/commits{/sha}","comments_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/comments{/number}","issue_comment_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/issues/comments{/number}","contents_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/contents/{+path}","compare_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/merges","archive_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/downloads","issues_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/issues{/number}","pulls_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/pulls{/number}","milestones_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/milestones{/number}","notifications_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/labels{/name}","releases_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/releases{/id}","deployments_url":"https://api.github.com/repos/Aprogiena/StratisBitcoinFullNode/deployments","created_at":"2017-07-11T08:37:52Z","updated_at":"2017-09-21T09:22:13Z","pushed_at":"2017-09-28T06:29:13Z","git_url":"git://github.com/Aprogiena/StratisBitcoinFullNode.git","ssh_url":"git@github.com:Aprogiena/StratisBitcoinFullNode.git","clone_url":"https://github.com/Aprogiena/StratisBitcoinFullNode.git","svn_url":"https://github.com/Aprogiena/StratisBitcoinFullNode","homepage":"https://stratisplatform.com","size":7866,"stargazers_count":1,"watchers_count":1,"language":"C#","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"stratisproject:master","ref":"master","sha":"b6ebb3af41585cbc3f7ae463f368c4b3597d3a78","user":{"login":"stratisproject","id":15229748,"avatar_url":"https://avatars0.githubusercontent.com/u/15229748?v=4","gravatar_id":"","url":"https://api.github.com/users/stratisproject","html_url":"https://github.com/stratisproject","followers_url":"https://api.github.com/users/stratisproject/followers","following_url":"https://api.github.com/users/stratisproject/following{/other_user}","gists_url":"https://api.github.com/users/stratisproject/gists{/gist_id}","starred_url":"https://api.github.com/users/stratisproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stratisproject/subscriptions","organizations_url":"https://api.github.com/users/stratisproject/orgs","repos_url":"https://api.github.com/users/stratisproject/repos","events_url":"https://api.github.com/users/stratisproject/events{/privacy}","received_events_url":"https://api.github.com/users/stratisproject/received_events","type":"Organization","site_admin":false},"repo":{"id":67271290,"name":"StratisBitcoinFullNode","full_name":"stratisproject/StratisBitcoinFullNode","owner":{"login":"stratisproject","id":15229748,"avatar_url":"https://avatars0.githubusercontent.com/u/15229748?v=4","gravatar_id":"","url":"https://api.github.com/users/stratisproject","html_url":"https://github.com/stratisproject","followers_url":"https://api.github.com/users/stratisproject/followers","following_url":"https://api.github.com/users/stratisproject/following{/other_user}","gists_url":"https://api.github.com/users/stratisproject/gists{/gist_id}","starred_url":"https://api.github.com/users/stratisproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stratisproject/subscriptions","organizations_url":"https://api.github.com/users/stratisproject/orgs","repos_url":"https://api.github.com/users/stratisproject/repos","events_url":"https://api.github.com/users/stratisproject/events{/privacy}","received_events_url":"https://api.github.com/users/stratisproject/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/stratisproject/StratisBitcoinFullNode","description":"Bitcoin full node in C#","fork":false,"url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode","forks_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/forks","keys_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/keys{/key_id}","collaborators_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/teams","hooks_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/hooks","issue_events_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/issues/events{/number}","events_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/events","assignees_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/assignees{/user}","branches_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/branches{/branch}","tags_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/tags","blobs_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/git/refs{/sha}","trees_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/git/trees{/sha}","statuses_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/statuses/{sha}","languages_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/languages","stargazers_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/stargazers","contributors_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/contributors","subscribers_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/subscribers","subscription_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/subscription","commits_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/commits{/sha}","git_commits_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/git/commits{/sha}","comments_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/comments{/number}","issue_comment_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/issues/comments{/number}","contents_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/contents/{+path}","compare_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/compare/{base}...{head}","merges_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/merges","archive_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/downloads","issues_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/issues{/number}","pulls_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls{/number}","milestones_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/milestones{/number}","notifications_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/labels{/name}","releases_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/releases{/id}","deployments_url":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/deployments","created_at":"2016-09-03T04:32:51Z","updated_at":"2017-09-20T22:01:09Z","pushed_at":"2017-09-28T06:29:15Z","git_url":"git://github.com/stratisproject/StratisBitcoinFullNode.git","ssh_url":"git@github.com:stratisproject/StratisBitcoinFullNode.git","clone_url":"https://github.com/stratisproject/StratisBitcoinFullNode.git","svn_url":"https://github.com/stratisproject/StratisBitcoinFullNode","homepage":"https://stratisplatform.com","size":7673,"stargazers_count":177,"watchers_count":177,"language":"C#","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":72,"mirror_url":null,"open_issues_count":58,"forks":72,"open_issues":58,"watchers":177,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/492"},"html":{"href":"https://github.com/stratisproject/StratisBitcoinFullNode/pull/492"},"issue":{"href":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/issues/492"},"comments":{"href":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/issues/492/comments"},"review_comments":{"href":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/492/comments"},"review_comment":{"href":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/pulls/492/commits"},"statuses":{"href":"https://api.github.com/repos/stratisproject/StratisBitcoinFullNode/statuses/e2d8cc991f507ea3c07f5db633f517e2ade64b2b"}},"author_association":"COLLABORATOR"}}
{"actor":{"display_login":"Aprogiena"}}