1 2017-09-04 00:02:36	0|meshcollider|no not that one, the one about the qt test directory
 2 2017-09-04 00:02:49	0|meshcollider|I got the catch statement thing haha I was only joking :)
 3 2017-09-04 00:02:58	0|meshcollider|I appreciate the reviews :)
 4 2017-09-04 00:03:05	0|sipa|oh
 5 2017-09-04 00:03:23	0|sipa|any reason why the non-qt tests wouldn't need that datadir logic?
 6 2017-09-04 00:05:50	0|meshcollider|they have it I thought? Its here for example: https://github.com/bitcoin/bitcoin/blob/master/src/test/test_bitcoin.cpp#L63
 7 2017-09-04 00:05:54	0|sipa|oh
 8 2017-09-04 00:06:40	0|sipa|meshcollider: ignore me
 9 2017-09-04 00:08:11	0|meshcollider|Ok but just this once ;) I pushed the fix for your last nit for the iswitness decode parameter btw
10 2017-09-04 00:16:14	0|sipa|awesome, thanks
11 2017-09-04 00:19:08	0|meshcollider|I'm sitting on a bus for 8 hours today across the country, anything would be useful for me to work on while I'm sitting here? Otherwise I'll just go and review some more PRs
12 2017-09-04 02:19:05	0|jjackson502|join
13 2017-09-04 03:57:43	0|meshcollider|Is it better to keep src/test/testutil.cpp|h for future use even if they're kinda redundant, or better to remove them because it results in fewer source files?
14 2017-09-04 04:02:14	0|sipa|i don't think anyone cares about the number of source files, as long as the separation in files makes sense (don't make a new file for eveey function...)
15 2017-09-04 04:17:49	0|jimpo|The stall timeout for headers sync (nHeadersSyncTimeout) appears to be a single timeout to sync the whole header chain instead of a smaller timeout for each successive headers message. Why is that?
16 2017-09-04 04:21:40	0|midnightmagic|+1 no file-per-function nonsense. not everyone uses a class-browser and ignores other structure.
17 2017-09-04 04:23:45	0|gmaxwell|jimpo: it's not very sophicated right now. I'm sure it could also have several tiers of timeouts, one thing to be warry of is the sum of small timeouts if fed slowly turning into an unreasonably long wait.
18 2017-09-04 04:44:14	0|meshcollider|midnightmagic: I'm suggesting removing a file with only one function (a redundant function which just calls another), not adding new files
19 2017-09-04 04:45:22	0|gmaxwell|meshcollider: if you don't know you can also feel free to just make a PR to remove it per your best judgement and if you were mistaken people will explain why. No big deal.
20 2017-09-04 05:54:15	0|kallewoof|sipa: Slightly off topic, but a new file for every public class would be sort of nice though (net.h has 4, for example).
21 2017-09-04 05:57:26	0|sipa|kallewoof: i couldn't disagree more, soory:)
22 2017-09-04 05:58:15	0|kallewoof|sipa: If they're small, I can live with it, but net.cpp is 3k lines. I think that's a bit long.
23 2017-09-04 05:59:14	0|sipa|oh, absolutely
24 2017-09-04 05:59:29	0|sipa|but nrt.cpp is also way more than one class
25 2017-09-04 06:01:37	0|kallewoof|CConnman is about 2k, the rest is CNetMessage and CNode and random help functions. Not sure what you mean by 'more than one class' -- that's what I'm saying too, it's 3 classes.
26 2017-09-04 06:04:01	0|kallewoof|Anyway, it was a random thought. I like files to be less than 1k lines if possible. Maybe that's just a personal preference.
27 2017-09-04 06:04:15	0|sipa|kallewoof: what i mean is that in an ideal world a lot of the functionality in net could be split up into more classes, but i think many of them may still belong in net
28 2017-09-04 06:04:52	0|sipa|i don't see a 3k line file as a big problem, but it is suboptimal
29 2017-09-04 06:05:23	0|sipa|but splitting up every class into its own file is throwing away the ability for files to convey a structure themselves
30 2017-09-04 06:17:34	0|kallewoof|That makes sense, yeah. I guess my erk is really with file size, not classes in the same file..
31 2017-09-04 06:18:19	0|sipa|right
32 2017-09-04 06:18:38	0|sipa|file structure is there to make you help find things
33 2017-09-04 06:19:24	0|sipa|if you know the name of every class in the project, and how they relate, then i guess one class per file is optimal
34 2017-09-04 06:19:59	0|sipa|but typically you don't, and if when investigating program flow you need to jump between files all the time, that's also not very helpful
35 2017-09-04 06:23:35	0|kallewoof|It feels like a lot of file jumping happens right now though. Everything and its aunt seems to at some point go into validation.cpp, for example. I really wish that file was at least 2 or 3 files with distinct purposes.
36 2017-09-04 06:24:01	0|kallewoof|I know it's been improved though. (I think it was 7k+ lines when I started and it was still called main.cpp)
37 2017-09-04 06:25:28	0|sipa|kallewoof: right, the key point is "with distinct purposes"
38 2017-09-04 09:17:37	0|promag|sipa: the same for big files?
39 2017-09-04 09:17:37	0|promag|> when investigating program flow you need to jump between files all the time, that's also not very helpful
40 2017-09-04 10:16:19	0|Victorsueca|did anyone check my question about Windows GUI fee setting?
41 2017-09-04 11:06:48	0|meshcollider|Victorsueca: sorry what question? Link?
42 2017-09-04 11:21:00	0|Victorsueca|meshcollider: when setting the fee on Windows with the gui it only displayed the estimate for 2 blocks
43 2017-09-04 11:21:42	0|Victorsueca|I was going to ask if it was intended, but seems it's just it was creating fee estimates
44 2017-09-04 11:22:31	0|Victorsueca|whatever setting I used would only display "Estimated to begin confirmation within 2 blocks"
45 2017-09-04 11:22:49	0|Victorsueca|now it's been up for several days and it display up to 150 blocks
46 2017-09-04 11:38:59	0|meshcollider|So it's working as expected now?
47 2017-09-04 11:39:37	0|meshcollider|Btw Travis needs rerunning on #11062 for someone with git superpowers :)
48 2017-09-04 11:44:04	0|Victorsueca|I have no idea how is it supposed to work, but seems usable to me
49 2017-09-04 12:16:31	0|trippysalmon|hi, i created a patch for #10757 (see: https://github.com/jtimon/bitcoin/pull/9) only travis is not happy about it in a build (see: https://travis-ci.org/jtimon/bitcoin/builds/271625340)
50 2017-09-04 12:17:00	0|trippysalmon|how do i go about debugging this? i've run the extended tests locally and it doesn't give an error
51 2017-09-04 15:48:11	0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #11230: [tests] fixup dbcrash interaction with add_nodes() (06master...06fixup_dbcrash) 02https://github.com/bitcoin/bitcoin/pull/11230
52 2017-09-04 17:41:16	0|bitcoin-git|[13bitcoin] 15danra opened pull request #11231: Improve netaddress implementation (06master...06refactor/safe-netaddress) 02https://github.com/bitcoin/bitcoin/pull/11231
53 2017-09-04 19:22:26	0|jnewbery|trippysalmon : that's a really weird failure. The test framework is failing to read a username:password from a file, even though it previously read them successfully. I think this must be some random Travis failure. I suggest you ask jtimon to restart the Travis job (since it's a PR to his repo)
54 2017-09-04 19:22:59	0|sipa|i can restart jobs
55 2017-09-04 19:23:24	0|trippysalmon|jnewbery: thanks, i thought something like that might be the case, haven't looked to deep into it after i found that out
56 2017-09-04 19:23:46	0|trippysalmon|sipa: that would be great
57 2017-09-04 19:23:53	0|sipa|which PR, sorry?
58 2017-09-04 19:24:33	0|trippysalmon|it's PR #9, but it's on jtimon's repo
59 2017-09-04 19:24:39	0|sipa|oh!
60 2017-09-04 19:24:41	0|sipa|then i can't
61 2017-09-04 19:25:03	0|trippysalmon|aww, thanks anyway, i'll ask jtimon when i see him around
62 2017-09-04 19:51:20	0|PRab|FYI, I did a gitian build of 0.15.0rc3, installed it (Windows 10 64 bit), had it upgrade my UTXO db (took ~15 minutes), and all appears to be running well! Keep up the great work!!
63 2017-09-04 19:56:05	0|sipa|PRab: cool
64 2017-09-04 20:17:19	0|jnewbery|trippysalmon : alternatively, you can  `git commit --amend` to update the timestamp on your most recent commit and then `git push -f`. New timestamp => new SHA => Travis will rerun.
65 2017-09-04 20:21:24	0|trippysalmon|jnewbery: clever man :) will do that, thanks
66 2017-09-04 20:27:12	0|MarcoFalke|tripy
67 2017-09-04 20:27:24	0|MarcoFalke|trippysalmon: The bug was fixed in current master
68 2017-09-04 20:27:45	0|MarcoFalke|The issue was that the cookie file was not atomically written on creation
69 2017-09-04 20:30:48	0|trippysalmon|MarcoFalke: ah ok, thanks for letting me know. this is my first time working on the core codebase so i was unsure if it was my mistake or not
70 2017-09-04 20:31:08	0|trippysalmon|ill leave the code as it is then and assume it will work out
71 2017-09-04 20:31:38	0|MarcoFalke|Jup, rerunning should work around the issue
72 2017-09-04 20:32:43	0|trippysalmon|it's currently running so i hope you're right
73 2017-09-04 20:54:46	0|trippysalmon|okay all jobs finished successfully, thanks all
74 2017-09-04 21:47:10	0|BlueMatt|note: after #10387 (or some way to identify a node as full but pruned) we should stop accepting requests to fast-relay compact blocks pre-validation from non-full nodes. Its a drastic reduction in their security model and unlikely to be worth the tiny change in relay time. I'm especially concerned that some of the spv clients are going to implement compact blocks and then someone who had set up bitcoin core as a border/firewall node
75 2017-09-04 21:47:11	0|BlueMatt|between their spv wallet and the world will suddenly have only spv security instead of full security without them even noticing
76 2017-09-04 21:51:20	0|bitcoin-git|[13bitcoin] 15practicalswift closed pull request #10972: [Qt] Check return value of addr.GetKeyID(keyid) on custom change address change (06master...06GetKeyID-assertion) 02https://github.com/bitcoin/bitcoin/pull/10972
77 2017-09-04 21:52:33	0|promag|should be foo(const CAmount& amount) or foo(CAmount amount) ?
78 2017-09-04 21:54:59	0|BlueMatt|I think we use both...cause CAmount is secretly just int64_t I'm not sure it matters, but there was some idea of making CAmount assert that you dont overflow at some point in the future
79 2017-09-04 21:55:10	0|BlueMatt|I think & is maybe slightly more-used
80 2017-09-04 21:55:12	0|BlueMatt|iirc
81 2017-09-04 22:06:56	0|promag|yes it is, ty
82 2017-09-04 22:17:16	0|bitcoin-git|[13bitcoin] 15jjz opened pull request #11232: Ensure that data types are consistent (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/11232