12016-08-05T00:00:28 <sipa> sdaftuar, luke-jr, gmaxwell, wumpus: i believe that downgrading from 0.13.1 to 0.13.0 will just work
22016-08-05T00:00:49 <sipa> sdaftuar: even in case of a reorg that takes you back into witness-enabled code
32016-08-05T00:00:55 <sipa> s/code/blocks/
42016-08-05T00:01:16 <luke-jr> well, that's good, I think
52016-08-05T00:01:16 *** Chris_Stewart_5 has quit IRC
62016-08-05T00:01:43 <sipa> the test that no witness data is present in non-witness blocks is only done in AcceptBlock, not in ConnectBlock
72016-08-05T00:02:01 <sipa> so we enforce it when receiving the block from somewhere, but not afterwards
82016-08-05T00:02:06 <sipa> which i think is exactly right
92016-08-05T00:02:44 <gmaxwell> sipa: what happens if you downgrad then upgrade again?
102016-08-05T00:03:33 <sipa> gmaxwell: newly added blocks will be rewinded
112016-08-05T00:15:14 *** fengling has joined #bitcoin-core-dev
122016-08-05T00:20:06 *** fengling has quit IRC
132016-08-05T00:33:32 *** jtimon has quit IRC
142016-08-05T00:41:39 *** TomMc has quit IRC
152016-08-05T00:49:42 <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
162016-08-05T00:50:56 <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
172016-08-05T00:51:52 <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
182016-08-05T00:52:14 <luke-jr> it doesn't serialise any more than without maxsize AFAIK.
192016-08-05T00:52:21 <luke-jr> GetSerializeSize doesn't serialize
202016-08-05T00:54:00 <morcos> ah so its expected that GetSerializeSize is fast? well that explains why anecdotally its not slower
212016-08-05T00:54:01 <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."
222016-08-05T00:54:25 <morcos> luke-jr: i assume you make suggestions like that on purpose
232016-08-05T00:54:25 <luke-jr> morcos: GetSerializeSize is literally just adding the sizes of each field
242016-08-05T00:54:48 <luke-jr> morcos: ?
252016-08-05T00:55:44 <sipa> i expect getserializesize time to be dominated by just memory access of iterating all vectors
262016-08-05T00:56:07 <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
272016-08-05T00:56:33 <luke-jr> morcos: it is clearly an issue today. I am shocked that anyone would disagree.
282016-08-05T00:56:53 <luke-jr> 0.13 will hopefully change that with CB, but it isn't deployed yet.
292016-08-05T00:58:10 <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
302016-08-05T00:58:26 <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"
312016-08-05T00:58:41 <sipa> a small constant factor was never something i opposed for block size. an expectation of it being increased under whatever perceived pressure is
322016-08-05T00:59:00 <luke-jr> morcos: that suggests miners ought not to limit size
332016-08-05T00:59:08 <sipa> i think miners ought not to limit size
342016-08-05T00:59:11 <morcos> something like that, not exactly "as opposed to" b/c that sounds like you're not enforcing the consensus limit
352016-08-05T00:59:19 <sipa> because it's unreasonable that all of them will
362016-08-05T00:59:54 <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.
372016-08-05T00:59:59 <morcos> which is TRUE
382016-08-05T01:00:24 <luke-jr> if they don't care, they should set it based on someone's recommendation, but not necessarily the big blocker recommendation
392016-08-05T01:01:03 <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.
402016-08-05T01:01:22 <sipa> luke-jr: as a collective, perhaps
412016-08-05T01:01:26 <luke-jr> brb
422016-08-05T01:01:41 <sipa> luke-jr: there could be a gentlemen's agreement not to set it to the maximum immediately
432016-08-05T01:01:44 *** jtimon has joined #bitcoin-core-dev
442016-08-05T01:01:56 <sipa> but i believe that's unrealistic at this point
452016-08-05T01:02:29 <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"
462016-08-05T01:02:52 <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?
472016-08-05T01:02:53 <morcos> i'm trying to make it as neutral as possible, while explaining the difference
482016-08-05T01:02:56 <luke-jr> (still brb mode)
492016-08-05T01:03:32 <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
502016-08-05T01:03:54 <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
512016-08-05T01:05:33 <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
522016-08-05T01:05:50 *** Ylbam has quit IRC
532016-08-05T01:05:58 <sipa> morcos: i heard
542016-08-05T01:06:08 <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
552016-08-05T01:06:39 *** fengling has joined #bitcoin-core-dev
562016-08-05T01:06:51 <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
572016-08-05T01:07:10 <morcos> i'm assuming its the fact that their caches have been blown away, but doesn't 5ms seem like a lot?
582016-08-05T01:08:20 <morcos> just wondering if you had any thoughts.. we're still working on it (mostly jeremy, i'm mostly just testing)
592016-08-05T01:09:07 <sipa> morcos: is the slowdown as significant when you have fewer threads?
602016-08-05T01:09:28 <morcos> sadly i didn't test that
612016-08-05T01:10:14 <morcos> yet
622016-08-05T01:11:44 <sipa> and this is 5ms clock time, not cpu time?
632016-08-05T01:13:04 <morcos> yes, 5ms clock time. it appears from debugging that the scriptthreads are just slower to run.
642016-08-05T01:13:53 *** spudowiar has quit IRC
652016-08-05T01:16:42 <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
662016-08-05T01:16:53 <sipa> but i'm not sure whether than makes sense
672016-08-05T01:18:01 <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
682016-08-05T01:22:51 *** belcher has quit IRC
692016-08-05T01:41:58 *** arowser has joined #bitcoin-core-dev
702016-08-05T01:42:33 *** arowser has left #bitcoin-core-dev
712016-08-05T01:43:07 *** arowser has joined #bitcoin-core-dev
722016-08-05T01:57:33 *** Chris_Stewart_5 has joined #bitcoin-core-dev
732016-08-05T02:22:37 *** arowser has quit IRC
742016-08-05T02:28:04 *** Chris_Stewart_5 has quit IRC
752016-08-05T03:09:11 *** Alopex has quit IRC
762016-08-05T03:10:17 *** Alopex has joined #bitcoin-core-dev
772016-08-05T03:10:59 <PatBoy> thx dev for all ur hard work !
782016-08-05T03:11:21 <luke-jr> sipa: I do not believe that opposing segwit is a serious suggestion.
792016-08-05T03:12:40 <luke-jr> morcos: then in the segwit announcement, we can re-evaluate and make recommendations that make sense given the new network conditions
802016-08-05T03:13:11 <luke-jr> (which might or might not include removing blockmaxsize depending on CB effectiveness and real-world testing)
812016-08-05T03:15:08 <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
822016-08-05T03:15:53 <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
832016-08-05T03:16:00 <sipa> *not
842016-08-05T03:16:06 <luke-jr> sipa: we don't know that yet, and our recommendations should always be what is sane even if they get ignored.
852016-08-05T03:18:12 <sipa> luke-jr: that's a reasonable position... but the code is written from a viewpoint that we will get weight-limited block construction
862016-08-05T03:18:28 <sipa> luke-jr: and the release notes should describe the code
872016-08-05T03:19:26 <luke-jr> then the code is broken (sabotaged, it sounds like) and fixing it should be considered a blocker for any release.
882016-08-05T03:19:56 <sipa> if that is your viewpoint, then it is segwit that is sabotaged
892016-08-05T03:20:25 <sipa> i disagree strongly with that
902016-08-05T03:20:45 <luke-jr> sipa: do you disagree strongly with 29000050e575a26b7f7c853cbccdb6bc60ddfdd9 for 0.13?
912016-08-05T03:21:22 <sipa> i think it's pointless
922016-08-05T03:21:45 <sipa> we're doing busy work to avoid stating reality in release notes
932016-08-05T03:21:56 <sipa> the reality is that segwit will mean that blocks go over 1 MB
942016-08-05T03:22:02 <luke-jr> reality is that blockmaxsize should be used for today and the near future
952016-08-05T03:22:11 <sipa> i disagree with that
962016-08-05T03:22:28 <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.
972016-08-05T03:22:36 <sipa> or rather, i would agree with that if there was a reasonable chance that the entire ecosystem will self limit in that way
982016-08-05T03:22:39 <sipa> it won't
992016-08-05T03:22:57 <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.
1002016-08-05T03:23:21 <luke-jr> sipa: the entire ecosystem doesn't need to; why is it all or nothing?
1012016-08-05T03:23:44 <luke-jr> gmaxwell: that's with 1 MB blocks. who knows if it will work out the same with 3 MB
1022016-08-05T03:24:21 <gmaxwell> it especially will work out that way, with CB and relay mitigating orphaning effects.
1032016-08-05T03:24:27 <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.
1042016-08-05T03:24:40 <gmaxwell> luke-jr: and if you want to posit a change in how things work you should suggest how.
1052016-08-05T03:25:08 <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.
1062016-08-05T03:25:17 <sipa> luke-jr: smaller blocks will always be better for propagation relay effects, and larger blocks will always be worse
1072016-08-05T03:25:30 <sipa> luke-jr: there is no 'ready' here; just a compromise that is acceptable
1082016-08-05T03:25:49 <sipa> segwit shifts the relay effects one way, but shifts other incentives the other way
1092016-08-05T03:27:37 <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.
1102016-08-05T03:28:19 <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.
1112016-08-05T03:28:28 <gmaxwell> You're thinking in terms of pixie unicorn miners.
1122016-08-05T03:28:35 <sipa> the only way miners can do something that is good for the network is by making themselves replacable
1132016-08-05T03:28:44 <gmaxwell> Go look at the actual blocks. The miners you're expecting just don't exist at a meaningful level.
1142016-08-05T03:30:19 <sipa> luke-jr: moreover, we're not removing the ability to limit the block size
1152016-08-05T03:31:22 <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
1162016-08-05T03:31:51 <sipa> block size limiting is supported, and we should explain how to use it for those who want to
1172016-08-05T03:32:01 <luke-jr> gmaxwell: yes, pixie unicorn miners who actually exist, despite being ignored by Core
1182016-08-05T03:32:42 <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.
1192016-08-05T03:32:51 <sipa> luke-jr: so, benchmark
1202016-08-05T03:32:52 <luke-jr> if it does, then it's sabotaging miners who use it
1212016-08-05T03:33:25 <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.
1222016-08-05T03:33:42 <sipa> luke-jr: limiting by block size is already taking a cut. fee income will be suboptimal if you limit by size
1232016-08-05T03:33:49 <sipa> luke-jr: that's as much sabotaging
1242016-08-05T03:33:51 <luke-jr> gmaxwell: so that justifies ignoring the requests of such miners, and sabotaging them with (allegedly) bad performance?
1252016-08-05T03:34:15 <sipa> limiting by weight is a design decision which has costs
1262016-08-05T03:34:18 <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?
1272016-08-05T03:34:40 <gmaxwell> This is such bullshit.
1282016-08-05T03:34:40 *** gmaxwell has left #bitcoin-core-dev
1292016-08-05T03:35:46 <luke-jr> ignoring that 0.12 broke numerous things Eligius provided that were left unmerged in Core since 0.6..
1302016-08-05T03:36:22 <sipa> such as?
1312016-08-05T03:36:25 <luke-jr> CPFP
1322016-08-05T03:37:22 <sipa> i never trusted that code enough to merge
1332016-08-05T03:37:25 *** gmaxwell has joined #bitcoin-core-dev
1342016-08-05T03:37:29 <sipa> and i think i was not alone
1352016-08-05T03:37:53 <gmaxwell> I am fed up with this.
1362016-08-05T03:38:09 <luke-jr> same here.
1372016-08-05T03:38:24 <gmaxwell> luke-jr: you are abusive towards me and the other contributors.
1382016-08-05T03:38:30 <gmaxwell> you are obsessing over minutia on top of minutia.
1392016-08-05T03:38:48 <gmaxwell> You are wasting countless hours exhausting all patience.
1402016-08-05T03:39:54 <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.
1412016-08-05T03:40:55 <gmaxwell> it litterally makes no difference in the world, and yet you force people to spend hours and hours debating these things.
1422016-08-05T03:41:28 <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.
1432016-08-05T03:45:27 <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.
1442016-08-05T03:47:48 <gmaxwell> When it does actually include transactions-- it appears to produce maximum size blocks just like everyone else: https://blockchain.info/block/0000000000000000050631b932ed81eb9de6267f1cb8b8a353dd59365fd07fd5
1452016-08-05T04:53:06 *** fengling has quit IRC
1462016-08-05T05:22:23 *** jtimon has quit IRC
1472016-08-05T05:30:20 *** fengling has joined #bitcoin-core-dev
1482016-08-05T05:42:35 *** d_t has joined #bitcoin-core-dev
1492016-08-05T05:49:26 *** fengling has quit IRC
1502016-08-05T06:17:23 *** zooko has joined #bitcoin-core-dev
1512016-08-05T06:19:42 *** gabridome has joined #bitcoin-core-dev
1522016-08-05T06:27:19 *** randy-waterhouse has quit IRC
1532016-08-05T06:29:33 *** fengling has joined #bitcoin-core-dev
1542016-08-05T06:32:14 *** gabridome has quit IRC
1552016-08-05T06:33:15 *** Silence_ has joined #bitcoin-core-dev
1562016-08-05T06:35:20 *** d_t has quit IRC
1572016-08-05T06:35:29 *** d_t has joined #bitcoin-core-dev
1582016-08-05T06:45:03 *** BashCo has quit IRC
1592016-08-05T06:45:54 *** zooko has quit IRC
1602016-08-05T07:05:48 *** Cory has quit IRC
1612016-08-05T07:06:39 *** Pasha has joined #bitcoin-core-dev
1622016-08-05T07:13:33 *** Pasha is now known as Cory
1632016-08-05T07:14:04 *** laurentmt has joined #bitcoin-core-dev
1642016-08-05T07:16:09 *** BashCo has joined #bitcoin-core-dev
1652016-08-05T07:18:13 *** LaudaM has quit IRC
1662016-08-05T07:31:11 *** Guyver2 has joined #bitcoin-core-dev
1672016-08-05T07:38:28 *** jgarzik has quit IRC
1682016-08-05T07:43:27 *** d_t has quit IRC
1692016-08-05T07:50:30 *** Giszmo has joined #bitcoin-core-dev
1702016-08-05T07:53:02 *** Giszmo has quit IRC
1712016-08-05T07:59:27 *** Evel-Knievel has quit IRC
1722016-08-05T08:01:34 *** Evel-Knievel has joined #bitcoin-core-dev
1732016-08-05T08:20:12 *** Guyver2 has quit IRC
1742016-08-05T08:23:45 *** kadoban has quit IRC
1752016-08-05T08:39:07 *** jtimon has joined #bitcoin-core-dev
1762016-08-05T08:40:23 *** rubensayshi has joined #bitcoin-core-dev
1772016-08-05T08:40:50 *** rubensayshi has quit IRC
1782016-08-05T08:41:38 *** rubensayshi has joined #bitcoin-core-dev
1792016-08-05T09:02:05 *** spudowiar has joined #bitcoin-core-dev
1802016-08-05T09:09:55 *** Ylbam has joined #bitcoin-core-dev
1812016-08-05T09:30:16 *** jtimon has quit IRC
1822016-08-05T09:37:57 *** shesek has joined #bitcoin-core-dev
1832016-08-05T10:40:02 <jonasschnelli> I think "memusage.h" should include "prevector.h"
1842016-08-05T10:46:31 *** arubi_ has joined #bitcoin-core-dev
1852016-08-05T10:50:15 *** arubi has quit IRC
1862016-08-05T10:55:26 *** fengling has quit IRC
1872016-08-05T10:56:08 <GitHub83> [bitcoin] jonasschnelli closed pull request #7865: [RPC] Add bumpfee command. (master...2016/04/rbf_combined) https://github.com/bitcoin/bitcoin/pull/7865
1882016-08-05T11:03:43 *** arubi__ has joined #bitcoin-core-dev
1892016-08-05T11:07:03 *** arubi_ has quit IRC
1902016-08-05T11:23:29 *** AaronvanW has joined #bitcoin-core-dev
1912016-08-05T11:25:50 *** Ylbam has quit IRC
1922016-08-05T11:33:36 *** spudowiar has quit IRC
1932016-08-05T11:37:02 *** Alopex has quit IRC
1942016-08-05T11:38:07 *** Alopex has joined #bitcoin-core-dev
1952016-08-05T12:03:25 *** Lauda has quit IRC
1962016-08-05T12:03:25 *** Lauda has joined #bitcoin-core-dev
1972016-08-05T12:11:32 *** cryptapus_afk is now known as cryptapus
1982016-08-05T12:22:31 <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
1992016-08-05T12:22:45 <wumpus> I'm a little rusty on the exact rules there though
2002016-08-05T12:23:16 <jonasschnelli> wumpus: I just added some stats stuff and included memusage.h, compiler was then complaining about the missing type prevector...
2012016-08-05T12:24:02 <jonasschnelli> memusage.h is directly using (in a template but directly) prevector
2022016-08-05T12:24:23 <jonasschnelli> But anyways... far away from important. :)
2032016-08-05T12:24:38 <wumpus> does the data structure that you're trying to measure include a prevector
2042016-08-05T12:24:41 <wumpus> ?
2052016-08-05T12:25:16 <jonasschnelli> no. I just want to measure a vector of structs... but mempool.h refers to prevector
2062016-08-05T12:25:39 <jonasschnelli> I guess all other places where mempool.h is included, prevector was previously already included
2072016-08-05T12:26:05 <wumpus> there is no mempool.h? I guess you mean memusage.h?
2082016-08-05T12:26:19 <jonasschnelli> argm. memusage.h
2092016-08-05T12:26:28 <wumpus> indeed, if including memusage.h forces you to include prevector.h then it should be included there
2102016-08-05T12:26:38 <jonasschnelli> static inline size_t DynamicUsage(const prevector<N, X, S, D>& v
2112016-08-05T12:27:03 <jonasschnelli> It's a nitpick and I'll try to cover that fix in one of my stats releated commits.
2122016-08-05T12:28:07 <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
2132016-08-05T12:28:12 <wumpus> right
2142016-08-05T12:29:00 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2152016-08-05T12:29:31 <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 :)
2162016-08-05T13:04:37 *** Chris_Stewart_5 has quit IRC
2172016-08-05T13:08:11 *** BashCo has quit IRC
2182016-08-05T13:08:36 *** BashCo has joined #bitcoin-core-dev
2192016-08-05T13:19:55 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2202016-08-05T13:33:58 *** laurentmt1 has joined #bitcoin-core-dev
2212016-08-05T13:35:52 *** laurentmt has quit IRC
2222016-08-05T13:35:52 *** laurentmt1 is now known as laurentmt
2232016-08-05T13:47:05 *** TomMc has joined #bitcoin-core-dev
2242016-08-05T13:48:27 *** arubi_ has joined #bitcoin-core-dev
2252016-08-05T13:49:27 *** Chris_Stewart_5 has quit IRC
2262016-08-05T13:51:52 *** arubi__ has quit IRC
2272016-08-05T13:55:10 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2282016-08-05T14:19:56 *** arubi__ has joined #bitcoin-core-dev
2292016-08-05T14:22:30 *** spudowiar has joined #bitcoin-core-dev
2302016-08-05T14:23:18 *** arubi_ has quit IRC
2312016-08-05T14:54:11 *** Chris_Stewart_5 has quit IRC
2322016-08-05T14:58:48 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2332016-08-05T15:00:52 *** d_t has joined #bitcoin-core-dev
2342016-08-05T15:03:31 *** Ylbam has joined #bitcoin-core-dev
2352016-08-05T15:06:06 *** jtimon has joined #bitcoin-core-dev
2362016-08-05T15:21:22 *** zooko has joined #bitcoin-core-dev
2372016-08-05T15:29:50 *** mmortal03 has joined #bitcoin-core-dev
2382016-08-05T15:39:09 *** BashCo has quit IRC
2392016-08-05T15:45:10 *** Chris_Stewart_5 has quit IRC
2402016-08-05T15:52:35 *** laurentmt has quit IRC
2412016-08-05T16:04:29 <GitHub89> [bitcoin] Mirobit closed pull request #8283: Move AdvertiseLocal debug output to net category (master...master) https://github.com/bitcoin/bitcoin/pull/8283
2422016-08-05T16:09:13 *** Sosumi has quit IRC
2432016-08-05T16:12:44 *** rubensayshi has quit IRC
2442016-08-05T16:14:41 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2452016-08-05T16:15:35 *** d_t has quit IRC
2462016-08-05T16:23:46 *** Chris_Stewart_5 has quit IRC
2472016-08-05T16:36:43 *** laurentmt has joined #bitcoin-core-dev
2482016-08-05T16:37:31 *** laurentmt has quit IRC
2492016-08-05T16:38:21 <GitHub188> [bitcoin] Mirobit opened pull request #8462: Move AdvertiseLocal debug output to net category (master...advertiselocal) https://github.com/bitcoin/bitcoin/pull/8462
2502016-08-05T16:51:03 *** kvnn has joined #bitcoin-core-dev
2512016-08-05T16:51:06 <kvnn> Can anyone tell me how much disk space testnet currenty takes?
2522016-08-05T17:04:39 <GitHub149> [bitcoin] MarcoFalke opened pull request #8463: [qt] Remove Priority from coincontrol dialog (master...Mf1608-qtPrio) https://github.com/bitcoin/bitcoin/pull/8463
2532016-08-05T17:13:31 *** AaronvanW has quit IRC
2542016-08-05T17:16:37 <GitHub47> [bitcoin] JeremyRubin opened pull request #8464: "Lockfree" Checkqueue Implementation (master...lockfree-checkqueue-squashed) https://github.com/bitcoin/bitcoin/pull/8464
2552016-08-05T17:19:02 *** AaronvanW has joined #bitcoin-core-dev
2562016-08-05T17:19:03 *** AaronvanW has quit IRC
2572016-08-05T17:19:03 *** AaronvanW has joined #bitcoin-core-dev
2582016-08-05T17:19:17 <jeremyrubin> (forgot to mark as WIP fyi)
2592016-08-05T17:21:15 <btcdrak> you can edit the title
2602016-08-05T17:21:26 <btcdrak> jeremyrubin
2612016-08-05T17:22:42 <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
2622016-08-05T17:27:43 <jeremyrubin> btcdrak: what are the tasks?
2632016-08-05T17:27:59 <btcdrak> your todo list.
2642016-08-05T17:28:00 <jeremyrubin> btcdrak: the only stuff really remaining is more testing.
2652016-08-05T17:28:27 <jeremyrubin> btcdrak: actually I can think of two things
2662016-08-05T17:28:40 <sipa> jeremyrubin: if all that remains is testing, i wouldn't call it WIP :)
2672016-08-05T17:30:14 <btcdrak> sipa: exactly! if it's marked WIP no-one will test it.
2682016-08-05T17:31:46 <jeremyrubin> sipa: oh ok... I'll un WIP it then, it's ready for testing
2692016-08-05T17:33:03 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2702016-08-05T17:33:05 <jeremyrubin> sorry was unclear at what point I should WIP/vs non WIP, thanks for the guidance
2712016-08-05T17:35:05 <jeremyrubin> I'm going to be awk for a little bit, but will respond to more things later.
2722016-08-05T17:35:18 <jeremyrubin> sipa: you may just want to view the whole diff rather than commit by commit
2732016-08-05T17:41:36 *** zooko` has joined #bitcoin-core-dev
2742016-08-05T17:42:51 *** zooko has quit IRC
2752016-08-05T17:42:55 <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
2762016-08-05T17:48:46 *** kadoban has joined #bitcoin-core-dev
2772016-08-05T17:52:33 <jeremyrubin> yep I agree, each commit should compile and run on this PR but is maybe not fully documented ;)
2782016-08-05T17:53:42 *** Greybits has joined #bitcoin-core-dev
2792016-08-05T17:54:03 *** [Author] has quit IRC
2802016-08-05T18:07:33 *** [Author] has joined #bitcoin-core-dev
2812016-08-05T18:09:27 *** Chris_Stewart_5 has quit IRC
2822016-08-05T18:09:33 *** mmortal03 has quit IRC
2832016-08-05T18:12:08 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2842016-08-05T18:16:24 *** arubi__ is now known as arubi
2852016-08-05T18:24:56 *** kvnn has quit IRC
2862016-08-05T18:38:58 <bsm117532> wizkid057, the exploding bitcoin spammer, somehow got op permissions on #bitcoin-wizards and is banning people. Can someone here take care of him?
2872016-08-05T18:41:27 <jonasschnelli> jeremyrubin: impressive PR... my review will take a while. :)
2882016-08-05T18:42:28 *** Guyver2 has joined #bitcoin-core-dev
2892016-08-05T18:44:35 <pigeons> he's not banning people, just kicked you only and its not the spammer it really is wizkid. probably just a mistake
2902016-08-05T18:45:45 <bsm117532> sorry, don't know who wizkid is.
2912016-08-05T19:03:17 <GitHub178> [bitcoin] sipa opened pull request #8465: [0.13] Document reindexing changes (0.13...docreindex) https://github.com/bitcoin/bitcoin/pull/8465
2922016-08-05T19:07:43 <GitHub185> [bitcoin] paveljanik opened pull request #8466: [Trivial] Do not shadow variables in networking code (master...20160805_Wshadow_net) https://github.com/bitcoin/bitcoin/pull/8466
2932016-08-05T19:17:09 <sipa> i feel that 'New mempool information RPC calls' could move to 'Low-level RPC changes' in the release notes
2942016-08-05T19:17:14 <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?
2952016-08-05T19:18:00 <jtimon> this doesn't touch the script flags and doesn't move anything out of contextualCheckBlock like the previous one
2962016-08-05T19:18:41 <jtimon> it could be 1 or a few commits, but I kept it extremely separated just in case
2972016-08-05T19:22:00 <GitHub153> [bitcoin] paveljanik opened pull request #8467: [Trivial] Do not shadow members in dbwrapper (master...20160805_Wshadow_dbwrapper) https://github.com/bitcoin/bitcoin/pull/8467
2982016-08-05T19:22:51 *** belcher has joined #bitcoin-core-dev
2992016-08-05T19:32:07 <sdaftuar> sipa: ack
3002016-08-05T19:33:36 <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
3012016-08-05T19:36:14 <sipa> maybe a section that aggregates all non-user-visible code changes
3022016-08-05T19:36:25 <sipa> that would even include segwit, i guess
3032016-08-05T19:42:39 <sdaftuar> sure. fyi the mining stuff references segwit, so if you move those around relative to each other, the text might need some edits
3042016-08-05T19:55:04 <GitHub14> [bitcoin] paveljanik opened pull request #8468: Do not shadow member variables in serialization (master...20160805_Wshadow_serialization) https://github.com/bitcoin/bitcoin/pull/8468
3052016-08-05T20:04:47 *** zooko`` has joined #bitcoin-core-dev
3062016-08-05T20:06:13 *** zooko` has quit IRC
3072016-08-05T20:13:11 <Chris_Stewart_5> Is this entire block of code inside of bloom.cpp basically so that we can match redeemScript/pubkey hashes in scriptPubKeys?
3082016-08-05T20:13:14 <Chris_Stewart_5> https://github.com/bitcoin/bitcoin/blob/master/src/bloom.cpp#L163-LL177
3092016-08-05T20:16:34 <jonasschnelli> sipa: how do I get the DynamicUsage from a struct?
3102016-08-05T20:16:43 <jonasschnelli> MallocUsage(size_t alloc) is inline...
3112016-08-05T20:17:13 <sipa> jonasschnelli: sum the dynamic usages of its member fields
3122016-08-05T20:18:45 <jonasschnelli> sipa: what about MallocUsage(sizeof(mystruct))?
3132016-08-05T20:18:47 <jonasschnelli> +allow external calls to MallocUsage
3142016-08-05T20:19:04 <sipa> jonasschnelli: are you mallocing your stuct?
3152016-08-05T20:19:41 <jonasschnelli> sipa: ... ah... no.
3162016-08-05T20:19:55 <sipa> then you shouldn't be using either of them
3172016-08-05T20:20:04 <jonasschnelli> I guess std::vector<mystruct> is on the stack
3182016-08-05T20:20:17 <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.
3192016-08-05T20:20:36 <sipa> jonasschnelli: use DynamicUsage on the vector.
3202016-08-05T20:20:51 <sipa> The vector is allocated on the stack. Its entries are allocated on the heap.
3212016-08-05T20:21:07 <sipa> DynamicUsage(vector) will tell you how much memory that vector malloced.
3222016-08-05T20:21:13 <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.
3232016-08-05T20:21:30 <sipa> jonasschnelli: use sizeof.
3242016-08-05T20:21:42 <sipa> also, removing elements from a vector does not reduce its memory usage.
3252016-08-05T20:21:48 <sipa> you need to call shrink_to_fit
3262016-08-05T20:21:50 <jonasschnelli> okay... but aren't the element malloced?
3272016-08-05T20:22:05 <sipa> yes?
3282016-08-05T20:22:11 <sipa> but not individually
3292016-08-05T20:22:23 <sipa> a vector has a single allocated block with all its elements in it
3302016-08-05T20:22:47 <jonasschnelli> Okay. I'll use sizeof(struct) and good point about shrink_to_fit!
3312016-08-05T20:22:50 <jonasschnelli> thanks.
3322016-08-05T20:27:50 <jonasschnelli> sipa: sizeof(struct) = 32, memusage::DynamicUsage(vector)/size = 52?!
3332016-08-05T20:28:34 <jonasschnelli> calculating the amount of items to remove based on sizeof(struct) is not precise to get the memory usage target
3342016-08-05T20:28:44 <sipa> jonasschnelli: look at vector.capacity)(
3352016-08-05T20:28:51 <sipa> instead of size
3362016-08-05T20:29:10 <sipa> vectors allocate more than what is requested, so that they don't need to reallocate whenever a new element is added
3372016-08-05T20:29:52 <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
3382016-08-05T20:30:35 <jonasschnelli> sipa: /vector.capacity() == sizeof(struct) ... thanks!
3392016-08-05T20:31:03 <jtimon> sipa: the script flags are hidden behind ScriptFlagsFromConsensus() I thought that was what you wanted
3402016-08-05T20:31:17 <sipa> jtimon: https://github.com/bitcoin/bitcoin/compare/master...jtimon:0.13-consensus-flags?expand=1#diff-cefdf710ea5108806289afadb6cf8717R13
3412016-08-05T20:32:09 <jtimon> sipa: so how many sets of flags do we want to expose in libconsensus?
3422016-08-05T20:32:18 <sipa> jtimon: one for every layer
3432016-08-05T20:32:31 <jtimon> is locktime a layer?
3442016-08-05T20:32:35 <sipa> there will be a script validation API and a block validation API, right?
3452016-08-05T20:32:42 <sipa> maybe a transaction validation API as well
3462016-08-05T20:32:55 <sipa> i think it's wrong to make them share flags
3472016-08-05T20:33:32 <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
3482016-08-05T20:33:35 <jtimon> maybe a header validation API too, I think that would be the easiest thing to expose and discuss abstractions from storage
3492016-08-05T20:34:36 <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
3502016-08-05T20:34:53 <jtimon> and some internal script flags are not exposed
3512016-08-05T20:36:12 <jtimon> I don't undesrtand your txversion>5 example
3522016-08-05T20:36:31 <jtimon> the flag would be used at the tx validation level either way, no?
3532016-08-05T20:37:11 <jtimon> oh, I see what you mean
3542016-08-05T20:37:27 <sipa> say we had a new OP_CHECKSIGAWESOME, but only enabled in UTXOs created by transactions with nVersion >= 5
3552016-08-05T20:38:04 <sipa> there would be SCRIPT_VERIFY_CHECKSIGAWESOME, and a BIP9 deployment AWESOME
3562016-08-05T20:38:07 <sipa> you'd pass AWESOME to the block validation functions
3572016-08-05T20:38:25 <jtimon> yes, you're saying that you would do the consensus-to-internal-script conversion outside of the conversion function and inside, say verifyTx
3582016-08-05T20:38:45 <sipa> right... there is not necessarily a clean mapping between the two
3592016-08-05T20:39:02 <sipa> what flags the script code is called with is block level logic
3602016-08-05T20:39:07 <jtimon> well, what we're exposing in libconsensus are the BIP9/older_sf flags, no?
3612016-08-05T20:39:24 <sipa> for the script layer, sure
3622016-08-05T20:39:28 <sipa> wait, no
3632016-08-05T20:39:34 <jtimon> for all layers, no?
3642016-08-05T20:39:35 *** Chris_Stewart_5 has quit IRC
3652016-08-05T20:39:39 <sipa> i'm confused :)
3662016-08-05T20:39:54 <sipa> i'm assuming there will be a enum for block level validation flags
3672016-08-05T20:40:04 <jtimon> the flags we expose in libconsensus right now are a subset of the script internal ones
3682016-08-05T20:40:15 <sipa> and an enum (which already exists) for script level validation flags
3692016-08-05T20:40:30 <sipa> for some softforks there will be a flag in both
3702016-08-05T20:40:48 <jtimon> there's no flag for say bip68 and bip113, because thouse would be needed for verifyTx, but not for verifyScript
3712016-08-05T20:41:00 <sipa> right
3722016-08-05T20:41:49 <jtimon> well, the "block level" flags include all the "script level" flags, right?
3732016-08-05T20:42:00 <sipa> not necessarily
3742016-08-05T20:42:32 <jtimon> how do you tell verifyblock to verify p2sh otherwise?
3752016-08-05T20:43:06 <jtimon> I mean, verifytx
3762016-08-05T20:43:38 <jtimon> verifyblock could call getflags internally I guess, but that's not the point
3772016-08-05T20:44:53 <sipa> i'd expect verifyblock to have something like BLOCK_VERIFY_CSV, and verifyscript to have something like SCRIPT_VERIFY_CHECKSEQUENCEVERIFY
3782016-08-05T20:45:00 <jtimon> say I want to call verifytx specifying that I want p2sh and bip113 validated
3792016-08-05T20:45:26 <sipa> sure, CSV implies that CHECKSEQUENCEVERIFY is passed to script
3802016-08-05T20:45:30 <sipa> but CSV does much more than that
3812016-08-05T20:45:37 <sipa> it also enabled nsequence behaviour
3822016-08-05T20:45:43 <sipa> which has nothing to do with script
3832016-08-05T20:46:20 <jtimon> so BLOCK_VERIFY_CSV is mapped into bitcoinconsensus_SCRIPT_FLAGS_VERIFY_P2SH and then bitcoinconsensus_SCRIPT_FLAGS_VERIFY_P2SH into SCRIPT_VERIFY_P2SH
3842016-08-05T20:46:43 <jtimon> do we need also a TX_VERIFY_P2SH for exposing verifyTx ?
3852016-08-05T20:47:34 <sipa> i'd keep the tx-level flags and block-level flags the same
3862016-08-05T20:47:36 <sipa> but script flags seem separate
3872016-08-05T20:47:52 <jtimon> well, the flags for BIP68 and for BIP112 are separated in that branch
3882016-08-05T20:48:09 <sipa> that's just my opinion, btw... maybe other people feel that script and block flags should be combined
3892016-08-05T20:48:51 <jtimon> they are separated: there's "all consensus flags" and "all consensus flags related to script plus some more script specific flags"
3902016-08-05T20:49:47 <jtimon> well, from previous talks I thought this was exactly what you wanted, still not sure what would you prefer
3912016-08-05T20:49:53 <sipa> yes, i feel they should be completely separate
3922016-08-05T20:49:59 <sipa> not one being an extension of the other
3932016-08-05T20:50:18 <jtimon> like not repeating p2sh in both of them? I don't undesrtand
3942016-08-05T20:50:20 <sipa> but maybe others disagree... that's fine
3952016-08-05T20:50:31 <sipa> yes, P2SH would be in both
3962016-08-05T20:50:35 <jtimon> in this case one is not an extension of the other, what do you mean?
3972016-08-05T20:50:45 <jtimon> they share some (they currently share)
3982016-08-05T20:51:34 <sipa> https://github.com/bitcoin/bitcoin/compare/master...jtimon:0.13-consensus-flags?expand=1#diff-cefdf710ea5108806289afadb6cf8717R13
3992016-08-05T20:51:43 <sipa> you're just copying the script flags there
4002016-08-05T20:51:48 <sipa> and then adding other things
4012016-08-05T20:51:56 <sipa> just make it a completely independent enum
4022016-08-05T20:52:14 <jtimon> not from the script, see https://github.com/bitcoin/bitcoin/commit/82bf11faf9a1d915eb0fc40ea6db10da9908df2a
4032016-08-05T20:52:22 <paveljanik> cfields, thanks for review!
4042016-08-05T20:52:24 <jtimon> I'm just adding new non-script flags there
4052016-08-05T20:52:42 <sipa> bitcoinconsensus_BIP16, bitcoinconsensus_BIP66, bitcoinconsensus_BIP65, bitcoinconsensus_BIP68, ... for example
4062016-08-05T20:52:55 <jtimon> the script ones were already exposed in libconsensus
4072016-08-05T20:53:11 <sipa> and BIP16 would map to script verify P2SH
4082016-08-05T20:53:42 <jtimon> they already do by hardcoding the same bit positions in both, I'm trying to solve that
4092016-08-05T20:54:09 <jtimon> that's what ScriptFlagsFromConsensus() is supposed to solve
4102016-08-05T20:54:40 <sipa> well why do you have both LOCKTIME_VERIFY_SEQUENCE and bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKSEQUENCEVERIFY in the same enum?
4112016-08-05T20:54:41 *** Chris_Stewart_5 has joined #bitcoin-core-dev
4122016-08-05T20:54:52 <sipa> at the block level they're the same thing
4132016-08-05T20:56:00 <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
4142016-08-05T20:56:40 <sipa> to verifyblock you would just pass "CSV is enabled", and it would trigger all necessary things... including locktime nsequence checking and script checksequenceverify
4152016-08-05T20:56:49 <jtimon> let's say in libconsensus the "block level" flags would be reused for verifyTx and verfiyScript, does that make sense?
4162016-08-05T20:57:09 <sipa> i don't know what you mean by that
4172016-08-05T20:57:44 <jtimon> I see, you want the block level ones to correspond with deployments, ie bip68 and bip112 go together
4182016-08-05T20:58:13 <sipa> well if there is a reason why they would be implemented separately, they can be separate
4192016-08-05T20:58:46 <sipa> it's about abstraction... the caller of blockverify shouldn't need to know what every softfork corresponds to internally
4202016-08-05T20:59:07 <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)
4212016-08-05T20:59:07 <sipa> that enum you have there feels to me like you want to expose all internal choices to the caller... that isn't necessary
4222016-08-05T20:59:49 <sipa> right... have block validation flags that correspond to deployments/softforks, and script validation flags that correspond to script rules
4232016-08-05T20:59:51 <jtimon> yeah they can be separated, we could also have a bool param for every option
4242016-08-05T21:00:09 <sipa> some block validation flags will correspond to one or more script flags... some may not
4252016-08-05T21:00:20 <sipa> but the caller shouldn't need to know that
4262016-08-05T21:00:56 <sipa> again... all of that is just my opinion
4272016-08-05T21:00:58 <sipa> i'd like to hear some others too
4282016-08-05T21:01:01 <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
4292016-08-05T21:01:20 <sipa> just the naming of your flags is exposing things :)
4302016-08-05T21:01:21 <jtimon> the main question is how many enums do we want and what should it be in them
4312016-08-05T21:01:44 <jtimon> oh, come on, I expect people to bike-shedd
4322016-08-05T21:02:02 <jtimon> don't tell me that's the main complain
4332016-08-05T21:02:10 <sipa> at the block/tx level, you'd have bip16, bip34, bip66, bip65, bip68_112_113, bip141
4342016-08-05T21:02:12 <cfields> paveljanik: np. the serialization one will require more effort.
4352016-08-05T21:02:18 <sipa> at the script level, you'd have what already exists
4362016-08-05T21:02:26 <jtimon> asume you fully decide the names in the enum
4372016-08-05T21:02:50 <cfields> paveljanik: imo changing the name for those would be helpful, the version serialization is quite confusing as-is
4382016-08-05T21:02:51 <sipa> jtimon: i don't care about the name... i care about the fact that it's mixing several layers!
4392016-08-05T21:02:53 <jtimon> why are bip68_112_113 together?
4402016-08-05T21:03:10 <jtimon> bip113 was deployed separately
4412016-08-05T21:03:25 <sipa> because someone calling verifyblock shouldn't need to know that part of it is a script rule and part of it is not
4422016-08-05T21:03:26 <jtimon> for the libconsensus caller, all he cares about is deployments, no?
4432016-08-05T21:03:55 <sipa> IMHO, there should not be any flags at all, and you just give it the block headers :)
4442016-08-05T21:04:02 <sipa> but i understand we're not there yet
4452016-08-05T21:04:07 <jtimon> with my approach all they need to know is that each flag is a deployment
4462016-08-05T21:04:25 <sipa> ok, i don't like it, sorry
4472016-08-05T21:04:28 <sipa> that's not a NACK
4482016-08-05T21:04:37 <sipa> but i'm tired of discussing it
4492016-08-05T21:04:38 <jtimon> well, we can expose consensus::getflags
4502016-08-05T21:04:45 *** sipa has left #bitcoin-core-dev
4512016-08-05T21:05:00 <jtimon> but are you saying that verifyTx should call getflags internally?
4522016-08-05T21:05:50 *** Ylbam has quit IRC
4532016-08-05T21:05:57 <paveljanik> cfields, yes. I took me a lot of time to get it done. This was my third attempt to do so.
4542016-08-05T21:06:11 <cfields> paveljanik: understood. it's much appreciated.
4552016-08-05T21:06:32 <paveljanik> cfields, I have more huge task... LOCK inside LOCK
4562016-08-05T21:06:33 <cfields> paveljanik: one high-level request: if the param isn't used, please just leave the var unspecified
4572016-08-05T21:06:43 *** sipa has joined #bitcoin-core-dev
4582016-08-05T21:07:07 <paveljanik> cfields, I thought about it too (esp. nVersion...) but this would make review more difficult.
4592016-08-05T21:07:28 <cfields> paveljanik: otherwise we're not getting any closer to being able to enable -Wunused-parameter :)
4602016-08-05T21:07:35 <cfields> paveljanik: how so?
4612016-08-05T21:08:10 <paveljanik> every reviewer must read the whole function code...
4622016-08-05T21:08:53 <paveljanik> without this change, it is almost enough to read the context
4632016-08-05T21:10:08 <cfields> ok, fair enough
4642016-08-05T21:10:41 *** molly has joined #bitcoin-core-dev
4652016-08-05T21:12:23 <paveljanik> of course the compilation can find problems there...
4662016-08-05T21:13:40 *** molz has quit IRC
4672016-08-05T21:14:16 *** english- has joined #bitcoin-core-dev
4682016-08-05T21:16:06 <paveljanik> BTW - LOCK inside LOCK problem: LOCK macro defines a local variable CCriticalBlock criticalblock. And thus LOCK inside LOCK shadows it.
4692016-08-05T21:16:36 <paveljanik> I do not want to make the change in the syntax of LOCK, so I have to change the macro.
4702016-08-05T21:16:39 <gmaxwell> thats presumably why there are LOCKN macros?
4712016-08-05T21:17:26 <paveljanik> yes, but... LOCK2 at the beginning of the function grabs both at the beginning.
4722016-08-05T21:17:32 <paveljanik> I do not think this is wanted.
4732016-08-05T21:18:03 <paveljanik> I had an idea to change the name of the variable somehow - e.g. criticalblockLINENR or so.
4742016-08-05T21:18:39 <paveljanik> but implementing this in the preprocessor's ## ## ## is currently above my knowledge ;-)
4752016-08-05T21:19:18 <paveljanik> see e.g. ProcessGetData in main.cpp.
4762016-08-05T21:19:24 <sipa> paveljanik: use the file number in the name of the variable :)
4772016-08-05T21:19:33 <sipa> eh, line number
4782016-08-05T21:19:56 <paveljanik> that was my plan :-) But not so easy to write it 8)
4792016-08-05T21:20:49 <sipa> i'll try
4802016-08-05T21:23:11 <cfields> can't you just use the incoming param's name?
4812016-08-05T21:25:09 <sipa> cfields: what do you if it's called pnode->buf[(int)(sin(x)*35)].cs?
4822016-08-05T21:25:32 <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)
4832016-08-05T21:25:59 <jtimon> NicolasDorier: sure we can move there
4842016-08-05T21:28:05 <cfields> sipa: heh, ok. that's a stretch, though :)
4852016-08-05T21:28:36 <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
4862016-08-05T21:28:52 *** BashCo has joined #bitcoin-core-dev
4872016-08-05T21:30:55 <sipa> paveljanik:
4882016-08-05T21:31:12 <sipa> #define LOCK(cs) CCriticalBlock criticalblock ## __LINE__(cs, #cs, __FILE__, __LINE__)
4892016-08-05T21:31:15 <sipa> does that not work?
4902016-08-05T21:31:50 *** jannes has quit IRC
4912016-08-05T21:31:51 <paveljanik> IIRC I have already tried this. Will try again
4922016-08-05T21:33:21 *** d_t has joined #bitcoin-core-dev
4932016-08-05T21:33:22 <cfields> sipa: (on one line) std::mutex m1; std::mutex m2; LOCK(m1); LOCK(m2);
4942016-08-05T21:33:45 <sipa> cfields: ?
4952016-08-05T21:34:05 <paveljanik> both variables will be named the same.
4962016-08-05T21:34:26 <sipa> ah.
4972016-08-05T21:34:27 <paveljanik> main.cpp:6543:28: note: previous declaration is here
4982016-08-05T21:34:27 <paveljanik> CCriticalBlock criticalblock__LINE__(pto->cs_inventory, "pto->cs_inventory", "main.cpp", 6543);
4992016-08-05T21:34:55 <sipa> ok, that looks wrong
5002016-08-05T21:34:55 <cfields> sipa: only because you poked a hole in mine first :)
5012016-08-05T21:35:15 <sipa> cfields: well use LOCK2 in that case
5022016-08-05T21:35:38 <paveljanik> :-)
5032016-08-05T21:36:59 <sipa> paveljanik: bip112
5042016-08-05T21:37:00 <sipa> eh
5052016-08-05T21:37:09 <sipa> paveljanik: http://stackoverflow.com/a/1597129
5062016-08-05T21:37:33 <sipa> __COUNTER__ looks even nicer
5072016-08-05T21:39:41 <paveljanik> I'll see.
5082016-08-05T21:40:02 <sipa> #define paste1(a,b) a ## b
5092016-08-05T21:40:08 <sipa> #define paste(a,b) paste1(a,b)
5102016-08-05T21:40:42 <sipa> #define LOCK(cs) CCriticalBlock paste(criticalblock,__COUNTER__)(cs, #cs, __FILE__, __LINE__)
5112016-08-05T21:40:51 <paveljanik> #define TOKENPASTE(x, y) x ## y
5122016-08-05T21:40:52 <paveljanik> #define TOKENPASTE2(x, y) TOKENPASTE(x, y)
5132016-08-05T21:40:52 <paveljanik> #define LOCK(cs) CCriticalBlock TOKENPASTE2(criticalblock, __LINE__)(cs, #cs, __FILE__, __LINE__)
5142016-08-05T21:40:55 <paveljanik> seems to work
5152016-08-05T21:41:07 <sipa> \o/
5162016-08-05T21:41:09 <paveljanik> will try COUNTER here
5172016-08-05T21:42:25 <paveljanik> works as well
5182016-08-05T21:43:06 *** tunafizz has joined #bitcoin-core-dev
5192016-08-05T21:45:09 <paveljanik> thank you sipa!
5202016-08-05T21:45:16 <paveljanik> Looks like I need some sleep 8)
5212016-08-05T21:46:01 <sipa> night
5222016-08-05T21:47:10 *** zooko`` has quit IRC
5232016-08-05T21:50:10 *** english- has quit IRC
5242016-08-05T21:51:32 *** Ylbam has joined #bitcoin-core-dev
5252016-08-05T22:02:14 *** spudowiar has quit IRC
5262016-08-05T22:03:41 *** spudowiar has joined #bitcoin-core-dev
5272016-08-05T22:07:53 *** spudowiar has quit IRC
5282016-08-05T22:12:58 *** spudowiar has joined #bitcoin-core-dev
5292016-08-05T22:21:19 *** dvsdude has joined #bitcoin-core-dev
5302016-08-05T22:28:51 *** Guyver2 has quit IRC
5312016-08-05T22:30:45 *** spudowiar has quit IRC
5322016-08-05T22:31:44 *** cryptapus is now known as cryptapus_afk
5332016-08-05T22:33:10 *** spudowiar has joined #bitcoin-core-dev
5342016-08-05T22:39:32 *** spudowiar has quit IRC
5352016-08-05T22:40:24 *** spudowiar has joined #bitcoin-core-dev
5362016-08-05T22:43:00 *** jgarzik has joined #bitcoin-core-dev
5372016-08-05T22:51:01 *** dvsdude has left #bitcoin-core-dev
5382016-08-05T22:52:40 *** jgarzik has quit IRC
5392016-08-05T22:54:23 *** jgarzik has joined #bitcoin-core-dev
5402016-08-05T22:54:23 *** jgarzik has joined #bitcoin-core-dev
5412016-08-05T22:58:37 *** cryptapus_afk is now known as cryptapus
5422016-08-05T22:59:01 *** Alopex has quit IRC
5432016-08-05T23:00:07 *** Alopex has joined #bitcoin-core-dev
5442016-08-05T23:00:40 *** TomMc has quit IRC
5452016-08-05T23:05:05 *** cryptapus is now known as cryptapus_afk
5462016-08-05T23:24:37 *** d_t has quit IRC
5472016-08-05T23:28:26 *** randy-waterhouse has joined #bitcoin-core-dev
5482016-08-05T23:28:49 *** goregrind has quit IRC
5492016-08-05T23:51:36 *** randy-waterhouse has quit IRC
5502016-08-05T23:51:37 *** Chris_Stewart_5 has quit IRC
5512016-08-05T23:53:59 *** Chris_Stewart_5 has joined #bitcoin-core-dev