< * sipa> just learned how to rebase a branch that has merges
< sipa> (where the merges have merge conflicts)
< btcdrak> sipa: what dark magic is this?
< sipa> first use git rebase -i -p <base commit>
< btcdrak> TIL: -p
< sipa> that will complain whenever the merge commit is to be merged, as it has merge conflicts
< sipa> (and rebase -p can't deal with reapplying merge resolution)
< sipa> then use git checkout -p <original merge commit id>
< sipa> wait, first use git add -P, to mark all merge conflicts as resolved (you're lying, they aren't)
< sipa> and then use git checkout -p <original merge commit id>, which applies all changes between the current tree and the tree after that commit
< sipa> sorry, git add -A
< sipa> grr
< sipa> that git checkout -p will show you all the differences between the current tree (which includes the <<< === >>> markers from conflicts you haven't actually resolved) and the result of the original merge commit
< sipa> which you all accept
< sipa> except the changes that are due to changes made earlier in history
< sipa> as you don't want those reset to the original
< NicolasDorier> sipa: are you here ? I noticed strange incoherence between the BIP and CMPTBLK implementation, I'm wondering if I've not missed something
< NicolasDorier> I've added some comment during my review, but either I'm completely misunderstanding something or the BIP and implementation is completely off
< phantomcircuit> NicolasDorier, something something dont ask to ask
< phantomcircuit> :P
< NicolasDorier> what does it mean ? :p
< NicolasDorier> so basically my problem is
< NicolasDorier> SENDCMPCT should have a boolean which indicate in which mode the peer want to receive new blocks
< NicolasDorier> either with INV or with CMPCT BLK
< sipa> indeed, and a version number
< NicolasDorier> problem is, in the PR, this boolean is used to indicate whether the sender provide or not CMPCTBLK
< NicolasDorier> oups
< NicolasDorier> no
< NicolasDorier> this
< sipa> NicolasDorier: i think you're right
< NicolasDorier> sipa: I guess the spec changed after the code was released for example: https://github.com/bitcoin/bitcoin/pull/8068/files#diff-7ec3c68a81efff79b6ca22ac1f1eabbaR4920
< NicolasDorier> the intention was to use the bool to activate or deactivate CMPCT
< NicolasDorier> as far as I understand
< NicolasDorier> I can work on fixing it, but I heard you are working on the PR right now sipa ?
< sipa> i think fProvidesHeaderAndIDs should just be set to true in response to SENDCMPCT
< NicolasDorier> mmh it is not the same semantic
< NicolasDorier> SENDCMPCT tell you about the want of the remote node
< NicolasDorier> not about his capabilities
< sipa> both
< NicolasDorier> well, you can already use the version in the handshake for it
< sipa> no, you can't
< sipa> nVersion >= 70014 does not imply you support compact blocks
< NicolasDorier> ? why ? because of pruned nodes ?
< sipa> because we don't want everyone in the network to be forced to implement this
< sipa> maybe 70015 introduces another features that is easy to implement
< NicolasDorier> in such case maybe a service BIT can be useful
< sipa> service bits are expensive
< sipa> we only have 48
< sipa> sorry, 56
< sipa> BIP130 also does not use a service bit
< NicolasDorier> is it a problem ? if we run out of them we can release a new protocol version with more bits
< sipa> maybe :)
< sipa> but there is a lot of infrastructure
< sipa> that uses it
< NicolasDorier> understood, are you working on it ? I can refactor things a bit and fix the terminology to match the bip
< sipa> BlueMatt commented on the suggestion to use a service bit here: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2016-May/012630.html
< sipa> yes, i commented on the duplication
< sipa> you also commented on some of the refcounting... the refcounting is gone in my branch (https://github.com/sipa/bitcoin/commits/compactblocks)
< NicolasDorier> ok I'll continue my review on your branch instead
< sipa> i think the two assignments to preferheadersandids and providesheadersandids just need to be swapped
< NicolasDorier> it is also a bit confusing: I fail to understand if preferHeadersAndIds means that we use the "high bandwidth (without inv)" or the "low bandwidth" one
< sipa> that's exactly what it means
< NicolasDorier> it can also mean that it does not support CMPCTBLK at all
< sipa> no, that's providesheadersandids
< NicolasDorier> an enum with 3 values would be easier imhi
< sipa> providesheadersandids is something that affects our request logic
< sipa> preferheadersandids is something that affects our send logic
< NicolasDorier> sipa: on send logic we have 3 cases, legacy, high bandwidth and low bandwidth. PreferHeadersAndIds is a boolean
< NicolasDorier> oh
< NicolasDorier> oh no I get it
< NicolasDorier> because low bandwidth still use INV, it is not different from legacy
< NicolasDorier> ok, thanks... continuing my review I think it is clearer now
< sipa> indeed; the difference is that the peer will respond with a getdata MSG_COMPCT_BLOCK rather than MSG_COMPCT_BLOCK
< sipa> eh
< sipa> rather than MSG_BLOCK
< sipa> but that's not our worry
< NicolasDorier> yes make sense thanks!
< * MarcoFalke> looks at travis merge commit hash 4222221c0000... and wonders about the odds
< sipa> MarcoFalke: i have considered grinding commit hashes in segwit to be consecutive numbers :p
< GitHub189> [bitcoin] jonasschnelli pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/8c1e49ba13a8...d46b8b50fc3e
< GitHub189> bitcoin/master e39dc69 instagibbs: comment nit: miners don't vote
< GitHub189> bitcoin/master d46b8b5 Jonas Schnelli: Merge #8143: comment nit: miners don't vote...
< MarcoFalke> You should do it after the rebase :P
< GitHub195> [bitcoin] jonasschnelli closed pull request #8143: comment nit: miners don't vote (master...notavote) https://github.com/bitcoin/bitcoin/pull/8143
< MarcoFalke> sipa: Could you push a `git commit --allow-empty` or something to have at least one travis result for https://github.com/bitcoin/bitcoin/pull/7749#issuecomment-223558640 ?
< * MarcoFalke> wonders what happens if someone merges https://github.com/bitcoin/bitcoin/pull/7510 via the GitHub GUI. (The pull conflicts with master but GitHub shows no conflicts... o0 )
< sipa> MarcoFalke: dragons
< MarcoFalke> GitHub unicorn
< MarcoFalke> They have figured out AI that can solve conflicts for you
< sipa> we should invent a programming language in which every sequence of ascii characters is a valid program
< sipa> no more merge conflicts
< sipa> the result may however not be code you want to run
< sipa> <<< should mean "format disk"
< btcdrak> babies speak that but they forget it when they grow up so cant teach the adults