{"action":"created","comment":{"url":"https://api.github.com/repos/petertodd/bitcoin/pulls/comments/43469418","id":43469418,"diff_hunk":"@@ -979,27 +979,47 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa\n         // than the ones it replaces.\n         CAmount nConflictingFees = 0;\n         size_t nConflictingSize = 0;\n+        CTxMemPool::setEntries allConflicting;\n         if (setConflicts.size())\n         {\n             LOCK(pool.cs);\n \n             // Check if it's economically rational to mine this transaction\n-            // rather than the ones it replaces. For efficiency we simply sum\n-            // up the pre-calculated fees/size-with-descendants values from the\n-            // mempool package tracking; this does mean the pathological case\n-            // of diamond tx graphs will be overcounted.\n-            BOOST_FOREACH(const uint256 hashConflicting, setConflicts)\n+            // rather than the ones it replaces.\n+            const int maxDescendantsToVisit = 100;\n+            BOOST_FOREACH(const uint256 &hashConflicting, setConflicts)\n             {\n                 CTxMemPool::txiter mi = pool.mapTx.find(hashConflicting);\n                 if (mi == pool.mapTx.end())\n                     continue;\n-                nConflictingFees += mi->GetFeesWithDescendants();\n-                nConflictingSize += mi->GetSizeWithDescendants();\n+                pool.CalculateDescendants(mi, allConflicting);","path":"src/main.cpp","position":24,"original_position":24,"commit_id":"3288aceb3daccc234ba2e723005361f86888eae2","original_commit_id":"3288aceb3daccc234ba2e723005361f86888eae2","user":{"login":"sdaftuar","id":7463573,"avatar_url":"https://avatars.githubusercontent.com/u/7463573?v=3","gravatar_id":"","url":"https://api.github.com/users/sdaftuar","html_url":"https://github.com/sdaftuar","followers_url":"https://api.github.com/users/sdaftuar/followers","following_url":"https://api.github.com/users/sdaftuar/following{/other_user}","gists_url":"https://api.github.com/users/sdaftuar/gists{/gist_id}","starred_url":"https://api.github.com/users/sdaftuar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sdaftuar/subscriptions","organizations_url":"https://api.github.com/users/sdaftuar/orgs","repos_url":"https://api.github.com/users/sdaftuar/repos","events_url":"https://api.github.com/users/sdaftuar/events{/privacy}","received_events_url":"https://api.github.com/users/sdaftuar/received_events","type":"User","site_admin":false},"body":"Only if the policy limits are violated... Which can happen but it's pretty hard (requires a reorg).\r\n\r\nBut true, we could tighten this up either by adding a work limit or just by looking at the descendant counts first (which will overstate the number of actual descendants when there's overlap).","created_at":"2015-10-30T03:41:58Z","updated_at":"2015-10-30T03:41:58Z","html_url":"https://github.com/petertodd/bitcoin/pull/4#discussion_r43469418","pull_request_url":"https://api.github.com/repos/petertodd/bitcoin/pulls/4","_links":{"self":{"href":"https://api.github.com/repos/petertodd/bitcoin/pulls/comments/43469418"},"html":{"href":"https://github.com/petertodd/bitcoin/pull/4#discussion_r43469418"},"pull_request":{"href":"https://api.github.com/repos/petertodd/bitcoin/pulls/4"}}},"pull_request":{"url":"https://api.github.com/repos/petertodd/bitcoin/pulls/4","id":49227593,"html_url":"https://github.com/petertodd/bitcoin/pull/4","diff_url":"https://github.com/petertodd/bitcoin/pull/4.diff","patch_url":"https://github.com/petertodd/bitcoin/pull/4.patch","issue_url":"https://api.github.com/repos/petertodd/bitcoin/issues/4","number":4,"state":"open","locked":false,"title":"RBF improvements","user":{"login":"sdaftuar","id":7463573,"avatar_url":"https://avatars.githubusercontent.com/u/7463573?v=3","gravatar_id":"","url":"https://api.github.com/users/sdaftuar","html_url":"https://github.com/sdaftuar","followers_url":"https://api.github.com/users/sdaftuar/followers","following_url":"https://api.github.com/users/sdaftuar/following{/other_user}","gists_url":"https://api.github.com/users/sdaftuar/gists{/gist_id}","starred_url":"https://api.github.com/users/sdaftuar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sdaftuar/subscriptions","organizations_url":"https://api.github.com/users/sdaftuar/orgs","repos_url":"https://api.github.com/users/sdaftuar/repos","events_url":"https://api.github.com/users/sdaftuar/events{/privacy}","received_events_url":"https://api.github.com/users/sdaftuar/received_events","type":"User","site_admin":false},"body":"I tried coding up what I had in mind to see how complicated it would be.  I don't think it's that bad, the diff looks bigger than it really is due to some code moves and comments/debugging.\r\n\r\nFix the calculation of conflicting size/conflicting fees, and introduce a new policy limit of the maximum number of conflicts allowed in an RBF calculation.  I just hardcoded a value for a quick and dirty change, we'd probably want to make this some new constant somewhere.\r\n\r\nAlso require that for each directly conflicting transaction T:\r\nfeerate(replacement) >  max(feerate(T), feerate(T with descendants))\r\n\r\nFinally also require that any new parents (that aren't spent by any of the directly conflicting transactions) are not in the mempool.\r\n\r\nOh I just hacked this up tonight, and the only testing I did was make sure it compiles and passes your rbf tests.  I'll try to test more tomorrow...\r\n\r\n","created_at":"2015-10-30T03:26:53Z","updated_at":"2015-10-30T03:41:58Z","closed_at":null,"merged_at":null,"merge_commit_sha":"d88d7578d1c753be745084f0218607f771b04588","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/petertodd/bitcoin/pulls/4/commits","review_comments_url":"https://api.github.com/repos/petertodd/bitcoin/pulls/4/comments","review_comment_url":"https://api.github.com/repos/petertodd/bitcoin/pulls/comments{/number}","comments_url":"https://api.github.com/repos/petertodd/bitcoin/issues/4/comments","statuses_url":"https://api.github.com/repos/petertodd/bitcoin/statuses/3288aceb3daccc234ba2e723005361f86888eae2","head":{"label":"sdaftuar:fixup-rbf","ref":"fixup-rbf","sha":"3288aceb3daccc234ba2e723005361f86888eae2","user":{"login":"sdaftuar","id":7463573,"avatar_url":"https://avatars.githubusercontent.com/u/7463573?v=3","gravatar_id":"","url":"https://api.github.com/users/sdaftuar","html_url":"https://github.com/sdaftuar","followers_url":"https://api.github.com/users/sdaftuar/followers","following_url":"https://api.github.com/users/sdaftuar/following{/other_user}","gists_url":"https://api.github.com/users/sdaftuar/gists{/gist_id}","starred_url":"https://api.github.com/users/sdaftuar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sdaftuar/subscriptions","organizations_url":"https://api.github.com/users/sdaftuar/orgs","repos_url":"https://api.github.com/users/sdaftuar/repos","events_url":"https://api.github.com/users/sdaftuar/events{/privacy}","received_events_url":"https://api.github.com/users/sdaftuar/received_events","type":"User","site_admin":false},"repo":{"id":28761781,"name":"bitcoin","full_name":"sdaftuar/bitcoin","owner":{"login":"sdaftuar","id":7463573,"avatar_url":"https://avatars.githubusercontent.com/u/7463573?v=3","gravatar_id":"","url":"https://api.github.com/users/sdaftuar","html_url":"https://github.com/sdaftuar","followers_url":"https://api.github.com/users/sdaftuar/followers","following_url":"https://api.github.com/users/sdaftuar/following{/other_user}","gists_url":"https://api.github.com/users/sdaftuar/gists{/gist_id}","starred_url":"https://api.github.com/users/sdaftuar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sdaftuar/subscriptions","organizations_url":"https://api.github.com/users/sdaftuar/orgs","repos_url":"https://api.github.com/users/sdaftuar/repos","events_url":"https://api.github.com/users/sdaftuar/events{/privacy}","received_events_url":"https://api.github.com/users/sdaftuar/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/sdaftuar/bitcoin","description":"Bitcoin Core integration/staging tree","fork":true,"url":"https://api.github.com/repos/sdaftuar/bitcoin","forks_url":"https://api.github.com/repos/sdaftuar/bitcoin/forks","keys_url":"https://api.github.com/repos/sdaftuar/bitcoin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/sdaftuar/bitcoin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/sdaftuar/bitcoin/teams","hooks_url":"https://api.github.com/repos/sdaftuar/bitcoin/hooks","issue_events_url":"https://api.github.com/repos/sdaftuar/bitcoin/issues/events{/number}","events_url":"https://api.github.com/repos/sdaftuar/bitcoin/events","assignees_url":"https://api.github.com/repos/sdaftuar/bitcoin/assignees{/user}","branches_url":"https://api.github.com/repos/sdaftuar/bitcoin/branches{/branch}","tags_url":"https://api.github.com/repos/sdaftuar/bitcoin/tags","blobs_url":"https://api.github.com/repos/sdaftuar/bitcoin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/sdaftuar/bitcoin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/sdaftuar/bitcoin/git/refs{/sha}","trees_url":"https://api.github.com/repos/sdaftuar/bitcoin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/sdaftuar/bitcoin/statuses/{sha}","languages_url":"https://api.github.com/repos/sdaftuar/bitcoin/languages","stargazers_url":"https://api.github.com/repos/sdaftuar/bitcoin/stargazers","contributors_url":"https://api.github.com/repos/sdaftuar/bitcoin/contributors","subscribers_url":"https://api.github.com/repos/sdaftuar/bitcoin/subscribers","subscription_url":"https://api.github.com/repos/sdaftuar/bitcoin/subscription","commits_url":"https://api.github.com/repos/sdaftuar/bitcoin/commits{/sha}","git_commits_url":"https://api.github.com/repos/sdaftuar/bitcoin/git/commits{/sha}","comments_url":"https://api.github.com/repos/sdaftuar/bitcoin/comments{/number}","issue_comment_url":"https://api.github.com/repos/sdaftuar/bitcoin/issues/comments{/number}","contents_url":"https://api.github.com/repos/sdaftuar/bitcoin/contents/{+path}","compare_url":"https://api.github.com/repos/sdaftuar/bitcoin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/sdaftuar/bitcoin/merges","archive_url":"https://api.github.com/repos/sdaftuar/bitcoin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/sdaftuar/bitcoin/downloads","issues_url":"https://api.github.com/repos/sdaftuar/bitcoin/issues{/number}","pulls_url":"https://api.github.com/repos/sdaftuar/bitcoin/pulls{/number}","milestones_url":"https://api.github.com/repos/sdaftuar/bitcoin/milestones{/number}","notifications_url":"https://api.github.com/repos/sdaftuar/bitcoin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/sdaftuar/bitcoin/labels{/name}","releases_url":"https://api.github.com/repos/sdaftuar/bitcoin/releases{/id}","created_at":"2015-01-04T02:52:13Z","updated_at":"2015-04-24T18:52:36Z","pushed_at":"2015-10-30T03:23:13Z","git_url":"git://github.com/sdaftuar/bitcoin.git","ssh_url":"git@github.com:sdaftuar/bitcoin.git","clone_url":"https://github.com/sdaftuar/bitcoin.git","svn_url":"https://github.com/sdaftuar/bitcoin","homepage":"https://bitcoin.org/en/download","size":57271,"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":"petertodd:2015-10-rbf-with-opt-out","ref":"2015-10-rbf-with-opt-out","sha":"03936dd335eab98608f1cefc21b3ec5e9a2c8d87","user":{"login":"petertodd","id":7042,"avatar_url":"https://avatars.githubusercontent.com/u/7042?v=3","gravatar_id":"","url":"https://api.github.com/users/petertodd","html_url":"https://github.com/petertodd","followers_url":"https://api.github.com/users/petertodd/followers","following_url":"https://api.github.com/users/petertodd/following{/other_user}","gists_url":"https://api.github.com/users/petertodd/gists{/gist_id}","starred_url":"https://api.github.com/users/petertodd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/petertodd/subscriptions","organizations_url":"https://api.github.com/users/petertodd/orgs","repos_url":"https://api.github.com/users/petertodd/repos","events_url":"https://api.github.com/users/petertodd/events{/privacy}","received_events_url":"https://api.github.com/users/petertodd/received_events","type":"User","site_admin":false},"repo":{"id":5992112,"name":"bitcoin","full_name":"petertodd/bitcoin","owner":{"login":"petertodd","id":7042,"avatar_url":"https://avatars.githubusercontent.com/u/7042?v=3","gravatar_id":"","url":"https://api.github.com/users/petertodd","html_url":"https://github.com/petertodd","followers_url":"https://api.github.com/users/petertodd/followers","following_url":"https://api.github.com/users/petertodd/following{/other_user}","gists_url":"https://api.github.com/users/petertodd/gists{/gist_id}","starred_url":"https://api.github.com/users/petertodd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/petertodd/subscriptions","organizations_url":"https://api.github.com/users/petertodd/orgs","repos_url":"https://api.github.com/users/petertodd/repos","events_url":"https://api.github.com/users/petertodd/events{/privacy}","received_events_url":"https://api.github.com/users/petertodd/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/petertodd/bitcoin","description":"Bitcoin integration/staging tree","fork":true,"url":"https://api.github.com/repos/petertodd/bitcoin","forks_url":"https://api.github.com/repos/petertodd/bitcoin/forks","keys_url":"https://api.github.com/repos/petertodd/bitcoin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/petertodd/bitcoin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/petertodd/bitcoin/teams","hooks_url":"https://api.github.com/repos/petertodd/bitcoin/hooks","issue_events_url":"https://api.github.com/repos/petertodd/bitcoin/issues/events{/number}","events_url":"https://api.github.com/repos/petertodd/bitcoin/events","assignees_url":"https://api.github.com/repos/petertodd/bitcoin/assignees{/user}","branches_url":"https://api.github.com/repos/petertodd/bitcoin/branches{/branch}","tags_url":"https://api.github.com/repos/petertodd/bitcoin/tags","blobs_url":"https://api.github.com/repos/petertodd/bitcoin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/petertodd/bitcoin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/petertodd/bitcoin/git/refs{/sha}","trees_url":"https://api.github.com/repos/petertodd/bitcoin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/petertodd/bitcoin/statuses/{sha}","languages_url":"https://api.github.com/repos/petertodd/bitcoin/languages","stargazers_url":"https://api.github.com/repos/petertodd/bitcoin/stargazers","contributors_url":"https://api.github.com/repos/petertodd/bitcoin/contributors","subscribers_url":"https://api.github.com/repos/petertodd/bitcoin/subscribers","subscription_url":"https://api.github.com/repos/petertodd/bitcoin/subscription","commits_url":"https://api.github.com/repos/petertodd/bitcoin/commits{/sha}","git_commits_url":"https://api.github.com/repos/petertodd/bitcoin/git/commits{/sha}","comments_url":"https://api.github.com/repos/petertodd/bitcoin/comments{/number}","issue_comment_url":"https://api.github.com/repos/petertodd/bitcoin/issues/comments{/number}","contents_url":"https://api.github.com/repos/petertodd/bitcoin/contents/{+path}","compare_url":"https://api.github.com/repos/petertodd/bitcoin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/petertodd/bitcoin/merges","archive_url":"https://api.github.com/repos/petertodd/bitcoin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/petertodd/bitcoin/downloads","issues_url":"https://api.github.com/repos/petertodd/bitcoin/issues{/number}","pulls_url":"https://api.github.com/repos/petertodd/bitcoin/pulls{/number}","milestones_url":"https://api.github.com/repos/petertodd/bitcoin/milestones{/number}","notifications_url":"https://api.github.com/repos/petertodd/bitcoin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/petertodd/bitcoin/labels{/name}","releases_url":"https://api.github.com/repos/petertodd/bitcoin/releases{/id}","created_at":"2012-09-28T03:31:40Z","updated_at":"2015-07-10T20:43:06Z","pushed_at":"2015-10-30T03:26:53Z","git_url":"git://github.com/petertodd/bitcoin.git","ssh_url":"git@github.com:petertodd/bitcoin.git","clone_url":"https://github.com/petertodd/bitcoin.git","svn_url":"https://github.com/petertodd/bitcoin","homepage":"http://www.bitcoin.org","size":61130,"stargazers_count":6,"watchers_count":6,"language":"TypeScript","has_issues":false,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":1,"forks":1,"open_issues":1,"watchers":6,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/petertodd/bitcoin/pulls/4"},"html":{"href":"https://github.com/petertodd/bitcoin/pull/4"},"issue":{"href":"https://api.github.com/repos/petertodd/bitcoin/issues/4"},"comments":{"href":"https://api.github.com/repos/petertodd/bitcoin/issues/4/comments"},"review_comments":{"href":"https://api.github.com/repos/petertodd/bitcoin/pulls/4/comments"},"review_comment":{"href":"https://api.github.com/repos/petertodd/bitcoin/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/petertodd/bitcoin/pulls/4/commits"},"statuses":{"href":"https://api.github.com/repos/petertodd/bitcoin/statuses/3288aceb3daccc234ba2e723005361f86888eae2"}}}}