{"action":"created","issue":{"url":"https://api.github.com/repos/bitcoin/bitcoin/issues/8068","repository_url":"https://api.github.com/repos/bitcoin/bitcoin","labels_url":"https://api.github.com/repos/bitcoin/bitcoin/issues/8068/labels{/name}","comments_url":"https://api.github.com/repos/bitcoin/bitcoin/issues/8068/comments","events_url":"https://api.github.com/repos/bitcoin/bitcoin/issues/8068/events","html_url":"https://github.com/bitcoin/bitcoin/pull/8068","id":155400258,"number":8068,"title":"Compact Blocks","user":{"login":"TheBlueMatt","id":649246,"avatar_url":"https://avatars.githubusercontent.com/u/649246?v=3","gravatar_id":"","url":"https://api.github.com/users/TheBlueMatt","html_url":"https://github.com/TheBlueMatt","followers_url":"https://api.github.com/users/TheBlueMatt/followers","following_url":"https://api.github.com/users/TheBlueMatt/following{/other_user}","gists_url":"https://api.github.com/users/TheBlueMatt/gists{/gist_id}","starred_url":"https://api.github.com/users/TheBlueMatt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TheBlueMatt/subscriptions","organizations_url":"https://api.github.com/users/TheBlueMatt/orgs","repos_url":"https://api.github.com/users/TheBlueMatt/repos","events_url":"https://api.github.com/users/TheBlueMatt/events{/privacy}","received_events_url":"https://api.github.com/users/TheBlueMatt/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/bitcoin/bitcoin/labels/P2P","name":"P2P","color":"006b75"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":22,"created_at":"2016-05-18T01:49:04Z","updated_at":"2016-06-11T01:48:20Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/bitcoin/bitcoin/pulls/8068","html_url":"https://github.com/bitcoin/bitcoin/pull/8068","diff_url":"https://github.com/bitcoin/bitcoin/pull/8068.diff","patch_url":"https://github.com/bitcoin/bitcoin/pull/8068.patch"},"body":"This is based on #8020 and implements the BIP 152 draft from https://github.com/TheBlueMatt/bips/blob/152/bip-0152.mediawiki.\r\n\r\nIn short, it sends blocks as a set of short transaction IDs to allow nodes to not double-relay transactions in blocks. There are lots of details in both the BIP and the discussion on the bitcoin-dev ML.\r\n\r\nThere are still some TODOs in the code here, but I do not think they are blockers for this, as they are not protocol-level changes and can be implemented separately to make the protocol more effecient."},"comment":{"url":"https://api.github.com/repos/bitcoin/bitcoin/issues/comments/225330119","html_url":"https://github.com/bitcoin/bitcoin/pull/8068#issuecomment-225330119","issue_url":"https://api.github.com/repos/bitcoin/bitcoin/issues/8068","id":225330119,"user":{"login":"JeremyRubin","id":886523,"avatar_url":"https://avatars.githubusercontent.com/u/886523?v=3","gravatar_id":"","url":"https://api.github.com/users/JeremyRubin","html_url":"https://github.com/JeremyRubin","followers_url":"https://api.github.com/users/JeremyRubin/followers","following_url":"https://api.github.com/users/JeremyRubin/following{/other_user}","gists_url":"https://api.github.com/users/JeremyRubin/gists{/gist_id}","starred_url":"https://api.github.com/users/JeremyRubin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JeremyRubin/subscriptions","organizations_url":"https://api.github.com/users/JeremyRubin/orgs","repos_url":"https://api.github.com/users/JeremyRubin/repos","events_url":"https://api.github.com/users/JeremyRubin/events{/privacy}","received_events_url":"https://api.github.com/users/JeremyRubin/received_events","type":"User","site_admin":false},"created_at":"2016-06-11T01:48:20Z","updated_at":"2016-06-11T01:48:20Z","body":"> This is already designed targeting several times the maximum size permitted in the protocol. Also the size of the block does not weigh that strongly in the result (mempool size dominates), and the implementation deals fairly gracefully with small numbers of collisions.\r\n\r\nYes, mempool size should dominate. This seems like a completely-other issue. Mining nodes who keep large mempools will run into large collision frequencies and this will be less useful.\r\n\r\nThis is furthermore troubling because the size used in the bip (m=100000 txns or m*225 bytes not including overhead) is around 22 megabytes whereas the defaults in policy.h seems to be 300 mb of mempool, unless I'm mistaken. Adding a factor of 10 txns to m puts the bits above 48. 56 bits allows for up to 20 gigabytes or so, which seems more appropriate. Although it may make the most sense to have peers communicate to each other how full their mempools are and set hash length based on that.\r\n\r\n\r\n> The protocol is versioned, any change would need to be a new version, so I don't see what extra states would do there.\r\n\r\n👍 \r\nyup I struck that one out before you replied :) \r\n\r\n> It's 27 times faster in a performance important path. Siphash was an upgrade from using an even simpler function previously.\r\n\r\n> A global salt would provide absolutely no protection against the collision attack. The value must be unpredictable to an attacker and cannot be precomputed. There should be no total reduction in hashing the mempool- it's just once per received block. Avoiding latency is important but time taken for siphash a maxed out mempool on a single 3GHz core is 0.17ms, this is insubstantial compared to the rest of block processing.\r\n\r\nI'd like to see more of a threat model on this. What's the attack? An attacker makes a block that they mined slow to propagate? Why would they do that, and not just withhold the block? An attacker broadcasts transactions to be included which would cause a lot of conflicts into the networks mempool on every block? This is expensive to do. Overall, I don't think there is any real security concern with making the nonce be deterministically based on the previous block.\r\n\r\nLet's say we pick a more difficult hash to compute, but make it possible to compute before receiving the block this minimizes the hash-grinding concern, right? But this also still makes it possible to precompute the table which removes the 0.17 ms latency in the hot path. If the goal is < 10ms validation of a block, then 0.17 is not nothing if it can be shaved off. (Furthermore, currently the code re-hashes on a per-peer basis rather than implementing the BIP implementation note 6, which would be a bit better)\r\n\r\n> I originally implemented that, in fact. But low hitrates are unobservably rare after the initial start... spending a byte to signal it is a net loss.. plus the code is more complex. It's also the case that low hitrate data is naturally smaller because all the differences will be very small.\r\n\r\n👍 \r\n\r\n> That is an interesting point however, the coinbase txn is always required; so doing exactly that would not improve anything.\r\n>Ignoring the coinbase implementation would be rather complex: ... if you complete the decode and the prefilled haven't arrived yet, do you wait? how long? or do you just gettxn potentially redundantly with the transactions in flight? if you wait and time out do you ban the peer when it doesn't respond or? Basically when thinking about implementation complexity you should think about the complexity being exponential with the number of communication states you might have with a peer. :)\r\n\r\nWell coinbase txn could always be included outside of the prefilled separate block. I actually like that as a design anyways -- perhaps there should be a difference between txns we KNOW to not be in their mempool and ones we EXPECT to not be in the mempool. At the cost of an extra length field, perhaps not worth it, but if it is just the coinbase (not needing length) perhaps it is worth it.\r\n\r\n\r\n> I've tested this and it works great. The recommendations in BIP152 were based precisely on doing this. It's not included in this PR to minimize the scope as it doesn't change the protocol and can be added any point. (there are further refinements you can do, like leaving out txn that a target peer has INVed to you, even if you didn't know them)\r\n\r\n👍 \r\n\r\nI don't see these recommendations specifically in the BIP, but I support adding this later."}}
{"actor":{"display_login":"JeremyRubin"}}