1 2016-08-24 03:30:01	0|GitHub117|[13bitcoin] 15nomnombtc opened pull request #8568: new var DIST_CONTRIB adds useful things for packagers from contrib (06master...06DIST_CONTRIB) 02https://github.com/bitcoin/bitcoin/pull/8568
  2 2016-08-24 05:43:01	0|midnightmagic|Lots of people getting into the gitian builds. Surprising number, really. A lot of people are taking action as a result of that warning.
  3 2016-08-24 05:47:03	0|luke-jr|midnightmagic: it makes me more confident Bitcoin in general can gain user awareness and security needed to succeed
  4 2016-08-24 05:48:17	0|midnightmagic|This is one of those things: lots of people want to help but either don't know how or can't think of what *they* can do to help.
  5 2016-08-24 05:51:44	0|gmaxwell|There are only 200 or so publically reachable nodes running 0.13. ... something else people can do. :) upgrade their nodes.
  6 2016-08-24 05:53:50	0|luke-jr|gmaxwell: I actually moved most of my hot wallet funds out today in preparation for upgrading mine (and hopefully being able to run it 24/7 again)
  7 2016-08-24 06:54:26	0|GitHub121|[13bitcoin] 15rebroad opened pull request #8571: Don't disconnect just because a node's services have changed. (06master...06UnexpectedServicesNoDisconnect) 02https://github.com/bitcoin/bitcoin/pull/8571
  8 2016-08-24 07:08:29	0|GitHub21|[13bitcoin] 15rebroad opened pull request #8572: Don't effectively blacklist pruned nodes. (06master...06Don'tBanPrunedNodes) 02https://github.com/bitcoin/bitcoin/pull/8572
  9 2016-08-24 07:08:47	0|GitHub193|[13bitcoin] 15jonasschnelli opened pull request #8573: Set jonasschnellis dns-seeder filter flag (06master...062016/08/filter_seed) 02https://github.com/bitcoin/bitcoin/pull/8573
 10 2016-08-24 07:15:23	0|wumpus|midnightmagic: yes, lots of gitian builders this time around, this is great
 11 2016-08-24 07:15:39	0|wumpus|(although 0.12.0 had a lot too)
 12 2016-08-24 07:16:54	0|sipa|14 for 0.13.0-linux
 13 2016-08-24 07:17:00	0|sipa|i think that's a record
 14 2016-08-24 07:17:54	0|sipa|oh, no, 15 for 0.12.0-linux
 15 2016-08-24 08:15:35	0|GitHub21|[13bitcoin] 15jonasschnelli opened pull request #8574: [Wallet] refactor CWallet/CWalletDB/CDB (06master...062016/08/bdb_abstraction_2) 02https://github.com/bitcoin/bitcoin/pull/8574
 16 2016-08-24 08:16:03	0|wumpus|sipa: though those accumulated over a longer time
 17 2016-08-24 08:16:14	0|wumpus|sipa: very likely 0.13.0 will still be a record
 18 2016-08-24 08:16:26	0|sipa|yeah
 19 2016-08-24 08:21:30	0|jonasschnelli|The CWallet/CWalletDB/CDB interleaving is really a mess. I fear nobody will review the just opened refactoring PR. They are much larger then I had originally expected.
 20 2016-08-24 08:22:18	0|jonasschnelli|But if we want to add support for a different wallet database format, we need better abstraction of wallet logic, wallet database logic and pure database storage logic
 21 2016-08-24 08:22:55	0|jonasschnelli|Somehow I think we need to do this before we add Multi Wallet Support,...
 22 2016-08-24 08:24:10	0|wumpus|agree that some refactoring of the wallet code is long due
 23 2016-08-24 08:24:23	0|sipa|jonasschnelli: will have a look
 24 2016-08-24 08:24:24	0|wumpus|but yes, assuring that it doesnt introduce nasty bugs or funcitonality people are relying on is hard
 25 2016-08-24 08:24:33	0|wumpus|+break
 26 2016-08-24 08:24:52	0|wumpus|which reminds me we still haven't add the label API to replace the accounts API
 27 2016-08-24 08:25:11	0|jonasschnelli|Yes. I try to move the BDB code carefully and keep its behavior (even if it silly sometimes).
 28 2016-08-24 08:25:20	0|wumpus|that makes sense for a refactor
 29 2016-08-24 08:25:21	0|jonasschnelli|wumpus: We should have a look at your PR
 30 2016-08-24 08:25:59	0|jonasschnelli|BTW: travis has failed on walletbackup.py randomly, the PR does not change anything there IMO: https://travis-ci.org/bitcoin/bitcoin/jobs/154141125#L1464
 31 2016-08-24 08:26:11	0|jonasschnelli|Will re-trigger travis on that PR
 32 2016-08-24 08:26:11	0|wumpus|yes, the code is pretty much outdated and useless at this point, but we should make sure at least the API is what we want
 33 2016-08-24 08:26:21	0|gmaxwell|FWIW, phantomcircuit spent basically months with a bunch of wallet refactors pipelined waiting for changes to go in.
 34 2016-08-24 08:27:27	0|gmaxwell|If people turn in broken up changes, they get randomly stuck waiting, presumably because few care to review things that don't seem important.  If people submit large all at once changes, they get stuck because no one is able to review big changes.
 35 2016-08-24 08:27:46	0|wumpus|this is especially true for the wallet
 36 2016-08-24 08:28:04	0|jonasschnelli|Indeed. Maybe I should split up the PR in smaller chunks..
 37 2016-08-24 08:28:04	0|wumpus|and, perversely, the refactors would probably make it easier to review changes in the future
 38 2016-08-24 08:28:29	0|jonasschnelli|There is a very small PR for those we care: :) https://github.com/bitcoin/bitcoin/pull/8564
 39 2016-08-24 08:28:44	0|wumpus|but quite some people get a headache just for looking at the wallet code as it is now, which prevents them from reviewing it, and thus from those (necessary) changes from getting in
 40 2016-08-24 08:28:48	0|gmaxwell|what I'm saying is that phantomcircuit did split up his changes into chunks, submitted the first one, then it sat for ages.
 41 2016-08-24 08:29:32	0|wumpus|yes, that's how it has always gone with wallet changes, even CodeShark's multiwallet PR back in the day sat for ages, without useful review or testing
 42 2016-08-24 08:29:33	0|gmaxwell|He had more pipelined, but I don't know if he gave up, or if its just because he's been sick that he hasn't PRed more of them since the longest standing went in just recently.
 43 2016-08-24 08:30:03	0|gmaxwell|:)
 44 2016-08-24 08:30:03	0|gmaxwell|probably the latter, he's not prone to giving up.
 45 2016-08-24 08:30:38	0|wumpus|it's just difficult to do, I don't have any useful advice for anyone trying unfortunately, as you say both breaking the change up as well as doing a lot at the same time run against barreirs
 46 2016-08-24 08:30:57	0|wumpus|changing the consensus code seems easier :)
 47 2016-08-24 08:31:12	0|jonasschnelli|I think https://github.com/bitcoin/bitcoin/pull/8445 is ready for merge
 48 2016-08-24 08:31:22	0|jonasschnelli|(one of Patricks)
 49 2016-08-24 08:31:38	0|wumpus|let's merge it then
 50 2016-08-24 08:33:37	0|wumpus|maybe Patrick?
 51 2016-08-24 08:33:45	0|GitHub147|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/9358893518a1...f9167003d947
 52 2016-08-24 08:33:46	0|GitHub147|13bitcoin/06master 148680d3a 15Patrick Strateman: Move wallet initialization logic from AppInit2 to CWallet::InitLoadWallet
 53 2016-08-24 08:33:46	0|GitHub147|13bitcoin/06master 14e86eb71 15Patrick Strateman: Move CWallet::setKeyPool to private section of CWallet
 54 2016-08-24 08:33:47	0|GitHub147|13bitcoin/06master 14f916700 15Wladimir J. van der Laan: Merge #8445: Move CWallet::setKeyPool to private section of CWallet....
 55 2016-08-24 08:33:55	0|GitHub83|[13bitcoin] 15laanwj closed pull request #8445: Move CWallet::setKeyPool to private section of CWallet. (06master...062016-07-01-cwallet-api-cleanup) 02https://github.com/bitcoin/bitcoin/pull/8445
 56 2016-08-24 08:35:51	0|jonasschnelli|I'm happy to co-maintain the wallet. I think I can now understand most of its code (also the silly one)
 57 2016-08-24 08:37:01	0|wumpus|thanks, you're very welcome to participate in wallet changes, but I think it'd be good to bring someone else on board
 58 2016-08-24 08:37:21	0|wumpus|otherwise you end up with everything on your plate
 59 2016-08-24 08:37:44	0|jonasschnelli|Yes. Agree.
 60 2016-08-24 08:38:22	0|murch|What responsibilities does being the wallet maintainer entail?
 61 2016-08-24 08:38:58	0|wumpus|work on the wallet, do a lot of work on the wallet, and review other people's work on the wallet
 62 2016-08-24 08:39:04	0|jonasschnelli|Review PRs, make sure we make progress with the wallet
 63 2016-08-24 08:39:22	0|wumpus|and have a very strong clue about the crypto and how the parts interact
 64 2016-08-24 08:41:04	0|wumpus|and doesn't get dizzyness and/or headaches or bouts of panic while reading the current wallet code, or changes to it
 65 2016-08-24 08:41:46	0|sipa|i'd love to spend time on making bitcoin core function as an SPV client, and make the wallet changes that are necessary
 66 2016-08-24 08:41:52	0|wumpus|(as I do)
 67 2016-08-24 08:42:03	0|sipa|but i think i have a bit too many other things to do first
 68 2016-08-24 08:42:11	0|wumpus|yes, I think you have enough on your plate too
 69 2016-08-24 08:42:13	0|murch|Unfortunately, I'm not really familiar with the crypto used in the wallet. I mean, I know in broad strokes what it is supposed to do, but I'd be lying if I said I have a strong clue.
 70 2016-08-24 08:42:17	0|wumpus|and you're already maintainer too
 71 2016-08-24 08:42:29	0|jonasschnelli|gmaxwell: "cat dnsseed.dump | grep 00000005 | grep "    1   " | wc -l" = 2590
 72 2016-08-24 08:42:33	0|GitHub15|13bitcoin/06master 147bd5ff4 15Christian Barcenas: Trivial: Fix two VarInt examples in serialize.h
 73 2016-08-24 08:42:33	0|GitHub15|[13bitcoin] 15sipa pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/f9167003d947...f12d2b5a8ac3
 74 2016-08-24 08:42:34	0|GitHub15|13bitcoin/06master 14f12d2b5 15Pieter Wuille: Merge #8560: Trivial: Fix two VarInt examples in serialize.h...
 75 2016-08-24 08:42:37	0|sipa|yes, i'm not volunteering for the wallet maintainer position... just saying i'd like to work more on it :)
 76 2016-08-24 08:42:43	0|GitHub188|[13bitcoin] 15sipa closed pull request #8560: Trivial: Fix two VarInt examples in serialize.h (06master...06fix_varint_examples) 02https://github.com/bitcoin/bitcoin/pull/8560
 77 2016-08-24 08:42:52	0|jonasschnelli|I guess there are plenty of node signaling NODE_BLOOM, though not sure if they are really offering the service
 78 2016-08-24 08:43:11	0|sipa|can i have ACK on 8451?
 79 2016-08-24 08:43:24	0|jonasschnelli|sipa: Not sure if the SPV mode would be utterly complex.
 80 2016-08-24 08:43:43	0|sipa|jonasschnelli: it won't be, but there are a few subtle changes to make
 81 2016-08-24 08:43:50	0|jonasschnelli|A first step would be to allow the wallet to run in mode without mempool knownlage
 82 2016-08-24 08:44:44	0|jonasschnelli|Then a way how the wallet can "ask for blocks",... main.cpp needs to download them, check headers (SPV pure PoW check) and passes the txes through the SyncWithWallet signal
 83 2016-08-24 08:44:45	0|wumpus|I don't like #8451 very much, it seems a step the wrong way around
 84 2016-08-24 08:45:32	0|gmaxwell|using undefined behavior isn't acceptable however.
 85 2016-08-24 08:45:42	0|jonasschnelli|sipa: what are the benefits of 8451?
 86 2016-08-24 08:45:58	0|wumpus|no, it's not, but that dosn't mean I have to like the solution
 87 2016-08-24 08:46:18	0|sipa|jonasschnelli: fixing our code, which is currently undefined (as in: the compiler is allowed to make it fire all the nukes)
 88 2016-08-24 08:46:25	0|sipa|wumpus: understood, and agree
 89 2016-08-24 08:46:45	0|wumpus|I really like the idea of having a transaction-set-ins-stone object, as well as a transaction builder (ImmutableTransaction). We should have that for more things, such as Script
 90 2016-08-24 08:46:51	0|sipa|but i think a better solution is too far off (modifying the serializing framework to support constructing-while-deserializing)
 91 2016-08-24 08:47:02	0|sipa|or encapsulating all fields in CTransaction
 92 2016-08-24 08:47:10	0|wumpus|eh s/ImmutableTransaction/MutableTransaction
 93 2016-08-24 08:47:33	0|wumpus|it separates building and checking at aconceptual level
 94 2016-08-24 08:47:56	0|gmaxwell|wumpus: by builder, do you imagine that a transaction would always be built in one atomic step?
 95 2016-08-24 08:48:13	0|sipa|hmm, maybe there is a possibility to change all the places where CTransaction is deserialized to first deserialize into a CMutableTransaction, and then convert to a CTransaction?
 96 2016-08-24 08:48:20	0|sipa|there may not be very many of those
 97 2016-08-24 08:48:34	0|wumpus|gmaxwell: no. Not necessarily, but it is a  "MutableTransaction" scaffold until it no longer needs to be one
 98 2016-08-24 08:48:41	0|gmaxwell|gotcha.
 99 2016-08-24 08:48:54	0|sipa|let's try.
100 2016-08-24 08:49:06	0|gmaxwell|Thats really what I thought the intent was there in the past.
101 2016-08-24 08:49:17	0|wumpus|sipa: sounds quite sensible
102 2016-08-24 08:49:29	0|wumpus|the serialization framework *needs* a mutable object,so give it one
103 2016-08-24 08:49:33	0|gmaxwell|(in particular making CTransaction always fully immutable allows changing its representation to an efficient one)
104 2016-08-24 08:49:39	0|wumpus|gmaxwell: exactly!
105 2016-08-24 08:49:55	0|sipa|gmaxwell: it requires more than that though
106 2016-08-24 08:50:01	0|sipa|(also encapsulating its fields)
107 2016-08-24 08:50:10	0|gmaxwell|I know.
108 2016-08-24 08:50:15	0|wumpus|sipa: sure, it's only one step
109 2016-08-24 08:50:18	0|gmaxwell|(as in one malloc per CTransaction, and only one-- oh to dream)
110 2016-08-24 08:50:33	0|wumpus|that's what I meant with that making Transaction mutable is a step in the wrong direction
111 2016-08-24 08:50:47	0|wumpus|just a step, not armageddon
112 2016-08-24 08:51:22	0|gmaxwell|well, I think in C++ we shouldn't equat const/non-constness with immutability. They're partially orthorgonal concepts.
113 2016-08-24 08:51:27	0|gmaxwell|er equate*
114 2016-08-24 08:51:41	0|wumpus|I agree, though const helps enforce that at the compiler level
115 2016-08-24 08:51:53	0|sipa|ugh.
116 2016-08-24 08:52:06	0|gmaxwell|oh no, sipa found something awful, I can tell.
117 2016-08-24 08:52:09	0|sipa|no more CTransaction::operator= or CTransaction::swap
118 2016-08-24 08:52:09	0|wumpus|uh oh
119 2016-08-24 08:52:34	0|wumpus|hmm. atomic replacement is ungood too?
120 2016-08-24 08:53:03	0|wumpus|when you regard CTransaction as a reference to a transaction, those may be acceptable
121 2016-08-24 08:53:25	0|wumpus|otherwise, yes, it's mutating
122 2016-08-24 08:55:57	0|wumpus|though the future 'CTransaction needs one malloc' would imply it has one pointer, which could trivially be swapped with another one
123 2016-08-24 08:56:24	0|wumpus|assignment would still need copying, or a shared pointer
124 2016-08-24 08:56:51	0|gmaxwell|wumpus: well no, because a single malloced chunk could have pointers internally... so you can't say it implies it alone
125 2016-08-24 08:57:01	0|gmaxwell|though obviously it would be better if it didn't. :)
126 2016-08-24 08:57:30	0|wumpus|gmaxwell: oh, sure, then you'd need to do some more accounting
127 2016-08-24 08:57:54	0|wumpus|I don't mean 'atomic' in the threading sense
128 2016-08-24 08:58:14	0|gmaxwell|I was reading 'trivially swapped' meaning not needing any deep operations.
129 2016-08-24 08:58:27	0|wumpus|it doesn't need any deep operations, just swap the surface pointers
130 2016-08-24 08:58:39	0|wumpus|if there are more, pointing inside the structure, well swap all of them
131 2016-08-24 09:00:34	0|wumpus|but if you have all pointers pointing inside the stucture as part of the malloc'ed structure itself you can easily keep it to one pointer
132 2016-08-24 09:01:01	0|wumpus|depends on how much indirection is a performance bottleneck I guess...
133 2016-08-24 09:04:03	0|gmaxwell|wumpus: fyi there was a thread on bitcointalk recently where someone profiled bitcoin core, and was asking why we weren't using faster sha2 because it was 35% in their benchmarks. I pointed them to your work with the SIMD sha2.  They seemed to think that there would be 10x speedups, so I was sad to have to disappoint them. :)
134 2016-08-24 09:05:48	0|wumpus|gmaxwell: yeah it was disappointing :( would still make some sense to integrate that, for a bit of speedup, but the overall gains were so little it demotivated me at least
135 2016-08-24 09:06:10	0|gmaxwell|they were clearly measurable at least.
136 2016-08-24 09:06:36	0|wumpus|but if anyone wants to continue that work they're welcome to, of course
137 2016-08-24 09:06:40	0|gmaxwell|Thats something to be proud of, even if they aren't huge. They'll become more important as everything else gets optimized.
138 2016-08-24 09:07:13	0|wumpus|true
139 2016-08-24 09:08:11	0|wumpus|which reminds me of #8524
140 2016-08-24 09:08:30	0|gmaxwell|he also commented on the leveldb crc
141 2016-08-24 09:08:57	0|wumpus|hah  :)
142 2016-08-24 09:09:50	0|MarcoFalke|jonasschnelli: Can you explain?
143 2016-08-24 09:10:03	0|MarcoFalke|we don't have auto stretch in master
144 2016-08-24 09:10:08	0|MarcoFalke|I think
145 2016-08-24 09:10:55	0|jonasschnelli|MarcoFalke: let me look at the code..
146 2016-08-24 09:11:15	0|jonasschnelli|It just looks strange (part of the row don't use full width)
147 2016-08-24 09:11:24	0|jonasschnelli|And I think its avoidable
148 2016-08-24 09:11:33	0|wumpus|did that once, too  https://github.com/laanwj/bitcoin/commit/431c1b987b34589f32f4c2d0ee0f2571ba70e349  I don't really know where that went, looks like I never did any high-level performance measurements, only benchmark of the CRC function itself
149 2016-08-24 09:12:45	0|MarcoFalke|ok, let me check...
150 2016-08-24 09:12:47	0|jonasschnelli|MarcoFalke: https://github.com/bitcoin/bitcoin/pull/8463/files#r76021780
151 2016-08-24 09:13:19	0|wumpus|but the CRC instruction is 7-8 times faster than leveldb's crc implementatino on Haswell: https://github.com/laanwj/crcbench
152 2016-08-24 09:14:28	0|jonasschnelli|MarcoFalke: 84+100+170+290+110+100+100 = 954, window width = 1000px, some padding
153 2016-08-24 09:14:31	0|wumpus|and it works, I verified it's the same ariant of CRC (crc32c) and has the same output
154 2016-08-24 09:15:41	0|wumpus|what is lacking/to be done: CPU detection at startup, replace those functions by function pointers, based on the best implementation for detected CPU
155 2016-08-24 09:16:43	0|wumpus|in the case of leveldb maybe trying to upstream it
156 2016-08-24 09:17:04	0|jonasschnelli|wumpus: nice work!
157 2016-08-24 09:17:34	0|wumpus|thanks :)
158 2016-08-24 09:17:37	0|jonasschnelli|Not sure hoe active upstream is
159 2016-08-24 09:17:58	0|wumpus|agreed, that's why that's a 'maybe', normally it'd be a requirement
160 2016-08-24 09:18:04	0|jonasschnelli|heh... yes.
161 2016-08-24 09:20:06	0|wumpus|https://github.com/google/leveldb: last update 13 days ago - it's less dead than univalue's upstream at least
162 2016-08-24 09:20:40	0|jonasschnelli|Indeed. :-) 3xpings from paveljanik
163 2016-08-24 09:21:08	0|jonasschnelli|But UniValue upstream is now bitcoin-core/univalue, right?
164 2016-08-24 09:21:28	0|wumpus|though I think all of recent leveldb changes have been OS-support changes, not so much consequential database performance changes, but I haven't looked very recently
165 2016-08-24 09:21:43	0|MarcoFalke|No, I think we keep bitcoin-core/stuff only for bitcoin-core related patches
166 2016-08-24 09:21:44	0|wumpus|(or correctness fixes for that matter)
167 2016-08-24 09:22:13	0|wumpus|jonasschnelli: well, effectively, for bitcoin it is. I think we're the only client for univalue anyhow.
168 2016-08-24 09:22:50	0|jonasschnelli|Digital Bitbox also depends on UniValue. But thats it I guess.
169 2016-08-24 09:23:13	0|jonasschnelli|IMO if jeff is not maintaining it, we should take care about bitcoin-core/UniValue
170 2016-08-24 09:23:29	0|paveljanik|first, we should ask jeff.
171 2016-08-24 09:23:36	0|wumpus|I tend to agree, though I can't spend much time being maintainer of yet another project
172 2016-08-24 09:23:47	0|MarcoFalke|pff, it is always a mess to have several repos on github and no one knows whcih one is maintained
173 2016-08-24 09:24:12	0|wumpus|bitcoin-core's is maintained, in the sense that it will always be the best version for our use
174 2016-08-24 09:25:02	0|wumpus|well I know only one thing for sure: Jeff's isn't
175 2016-08-24 09:25:16	0|wumpus|I'll merge patches when necessary for bitcoin or general performance/correctness
176 2016-08-24 09:25:58	0|wumpus|which reminds me I still need to cean up my univalue fuzzing framework some time
177 2016-08-24 09:26:48	0|wumpus|(and do the necessary build system changes, analogous to Patrick's https://github.com/bitcoin/bitcoin/pull/7940)
178 2016-08-24 09:27:39	0|gmaxwell|I don't know if andytoshi published his test harness.  He rewrote a functional equivilent to univale in rust, then wrote a test harness that compared the two against each other and found a number of bugs in univale (though I believe they were all already bugs wumpus had found).
179 2016-08-24 09:27:52	0|gmaxwell|I think thats a really really useful testing technique for a parser.
180 2016-08-24 09:28:36	0|wumpus|jonasschnelli: in any case univalue is a good choice when dealing with bitcoind client-side from C or C++, I don't think using it is a bad chocie, maybe it just needs very little maintenance
181 2016-08-24 09:28:51	0|wumpus|gmaxwell: yes having two implementations and comparing them is a good way of testing
182 2016-08-24 09:38:09	0|wumpus|cfields_: we should start working at replacing boost::chrono too :) https://github.com/zcash/zcash/issues/1241
183 2016-08-24 09:38:39	0|gmaxwell|I got cancer from just looking at that backtrace.
184 2016-08-24 09:39:13	0|gmaxwell|I've run core in valgrind quite a bit and not seen those errors.
185 2016-08-24 09:39:51	0|gmaxwell|not that I doubt them.
186 2016-08-24 09:44:45	0|paveljanik|Can anyone please explain to me, what is the purpose of this line? https://github.com/bitcoin/bitcoin/blob/master/src/qt/recentrequeststablemodel.h#L34?
187 2016-08-24 09:55:28	0|wumpus|gmaxwell: boost is so difficult to debug, with all the c++ template magic
188 2016-08-24 09:55:49	0|wumpus|gruesomely overdesigned, even for something like time getting/conversion
189 2016-08-24 09:56:22	0|wumpus|paveljanik: I think it is spurious, don't take my word for it though
190 2016-08-24 09:57:30	0|jonasschnelli|paveljanik: agree with wumpus
191 2016-08-24 09:57:50	0|jonasschnelli|How could such code end up here in the first place. :-)
192 2016-08-24 09:58:30	0|paveljanik|I think it was cut&pasted...
193 2016-08-24 09:58:45	0|paveljanik|the same line is present 6 times in our sources...
194 2016-08-24 09:59:02	0|wumpus|it makes sense to have (de)serialization code there, as those entries are (de)serialized
195 2016-08-24 09:59:57	0|wumpus|paveljanik: but the big question is, does gcc even generate code for them :D
196 2016-08-24 10:00:27	0|wumpus|it would only make sense if there is a local veriable nVersion
197 2016-08-24 10:00:42	0|GitHub158|[13bitcoin] 15ajtowns opened pull request #8575: leveldb: generate lib independent of locale sort (06master...06leveldb-locale-reproducible) 02https://github.com/bitcoin/bitcoin/pull/8575
198 2016-08-24 10:00:46	0|wumpus|which there isn't, so it'sa self-assign
199 2016-08-24 10:01:17	0|wumpus|EH. Well there is, one of the arguments
200 2016-08-24 10:01:24	0|paveljanik|yes 8)
201 2016-08-24 10:02:14	0|wumpus|so take care that if nVersion is used after that line, you can't remove the line (without changing the use of nVersion to this->nVersion)
202 2016-08-24 10:02:18	0|paveljanik|and this is where I'm lost ;-)
203 2016-08-24 10:02:42	0|wumpus|having the structure field have the same name as a parameter is unadvisable here, as it causes confustion
204 2016-08-24 10:02:49	0|wumpus|confusion*
205 2016-08-24 10:03:15	0|wumpus|I think I get it
206 2016-08-24 10:03:21	0|paveljanik|nVersion is not used at all after this line in all 6 cases.
207 2016-08-24 10:03:22	0|wumpus|the same function is used for reading and writing
208 2016-08-24 10:03:46	0|wumpus|on writing, this->nVersion is written to the serialization
209 2016-08-24 10:03:57	0|wumpus|on reading, this->nVersion is read from the serialization, then assigned to nVersion
210 2016-08-24 10:04:33	0|wumpus|but why? nVersion argument to Serialization is of a different sort than nVersion used internally
211 2016-08-24 10:04:43	0|wumpus|.?!?!?? core dump
212 2016-08-24 10:05:06	0|wumpus|I know I just copied that code, I can't have written it :)
213 2016-08-24 10:05:28	0|MarcoFalke|The code is copied from the initial git commit
214 2016-08-24 10:05:31	0|paveljanik|we have to ask the "TwoSpaces" man ;-)
215 2016-08-24 10:05:35	0|MarcoFalke|ask satoshi or something
216 2016-08-24 10:05:43	0|wumpus|I'd be very careful with touching it though or the whole pyramid may come down
217 2016-08-24 10:05:46	0|wumpus|hah yes
218 2016-08-24 10:05:55	0|paveljanik|yes, definitely
219 2016-08-24 10:07:08	0|wumpus|this is why I wondered whether gcc generates code for it in the first place
220 2016-08-24 10:07:44	0|wumpus|"-*- sipa tries compiling after making CTransaction deserializable-at-construct-time", that was his last comment and then he went silent, ominous
221 2016-08-24 10:07:54	0|gmaxwell|haha
222 2016-08-24 10:09:07	0|phantomcircuit|gmaxwell: im waiting on 8450 before making more changes
223 2016-08-24 10:10:07	0|phantomcircuit|the next one is to change the accounting tests, but that needs to actually change the tests themselves
224 2016-08-24 10:10:10	0|wumpus|are you waiting on that one? let's merge it, it's a test change and two acks already
225 2016-08-24 10:10:12	0|phantomcircuit|so it's a bit more involved
226 2016-08-24 10:10:26	0|phantomcircuit|(basically im going to be removing like 50% of the tests since they're nonsensical)
227 2016-08-24 10:11:03	0|GitHub65|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/f12d2b5a8ac3...21857d2bf746
228 2016-08-24 10:11:04	0|GitHub65|13bitcoin/06master 1425400c4 15Patrick Strateman: Account wallet feature RPC tests.
229 2016-08-24 10:11:04	0|GitHub65|13bitcoin/06master 149578333 15Patrick Strateman: Remove rpc_wallet_tests.cpp
230 2016-08-24 10:11:05	0|GitHub65|13bitcoin/06master 1421857d2 15Wladimir J. van der Laan: Merge #8450: [Test] Replace rpc_wallet_tests.cpp with python RPC unit tests...
231 2016-08-24 10:11:08	0|GitHub112|[13bitcoin] 15laanwj closed pull request #8450: [Test] Replace rpc_wallet_tests.cpp with python RPC unit tests (06master...062016-08-03-remove-rpc-wallet-tests) 02https://github.com/bitcoin/bitcoin/pull/8450
232 2016-08-24 10:11:08	0|phantomcircuit|wumpus: yeah i was waiting
233 2016-08-24 10:11:34	0|phantomcircuit|great there's now only a single thing external to CWallet using CWalletDB which is the accounting_tests.cpp
234 2016-08-24 10:11:38	0|phantomcircuit|which mostly make no sense
235 2016-08-24 10:11:51	0|wumpus|why does it use CWalletDB?
236 2016-08-24 10:11:57	0|wumpus|or maybe better not to ask, sorry
237 2016-08-24 10:12:08	0|wumpus|I'm sure accounting tests can be done with just a CWallet object
238 2016-08-24 10:12:33	0|phantomcircuit|indeed better not to ask (it's adding the accounting entries directly...)
239 2016-08-24 10:14:26	0|phantomcircuit|walletdb.ListAccountCreditDebit is the culprit really
240 2016-08-24 10:15:06	0|wumpus|oh, crap related to the account sytem
241 2016-08-24 10:15:53	0|sipa|accounting details are the only thing from wallet db we don't load into memory
242 2016-08-24 10:17:01	0|wumpus|if we only removed that functionality already, we could have effortlessly gotten rid of this
243 2016-08-24 10:24:35	0|paveljanik|wumpus, qt_libbitcoinqt_a-recentrequeststablemodel.o is different when this line is commented, so it is used...
244 2016-08-24 10:25:52	0|gmaxwell|wumpus: https://bitcoin.org/laanwj-releases.asc should probably come with the signature of that key by your personal key. Keyserver retention of keys that are leafs isn't always all that good.
245 2016-08-24 10:25:58	0|paveljanik|but I think this line has no meaning in the read case as the value is assigned to the shadowed nVersion, argument one, and discarded at the end...
246 2016-08-24 10:34:14	0|wumpus|gmaxwell: yes, good idea
247 2016-08-24 10:39:54	0|wumpus|gmaxwell: do I need to do anything special for that? I just did gpg --export --armor 0x90C8019E36C2E964 > ../websites/bitcoin.org/laanwj-releases.asc  and git sees no difference
248 2016-08-24 10:41:26	0|gmaxwell|hm. maybe I'm wrong and it has it already, let me check.
249 2016-08-24 10:42:06	0|gmaxwell|it does, I'm sorry to have wasted your time.
250 2016-08-24 10:42:42	0|wumpus|ok, well good to know
251 2016-08-24 10:43:01	0|gmaxwell|someone in #bitcoin said it didn't, which primed me to conclude it didn't when I looked. :)
252 2016-08-24 11:09:28	0|wumpus|should probably update my key at bitcoin.org with all the new signatures though
253 2016-08-24 11:56:50	0|GitHub145|13bitcoin/06master 14c911035 15djpnewton: Add default port numbers to REST doc
254 2016-08-24 11:56:50	0|GitHub145|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/21857d2bf746...85d4e21a6178
255 2016-08-24 11:56:51	0|GitHub145|13bitcoin/06master 1485d4e21 15Wladimir J. van der Laan: Merge #8567: Add default port numbers to REST doc...
256 2016-08-24 11:57:04	0|GitHub2|[13bitcoin] 15laanwj closed pull request #8567: Add default port numbers to REST doc (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/8567
257 2016-08-24 12:00:31	0|GitHub112|13bitcoin/06master 14fa8dd78 15MarcoFalke: [qt] Remove Priority from coincontrol dialog
258 2016-08-24 12:00:31	0|GitHub112|[13bitcoin] 15jonasschnelli pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/85d4e21a6178...62a5a8a01866
259 2016-08-24 12:00:32	0|GitHub112|13bitcoin/06master 1462a5a8a 15Jonas Schnelli: Merge #8463: [qt] Remove Priority from coincontrol dialog...
260 2016-08-24 12:00:41	0|GitHub94|[13bitcoin] 15jonasschnelli closed pull request #8463: [qt] Remove Priority from coincontrol dialog (06master...06Mf1608-qtPrio) 02https://github.com/bitcoin/bitcoin/pull/8463
261 2016-08-24 12:12:56	0|jonasschnelli|MarcoFalke, paveljanik: interested in testing an ACK/NACK https://github.com/bitcoin/bitcoin/pull/7826?
262 2016-08-24 12:16:45	0|paveljanik|jonasschnelli, well, Marco's suggestion to add "how to test" would be great ;-) I'll test compilation now at least.
263 2016-08-24 12:16:55	0|paveljanik|I do not use GUI much...
264 2016-08-24 12:18:00	0|jonasschnelli|Ah. Right. Also, I overlooked his point with non-std transactions.. will try to fix
265 2016-08-24 12:41:57	0|sipa|making CTransaction actually immutable is a huge rathole...
266 2016-08-24 12:44:31	0|sipa|3 hours straight of fixing compile errors, and now the tests don't run :)
267 2016-08-24 12:44:54	0|sipa|notably, almost all the compile errors were in the compact blocks code and in wallet tests
268 2016-08-24 13:32:38	0|paveljanik|FWIW, Jeff just merged one univalue PR...
269 2016-08-24 14:00:07	0|jonasschnelli|Oh. Bitcoin-Core 0.13.0 does not run on OSX 10.7, sanity check issue
270 2016-08-24 14:01:05	0|jonasschnelli|which means ECC or glibc sanity check are firing...
271 2016-08-24 15:21:46	0|GitHub13|[13bitcoin] 15MarcoFalke opened pull request #8578: [test] Remove unused code (06master...06Mf1608-qaUnused) 02https://github.com/bitcoin/bitcoin/pull/8578
272 2016-08-24 15:52:22	0|sipa|wumpus: ok, tests run; ~400 lines changed
273 2016-08-24 15:54:04	0|sipa|i'm not sure if i dare trying to sync
274 2016-08-24 16:35:08	0|sipa|wumpus, gmaxwell: so, i think it works, but there is one giant uglyness
275 2016-08-24 16:35:31	0|sipa|you can't modify the first transaction in a std::vector<CTransaction> without destroying the entire bector
276 2016-08-24 16:35:41	0|sipa|which is needed in some of the mining code
277 2016-08-24 16:36:00	0|sipa|two possible solutions: copy the entire transaction set of a block (that's a deep copy, even...)
278 2016-08-24 16:36:18	0|sipa|or deconstruct and reconstruct the coinbase in-place using placement new
279 2016-08-24 16:36:29	0|sipa|(which is incredibly ugly, but i think well-defined)
280 2016-08-24 16:37:23	0|GitHub185|[13bitcoin] 15MarcoFalke opened pull request #8579: Performance: Prefer prefix operator for non-primitive types (06master...06Mf1608-perfIter) 02https://github.com/bitcoin/bitcoin/pull/8579
281 2016-08-24 16:47:17	0|GitHub18|[13bitcoin] 15sipa opened pull request #8580: Make CTransaction actually immutable (06master...06pain) 02https://github.com/bitcoin/bitcoin/pull/8580
282 2016-08-24 17:17:52	0|MarcoFalke|(master...pain)
283 2016-08-24 17:17:54	0|MarcoFalke|lol
284 2016-08-24 17:30:16	0|GitHub45|[13bitcoin] 15MarcoFalke opened pull request #8581: [wallet] rpc: Drop misleading option (06master...06Mf1608-walletDropRpc) 02https://github.com/bitcoin/bitcoin/pull/8581
285 2016-08-24 17:35:08	0|arubi|MarcoFalke, <3 "swinging the (block)chain.."
286 2016-08-24 17:36:03	0|sipa|MarcoFalke: it was painful!
287 2016-08-24 17:36:21	0|MarcoFalke|There is more pain to come
288 2016-08-24 17:36:41	0|MarcoFalke|I suggest you don't look at the travi result
289 2016-08-24 17:37:04	0|sipa|already on it
290 2016-08-24 19:52:46	0|cfields_|wumpus: yes, boost::chrono is very much in my sights. Got to dump boost::condition_variable first, though.
291 2016-08-24 19:53:31	0|cfields_|wumpus: I'm making headway on killing off the interruptible threads. I slashed a bunch of them last week, but got distracted with something else.
292 2016-08-24 23:26:39	0|kanzure|wumpus: is there an issue or pull request for http/2 things?
293 2016-08-24 23:27:49	0|kanzure|wumpus: specifically i am wondering what you meant by "http streaming"