1 2017-02-02T00:29:17  *** AaronvanW has quit IRC
  2 2017-02-02T00:41:55  *** Squidicc is now known as squidicuz
  3 2017-02-02T01:03:28  *** waxwing has joined #bitcoin-core-dev
  4 2017-02-02T01:07:53  *** wasi has quit IRC
  5 2017-02-02T01:47:07  *** waxwing has quit IRC
  6 2017-02-02T01:47:32  *** Ylbam has quit IRC
  7 2017-02-02T02:22:47  *** Chris_Stewart_5 has quit IRC
  8 2017-02-02T02:39:32  *** Chris_Stewart_5 has joined #bitcoin-core-dev
  9 2017-02-02T02:57:50  *** waxwing has joined #bitcoin-core-dev
 10 2017-02-02T03:00:02  *** abpa has quit IRC
 11 2017-02-02T03:00:03  *** dermoth has quit IRC
 12 2017-02-02T03:00:49  *** dermoth has joined #bitcoin-core-dev
 13 2017-02-02T03:18:21  *** CubicEarth has quit IRC
 14 2017-02-02T03:19:26  *** CubicEarth has joined #bitcoin-core-dev
 15 2017-02-02T04:51:58  *** chjj has quit IRC
 16 2017-02-02T04:51:59  *** windsok has quit IRC
 17 2017-02-02T04:53:53  *** chris2000 has joined #bitcoin-core-dev
 18 2017-02-02T04:57:11  *** chris200_ has quit IRC
 19 2017-02-02T05:00:08  *** dermoth has quit IRC
 20 2017-02-02T05:00:59  *** dermoth has joined #bitcoin-core-dev
 21 2017-02-02T05:01:37  *** windsok has joined #bitcoin-core-dev
 22 2017-02-02T05:19:26  *** chjj has joined #bitcoin-core-dev
 23 2017-02-02T05:21:43  *** abpa has joined #bitcoin-core-dev
 24 2017-02-02T05:40:17  *** owowo has quit IRC
 25 2017-02-02T05:45:01  *** owowo has joined #bitcoin-core-dev
 26 2017-02-02T05:49:27  *** jtimon has quit IRC
 27 2017-02-02T05:55:26  *** kadoban has quit IRC
 28 2017-02-02T06:09:37  *** echonaut4 has joined #bitcoin-core-dev
 29 2017-02-02T06:09:51  *** pavel_ has joined #bitcoin-core-dev
 30 2017-02-02T06:11:31  *** sanada` has joined #bitcoin-core-dev
 31 2017-02-02T06:12:22  *** paveljanik has quit IRC
 32 2017-02-02T06:12:22  *** cysm has quit IRC
 33 2017-02-02T06:12:48  *** paracyst_ has joined #bitcoin-core-dev
 34 2017-02-02T06:12:58  *** echonaut has quit IRC
 35 2017-02-02T06:12:58  *** bsm117532 has quit IRC
 36 2017-02-02T06:12:58  *** sanada has quit IRC
 37 2017-02-02T06:13:34  *** paracyst has quit IRC
 38 2017-02-02T06:13:46  *** cysm has joined #bitcoin-core-dev
 39 2017-02-02T06:40:28  <bitcoin-git> [bitcoin] sosochain opened pull request #9669: 0.13 (master...0.13) https://github.com/bitcoin/bitcoin/pull/9669
 40 2017-02-02T06:41:23  <bitcoin-git> [bitcoin] fanquake closed pull request #9669: 0.13 (master...0.13) https://github.com/bitcoin/bitcoin/pull/9669
 41 2017-02-02T06:45:32  *** pavel_ has quit IRC
 42 2017-02-02T06:50:58  *** lclc has joined #bitcoin-core-dev
 43 2017-02-02T07:00:55  *** Ylbam has joined #bitcoin-core-dev
 44 2017-02-02T07:16:17  *** paveljanik has joined #bitcoin-core-dev
 45 2017-02-02T07:16:18  *** paveljanik has joined #bitcoin-core-dev
 46 2017-02-02T07:39:40  *** isle2983 has quit IRC
 47 2017-02-02T07:48:17  *** waxwing has quit IRC
 48 2017-02-02T08:04:39  *** BashCo has quit IRC
 49 2017-02-02T08:26:40  *** BashCo has joined #bitcoin-core-dev
 50 2017-02-02T08:55:02  <bitcoin-git> [bitcoin] laanwj closed pull request #9658: Add clang_format.py to help automate code style analysis (master...PR-clang-format) https://github.com/bitcoin/bitcoin/pull/9658
 51 2017-02-02T08:55:08  *** AaronvanW has joined #bitcoin-core-dev
 52 2017-02-02T09:00:57  <bitcoin-git> [bitcoin] laanwj closed pull request #9632: Add clang_static_analysis.py to help automate static analysis runs (master...PR-clang-static) https://github.com/bitcoin/bitcoin/pull/9632
 53 2017-02-02T09:06:15  *** MarcoFalke has joined #bitcoin-core-dev
 54 2017-02-02T09:13:48  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/77bd8c4cab67...e30d9287fd48
 55 2017-02-02T09:13:49  <bitcoin-git> bitcoin/master 3eba88d Gregory Sanders: clarify listunspent amount description
 56 2017-02-02T09:13:49  <bitcoin-git> bitcoin/master e30d928 Wladimir J. van der Laan: Merge #9663: [RPC] clarify listunspent amount description...
 57 2017-02-02T09:14:05  <bitcoin-git> [bitcoin] laanwj closed pull request #9663: [RPC] clarify listunspent amount description (master...listoutput) https://github.com/bitcoin/bitcoin/pull/9663
 58 2017-02-02T09:17:13  *** jannes has joined #bitcoin-core-dev
 59 2017-02-02T09:19:44  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/e30d9287fd48...ae972a5e996a
 60 2017-02-02T09:19:44  <bitcoin-git> bitcoin/master b9d95bd Douglas Roark: Fix various minor linearization script issues...
 61 2017-02-02T09:19:45  <bitcoin-git> bitcoin/master ae972a5 Wladimir J. van der Laan: Merge #9580: Fix various minor linearization script issues...
 62 2017-02-02T09:19:56  <bitcoin-git> [bitcoin] laanwj closed pull request #9580: Fix various minor linearization script issues (master...linearizefix) https://github.com/bitcoin/bitcoin/pull/9580
 63 2017-02-02T09:43:32  *** rabidus_ is now known as rabidus
 64 2017-02-02T09:43:53  *** Guyver2 has joined #bitcoin-core-dev
 65 2017-02-02T10:00:50  *** lclc has quit IRC
 66 2017-02-02T10:32:02  *** harrymm has quit IRC
 67 2017-02-02T10:47:42  *** harrymm has joined #bitcoin-core-dev
 68 2017-02-02T10:58:21  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ae972a5e996a...4e19efba0331
 69 2017-02-02T10:58:21  <bitcoin-git> bitcoin/master 8fc6989 practicalswift: Remove redundant semicolons
 70 2017-02-02T10:58:22  <bitcoin-git> bitcoin/master 4e19efb Wladimir J. van der Laan: Merge #9556: Remove redundant semicolons...
 71 2017-02-02T10:58:36  <bitcoin-git> [bitcoin] laanwj closed pull request #9556: Remove redundant semicolons (master...remove-redundant-braces) https://github.com/bitcoin/bitcoin/pull/9556
 72 2017-02-02T11:41:34  *** lclc has joined #bitcoin-core-dev
 73 2017-02-02T12:05:13  <bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/4e19efba0331...7c93952feccb
 74 2017-02-02T12:05:14  <bitcoin-git> bitcoin/master 3e900ac Matt Corallo: Require merge commits merge branches on top of other merge commits...
 75 2017-02-02T12:05:14  <bitcoin-git> bitcoin/master ba94426 Matt Corallo: Test that pushes to bitcoin/bitcoin are signed per verify-commits
 76 2017-02-02T12:05:15  <bitcoin-git> bitcoin/master 7c93952 Wladimir J. van der Laan: Merge #9656: Check verify-commits on pushes to master...
 77 2017-02-02T12:05:31  <bitcoin-git> [bitcoin] laanwj closed pull request #9656: Check verify-commits on pushes to master (master...2017-01-fix-verify-commits) https://github.com/bitcoin/bitcoin/pull/9656
 78 2017-02-02T12:14:45  <bitcoin-git> [bitcoin] laanwj opened pull request #9670: contrib: github-merge improvements (master...2017_01_ghmerge_update) https://github.com/bitcoin/bitcoin/pull/9670
 79 2017-02-02T12:24:26  *** laurentmt has joined #bitcoin-core-dev
 80 2017-02-02T12:24:46  *** laurentmt has quit IRC
 81 2017-02-02T12:26:25  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/7c93952feccb...1c2edd9f6707
 82 2017-02-02T12:26:25  <bitcoin-git> bitcoin/master 178454d Jorge Timón: Contrib: Add jtimon pgp keys for commit sigs and future gitian builds
 83 2017-02-02T12:26:26  <bitcoin-git> bitcoin/master 1c2edd9 Wladimir J. van der Laan: Merge #9654: Add jtimon pgp keys for commit sigs and future gitian builds...
 84 2017-02-02T12:26:41  <bitcoin-git> [bitcoin] laanwj closed pull request #9654: Add jtimon pgp keys for commit sigs and future gitian builds (master...jtimon-key-gpg) https://github.com/bitcoin/bitcoin/pull/9654
 85 2017-02-02T13:08:48  *** MarcoFalke has quit IRC
 86 2017-02-02T13:16:27  *** lclc has quit IRC
 87 2017-02-02T13:36:48  *** laurentmt has joined #bitcoin-core-dev
 88 2017-02-02T13:37:40  *** laurentmt has quit IRC
 89 2017-02-02T13:38:20  *** CubicEarth has quit IRC
 90 2017-02-02T14:01:58  *** isle2983 has joined #bitcoin-core-dev
 91 2017-02-02T14:27:33  *** Sosumi has joined #bitcoin-core-dev
 92 2017-02-02T14:27:56  <BlueMatt> <BlueMatt> #9650 (and, by extension, #9634) are probably 0.14
 93 2017-02-02T14:27:58  <gribble> https://github.com/bitcoin/bitcoin/issues/9650 | Better handle invalid parameters to signrawtransaction by TheBlueMatt · Pull Request #9650 · bitcoin/bitcoin · GitHub
 94 2017-02-02T14:28:00  <gribble> https://github.com/bitcoin/bitcoin/issues/9634 | Fail in DecodeHexTx if there is extra data at the end by jtimon · Pull Request #9634 · bitcoin/bitcoin · GitHub
 95 2017-02-02T14:28:03  <BlueMatt> (ie need tag)
 96 2017-02-02T14:28:33  *** waxwing has joined #bitcoin-core-dev
 97 2017-02-02T14:29:04  <wumpus> tagged
 98 2017-02-02T14:29:08  <BlueMatt> thanks
 99 2017-02-02T14:29:40  <wumpus> 9634 seems ready, it would be nice if it had a test though - this seems like something that could regress early if it's not caught by the tests
100 2017-02-02T14:53:34  *** jtimon has joined #bitcoin-core-dev
101 2017-02-02T15:06:01  <wumpus> it doesn't necessarily need to hold up the pull request, but there's not been a reply from the author yet
102 2017-02-02T15:11:51  *** handlex has joined #bitcoin-core-dev
103 2017-02-02T15:34:12  *** handlex has quit IRC
104 2017-02-02T15:46:28  *** Chris_Stewart_5 has quit IRC
105 2017-02-02T15:47:30  *** handlex has joined #bitcoin-core-dev
106 2017-02-02T15:52:06  *** handlex has joined #bitcoin-core-dev
107 2017-02-02T16:02:01  *** handlex has quit IRC
108 2017-02-02T16:02:59  *** Chris_Stewart_5 has joined #bitcoin-core-dev
109 2017-02-02T16:07:31  *** laurentmt has joined #bitcoin-core-dev
110 2017-02-02T16:07:36  *** laurentmt has quit IRC
111 2017-02-02T16:34:35  <BlueMatt> wumpus: I'll take a look at doing that in a bit
112 2017-02-02T16:35:48  *** waxwing has quit IRC
113 2017-02-02T16:35:51  <BlueMatt> question, would anyone object to me trying to add something like https://github.com/TheBlueMatt/RelayNode/blob/next/c%2B%2B/preinclude.h to bitcoin? it makes atomics replaceable with either std::atomic or a custom thing which has valgrind annotations so that helgrind/drd actually work and dont spew spurious crap all the time
114 2017-02-02T16:36:05  <BlueMatt> means atomics get declared with DECLARE_ATOMIC instead of std::atomic x, though
115 2017-02-02T16:36:29  <BlueMatt> well, suppose they could be OUR_ATOMIC_OR_NOT, but either way
116 2017-02-02T16:37:14  <sipa> why use macros?
117 2017-02-02T16:37:27  <BlueMatt> because otherwise you override std::atomic?
118 2017-02-02T16:37:40  <BlueMatt> i mean you can say all atomics are now of type MACRO_ATOMIC_TYPE
119 2017-02-02T16:38:31  <sipa> no, if you're going to switch all places where atomocs are used to macros, why not switch them with a self defined class directly
120 2017-02-02T16:38:52  <sipa> and when you don't need the self defined type, have it be typedefed to be std::atomic
121 2017-02-02T16:39:03  <BlueMatt> yea, well same thing either way
122 2017-02-02T16:42:03  <BlueMatt> also, due to the presence of _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE, that file would need to be included everywhere prior to the inclusion of any std includes
123 2017-02-02T16:42:09  <BlueMatt> well, the atomics could be in a separate file
124 2017-02-02T16:42:14  <BlueMatt> but...
125 2017-02-02T16:48:42  *** waxwing has joined #bitcoin-core-dev
126 2017-02-02T16:50:02  *** abpa has quit IRC
127 2017-02-02T17:06:41  *** handlex has joined #bitcoin-core-dev
128 2017-02-02T17:16:23  *** nickler has quit IRC
129 2017-02-02T17:17:00  *** nickler has joined #bitcoin-core-dev
130 2017-02-02T17:27:31  *** kadoban has joined #bitcoin-core-dev
131 2017-02-02T17:37:45  <isle2983> please don't talk about coding style at the meeting
132 2017-02-02T17:38:03  <isle2983> I have some automation pieces coming togeher, but it might take a little while to get it done
133 2017-02-02T17:38:21  <wumpus> BlueMatt: wouldn't it be possible to do this with std::atomic? I like the move towards using standard primitives instead of self-defined ttpes
134 2017-02-02T17:38:26  <isle2983> I am working near-full-time on this in the short term
135 2017-02-02T17:39:10  <isle2983> a viable end objective might be a 'Nit Bot' that can analize the commits from a PR and detect a few categories of things
136 2017-02-02T17:39:39  <isle2983> it looks like you can load a github acct auth token into the TravisCI account as an env var
137 2017-02-02T17:39:56  <isle2983> so, securely the travisCI script can post content back to github
138 2017-02-02T17:40:43  <wumpus> are you sure that's worth spending so much work on? there are some much more pressing issues than code style
139 2017-02-02T17:41:20  <wumpus> we've considered using that but the TravisCI token stuff is easy to circumvent if it's used for testing pulls - the test script for the pull could just output the token or upload it somewhere
140 2017-02-02T17:43:21  <isle2983> I am not sure it is the ultimate thing, but there is a cost to the project - you spent 20 minutes talking about it last week
141 2017-02-02T17:43:44  <isle2983> and there is some annoyance at trivial pulls
142 2017-02-02T17:43:46  <wumpus> that's a rarity, usually it doesn't come up at all
143 2017-02-02T17:43:48  *** abpa has joined #bitcoin-core-dev
144 2017-02-02T17:44:08  <isle2983> if it can be automated, it should be automated at some point
145 2017-02-02T17:45:27  <isle2983> I am also of the school that says code is written once, and read 100s of times afterwards, so make reading easy
146 2017-02-02T17:45:46  <isle2983> probably 100,000s of times afterwards for bitcoin
147 2017-02-02T17:45:53  <wumpus> BlueMatt: in general the more 'you need to use this special type' rules a project has, the harder it is to contribute to. Though making it easier to valgrind/helgrind is certainly a valuable goal.
148 2017-02-02T17:46:23  *** bsm117532 has joined #bitcoin-core-dev
149 2017-02-02T17:49:15  <wumpus> isle2983: all too true, but I don't think most of the (superficial) code style makes much difference to that. Having a sensible design and the logic and the reason for things documented/commented is what is important to understanding difficult code
150 2017-02-02T17:54:29  <isle2983> totally agreed, but I see that as all the more reason to filter out distractions so that attention can focus 100% on the important things
151 2017-02-02T17:54:40  <wumpus> focusing too much on moving around spaces and such takes away the focus from what really matters, and it's much easier so it easily crowds out deeper thinking
152 2017-02-02T17:55:30  <wumpus> well as I see it a 'style nit bot' would add distractions, not remove them
153 2017-02-02T17:57:22  <gmaxwell> talking about pointless stuff is good for community. :)
154 2017-02-02T17:57:40  <wumpus> unless it annoys people
155 2017-02-02T17:58:50  <isle2983> in the short term, perhaps. but in the long term it would train submitters to get the PR right offline. Using common tools and scripts also lets the coders just integrate it into their text editor so they don't have to think.
156 2017-02-02T17:59:16  <wumpus> there's two ways to go at this sanely: one is to use something like clang-format from the begining. For example in golang projects everyone uses the same formatter with the same style, preventing any disagreements about style. The other is to do best effort and just not worry too much about it. It's too late for the former.
157 2017-02-02T17:59:31  <wumpus> our goal is not to 'train submitters'
158 2017-02-02T18:00:52  <wumpus> it's just importing concerns that shouldn't be part of this, creating extra bureaucratic barriers
159 2017-02-02T18:01:47  <BlueMatt> wumpus: yea, I tend to agree, I'm not sure I really want to do it, but if valgrind doesnt get an update to actually support std::atomic I might have to do it
160 2017-02-02T18:03:57  <gmaxwell> wumpus: I wouldn't worry too much; thats what review is for.. and that is the kind of thing everyone will spot, no one should mind fixing up, especially since it would be visible in practice all over...  Obviously preferable to not.
161 2017-02-02T18:05:21  <wumpus> gmaxwell: I do worry about it. We've had this problem in the past with a certain person commenting on every pull about e.g. sorting include headers, whitespace, and so on, and it was incredibly annoying
162 2017-02-02T18:07:09  <gmaxwell> ah, I was commenting more on the macro for atomics.
163 2017-02-02T18:07:23  <gmaxwell> Yes. It certantly doesn't work if everyeone doesn't support it, especially if its merely cosmetic.
164 2017-02-02T18:08:00  <wumpus> right, I'm not worried about using a custom type for std::atomic, it shouldn't be used that much anyway. THough it seems to be something a tool should just handle.
165 2017-02-02T18:08:08  <gmaxwell> but "make valgrind usable to help us find data-races" is a substantial boon I hope we could all support. :)
166 2017-02-02T18:09:52  <wumpus> sure
167 2017-02-02T18:10:27  <wumpus> though it seems reasonably important to have it work for std::atomic so that the tool can analyse all projects using it, without needing special support at that side
168 2017-02-02T18:13:58  *** lclc has joined #bitcoin-core-dev
169 2017-02-02T18:18:30  <cfields> wumpus: i wonder if that one, specifically, gets solved with newer versions of std libraries. For ex, iirc newer libc++ adds attributes to std::mutex/std::lock, etc, so that we won't need the wrappers in threadsafety.h
170 2017-02-02T18:18:43  <isle2983> I am happy to talk more about style automation on side channels - or also hear suggestions for what else I could be working on. I am in the learning phase and am open to anything that needs an extra brain and can help the project. Yielding now for important topics. Thanks.
171 2017-02-02T18:19:14  *** Evel-Laptop has joined #bitcoin-core-dev
172 2017-02-02T18:20:45  *** CubicEarth has joined #bitcoin-core-dev
173 2017-02-02T18:21:53  *** paveljanik has quit IRC
174 2017-02-02T18:22:32  <wumpus> isle2983: to be clear I'm not trying to bash your work, just trying to set expectation how these kind of things are received, so you don't spend a lot of work automating something that won't be used
175 2017-02-02T18:23:49  <wumpus> cfields: indeed it could also be improved from the side of the library
176 2017-02-02T18:24:13  <cfields> yes, found it: https://reviews.llvm.org/D14731
177 2017-02-02T18:24:39  <cfields> i guess runtime tools wouldn't get access to those attributes, though
178 2017-02-02T18:25:32  <wumpus> they could if it would be exposed in debug metadata in some way
179 2017-02-02T18:26:28  <wumpus> isle2983: as for things to be working on, improving the tests is always very welcome :)
180 2017-02-02T18:28:44  <Chris_Stewart_5> isle2983: If you are interested, I'm working on a new testing paradigm in core in #8469
181 2017-02-02T18:28:47  <gribble> https://github.com/bitcoin/bitcoin/issues/8469 | [POC] Introducing property based testing to Core by Christewart · Pull Request #8469 · bitcoin/bitcoin · GitHub
182 2017-02-02T18:29:59  <Chris_Stewart_5> I've found it to be a useful way to get familiar with the core codebase as well
183 2017-02-02T18:30:16  <Chris_Stewart_5> wrt to what wumpus said about improving/adding tests
184 2017-02-02T18:30:35  <isle2983> wumpus: hey no worries. I wouldn't want to work on a project that didn't have strong skepticism and pushback
185 2017-02-02T18:31:04  <isle2983> Chris_Stewart_5: cool, I will take a peek. Thanks.
186 2017-02-02T18:34:17  <sipa> BlueMatt: are relaxed reads from an atomic even recognizable from the binary?
187 2017-02-02T18:34:39  <BlueMatt> relaxed I dont know, but certainly most reads/writes show up in the st as load()/store()
188 2017-02-02T18:35:50  <BlueMatt> its unclear to me whether helgrind is just being overly conservative and reporting them anyway because they could still be logic-races, or whether helgrind also isnt taking into account the ordering there
189 2017-02-02T18:36:16  <BlueMatt> its also unclear to me whether libc++ is doing the right thing with _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE set in their atomics and I just dont have that mapped to helgrind's equivalent
190 2017-02-02T18:36:23  <BlueMatt> which may be an easy fix
191 2017-02-02T18:36:41  <BlueMatt> (but that hadnt previously fixed it months ago when I was doing that for the relay network code)
192 2017-02-02T18:37:19  *** handlex has joined #bitcoin-core-dev
193 2017-02-02T18:38:08  *** lclc has quit IRC
194 2017-02-02T18:38:14  *** handlex has quit IRC
195 2017-02-02T18:46:15  *** Guyver2 has quit IRC
196 2017-02-02T18:48:47  *** GAit has joined #bitcoin-core-dev
197 2017-02-02T18:55:42  <bitcoin-git> [bitcoin] TheBlueMatt opened pull request #9671: Fix super-unlikely race introduced in 236618061a445d2cb11e72 (master...2017-02-fix-initnode-race) https://github.com/bitcoin/bitcoin/pull/9671
198 2017-02-02T18:55:45  <BlueMatt> cfields: I blame you for ^, btw, you asked me to do it :p
199 2017-02-02T18:59:19  <cfields> BlueMatt: In my defense I'm pretty sure i only said that it was ugly and that it should be cleaned up later, but I didn't complain when you went ahead and moved it either. So I'll take the blame :)
200 2017-02-02T18:59:50  *** MarcoFalke has joined #bitcoin-core-dev
201 2017-02-02T18:59:52  <BlueMatt> lol, yea, well /someone/ should have bothered to look at the implications :(
202 2017-02-02T19:00:02  <wumpus> #startmeeting
203 2017-02-02T19:00:02  <lightningbot> Meeting started Thu Feb  2 19:00:02 2017 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
204 2017-02-02T19:00:02  <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
205 2017-02-02T19:00:07  <jonasschnelli> hi
206 2017-02-02T19:00:08  <BlueMatt> oh thats today?
207 2017-02-02T19:00:18  <gmaxwell> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier
208 2017-02-02T19:00:29  <sipa> yow.
209 2017-02-02T19:00:37  <luke-jr> BlueMatt: no, it's fake news.
210 2017-02-02T19:00:37  <wumpus> BlueMatt: it's thursday I hope?
211 2017-02-02T19:00:38  <luke-jr> /s
212 2017-02-02T19:00:49  <sipa> luke-jr: alternative news
213 2017-02-02T19:00:52  <BlueMatt> wumpus: alternative facts
214 2017-02-02T19:00:53  <BlueMatt> heh
215 2017-02-02T19:00:59  <sipa> jinx
216 2017-02-02T19:01:28  <sipa> i don't have topics
217 2017-02-02T19:01:32  <wumpus> foremost topic would be what to still include in 0.14, as rc1 release is planned for monday
218 2017-02-02T19:02:09  <gmaxwell> I propose not including any bugs.
219 2017-02-02T19:02:13  <BlueMatt> I think the current list (+#9671) are going to be required for release, sooooo
220 2017-02-02T19:02:15  <gribble> https://github.com/bitcoin/bitcoin/issues/9671 | Fix super-unlikely race introduced in 236618061a445d2cb11e72 by TheBlueMatt · Pull Request #9671 · bitcoin/bitcoin · GitHub
221 2017-02-02T19:02:25  <jonasschnelli> These are the issues tagged for 0.14 : https://github.com/bitcoin/bitcoin/issues?q=is%3Aopen+is%3Aissue+milestone%3A0.14.0
222 2017-02-02T19:02:34  <MarcoFalke> I think only remaining is the net stuff?
223 2017-02-02T19:03:02  <cfields> rebasing 9609 now.
224 2017-02-02T19:03:14  <instagibbs> hi
225 2017-02-02T19:03:25  <BlueMatt> MarcoFalke: we added the signrawtransaction assertion too
226 2017-02-02T19:03:25  <wumpus> do we have a fix for #9027 in the works yet?
227 2017-02-02T19:03:26  <gribble> https://github.com/bitcoin/bitcoin/issues/9027 | Unbounded reorg memory usage · Issue #9027 · bitcoin/bitcoin · GitHub
228 2017-02-02T19:03:27  <jonasschnelli> Imo the importmulti fixes can be done after 0.14
229 2017-02-02T19:03:28  <BlueMatt> and importmulti
230 2017-02-02T19:03:29  <cfields> probably a good time to call for release notes additions
231 2017-02-02T19:03:33  <BlueMatt> wumpus: I vote we push that one
232 2017-02-02T19:03:36  <BlueMatt> (to 0.15)
233 2017-02-02T19:03:48  <wumpus> BlueMatt: ok, no problem with that
234 2017-02-02T19:03:55  <BlueMatt> because not-regression, it turns out
235 2017-02-02T19:04:03  <sipa> not regression?
236 2017-02-02T19:04:07  <BlueMatt> slightly worse than it tused to be, but not so massively so that I think we definitely need to fix it asap
237 2017-02-02T19:04:09  <gmaxwell> changing the API in the very next release would be unfortunate.
238 2017-02-02T19:04:24  <achow101> are the three rpc things necessary for this release?
239 2017-02-02T19:04:24  <BlueMatt> gmaxwell: context?
240 2017-02-02T19:04:29  <BlueMatt> sipa: you're the one who decided it wasnt!
241 2017-02-02T19:04:30  <jonasschnelli> gmaxwell: importmulti?
242 2017-02-02T19:04:32  <sipa> gmaxwell: fixing 9027 does not need an api change afaik
243 2017-02-02T19:04:42  <sipa> BlueMatt: oh!
244 2017-02-02T19:05:04  <wumpus> we could disable importmulti if it isn't safe in time for the release
245 2017-02-02T19:05:31  <sipa> or leave it undocumented?
246 2017-02-02T19:05:44  <wumpus> depends on whether the issue of #9491 is serious enough
247 2017-02-02T19:05:45  <gribble> https://github.com/bitcoin/bitcoin/issues/9491 | Importmulti api is confusing in a way that could lead to funds loss. · Issue #9491 · bitcoin/bitcoin · GitHub
248 2017-02-02T19:05:58  <gmaxwell> fixing the fact that it's very easy to fail to rescan anything, when you thought it was... does.
249 2017-02-02T19:06:22  <wumpus> yes undocumented or could add a "warning: experimental, API will likely change next release" in any case too
250 2017-02-02T19:06:41  <jonasschnelli> Or we just fix 9491... seems not very complex?
251 2017-02-02T19:06:51  <jonasschnelli> Can fix in rc2 if it's to late for monday?
252 2017-02-02T19:07:26  <wumpus> sure
253 2017-02-02T19:07:36  <wumpus> but there's no guarantee there is a rc2
254 2017-02-02T19:07:40  <gmaxwell> okay, lets see where that goes in the next couple days.
255 2017-02-02T19:07:47  <wumpus> I don't know how hard it is? it seems to have caused quite a discussion but no fix
256 2017-02-02T19:07:50  <luke-jr> importmulti seems akin to importprivkey which shouldn't be used by users anyway?
257 2017-02-02T19:07:57  <gmaxwell> We can hide it right before cutting RC1 if nothing else.
258 2017-02-02T19:08:10  <wumpus> yes
259 2017-02-02T19:08:19  <gmaxwell> ashame, as it's a nice improvement.
260 2017-02-02T19:08:27  <sipa> i think the fix would be easy?
261 2017-02-02T19:08:43  <gmaxwell> sure, that is why I said lets see.
262 2017-02-02T19:08:52  <BlueMatt> who is working on it?
263 2017-02-02T19:08:53  <achow101> can't you just change the default timestamp to be 0?
264 2017-02-02T19:08:55  <gmaxwell> but we have a fallback if it doesn't get fixed.
265 2017-02-02T19:08:58  <BlueMatt> "lets see" only works if someone does it :p
266 2017-02-02T19:09:08  <gmaxwell> achow101: then there is no easy way to express now.
267 2017-02-02T19:09:29  <jonasschnelli> Would a fix where we set the importmulti timestamp to 0 instead of "now" do it for 0.14?
268 2017-02-02T19:09:37  <luke-jr> gmaxwell: using time() from your OS?
269 2017-02-02T19:09:40  <jonasschnelli> *default timestamp
270 2017-02-02T19:10:04  <achow101> -1 for "now"
271 2017-02-02T19:10:05  <wumpus> or have no default at all and require a time to be specified
272 2017-02-02T19:10:14  <jonasschnelli> wumpus: +1
273 2017-02-02T19:10:19  <luke-jr> wumpus: no default at all is nice since it allows a default to be chosen later
274 2017-02-02T19:10:31  <jonasschnelli> 0 as timestamp is very inefficient.
275 2017-02-02T19:10:34  <gmaxwell> Lets not hash it out here, there is an issue.
276 2017-02-02T19:10:45  <jonasschnelli> Okay. Lets comment there. Agree with gmaxwell
277 2017-02-02T19:10:57  <gmaxwell> I agree with jonasschnelli in the sense there that we really have to stop assuming a full rescan is possible.
278 2017-02-02T19:11:28  <wumpus> good point, yes
279 2017-02-02T19:11:34  <jonasschnelli> Is also very inefficient if you have pruned or run hybrid SPV
280 2017-02-02T19:11:37  <wumpus> it certainly shouldn't be the default
281 2017-02-02T19:11:37  <gmaxwell> It takes many hours on my normal development system, and is still quite slow even on the fastest hardware available.   But avoiding the rescan takes second seat to surprising the user. :)
282 2017-02-02T19:11:40  <wumpus> it's inefficient and lazy
283 2017-02-02T19:12:37  <gmaxwell> in my view, except for certan recover operations that are infrequently done-- rescan effectively doesn't work anymore (takes more time than converting your entire usage to a third party api...)
284 2017-02-02T19:13:22  <wumpus> users of the API should be encouraged to keep track of key birthdates
285 2017-02-02T19:13:31  *** jnewbery has joined #bitcoin-core-dev
286 2017-02-02T19:14:05  <gmaxwell> if we define a new private key format in the not so far future, we should make sure its string clearly integrates a birthdate. :P
287 2017-02-02T19:14:25  <BlueMatt> ok, so discuss on the issue....next topic?
288 2017-02-02T19:14:26  <wumpus> a full rescan is indeed only something that should be done for infrequent recovery reasons
289 2017-02-02T19:15:58  <wumpus> no other topics?
290 2017-02-02T19:16:22  <MarcoFalke> shortest meeting ever
291 2017-02-02T19:16:58  <wumpus> I had expected heated debates on what to include last-minute in 0.14 and why to delay the rc, what a disappointment! </s>
292 2017-02-02T19:17:02  <BlueMatt> great, lets get 0.14 done so I can get back to writing code :)
293 2017-02-02T19:17:10  <jonasschnelli> Heh.
294 2017-02-02T19:17:13  <sdaftuar> let's talk about code style again
295 2017-02-02T19:17:15  <BlueMatt> wumpus: I vote we push it back a month so we can do all the things we wanted to a month ago :p
296 2017-02-02T19:17:25  <wumpus> BlueMatt: lol!
297 2017-02-02T19:17:42  <BlueMatt> wait, i had something to talk about re: cde style
298 2017-02-02T19:17:43  <BlueMatt> hum
299 2017-02-02T19:17:54  <gmaxwell> BlueMatt: die
300 2017-02-02T19:17:55  <sdaftuar> i'll get the baseball bat
301 2017-02-02T19:17:56  <BlueMatt> oh, auto
302 2017-02-02T19:18:02  <jonasschnelli> Bumpfee: is there a reason why the logic is in the rpcwallet.cpp and not in wallet.cpp?
303 2017-02-02T19:18:13  <jonasschnelli> Makes it really hard to use in the gui...
304 2017-02-02T19:18:24  <BlueMatt> jonasschnelli: please move it, agreed
305 2017-02-02T19:18:25  <luke-jr> jonasschnelli: it can be moved
306 2017-02-02T19:18:25  <sdaftuar> jonasschnelli: i think we can refactor as needed
307 2017-02-02T19:18:29  <luke-jr> in 0.15*
308 2017-02-02T19:18:33  <wumpus> jonasschnelli: because it's only used in rpcwallet.cpp, if you need it in a more general place move it
309 2017-02-02T19:18:40  <jonasschnelli> Okay.
310 2017-02-02T19:18:54  <sipa> BlueMatt: i am strongly in favor of auto.
311 2017-02-02T19:18:59  * sipa hides
312 2017-02-02T19:19:00  <wumpus> jonasschnelli: although moving everything to wallet.cpp isn't very nice either, we should refactor the wallet code some day
313 2017-02-02T19:19:03  <luke-jr> I did suggest it earlier, but didn't seem like a blocker for merging
314 2017-02-02T19:19:06  <wumpus> I'm also in favor of auto
315 2017-02-02T19:19:15  <BlueMatt> sipa: it makes certain review much, much harder (I often grep for "everywhere X is used")
316 2017-02-02T19:19:15  <luke-jr> wumpus: well, it's wallet code..
317 2017-02-02T19:19:23  <BlueMatt> and have already missed things as a result
318 2017-02-02T19:19:24  <wumpus> luke-jr: so? not all wallet code needs to be in one file
319 2017-02-02T19:19:35  <wumpus> forbidding auto is just masochism
320 2017-02-02T19:19:41  <jonasschnelli> wumpus: yes. Thats a good point.
321 2017-02-02T19:19:42  <BlueMatt> wumpus: i wasnt voding forbidding it
322 2017-02-02T19:19:46  <sipa> BlueMatt: introduce an incompatible change to the type, and recompile. tadaa, all places it is used
323 2017-02-02T19:19:49  <BlueMatt> only carefully considering its use
324 2017-02-02T19:19:50  <cfields> sipa: same. BlueMatt: maybe paste the thread in question?
325 2017-02-02T19:19:56  <luke-jr> I like auto when the type is implied by some other type; eg, instead of xyz::value_type
326 2017-02-02T19:20:05  <jtimon> yeah, but not forbidding it doesn't mean recommending it always either
327 2017-02-02T19:20:10  <BlueMatt> https://github.com/bitcoin/bitcoin/pull/9609#discussion_r98335218
328 2017-02-02T19:20:15  <wumpus> we have a whole src/wallet directory which could have tons of different implementation files for different facets of the wallet, instead of stashing it all into one file
329 2017-02-02T19:20:39  <BlueMatt> (I believe gmaxwell's comment there was intedned for a different line)
330 2017-02-02T19:20:44  <jonasschnelli> yes. Stuff like coin selection should be more modular
331 2017-02-02T19:21:04  <wumpus> sure, as with any use of any c++ statement, use of auto should be measured
332 2017-02-02T19:21:08  <MarcoFalke> Lets do it after priority removal
333 2017-02-02T19:21:18  <MarcoFalke> Otherwise we step on each others toes
334 2017-02-02T19:21:24  <wumpus> if you have some specific cases where it's bad to use auto, please document them
335 2017-02-02T19:21:27  <BlueMatt> wumpus: mostly only things that are /actually/ a mile of text to type, imo
336 2017-02-02T19:21:57  <sipa> BlueMatt: and not needing to change things all over the place when you turn a tuple into a struct
337 2017-02-02T19:22:07  <sipa> or add a wrapper
338 2017-02-02T19:22:08  <BlueMatt> sipa: I have no problem reviewing sed-based changes
339 2017-02-02T19:22:16  <BlueMatt> in fact prefer that
340 2017-02-02T19:22:18  <wumpus> BlueMatt: there's plenty of those - c++ is overly verbose, auto is a great advancement
341 2017-02-02T19:22:21  <sipa> they're still annoying to fo
342 2017-02-02T19:22:27  <BlueMatt> since I'm gonna go read every single place the change effected anyway
343 2017-02-02T19:22:28  <BlueMatt> to review
344 2017-02-02T19:22:30  <sipa> *to do
345 2017-02-02T19:22:50  <sipa> and of course, let's consider on a case by case basis
346 2017-02-02T19:22:55  <wumpus> right
347 2017-02-02T19:22:56  <BlueMatt> wumpus: sure, iterators in iterators, np
348 2017-02-02T19:23:03  <BlueMatt> yea, ok, whatever, I'll shut up
349 2017-02-02T19:23:04  <sipa> but in my own preference, that is overwhelmingly the case
350 2017-02-02T19:24:00  <cfields> well the specific case here is for loops. "for (auto& foo : bar)"
351 2017-02-02T19:24:01  <MarcoFalke> Agree with BlueMatt, that auto should not be used unless necessary.
352 2017-02-02T19:24:04  <cfields> any reason not to use auto there?
353 2017-02-02T19:24:04  <jtimon> BlueMatt: the question is, do you have a general advice on when not to use auto?
354 2017-02-02T19:24:22  <wumpus> it's never *necessary* auto is just nice
355 2017-02-02T19:24:24  <BlueMatt> cfields: yes, so I can grep and review if the type's behavior changes in some way
356 2017-02-02T19:24:36  <BlueMatt> jtimon: personally, if the type really, really doesnt matter
357 2017-02-02T19:24:40  <luke-jr> cfields: if it's liable to produce bad results with bar changing under it
358 2017-02-02T19:24:50  <BlueMatt> (which means very rarely use it)
359 2017-02-02T19:24:56  <jtimon> BlueMatt: I'm afraid "doesn't matter" it's too vague here
360 2017-02-02T19:25:04  <wumpus> I don't think this is going anywhere, too much isagreement
361 2017-02-02T19:25:04  <BlueMatt> eg if you're taking an iterator and passing it through to another function
362 2017-02-02T19:25:08  <wumpus> any other topics?
363 2017-02-02T19:25:22  <wumpus> BlueMatt: function arguments can't use auto, right?
364 2017-02-02T19:25:29  <sipa> indeed
365 2017-02-02T19:25:50  <sipa> c++14 and later introduce some auto types in lambdas
366 2017-02-02T19:25:53  <BlueMatt> wumpus: correct, but eg doing auto it = map.find(thing); if (it != ma.end()) DoThingWith(*it);
367 2017-02-02T19:25:56  <BlueMatt> is like not a problem
368 2017-02-02T19:26:23  <BlueMatt> auto it = map.find(thing); if (it != ma.end()) ILikePonies(it->second.rainbows); I do not like
369 2017-02-02T19:26:24  <sipa> BlueMatt: how is that different from a for (const auto& x : container) {}
370 2017-02-02T19:27:08  <BlueMatt> sipa: because in the specific case here the thing in the loop is not defined to take a specific type
371 2017-02-02T19:27:12  <BlueMatt> it is templated
372 2017-02-02T19:27:15  <jtimon> my question was, do you have a deductive method for finding the not ok cases instead of an inductive one for the "not a problem cases"?
373 2017-02-02T19:27:21  <sipa> you can see that as an oblivious loop with iterators, and passing *it to a function that is tje body of the loop
374 2017-02-02T19:27:38  <BlueMatt> jtimon: <BlueMatt> jtimon: personally, if the type really, really doesnt matter
375 2017-02-02T19:28:12  <sipa> i see your point, but i don't think it weighs up against the benefitd
376 2017-02-02T19:28:14  <sipa> *benefits
377 2017-02-02T19:28:20  <wumpus> if you're iterating over some container, the type of container usually really doesn't matter, unless you make specific assumptions (but then you'd generally not be using a range for loop in the first place)
378 2017-02-02T19:28:41  <BlueMatt> wumpus: imo if you are ever actually dereferencing the type you should not use auto
379 2017-02-02T19:28:56  <sipa> BlueMatt: your own example dereferences...
380 2017-02-02T19:28:57  <BlueMatt> if you're dereferencing the iterator to eg a pair or just taking the element and passing it to something else, ok
381 2017-02-02T19:29:08  <BlueMatt> but if you're dereferencing it and accessing something inside it, no
382 2017-02-02T19:29:27  <sipa> then we might as well not use it at all, i think
383 2017-02-02T19:29:46  <jtimon> ok, I think I get what you mean by "doesn't matter" now
384 2017-02-02T19:29:52  <BlueMatt> sipa: there are many places where you might do for (auto& thing: list) ActOn(thing);
385 2017-02-02T19:29:55  <BlueMatt> thats reasonable
386 2017-02-02T19:30:08  <sipa> requiring programmers to spell out redundant information just so you can grep for it seems extreme to me
387 2017-02-02T19:30:27  <gmaxwell> sipa: so functions shouldn't have prototypes? :)
388 2017-02-02T19:30:27  <BlueMatt> yes, I didnt expect people to agree with me...I have extreme distaste for auto, personally
389 2017-02-02T19:30:33  <wumpus> yes, that' extreme, and not going tohhappen. Just use smarter tools.
390 2017-02-02T19:30:42  <BlueMatt> wumpus: suggestions?
391 2017-02-02T19:31:00  * BlueMatt would love a grep --allusesoftype thing
392 2017-02-02T19:31:30  <wumpus> it should be fairly easy to implement using clang's parser, would be surprised if it doesn't exist
393 2017-02-02T19:31:50  <gmaxwell> There is another side to it is that auto enables you to write code that acts on a type while having no idea of the type yourself. Which is safe 99% of the time and deadly the rest.
394 2017-02-02T19:32:20  <gmaxwell> because in C++ not all operations which are catgorically unsafe on a type are actually stopped by typechecking. :(
395 2017-02-02T19:33:41  <gmaxwell> I have an auto to a container... and then I extract an auto to an iterator on it and erase things. Is my code guilty of the sin of using an invalidated iterator? It depends on what container was in use, and that was hid by auto...
396 2017-02-02T19:34:08  <gmaxwell> But... that sort of thing is an edge case, I'd love to see a realistic list of where auto is likely to cause problems, just to keep it in mind.
397 2017-02-02T19:34:29  <wumpus> right - just keep it in mind while reviewing
398 2017-02-02T19:34:43  <wumpus> and if there are well-defined cases where auto is dangerous, they should be documented in the developer notes
399 2017-02-02T19:34:57  <BlueMatt> ehh, ok, well I go read all of wallet half the time reviewing wallet changes, i guess I'll just start doing that for net, too :p
400 2017-02-02T19:35:13  <BlueMatt> (not a bad thing, that)
401 2017-02-02T19:35:29  <gmaxwell> unfortunately, auto is most interesting when you have some horrible complex signature.  But those are the cases where it is also more of an issue.
402 2017-02-02T19:36:07  <cfields> gmaxwell: for(auto& : foo) doesn't give you an iterator though, just a reference. So imo that should be highly preferred when possible to avoid your example.
403 2017-02-02T19:36:19  <wumpus> well no, it's most interesting for bog-standard loops, 99% of the cases. If you're doing anything horribly complex, that's probably where you should be careful
404 2017-02-02T19:36:32  <jtimon> BlueMatt: we can agree that auto is totally fine for unittests too, right? :p
405 2017-02-02T19:37:20  <cfields> (preferred over auto foo = bar.begin(), that is)
406 2017-02-02T19:37:23  <gmaxwell> wumpus: well my point is that stating the type explicitly is just as easy as auto when it's simple and obvious.
407 2017-02-02T19:37:29  <sipa> gmaxwell: when you have some horrid complex type signature, best practice is to introduce a typedef for it... that also results in succint usage, and lacks the review concerns that BlueMatt has i think
408 2017-02-02T19:37:36  <jtimon> I agree it removes clarity some times
409 2017-02-02T19:37:48  <wumpus> gmaxwell: it's *easy* but the point is to avoid unnecessary verbosity/typing, not so you can forget the type
410 2017-02-02T19:37:54  <BlueMatt> sipa: yes, agreed, also means dont use auto in place, which some people like to do
411 2017-02-02T19:38:10  <jtimon> but I don't have a clear criterion on when to use it like matt
412 2017-02-02T19:38:14  <BlueMatt> wumpus: I'm generally 100% in favor of extra verbosity
413 2017-02-02T19:38:25  <wumpus> e.g. to avoid having to type std::vector<std::Strring> for the zillionth time
414 2017-02-02T19:38:25  <gmaxwell> fking java programmers. :P
415 2017-02-02T19:38:27  <wumpus> BlueMatt: go use java
416 2017-02-02T19:38:28  <BlueMatt> extra verbosity generally means less magic, which makes review easier
417 2017-02-02T19:38:33  <BlueMatt> lol, i expected that....
418 2017-02-02T19:38:36  <gmaxwell> (though on type signatures, I usually also prefer being explicit more often)
419 2017-02-02T19:38:44  <wumpus> BlueMatt: that's not categorically true, more verbosity also means more distraction
420 2017-02-02T19:38:51  <sipa> agree
421 2017-02-02T19:38:58  <BlueMatt> wumpus: well, ok, more verbosity as long as it actually provides information
422 2017-02-02T19:39:05  <sipa> nobody actually looks at what large type definitions contain
423 2017-02-02T19:39:06  <wumpus> having lot's of boilerplate does *not* equal easier review
424 2017-02-02T19:39:10  *** Evel-Laptop has quit IRC
425 2017-02-02T19:39:11  <BlueMatt> public static void main(String[] args) {} probably doesnt provide more information
426 2017-02-02T19:39:26  <wumpus> anyhow
427 2017-02-02T19:39:29  <BlueMatt> sipa: I do!
428 2017-02-02T19:39:35  <wumpus> any other topics? this is going the wrong way
429 2017-02-02T19:39:40  <sipa> haha
430 2017-02-02T19:39:44  <luke-jr> lol
431 2017-02-02T19:40:23  <BlueMatt> soooo...endmeeting?
432 2017-02-02T19:40:26  <wumpus> #endmeeting
433 2017-02-02T19:40:26  <lightningbot> Meeting ended Thu Feb  2 19:40:26 2017 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
434 2017-02-02T19:40:26  <lightningbot> Minutes:        http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-02-02-19.00.html
435 2017-02-02T19:40:26  <lightningbot> Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-02-02-19.00.txt
436 2017-02-02T19:40:26  <lightningbot> Log:            http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-02-02-19.00.log.html
437 2017-02-02T19:40:39  <gmaxwell> wumpus: your request is a little explicit, you could have just said...  for auto meetingstep.
438 2017-02-02T19:41:01  <BlueMatt> heh
439 2017-02-02T19:41:03  <wumpus> gmaxwell: #endcurrentcontext
440 2017-02-02T19:42:22  <jtimon> regarding https://github.com/bitcoin/bitcoin/pull/9609#issuecomment-276974228 I generally dislike habing the release branches differ from the master they branched from more than they need "artificially"
441 2017-02-02T19:43:25  <jtimon> also for "we can fix that after the branch" types of things
442 2017-02-02T19:44:31  <MarcoFalke> jtimon: master is for testing, release branches are for production
443 2017-02-02T19:44:43  <jtimon> MarcoFalke: I know
444 2017-02-02T19:44:54  <gmaxwell> The asserts don't leve enough record in any case, during development.
445 2017-02-02T19:44:55  *** lclc has joined #bitcoin-core-dev
446 2017-02-02T19:45:26  <MarcoFalke> jtimon: The thing is you may want asserts during testing, but you may not want them in production
447 2017-02-02T19:45:31  <gmaxwell> I've had asserts fire and have had no idea which assert fired or why, only that my daemon went away.  So a crash was introduced but we didn't even learn from it.
448 2017-02-02T19:45:46  <MarcoFalke> jtimon: We can not compile without asserts, so this is the only way?
449 2017-02-02T19:46:07  <jtimon> MarcoFalke: perhaps something like if(debug) ?
450 2017-02-02T19:46:33  <gmaxwell> (moreover, differences in behavior between testing and production exposes you to bugs that the testing code fixes)
451 2017-02-02T19:46:41  <gmaxwell> e.g. when an asserts check has a side effect.
452 2017-02-02T19:47:33  <gmaxwell> In any case, asserts around the net stuff are also special because the testing there is woefully inadequate right now.
453 2017-02-02T19:48:00  <gmaxwell> As evidenced by the fact that the version assert was there for months and only triggered a couple times, but yet was still triggerable!
454 2017-02-02T19:48:36  <jtimon> yeah, I'm talking more generally about the "we can fix that after the branch" or let's do A and master and B in 0.14 attitude
455 2017-02-02T19:48:44  <luke-jr> #9619 has plenty of ACKs; shall I go add test cases, or merge this and PR test cases separate?
456 2017-02-02T19:48:46  <gribble> https://github.com/bitcoin/bitcoin/issues/9619 | Bugfix: RPC/Mining: GBT should return 1 MB sizelimit before segwit activates by luke-jr · Pull Request #9619 · bitcoin/bitcoin · GitHub
457 2017-02-02T19:49:25  <BlueMatt> I do believe we need to start talking about making our assert infrastructure better - building test bins by default with DEBUG_LOCKORDER and many more asserts, with most of those asserts just printing warnings in production
458 2017-02-02T19:50:47  <jtimon> yeah, and that doesn't require to have different code, maybe just change the value of a constant or something
459 2017-02-02T19:52:15  <jtimon> or do all that stuff when -debug
460 2017-02-02T19:52:58  <bitcoin-git> [bitcoin] isle2983 closed pull request #9459: Improvements to copyright_header.py and some minor copyright header tweaks. (master...PR-copyright-script-improve) https://github.com/bitcoin/bitcoin/pull/9459
461 2017-02-02T19:53:25  <bitcoin-git> [bitcoin] isle2983 closed pull request #9603: Add basic_style.py to automate some style checking. (master...PR-basic-style) https://github.com/bitcoin/bitcoin/pull/9603
462 2017-02-02T19:53:55  *** chjj has quit IRC
463 2017-02-02T20:09:48  *** Chris_Stewart_5 has quit IRC
464 2017-02-02T20:14:04  <jtimon> ugh, I missed the change in style... https://github.com/bitcoin/bitcoin/pull/9566#discussion_r98842239
465 2017-02-02T20:15:39  <jtimon> there's a lot of code of the form
466 2017-02-02T20:15:39  <jtimon> if (checkwhatever)
467 2017-02-02T20:15:39  <jtimon>     return false/error(...)/state.DoS(...)
468 2017-02-02T20:15:39  <jtimon> that's no suddenly against our style...
469 2017-02-02T20:15:56  <jtimon> s/that's no/that's now/
470 2017-02-02T20:16:27  <sipa> jtimon: indeed. my preference is that anytime you're touching code (except simple move-onlys), you adapt the style of the code
471 2017-02-02T20:16:39  <sipa> but no big refactors changing the style all over
472 2017-02-02T20:17:56  <jtimon> right, my point is I would prefer that our rules for style was something that we're actually closer to achieve, even if that's something less strict
473 2017-02-02T20:19:09  <jtimon> this is an invitation for someone to create a PR correcting some of those cases
474 2017-02-02T20:21:01  *** chjj has joined #bitcoin-core-dev
475 2017-02-02T20:22:27  *** Chris_Stewart_5 has joined #bitcoin-core-dev
476 2017-02-02T20:27:40  <isle2983> it seems strange to me that the style is a moving target. is there some nuance that I am missing?
477 2017-02-02T20:28:18  <isle2983> I would think that one of the off-the-shelf choices would be best.
478 2017-02-02T20:28:47  <sipa> isle2983: the original satoshi code used a very arcane style, which many people dislike, and has in practice not been followed by contributors after satoshi left
479 2017-02-02T20:29:10  <sipa> at some point we formalized the effective style people were using in the developer notes, but reviewers didn't actually enforce it
480 2017-02-02T20:29:34  <sipa> plus, there was a strong "mimick the style around what you're editing" tendency, which doesn't really help converging
481 2017-02-02T20:29:50  <sipa> i hope that going forward we start enforcing it
482 2017-02-02T20:29:55  <sipa> in modified code, at least
483 2017-02-02T20:30:05  <sipa> https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#developer-notes
484 2017-02-02T20:36:00  <isle2983> yeah, that makes sense to me. but I am not going to poke the enforcement side too hard.
485 2017-02-02T20:36:33  *** F0sea has joined #bitcoin-core-dev
486 2017-02-02T20:36:41  <jtimon> hmm, I'm also not sure https://github.com/bitcoin/bitcoin/blob/master/src/.clang-format enforces the "always braces except if everything in one line" rule
487 2017-02-02T20:37:06  <sipa> i'm not sure that it can
488 2017-02-02T20:37:42  <isle2983> the clang-format script in #9658 generates a 'score' metric
489 2017-02-02T20:37:44  <gribble> https://github.com/bitcoin/bitcoin/issues/9658 | Add clang_format.py to help automate code style analysis by isle2983 · Pull Request #9658 · bitcoin/bitcoin · GitHub
490 2017-02-02T20:38:03  <isle2983> so at the very least, we can watch it converge rather than diverge over time
491 2017-02-02T20:39:39  <jtimon> yeah, it seems some (all?) of your tools would fit  better in https://github.com/bitcoin-core/bitcoin-maintainer-tools from what other people said
492 2017-02-02T20:40:10  <isle2983> yep
493 2017-02-02T20:40:24  <isle2983> I am happy with that as a starting place
494 2017-02-02T20:41:11  <jtimon> IMO, style rules aren't so useful until you start to enforce it with the help of automatic tools in the whole project, that's the only way you can really stop talking about style, which is the goal of having style rules in the first place, right?
495 2017-02-02T20:41:20  <gmaxwell> sipa: clang-format (latest versions) can enforce that rule.
496 2017-02-02T20:41:46  <gmaxwell> jtimon: I don't agree. Code style existed long before tools to do anything special with it.
497 2017-02-02T20:42:00  <jtimon> gmaxwell: thanks for confirming, that's what I thought, including the exception for the single line ifs
498 2017-02-02T20:42:21  <gmaxwell> I think automatic tools often handicap style discussions, because they enforce it stupidly to the point where they can irritate people and cause general opposition to standards.
499 2017-02-02T20:42:29  *** handlex has joined #bitcoin-core-dev
500 2017-02-02T20:42:38  <gmaxwell> jtimon: yes, including the exception, according to the docs. I have not tested it.
501 2017-02-02T20:42:49  <jtimon> well, code style without automatic rules may save you some style discussions, but not all
502 2017-02-02T20:42:56  <gmaxwell> (I don't have a current version of clang on my laptop -- another challenge with formating tools, they change.)
503 2017-02-02T20:43:04  *** handlex has quit IRC
504 2017-02-02T20:43:11  <jtimon> yep
505 2017-02-02T20:43:18  <gmaxwell> jtimon: they give people a way to answer the question without first asking everyone else all the time. :)
506 2017-02-02T20:44:00  <jtimon> absolutely
507 2017-02-02T20:44:46  <gmaxwell> (I'm not opposed to tools, but they work MUcH better for single developers and single companies with rigidly enforced development workstation configs.. than they do for big projects)
508 2017-02-02T20:46:27  <jtimon> true that, my very positive experience where with enforced development confgs per project, the project that had discussions about syle were those not enforcing the style automatically
509 2017-02-02T20:46:58  <jtimon> s/experience where/experiences were
510 2017-02-02T20:47:31  <isle2983> the trend does seem to be towards containerized environments for builds there the dependencies can be managed. That might be wrong for bitcoin, however.
511 2017-02-02T20:47:35  <luke-jr> git-clang-format looks interesting
512 2017-02-02T20:47:49  <isle2983> *where
513 2017-02-02T20:49:03  <jtimon> luke-jr: https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/git-clang-format ? seems the same concept as MarcoFalke's script we don't use, no?
514 2017-02-02T20:49:37  <luke-jr> I haven't followed all the discussion, but an upstreamed tool seems better than one we maintain
515 2017-02-02T20:50:07  <jtimon> ie this one https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/master/clang-format.py but yeah, this seems to be more interesting as it integrates with git
516 2017-02-02T20:51:51  *** Sosumi has quit IRC
517 2017-02-02T20:52:09  <luke-jr> step 1: provide a config file so we can just use git-clang-format manually for correct output ;)
518 2017-02-02T20:53:12  <cfields> luke-jr: even better, i assume it can be a commit hook?
519 2017-02-02T20:53:51  <luke-jr> cfields: I would think so, but I haven't tried it
520 2017-02-02T20:53:52  <cfields> (that's a bit dangerous though i suppose, if the script breaks and you lose your commit)
521 2017-02-02T20:54:23  <luke-jr> repo-wide commit hooks seem like a complex enough thing with git that I'd make that a full step 2 though ;)
522 2017-02-02T20:54:42  <cfields> heh, yes
523 2017-02-02T21:17:28  *** BashCo has quit IRC
524 2017-02-02T21:21:38  <MarcoFalke> luke-jr: It is a 1-1 copy of llvm
525 2017-02-02T21:30:06  <luke-jr> >_<
526 2017-02-02T21:30:12  <MarcoFalke> Eh, whatever. Misread the scorllback
527 2017-02-02T21:30:38  <MarcoFalke> We could add the git-clang-format as well, so it is in our tree
528 2017-02-02T21:35:33  *** BashCo has joined #bitcoin-core-dev
529 2017-02-02T21:57:22  *** lclc has quit IRC
530 2017-02-02T21:59:04  *** laurentmt has joined #bitcoin-core-dev
531 2017-02-02T21:59:38  *** laurentmt has quit IRC
532 2017-02-02T22:00:39  <luke-jr> I don't see how this is possible https://github.com/bitcoin/bitcoin/pull/9619#issuecomment-277096336
533 2017-02-02T22:19:37  <MarcoFalke> Is there a way to bisect only on (signed) merge commits?
534 2017-02-02T22:22:29  <MarcoFalke> Otherwise merging this will probably upset a few: https://github.com/bitcoin/bitcoin/pull/9657#issuecomment-277094673
535 2017-02-02T22:23:57  <luke-jr> it wouldn't be the first time we have an untestable tree merged in history
536 2017-02-02T22:24:09  <luke-jr> it's a potential security issue to keep in mind though
537 2017-02-02T22:24:17  <luke-jr> (bisecting unsigned commits)
538 2017-02-02T22:25:09  <MarcoFalke> Well, I hope when people review, they check that the intermediate commits don't add random binary blobs
539 2017-02-02T22:26:45  <gmaxwell> MarcoFalke: code that totally backdoors your machine could be a couple lines of shellscript.  And the concern there is just that you pull and some is slipped in and you run in bisect... it's worried me before, but didn't seem like there was anything to do over it.
540 2017-02-02T22:27:18  <MarcoFalke> Still should be part of review to ensure this does not happen
541 2017-02-02T22:27:36  <gmaxwell> it's not something review can control.
542 2017-02-02T22:27:40  <kanzure> hi am i late
543 2017-02-02T22:27:54  <gmaxwell> I do wish you could check in a .gitbisectuntestable which has a list of commits bisect should just skip over.
544 2017-02-02T22:28:20  <MarcoFalke> Please explain why review can not control it
545 2017-02-02T22:28:52  <sipa> gmaxwell, MarcoFalke: write a script that produces a list of all unsigned commits, and feed those to 'git bisect skip
546 2017-02-02T22:28:55  <sipa> ?
547 2017-02-02T22:30:23  <jtimon> BlueMatt: at this point, should I just close #9634 as included in #9650 ?
548 2017-02-02T22:30:25  <gribble> https://github.com/bitcoin/bitcoin/issues/9634 | Fail in DecodeHexTx if there is extra data at the end by jtimon · Pull Request #9634 · bitcoin/bitcoin · GitHub
549 2017-02-02T22:30:26  <gribble> https://github.com/bitcoin/bitcoin/issues/9650 | Better handle invalid parameters to signrawtransaction by TheBlueMatt · Pull Request #9650 · bitcoin/bitcoin · GitHub
550 2017-02-02T22:30:42  <BlueMatt> jtimon: I suppose so
551 2017-02-02T22:30:49  <gmaxwell> MarcoFalke: Because compromised code can hide things from reviewers. And there is no guarentee that things that hit the tree have been reviewed (particularly if a commiter is compromised). So if you get compromised you might merge bad commits. You can't see they're bad because the compromise hides them.  Other people don't see they're bad because they only review the ultimate commit if at all.
552 2017-02-02T22:30:55  <gmaxwell> .. and becuase once they run something in a bad commit they become compromised and can't see it either.
553 2017-02-02T22:31:03  <jtimon> BlueMatt: ok, thanks for the tests!
554 2017-02-02T22:32:11  <luke-jr> all the more reason to sandbox dev
555 2017-02-02T22:32:14  <gmaxwell> especially since we don't require that PR that we merge be based on the current tip (it would be unreasnable to do so), the intermediate commits will often not match exactly what anyone has reviewed.
556 2017-02-02T22:32:16  <bitcoin-git> [bitcoin] jtimon closed pull request #9634: Fail in DecodeHexTx if there is extra data at the end (master...upstream-fail-decode-tx) https://github.com/bitcoin/bitcoin/pull/9634
557 2017-02-02T22:32:24  <gmaxwell> Even where the final result does.
558 2017-02-02T22:32:47  <BlueMatt> jtimon: thanks for catching that I forgot to upstream this!
559 2017-02-02T22:32:56  <jtimon> np
560 2017-02-02T22:33:08  <luke-jr> it'd probably be more practical to review if we used merge commits instead of rebasing
561 2017-02-02T22:33:24  <MarcoFalke> Which is why I proposed we don't corrupt merge commits
562 2017-02-02T22:33:27  <MarcoFalke> See  #8089
563 2017-02-02T22:33:28  <gribble> https://github.com/bitcoin/bitcoin/issues/8089 | verify-commits should also check for malicious code in merge commits · Issue #8089 · bitcoin/bitcoin · GitHub
564 2017-02-02T22:33:57  *** CubicEarth has quit IRC
565 2017-02-02T22:34:36  <MarcoFalke> > Other people don't see they're bad because they only review the ultimate commit if at all.
566 2017-02-02T22:34:44  <MarcoFalke> Review should always be done commit-by-commit
567 2017-02-02T22:34:53  <gmaxwell> MarcoFalke: conflicts though is not the same as no difference at all.
568 2017-02-02T22:35:29  <gmaxwell> Bascially if you require there be no difference then we must require every PR rebase and re-review every time another PR is merged that touches the same files (or at least functions).
569 2017-02-02T22:35:48  <gmaxwell> Otherwise you can arrange the automatic resolution to add vulnerabilities.
570 2017-02-02T22:36:33  <gmaxwell> and already people will review commits, but not review that what is ultimately merged were the same commits exactly (usually aren't, due to other changes)
571 2017-02-02T22:38:25  <MarcoFalke> I don't understand how you end up with two different results when you merge the exact same commits.
572 2017-02-02T22:38:53  <MarcoFalke> I mean you get a different commit hash when you don't adjust the time to time-of-merge and author to merge-commit-author etc
573 2017-02-02T22:39:19  <MarcoFalke> but it should be possible to replay all merge commits and compare them to what peoples eyes reviewed
574 2017-02-02T22:40:00  <jtimon> BlueMatt: you missed one garbate in 9650 :p
575 2017-02-02T22:40:13  <BlueMatt> oh ffs, leave my spelling alone!
576 2017-02-02T22:40:14  <BlueMatt>  /s
577 2017-02-02T22:41:40  <gmaxwell> MarcoFalke: If I review PR X  which is on head Y (or which I merged to Q)  that is not the same as reviewing PR X merged against Z.  (particularly in adversarial condictions)
578 2017-02-02T22:42:30  <gmaxwell> I think this is getting too abstract, for this discussion I think it suffices to point out that people will pull code into their local branches before reviewing it, because pulling it into their branch is how they go about reviewing it.
579 2017-02-02T22:42:41  <MarcoFalke> Reviewers don't commit to "PR X merged against Z", they commit to "PR X".
580 2017-02-02T22:43:17  <MarcoFalke> They can only commit to the merge commit when the merge actually happened in the master branch
581 2017-02-02T22:45:07  <MarcoFalke> I mean I can merge a commit_A locally for testing, but when I publish my review I don't refer to my local merge commit by to commit_A
582 2017-02-02T22:45:21  <gmaxwell> You're saying the same thing as me--- I think. Reviewers review PR X and so cannot be counted on strongly to catch malicious behavior in X which is only exposed by its merge in Z.
583 2017-02-02T22:45:23  <MarcoFalke> /by/but/
584 2017-02-02T22:45:52  <gmaxwell> MarcoFalke: right, and so your review /may/ be ineffective, particularly if X was maliciously constructed.
585 2017-02-02T22:47:10  <bitcoin-git> [bitcoin] luke-jr opened pull request #9672: Opt-into-RBF for RPC & bitcoin-tx (master...rpc_rbf) https://github.com/bitcoin/bitcoin/pull/9672
586 2017-02-02T22:47:11  *** jannes has quit IRC
587 2017-02-02T22:47:32  * luke-jr kicks insta-conflict
588 2017-02-02T22:48:47  <MarcoFalke> Oh I think we were talking about two completely unrelated things, I think. You were raising concerns about possible exploits that only arise after gits merge algorithm processed them. I was talking about maintainers appending commit/ ammending commits/ editing merge commits or reviewers skipping individual commits of pull request which happen to contain a +++ exploit.sh and ---exploit.sh
589 2017-02-02T22:50:34  <gmaxwell> Right. and I am saying that I do not believe people review intermeadate commits in merges, because doing so cannot show issues that arose through the merge.  They look at PRs and at end results. If you slip in extra commits that do not change the end results, I believe it is unlikely to get noticed.
590 2017-02-02T22:50:39  <gmaxwell> Perhaps I'm wrong.
591 2017-02-02T22:53:24  <MarcoFalke> I don't think you are wrong, so we should work towards making it easy such that a uncompromised machine with a helper script and some verified keys can validate the current state of the bitcoin-git repo
592 2017-02-02T22:54:00  <luke-jr> ideally, but I think we need to prioritise getting reviews done over making reviews more effort
593 2017-02-02T22:54:55  <MarcoFalke> Yeah, that is the downside.
594 2017-02-02T23:31:35  *** dgenr8 has quit IRC
595 2017-02-02T23:31:38  <bitcoin-git> [bitcoin] ryanofsky opened pull request #9673: Set correct metadata on bumpfee wallet transactions (master...pr/bumpfee-meta) https://github.com/bitcoin/bitcoin/pull/9673
596 2017-02-02T23:47:09  *** CubicEarth has joined #bitcoin-core-dev
597 2017-02-02T23:48:09  *** dgenr8 has joined #bitcoin-core-dev
598 2017-02-02T23:51:49  *** CubicEarth has quit IRC