{"action":"created","comment":{"url":"https://api.github.com/repos/mimblewimble/grin/pulls/comments/188649000","pull_request_review_id":120664635,"id":188649000,"diff_hunk":"@@ -0,0 +1,142 @@\n+// Copyright 2018 The Grin Developers\n+//\n+// Licensed under the Apache License, Version 2.0 (the \"License\");\n+// you may not use this file except in compliance with the License.\n+// You may obtain a copy of the License at\n+//\n+//     http://www.apache.org/licenses/LICENSE-2.0\n+//\n+// Unless required by applicable law or agreed to in writing, software\n+// distributed under the License is distributed on an \"AS IS\" BASIS,\n+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n+// See the License for the specific language governing permissions and\n+// limitations under the License.\n+\n+//! A minimal (EXPERIMENTAL) transaction pool implementation\n+\n+use std::collections::HashMap;\n+use std::sync::Arc;\n+use core::core::hash::{Hash, Hashed};\n+use core::core::id::ShortIdentifiable;\n+use core::core::transaction;\n+use core::core::{Block, Committed, CompactBlock, Transaction};\n+use keychain::BlindingFactor;\n+use pool::Pool;\n+use types::*;\n+use util::{secp_static, static_secp_instance};\n+use util::secp::pedersen::Commitment;\n+use util::LOGGER;\n+\n+/// A minimal (EXPERIMENTAL) transaction pool implementation\n+pub struct TransactionPool<T> {\n+\t/// Pool Config\n+\tpub config: PoolConfig,\n+\n+\t/// Our transaction pool.\n+\tpub pool: Pool<T>,\n+\t/// TODO - Our stem transaction pool (just another instance of a pool?)\n+\tpub stem_pool: Pool<T>,","path":"pool/src/transaction_pool.rs","position":38,"original_position":38,"commit_id":"c30c9ded0ee88b907333b79178698ac29988f08d","original_commit_id":"c30c9ded0ee88b907333b79178698ac29988f08d","user":{"login":"quentinlesceller","id":11842328,"avatar_url":"https://avatars1.githubusercontent.com/u/11842328?v=4","gravatar_id":"","url":"https://api.github.com/users/quentinlesceller","html_url":"https://github.com/quentinlesceller","followers_url":"https://api.github.com/users/quentinlesceller/followers","following_url":"https://api.github.com/users/quentinlesceller/following{/other_user}","gists_url":"https://api.github.com/users/quentinlesceller/gists{/gist_id}","starred_url":"https://api.github.com/users/quentinlesceller/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/quentinlesceller/subscriptions","organizations_url":"https://api.github.com/users/quentinlesceller/orgs","repos_url":"https://api.github.com/users/quentinlesceller/repos","events_url":"https://api.github.com/users/quentinlesceller/events{/privacy}","received_events_url":"https://api.github.com/users/quentinlesceller/received_events","type":"User","site_admin":false},"body":"We can use the same pool instance or simplify it by creating another instance ```Stempool``` (which implement the ```Pool```) which is basically the same but with ```time_stem_transactions``` (to track the transactions). \r\nAlso ```stempool``` and not ```stem_pool``` :) https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-May/015956.html.","created_at":"2018-05-16T14:35:18Z","updated_at":"2018-05-16T14:44:41Z","html_url":"https://github.com/mimblewimble/grin/pull/1067#discussion_r188649000","pull_request_url":"https://api.github.com/repos/mimblewimble/grin/pulls/1067","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/mimblewimble/grin/pulls/comments/188649000"},"html":{"href":"https://github.com/mimblewimble/grin/pull/1067#discussion_r188649000"},"pull_request":{"href":"https://api.github.com/repos/mimblewimble/grin/pulls/1067"}}},"pull_request":{"url":"https://api.github.com/repos/mimblewimble/grin/pulls/1067","id":188180633,"html_url":"https://github.com/mimblewimble/grin/pull/1067","diff_url":"https://github.com/mimblewimble/grin/pull/1067.diff","patch_url":"https://github.com/mimblewimble/grin/pull/1067.patch","issue_url":"https://api.github.com/repos/mimblewimble/grin/issues/1067","number":1067,"state":"open","locked":false,"title":"[DNM] Minimal Transaction Pool","user":{"login":"antiochp","id":30642645,"avatar_url":"https://avatars2.githubusercontent.com/u/30642645?v=4","gravatar_id":"","url":"https://api.github.com/users/antiochp","html_url":"https://github.com/antiochp","followers_url":"https://api.github.com/users/antiochp/followers","following_url":"https://api.github.com/users/antiochp/following{/other_user}","gists_url":"https://api.github.com/users/antiochp/gists{/gist_id}","starred_url":"https://api.github.com/users/antiochp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/antiochp/subscriptions","organizations_url":"https://api.github.com/users/antiochp/orgs","repos_url":"https://api.github.com/users/antiochp/repos","events_url":"https://api.github.com/users/antiochp/events{/privacy}","received_events_url":"https://api.github.com/users/antiochp/received_events","type":"User","site_admin":false},"body":"This is still very early - but wanted to push this up to get feedback before going any further on this.\r\n\r\nThe existing tx pool impl is pretty large (~2000 lines of code) and relatively complex.\r\nThis PR proposes an alternative solution - leveraging our `txhashset` to check if we can add a new tx to the pool (as an alternative to building and maintaining the \"directed acyclic graph\" of inputs/outputs in the existing impl).\r\n\r\n Basically - if aggregate new tx and the existing tx pool together we can check via a readonly txhashset extension is we can successfully add this single aggregated tx to the current chain state via the extension.\r\n\r\nThis covers all the usual checks -\r\n  * attempting to spend non-existent outputs\r\n  * attempting to \"double spend\" outputs\r\n  * coinbase maturity checks\r\n\r\n__But__ - we can leverage all our existing txhashset code to do all these checks, in the _exact same way_ we do when applying new blocks to the chain.\r\n\r\nPossible downsides - \r\n  * Is this slower? How much slower?\r\n  * Reconciling new blocks against the pool - we lose some of the benefits of the DAG here, when calculating which txs are still compatible with the more recent chain state and which txs must be evicted from the pool.\r\n\r\nNot yet implemented - \r\n  - [ ] this PR rips out (temporarily, work is progress) the stempool and Dandelion functionality - we need to reintroduce this (but I think we can follow the same pattern here, leveraging the txhashset extension validation to check for tx validity).\r\n\r\n@quentinlesceller keen to hear your thoughts on this, specifically around the stem_pool behavior.\r\nI think we can add txs to the stempool if we can aggregate the txs in the stempool with the txs in the regular pool and have this single aggregated tx successfully validate agains the txhashset.\r\n\r\n* If `tx + pool + txhashset` is valid then we can add `tx` to the regular pool.\r\n* If `tx + stem_pool + pool + txhashset` is valid then we can add `tx` to the stem_pool.\r\n\r\ni.e. at any given time we know that we can take _all_ (or a subset) txs from the pool, put them all in a block and have that block validate successfully against the chain state (the txhashset).\r\n\r\nThis standardizes validation even further in Grin - all the following behave in the same way and we can validate them all consistently - \r\n\r\n  * an individual tx\r\n  * a set of aggregated (and possibly cut-through) txs\r\n  * the tx pool\r\n  * the stem pool\r\n  * an individual block\r\n  * a set of blocks\r\n  * full chain state (txhashset)\r\n\r\n\r\nSee #1037 for some early thoughts around this.\r\n\r\n@MoaningMyrtle You wrote a lot of the initial pool implementation. \r\nAny strong objections to considering simplifying and consolidating the logic here?\r\n\r\n","created_at":"2018-05-15T16:09:22Z","updated_at":"2018-05-16T14:44:41Z","closed_at":null,"merged_at":null,"merge_commit_sha":"7e867988071a94a1139ec6ce88cc29ec81e40249","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/mimblewimble/grin/pulls/1067/commits","review_comments_url":"https://api.github.com/repos/mimblewimble/grin/pulls/1067/comments","review_comment_url":"https://api.github.com/repos/mimblewimble/grin/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mimblewimble/grin/issues/1067/comments","statuses_url":"https://api.github.com/repos/mimblewimble/grin/statuses/c30c9ded0ee88b907333b79178698ac29988f08d","head":{"label":"antiochp:minimal_tx_pool","ref":"minimal_tx_pool","sha":"c30c9ded0ee88b907333b79178698ac29988f08d","user":{"login":"antiochp","id":30642645,"avatar_url":"https://avatars2.githubusercontent.com/u/30642645?v=4","gravatar_id":"","url":"https://api.github.com/users/antiochp","html_url":"https://github.com/antiochp","followers_url":"https://api.github.com/users/antiochp/followers","following_url":"https://api.github.com/users/antiochp/following{/other_user}","gists_url":"https://api.github.com/users/antiochp/gists{/gist_id}","starred_url":"https://api.github.com/users/antiochp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/antiochp/subscriptions","organizations_url":"https://api.github.com/users/antiochp/orgs","repos_url":"https://api.github.com/users/antiochp/repos","events_url":"https://api.github.com/users/antiochp/events{/privacy}","received_events_url":"https://api.github.com/users/antiochp/received_events","type":"User","site_admin":false},"repo":{"id":99061244,"name":"grin","full_name":"antiochp/grin","owner":{"login":"antiochp","id":30642645,"avatar_url":"https://avatars2.githubusercontent.com/u/30642645?v=4","gravatar_id":"","url":"https://api.github.com/users/antiochp","html_url":"https://github.com/antiochp","followers_url":"https://api.github.com/users/antiochp/followers","following_url":"https://api.github.com/users/antiochp/following{/other_user}","gists_url":"https://api.github.com/users/antiochp/gists{/gist_id}","starred_url":"https://api.github.com/users/antiochp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/antiochp/subscriptions","organizations_url":"https://api.github.com/users/antiochp/orgs","repos_url":"https://api.github.com/users/antiochp/repos","events_url":"https://api.github.com/users/antiochp/events{/privacy}","received_events_url":"https://api.github.com/users/antiochp/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/antiochp/grin","description":"Minimal implementation of the MimbleWimble protocol.","fork":true,"url":"https://api.github.com/repos/antiochp/grin","forks_url":"https://api.github.com/repos/antiochp/grin/forks","keys_url":"https://api.github.com/repos/antiochp/grin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/antiochp/grin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/antiochp/grin/teams","hooks_url":"https://api.github.com/repos/antiochp/grin/hooks","issue_events_url":"https://api.github.com/repos/antiochp/grin/issues/events{/number}","events_url":"https://api.github.com/repos/antiochp/grin/events","assignees_url":"https://api.github.com/repos/antiochp/grin/assignees{/user}","branches_url":"https://api.github.com/repos/antiochp/grin/branches{/branch}","tags_url":"https://api.github.com/repos/antiochp/grin/tags","blobs_url":"https://api.github.com/repos/antiochp/grin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/antiochp/grin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/antiochp/grin/git/refs{/sha}","trees_url":"https://api.github.com/repos/antiochp/grin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/antiochp/grin/statuses/{sha}","languages_url":"https://api.github.com/repos/antiochp/grin/languages","stargazers_url":"https://api.github.com/repos/antiochp/grin/stargazers","contributors_url":"https://api.github.com/repos/antiochp/grin/contributors","subscribers_url":"https://api.github.com/repos/antiochp/grin/subscribers","subscription_url":"https://api.github.com/repos/antiochp/grin/subscription","commits_url":"https://api.github.com/repos/antiochp/grin/commits{/sha}","git_commits_url":"https://api.github.com/repos/antiochp/grin/git/commits{/sha}","comments_url":"https://api.github.com/repos/antiochp/grin/comments{/number}","issue_comment_url":"https://api.github.com/repos/antiochp/grin/issues/comments{/number}","contents_url":"https://api.github.com/repos/antiochp/grin/contents/{+path}","compare_url":"https://api.github.com/repos/antiochp/grin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/antiochp/grin/merges","archive_url":"https://api.github.com/repos/antiochp/grin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/antiochp/grin/downloads","issues_url":"https://api.github.com/repos/antiochp/grin/issues{/number}","pulls_url":"https://api.github.com/repos/antiochp/grin/pulls{/number}","milestones_url":"https://api.github.com/repos/antiochp/grin/milestones{/number}","notifications_url":"https://api.github.com/repos/antiochp/grin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/antiochp/grin/labels{/name}","releases_url":"https://api.github.com/repos/antiochp/grin/releases{/id}","deployments_url":"https://api.github.com/repos/antiochp/grin/deployments","created_at":"2017-08-02T02:04:29Z","updated_at":"2018-05-16T13:16:55Z","pushed_at":"2018-05-16T13:25:04Z","git_url":"git://github.com/antiochp/grin.git","ssh_url":"git@github.com:antiochp/grin.git","clone_url":"https://github.com/antiochp/grin.git","svn_url":"https://github.com/antiochp/grin","homepage":"","size":7101,"stargazers_count":0,"watchers_count":0,"language":"Rust","has_issues":false,"has_projects":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"mimblewimble:master","ref":"master","sha":"4bbaa8d05fcc7396867a08e4d5d1f44eab750968","user":{"login":"mimblewimble","id":23646132,"avatar_url":"https://avatars0.githubusercontent.com/u/23646132?v=4","gravatar_id":"","url":"https://api.github.com/users/mimblewimble","html_url":"https://github.com/mimblewimble","followers_url":"https://api.github.com/users/mimblewimble/followers","following_url":"https://api.github.com/users/mimblewimble/following{/other_user}","gists_url":"https://api.github.com/users/mimblewimble/gists{/gist_id}","starred_url":"https://api.github.com/users/mimblewimble/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mimblewimble/subscriptions","organizations_url":"https://api.github.com/users/mimblewimble/orgs","repos_url":"https://api.github.com/users/mimblewimble/repos","events_url":"https://api.github.com/users/mimblewimble/events{/privacy}","received_events_url":"https://api.github.com/users/mimblewimble/received_events","type":"Organization","site_admin":false},"repo":{"id":71514072,"name":"grin","full_name":"mimblewimble/grin","owner":{"login":"mimblewimble","id":23646132,"avatar_url":"https://avatars0.githubusercontent.com/u/23646132?v=4","gravatar_id":"","url":"https://api.github.com/users/mimblewimble","html_url":"https://github.com/mimblewimble","followers_url":"https://api.github.com/users/mimblewimble/followers","following_url":"https://api.github.com/users/mimblewimble/following{/other_user}","gists_url":"https://api.github.com/users/mimblewimble/gists{/gist_id}","starred_url":"https://api.github.com/users/mimblewimble/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mimblewimble/subscriptions","organizations_url":"https://api.github.com/users/mimblewimble/orgs","repos_url":"https://api.github.com/users/mimblewimble/repos","events_url":"https://api.github.com/users/mimblewimble/events{/privacy}","received_events_url":"https://api.github.com/users/mimblewimble/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mimblewimble/grin","description":"Minimal implementation of the MimbleWimble protocol.","fork":false,"url":"https://api.github.com/repos/mimblewimble/grin","forks_url":"https://api.github.com/repos/mimblewimble/grin/forks","keys_url":"https://api.github.com/repos/mimblewimble/grin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mimblewimble/grin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mimblewimble/grin/teams","hooks_url":"https://api.github.com/repos/mimblewimble/grin/hooks","issue_events_url":"https://api.github.com/repos/mimblewimble/grin/issues/events{/number}","events_url":"https://api.github.com/repos/mimblewimble/grin/events","assignees_url":"https://api.github.com/repos/mimblewimble/grin/assignees{/user}","branches_url":"https://api.github.com/repos/mimblewimble/grin/branches{/branch}","tags_url":"https://api.github.com/repos/mimblewimble/grin/tags","blobs_url":"https://api.github.com/repos/mimblewimble/grin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mimblewimble/grin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mimblewimble/grin/git/refs{/sha}","trees_url":"https://api.github.com/repos/mimblewimble/grin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mimblewimble/grin/statuses/{sha}","languages_url":"https://api.github.com/repos/mimblewimble/grin/languages","stargazers_url":"https://api.github.com/repos/mimblewimble/grin/stargazers","contributors_url":"https://api.github.com/repos/mimblewimble/grin/contributors","subscribers_url":"https://api.github.com/repos/mimblewimble/grin/subscribers","subscription_url":"https://api.github.com/repos/mimblewimble/grin/subscription","commits_url":"https://api.github.com/repos/mimblewimble/grin/commits{/sha}","git_commits_url":"https://api.github.com/repos/mimblewimble/grin/git/commits{/sha}","comments_url":"https://api.github.com/repos/mimblewimble/grin/comments{/number}","issue_comment_url":"https://api.github.com/repos/mimblewimble/grin/issues/comments{/number}","contents_url":"https://api.github.com/repos/mimblewimble/grin/contents/{+path}","compare_url":"https://api.github.com/repos/mimblewimble/grin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mimblewimble/grin/merges","archive_url":"https://api.github.com/repos/mimblewimble/grin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mimblewimble/grin/downloads","issues_url":"https://api.github.com/repos/mimblewimble/grin/issues{/number}","pulls_url":"https://api.github.com/repos/mimblewimble/grin/pulls{/number}","milestones_url":"https://api.github.com/repos/mimblewimble/grin/milestones{/number}","notifications_url":"https://api.github.com/repos/mimblewimble/grin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mimblewimble/grin/labels{/name}","releases_url":"https://api.github.com/repos/mimblewimble/grin/releases{/id}","deployments_url":"https://api.github.com/repos/mimblewimble/grin/deployments","created_at":"2016-10-21T00:02:23Z","updated_at":"2018-05-16T12:18:12Z","pushed_at":"2018-05-16T13:25:06Z","git_url":"git://github.com/mimblewimble/grin.git","ssh_url":"git@github.com:mimblewimble/grin.git","clone_url":"https://github.com/mimblewimble/grin.git","svn_url":"https://github.com/mimblewimble/grin","homepage":"http://grin-tech.org/","size":7348,"stargazers_count":1125,"watchers_count":1125,"language":"Rust","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":201,"mirror_url":null,"archived":false,"open_issues_count":100,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":201,"open_issues":100,"watchers":1125,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mimblewimble/grin/pulls/1067"},"html":{"href":"https://github.com/mimblewimble/grin/pull/1067"},"issue":{"href":"https://api.github.com/repos/mimblewimble/grin/issues/1067"},"comments":{"href":"https://api.github.com/repos/mimblewimble/grin/issues/1067/comments"},"review_comments":{"href":"https://api.github.com/repos/mimblewimble/grin/pulls/1067/comments"},"review_comment":{"href":"https://api.github.com/repos/mimblewimble/grin/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mimblewimble/grin/pulls/1067/commits"},"statuses":{"href":"https://api.github.com/repos/mimblewimble/grin/statuses/c30c9ded0ee88b907333b79178698ac29988f08d"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"quentinlesceller"}}