1 2016-08-05 00:00:29	0|sipa|sdaftuar, luke-jr, gmaxwell, wumpus: i believe that downgrading from 0.13.1 to 0.13.0 will just work
  2 2016-08-05 00:00:49	0|sipa|sdaftuar: even in case of a reorg that takes you back into witness-enabled code
  3 2016-08-05 00:00:56	0|sipa|s/code/blocks/
  4 2016-08-05 00:01:16	0|luke-jr|well, that's good, I think
  5 2016-08-05 00:01:43	0|sipa|the test that no witness data is present in non-witness blocks is only done in AcceptBlock, not in ConnectBlock
  6 2016-08-05 00:02:02	0|sipa|so we enforce it when receiving the block from somewhere, but not afterwards
  7 2016-08-05 00:02:07	0|sipa|which i think is exactly right
  8 2016-08-05 00:02:44	0|gmaxwell|sipa: what happens if you downgrad then upgrade again?
  9 2016-08-05 00:03:33	0|sipa|gmaxwell: newly added blocks will be rewinded
 10 2016-08-05 00:49:42	0|morcos|sipa: luke-jr: sorry, but i'm not sure i understand sipa's suggested compromise in 8459.  sorry i don't remember exactly what the first version was, but i think it makes sense to explain that you don't NEED to specify both size and weight, otherwise its just confusing as to how you are supposed to set things up
 11 2016-08-05 00:50:56	0|morcos|as to making a recommendation whether or not to set size...  i'm not sure why you say its not worse than 0.12, we don't know that.  my guess is that it isn't significantly worse now, but it certainly could be post segwit, when you are trying to fit the last tx in a block and you keep trying EVERYTHING b/c the limit thats stopping you isn't the consensus limit
 12 2016-08-05 00:51:52	0|morcos|i do apologize for not testing at least the serialization effect (the other would be tricky to test), but i've been too caught up in trying to figure out why waking up threads is so abysmally slow
 13 2016-08-05 00:52:14	0|luke-jr|it doesn't serialise any more than without maxsize AFAIK.
 14 2016-08-05 00:52:21	0|luke-jr|GetSerializeSize doesn't serialize
 15 2016-08-05 00:54:01	0|luke-jr|morcos: I thought it was obvious the options were optional.. but if you disagree, I can add perhaps: "Both of these are optional, but at least blockmaxsize ought to be assigned by miners, to determine the largest block they are willing to create."
 16 2016-08-05 00:54:01	0|morcos|ah so its expected that GetSerializeSize is fast?  well that explains why anecdotally its not slower
 17 2016-08-05 00:54:25	0|luke-jr|morcos: GetSerializeSize is literally just adding the sizes of each field
 18 2016-08-05 00:54:25	0|morcos|luke-jr: i assume you make suggestions like that on purpose
 19 2016-08-05 00:54:49	0|luke-jr|morcos: ?
 20 2016-08-05 00:55:45	0|sipa|i expect getserializesize time to be dominated by just memory access of iterating all vectors
 21 2016-08-05 00:56:08	0|morcos|luke-jr: you are against what you see as advice in the notes to use weight, but surely you understand that all of us are against advice to limit based on bytes?  if any of us believed that was an issue, we'd be opposed to segwit
 22 2016-08-05 00:56:33	0|luke-jr|morcos: it is clearly an issue today. I am shocked that anyone would disagree.
 23 2016-08-05 00:56:53	0|luke-jr|0.13 will hopefully change that with CB, but it isn't deployed yet.
 24 2016-08-05 00:58:10	0|sipa|luke-jr: my view is that at worst it may make block propagation slightly worse, but at the same time it improves utxo storage incentives, which i'm much more concerned about long term
 25 2016-08-05 00:58:26	0|morcos|how about language along the lines of "If a miner wishes to limit the actual size of a block in bytes (as opposed to the consensus weight limit) then it is necessary to specify a blockmaxsize explicitly.  If a miner does not wish to do that, it would be better to only specify a blockmaxweight"
 26 2016-08-05 00:58:41	0|sipa|a small constant factor was never something i opposed for block size. an expectation of it being increased under whatever perceived pressure is
 27 2016-08-05 00:59:00	0|luke-jr|morcos: that suggests miners ought not to limit size
 28 2016-08-05 00:59:09	0|sipa|i think miners ought not to limit size
 29 2016-08-05 00:59:12	0|morcos|something like that, not exactly "as opposed to" b/c that sounds like you're not enforcing the consensus limit
 30 2016-08-05 00:59:19	0|sipa|because it's unreasonable that all of them will
 31 2016-08-05 00:59:55	0|morcos|luke-jr: why does that language suggest they ought not limit size.   its just saying that if you dont' care about a limit on actual size, then its better to not set blockmaxsize.
 32 2016-08-05 00:59:59	0|morcos|which is TRUE
 33 2016-08-05 01:00:25	0|luke-jr|if they don't care, they should set it based on someone's recommendation, but not necessarily the big blocker recommendation
 34 2016-08-05 01:01:03	0|luke-jr|if they care about Bitcoin (but not per se enough to research their own opinion on block size limits), they should follow recommendations to set it lower.
 35 2016-08-05 01:01:23	0|sipa|luke-jr: as a collective, perhaps
 36 2016-08-05 01:01:26	0|luke-jr|brb
 37 2016-08-05 01:01:41	0|sipa|luke-jr: there could be a gentlemen's agreement not to set it to the maximum immediately
 38 2016-08-05 01:01:57	0|sipa|but i believe that's unrealistic at this point
 39 2016-08-05 01:02:29	0|morcos|"If a miner wishes to limit the actual size of a block in bytes then it is necessary to specify a  blockmaxsize explicitly.  If a miner wishes only to limit block creation by weight (BIP141) regardless of how large in bytes the block is they should only specify a blockmaxweight"
 40 2016-08-05 01:02:52	0|luke-jr|sipa: every time I brought this up before, I was asked to "wait for the segwit release notes"; now I'm told it's too late?
 41 2016-08-05 01:02:54	0|morcos|i'm trying to make it as neutral as possible, while explaining the difference
 42 2016-08-05 01:02:56	0|luke-jr|(still brb mode)
 43 2016-08-05 01:03:32	0|morcos|what i don't want is a bunch of miners trying to mine max blocks and having a blockmaxsize set (especially after segwit).  and we have to explain in detail enough for that to happen.  i have no objection if they actually want to limit bytes
 44 2016-08-05 01:03:54	0|sipa|luke-jr: i can't remember ever recommending to wait for release notes. i have told you many times that if you disagree that weight is the limiting factor on the network, you should oppose segwit
 45 2016-08-05 01:05:33	0|morcos|sipa: want to think about something different an arcane for a second?   in those checkqueue graphs i presented you, the scriptthreads were busyspinning waiting for work
 46 2016-08-05 01:05:58	0|sipa|morcos: i heard
 47 2016-08-05 01:06:08	0|morcos|it slows down the whole thing quite a bit (about 5ms per block) but still a big improvement, if you wake up the scripthreads only when there is work
 48 2016-08-05 01:06:52	0|morcos|even though it takes only about 100us for them all to wake up, and you can make that happen as early as the start of ConnectTip so they are all awake and busy spinning well before work comes in
 49 2016-08-05 01:07:11	0|morcos|i'm assuming its the fact that their caches have been blown away, but doesn't 5ms seem like a lot?
 50 2016-08-05 01:08:20	0|morcos|just wondering if you had any thoughts..  we're still working on it (mostly jeremy, i'm mostly just testing)
 51 2016-08-05 01:09:07	0|sipa|morcos: is the slowdown as significant when you have fewer threads?
 52 2016-08-05 01:09:28	0|morcos|sadly i didn't test that
 53 2016-08-05 01:10:15	0|morcos|yet
 54 2016-08-05 01:11:44	0|sipa|and this is 5ms clock time, not cpu time?
 55 2016-08-05 01:13:04	0|morcos|yes, 5ms clock time.  it appears from debugging that the scriptthreads are just slower to run.
 56 2016-08-05 01:16:43	0|sipa|i guess if there are many memory locations to access one by one for script validation to run, the effect of not running could be many ram latencies
 57 2016-08-05 01:16:54	0|sipa|but i'm not sure whether than makes sense
 58 2016-08-05 01:18:01	0|morcos|ok, don't worry about it.. its mostly just bugging me.  i realize that it doesn't make sense to micro optimize for one architecture anyway, but just trying to understand the effect
 59 2016-08-05 03:10:59	0|PatBoy|thx dev for all ur hard work !
 60 2016-08-05 03:11:22	0|luke-jr|sipa: I do not believe that opposing segwit is a serious suggestion.
 61 2016-08-05 03:12:40	0|luke-jr|morcos: then in the segwit announcement, we can re-evaluate and make recommendations that make sense given the new network conditions
 62 2016-08-05 03:13:11	0|luke-jr|(which might or might not include removing blockmaxsize depending on CB effectiveness and real-world testing)
 63 2016-08-05 03:15:08	0|sipa|luke-jr: miners will set block and weight likely to the maximum... that may be suboptimal, but it's inevitable that this is what will happen if segwit goes through
 64 2016-08-05 03:15:54	0|sipa|people can suggest to not do that... but for centralization risk (which is a long term effect), the important question is now what people do, but what they could do
 65 2016-08-05 03:16:01	0|sipa|*not
 66 2016-08-05 03:16:06	0|luke-jr|sipa: we don't know that yet, and our recommendations should always be what is sane even if they get ignored.
 67 2016-08-05 03:18:13	0|sipa|luke-jr: that's a reasonable position... but the code is written from a viewpoint that we will get weight-limited block construction
 68 2016-08-05 03:18:28	0|sipa|luke-jr: and the release notes should describe the code
 69 2016-08-05 03:19:26	0|luke-jr|then the code is broken (sabotaged, it sounds like) and fixing it should be considered a blocker for any release.
 70 2016-08-05 03:19:56	0|sipa|if that is your viewpoint, then it is segwit that is sabotaged
 71 2016-08-05 03:20:26	0|sipa|i disagree strongly with that
 72 2016-08-05 03:20:46	0|luke-jr|sipa: do you disagree strongly with 29000050e575a26b7f7c853cbccdb6bc60ddfdd9 for 0.13?
 73 2016-08-05 03:21:22	0|sipa|i think it's pointless
 74 2016-08-05 03:21:45	0|sipa|we're doing busy work to avoid stating reality in release notes
 75 2016-08-05 03:21:56	0|sipa|the reality is that segwit will mean that blocks go over 1 MB
 76 2016-08-05 03:22:03	0|luke-jr|reality is that blockmaxsize should be used for today and the near future
 77 2016-08-05 03:22:12	0|sipa|i disagree with that
 78 2016-08-05 03:22:28	0|gmaxwell|luke-jr: I think you are not paying attention to reality. Virtually all miners make blocks as large as they can. If at any point they make a block smaller than maximum, angry mobs show up and demand them to make maximum size blocks. Not wanting to deal with such nonsense, they simply do it.
 79 2016-08-05 03:22:36	0|sipa|or rather, i would agree with that if there was a reasonable chance that the entire ecosystem will self limit in that way
 80 2016-08-05 03:22:40	0|sipa|it won't
 81 2016-08-05 03:22:58	0|gmaxwell|this is the existing behavior in the network, and has been for a long time-- more or less since we made it configurable at all.
 82 2016-08-05 03:23:21	0|luke-jr|sipa: the entire ecosystem doesn't need to; why is it all or nothing?
 83 2016-08-05 03:23:45	0|luke-jr|gmaxwell: that's with 1 MB blocks. who knows if it will work out the same with 3 MB
 84 2016-08-05 03:24:21	0|gmaxwell|it especially will work out that way, with CB and relay mitigating orphaning effects.
 85 2016-08-05 03:24:28	0|luke-jr|I know that if miners start doing >1 MB blocks before the network is ready, I will intentionally NOT use a segwit wallet, to do my small part in preventing them from bloating the blocks. I think a lot of the community could be convinced to do the same.
 86 2016-08-05 03:24:40	0|gmaxwell|luke-jr: and if you want to posit a change in how things work you should suggest how.
 87 2016-08-05 03:25:09	0|gmaxwell|luke-jr: nothing held back blocksizes from growing from 250k to 1mb... not even the defaults in bitcoin core-- or large increases in orphaning rates.
 88 2016-08-05 03:25:17	0|sipa|luke-jr: smaller blocks will always be better for propagation relay effects, and larger blocks will always be worse
 89 2016-08-05 03:25:30	0|sipa|luke-jr: there is no 'ready' here; just a compromise that is acceptable
 90 2016-08-05 03:25:50	0|sipa|segwit shifts the relay effects one way, but shifts other incentives the other way
 91 2016-08-05 03:27:37	0|luke-jr|ugh, even IF miners will end up doing terrible things, it doesn't mean we need to sabotage miners who WANT to do what is best for the network.
 92 2016-08-05 03:28:20	0|gmaxwell|its not a question of "want to do what is best"; most people don't want to decide and do not have the time to do so in any case.
 93 2016-08-05 03:28:28	0|gmaxwell|You're thinking in terms of pixie unicorn miners.
 94 2016-08-05 03:28:35	0|sipa|the only way miners can do something that is good for the network is by making themselves replacable
 95 2016-08-05 03:28:44	0|gmaxwell|Go look at the actual blocks. The miners you're expecting just don't exist at a meaningful level.
 96 2016-08-05 03:30:20	0|sipa|luke-jr: moreover, we're not removing the ability to limit the block size
 97 2016-08-05 03:31:22	0|sipa|reality is that the code is optimized for limiting by weight, and segwit is designed with the assumption that that is what will happen... your suggestions seem to be that we should hide that
 98 2016-08-05 03:31:52	0|sipa|block size limiting is supported, and we should explain how to use it for those who want to
 99 2016-08-05 03:32:02	0|luke-jr|gmaxwell: yes, pixie unicorn miners who actually exist, despite being ignored by Core
100 2016-08-05 03:32:42	0|luke-jr|sipa: either blockmaxsize has a notable cost or it doesn't. if it doesn't, there is no reason to claim it does.
101 2016-08-05 03:32:51	0|sipa|luke-jr: so, benchmark
102 2016-08-05 03:32:52	0|luke-jr|if it does, then it's sabotaging miners who use it
103 2016-08-05 03:33:26	0|gmaxwell|luke-jr: they don't exist in a meaningful sense. a miner that gets a block a day is not making a meaningful dent in the systems' survivability.
104 2016-08-05 03:33:42	0|sipa|luke-jr: limiting by block size is already taking a cut. fee income will be suboptimal if you limit by size
105 2016-08-05 03:33:49	0|sipa|luke-jr: that's as much sabotaging
106 2016-08-05 03:33:52	0|luke-jr|gmaxwell: so that justifies ignoring the requests of such miners, and sabotaging them with (allegedly) bad performance?
107 2016-08-05 03:34:16	0|sipa|limiting by weight is a design decision which has costs
108 2016-08-05 03:34:18	0|gmaxwell|'sabotaging' after I had to jump up and down and yell for over a month to get eligius to upgrade to 0.12 even though it made a _tens of fold_ increase in createnewblocktime?
109 2016-08-05 03:34:40	0|gmaxwell|This is such bullshit.
110 2016-08-05 03:35:47	0|luke-jr|ignoring that 0.12 broke numerous things Eligius provided that were left unmerged in Core since 0.6..
111 2016-08-05 03:36:23	0|sipa|such as?
112 2016-08-05 03:36:25	0|luke-jr|CPFP
113 2016-08-05 03:37:22	0|sipa|i never trusted that code enough to merge
114 2016-08-05 03:37:29	0|sipa|and i think i was not alone
115 2016-08-05 03:37:53	0|gmaxwell|I am fed up with this.
116 2016-08-05 03:38:09	0|luke-jr|same here.
117 2016-08-05 03:38:24	0|gmaxwell|luke-jr: you are abusive towards me and the other contributors.
118 2016-08-05 03:38:31	0|gmaxwell|you are obsessing over minutia on top of minutia.
119 2016-08-05 03:38:49	0|gmaxwell|You are wasting countless hours exhausting all patience.
120 2016-08-05 03:39:55	0|gmaxwell|Over matters which _do_ _not_ _matter_.  The few obscure miners which will set non-defaults even though they get abusive and threatening contact from users (which drives away their hashpower); can _still_ do so. If it's slightly slower? so what--- the latest software is dozens of times faster to creates blocks than older software and they hardly cared to upgrade.
121 2016-08-05 03:40:55	0|gmaxwell|it litterally makes no difference in the world, and yet you force people to spend hours and hours debating these things.
122 2016-08-05 03:41:28	0|gmaxwell|and I get to spend my time asking others to not leave the project because they are exhausted by you; but it even exhausts me too.
123 2016-08-05 03:45:27	0|gmaxwell|The last block from eligius was 64 hours ago.  It contained _NO_ transactions.  I would say that createnew block being merely 29.5 times faster than the old code it was running until recently instead of 30x faster won't matter. ... except it won't even see that difference when it mines empty blocks with no transactions at all.
124 2016-08-05 03:47:49	0|gmaxwell|When it does actually include transactions-- it appears to produce maximum size blocks just like everyone else: https://blockchain.info/block/0000000000000000050631b932ed81eb9de6267f1cb8b8a353dd59365fd07fd5
125 2016-08-05 10:40:02	0|jonasschnelli|I think "memusage.h" should include "prevector.h"
126 2016-08-05 10:56:08	0|GitHub83|[13bitcoin] 15jonasschnelli closed pull request #7865: [RPC] Add bumpfee command. (06master...062016/04/rbf_combined) 02https://github.com/bitcoin/bitcoin/pull/7865
127 2016-08-05 12:22:32	0|wumpus|jonasschnelli: you'd say so, as the type is used; but does any compiler complain about this in practice? it's in a template, so it could be that you only need it at instantiation time
128 2016-08-05 12:22:45	0|wumpus|I'm a little rusty on the exact rules there though
129 2016-08-05 12:23:16	0|jonasschnelli|wumpus: I just added some stats stuff and included memusage.h, compiler was then complaining about the missing type prevector...
130 2016-08-05 12:24:02	0|jonasschnelli|memusage.h is directly using (in a template but directly) prevector
131 2016-08-05 12:24:24	0|jonasschnelli|But anyways... far away from important. :)
132 2016-08-05 12:24:38	0|wumpus|does the data structure that you're trying to measure include a prevector
133 2016-08-05 12:24:41	0|wumpus|?
134 2016-08-05 12:25:16	0|jonasschnelli|no. I just want to measure a vector of structs... but mempool.h refers to prevector
135 2016-08-05 12:25:39	0|jonasschnelli|I guess all other places where mempool.h is included, prevector was previously already included
136 2016-08-05 12:26:05	0|wumpus|there is no mempool.h? I guess you mean memusage.h?
137 2016-08-05 12:26:19	0|jonasschnelli|argm. memusage.h
138 2016-08-05 12:26:28	0|wumpus|indeed, if including memusage.h forces you to include prevector.h then it should be included there
139 2016-08-05 12:26:39	0|jonasschnelli|static inline size_t DynamicUsage(const prevector<N, X, S, D>& v
140 2016-08-05 12:27:03	0|jonasschnelli|It's a nitpick and I'll try to cover that fix in one of my stats releated commits.
141 2016-08-05 12:28:07	0|wumpus|sometimes the c/c++ dependency management is a bit unfortunate, very easy to accidentally have something creep into your scope so you forget that direct dependencies exist but have no include, then it turns up later when the header happens to be used independently
142 2016-08-05 12:28:12	0|wumpus|right
143 2016-08-05 12:29:32	0|wumpus|unlike missing prototypes in C, at least this cannot lead to ugly type-unsafety bugs, you either have to include it somewhere or it just won't compile :)
144 2016-08-05 16:04:29	0|GitHub89|[13bitcoin] 15Mirobit closed pull request #8283: Move AdvertiseLocal debug output to net category (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/8283
145 2016-08-05 16:38:21	0|GitHub188|[13bitcoin] 15Mirobit opened pull request #8462: Move AdvertiseLocal debug output to net category (06master...06advertiselocal) 02https://github.com/bitcoin/bitcoin/pull/8462
146 2016-08-05 16:51:06	0|kvnn|Can anyone tell me how much disk space testnet currenty takes?
147 2016-08-05 17:04:39	0|GitHub149|[13bitcoin] 15MarcoFalke opened pull request #8463: [qt] Remove Priority from coincontrol dialog (06master...06Mf1608-qtPrio) 02https://github.com/bitcoin/bitcoin/pull/8463
148 2016-08-05 17:16:37	0|GitHub47|[13bitcoin] 15JeremyRubin opened pull request #8464: "Lockfree" Checkqueue Implementation (06master...06lockfree-checkqueue-squashed) 02https://github.com/bitcoin/bitcoin/pull/8464
149 2016-08-05 17:19:17	0|jeremyrubin|(forgot to mark as WIP fyi)
150 2016-08-05 17:21:16	0|btcdrak|you can edit the title
151 2016-08-05 17:21:26	0|btcdrak|jeremyrubin
152 2016-08-05 17:22:43	0|btcdrak|jeremyrubin: it's quite useful to use markdown task lists in the PR body, as those show up in lists and references on other tickets/PRs https://github.com/blog/1825-task-lists-in-all-markdown-documents and it also has the side benefit of making the WIP status clear
153 2016-08-05 17:27:43	0|jeremyrubin|btcdrak: what are the tasks?
154 2016-08-05 17:27:59	0|btcdrak|your todo list.
155 2016-08-05 17:28:00	0|jeremyrubin|btcdrak: the only stuff really remaining is more testing.
156 2016-08-05 17:28:27	0|jeremyrubin|btcdrak: actually I can think of two things
157 2016-08-05 17:28:40	0|sipa|jeremyrubin: if all that remains is testing, i wouldn't call it WIP :)
158 2016-08-05 17:30:14	0|btcdrak|sipa: exactly! if it's marked WIP no-one will test it.
159 2016-08-05 17:31:47	0|jeremyrubin|sipa: oh ok... I'll un WIP it then, it's ready for testing
160 2016-08-05 17:33:05	0|jeremyrubin|sorry was unclear at what point I should WIP/vs non WIP, thanks for the guidance
161 2016-08-05 17:35:05	0|jeremyrubin|I'm going to be awk for a little bit, but will respond to more things later.
162 2016-08-05 17:35:19	0|jeremyrubin|sipa: you may just want to view the whole diff rather than commit by commit
163 2016-08-05 17:42:55	0|sipa|jeremyrubin: i tend to review per commit, as i think every commit should logically make sense... the combination into a pull request is there to see the bigger picture
164 2016-08-05 17:52:33	0|jeremyrubin|yep I agree, each commit should compile and run on this PR but is maybe not fully documented ;)
165 2016-08-05 18:38:58	0|bsm117532|wizkid057, the exploding bitcoin spammer, somehow got op permissions on #bitcoin-wizards and is banning people.  Can someone here take care of him?
166 2016-08-05 18:41:27	0|jonasschnelli|jeremyrubin: impressive PR... my review will take a while. :)
167 2016-08-05 18:44:35	0|pigeons|he's not banning people, just kicked you only and its not the spammer it really is wizkid. probably just a mistake
168 2016-08-05 18:45:45	0|bsm117532|sorry, don't know who wizkid is.
169 2016-08-05 19:03:17	0|GitHub178|[13bitcoin] 15sipa opened pull request #8465: [0.13] Document reindexing changes (060.13...06docreindex) 02https://github.com/bitcoin/bitcoin/pull/8465
170 2016-08-05 19:07:43	0|GitHub185|[13bitcoin] 15paveljanik opened pull request #8466: [Trivial] Do not shadow variables in networking code (06master...0620160805_Wshadow_net) 02https://github.com/bitcoin/bitcoin/pull/8466
171 2016-08-05 19:17:09	0|sipa|i feel that 'New mempool information RPC calls' could move to 'Low-level RPC changes' in the release notes
172 2016-08-05 19:17:14	0|jtimon|thoughts on https://github.com/bitcoin/bitcoin/compare/master...jtimon:0.13-consensus-flags?expand=1 NicolasDorier btcdrak sipa should I open a PR?
173 2016-08-05 19:18:00	0|jtimon|this doesn't touch the script flags and doesn't move anything out of contextualCheckBlock like the previous one
174 2016-08-05 19:18:41	0|jtimon|it could be 1 or a few commits, but I kept it extremely separated just in case
175 2016-08-05 19:22:00	0|GitHub153|[13bitcoin] 15paveljanik opened pull request #8467: [Trivial] Do not shadow members in dbwrapper (06master...0620160805_Wshadow_dbwrapper) 02https://github.com/bitcoin/bitcoin/pull/8467
176 2016-08-05 19:32:08	0|sdaftuar|sipa: ack
177 2016-08-05 19:33:36	0|sdaftuar|sipa: more generally i think if you're going to clean up further, perhaps reordering the Notable Changes section might be called for.  eg hd wallet support, segwit, cpfp mining, and compact blocks strike me as more important to highlight than c++11 code modernizations
178 2016-08-05 19:36:14	0|sipa|maybe a section that aggregates all non-user-visible code changes
179 2016-08-05 19:36:25	0|sipa|that would even include segwit, i guess
180 2016-08-05 19:42:39	0|sdaftuar|sure.  fyi the mining stuff references segwit, so if you move those around relative to each other, the text might need some edits
181 2016-08-05 19:55:05	0|GitHub14|[13bitcoin] 15paveljanik opened pull request #8468: Do not shadow member variables in serialization (06master...0620160805_Wshadow_serialization) 02https://github.com/bitcoin/bitcoin/pull/8468
182 2016-08-05 20:13:11	0|Chris_Stewart_5|Is this entire block of code inside of bloom.cpp basically so that we can match redeemScript/pubkey hashes in scriptPubKeys?
183 2016-08-05 20:13:14	0|Chris_Stewart_5|https://github.com/bitcoin/bitcoin/blob/master/src/bloom.cpp#L163-LL177
184 2016-08-05 20:16:34	0|jonasschnelli|sipa: how do I get the DynamicUsage from a struct?
185 2016-08-05 20:16:43	0|jonasschnelli|MallocUsage(size_t alloc) is inline...
186 2016-08-05 20:17:13	0|sipa|jonasschnelli: sum the dynamic usages of its member fields
187 2016-08-05 20:18:45	0|jonasschnelli|sipa: what about MallocUsage(sizeof(mystruct))?
188 2016-08-05 20:18:47	0|jonasschnelli|+allow external calls to MallocUsage
189 2016-08-05 20:19:04	0|sipa|jonasschnelli: are you mallocing your stuct?
190 2016-08-05 20:19:41	0|jonasschnelli|sipa: ... ah... no.
191 2016-08-05 20:19:56	0|sipa|then you shouldn't be using either of them
192 2016-08-05 20:20:05	0|jonasschnelli|I guess std::vector<mystruct> is on the stack
193 2016-08-05 20:20:17	0|sipa|DynamicUsage is not how much memory a struct uses. It's how much _dynamic_ memory it uses. The static memory you find just using sizeof.
194 2016-08-05 20:20:36	0|sipa|jonasschnelli: use DynamicUsage on the vector.
195 2016-08-05 20:20:51	0|sipa|The vector is allocated on the stack. Its entries are allocated on the heap.
196 2016-08-05 20:21:07	0|sipa|DynamicUsage(vector) will tell you how much memory that vector malloced.
197 2016-08-05 20:21:14	0|jonasschnelli|sipa: yes. I'll do that. But i'd like to know how many elements i have to remove from the vector to match a usage target... therefore I need to calculate the size of a single element.
198 2016-08-05 20:21:30	0|sipa|jonasschnelli: use sizeof.
199 2016-08-05 20:21:42	0|sipa|also, removing elements from a vector does not reduce its memory usage.
200 2016-08-05 20:21:49	0|sipa|you need to call shrink_to_fit
201 2016-08-05 20:21:51	0|jonasschnelli|okay... but aren't the element malloced?
202 2016-08-05 20:22:05	0|sipa|yes?
203 2016-08-05 20:22:11	0|sipa|but not individually
204 2016-08-05 20:22:24	0|sipa|a vector has a single allocated block with all its elements in it
205 2016-08-05 20:22:48	0|jonasschnelli|Okay. I'll use sizeof(struct) and good point about shrink_to_fit!
206 2016-08-05 20:22:51	0|jonasschnelli|thanks.
207 2016-08-05 20:27:50	0|jonasschnelli|sipa: sizeof(struct) = 32, memusage::DynamicUsage(vector)/size = 52?!
208 2016-08-05 20:28:34	0|jonasschnelli|calculating the amount of items to remove based on sizeof(struct) is not precise to get the memory usage target
209 2016-08-05 20:28:45	0|sipa|jonasschnelli: look at vector.capacity)(
210 2016-08-05 20:28:51	0|sipa|instead of size
211 2016-08-05 20:29:10	0|sipa|vectors allocate more than what is requested, so that they don't need to reallocate whenever a new element is added
212 2016-08-05 20:29:53	0|sipa|jtimon: i think it's very ugly to have a single set of flags that contains both block level validation rules and script level validation rules
213 2016-08-05 20:30:36	0|jonasschnelli|sipa: /vector.capacity() == sizeof(struct) ... thanks!
214 2016-08-05 20:31:03	0|jtimon|sipa: the script flags are hidden behind ScriptFlagsFromConsensus() I thought that was what you wanted
215 2016-08-05 20:31:17	0|sipa|jtimon: https://github.com/bitcoin/bitcoin/compare/master...jtimon:0.13-consensus-flags?expand=1#diff-cefdf710ea5108806289afadb6cf8717R13
216 2016-08-05 20:32:09	0|jtimon|sipa: so how many sets of flags do we want to expose in libconsensus?
217 2016-08-05 20:32:18	0|sipa|jtimon: one for every layer
218 2016-08-05 20:32:31	0|jtimon|is locktime a layer?
219 2016-08-05 20:32:36	0|sipa|there will be a script validation API and a block validation API, right?
220 2016-08-05 20:32:42	0|sipa|maybe a transaction validation API as well
221 2016-08-05 20:32:55	0|sipa|i think it's wrong to make them share flags
222 2016-08-05 20:33:32	0|sipa|what if there was a script change that only applied to transactions with nVersion>=5... you'd have a block level flag to enable the soft fork, but it wouldn't map one-to-one to the script flag
223 2016-08-05 20:33:35	0|jtimon|maybe a header validation API too, I think that would be the easiest thing to expose and discuss abstractions from storage
224 2016-08-05 20:34:36	0|jtimon|sipa:yeah, now they don't need to map, the non-script consensus flags don't need to be in the internal script flags
225 2016-08-05 20:34:53	0|jtimon|and some internal script flags are not exposed
226 2016-08-05 20:36:12	0|jtimon|I don't undesrtand your txversion>5 example
227 2016-08-05 20:36:31	0|jtimon|the flag would be used at the tx validation level either way, no?
228 2016-08-05 20:37:12	0|jtimon|oh, I see what you mean
229 2016-08-05 20:37:27	0|sipa|say we had a new OP_CHECKSIGAWESOME, but only enabled in UTXOs created by transactions with nVersion >= 5
230 2016-08-05 20:38:04	0|sipa|there would be SCRIPT_VERIFY_CHECKSIGAWESOME, and a BIP9 deployment AWESOME
231 2016-08-05 20:38:07	0|sipa|you'd pass AWESOME to the block validation functions
232 2016-08-05 20:38:26	0|jtimon|yes, you're saying that you would do the consensus-to-internal-script conversion outside of the conversion function and inside, say verifyTx
233 2016-08-05 20:38:45	0|sipa|right... there is not necessarily a clean mapping between the two
234 2016-08-05 20:39:03	0|sipa|what flags the script code is called with is block level logic
235 2016-08-05 20:39:08	0|jtimon|well, what we're exposing in libconsensus are the BIP9/older_sf flags, no?
236 2016-08-05 20:39:25	0|sipa|for the script layer, sure
237 2016-08-05 20:39:29	0|sipa|wait, no
238 2016-08-05 20:39:35	0|jtimon|for all layers, no?
239 2016-08-05 20:39:40	0|sipa|i'm confused :)
240 2016-08-05 20:39:54	0|sipa|i'm assuming there will be a enum for block level validation flags
241 2016-08-05 20:40:05	0|jtimon|the flags we expose in libconsensus right now are a subset of the script internal ones
242 2016-08-05 20:40:15	0|sipa|and an enum (which already exists) for script level validation flags
243 2016-08-05 20:40:31	0|sipa|for some softforks there will be a flag in both
244 2016-08-05 20:40:48	0|jtimon|there's no flag for say bip68 and bip113, because thouse would be needed for verifyTx, but not for verifyScript
245 2016-08-05 20:41:00	0|sipa|right
246 2016-08-05 20:41:49	0|jtimon|well, the "block level" flags include all the "script level" flags, right?
247 2016-08-05 20:42:00	0|sipa|not necessarily
248 2016-08-05 20:42:33	0|jtimon|how do you tell verifyblock to verify p2sh otherwise?
249 2016-08-05 20:43:06	0|jtimon|I mean, verifytx
250 2016-08-05 20:43:38	0|jtimon|verifyblock could call getflags internally I guess, but that's not the point
251 2016-08-05 20:44:54	0|sipa|i'd expect verifyblock to have something like BLOCK_VERIFY_CSV, and verifyscript to have something like SCRIPT_VERIFY_CHECKSEQUENCEVERIFY
252 2016-08-05 20:45:00	0|jtimon|say I want to call verifytx specifying that I want p2sh and bip113 validated
253 2016-08-05 20:45:26	0|sipa|sure, CSV implies that CHECKSEQUENCEVERIFY is passed to script
254 2016-08-05 20:45:30	0|sipa|but CSV does much more than that
255 2016-08-05 20:45:38	0|sipa|it also enabled nsequence behaviour
256 2016-08-05 20:45:43	0|sipa|which has nothing to do with script
257 2016-08-05 20:46:21	0|jtimon|so BLOCK_VERIFY_CSV is mapped into bitcoinconsensus_SCRIPT_FLAGS_VERIFY_P2SH and then bitcoinconsensus_SCRIPT_FLAGS_VERIFY_P2SH into SCRIPT_VERIFY_P2SH
258 2016-08-05 20:46:43	0|jtimon|do we need also a TX_VERIFY_P2SH for exposing verifyTx ?
259 2016-08-05 20:47:35	0|sipa|i'd keep the tx-level flags and block-level flags the same
260 2016-08-05 20:47:36	0|sipa|but script flags seem separate
261 2016-08-05 20:47:52	0|jtimon|well, the flags for BIP68 and for BIP112 are separated in that branch
262 2016-08-05 20:48:09	0|sipa|that's just my opinion, btw... maybe other people feel that script and block flags should be combined
263 2016-08-05 20:48:51	0|jtimon|they are separated: there's "all consensus flags" and "all consensus flags related to script plus some more script specific flags"
264 2016-08-05 20:49:47	0|jtimon|well, from previous talks I thought this was exactly what you wanted, still not sure what would you prefer
265 2016-08-05 20:49:53	0|sipa|yes, i feel they should be completely separate
266 2016-08-05 20:50:00	0|sipa|not one being an extension of the other
267 2016-08-05 20:50:19	0|jtimon|like not repeating p2sh in both of them? I don't undesrtand
268 2016-08-05 20:50:20	0|sipa|but maybe others disagree... that's fine
269 2016-08-05 20:50:32	0|sipa|yes, P2SH would be in both
270 2016-08-05 20:50:36	0|jtimon|in this case one is not an extension of the other, what do you mean?
271 2016-08-05 20:50:45	0|jtimon|they share some (they currently share)
272 2016-08-05 20:51:34	0|sipa|https://github.com/bitcoin/bitcoin/compare/master...jtimon:0.13-consensus-flags?expand=1#diff-cefdf710ea5108806289afadb6cf8717R13
273 2016-08-05 20:51:43	0|sipa|you're just copying the script flags there
274 2016-08-05 20:51:48	0|sipa|and then adding other things
275 2016-08-05 20:51:56	0|sipa|just make it a completely independent enum
276 2016-08-05 20:52:15	0|jtimon|not from the script, see https://github.com/bitcoin/bitcoin/commit/82bf11faf9a1d915eb0fc40ea6db10da9908df2a
277 2016-08-05 20:52:23	0|paveljanik|cfields, thanks for review!
278 2016-08-05 20:52:24	0|jtimon|I'm just adding new non-script flags there
279 2016-08-05 20:52:42	0|sipa|bitcoinconsensus_BIP16, bitcoinconsensus_BIP66, bitcoinconsensus_BIP65, bitcoinconsensus_BIP68, ... for example
280 2016-08-05 20:52:56	0|jtimon|the script ones were already exposed in libconsensus
281 2016-08-05 20:53:11	0|sipa|and BIP16 would map to script verify P2SH
282 2016-08-05 20:53:43	0|jtimon|they already do by hardcoding the same bit positions in both, I'm trying to solve that
283 2016-08-05 20:54:09	0|jtimon|that's what ScriptFlagsFromConsensus() is supposed to solve
284 2016-08-05 20:54:41	0|sipa|well why do you have both LOCKTIME_VERIFY_SEQUENCE and bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKSEQUENCEVERIFY in the same enum?
285 2016-08-05 20:54:52	0|sipa|at the block level they're the same thing
286 2016-08-05 20:56:00	0|jtimon|because I'm not creating a new enum for libconsensus, there's currently one for libconsensus and another for scripts, I'm keeping it that way, just decoupling each other from the bit positions having to match
287 2016-08-05 20:56:41	0|sipa|to verifyblock you would just pass "CSV is enabled", and it would trigger all necessary things... including locktime nsequence checking and script checksequenceverify
288 2016-08-05 20:56:49	0|jtimon|let's say in libconsensus the "block level" flags would be reused for verifyTx and verfiyScript, does that make sense?
289 2016-08-05 20:57:10	0|sipa|i don't know what you mean by that
290 2016-08-05 20:57:44	0|jtimon|I see, you want the block level ones to correspond with deployments, ie bip68 and bip112 go together
291 2016-08-05 20:58:13	0|sipa|well if there is a reason why they would be implemented separately, they can be separate
292 2016-08-05 20:58:46	0|sipa|it's about abstraction... the caller of blockverify shouldn't need to know what every softfork corresponds to internally
293 2016-08-05 20:59:07	0|jtimon|but you don't want verifyScript to receive a CSV_bip68_bip112 flag that it internally maps to SCRIPT_VERIFY_CHECKSEQUENCEVERIFY (ignoring bip68 because at the script level it doesn't care)
294 2016-08-05 20:59:08	0|sipa|that enum you have there feels to me like you want to expose all internal choices to the caller... that isn't necessary
295 2016-08-05 20:59:49	0|sipa|right... have block validation flags that correspond to deployments/softforks, and script validation flags that correspond to script rules
296 2016-08-05 20:59:52	0|jtimon|yeah they can be separated, we could also have a bool param for every option
297 2016-08-05 21:00:10	0|sipa|some block validation flags will correspond to one or more script flags... some may not
298 2016-08-05 21:00:21	0|sipa|but the caller shouldn't need to know that
299 2016-08-05 21:00:57	0|sipa|again... all of that is just my opinion
300 2016-08-05 21:00:58	0|sipa|i'd like to hear some others too
301 2016-08-05 21:01:01	0|jtimon|agree on what you said about the caller, disagree that I want to expose more details than necessary to the caller, I'm happy to merge bip68 and bip112 flags, that seems orthogonal
302 2016-08-05 21:01:20	0|sipa|just the naming of your flags is exposing things :)
303 2016-08-05 21:01:21	0|jtimon|the main question is how many enums do we want and what should it be in them
304 2016-08-05 21:01:44	0|jtimon|oh, come on, I expect people to bike-shedd
305 2016-08-05 21:02:02	0|jtimon|don't tell me that's the main complain
306 2016-08-05 21:02:10	0|sipa|at the block/tx level, you'd have bip16, bip34, bip66, bip65, bip68_112_113, bip141
307 2016-08-05 21:02:12	0|cfields|paveljanik: np. the serialization one will require more effort.
308 2016-08-05 21:02:18	0|sipa|at the script level, you'd have what already exists
309 2016-08-05 21:02:27	0|jtimon|asume you fully decide the names in the enum
310 2016-08-05 21:02:50	0|cfields|paveljanik: imo changing the name for those would be helpful, the version serialization is quite confusing as-is
311 2016-08-05 21:02:52	0|sipa|jtimon: i don't care about the name... i care about the fact that it's mixing several layers!
312 2016-08-05 21:02:54	0|jtimon|why are bip68_112_113 together?
313 2016-08-05 21:03:11	0|jtimon|bip113 was deployed separately
314 2016-08-05 21:03:26	0|jtimon|for the libconsensus caller, all he cares about is deployments, no?
315 2016-08-05 21:03:26	0|sipa|because someone calling verifyblock shouldn't need to know that part of it is a script rule and part of it is not
316 2016-08-05 21:03:55	0|sipa|IMHO, there should not be any flags at all, and you just give it the block headers :)
317 2016-08-05 21:04:03	0|sipa|but i understand we're not there yet
318 2016-08-05 21:04:07	0|jtimon|with my approach all they need to know is that each flag is a deployment
319 2016-08-05 21:04:25	0|sipa|ok, i don't like it, sorry
320 2016-08-05 21:04:28	0|sipa|that's not a NACK
321 2016-08-05 21:04:37	0|sipa|but i'm tired of discussing it
322 2016-08-05 21:04:38	0|jtimon|well, we can expose consensus::getflags
323 2016-08-05 21:05:00	0|jtimon|but are you saying that verifyTx should call getflags internally?
324 2016-08-05 21:05:57	0|paveljanik|cfields, yes. I took me a lot of time to get it done. This was my third attempt to do so.
325 2016-08-05 21:06:12	0|cfields|paveljanik: understood. it's much appreciated.
326 2016-08-05 21:06:32	0|paveljanik|cfields, I have more huge task... LOCK inside LOCK
327 2016-08-05 21:06:33	0|cfields|paveljanik: one high-level request: if the param isn't used, please just leave the var unspecified
328 2016-08-05 21:07:07	0|paveljanik|cfields, I thought about it too (esp. nVersion...) but this would make review more difficult.
329 2016-08-05 21:07:29	0|cfields|paveljanik: otherwise we're not getting any closer to being able to enable -Wunused-parameter :)
330 2016-08-05 21:07:36	0|cfields|paveljanik: how so?
331 2016-08-05 21:08:10	0|paveljanik|every reviewer must read the whole function code...
332 2016-08-05 21:08:53	0|paveljanik|without this change, it is almost enough to read the context
333 2016-08-05 21:10:08	0|cfields|ok, fair enough
334 2016-08-05 21:12:23	0|paveljanik|of course the compilation can find problems there...
335 2016-08-05 21:16:07	0|paveljanik|BTW - LOCK inside LOCK problem: LOCK macro defines a local variable CCriticalBlock criticalblock. And thus LOCK inside LOCK shadows it.
336 2016-08-05 21:16:36	0|paveljanik|I do not want to make the change in the syntax of LOCK, so I have to change the macro.
337 2016-08-05 21:16:40	0|gmaxwell|thats presumably why there are LOCKN macros?
338 2016-08-05 21:17:26	0|paveljanik|yes, but... LOCK2 at the beginning of the function grabs both at the beginning.
339 2016-08-05 21:17:32	0|paveljanik|I do not think this is wanted.
340 2016-08-05 21:18:03	0|paveljanik|I had an idea to change the name of the variable somehow - e.g. criticalblockLINENR or so.
341 2016-08-05 21:18:39	0|paveljanik|but implementing this in the preprocessor's ## ## ## is currently above my knowledge ;-)
342 2016-08-05 21:19:18	0|paveljanik|see e.g. ProcessGetData in main.cpp.
343 2016-08-05 21:19:25	0|sipa|paveljanik: use the file number in the name of the variable :)
344 2016-08-05 21:19:34	0|sipa|eh, line number
345 2016-08-05 21:19:56	0|paveljanik|that was my plan :-) But not so easy to write it 8)
346 2016-08-05 21:20:50	0|sipa|i'll try
347 2016-08-05 21:23:12	0|cfields|can't you just use the incoming param's name?
348 2016-08-05 21:25:10	0|sipa|cfields: what do you if it's called pnode->buf[(int)(sin(x)*35)].cs?
349 2016-08-05 21:25:32	0|NicolasDorier|jtimon: I think we shoudl talk about that in ##libconsensus on my side I'm still not decided whther I prefer the one flag approach of my approach (https://github.com/bitcoin/bitcoin/pull/8339/commits/a59f79dfb7a996e9b309aa43d699499339b1a7c4)
350 2016-08-05 21:26:00	0|jtimon|NicolasDorier: sure we can move there
351 2016-08-05 21:28:06	0|cfields|sipa: heh, ok. that's a stretch, though :)
352 2016-08-05 21:28:37	0|paveljanik|cfields, self-assing: good catch. Look like I do not even understand the original code 8) https://github.com/bitcoin/bitcoin/blob/master/src/net.h#L289
353 2016-08-05 21:30:56	0|sipa|paveljanik:
354 2016-08-05 21:31:13	0|sipa|#define LOCK(cs) CCriticalBlock criticalblock ## __LINE__(cs, #cs, __FILE__, __LINE__)
355 2016-08-05 21:31:16	0|sipa|does that not work?
356 2016-08-05 21:31:51	0|paveljanik|IIRC I have already tried this. Will try again
357 2016-08-05 21:33:23	0|cfields|sipa: (on one line) std::mutex m1; std::mutex m2; LOCK(m1); LOCK(m2);
358 2016-08-05 21:33:45	0|sipa|cfields: ?
359 2016-08-05 21:34:05	0|paveljanik|both variables will be named the same.
360 2016-08-05 21:34:26	0|sipa|ah.
361 2016-08-05 21:34:27	0|paveljanik|CCriticalBlock criticalblock__LINE__(pto->cs_inventory, "pto->cs_inventory", "main.cpp", 6543);
362 2016-08-05 21:34:27	0|paveljanik|main.cpp:6543:28: note: previous declaration is here
363 2016-08-05 21:34:55	0|sipa|ok, that looks wrong
364 2016-08-05 21:34:56	0|cfields|sipa: only because you poked a hole in mine first :)
365 2016-08-05 21:35:15	0|sipa|cfields: well use LOCK2 in that case
366 2016-08-05 21:35:38	0|paveljanik|:-)
367 2016-08-05 21:36:59	0|sipa|paveljanik: bip112
368 2016-08-05 21:37:00	0|sipa|eh
369 2016-08-05 21:37:09	0|sipa|paveljanik: http://stackoverflow.com/a/1597129
370 2016-08-05 21:37:33	0|sipa|__COUNTER__ looks even nicer
371 2016-08-05 21:39:42	0|paveljanik|I'll see.
372 2016-08-05 21:40:02	0|sipa|#define paste1(a,b) a ## b
373 2016-08-05 21:40:08	0|sipa|#define paste(a,b) paste1(a,b)
374 2016-08-05 21:40:42	0|sipa|#define LOCK(cs) CCriticalBlock paste(criticalblock,__COUNTER__)(cs, #cs, __FILE__, __LINE__)
375 2016-08-05 21:40:52	0|paveljanik|#define LOCK(cs) CCriticalBlock TOKENPASTE2(criticalblock, __LINE__)(cs, #cs, __FILE__, __LINE__)
376 2016-08-05 21:40:52	0|paveljanik|#define TOKENPASTE2(x, y) TOKENPASTE(x, y)
377 2016-08-05 21:40:52	0|paveljanik|#define TOKENPASTE(x, y) x ## y
378 2016-08-05 21:40:55	0|paveljanik|seems to work
379 2016-08-05 21:41:07	0|sipa|\o/
380 2016-08-05 21:41:10	0|paveljanik|will try COUNTER here
381 2016-08-05 21:42:25	0|paveljanik|works as well
382 2016-08-05 21:45:09	0|paveljanik|thank you sipa!
383 2016-08-05 21:45:16	0|paveljanik|Looks like I need some sleep 8)
384 2016-08-05 21:46:01	0|sipa|night