{"action":"created","comment":{"url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/comments/183505818","pull_request_review_id":114508551,"id":183505818,"diff_hunk":"@@ -0,0 +1,106 @@\n+// Copyright (c) 2018 The Bitcoin Core developers\n+// Distributed under the MIT software license, see the accompanying\n+// file COPYING or http://www.opensource.org/licenses/mit-license.php.\n+\n+#ifndef BITCOIN_VALIDATION_LAYER_H\n+#define BITCOIN_VALIDATION_LAYER_H\n+\n+#include <future>\n+\n+#include <chainparams.h>\n+#include <core/consumerthread.h>\n+#include <core/producerconsumerqueue.h>\n+#include <util.h>\n+#include <validation.h>\n+\n+class ValidationLayer;\n+\n+template<typename RESPONSE>\n+class ValidationRequest : public WorkItem<WorkerMode::BLOCKING> {\n+    friend class ValidationLayer;\n+protected:\n+    std::promise<RESPONSE> m_promise;\n+};\n+\n+struct BlockValidationResponse {\n+    const bool is_new;\n+    const bool not_immediately_invalid;\n+\n+    BlockValidationResponse(bool _not_immediately_invalid, bool _is_new)\n+      :is_new(_is_new), not_immediately_invalid(_not_immediately_invalid) {};\n+};\n+\n+class BlockValidationRequest : public ValidationRequest<BlockValidationResponse> {\n+    friend ValidationLayer;\n+\n+private:\n+    BlockValidationRequest(ValidationLayer& validation_layer, const std::shared_ptr<const CBlock> block, bool force_processing, const std::function<void()> on_ready)\n+    :m_validation_layer(validation_layer), m_block(block), m_force_processing(force_processing), m_on_ready(on_ready) {};\n+\n+    void operator()();\n+\n+    const ValidationLayer& m_validation_layer;\n+    const std::shared_ptr<const CBlock> m_block;\n+    const bool m_force_processing;\n+    const std::function<void()> m_on_ready;\n+};\n+\n+/**\n+ * Public interface to block and header validation\n+ *\n+ * Two apis:\n+ *  - asynchronous: Submitforvalidation(object) -> future<Response>\n+ *  - synchronous: Validate(object) -> Response\n+ * object may be a block or block header\n+ */\n+class ValidationLayer {\n+    typedef WorkQueue<WorkerMode::BLOCKING> ValidationQueue;\n+    typedef ConsumerThread<WorkerMode::BLOCKING> ValidationThread;\n+\n+public:\n+    ValidationLayer(const CChainParams& chainparams)\n+        :m_chainparams(chainparams), m_validation_queue(std::shared_ptr<ValidationQueue>(new ValidationQueue(100))) {}","path":"src/validation_layer.h","position":62,"original_position":62,"commit_id":"d30aba7de9288ecf4573d1a04a17b351695043e0","original_commit_id":"c10466c7a25a3b38c0e81337f56ab5fa59ac4bef","user":{"login":"ryanofsky","id":7133040,"avatar_url":"https://avatars2.githubusercontent.com/u/7133040?v=4","gravatar_id":"","url":"https://api.github.com/users/ryanofsky","html_url":"https://github.com/ryanofsky","followers_url":"https://api.github.com/users/ryanofsky/followers","following_url":"https://api.github.com/users/ryanofsky/following{/other_user}","gists_url":"https://api.github.com/users/ryanofsky/gists{/gist_id}","starred_url":"https://api.github.com/users/ryanofsky/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryanofsky/subscriptions","organizations_url":"https://api.github.com/users/ryanofsky/orgs","repos_url":"https://api.github.com/users/ryanofsky/repos","events_url":"https://api.github.com/users/ryanofsky/events{/privacy}","received_events_url":"https://api.github.com/users/ryanofsky/received_events","type":"User","site_admin":false},"body":"Could use make_shared here, MakeUnique below","created_at":"2018-04-23T19:09:03Z","updated_at":"2018-04-23T19:25:19Z","html_url":"https://github.com/bitcoin/bitcoin/pull/12934#discussion_r183505818","pull_request_url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/12934","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/comments/183505818"},"html":{"href":"https://github.com/bitcoin/bitcoin/pull/12934#discussion_r183505818"},"pull_request":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/12934"}}},"pull_request":{"url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/12934","id":180738473,"html_url":"https://github.com/bitcoin/bitcoin/pull/12934","diff_url":"https://github.com/bitcoin/bitcoin/pull/12934.diff","patch_url":"https://github.com/bitcoin/bitcoin/pull/12934.patch","issue_url":"https://api.github.com/repos/bitcoin/bitcoin/issues/12934","number":12934,"state":"open","locked":false,"title":"[WIP] [net] [validation] Call ProcessNewBlock() asynchronously","user":{"login":"skeees","id":195769,"avatar_url":"https://avatars0.githubusercontent.com/u/195769?v=4","gravatar_id":"","url":"https://api.github.com/users/skeees","html_url":"https://github.com/skeees","followers_url":"https://api.github.com/users/skeees/followers","following_url":"https://api.github.com/users/skeees/following{/other_user}","gists_url":"https://api.github.com/users/skeees/gists{/gist_id}","starred_url":"https://api.github.com/users/skeees/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/skeees/subscriptions","organizations_url":"https://api.github.com/users/skeees/orgs","repos_url":"https://api.github.com/users/skeees/repos","events_url":"https://api.github.com/users/skeees/events{/privacy}","received_events_url":"https://api.github.com/users/skeees/received_events","type":"User","site_admin":false},"body":"This is still in progress and not fully completed yet, but wanted to put it out for review in terms of overall design/architecture\r\n\r\nThe high level goal here (in this, and if accepted, subsequent PRs) is to allow for net and validation to live on separate threads and communicate mostly via message passing - both for the efficiency benefits that further parallelism in the net layer might provide, but also perhaps moreso as a step towards the goal of reducing the amount of shared state and forcing a cleaner separation between the net and validation layers in the core node. \r\n\r\nTo keep this PR as self contained as possible - this set of commits does the following:\r\n  - defines `ProducerConsumerQueue() / ConsumerThread()`: infrastructure to facilitate async communication between the net and validation layers\r\n  - defines ValidationLayer(): an interface where requests for (just `CBlock` for now) validation can be submitted and processed asynchronously\r\n  - replaces synchronous calls of `ProcessNewBlock()` in net_processing with the new async interface `ValidationLayer::SubmitForValidation(CBlock) -> std::future<BlockValidationResult>`\r\n\r\nBecause the P2P layer assumes that for a given node every message is fully processed before any subsequent messages are processed, when an asynchronous validation request is submitted for a block coming from a node - that node is \"frozen\" until that request has been fully validated. In the meantime - the net layer may continue servicing other nodes that do not have pending asynchronous validation requests.\r\n\r\nThe ProducerConsumerQueue() was left sufficiently generic so that it may be interposed  in other places where separation of components via asynchronous message passing might make sense from a design perspective.","created_at":"2018-04-10T21:54:36Z","updated_at":"2018-04-23T19:25:19Z","closed_at":null,"merged_at":null,"merge_commit_sha":"4e735054c380c10e3344458c3cd4d68b4a7f2743","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/12934/commits","review_comments_url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/12934/comments","review_comment_url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/comments{/number}","comments_url":"https://api.github.com/repos/bitcoin/bitcoin/issues/12934/comments","statuses_url":"https://api.github.com/repos/bitcoin/bitcoin/statuses/d30aba7de9288ecf4573d1a04a17b351695043e0","head":{"label":"skeees:module-isolation","ref":"module-isolation","sha":"d30aba7de9288ecf4573d1a04a17b351695043e0","user":{"login":"skeees","id":195769,"avatar_url":"https://avatars0.githubusercontent.com/u/195769?v=4","gravatar_id":"","url":"https://api.github.com/users/skeees","html_url":"https://github.com/skeees","followers_url":"https://api.github.com/users/skeees/followers","following_url":"https://api.github.com/users/skeees/following{/other_user}","gists_url":"https://api.github.com/users/skeees/gists{/gist_id}","starred_url":"https://api.github.com/users/skeees/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/skeees/subscriptions","organizations_url":"https://api.github.com/users/skeees/orgs","repos_url":"https://api.github.com/users/skeees/repos","events_url":"https://api.github.com/users/skeees/events{/privacy}","received_events_url":"https://api.github.com/users/skeees/received_events","type":"User","site_admin":false},"repo":{"id":118956449,"name":"bitcoin","full_name":"skeees/bitcoin","owner":{"login":"skeees","id":195769,"avatar_url":"https://avatars0.githubusercontent.com/u/195769?v=4","gravatar_id":"","url":"https://api.github.com/users/skeees","html_url":"https://github.com/skeees","followers_url":"https://api.github.com/users/skeees/followers","following_url":"https://api.github.com/users/skeees/following{/other_user}","gists_url":"https://api.github.com/users/skeees/gists{/gist_id}","starred_url":"https://api.github.com/users/skeees/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/skeees/subscriptions","organizations_url":"https://api.github.com/users/skeees/orgs","repos_url":"https://api.github.com/users/skeees/repos","events_url":"https://api.github.com/users/skeees/events{/privacy}","received_events_url":"https://api.github.com/users/skeees/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/skeees/bitcoin","description":"Bitcoin Core integration/staging tree","fork":true,"url":"https://api.github.com/repos/skeees/bitcoin","forks_url":"https://api.github.com/repos/skeees/bitcoin/forks","keys_url":"https://api.github.com/repos/skeees/bitcoin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/skeees/bitcoin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/skeees/bitcoin/teams","hooks_url":"https://api.github.com/repos/skeees/bitcoin/hooks","issue_events_url":"https://api.github.com/repos/skeees/bitcoin/issues/events{/number}","events_url":"https://api.github.com/repos/skeees/bitcoin/events","assignees_url":"https://api.github.com/repos/skeees/bitcoin/assignees{/user}","branches_url":"https://api.github.com/repos/skeees/bitcoin/branches{/branch}","tags_url":"https://api.github.com/repos/skeees/bitcoin/tags","blobs_url":"https://api.github.com/repos/skeees/bitcoin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/skeees/bitcoin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/skeees/bitcoin/git/refs{/sha}","trees_url":"https://api.github.com/repos/skeees/bitcoin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/skeees/bitcoin/statuses/{sha}","languages_url":"https://api.github.com/repos/skeees/bitcoin/languages","stargazers_url":"https://api.github.com/repos/skeees/bitcoin/stargazers","contributors_url":"https://api.github.com/repos/skeees/bitcoin/contributors","subscribers_url":"https://api.github.com/repos/skeees/bitcoin/subscribers","subscription_url":"https://api.github.com/repos/skeees/bitcoin/subscription","commits_url":"https://api.github.com/repos/skeees/bitcoin/commits{/sha}","git_commits_url":"https://api.github.com/repos/skeees/bitcoin/git/commits{/sha}","comments_url":"https://api.github.com/repos/skeees/bitcoin/comments{/number}","issue_comment_url":"https://api.github.com/repos/skeees/bitcoin/issues/comments{/number}","contents_url":"https://api.github.com/repos/skeees/bitcoin/contents/{+path}","compare_url":"https://api.github.com/repos/skeees/bitcoin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/skeees/bitcoin/merges","archive_url":"https://api.github.com/repos/skeees/bitcoin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/skeees/bitcoin/downloads","issues_url":"https://api.github.com/repos/skeees/bitcoin/issues{/number}","pulls_url":"https://api.github.com/repos/skeees/bitcoin/pulls{/number}","milestones_url":"https://api.github.com/repos/skeees/bitcoin/milestones{/number}","notifications_url":"https://api.github.com/repos/skeees/bitcoin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/skeees/bitcoin/labels{/name}","releases_url":"https://api.github.com/repos/skeees/bitcoin/releases{/id}","deployments_url":"https://api.github.com/repos/skeees/bitcoin/deployments","created_at":"2018-01-25T19:15:49Z","updated_at":"2018-04-20T15:20:54Z","pushed_at":"2018-04-21T13:41:52Z","git_url":"git://github.com/skeees/bitcoin.git","ssh_url":"git@github.com:skeees/bitcoin.git","clone_url":"https://github.com/skeees/bitcoin.git","svn_url":"https://github.com/skeees/bitcoin","homepage":"https://bitcoin.org/en/download","size":104560,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":null,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"bitcoin:master","ref":"master","sha":"0a8054e7cd5c76d01e4ac7234e3883d05f6f5fdd","user":{"login":"bitcoin","id":528860,"avatar_url":"https://avatars2.githubusercontent.com/u/528860?v=4","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://avatars2.githubusercontent.com/u/528860?v=4","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}","deployments_url":"https://api.github.com/repos/bitcoin/bitcoin/deployments","created_at":"2010-12-19T15:16:43Z","updated_at":"2018-04-23T19:21:40Z","pushed_at":"2018-04-23T19:21:12Z","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":104971,"stargazers_count":30855,"watchers_count":30855,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":18529,"mirror_url":null,"archived":false,"open_issues_count":771,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit"},"forks":18529,"open_issues":771,"watchers":30855,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/12934"},"html":{"href":"https://github.com/bitcoin/bitcoin/pull/12934"},"issue":{"href":"https://api.github.com/repos/bitcoin/bitcoin/issues/12934"},"comments":{"href":"https://api.github.com/repos/bitcoin/bitcoin/issues/12934/comments"},"review_comments":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/12934/comments"},"review_comment":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/bitcoin/bitcoin/pulls/12934/commits"},"statuses":{"href":"https://api.github.com/repos/bitcoin/bitcoin/statuses/d30aba7de9288ecf4573d1a04a17b351695043e0"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"ryanofsky"}}