1 2016-11-09T00:07:03  *** btcdrak has joined #bitcoin-core-dev
  2 2016-11-09T00:22:56  *** veleiro has joined #bitcoin-core-dev
  3 2016-11-09T00:33:56  *** AaronvanW has quit IRC
  4 2016-11-09T00:45:26  <sdaftuar> cfields: i think it'd also get tricky because the CValidationState passed in to ProcessNewBlock can be referencing a different block than where it might be used in ActivateBestChainStep
  5 2016-11-09T00:45:43  <sipa> sdaftuar: good point
  6 2016-11-09T00:51:17  <cfields> sdaftuar: yes, makes sense. thanks.
  7 2016-11-09T01:03:13  *** davec has quit IRC
  8 2016-11-09T01:19:44  <bitcoin-git> [bitcoin] TheBlueMatt closed pull request #9014: Fix block-connection performance regression (master...2016-10-fix-tx-regression) https://github.com/bitcoin/bitcoin/pull/9014
  9 2016-11-09T01:24:50  *** davec has joined #bitcoin-core-dev
 10 2016-11-09T01:42:27  *** jtimon has joined #bitcoin-core-dev
 11 2016-11-09T01:42:46  <BlueMatt> jtimon: so i think i need to revert #9087 for #9075
 12 2016-11-09T01:42:47  <gribble> https://github.com/bitcoin/bitcoin/issues/9087 | RPC: why not give more details when "generate" fails? by jtimon · Pull Request #9087 · bitcoin/bitcoin · GitHub
 13 2016-11-09T01:42:49  <gribble> https://github.com/bitcoin/bitcoin/issues/9075 | Decouple peer-processing-logic from block-connection-logic (#3) by TheBlueMatt · Pull Request #9075 · bitcoin/bitcoin · GitHub
 14 2016-11-09T01:43:06  <BlueMatt> because 9074 removes the state parameter from ProcessNewBlock - it only is used if CheckBlock fails, not if there is an actual failure
 15 2016-11-09T01:45:22  <jtimon> #9087 is just something I found while trying to fix the blocksigning branch, but it surprises me that you need to revert it
 16 2016-11-09T01:45:24  <gribble> https://github.com/bitcoin/bitcoin/issues/9087 | RPC: why not give more details when "generate" fails? by jtimon · Pull Request #9087 · bitcoin/bitcoin · GitHub
 17 2016-11-09T01:45:54  <BlueMatt> well it already doesnt work - the state object to ProcessNewBlock is only used if CheckBlock fails, not if the block is invalid for any other reason
 18 2016-11-09T01:46:09  <BlueMatt> the only way to get useful info from ProcessNewBlock is to register, then de-register a CValidationInterface, already
 19 2016-11-09T01:46:12  <BlueMatt> so I was removing it
 20 2016-11-09T01:46:21  <sipa> or if processblock doens't run... if someone another new block was received?
 21 2016-11-09T01:46:24  <sipa> *somehow
 22 2016-11-09T01:46:41  <jtimon> oh, I see, and in my case it was failing on checkblockheader
 23 2016-11-09T01:47:16  <jtimon> so with 9087 I was able to see "high hash"
 24 2016-11-09T01:47:31  <BlueMatt> if someone actually cares, I can fix it by doing the register, de-register hack
 25 2016-11-09T01:47:41  <BlueMatt> but....its a dirty hack
 26 2016-11-09T01:48:28  <jtimon> btw, after you matt took the time check out checkblockheader is just like checkpow but putting a message on the validation state and with and absurd bool arg
 27 2016-11-09T01:48:31  <sipa> jtimon: assuming the block wasn't reorged out, for what reason other than high hash would generate fail?
 28 2016-11-09T01:49:52  <jtimon> feel free to revert the change, I can always have it locally, it shouldn't normally fail anyway, just because I'm doing another version of checkpow for the block signing stuff
 29 2016-11-09T01:50:35  <sipa> jtimon: with matt's change, processnewblock doesn't take a CValidateState anymore
 30 2016-11-09T01:51:02  <jtimon> I just thought there was no harm and maybe was useful in more cases besides mine, seriously, no need to pass CValidateState to processnewblock only for rpc/mining
 31 2016-11-09T01:51:12  <sipa> BlueMatt, jtimon: perhaps it can be fixed by invoking AcceptNewHeader explicitly from RPC, and report the result from that?
 32 2016-11-09T01:51:31  <BlueMatt> ehh, please no...CheckBlock
 33 2016-11-09T01:51:35  <BlueMatt> not Accept*Header
 34 2016-11-09T01:51:40  <sipa> ah
 35 2016-11-09T01:51:42  <sipa> ok
 36 2016-11-09T01:52:21  <jtimon> I can do it locally if I need to, but right now I know where the things are failing, just don't understand why, please feel free to revert 9087, np
 37 2016-11-09T01:53:06  <BlueMatt> ok
 38 2016-11-09T01:53:28  <jtimon> it just felt kind of free to upstream
 39 2016-11-09T01:53:40  <BlueMatt> yupyup, all right decisions, just randomly gets in the way :)
 40 2016-11-09T01:54:14  <jtimon> ok, good, same page it seems
 41 2016-11-09T01:56:15  <jtimon> sipa: the last idea may be interesting for mining::generate though
 42 2016-11-09T01:56:51  <jtimon> but yeah, later
 43 2016-11-09T01:57:15  *** abpa has quit IRC
 44 2016-11-09T01:59:06  <jtimon> does it make sense for me to try that the custom chain (by default just like regstes, just with a different genesis block and a different directory) passes all rpc python tests just like regtest does?
 45 2016-11-09T02:00:09  *** Ylbam has quit IRC
 46 2016-11-09T02:00:17  *** fengling has joined #bitcoin-core-dev
 47 2016-11-09T02:00:47  <jtimon> never mind, I'll try first, ask later
 48 2016-11-09T02:17:57  *** btcdrak has quit IRC
 49 2016-11-09T02:18:31  *** DigiByteDev has joined #bitcoin-core-dev
 50 2016-11-09T02:22:01  *** afk11_ has quit IRC
 51 2016-11-09T02:23:18  *** afk11 has joined #bitcoin-core-dev
 52 2016-11-09T02:23:18  *** afk11 has quit IRC
 53 2016-11-09T02:23:18  *** afk11 has joined #bitcoin-core-dev
 54 2016-11-09T02:41:40  *** grubles has quit IRC
 55 2016-11-09T02:43:59  *** Squidicuz has joined #bitcoin-core-dev
 56 2016-11-09T02:54:57  *** grubles has joined #bitcoin-core-dev
 57 2016-11-09T03:18:18  *** Chris_Stewart_5 has quit IRC
 58 2016-11-09T03:27:01  *** To7 has quit IRC
 59 2016-11-09T03:31:30  *** Chris_Stewart_5 has joined #bitcoin-core-dev
 60 2016-11-09T03:39:49  *** Chris_Stewart_5 has quit IRC
 61 2016-11-09T03:57:06  *** btcdrak has joined #bitcoin-core-dev
 62 2016-11-09T03:59:20  *** abpa has joined #bitcoin-core-dev
 63 2016-11-09T04:03:42  *** fanquake has joined #bitcoin-core-dev
 64 2016-11-09T04:04:24  *** jtimon has quit IRC
 65 2016-11-09T04:12:44  *** justanotheruser has quit IRC
 66 2016-11-09T04:15:50  *** justanotheruser has joined #bitcoin-core-dev
 67 2016-11-09T04:39:12  *** shesek has quit IRC
 68 2016-11-09T04:53:45  *** veleiro has quit IRC
 69 2016-11-09T05:14:02  *** juscamarena has quit IRC
 70 2016-11-09T05:18:00  *** go1111111 has joined #bitcoin-core-dev
 71 2016-11-09T05:40:22  *** Chris_Stewart_5 has joined #bitcoin-core-dev
 72 2016-11-09T05:42:01  *** fengling has quit IRC
 73 2016-11-09T05:49:10  *** davec has quit IRC
 74 2016-11-09T05:58:39  *** Chris_Stewart_5 has quit IRC
 75 2016-11-09T06:07:36  *** wasi has quit IRC
 76 2016-11-09T06:07:37  *** testnet has quit IRC
 77 2016-11-09T06:09:08  *** fengling has joined #bitcoin-core-dev
 78 2016-11-09T06:10:44  *** davec has joined #bitcoin-core-dev
 79 2016-11-09T06:11:20  *** Giszmo has quit IRC
 80 2016-11-09T06:11:51  *** wasi has joined #bitcoin-core-dev
 81 2016-11-09T06:11:57  *** testnet has joined #bitcoin-core-dev
 82 2016-11-09T06:16:05  *** DigiByteDev has quit IRC
 83 2016-11-09T06:17:57  *** btcdrak has quit IRC
 84 2016-11-09T06:49:29  *** DigiByteDev has joined #bitcoin-core-dev
 85 2016-11-09T07:11:53  *** btcdrak has joined #bitcoin-core-dev
 86 2016-11-09T07:36:34  *** LeMiner2 has joined #bitcoin-core-dev
 87 2016-11-09T07:39:04  *** LeMiner has quit IRC
 88 2016-11-09T07:39:04  *** LeMiner2 is now known as LeMiner
 89 2016-11-09T07:47:19  *** BashCo has quit IRC
 90 2016-11-09T07:59:43  *** DigiByteDev has quit IRC
 91 2016-11-09T08:10:49  *** BashCo has joined #bitcoin-core-dev
 92 2016-11-09T08:18:04  *** DigiByteDev has joined #bitcoin-core-dev
 93 2016-11-09T08:18:37  *** davec has quit IRC
 94 2016-11-09T08:20:41  *** davec has joined #bitcoin-core-dev
 95 2016-11-09T08:25:13  *** rubensayshi has joined #bitcoin-core-dev
 96 2016-11-09T08:32:49  *** davec has quit IRC
 97 2016-11-09T08:34:28  *** davec has joined #bitcoin-core-dev
 98 2016-11-09T09:04:56  *** AaronvanW has joined #bitcoin-core-dev
 99 2016-11-09T09:04:56  *** AaronvanW has quit IRC
100 2016-11-09T09:04:56  *** AaronvanW has joined #bitcoin-core-dev
101 2016-11-09T09:10:22  *** ChillazZ has joined #bitcoin-core-dev
102 2016-11-09T09:10:48  *** ChillazZ is now known as Guest82720
103 2016-11-09T09:13:34  *** laurentmt has joined #bitcoin-core-dev
104 2016-11-09T09:16:08  *** rebroad has joined #bitcoin-core-dev
105 2016-11-09T09:16:35  <rebroad> gmaxwell, wumpus, sipa, I think I may have found a bug that can crash bitcoin nodes
106 2016-11-09T09:17:15  <rebroad> potential DoS
107 2016-11-09T09:17:44  <Victorsueca> rebroad: in latest version?
108 2016-11-09T09:17:48  <rebroad> yes
109 2016-11-09T09:18:18  <rebroad> or at least, someone else found it and used it against my node
110 2016-11-09T09:18:24  <rebroad> but it's easy to fix
111 2016-11-09T09:18:39  <jonasschnelli> rebroad: Does it crash or do you just get an exception?
112 2016-11-09T09:18:43  <rebroad> crashes
113 2016-11-09T09:18:47  <rebroad> well, both
114 2016-11-09T09:19:06  <jonasschnelli> I guess we are talking about 9110
115 2016-11-09T09:19:07  <jonasschnelli> PR #9110
116 2016-11-09T09:19:08  <gribble> https://github.com/bitcoin/bitcoin/issues/9110 | CInv::GetCommand(): type=1373179482 unknown type · Issue #9110 · bitcoin/bitcoin · GitHub
117 2016-11-09T09:19:15  <rebroad> yes
118 2016-11-09T09:19:31  <rebroad> only if debug=net though
119 2016-11-09T09:19:37  <rebroad> the "got inv" debug messsage
120 2016-11-09T09:20:09  <jonasschnelli> Okay...
121 2016-11-09T09:20:19  <Victorsueca> sounds like bitcoin core fails some assertion in there
122 2016-11-09T09:20:23  <paveljanik> please add the info about debug=net to the PR...
123 2016-11-09T09:21:18  *** laurentmt has quit IRC
124 2016-11-09T09:21:18  <Victorsueca> rebroad: it crashes only with debug=net enabled? or it still crashes anyway but without log exception when disabled?
125 2016-11-09T09:21:27  <rebroad> I have changed the code to simply strprintf an unknown and I think this should be an ok fix..  not tested sufficiently yet (as I don't have code to generate invalid inv messages yet)
126 2016-11-09T09:21:49  <rebroad> Victorsueca, I suspect it'll only crash if debug=net but not tested with it not set yet
127 2016-11-09T09:22:37  <rebroad> but someone out there is already sending these invalid invs... the address it came from was
128 2016-11-09T09:23:13  <Victorsueca> if that's the case then it would be a bug at generating the exception warning message or writing it to the log
129 2016-11-09T09:23:25  <jonasschnelli> I though ProcessMessages() executes always in a try/catch...
130 2016-11-09T09:23:31  *** montau is now known as whphhg
131 2016-11-09T09:23:53  <rebroad> it's is possible that I have made a change that is making it vulnerable
132 2016-11-09T09:24:02  <jonasschnelli> I think so...
133 2016-11-09T09:24:07  <jonasschnelli> Going to test this now...
134 2016-11-09T09:24:07  <rebroad> I don't think I have, but it's not beyond belief
135 2016-11-09T09:24:22  *** DigiByteDev has quit IRC
136 2016-11-09T09:24:38  *** fengling has quit IRC
137 2016-11-09T09:24:55  <Victorsueca> maybe somebody should make a honeypot and wireshark it to see what message is causing this
138 2016-11-09T09:25:06  <jonasschnelli> rebroad: you could try to reproduce the issue on master with a simple new RPC test (maybe called invalidinv.py)
139 2016-11-09T09:25:13  <jonasschnelli> Victorsueca: it's simple to test... no need for a honeypot
140 2016-11-09T09:25:39  <paveljanik> rebroad, and in every case: responsible disclosure next time, please...
141 2016-11-09T09:26:28  <Victorsueca> ^ yeah, this things should be PGPed to the team
142 2016-11-09T09:26:52  <jonasschnelli> Indeed (if you think your right)
143 2016-11-09T09:27:58  <wumpus> I'm fairly sure tI've tried to do this before, though probably not with debug=net
144 2016-11-09T09:28:34  *** rebroad has quit IRC
145 2016-11-09T09:32:45  *** fanquake has quit IRC
146 2016-11-09T09:33:23  *** fanquake has joined #bitcoin-core-dev
147 2016-11-09T09:33:37  *** DigiByteDev has joined #bitcoin-core-dev
148 2016-11-09T09:33:42  <wumpus> it'd need to call CInv::ToString or CInv::GetCommand to get this exception, indeed something that only happens with debugging on
149 2016-11-09T09:33:58  <wumpus> I think it's somewhat unexpected for a ToString call to raise an exception
150 2016-11-09T09:34:06  <wumpus> however the exception should not crash the program
151 2016-11-09T09:39:55  <Victorsueca> the log says something about St12out_of_range, that sounds like something that was asserted to be within certain range is not
152 2016-11-09T09:40:04  <Victorsueca> maybe the type number?
153 2016-11-09T09:41:04  <fanquake> wumpus Yes you have. See #1625
154 2016-11-09T09:41:05  <gribble> https://github.com/bitcoin/bitcoin/issues/1625 | Bitcoin-Qt: exception in CInv::GetCommand() when using -onlynet="IPv6" · Issue #1625 · bitcoin/bitcoin · GitHub
155 2016-11-09T09:41:28  <fanquake> So we've seen this/a similar issue before.
156 2016-11-09T09:41:49  <wumpus> managed to get the message in the log, however no crash
157 2016-11-09T09:51:29  *** fengling has joined #bitcoin-core-dev
158 2016-11-09T09:54:03  <bitcoin-git> [bitcoin] fanquake opened pull request #9111: Remove unused variable UNLIKELY_PCT from fees.h (master...remove-unused-unlikely-pct) https://github.com/bitcoin/bitcoin/pull/9111
159 2016-11-09T09:54:29  *** ratoder has quit IRC
160 2016-11-09T10:05:43  <bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/924de0bd75a7...e9847303e708
161 2016-11-09T10:05:44  <bitcoin-git> bitcoin/master addfdeb Andrew Chow: Multiple Selection for peer and ban tables...
162 2016-11-09T10:05:44  <bitcoin-git> bitcoin/master 1077577 Andrew Chow: Fix auto-deselection of peers
163 2016-11-09T10:05:45  <bitcoin-git> bitcoin/master e984730 Wladimir J. van der Laan: Merge #8874: Multiple Selection for peer and ban tables...
164 2016-11-09T10:11:53  *** rebroad has joined #bitcoin-core-dev
165 2016-11-09T10:13:07  <rebroad> apologies for the less than ideal disclosure...  I purposefully did not mention the issue I'd raised on here for that reason (bad internet here, otherwise I'd already have wiped the issue the moment it got mentioned)
166 2016-11-09T10:16:36  <bitcoin-git> [bitcoin] laanwj opened pull request #9112: Avoid ugly exception in log on unknown inv type (master...2016_11_invtype_debugging) https://github.com/bitcoin/bitcoin/pull/9112
167 2016-11-09T10:25:20  <bitcoin-git> [bitcoin] rebroad opened pull request #9113: Return the type when it's unknown instead of throwing an exception (master...ReturnNotThrow) https://github.com/bitcoin/bitcoin/pull/9113
168 2016-11-09T10:47:24  *** DigiByteDev has quit IRC
169 2016-11-09T11:01:16  <bitcoin-git> [bitcoin] fanquake opened pull request #9114: [depends] Set OSX_MIN_VERSION to 10.8 (master...depends-min-osx-10-8) https://github.com/bitcoin/bitcoin/pull/9114
170 2016-11-09T11:10:38  *** LeMiner2 has joined #bitcoin-core-dev
171 2016-11-09T11:12:48  *** LeMiner has quit IRC
172 2016-11-09T11:12:48  *** LeMiner2 is now known as LeMiner
173 2016-11-09T11:15:20  *** DigiByteDev has joined #bitcoin-core-dev
174 2016-11-09T11:23:12  *** cdecker has joined #bitcoin-core-dev
175 2016-11-09T11:23:43  *** aalex has joined #bitcoin-core-dev
176 2016-11-09T11:25:17  *** rebroad has quit IRC
177 2016-11-09T11:28:59  <jonasschnelli> I have started with the BIP151 implementation and are trying to split it into different PRs, though, not sure how easy that is.
178 2016-11-09T11:34:56  <wumpus> indeed, for next time: if you suspect a remote-triggerable crash bug, even if you're not sure, please don't immediately throw it in here, but use private ways of contacting
179 2016-11-09T11:36:33  <wumpus> </panicmode>
180 2016-11-09T11:37:24  *** Giszmo has joined #bitcoin-core-dev
181 2016-11-09T11:37:26  <wumpus> jonasschnelli: at least coordinate with cfields on the network level changes
182 2016-11-09T11:54:37  <bitcoin-git> [bitcoin] paveljanik opened pull request #9115: Mention reporting security issues responsibly (master...20161109_secissues) https://github.com/bitcoin/bitcoin/pull/9115
183 2016-11-09T12:01:05  *** DigiByteDev has quit IRC
184 2016-11-09T12:03:07  *** fengling has quit IRC
185 2016-11-09T12:07:08  *** davec has quit IRC
186 2016-11-09T12:10:22  *** aalex has quit IRC
187 2016-11-09T12:21:56  <bitcoin-git> [bitcoin] ondrejsika opened pull request #9116: Allow getblocktemlate for not connected regtest node (master...master) https://github.com/bitcoin/bitcoin/pull/9116
188 2016-11-09T12:31:20  *** murch has joined #bitcoin-core-dev
189 2016-11-09T12:37:33  <jonasschnelli> wumpus: Yes. Good idea. Not sure if a PR with the implementation of the new message format without the actual encryption itself could make sense.
190 2016-11-09T12:48:01  *** rebroad has joined #bitcoin-core-dev
191 2016-11-09T12:52:31  <bitcoin-git> [bitcoin] laanwj pushed 10 new commits to master: https://github.com/bitcoin/bitcoin/compare/e9847303e708...e81df49644c2
192 2016-11-09T12:52:32  <bitcoin-git> bitcoin/master 50e8a9c Pieter Wuille: Remove unused ReadVersion and WriteVersion...
193 2016-11-09T12:52:33  <bitcoin-git> bitcoin/master c2c5d42 Pieter Wuille: Make streams' read and write return void...
194 2016-11-09T12:52:33  <bitcoin-git> bitcoin/master fad9b66 Pieter Wuille: Make nType and nVersion private and sometimes const...
195 2016-11-09T12:52:41  <bitcoin-git> [bitcoin] laanwj closed pull request #9039: Various serialization simplifcations and optimizations (master...simpleserial) https://github.com/bitcoin/bitcoin/pull/9039
196 2016-11-09T12:56:43  *** rebroad has quit IRC
197 2016-11-09T13:03:59  *** MarcoFalke has joined #bitcoin-core-dev
198 2016-11-09T13:11:54  *** rebroad has joined #bitcoin-core-dev
199 2016-11-09T13:12:42  <bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/e81df49644c2...e0477f6d2072
200 2016-11-09T13:12:43  <bitcoin-git> bitcoin/master fd5654c Pavel Janík: Check and enable -Wshadow by default.
201 2016-11-09T13:12:43  <bitcoin-git> bitcoin/master 359bac7 Pavel Janík: Add notes about variable names and shadowing
202 2016-11-09T13:12:44  <bitcoin-git> bitcoin/master e0477f6 Wladimir J. van der Laan: Merge #8794: Enable -Wshadow by default...
203 2016-11-09T13:12:52  <bitcoin-git> [bitcoin] laanwj closed pull request #8794: Enable -Wshadow by default (master...20160922_Wshadow_enable) https://github.com/bitcoin/bitcoin/pull/8794
204 2016-11-09T13:43:45  <jonasschnelli> cfields: net.h what's the advantage of using a Callable (ForEachNodeContinueIf(Callable&& func)) instead of just passing in a std::function?
205 2016-11-09T13:51:29  *** aalex has joined #bitcoin-core-dev
206 2016-11-09T13:51:37  <cfields> jonasschnelli: so that lambdas are overhead-free
207 2016-11-09T13:55:19  <jonasschnelli> cfields: can you explain me what kind of overhead? (Sorry to bother you with basic c++)
208 2016-11-09T13:57:13  <cfields> jonasschnelli: np. std::function necessarily does an allocation, is copyable, etc. But lambdas can essentially create inline code.
209 2016-11-09T13:58:24  <cfields> by using a callable, you can pass in a lambda for "free", or pass in a std::function for flexibility. It gives the flexibility of both.
210 2016-11-09T13:59:10  <cfields> (maybe I should've mentioned that earlier. You can pass a std::function into it if you'd like, it'll work as you'd want it to)
211 2016-11-09T14:00:32  <jonasschnelli> cfields: here I pass in Lambda https://github.com/bitcoin/bitcoin/pull/9076/files#diff-b2bb174788c7409b671c46ccc86034bdR3824 ... the function structure defines the parameter as std::function, does this always result in a copy for the lambda?
212 2016-11-09T14:00:35  *** Ylbam has joined #bitcoin-core-dev
213 2016-11-09T14:01:22  *** Guyver2 has joined #bitcoin-core-dev
214 2016-11-09T14:01:55  <cfields> jonasschnelli: yes. a std::function is constructed there.
215 2016-11-09T14:02:49  <jonasschnelli> Okay. So changing it to a callable would be more efficient.. right?
216 2016-11-09T14:02:56  <jonasschnelli> (reducing a copy)
217 2016-11-09T14:03:53  <cfields> jonasschnelli: yep, assuming you can neatly use a template in the definition
218 2016-11-09T14:05:15  *** murch has quit IRC
219 2016-11-09T14:06:37  <cfields> jonasschnelli: think of it like something "printable". You could pass the constant "foo" into a "Print(std::string s) { printf("string" %s, s); }, which will construct a std::string just to throw it away
220 2016-11-09T14:07:14  <jonasschnelli> cfields: Thanks. I think I see you point now.
221 2016-11-09T14:07:21  <cfields> but if you used "template <typename T> Print(T&& s) { std::cout << s; }, you can pass in a raw string for free
222 2016-11-09T14:18:44  *** davec has joined #bitcoin-core-dev
223 2016-11-09T14:19:30  <fanquake> paveljanik I'm probably wrong, but the hex/line numbers was the first thing that jumped out at me.
224 2016-11-09T14:20:01  <paveljanik> fanquake, sure. But this is probably some issue on the builder side as it does not touch leveldb at all...
225 2016-11-09T14:20:56  *** Chris_Stewart_5 has joined #bitcoin-core-dev
226 2016-11-09T14:24:51  <MarcoFalke> I think I was hitting the assert in net.h https://github.com/bitcoin/bitcoin/blob/e9847303e708ab71b4d2c22ceb7d65c89615e18a/src/net.h#L772 ... I couldn't reproduce and debug.log only says ProcessMessages(version, 102 bytes) FAILED peer=10 http://pastebin.ubuntu.com/23451144/
227 2016-11-09T14:25:29  *** sdaftuar has quit IRC
228 2016-11-09T14:26:08  *** ryanofsky has quit IRC
229 2016-11-09T14:27:42  *** sdaftuar has joined #bitcoin-core-dev
230 2016-11-09T14:35:18  *** sdaftuar has quit IRC
231 2016-11-09T14:37:20  *** sdaftuar has joined #bitcoin-core-dev
232 2016-11-09T14:39:04  *** ryanofsky has joined #bitcoin-core-dev
233 2016-11-09T14:46:16  <cfields> MarcoFalke: yes, i assumed that would cause a few crashes for the first few days
234 2016-11-09T14:47:54  <cfields> MarcoFalke: can you run with -debug for a while?
235 2016-11-09T14:48:06  <MarcoFalke> sure
236 2016-11-09T14:49:05  <cfields> MarcoFalke: the issue is: we continue to send messages, even after the handshake has failed and we've decided to disconnect. I think it's a good thing to find out what those cases are.
237 2016-11-09T14:53:34  *** rebroad has quit IRC
238 2016-11-09T14:55:28  *** shesek has joined #bitcoin-core-dev
239 2016-11-09T14:57:17  *** fanquake has left #bitcoin-core-dev
240 2016-11-09T15:06:18  *** Chris_Stewart_5 has quit IRC
241 2016-11-09T15:18:26  *** abpa has quit IRC
242 2016-11-09T15:28:47  *** jtimon has joined #bitcoin-core-dev
243 2016-11-09T15:54:12  *** laurentmt has joined #bitcoin-core-dev
244 2016-11-09T16:00:44  *** laurentmt has quit IRC
245 2016-11-09T16:01:57  *** whphhg has joined #bitcoin-core-dev
246 2016-11-09T16:17:38  *** ryanofsky has quit IRC
247 2016-11-09T16:17:47  *** sdaftuar has quit IRC
248 2016-11-09T16:19:28  *** sdaftuar has joined #bitcoin-core-dev
249 2016-11-09T16:21:43  *** Guyver2__ has joined #bitcoin-core-dev
250 2016-11-09T16:23:23  <MarcoFalke> cfields: Prob when expected != offered. See debug.log: https://paste.fedoraproject.org/476493/87085851/
251 2016-11-09T16:23:36  <MarcoFalke> or bt https://paste.fedoraproject.org/476491/08356147/
252 2016-11-09T16:23:52  <cfields> MarcoFalke: perfect, thanks
253 2016-11-09T16:24:42  <cfields> MarcoFalke: hmm, seems to be in sending the reject...
254 2016-11-09T16:24:54  *** cysm has quit IRC
255 2016-11-09T16:25:14  *** Guyver2 has quit IRC
256 2016-11-09T16:25:15  <MarcoFalke> And what is the feefilter in the bt?
257 2016-11-09T16:25:16  *** Guyver2__ is now known as Guyver2
258 2016-11-09T16:26:20  <cfields> MarcoFalke: aha! I didn't see the bt
259 2016-11-09T16:27:07  <cfields> MarcoFalke: yep, that's the problem.
260 2016-11-09T16:28:26  <cfields> MarcoFalke: I'll pr a quick/dirty fix in a few min, and I'm working on a more robust solution
261 2016-11-09T16:29:11  <cfields> MarcoFalke: quick fix: http://pastebin.com/raw/E8ZXsKYa
262 2016-11-09T16:29:21  <MarcoFalke> that was quick :)
263 2016-11-09T16:30:18  *** ryanofsky has joined #bitcoin-core-dev
264 2016-11-09T16:30:53  <cfields> MarcoFalke: well like i said, I expected it to point out a few issues. They should all be very easy to fix individually.
265 2016-11-09T16:31:08  <cfields> (as long as there's a nice log/backtrace, that is :)
266 2016-11-09T16:49:18  *** abpa has joined #bitcoin-core-dev
267 2016-11-09T17:06:50  *** Chris_Stewart_5 has joined #bitcoin-core-dev
268 2016-11-09T17:18:59  *** testnet has quit IRC
269 2016-11-09T17:25:39  *** rubensayshi has quit IRC
270 2016-11-09T17:28:55  *** BashCo has quit IRC
271 2016-11-09T17:30:12  *** igno_peverell has joined #bitcoin-core-dev
272 2016-11-09T17:30:21  *** igno_peverell has left #bitcoin-core-dev
273 2016-11-09T17:40:17  *** ryanofsky has quit IRC
274 2016-11-09T17:40:29  *** sdaftuar has quit IRC
275 2016-11-09T17:41:51  *** testnet has joined #bitcoin-core-dev
276 2016-11-09T17:42:18  *** sdaftuar has joined #bitcoin-core-dev
277 2016-11-09T17:50:17  *** BashCo has joined #bitcoin-core-dev
278 2016-11-09T17:53:28  *** ryanofsky has joined #bitcoin-core-dev
279 2016-11-09T18:00:15  *** cysm has joined #bitcoin-core-dev
280 2016-11-09T18:16:53  *** Chris_Stewart_5 has quit IRC
281 2016-11-09T18:19:50  <bitcoin-git> [bitcoin] theuni opened pull request #9117: net: don't send feefilter messages before the version handshake is complete (master...feefilter-assert) https://github.com/bitcoin/bitcoin/pull/9117
282 2016-11-09T18:20:25  *** Chris_Stewart_5 has joined #bitcoin-core-dev
283 2016-11-09T18:23:51  <sipa> jonasschnelli: i was planning to work on bip151, but i'm not going to touch anything before the ongoing net refactors are done :)
284 2016-11-09T18:26:31  <gmaxwell> cfields: if the issue in 9117 is feefilter before handshake, should it not be checking fSuccessfullyConnected?
285 2016-11-09T18:27:03  <cfields> jonasschnelli and i just discussed a bit, i'm making sure that the current refactors at least make bip151 not harder, but hopefully easier too
286 2016-11-09T18:30:44  <cfields> gmaxwell: i suppose that would work too in this case. just that fDisconnected happens to also catch the case when we've just decided to disconnect after a successful handshake
287 2016-11-09T18:31:08  <MarcoFalke> gmaxwell: I think the handshake was done in the case I saw above: https://paste.fedoraproject.org/476493/87085851/raw/
288 2016-11-09T18:31:12  <cfields> er, at any time in the session after the handshake is done
289 2016-11-09T18:31:49  <cfields> MarcoFalke: no, it's halfway done. We got their version and set it, but didn't like it, so didn't set a sendVersion
290 2016-11-09T18:32:48  <gmaxwell> but it wouldn't catch cases where we weren't done but also weren't planning on disconnecting them?
291 2016-11-09T18:34:35  <cfields> gmaxwell: i don't think that's a possibility
292 2016-11-09T18:36:56  <cfields> gmaxwell: i'm cleaning up the disconnect logic atm so that it's hopefully more clear. I believe the only case where we'd want to send a message after we've set fDisconnect is for reject messages, no?
293 2016-11-09T18:36:57  *** davec has quit IRC
294 2016-11-09T18:37:56  <gmaxwell> I don't really think we should be sending any messages after flipping that flag.
295 2016-11-09T18:40:11  *** davec has joined #bitcoin-core-dev
296 2016-11-09T18:43:12  *** Chris_Stewart_5 has quit IRC
297 2016-11-09T18:43:24  <cfields> hmm, may be able to make that work
298 2016-11-09T18:49:37  *** Chris_Stewart_5 has joined #bitcoin-core-dev
299 2016-11-09T18:49:47  *** davec has quit IRC
300 2016-11-09T18:53:19  <morcos> sipa: Did you intend for fImporting to be set during LoadMempool?  It is now, but I thought that was a bug, indeed it causes pruning.py to fail b/c sync isn't happening during the potentially long process of loading the mempool.  (fImporting causes IBD to be true)
301 2016-11-09T18:54:02  <morcos> I was going to PR a fix, but I realize now that fee estimation conveniently ignores the LoadMempool txs due to this, so now it requires a lot more thinking about what the correct behavior is
302 2016-11-09T18:55:38  <sipa> morcos: hmm, IBD should probably not be true during LoadMempool
303 2016-11-09T18:59:28  *** davec has joined #bitcoin-core-dev
304 2016-11-09T19:16:32  *** davec has quit IRC
305 2016-11-09T19:18:47  *** davec has joined #bitcoin-core-dev
306 2016-11-09T19:22:12  *** laurentmt has joined #bitcoin-core-dev
307 2016-11-09T19:26:05  *** laurentmt has quit IRC
308 2016-11-09T19:32:45  *** Chris_Stewart_5 has quit IRC
309 2016-11-09T19:35:42  *** Chris_Stewart_5 has joined #bitcoin-core-dev
310 2016-11-09T19:37:58  *** btcdrak has quit IRC
311 2016-11-09T19:44:09  *** davec has quit IRC
312 2016-11-09T19:45:02  *** Guyver2 has quit IRC
313 2016-11-09T19:48:14  *** Guyver2 has joined #bitcoin-core-dev
314 2016-11-09T19:53:09  <gmaxwell> the build is now a _flood_ ofr shadowing warnings for me.
315 2016-11-09T19:53:25  <gmaxwell> thousands of them.
316 2016-11-09T19:53:57  <paveljanik> gmaxwell, what compiler? Can you upload the log please?
317 2016-11-09T19:54:05  <gmaxwell> gcc version 6.1.1 20160802 (Debian 6.1.1-11)
318 2016-11-09T19:54:07  <paveljanik> old gcc?
319 2016-11-09T19:54:26  <sipa> that's a very new gcc
320 2016-11-09T19:54:30  <sipa> i also get those warning
321 2016-11-09T19:55:17  *** davec has joined #bitcoin-core-dev
322 2016-11-09T19:55:21  <Chris_Stewart_5> Is the CHECKSIG operation missing in BIP141 for P2WPKH & P2WPKH nested inside of P2SH? https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#p2wpkh
323 2016-11-09T19:56:20  <paveljanik> can you please upload the log somewhere?
324 2016-11-09T19:57:10  <paveljanik> There is #8808 but maybe we need even more for new gcc
325 2016-11-09T19:57:12  <gribble> https://github.com/bitcoin/bitcoin/issues/8808 | Do not shadow variables (gcc set) by paveljanik · Pull Request #8808 · bitcoin/bitcoin · GitHub
326 2016-11-09T19:58:38  <gmaxwell> paveljanik: http://0bin.net/paste/C-XT2Ww24Gb0-oSk#gTvVzT2VMvRmjuKhVgAP25zWIpGHhJUbOKamb4KKFi-
327 2016-11-09T20:00:32  <paveljanik> fFlushOnClose is fixed there
328 2016-11-09T20:01:28  <paveljanik> gmaxwell, looks like you are good candidate for ACK on #8808 8)
329 2016-11-09T20:01:29  <gribble> https://github.com/bitcoin/bitcoin/issues/8808 | Do not shadow variables (gcc set) by paveljanik · Pull Request #8808 · bitcoin/bitcoin · GitHub
330 2016-11-09T20:01:43  <jonasschnelli> sipa: Okay. That's great news. I can re-focus then on the SPV mode.
331 2016-11-09T20:03:59  <gmaxwell> Where did we turn on these Wshadow warnings?
332 2016-11-09T20:06:17  <gmaxwell> oh, I'd already pulled it off while bisecting. 8794
333 2016-11-09T20:10:20  *** davec has quit IRC
334 2016-11-09T20:11:17  *** Chris_Stewart_5 has quit IRC
335 2016-11-09T20:15:13  <paveljanik> yes, but 8808 is missing.
336 2016-11-09T20:15:32  <paveljanik> and I'm now testing with a bit older gcc than yours...
337 2016-11-09T20:16:02  <sipa> i can test with 6.2.0
338 2016-11-09T20:16:26  <paveljanik> great! I will install the VM with some newer one later
339 2016-11-09T20:19:56  *** davec has joined #bitcoin-core-dev
340 2016-11-09T20:20:34  <phantomcircuit> gmaxwell: yeah we shadow things all over the place
341 2016-11-09T20:21:46  <sipa> let's fix it
342 2016-11-09T20:25:46  <wumpus> strange, I had no shadow warnings at all before I merged that
343 2016-11-09T20:26:09  <wumpus> or maybe one in leveldb or so, but certainly no flood
344 2016-11-09T20:26:12  <wumpus> I don't get it
345 2016-11-09T20:26:14  <wumpus> going to revert
346 2016-11-09T20:27:18  *** Chris_Stewart_5 has joined #bitcoin-core-dev
347 2016-11-09T20:27:23  <wumpus> maybe we shouldn't do it at all, it's too compiler-dependent
348 2016-11-09T20:28:07  <sipa> i think all the difficulty we have in figuring out how to resolve the -Wshadow warning exactly shows what problems actual unintentional shadowing could have
349 2016-11-09T20:28:36  <sipa> and there is a clear upper bound on what -Wshadow can mean... i'm a bit surprised there even is compiler dependence here
350 2016-11-09T20:28:43  <gmaxwell> In C it's utterly unambigious and fine, I'm less sure about C++ but I expect it's good to enable. But we should eliminate all the warnings first. Very weird that you weren't seeing them!
351 2016-11-09T20:29:05  <MarcoFalke> I could check #8808 with 6.2.1
352 2016-11-09T20:29:07  <gribble> https://github.com/bitcoin/bitcoin/issues/8808 | Do not shadow variables (gcc set) by paveljanik · Pull Request #8808 · bitcoin/bitcoin · GitHub
353 2016-11-09T20:29:14  <gmaxwell> (by all I mean almost all, of course, obviously I don't care about one in leveldb)
354 2016-11-09T20:29:16  <wumpus> I'm not going to bother with it anymore, at least
355 2016-11-09T20:29:38  <wumpus> not just me *multiple people* tested it here: https://github.com/bitcoin/bitcoin/pull/8794
356 2016-11-09T20:29:39  <bitcoin-git> [bitcoin] laanwj pushed 1 new commit to master: https://github.com/bitcoin/bitcoin/commit/f445d886126c00814c71b855fc2f36fdd7e11098
357 2016-11-09T20:29:39  <bitcoin-git> bitcoin/master f445d88 Wladimir J. van der Laan: Revert "Check and enable -Wshadow by default."...
358 2016-11-09T20:30:13  <wumpus> seems just needless busywork, how many 'prevent shadowing' commits have we had and it's still a problem
359 2016-11-09T20:31:54  <gmaxwell> Coding convention wise, I notice that 8808 adds _ to many local variables.  I think there moderately common convention for using _ on function parameters.
360 2016-11-09T20:33:21  <gmaxwell> Many other things are wshadow with no issues, but we have classes with many flag generic top level names. like "bits", "flags", "nversion"... and a codebase that hasn't had the warning for a long time.
361 2016-11-09T20:33:42  <wumpus> but those were supposed to be fixed already
362 2016-11-09T20:33:47  <wumpus> where are you getting all these warnings?
363 2016-11-09T20:34:13  <wumpus> can you maybe pastebin and post in that issue?
364 2016-11-09T20:34:23  <gmaxwell> I pastebinned the warnings above: http://0bin.net/paste/C-XT2Ww24Gb0-oSk#gTvVzT2VMvRmjuKhVgAP25zWIpGHhJUbOKamb4KKFi-
365 2016-11-09T20:34:26  <wumpus> maybe paveljanik would like to know
366 2016-11-09T20:34:38  <gmaxwell> Glad to help squash them all.
367 2016-11-09T20:34:59  <wumpus> jdid't we get rid of nVersion?
368 2016-11-09T20:35:11  <sipa> of the serialize.h-based nVersion
369 2016-11-09T20:35:21  <sipa> but there are still some nVersion variables scattered
370 2016-11-09T20:35:33  <wumpus> nah, I think there's more important things
371 2016-11-09T20:35:45  <wumpus> I thought this was done, but if there's another load of that bullshit
372 2016-11-09T20:36:09  <gmaxwell> I really would like to understand how compliler differences came into play.
373 2016-11-09T20:37:54  <wumpus> probably they consider slightly different scopes
374 2016-11-09T20:41:21  *** Chris_Stewart_5 has quit IRC
375 2016-11-09T20:42:16  * paveljanik back
376 2016-11-09T20:42:33  <paveljanik> I first done Wshadow clean build on OS X/clang
377 2016-11-09T20:42:39  <paveljanik> then tested on newer clang
378 2016-11-09T20:42:46  <paveljanik> than newer clang on Linux
379 2016-11-09T20:42:52  <paveljanik> then old gcc on Linux.
380 2016-11-09T20:43:14  <paveljanik> I'm now Wshadow clean on all clangs available to me and one gcc version (after 8808).
381 2016-11-09T20:43:36  <paveljanik> There are millions of differences in them emitting warnings.
382 2016-11-09T20:44:02  <wumpus> I tried with the gcc that comes with Ubuntu 16.04 (5.4.0) as well as clang 4.0 master git on Linux
383 2016-11-09T20:44:04  <MarcoFalke> I think gcc is more aggressive and also complains if a local var shadows a function name (such as size() or one of our own functions)
384 2016-11-09T20:44:41  <paveljanik> exactly!
385 2016-11-09T20:45:26  <wumpus> haven't tested gcc 6.x, maybe that is uselessly agressive
386 2016-11-09T20:47:08  *** jl2012 has quit IRC
387 2016-11-09T20:47:09  <wumpus> http://stackoverflow.com/questions/2958457/gcc-wshadow-is-too-strict hah
388 2016-11-09T20:47:28  *** jl2012 has joined #bitcoin-core-dev
389 2016-11-09T20:51:02  *** btcdrak has joined #bitcoin-core-dev
390 2016-11-09T20:55:26  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/f445d886126c...fb156100f9b4
391 2016-11-09T20:55:26  <bitcoin-git> bitcoin/master d8edf03 fanquake: Remove unused var UNLIKELY_PCT from fees.h
392 2016-11-09T20:55:27  <bitcoin-git> bitcoin/master fb15610 Wladimir J. van der Laan: Merge #9111: Remove unused variable UNLIKELY_PCT from fees.h...
393 2016-11-09T20:55:36  <bitcoin-git> [bitcoin] laanwj closed pull request #9111: Remove unused variable UNLIKELY_PCT from fees.h (master...remove-unused-unlikely-pct) https://github.com/bitcoin/bitcoin/pull/9111
394 2016-11-09T21:06:35  *** LeMiner has quit IRC
395 2016-11-09T21:07:26  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/fb156100f9b4...faec09bc7f03
396 2016-11-09T21:07:26  <bitcoin-git> bitcoin/master e5d682f jnewbery: Fix mininode version message format
397 2016-11-09T21:07:27  <bitcoin-git> bitcoin/master faec09b Wladimir J. van der Laan: Merge #8894: [Testing] Include fRelay in mininode version messages...
398 2016-11-09T21:07:35  <bitcoin-git> [bitcoin] dooglus opened pull request #9118: Remove requireGreater argment from TxConfirmStats::EstimateMedianVal() (master...remove-dead-code) https://github.com/bitcoin/bitcoin/pull/9118
399 2016-11-09T21:11:29  *** crescendo has joined #bitcoin-core-dev
400 2016-11-09T21:12:10  <bitcoin-git> [bitcoin] laanwj closed pull request #9048: [0.13 backport] Fix handling of invalid compact blocks (0.13...fix-invalid-cb-ban-0.13) https://github.com/bitcoin/bitcoin/pull/9048
401 2016-11-09T21:12:30  <morcos> wumpus: is it critical to be doing all this clean up on fee estimation right now?
402 2016-11-09T21:12:46  <morcos> i'd at least like a chance to think about it before you merge all these changes
403 2016-11-09T21:12:57  <morcos> i understand those variables are unused now
404 2016-11-09T21:13:18  <morcos> but they may still be useful functionality for further extension of the fee estimation code
405 2016-11-09T21:14:39  <wumpus> morcos: no, imo it's not necessary, but I haven't heared you complain about it before
406 2016-11-09T21:14:47  <wumpus> you could just NACK those pulls
407 2016-11-09T21:15:04  <wumpus> if no one complains then dead code cleanups seems like a no-brainer
408 2016-11-09T21:15:10  <morcos> i guess i just panicked b/c i saw one of them got merged before i even saw it
409 2016-11-09T21:15:15  <morcos> within a 12 hours
410 2016-11-09T21:15:20  <morcos> but that one actually was trivial
411 2016-11-09T21:15:22  <wumpus> that was just a one-liner
412 2016-11-09T21:15:23  <morcos> i will comment on the other
413 2016-11-09T21:15:35  <morcos> i hadn't actually looked at the changes on either of them yet
414 2016-11-09T21:15:53  <wumpus> also our usual policy is to remove dead code, we're using git to make it easy to bring back things if necessary
415 2016-11-09T21:15:59  <morcos> but yes i'm in favor of the priority removal that was done of course, but the require greater logic was tricky to get right the first time
416 2016-11-09T21:16:08  <morcos> i'd rather leave it until we're sure we won't want it again
417 2016-11-09T21:16:20  <wumpus> I'll just stop merging things now, only caused problems today I feel :/
418 2016-11-09T21:18:54  <wumpus> just too many pulls...
419 2016-11-09T21:22:20  <morcos> wumpus: i'm impressed you keep up as well as you do..  my head is spinning...  i started today trying to doing a posthumous ACK to the serialization cleanups, and i still haven't gotten around to looking at the 2nd commit.
420 2016-11-09T21:25:08  <wumpus> I also have a harder and harder time keeping up
421 2016-11-09T21:26:21  <morcos> i suppose its a good problem to have
422 2016-11-09T21:26:48  <gmaxwell> wumpus: re the Wshadow warning, is it possible that the files aren't dirty when you change the flag so make isn't rebuilding them?
423 2016-11-09T21:27:18  <gmaxwell> I was rebasing a branch from a distant version when I noticed the warngings, so my tree had been effectively make cleaned.
424 2016-11-09T21:27:21  <bitcoin-git> [bitcoin] laanwj closed pull request #9118: Remove requireGreater argment from TxConfirmStats::EstimateMedianVal() (master...remove-dead-code) https://github.com/bitcoin/bitcoin/pull/9118
425 2016-11-09T21:27:30  <wumpus> gmaxwell: no, I cleaned both the tree and the ccache, for both compilers
426 2016-11-09T21:28:08  <wumpus> but I don't want to bother with the Wshadow warnings anymore, let's focus the rare time we have on more important things
427 2016-11-09T21:31:02  <paveljanik> gmaxwell, can you please try http://tmp.janik.cz/q.diff?
428 2016-11-09T21:31:14  <paveljanik> it should clear the nVersion from chain.h and others.
429 2016-11-09T21:33:02  <gmaxwell> paveljanik: the only ones remaining with that are leveldb and bench/lockedpool.cpp
430 2016-11-09T21:33:16  <paveljanik> gmaxwell, great!
431 2016-11-09T21:33:41  <paveljanik> bench/lockedpool is clear IIRC.
432 2016-11-09T21:33:46  <paveljanik> and leveldb is upstream 8)
433 2016-11-09T21:33:49  <paveljanik> so fixed? ;-)
434 2016-11-09T21:34:06  <gmaxwell> paveljanik: I see you're renaming local's to _local.  That is a violation of the common convention in other codebases that function paramters get _prefixes.
435 2016-11-09T21:34:08  <paveljanik> but I'll first retest on some rolling upstream Linux distro
436 2016-11-09T21:34:35  <paveljanik> we used that even before and people agreed on it.
437 2016-11-09T21:34:53  <paveljanik> some parts of the code use _, some _in, some In postfix..
438 2016-11-09T21:35:15  <gmaxwell> Were we really using it for _local_ variables before, not just function parameters?
439 2016-11-09T21:35:21  <paveljanik> yes
440 2016-11-09T21:35:34  <paveljanik> I was inspired by it.
441 2016-11-09T21:36:17  <paveljanik> but sometimes, it was better to rename the variables...
442 2016-11-09T21:36:23  <paveljanik> and thus I did so
443 2016-11-09T21:36:38  <paveljanik> e.g. data -> vData to follow the typeName conventions used
444 2016-11-09T21:37:51  <paveljanik> I understand it is not sexy to read all those diffs though 8)
445 2016-11-09T21:38:01  <paveljanik> but please be patient
446 2016-11-09T21:38:54  <gmaxwell> paveljanik: for example, in 8808  src/streams.h
447 2016-11-09T21:39:37  <paveljanik> yes, read -> _read.
448 2016-11-09T21:39:44  <gmaxwell> you change read to _read  it would be more ideomatic to change it to someghing like nBytes or bread or nread or something like that.
449 2016-11-09T21:40:38  <paveljanik> no problem, just comment there and I will change it ;-)
450 2016-11-09T21:41:20  <gmaxwell> When you make these updates are you checking to make sure the existing code wasn't buggy?
451 2016-11-09T21:41:49  <paveljanik> sometimes. I even found some bugs, yes.
452 2016-11-09T21:42:24  <gmaxwell> paveljanik: Good. I'll gladly leave some comments.
453 2016-11-09T21:50:44  *** MarcoFalke has left #bitcoin-core-dev
454 2016-11-09T22:04:48  *** Guyver2 has quit IRC
455 2016-11-09T22:04:56  *** Guyver2 has joined #bitcoin-core-dev
456 2016-11-09T22:05:12  <bitcoin-git> [bitcoin] UdjinM6 opened pull request #9120: bug: Missed one "return false" in recent refactoring in #9067 (master...fixExitCodesBitcoinTx) https://github.com/bitcoin/bitcoin/pull/9120
457 2016-11-09T22:07:17  <paveljanik> gmaxwell, thank you!
458 2016-11-09T22:12:12  *** belcher has joined #bitcoin-core-dev
459 2016-11-09T22:26:45  <jtimon> morcos: no, I'm still getting my weird error on walletbackup even with #9058, really killall bitcoind ; rm -rf ./qa/cache ; doesn't seem enough
460 2016-11-09T22:26:46  <gribble> https://github.com/bitcoin/bitcoin/issues/9058 | Fixes for p2p-compactblocks.py test timeouts on travis (#8842) by ryanofsky · Pull Request #9058 · bitcoin/bitcoin · GitHub
461 2016-11-09T22:27:20  <morcos> jtimon: not for me right?
462 2016-11-09T22:37:24  *** Chris_Stewart_5 has joined #bitcoin-core-dev
463 2016-11-09T22:43:18  <jtimon> reminder: I only get it by running all rpc tests, not running walletbackupt individually
464 2016-11-09T22:43:48  <jtimon> you told me 9058 may help the other day, but it seems is unrelated :(
465 2016-11-09T22:46:03  <jtimon> I'm locally getting errors on your branch, but I was getting them already and at this point I'm just doing something wrong with rpc tests somehow, so yeah, sorry for the too long explanation
466 2016-11-09T22:51:01  <jtimon> btw, general thoughts on moving some fast extended tests to the common ones and maybe also some slow ones from the common ones to the extended ones?
467 2016-11-09T22:56:19  <jtimon> re warnings you can still fix some shadowing even if you don't put the new flag yet
468 2016-11-09T23:00:23  *** Guyver2 has quit IRC
469 2016-11-09T23:37:20  *** davec has quit IRC
470 2016-11-09T23:42:05  *** cryptapus has joined #bitcoin-core-dev
471 2016-11-09T23:43:36  *** davec has joined #bitcoin-core-dev
472 2016-11-09T23:45:20  <jtimon> I'm starting to think it may not be just be but actually something between 1adf82ac and faec09bc
473 2016-11-09T23:46:09  *** cdecker has quit IRC
474 2016-11-09T23:47:00  *** cryptapus has quit IRC
475 2016-11-09T23:53:34  <jtimon> s/just be/just me/
476 2016-11-09T23:57:58  *** btcdrak has quit IRC