1 2020-07-02T00:00:02  *** bitprophet1 has quit IRC
  2 2020-07-02T00:02:26  *** bitcoin-git has joined #bitcoin-core-dev
  3 2020-07-02T00:02:27  <bitcoin-git> [bitcoin] MarcoFalke opened pull request #19429: test: Fix intermittent failure in wallet_encryption (master...2007-testFixInt) https://github.com/bitcoin/bitcoin/pull/19429
  4 2020-07-02T00:02:27  *** bitcoin-git has left #bitcoin-core-dev
  5 2020-07-02T00:22:07  *** tummy has joined #bitcoin-core-dev
  6 2020-07-02T00:24:14  *** mol has quit IRC
  7 2020-07-02T00:35:27  *** promag has quit IRC
  8 2020-07-02T00:37:41  *** bitcoin-git has joined #bitcoin-core-dev
  9 2020-07-02T00:37:42  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/501203aa9101...d6fe5b28dff4
 10 2020-07-02T00:37:42  <bitcoin-git> bitcoin/master fa23fbb MarcoFalke: ci: Run all tests on native mac again
 11 2020-07-02T00:37:43  <bitcoin-git> bitcoin/master d6fe5b2 MarcoFalke: Merge #19427: ci: Run all tests on native mac again
 12 2020-07-02T00:37:44  *** bitcoin-git has left #bitcoin-core-dev
 13 2020-07-02T00:38:02  *** bitcoin-git has joined #bitcoin-core-dev
 14 2020-07-02T00:38:02  <bitcoin-git> [bitcoin] MarcoFalke merged pull request #19427: ci: Run all tests on native mac again (master...2007-ciMac) https://github.com/bitcoin/bitcoin/pull/19427
 15 2020-07-02T00:38:03  *** bitcoin-git has left #bitcoin-core-dev
 16 2020-07-02T00:51:52  *** proofofkeags has quit IRC
 17 2020-07-02T00:52:26  *** proofofkeags has joined #bitcoin-core-dev
 18 2020-07-02T00:52:58  *** Relis has quit IRC
 19 2020-07-02T00:56:41  *** proofofkeags has quit IRC
 20 2020-07-02T01:12:16  *** justanotheruser has quit IRC
 21 2020-07-02T01:29:51  *** proofofkeags has joined #bitcoin-core-dev
 22 2020-07-02T01:32:51  *** DeanWeen has quit IRC
 23 2020-07-02T01:34:30  *** proofofkeags has quit IRC
 24 2020-07-02T01:49:47  *** proofofkeags has joined #bitcoin-core-dev
 25 2020-07-02T01:52:16  *** DeanGuss has joined #bitcoin-core-dev
 26 2020-07-02T01:53:21  *** mol has joined #bitcoin-core-dev
 27 2020-07-02T01:58:01  *** justanotheruser has joined #bitcoin-core-dev
 28 2020-07-02T02:03:19  *** proofofkeags has quit IRC
 29 2020-07-02T02:17:15  *** DeanGuss has quit IRC
 30 2020-07-02T02:22:00  *** DeanGuss has joined #bitcoin-core-dev
 31 2020-07-02T02:23:20  *** Highway61 has quit IRC
 32 2020-07-02T02:27:10  *** andrewtoth_ has quit IRC
 33 2020-07-02T02:44:10  *** GAit has quit IRC
 34 2020-07-02T02:44:16  *** proofofkeags has joined #bitcoin-core-dev
 35 2020-07-02T02:48:40  *** proofofkeags has quit IRC
 36 2020-07-02T03:00:02  *** tummy has quit IRC
 37 2020-07-02T03:04:00  *** proofofkeags has joined #bitcoin-core-dev
 38 2020-07-02T03:12:44  *** troygiorshev has joined #bitcoin-core-dev
 39 2020-07-02T03:19:19  *** proofofkeags has quit IRC
 40 2020-07-02T03:31:12  *** proofofkeags has joined #bitcoin-core-dev
 41 2020-07-02T03:31:12  *** proofofkeags has quit IRC
 42 2020-07-02T03:31:24  *** proofofkeags has joined #bitcoin-core-dev
 43 2020-07-02T03:31:31  *** proofofkeags has quit IRC
 44 2020-07-02T03:34:59  *** EagleTM has joined #bitcoin-core-dev
 45 2020-07-02T03:35:56  *** Eagle[TM] has quit IRC
 46 2020-07-02T03:47:26  *** proofofkeags has joined #bitcoin-core-dev
 47 2020-07-02T03:52:26  *** proofofkeags has quit IRC
 48 2020-07-02T03:54:16  *** luke-jr has quit IRC
 49 2020-07-02T03:55:25  *** Matt_P has joined #bitcoin-core-dev
 50 2020-07-02T03:56:20  *** luke-jr has joined #bitcoin-core-dev
 51 2020-07-02T04:08:39  *** shesek has joined #bitcoin-core-dev
 52 2020-07-02T04:08:39  *** shesek has joined #bitcoin-core-dev
 53 2020-07-02T04:39:12  *** bitdex has joined #bitcoin-core-dev
 54 2020-07-02T04:46:08  *** bitdex has quit IRC
 55 2020-07-02T04:58:43  *** vasild has quit IRC
 56 2020-07-02T04:58:59  *** vasild has joined #bitcoin-core-dev
 57 2020-07-02T05:00:55  *** troygiorshev has quit IRC
 58 2020-07-02T05:01:58  *** vasild has quit IRC
 59 2020-07-02T05:25:47  *** bitdex has joined #bitcoin-core-dev
 60 2020-07-02T05:37:39  *** bitdex has quit IRC
 61 2020-07-02T05:41:46  *** bitdex has joined #bitcoin-core-dev
 62 2020-07-02T06:00:02  *** Matt_P has quit IRC
 63 2020-07-02T06:09:03  *** alko has joined #bitcoin-core-dev
 64 2020-07-02T06:09:22  *** EagleTM has quit IRC
 65 2020-07-02T06:17:12  *** EagleTM has joined #bitcoin-core-dev
 66 2020-07-02T06:22:02  *** aqu4 has joined #bitcoin-core-dev
 67 2020-07-02T06:24:18  *** EagleTM has quit IRC
 68 2020-07-02T06:25:06  *** marcoagner has joined #bitcoin-core-dev
 69 2020-07-02T06:31:52  *** cfields has quit IRC
 70 2020-07-02T06:33:00  *** cfields has joined #bitcoin-core-dev
 71 2020-07-02T06:34:16  *** jonatack has quit IRC
 72 2020-07-02T06:50:40  *** vasild has joined #bitcoin-core-dev
 73 2020-07-02T06:58:17  *** Guyver2 has joined #bitcoin-core-dev
 74 2020-07-02T07:25:49  *** afk11` has quit IRC
 75 2020-07-02T07:26:15  *** afk11` has joined #bitcoin-core-dev
 76 2020-07-02T07:57:05  *** kljasdfvv has quit IRC
 77 2020-07-02T08:01:39  *** kljasdfvv has joined #bitcoin-core-dev
 78 2020-07-02T08:29:03  *** AaronvanW has joined #bitcoin-core-dev
 79 2020-07-02T08:30:29  *** go121212 has joined #bitcoin-core-dev
 80 2020-07-02T08:32:47  *** go11111111111 has quit IRC
 81 2020-07-02T09:00:02  *** aqu4 has quit IRC
 82 2020-07-02T09:03:03  *** vasild has quit IRC
 83 2020-07-02T09:19:59  *** vasild has joined #bitcoin-core-dev
 84 2020-07-02T09:22:29  *** sledges has joined #bitcoin-core-dev
 85 2020-07-02T09:44:08  *** dfmb_ has joined #bitcoin-core-dev
 86 2020-07-02T09:48:53  *** promag has joined #bitcoin-core-dev
 87 2020-07-02T09:51:29  *** dfmb_ has quit IRC
 88 2020-07-02T10:02:48  *** isis_ is now known as isis
 89 2020-07-02T10:03:28  *** Dino76Hettinger has joined #bitcoin-core-dev
 90 2020-07-02T10:09:06  *** sdaftuar_ has quit IRC
 91 2020-07-02T10:09:33  *** sdaftuar_ has joined #bitcoin-core-dev
 92 2020-07-02T10:09:54  *** Pavlenex has joined #bitcoin-core-dev
 93 2020-07-02T10:37:19  *** Dino76Hettinger has quit IRC
 94 2020-07-02T10:42:09  *** promag has quit IRC
 95 2020-07-02T10:51:58  *** reallll has joined #bitcoin-core-dev
 96 2020-07-02T10:54:32  *** promag has joined #bitcoin-core-dev
 97 2020-07-02T10:55:25  *** belcher has quit IRC
 98 2020-07-02T11:26:51  *** bitcoin-git has joined #bitcoin-core-dev
 99 2020-07-02T11:26:51  <bitcoin-git> [bitcoin] midnightmagic opened pull request #19430: build: configure.ac and netbsd-build.md for NetBSD (master...simple-fixes-for-netbsd-eventually-for-v0.20.0) https://github.com/bitcoin/bitcoin/pull/19430
100 2020-07-02T11:26:52  *** bitcoin-git has left #bitcoin-core-dev
101 2020-07-02T11:27:23  *** jonatack has joined #bitcoin-core-dev
102 2020-07-02T11:33:23  *** vasild has quit IRC
103 2020-07-02T11:36:01  *** vasild has joined #bitcoin-core-dev
104 2020-07-02T11:36:19  *** promag has quit IRC
105 2020-07-02T11:38:40  *** bitcoin-git has joined #bitcoin-core-dev
106 2020-07-02T11:38:40  <bitcoin-git> [bitcoin] MarcoFalke opened pull request #19431: ci: Avoid failing pull requests destory the appveyor cache (master...2007-ciAppv) https://github.com/bitcoin/bitcoin/pull/19431
107 2020-07-02T11:38:41  *** bitcoin-git has left #bitcoin-core-dev
108 2020-07-02T11:48:42  *** promag has joined #bitcoin-core-dev
109 2020-07-02T11:52:15  *** gleb1 has joined #bitcoin-core-dev
110 2020-07-02T11:53:33  *** promag has quit IRC
111 2020-07-02T11:54:34  *** gleb has quit IRC
112 2020-07-02T11:54:35  *** gleb1 is now known as gleb
113 2020-07-02T12:00:02  *** sledges has quit IRC
114 2020-07-02T12:05:10  *** Pavlenex has quit IRC
115 2020-07-02T12:17:39  *** Guyver2_ has joined #bitcoin-core-dev
116 2020-07-02T12:20:50  *** Guyver2 has quit IRC
117 2020-07-02T12:22:20  *** jesusabdullah has joined #bitcoin-core-dev
118 2020-07-02T12:24:17  *** AaronvanW has quit IRC
119 2020-07-02T12:27:13  *** Guyver2_ has quit IRC
120 2020-07-02T12:31:13  *** ppisati has joined #bitcoin-core-dev
121 2020-07-02T12:31:30  *** Highway61 has joined #bitcoin-core-dev
122 2020-07-02T12:33:53  *** bitcoin-git has joined #bitcoin-core-dev
123 2020-07-02T12:33:53  <bitcoin-git> [bitcoin] MarcoFalke closed pull request #19431: ci: Avoid failing pull requests destory the appveyor cache (master...2007-ciAppv) https://github.com/bitcoin/bitcoin/pull/19431
124 2020-07-02T12:33:54  *** bitcoin-git has left #bitcoin-core-dev
125 2020-07-02T12:34:13  *** bitcoin-git has joined #bitcoin-core-dev
126 2020-07-02T12:34:13  <bitcoin-git> [bitcoin] MarcoFalke reopened pull request #19431: ci: Avoid failing pull requests destory the appveyor cache (master...2007-ciAppv) https://github.com/bitcoin/bitcoin/pull/19431
127 2020-07-02T12:34:14  *** bitcoin-git has left #bitcoin-core-dev
128 2020-07-02T12:34:50  *** shesek has quit IRC
129 2020-07-02T12:38:20  *** IGHOR has quit IRC
130 2020-07-02T12:42:04  *** IGHOR has joined #bitcoin-core-dev
131 2020-07-02T12:46:06  *** ipriver has joined #bitcoin-core-dev
132 2020-07-02T12:46:51  *** spaced0ut has joined #bitcoin-core-dev
133 2020-07-02T12:47:11  *** promag has joined #bitcoin-core-dev
134 2020-07-02T12:50:41  *** ipriver has quit IRC
135 2020-07-02T12:52:46  *** reallll is now known as belcher
136 2020-07-02T13:16:51  *** mdunnio has joined #bitcoin-core-dev
137 2020-07-02T13:19:21  *** afk11` is now known as afk11
138 2020-07-02T13:19:26  *** proofofkeags has joined #bitcoin-core-dev
139 2020-07-02T13:34:58  *** Davterra has quit IRC
140 2020-07-02T13:42:22  *** Davterra has joined #bitcoin-core-dev
141 2020-07-02T13:47:12  *** troygiorshev has joined #bitcoin-core-dev
142 2020-07-02T14:07:02  *** mdunnio has quit IRC
143 2020-07-02T14:07:18  *** mdunnio has joined #bitcoin-core-dev
144 2020-07-02T14:11:58  *** bitcoin-git has joined #bitcoin-core-dev
145 2020-07-02T14:11:59  <bitcoin-git> [bitcoin] laanwj pushed 4 commits to master: https://github.com/bitcoin/bitcoin/compare/d6fe5b28dff4...7173a3c73ba9
146 2020-07-02T14:12:00  <bitcoin-git> bitcoin/master fa2eb38 MarcoFalke: interfaces: Remove unused getDefaultChangeType
147 2020-07-02T14:12:00  <bitcoin-git> bitcoin/master faddad7 MarcoFalke: Remove confusing OutputType::CHANGE_AUTO
148 2020-07-02T14:12:00  <bitcoin-git> bitcoin/master fa927ff MarcoFalke: Enable Wswitch for OutputType
149 2020-07-02T14:12:01  *** bitcoin-git has left #bitcoin-core-dev
150 2020-07-02T14:12:19  *** bitcoin-git has joined #bitcoin-core-dev
151 2020-07-02T14:12:19  <bitcoin-git> [bitcoin] laanwj merged pull request #19396: refactor: Remove confusing OutputType::CHANGE_AUTO (master...2006-WswitchOutputType) https://github.com/bitcoin/bitcoin/pull/19396
152 2020-07-02T14:12:20  *** bitcoin-git has left #bitcoin-core-dev
153 2020-07-02T14:15:09  *** Guyver2 has joined #bitcoin-core-dev
154 2020-07-02T14:23:18  *** bitcoin-git has joined #bitcoin-core-dev
155 2020-07-02T14:23:18  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/7173a3c73ba9...d77170d52687
156 2020-07-02T14:23:19  <bitcoin-git> bitcoin/master fa12d8d MarcoFalke: ci: Add tsan suppression for race in wallet
157 2020-07-02T14:23:19  <bitcoin-git> bitcoin/master d77170d MarcoFalke: Merge #19422: ci: Add tsan suppression for race in wallet
158 2020-07-02T14:23:21  *** bitcoin-git has left #bitcoin-core-dev
159 2020-07-02T14:23:38  *** bitcoin-git has joined #bitcoin-core-dev
160 2020-07-02T14:23:38  <bitcoin-git> [bitcoin] MarcoFalke merged pull request #19422: ci: Add tsan suppression for race in wallet (master...2007-ciTsanSupW) https://github.com/bitcoin/bitcoin/pull/19422
161 2020-07-02T14:23:39  *** bitcoin-git has left #bitcoin-core-dev
162 2020-07-02T14:25:25  *** snifflin has joined #bitcoin-core-dev
163 2020-07-02T14:28:27  *** bitcoin-git has joined #bitcoin-core-dev
164 2020-07-02T14:28:27  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/d77170d52687...7027c67cac85
165 2020-07-02T14:28:27  <bitcoin-git> bitcoin/master 870f0cd practicalswift: build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized...
166 2020-07-02T14:28:28  <bitcoin-git> bitcoin/master 7027c67 MarcoFalke: Merge #18288: build: Add MemorySanitizer (MSan) in Travis to detect use of...
167 2020-07-02T14:28:29  *** bitcoin-git has left #bitcoin-core-dev
168 2020-07-02T14:29:27  *** bitcoin-git has joined #bitcoin-core-dev
169 2020-07-02T14:29:27  <bitcoin-git> [bitcoin] MarcoFalke merged pull request #18288: build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory (master...msan-in-travis) https://github.com/bitcoin/bitcoin/pull/18288
170 2020-07-02T14:29:29  *** bitcoin-git has left #bitcoin-core-dev
171 2020-07-02T14:32:05  *** kristapsk has joined #bitcoin-core-dev
172 2020-07-02T14:32:35  <jnewbery> aj: not that I know of. I just assumed that because of the comment "Maintain validation-specific state about nodes, protected by cs_main" there was some validation-specific state that needed to be guarded by cs_main. Maybe there isn't.
173 2020-07-02T14:33:25  <aj> jnewbery: i took it as "it needs to be guarded by something, cs_main is something, ..."
174 2020-07-02T14:33:33  *** GAit has joined #bitcoin-core-dev
175 2020-07-02T14:33:45  *** bitdex has quit IRC
176 2020-07-02T14:42:17  <snifflin> midnight ------->> [[ You are as pointless and irrelevant as an odious ignorant faulty heap of rancid donkey balls ]] <<-------
177 2020-07-02T14:42:24  <snifflin> sorry for the interuption, carry on.
178 2020-07-02T14:43:06  *** Relis has joined #bitcoin-core-dev
179 2020-07-02T14:43:16  *** ChanServ sets mode: +o luke-jr
180 2020-07-02T14:43:17  *** snifflin was kicked by luke-jr (User terminated!)
181 2020-07-02T14:43:19  *** luke-jr sets mode: -o luke-jr
182 2020-07-02T14:43:37  *** snifflin has joined #bitcoin-core-dev
183 2020-07-02T14:43:57  *** snifflin has left #bitcoin-core-dev
184 2020-07-02T14:49:42  *** alko has quit IRC
185 2020-07-02T14:56:57  *** promag has quit IRC
186 2020-07-02T15:00:01  *** jesusabdullah has quit IRC
187 2020-07-02T15:01:16  *** promag has joined #bitcoin-core-dev
188 2020-07-02T15:05:34  *** promag has quit IRC
189 2020-07-02T15:08:51  *** proofofkeags has quit IRC
190 2020-07-02T15:09:23  *** proofofkeags has joined #bitcoin-core-dev
191 2020-07-02T15:10:23  *** go11111111111 has joined #bitcoin-core-dev
192 2020-07-02T15:11:22  *** dongcarl2 has joined #bitcoin-core-dev
193 2020-07-02T15:11:26  *** dongcarl2 has quit IRC
194 2020-07-02T15:12:00  *** Victorsueca has joined #bitcoin-core-dev
195 2020-07-02T15:12:11  *** niftynei_ has joined #bitcoin-core-dev
196 2020-07-02T15:12:38  *** sharpiro_ has joined #bitcoin-core-dev
197 2020-07-02T15:13:08  *** mol_ has joined #bitcoin-core-dev
198 2020-07-02T15:13:48  *** ovovo has joined #bitcoin-core-dev
199 2020-07-02T15:13:48  *** ovovo has joined #bitcoin-core-dev
200 2020-07-02T15:13:50  *** proofofkeags has quit IRC
201 2020-07-02T15:13:51  *** spaced0ut has quit IRC
202 2020-07-02T15:14:15  *** owowo has quit IRC
203 2020-07-02T15:14:15  *** niftynei has quit IRC
204 2020-07-02T15:14:15  *** Victor_sueca has quit IRC
205 2020-07-02T15:14:15  *** achow101 has quit IRC
206 2020-07-02T15:14:18  *** TheRec_ has joined #bitcoin-core-dev
207 2020-07-02T15:14:18  *** TheRec_ has joined #bitcoin-core-dev
208 2020-07-02T15:14:25  *** achow101 has joined #bitcoin-core-dev
209 2020-07-02T15:14:31  *** dongcarl has quit IRC
210 2020-07-02T15:14:32  *** so has quit IRC
211 2020-07-02T15:14:32  *** go121212 has quit IRC
212 2020-07-02T15:14:32  *** TheRec has quit IRC
213 2020-07-02T15:14:32  *** niska has quit IRC
214 2020-07-02T15:14:32  *** niska has joined #bitcoin-core-dev
215 2020-07-02T15:14:53  *** yancy has quit IRC
216 2020-07-02T15:15:11  *** yancy has joined #bitcoin-core-dev
217 2020-07-02T15:15:14  *** esotericnonsense has quit IRC
218 2020-07-02T15:15:14  *** davec has quit IRC
219 2020-07-02T15:15:22  *** a5m0 has quit IRC
220 2020-07-02T15:15:22  *** sharpiro has quit IRC
221 2020-07-02T15:15:22  *** TheHoliestRoger has quit IRC
222 2020-07-02T15:15:44  *** so has joined #bitcoin-core-dev
223 2020-07-02T15:15:50  *** mol has quit IRC
224 2020-07-02T15:16:27  *** davec has joined #bitcoin-core-dev
225 2020-07-02T15:17:00  *** a5m0 has joined #bitcoin-core-dev
226 2020-07-02T15:17:51  *** esotericnonsense has joined #bitcoin-core-dev
227 2020-07-02T15:17:59  *** TheHoliestRoger has joined #bitcoin-core-dev
228 2020-07-02T15:19:40  *** benthecarman has joined #bitcoin-core-dev
229 2020-07-02T15:20:37  *** dongcarl has joined #bitcoin-core-dev
230 2020-07-02T15:22:11  *** fimp has joined #bitcoin-core-dev
231 2020-07-02T15:22:36  *** fimp is now known as Guest36896
232 2020-07-02T15:30:08  *** Relis has quit IRC
233 2020-07-02T15:33:07  *** Pavlenex has joined #bitcoin-core-dev
234 2020-07-02T15:33:49  *** spinza has quit IRC
235 2020-07-02T15:35:10  *** promag has joined #bitcoin-core-dev
236 2020-07-02T15:36:13  *** Pavlenex has quit IRC
237 2020-07-02T15:38:21  *** Relis has joined #bitcoin-core-dev
238 2020-07-02T15:47:46  *** Relis has quit IRC
239 2020-07-02T15:48:07  *** spinza has joined #bitcoin-core-dev
240 2020-07-02T15:48:13  *** benthecarman_ has joined #bitcoin-core-dev
241 2020-07-02T15:50:54  *** benthecarman has quit IRC
242 2020-07-02T15:54:42  *** benthecarman_ has quit IRC
243 2020-07-02T15:58:30  *** Talkless has joined #bitcoin-core-dev
244 2020-07-02T16:06:48  *** vasild has quit IRC
245 2020-07-02T16:07:15  *** vasild has joined #bitcoin-core-dev
246 2020-07-02T16:13:19  <jnewbery> aj: you're probably right. A lot has changed since that comment was written
247 2020-07-02T16:13:46  <jnewbery> in which case, everything from CNodeState would eventually land in PeerState
248 2020-07-02T16:14:27  <jnewbery> but even if not, having somewhere to put non-cs_main-guarded per-peer data in net processing is useful
249 2020-07-02T16:17:13  <aj> jnewbery: i'd look at it more as "we're not 100% sure we can move everything in CNodeState out from under cs_main, so we'll do it piece by piece by moving parts of it to PeerState and making sure we understand each step", but same conclusion
250 2020-07-02T16:17:32  <jnewbery> yes, agree 100%
251 2020-07-02T16:18:01  <aj> jnewbery: having a shared_ptr and separate mutexes for each area struck me as odd, but i think i'm coming around to it
252 2020-07-02T16:20:11  *** justanotheruser has quit IRC
253 2020-07-02T16:20:41  <jnewbery> it's similar to how CNodes are currently managed, except we let the shared_ptrs do the refcounting instead of doing it ourselves
254 2020-07-02T16:22:14  <sipa> for something like how CNodes are managed, using shared_ptrs instead makes sense i think
255 2020-07-02T16:22:30  <sipa> i'm not convinced that's necessary or useful in net_processing though
256 2020-07-02T16:23:32  <jnewbery> sipa: I was about to summon you because I know you have opinions about this
257 2020-07-02T16:23:50  <sipa> (cnode has weird try_locks etc, and deletes node.objects while they're still in use etc)
258 2020-07-02T16:24:00  <sipa> i think net_processing is fundamentally much simpler
259 2020-07-02T16:24:54  <jnewbery> I think those try-locks can all be removed at this point. They were to protect against lock order inversions that can't happen now
260 2020-07-02T16:24:58  <aj> i think if you don't use shared_ptr's then you need to retain a lock on the map while you use the value while you use it?
261 2020-07-02T16:25:27  <sipa> i think all of net_processing can be one lock
262 2020-07-02T16:26:23  <sipa> perhaps if you really want you can have one lock for block downloading, one for tx downloasing, one for addrman stuff, ...
263 2020-07-02T16:26:32  <sipa> but i don't think that gains you much
264 2020-07-02T16:26:55  <sipa> net_processing is inherently a serial affair of processing messages in some order
265 2020-07-02T16:27:21  <aj> could process multiple peers simultaneously though?
266 2020-07-02T16:27:29  <sipa> not really
267 2020-07-02T16:27:38  <sipa> look at tx downloading
268 2020-07-02T16:27:39  <jnewbery> aj: I want to at least leave that option open
269 2020-07-02T16:28:28  <sipa> as soon as you hit any message related to tx downloading, you'd fall back to serial processing
270 2020-07-02T16:28:40  <sipa> because there is global state that is shared across all peers
271 2020-07-02T16:29:21  <sipa> for ping/pong you can do parallel processing, and maybe a few other small ones
272 2020-07-02T16:30:17  <sipa> if net processing is really a bottleneck, i think we're better off just running multiple connmanns/net_processong instances, each with their own group of peers
273 2020-07-02T16:30:25  <aj> jnewbery: (otoh, that option's always open: we just change the code later)
274 2020-07-02T16:31:09  <aj> i wonder occasionally at whether we should have some reader/writer locks
275 2020-07-02T16:31:11  <jnewbery> aj: breaking up global locks has proven to be difficult in the past. People have wanted to break up cs_main for as long as I've been on the project
276 2020-07-02T16:31:58  <aj> jnewbery: i think having GUARDED_BY now might make that easier, presuming we annotate things from the start
277 2020-07-02T16:32:23  <sipa> my ideal view is actually that net_processing has no locks; and is just a single "message processing" process that deals with incoming messages and creates outgoing one
278 2020-07-02T16:32:43  <sipa> no locks for any internal data structures necessary, because only one thread can ever access them
279 2020-07-02T16:33:04  <aj> you need to be able to access them via RPC for getpeerinfo and the like
280 2020-07-02T16:33:31  <aj> but that would be super nice
281 2020-07-02T16:33:50  <sipa> that could be done by having a separately-locked stats object that is updated by the net_processing thread
282 2020-07-02T16:34:08  <jnewbery> sipa: I understand what you're saying, but I think moving the same locks from CNode to PeerState with perhaps some minor tidyup along the way is both the most conservative change and leaves the most options open for future changes
283 2020-07-02T16:34:35  <sipa> jnewbery: i cannot comprehend why you want per-peer locks for thos
284 2020-07-02T16:34:50  <jnewbery> if in the future we want to consolidate all those locks under cs_net_processing or whatever, that's fine
285 2020-07-02T16:35:07  <sipa> for anything but the most basic messages, net_processing involves global state
286 2020-07-02T16:35:44  <sipa> this is true for tx download, block download, compact block processing, ip addrs, ...
287 2020-07-02T16:36:07  <jnewbery> sipa: because it allows us to make the changes slowly over time without having to completely change our locking model
288 2020-07-02T16:36:08  *** justanotheruser has joined #bitcoin-core-dev
289 2020-07-02T16:36:48  <sipa> jnewbery: sure, but you could do the same with a cs_tx_inv global lock, not a per-peer one
290 2020-07-02T16:38:01  <sipa> amd similar things
291 2020-07-02T16:38:49  <sipa> every piece of net_processing that has some (currently cs_main-locked) global state will need a replacement lock for that data structure
292 2020-07-02T16:40:13  <jnewbery> how do you synchronize access to the PeerState list? take a lock and hold it for the entire time you're in net_processing?
293 2020-07-02T16:41:15  <aj> map<nodeid, shared_ptr<PeerState>> GUARDED_BY(cs_peerstate), that only needs to be held while you access/update seems fine?
294 2020-07-02T16:41:56  <aj> (with each member of PeerState protected by some global cs_whatever)
295 2020-07-02T16:42:28  <jnewbery> if you're going to do that, then you need a refcount to make sure you don't delete it while someone else is holding the PeerState
296 2020-07-02T16:42:40  <aj> shared_ptr does that?
297 2020-07-02T16:42:50  <sipa> jnewbery: if all of net_processing is one lock, it's easy
298 2020-07-02T16:43:04  <sipa> jnewbery: if you have separate sublocks... yeah, shared_ptr
299 2020-07-02T16:44:47  <jnewbery> If we're doing separate subblocks (which I think we should for maximum flexibility) then there's very little difference between making those locks per-peer or global
300 2020-07-02T16:45:08  <jnewbery> except if you do it per-peer, you leave the possibility open that you can do more parallelism in future
301 2020-07-02T16:45:27  <jnewbery> if you make it a global it's very difficult to unpick that in a future change
302 2020-07-02T16:46:09  <sipa> jnewbery: do you think my TxRequestTracker needs per-peer locks?
303 2020-07-02T16:46:36  *** proofofkeags has joined #bitcoin-core-dev
304 2020-07-02T16:46:52  <aj> having all the data be private to a net processing thread and not needing any locking seems a better future direection (and in line with moving p2p to its own process to reduce attack surface) to me
305 2020-07-02T16:47:45  <jnewbery> sipa: no
306 2020-07-02T16:48:06  <sipa> but perhaps before that PR, you'd be inclined to give it per-peer locks?
307 2020-07-02T16:48:27  <jnewbery> sipa: probably not, no
308 2020-07-02T16:48:29  <sipa> (for the per-peer fields)
309 2020-07-02T16:48:52  <sipa> jnewbery: so why do you think it's useful for other things?
310 2020-07-02T16:49:34  <sipa> apart from ping/pong, i don't think there is any net_processing that does not involve some direct or indirect global state
311 2020-07-02T16:49:43  <jnewbery> I'm not arguing that having per-peer locks is the perfect way of doing things. It may not be. But it's the most obvious change for me that we bring the locks with us when we move the data from net to net_processing. And then maybe argue about what's the perfect way to do things.
312 2020-07-02T16:49:46  <sipa> (including mempool, addrman, ...)
313 2020-07-02T16:50:19  <sipa> jnewbery: i don't mind just moving things from net to net_processing, and taking the locks with it
314 2020-07-02T16:50:21  *** nik-j has joined #bitcoin-core-dev
315 2020-07-02T16:50:32  <jnewbery> there have been many attempts to split cs_main and none of them have gone anywhere because people have spent more energy arguing about what the perfect model is than making incremental improvements.
316 2020-07-02T16:51:03  *** nik-j has quit IRC
317 2020-07-02T16:51:05  <jnewbery> and we're in a position now where people actually add _more_ net processing data to net because it's easier to do that then fix the problem
318 2020-07-02T16:51:21  <sipa> but if you're going to make changes to the locking system, i think that keeping/introducing per-peer locks is just needless complication
319 2020-07-02T16:52:17  <jnewbery> sipa: have you reviewed: https://github.com/jnewbery/bitcoin/tree/2020-06-cs-main-split? I don't think it's too complicated.
320 2020-07-02T16:52:54  <sipa> will do!
321 2020-07-02T16:53:11  <jnewbery> and I think it's less reviewer effort to review something that reflects the existing locking rather than moving to a completely different synchronization system for net processing
322 2020-07-02T16:53:24  <sipa> that makes sense
323 2020-07-02T16:53:56  *** nik-j has joined #bitcoin-core-dev
324 2020-07-02T16:54:07  <jnewbery> issue is here: https://github.com/bitcoin/bitcoin/issues/19398. I might try to summarize this conversation there at some point so it's not lost
325 2020-07-02T16:54:12  <sipa> jnewbery: while i have you, regarding the global orphan processing... do you understand what i mean when saying that the current approach is observationally equivalent to immediately processing all dependent orphans recursively?
326 2020-07-02T16:55:10  <jnewbery> from a single peer, it's observationally equivalent. I understand
327 2020-07-02T16:55:42  <sipa> right ok
328 2020-07-02T16:55:53  <sipa> so i don't think that's comparable
329 2020-07-02T16:56:55  <sipa> it may be the case that we're ok with the behavior of making orphan processing observationally asynchronous... but that is a pretty fundamental change
330 2020-07-02T16:57:28  <jnewbery> hmm I'm just struggling to see how fundamental that change is
331 2020-07-02T16:57:41  <jnewbery> I have to go now, but would love to continue this conversation later
332 2020-07-02T16:57:48  <sipa> and your PR presents it as: there is no good reasons why this is per-peer
333 2020-07-02T16:58:05  <sipa> i think there is: it's to make orphan processing observationally synchronous
334 2020-07-02T16:58:17  *** jonatack has quit IRC
335 2020-07-02T16:58:50  <sipa> (and it took me a long time to realize that, i don't blame anyone for not realising that; it was badly documented too)
336 2020-07-02T17:00:09  <sipa> maybe that reason is silly or unnecessary, but since our protocol is (mostly) synchronous, i think deviations from that need more than a "probably nothing relies on this" justification
337 2020-07-02T17:02:02  <jnewbery> I think it's very different from getdata queueing, which should remain synchronous, and have more to say, but really must go now!
338 2020-07-02T17:03:37  <sipa> note that i didn't nack your change
339 2020-07-02T17:03:50  <sipa> i just think it needs more thought around potential impact
340 2020-07-02T17:04:51  <sipa> jnewbery: sure, we'll discuss more later
341 2020-07-02T17:08:29  *** molz_ has joined #bitcoin-core-dev
342 2020-07-02T17:11:31  *** mol_ has quit IRC
343 2020-07-02T17:13:56  *** nik-j_ has joined #bitcoin-core-dev
344 2020-07-02T17:17:37  *** nik-j has quit IRC
345 2020-07-02T17:19:05  *** nik-j_ has quit IRC
346 2020-07-02T17:19:47  *** nik-j has joined #bitcoin-core-dev
347 2020-07-02T17:21:06  *** promag has quit IRC
348 2020-07-02T17:23:10  *** promag has joined #bitcoin-core-dev
349 2020-07-02T17:23:16  *** ovovo has quit IRC
350 2020-07-02T17:24:18  *** nik-j has quit IRC
351 2020-07-02T17:24:23  *** owowo has joined #bitcoin-core-dev
352 2020-07-02T17:26:19  <wumpus> promag: wasn't there still some linux distro that we support (for building on) that uses 2.0.x? I don't know tbh, I'm kind of tired of version bumping discussions :)
353 2020-07-02T17:26:52  <promag> wumpus: I really don't know
354 2020-07-02T17:27:20  <promag> hebasto: you were on this land too right?
355 2020-07-02T17:27:43  <hebasto> wumpus: yes
356 2020-07-02T17:27:46  <wumpus> sometimes it's like that's such a large part of the work done, do we need to bump dependency X or depency Y and to what version and what can we support and how can we support the (sometimes years old) linux distributiosn it needs to build on and blabla
357 2020-07-02T17:28:03  <wumpus> kind of frustrating
358 2020-07-02T17:28:27  <wumpus> (that doesn't mean unimportant though...)
359 2020-07-02T17:28:50  <promag> but the argument is to support the libevent availble on the old distro package archive?
360 2020-07-02T17:28:50  <hebasto> it seems I miss the context: 2.0.x of what?
361 2020-07-02T17:28:56  <promag> libevent
362 2020-07-02T17:28:58  <wumpus> libeventt
363 2020-07-02T17:29:11  *** jonatack has joined #bitcoin-core-dev
364 2020-07-02T17:30:52  <luke-jr> I wonder if we can automate version lookups
365 2020-07-02T17:31:44  <hebasto> we should agree on supported distros list in advance, no?
366 2020-07-02T17:32:00  <luke-jr> hebasto: so far it's basically been latest stable of major distros
367 2020-07-02T17:32:07  <wumpus> luke-jr: that'd be nice
368 2020-07-02T17:32:13  <promag> ideally we would bump to 2.2.0 - then we could drop a workaround
369 2020-07-02T17:32:20  <wumpus> is that all?
370 2020-07-02T17:32:31  <luke-jr> I suspect we could probably get most of this just by checking versions when RHEL releases
371 2020-07-02T17:32:31  <promag> wumpus: ?
372 2020-07-02T17:32:33  <wumpus> 'drop a workaround' is not very pressing to really break support for anything
373 2020-07-02T17:32:39  <luke-jr> ^
374 2020-07-02T17:32:47  <promag> wumpus: no
375 2020-07-02T17:32:52  <wumpus> unless it's really a lot of code
376 2020-07-02T17:33:00  <luke-jr> promag: even Gentoo doesn't have 2.2.0 yet
377 2020-07-02T17:33:09  <promag> this #19420
378 2020-07-02T17:33:11  <gribble> https://github.com/bitcoin/bitcoin/issues/19420 | http: Track active requests and wait for last to finish by promag · Pull Request #19420 · bitcoin/bitcoin · GitHub
379 2020-07-02T17:33:21  <promag> luke-jr: and 2.1.*?
380 2020-07-02T17:33:28  <wumpus> it's fine to conditionally support things for newer versions
381 2020-07-02T17:33:32  <luke-jr> 2.1.x has been in Gentoo for a long time :P
382 2020-07-02T17:33:34  <wumpus> it doesn't mean you always have to drop support for older ones
383 2020-07-02T17:33:38  <luke-jr> 2.1.11 now
384 2020-07-02T17:33:51  <wumpus> it's very good to say 'with this new version of libevent things will work better'
385 2020-07-02T17:34:01  <wumpus> but unless it's a *critical* bug, breaking supporti s not worth it
386 2020-07-02T17:34:11  <promag> I can do that
387 2020-07-02T17:34:15  *** Evel-Knievel has joined #bitcoin-core-dev
388 2020-07-02T17:34:36  <wumpus> for many practical RPC users, for ex, it's not a problem that say, libevent can't handle 1000 connnections per second
389 2020-07-02T17:35:04  <wumpus> that it's in edge cases possible to exceed the number of file descriptors
390 2020-07-02T17:35:15  <wumpus> yes, some people run into that, but by far most don't
391 2020-07-02T17:35:18  <promag> the fix for that is not release I think
392 2020-07-02T17:35:31  <wumpus> yea,it's an example :)
393 2020-07-02T17:36:12  <promag> I also don't think it's really worth a bunch of #ifdef just to support some really old libevent
394 2020-07-02T17:36:17  <wumpus> I just mean, in cases like that, you could add to the release notes 'please use version XXX of libevent or higher if you suffer from this issue'
395 2020-07-02T17:37:06  <wumpus> that's very subjective, if it's a small amount of code that's only a little bit different
396 2020-07-02T17:37:33  <promag> ok, I'll try to make the patch conditional
397 2020-07-02T17:39:09  <wumpus> in any case if it passes travis it's probably ok, i think we now test the oldest versions that need to be supported in travis
398 2020-07-02T17:39:34  <wumpus> and when C++17 requirement hits verious versions are going to be bumped anyhow
399 2020-07-02T17:40:11  <wumpus> but if you have a PR open you're probably not going to want to wait for that
400 2020-07-02T17:40:42  <promag> wumpus: it does passes on travis
401 2020-07-02T17:41:50  <promag> right, we don't test what we say is the minimum supported version
402 2020-07-02T17:42:04  *** Evel-Knievel has quit IRC
403 2020-07-02T17:42:36  *** Evel-Knievel has joined #bitcoin-core-dev
404 2020-07-02T17:49:37  *** jonatack has quit IRC
405 2020-07-02T17:51:53  *** jonatack has joined #bitcoin-core-dev
406 2020-07-02T17:55:12  <wumpus> hm then I'm wrong, I thought we had a run with minimum versions (some fedora version) now
407 2020-07-02T17:55:25  *** alko89 has quit IRC
408 2020-07-02T17:55:43  *** alko89 has joined #bitcoin-core-dev
409 2020-07-02T17:56:29  *** filchef has joined #bitcoin-core-dev
410 2020-07-02T18:00:02  *** Guest36896 has quit IRC
411 2020-07-02T18:06:19  *** Talkless has quit IRC
412 2020-07-02T18:07:37  *** Talkless has joined #bitcoin-core-dev
413 2020-07-02T18:09:13  <promag> in doc/dependencies.md we say we support 2.0.21
414 2020-07-02T18:09:48  <promag> but in #19420 I'm using 2.1 stuff
415 2020-07-02T18:09:49  <gribble> https://github.com/bitcoin/bitcoin/issues/19420 | http: Track active requests and wait for last to finish by promag · Pull Request #19420 · bitcoin/bitcoin · GitHub
416 2020-07-02T18:10:28  *** Talkless has joined #bitcoin-core-dev
417 2020-07-02T18:21:27  *** corelax1 has joined #bitcoin-core-dev
418 2020-07-02T18:25:15  *** Relis has joined #bitcoin-core-dev
419 2020-07-02T18:26:18  *** alko has joined #bitcoin-core-dev
420 2020-07-02T18:34:23  <jnewbery> sipa: the difference between queueing getdata requests and queueing orphan reconsideration is that orphan reconsideration won't ever result in a response message being sent to the peer that provided the parent
421 2020-07-02T18:35:08  <jnewbery> (aside: it may result in the now-accepted orphan transaction being relayed to all peers, including the peer that provided the parent)
422 2020-07-02T18:35:55  <jnewbery> so the only difference is whether our internal state (mempool, chaintip) has changed by the time we process any subsequent messages
423 2020-07-02T18:37:30  <jnewbery> but that's already the case, since #15644 - the internal state may change because of messages received from other peers while the orphan reconsideration was queued
424 2020-07-02T18:37:33  <gribble> https://github.com/bitcoin/bitcoin/issues/15644 | Make orphan processing interruptible by sipa · Pull Request #15644 · bitcoin/bitcoin · GitHub
425 2020-07-02T18:38:18  <sipa> jnewbery: but that's not observable
426 2020-07-02T18:38:39  <sipa> the only change introduced by 15644 is inserting pauses
427 2020-07-02T18:38:58  <jnewbery> but the internal state can change during those pauses
428 2020-07-02T18:39:09  <sipa> it's equivalent to having some line delay before receiving certain messages
429 2020-07-02T18:40:49  <sipa> jnewbery: i think you don't understand what i mean by observation equivalence
430 2020-07-02T18:41:11  <sipa> yes, of course it's operationally a change - the code is different, is does something else
431 2020-07-02T18:41:17  <sipa> but peers can't tell that we do
432 2020-07-02T18:41:17  *** Evel-Knievel has quit IRC
433 2020-07-02T18:41:25  <sipa> (at least, individual peers can't)
434 2020-07-02T18:41:27  *** Evel-Knievel has joined #bitcoin-core-dev
435 2020-07-02T18:41:31  <jnewbery> the scenario you're concerned about, I think, is if a peer sends us TX1 followed by MSG2. We might process MSG2 before we've finished reconsidering the children of TX1
436 2020-07-02T18:41:35  *** mdunnio has quit IRC
437 2020-07-02T18:41:39  <sipa> indeed
438 2020-07-02T18:41:43  *** alko has quit IRC
439 2020-07-02T18:41:56  *** alko has joined #bitcoin-core-dev
440 2020-07-02T18:42:19  <jnewbery> and the response to MSG2 might be different depending on our processing of the children of TX1
441 2020-07-02T18:42:26  <sipa> one concern i had in particular was where MSG2 is a compact block, causing reconstruction to fail... but that's not the case because compact block reconconstruction also looks at the orphan pool
442 2020-07-02T18:42:32  <sipa> but things like that is what i worry about
443 2020-07-02T18:43:52  *** promag has quit IRC
444 2020-07-02T18:45:42  <jnewbery> but it's also true that the response to MSG2 might be different if we'd received a message from peerB between accepting TX1 and reconsidering its children
445 2020-07-02T18:46:28  <sipa> yes, obviously - but why is that relevant?
446 2020-07-02T18:46:39  <sipa> there are no guarantees about speed of network messages
447 2020-07-02T18:46:48  <sipa> or their ordering
448 2020-07-02T18:46:58  <sipa> only that the messages from one peer arrive in the order they're sent
449 2020-07-02T18:48:11  <jnewbery> because I don't see how that's any different from reconsidering the orphans on a global basis. There's no reason that the peer that provides the parent should care about orphans that another peer has sent, and there's no reason that processing should be coupled
450 2020-07-02T18:48:36  <sipa> maybe
451 2020-07-02T18:48:56  <sipa> it's just a fundamental change to suddenly not processing incoming messages in order anymore
452 2020-07-02T18:49:29  <sipa> and i don't think just cleaner code is a justification for changing that
453 2020-07-02T18:49:40  *** mdunnio has joined #bitcoin-core-dev
454 2020-07-02T18:50:02  *** pinheadmz has quit IRC
455 2020-07-02T18:50:52  <sipa> now, i cannot give an example for why this may matter to anyone
456 2020-07-02T18:51:29  <jnewbery> If I have peerA and peerB, and construct TX1 -> TX2 -> ... -> TX100, then provide TX2-TX100 to peerB, and provide TX1 to peerA
457 2020-07-02T18:51:49  *** Evel-Knievel has quit IRC
458 2020-07-02T18:52:19  <jnewbery> peerA relays TX1 to peerB, and peerB then reconsiders TX2-TX100 in the context of ReceiveMessages(peerA)
459 2020-07-02T18:52:28  *** Evel-Knievel has joined #bitcoin-core-dev
460 2020-07-02T18:53:09  <sipa> are you saying that the current behavior is a privacy leak?
461 2020-07-02T18:53:30  <sipa> if so, that'd be a better justification
462 2020-07-02T18:53:39  <jnewbery> No, I just don't see how it makes any sense
463 2020-07-02T18:53:56  <jnewbery> ok, I'll try to come up with a privacy leak for post-justification :)
464 2020-07-02T18:54:46  <sipa> if compact blocks didn't consider reconstruction from recent orphans, i think that'd be a very good example
465 2020-07-02T18:54:51  <sipa> it isn't - but it's very close
466 2020-07-02T18:55:43  <jnewbery> if at the same time, I provided TX2' to both peerA and peerB, then peerA would relay TX2' to peerB
467 2020-07-02T18:56:37  *** pinheadmz has joined #bitcoin-core-dev
468 2020-07-02T18:57:04  <jnewbery> prior to 15644, TX2 would always win, because it's reprocessed in the same ReceiveMessages() call
469 2020-07-02T18:57:33  <jnewbery> since 15644, TX2' might win, because it could be processed in one of those pauses
470 2020-07-02T18:58:20  <jnewbery> which is observable by peerA because peerB doesn't getdata(TX2')
471 2020-07-02T19:00:37  <wumpus> #startmeeting
472 2020-07-02T19:00:37  <lightningbot> Meeting started Thu Jul  2 19:00:37 2020 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
473 2020-07-02T19:00:37  <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
474 2020-07-02T19:00:40  *** gzhao408 has joined #bitcoin-core-dev
475 2020-07-02T19:00:42  <achow101> hi
476 2020-07-02T19:00:45  <jnewbery> hi
477 2020-07-02T19:00:45  <hebasto> hi
478 2020-07-02T19:00:45  <jkczyz> hi
479 2020-07-02T19:00:48  <jonasschnelli> Hi
480 2020-07-02T19:00:56  <wumpus> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator aj Chris_Stewart_5 dongcarl gwillen jamesob ken281221 ryanofsky gleb moneyball kvaciral ariard digi_james amiti fjahr
481 2020-07-02T19:00:58  <wumpus> jeremyrubin lightlike emilengler jonatack hebasto jb55 elichai2
482 2020-07-02T19:01:06  <kanzure> hi
483 2020-07-02T19:01:15  <provoostenator> hi
484 2020-07-02T19:01:17  <meshcollider> hi
485 2020-07-02T19:01:19  <jamesob> hi
486 2020-07-02T19:01:25  <aj> hi
487 2020-07-02T19:01:28  <fjahr> hi
488 2020-07-02T19:01:35  <pinheadmz> hi
489 2020-07-02T19:01:59  <jeremyrubin> hi
490 2020-07-02T19:02:07  <wumpus> looks like there have been no proposed meeting topics this week in  http://gnusha.org/bitcoin-core-dev/proposedmeetingtopics.txt
491 2020-07-02T19:02:29  <ariard> hi
492 2020-07-02T19:02:54  <dongcarl> hi
493 2020-07-02T19:02:58  <gzhao408> hi
494 2020-07-02T19:02:59  <elichai2> Hi
495 2020-07-02T19:03:43  <wumpus> any last minute topic proposals?
496 2020-07-02T19:04:04  <dongcarl> would like to discuss the various places we put documentation a bit, but perhaps after the meeting
497 2020-07-02T19:04:47  <wumpus> ok yes that's a bit vague, either you want to prpose it as a meeting topic or not?
498 2020-07-02T19:05:27  <wumpus> #topic High priority for review
499 2020-07-02T19:05:28  <dongcarl> sorry, I should be more clear, no, let's table that till after meeting
500 2020-07-02T19:05:38  <achow101> #19324 for hi prio
501 2020-07-02T19:05:42  <gribble> https://github.com/bitcoin/bitcoin/issues/19324 | wallet: Move BerkeleyBatch static functions to BerkeleyDatabase by achow101 · Pull Request #19324 · bitcoin/bitcoin · GitHub
502 2020-07-02T19:05:52  <wumpus> https://github.com/bitcoin/bitcoin/projects/8 11 blockers, 1 bugfix, 3 chasing concept ACK pending
503 2020-07-02T19:05:54  <wumpus> dongcarl: ok!
504 2020-07-02T19:06:10  <wumpus> achow101: yup. I predicted that one :)
505 2020-07-02T19:06:57  <wumpus> added
506 2020-07-02T19:06:59  <achow101> heh
507 2020-07-02T19:07:43  <dongcarl> #17919 please
508 2020-07-02T19:07:45  <gribble> https://github.com/bitcoin/bitcoin/issues/17919 | [DO NOT MERGE] depends: Allow building with system clang by dongcarl · Pull Request #17919 · bitcoin/bitcoin · GitHub
509 2020-07-02T19:08:04  *** Talkless has quit IRC
510 2020-07-02T19:08:08  <dongcarl> (the DO NOT MERGE is because of the last commit which is a demo, can be dropped easily)
511 2020-07-02T19:08:20  <wumpus> DO NOT MERGE ;)
512 2020-07-02T19:08:39  <wumpus> dongcarl: good to know!
513 2020-07-02T19:09:04  <wumpus> added
514 2020-07-02T19:09:05  <jeremyrubin> #proposedmeetingtopic I'd like to see the nanobench stuff get finished
515 2020-07-02T19:10:38  <wumpus> anything in high priority for review that is mergable or close to being mergable?
516 2020-07-02T19:10:49  *** Davterra has quit IRC
517 2020-07-02T19:11:02  <wumpus> we've merged a few smaller ones this week
518 2020-07-02T19:11:29  *** Davterra has joined #bitcoin-core-dev
519 2020-07-02T19:11:34  <wumpus> but looks like most are still busy in mid-review
520 2020-07-02T19:12:22  <jeremyrubin> I think #18191 is mergeable.
521 2020-07-02T19:12:24  <wumpus> #18637 looks somewhat nearing readyness
522 2020-07-02T19:12:24  <gribble> https://github.com/bitcoin/bitcoin/issues/18191 | Change UpdateForDescendants to use Epochs by JeremyRubin · Pull Request #18191 · bitcoin/bitcoin · GitHub
523 2020-07-02T19:12:27  <gribble> https://github.com/bitcoin/bitcoin/issues/18637 | coins: allow cache resize after init by jamesob · Pull Request #18637 · bitcoin/bitcoin · GitHub
524 2020-07-02T19:13:04  <jamesob> I'm +1 :)
525 2020-07-02T19:14:04  <wumpus> jeremyrubin: ok will take a look after the meeting
526 2020-07-02T19:14:29  <jnewbery> jeremyrubin: I can't see how 18191 is mergeable. I'd expect more than one ACK for changes to the mempool
527 2020-07-02T19:15:12  <wumpus> #topic Nanobench (jeremyrubin)
528 2020-07-02T19:15:15  <wumpus> jnewbery: yes
529 2020-07-02T19:15:53  <wumpus> seems a bit premature
530 2020-07-02T19:16:42  <jeremyrubin> fair
531 2020-07-02T19:17:39  <jeremyrubin> I had thought some of the other reviewers had correctness acked the code, but there is disagreement about benchmarking. I don't think it requires further benching at this time, as it's primarily a memory improvement and has some benches. But can always do more.
532 2020-07-02T19:17:53  <wumpus> I also wonder what sdaftuar_ thinks about it now, initially he didn't think it'd be worth the increase of complexity
533 2020-07-02T19:18:14  *** owowo has quit IRC
534 2020-07-02T19:19:10  <ariard> I'll try to review 18191 before next meeting, has been on my list for a while
535 2020-07-02T19:19:28  <wumpus> ariard: thanks!
536 2020-07-02T19:19:32  <jeremyrubin> Can send you some more details privately
537 2020-07-02T19:19:44  <jeremyrubin> it's a non trivial memory improvement for reorgs
538 2020-07-02T19:20:14  <jeremyrubin> ok onto nanobench
539 2020-07-02T19:20:21  <jeremyrubin> I think it's in pretty good shape
540 2020-07-02T19:20:55  <jeremyrubin> MarcoFalke needs to run with frequency lock enabled?
541 2020-07-02T19:20:55  <dongcarl> jeremyrubin: can you link to prs?
542 2020-07-02T19:20:58  <jeremyrubin> Uhh
543 2020-07-02T19:21:01  *** nik-j has joined #bitcoin-core-dev
544 2020-07-02T19:21:18  <jeremyrubin> #18011
545 2020-07-02T19:21:21  <gribble> https://github.com/bitcoin/bitcoin/issues/18011 | Replace current benchmarking framework with nanobench by martinus · Pull Request #18011 · bitcoin/bitcoin · GitHub
546 2020-07-02T19:22:51  <wumpus> I have no opinion on replacing the benchmark framework, will leave it to people more involved with benchmarking
547 2020-07-02T19:23:41  *** Evel-Knievel has quit IRC
548 2020-07-02T19:23:49  <jeremyrubin> I think we can just ship it and see what/if anything breaks. AFAIK it has been made output compatible with prior benching output format
549 2020-07-02T19:23:51  *** Evel-Knievel has joined #bitcoin-core-dev
550 2020-07-02T19:24:01  <jeremyrubin> the only concrete harm is that the relative numbers won't be comparable anymore
551 2020-07-02T19:24:18  <wumpus> so this doesn't add any dependencies?
552 2020-07-02T19:24:30  <jeremyrubin> It adds a checked in header file
553 2020-07-02T19:24:39  <jeremyrubin> Which martinus, the comitter, is the maintainer of
554 2020-07-02T19:24:45  <wumpus> yeah that's okay
555 2020-07-02T19:25:04  *** owowo has joined #bitcoin-core-dev
556 2020-07-02T19:25:09  <jeremyrubin> So I'd just be happy to have someone who actively is adding benching features
557 2020-07-02T19:25:22  *** nik-j has quit IRC
558 2020-07-02T19:25:27  <jeremyrubin> It has some nice new bells and whistles around helping auto categorize big o
559 2020-07-02T19:25:31  <wumpus> that's a point
560 2020-07-02T19:25:37  <sipa> jeremyrubin: why won't relative numbers be conparable?
561 2020-07-02T19:26:03  <jeremyrubin> It does a better job I think of auto detecting how many runs are required or something
562 2020-07-02T19:26:07  <jeremyrubin> so we do fewer runs
563 2020-07-02T19:26:20  <jeremyrubin> and there is less measurment-in-benchmark overhead I think?
564 2020-07-02T19:26:23  <sipa> ok, but the benchmark numbers should be the same?
565 2020-07-02T19:26:31  <sipa> or at least, closer to reality
566 2020-07-02T19:26:42  <dongcarl> "measure instructions, cycles, branches, instructions per cycle, branch misses" <- is this not done currently (w/o nanobench)?
567 2020-07-02T19:26:48  <jeremyrubin> Well the hot-loop code I think is faster? But the benches themselves won't change
568 2020-07-02T19:26:55  <jeremyrubin> No it is not dongcarl
569 2020-07-02T19:27:07  <jeremyrubin> So we get a bunch of new and cool outputs
570 2020-07-02T19:27:31  <jeremyrubin> But also a compatible output mode for tooling that can't be updated
571 2020-07-02T19:27:34  <sipa> current master just tests min/max/avg time of some fixed number of iterations of a test
572 2020-07-02T19:27:46  <wumpus> adding outputs is great
573 2020-07-02T19:27:56  <dongcarl> that sounds like a worthy feature to me
574 2020-07-02T19:28:13  <wumpus> does this still work on non-x86?
575 2020-07-02T19:29:04  <wumpus> I added measuring instruction cycles at some point but this was also removed again
576 2020-07-02T19:29:17  <jeremyrubin> Not sure wumpus
577 2020-07-02T19:29:39  <dongcarl> I don't see anything in their CI for non-x86
578 2020-07-02T19:29:53  <wumpus> jeremyrubin: I don't mind if it doesn't report all the numbers on non-x86, just that it compiles/works
579 2020-07-02T19:30:13  <sipa> that seems reasonable
580 2020-07-02T19:30:30  *** sipsorcery has joined #bitcoin-core-dev
581 2020-07-02T19:30:58  <jeremyrubin> I don'
582 2020-07-02T19:31:15  <jeremyrubin> *don't see anything explicitly incompatible except the turbo detection?
583 2020-07-02T19:31:17  <wumpus> the PR currently doesn't pass travis, not that that says everything :)
584 2020-07-02T19:31:23  <jeremyrubin> Maybe I'm missing some syscall
585 2020-07-02T19:31:59  <sipa> well how does it count instructions?
586 2020-07-02T19:32:15  <sipa> because on ARM only privileged code can do that
587 2020-07-02T19:32:29  <sipa> s/instructions/clock ticks/
588 2020-07-02T19:32:32  <jeremyrubin> Failures on Travis are build systme related
589 2020-07-02T19:32:43  <jeremyrubin> not inherent
590 2020-07-02T19:32:53  <jeremyrubin> https://travis-ci.org/github/bitcoin/bitcoin/builds/697933849
591 2020-07-02T19:33:07  <sipa> ok let's try to give it some.renewed review attention?
592 2020-07-02T19:33:32  <sipa> seems useful to me to make progress on (i think the current benchmark system is pretty stupid...)
593 2020-07-02T19:33:38  <wumpus> the ARM failure is really srange, just some timeout on package download
594 2020-07-02T19:33:47  <wumpus> restarting those jobs
595 2020-07-02T19:33:49  <dongcarl> classic Travis
596 2020-07-02T19:34:01  <wumpus> dongcarl: yes :)
597 2020-07-02T19:34:03  <dongcarl> *90's sitcom noises*
598 2020-07-02T19:34:41  <jeremyrubin> martinus claims that it only does performance counters where available
599 2020-07-02T19:34:44  <wumpus> I'm naive enough to think when ARM tests fails to think it's some real problem with non-x86 architectures
600 2020-07-02T19:34:49  <jeremyrubin> So my guess is it's auto detected
601 2020-07-02T19:34:58  *** AaronvanW has joined #bitcoin-core-dev
602 2020-07-02T19:34:59  <wumpus> anyhow, yes, let's continue review on it
603 2020-07-02T19:35:26  <wumpus> any other topics?
604 2020-07-02T19:37:40  *** Evel-Knievel has quit IRC
605 2020-07-02T19:37:48  * wumpus wonders why #18011 is in mempool improvements
606 2020-07-02T19:37:50  <gribble> https://github.com/bitcoin/bitcoin/issues/18011 | Replace current benchmarking framework with nanobench by martinus · Pull Request #18011 · bitcoin/bitcoin · GitHub
607 2020-07-02T19:38:14  <wumpus> let's add it to high prio for review as well at least
608 2020-07-02T19:38:37  *** Evel-Knievel has joined #bitcoin-core-dev
609 2020-07-02T19:39:25  <dongcarl> Should the "need conceptual review" tag be removed or no?
610 2020-07-02T19:40:13  <wumpus> dongcarl: do you think so?
611 2020-07-02T19:40:40  <dongcarl> I haven't heard oppositions to the concept, just code kinks we might need to work out?
612 2020-07-02T19:41:04  <wumpus> oh, you mean removing it from that particular PR, not in general, sorry, I agree :)
613 2020-07-02T19:41:34  <wumpus> done
614 2020-07-02T19:41:40  <dongcarl> :)
615 2020-07-02T19:42:06  <wumpus> #endmeeting
616 2020-07-02T19:42:06  <lightningbot> Meeting ended Thu Jul  2 19:42:06 2020 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
617 2020-07-02T19:42:06  <lightningbot> Minutes:        http://www.erisian.com.au/meetbot/bitcoin-core-dev/2020/bitcoin-core-dev.2020-07-02-19.00.html
618 2020-07-02T19:42:06  <lightningbot> Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2020/bitcoin-core-dev.2020-07-02-19.00.txt
619 2020-07-02T19:42:06  <lightningbot> Log:            http://www.erisian.com.au/meetbot/bitcoin-core-dev/2020/bitcoin-core-dev.2020-07-02-19.00.log.html
620 2020-07-02T19:42:10  *** shesek has joined #bitcoin-core-dev
621 2020-07-02T19:44:20  <jnewbery> sipa: is the scenario I describe above interesting/relevant? It does seem like there can be observable differences for peers before/after 15644, or maybe I really don't understand what observable equivalence is
622 2020-07-02T19:45:14  <sipa> jnewbery: yeah, i believe there are multiple-peer scenarios where overall behavior is differemt before/after 15644
623 2020-07-02T19:45:47  <sipa> i don't think it changes the discussion though
624 2020-07-02T19:46:28  <sipa> 15644 was an intentional change in behavior, which tries to maintain protocol guarantees as much as possible
625 2020-07-02T19:46:29  *** shesek has quit IRC
626 2020-07-02T19:47:16  <sipa> i don't like introducing p2p behavior for the sake of simplifying code
627 2020-07-02T19:47:20  <sipa> *changes
628 2020-07-02T19:47:44  <sipa> improving privacy, dos resistance, performance, ... are very different justifications
629 2020-07-02T19:47:54  <wumpus> not as only rationale
630 2020-07-02T19:47:58  <jeremyrubin> It's in mempool improvements because I introduced a framework for measuring asymptotics, to help with concerns that I didn't have benchmarks showing the refactors are stronger.
631 2020-07-02T19:48:11  <jeremyrubin> But my asymptote feature was rejected
632 2020-07-02T19:48:26  <jeremyrubin> And martinus introduced a separate new framework which solved my problem
633 2020-07-02T19:48:33  <jeremyrubin> So I dropped my patches in favor of his
634 2020-07-02T19:48:38  <jeremyrubin> And added it to the mempool project
635 2020-07-02T19:48:52  <wumpus> jeremyrubin: makes sense
636 2020-07-02T19:49:19  <jeremyrubin> And this all relates back to fixing a bunch of mempool problems people have, like txn pinning :)
637 2020-07-02T19:49:54  *** Pavlenex has joined #bitcoin-core-dev
638 2020-07-02T19:50:00  *** molz_ has quit IRC
639 2020-07-02T19:50:34  <wumpus> so the improvements in the benchmark framework are mostly motivated from increasing mempool performance, i think that's fair,that's one of the things where performance matters (besides initial sync validation)
640 2020-07-02T19:51:58  <jnewbery> sipa: I'm just struggling to see how 'processing a tx that another peer provided before processing you next message' is a protocol guarantee. It seems more a quirk of the existing implementation
641 2020-07-02T19:52:34  *** nik-j has joined #bitcoin-core-dev
642 2020-07-02T19:52:41  <sipa> jnewbery: i think processing messages in order, except in a few well-defined cases, is a protocol guarantee
643 2020-07-02T19:53:13  <sipa> and with a bunch of more thinking i can probably be convinced that it doesn't impact anything significant
644 2020-07-02T19:53:24  <sipa> but is that worth it, compared to just keeping the code?
645 2020-07-02T19:53:39  <jeremyrubin> jnewbery: btw my bad on ACK counting. Suhas had ACK'd the immediate predecessor to that PR, which still contained a bug, which the follow on had addressed. But he was yet to re-ack the follow on cleanups, which certainly should see more ACKs before merge.
646 2020-07-02T19:54:22  <jnewbery> yes, processing messages _from the same peer_ in order is the guarantee. That's not what's changing here
647 2020-07-02T19:55:16  <sipa> jnewbery: yes it is? there used to be a guarantee that if a peer sends a message, all successor messages are treated having fully processed that tx
648 2020-07-02T19:55:23  <sipa> that's no longer the case
649 2020-07-02T19:55:54  <sipa> perhaps you don't think processing descendants should be treated as part of processing a tx
650 2020-07-02T19:56:00  <jnewbery> there are no guarantees at all about the orphan pool
651 2020-07-02T19:56:09  <jnewbery> sipa: indeed I don't!
652 2020-07-02T19:56:10  <sipa> there used to be!
653 2020-07-02T19:57:02  <jnewbery> the orphan pool is unauthenticated data provided by any peer, that's limited by random eviction. How can there be any guarantees about it?
654 2020-07-02T19:57:22  <jnewbery> *unvalidated
655 2020-07-02T19:57:28  <sipa> that's fair
656 2020-07-02T19:58:12  <sipa> though right now, if i give you two dependent transactions, in a short timeframe, out of order, afterwards you will either have accepted them, or you're never going to
657 2020-07-02T19:58:46  <sipa> this changes that to be delayed to some arbitrary amount of time in the future
658 2020-07-02T19:59:45  <sipa> for no reason, i think
659 2020-07-02T19:59:53  *** mol has joined #bitcoin-core-dev
660 2020-07-02T20:03:24  *** AaronvanW has quit IRC
661 2020-07-02T20:04:20  *** alko has quit IRC
662 2020-07-02T20:05:06  <jnewbery> this seems true, although I can't think of why it'd be important. 'arbitrary amount of time in future' is the order of milliseconds.
663 2020-07-02T20:07:05  <sipa> it could be minutes if there is a block processed in between
664 2020-07-02T20:07:16  <sipa> and you need to load utxos from a slow disk
665 2020-07-02T20:07:36  <sipa> but i agree - which is why i say i may be convinced there is no risk
666 2020-07-02T20:07:50  <sipa> i just don't think it's worth my time or others' to figure this out
667 2020-07-02T20:07:58  <sipa> if the only justification is simpler code
668 2020-07-02T20:10:08  <jnewbery> The marginal benefit to you of simpler code is much smaller than the marginal benefit to other people of simpler code (since you wrote most of the existing code). Perhaps that's why our assessments of this are different
669 2020-07-02T20:10:41  <jnewbery> I've been working on the code for 4 years, and the split between net and net_processing still barely makes sense to me
670 2020-07-02T20:10:56  <sipa> sure
671 2020-07-02T20:11:04  <sipa> i'm all on favor of fixing that split!
672 2020-07-02T20:11:11  <sipa> but what does this have to do with that
673 2020-07-02T20:11:28  <jnewbery> moving orphan processing data from net to net processing
674 2020-07-02T20:11:40  <sipa> great
675 2020-07-02T20:11:53  <sipa> why does that need a global for this queue instead of a per peer structure?
676 2020-07-02T20:12:06  <jnewbery> because per-peer is protected by cs_main
677 2020-07-02T20:12:36  <sipa> and in the future it will be protected by some net_processing lock?
678 2020-07-02T20:12:45  <sipa> is that what is motivating this?
679 2020-07-02T20:14:20  <jnewbery> yes
680 2020-07-02T20:14:38  <jnewbery> removing application-layer data from CNode in net
681 2020-07-02T20:15:15  <sipa> you can move it from CNode to CNodeState, no?
682 2020-07-02T20:15:21  <sipa> without changing it to a global
683 2020-07-02T20:15:43  <sipa> it absolutely does not belong in CNode
684 2020-07-02T20:16:02  <jnewbery> I'd also like orphan tracking to be a separate global module, just like txrelay will be
685 2020-07-02T20:16:26  <sipa> sounds great
686 2020-07-02T20:16:34  <jnewbery> why did you put it in CNode?
687 2020-07-02T20:16:43  <sipa> because it was easier
688 2020-07-02T20:16:51  <sipa> i think
689 2020-07-02T20:17:23  <sipa> but it being a global module doesn't prevent it from having per-peer data
690 2020-07-02T20:19:54  <jnewbery> I can move it to CNodeState, it just seems a shame that a project which aims to reduce cs_main locking adds yet more data under that mutex
691 2020-07-02T20:20:19  <sipa> it's a shame that net_processing's globals are locked by cs_main
692 2020-07-02T20:20:31  <dongcarl> jnewbery: would it make sense in PeerState?
693 2020-07-02T20:20:39  <sipa> there is no solution to that except first moving everything to its place, and then separating the locks
694 2020-07-02T20:20:54  <sipa> i don't think moving things to their correct place is a step backward
695 2020-07-02T20:21:02  *** gzhao408 has quit IRC
696 2020-07-02T20:21:05  <jnewbery> dongcarl: yes, but we also disagree about how to implement PeerState
697 2020-07-02T20:21:08  <sipa> dongcarl: PeerState and CNodeState should eventually be one thing
698 2020-07-02T20:22:03  *** Pavlenex has quit IRC
699 2020-07-02T20:22:12  <dongcarl> What is the disagreement about PeerState?
700 2020-07-02T20:22:23  <sipa> no per-peer locks in net_processing'
701 2020-07-02T20:22:29  <sipa> (in my opinion)
702 2020-07-02T20:22:55  <jnewbery> whether we should have One Big Lock for net processing, or whether each peer should get its own lock. Scrollback to ~12:30
703 2020-07-02T20:23:24  <dongcarl> okay cool, will scroll
704 2020-07-02T20:23:32  <aj> a few big locks was still on the table i thought
705 2020-07-02T20:23:35  <sipa> very short summary: net_processing for almost everything has inherent cross-peer state for coordination; per peer locks don't mae sense if everything will end up grabbing global locks anyway
706 2020-07-02T20:24:04  <sipa> (of my opinion; i don't want to misrepresent anyone else's)
707 2020-07-02T20:24:07  <jnewbery> sipa: that's a short summary of your side!
708 2020-07-02T20:24:29  <sipa> yes, i clarified :)
709 2020-07-02T20:25:08  <sipa> jnewbery: you seem to somehow think that having smaller-scoped locks is inherently better than big ones
710 2020-07-02T20:25:11  <jnewbery> short summary of mine: we have per-peer locking of these concepts already in net. Keeping those locks when you move the data to net processing is the simplest to review change, and would easily allow consolidating them later if that's what's desired.
711 2020-07-02T20:25:34  <sipa> jnewbery: i have no problem with keeping the locks while just moving things
712 2020-07-02T20:25:57  <dongcarl> I don't see a disagreement about the end goal
713 2020-07-02T20:26:04  <jnewbery> sipa: phew, we agree on something :)
714 2020-07-02T20:26:12  <dongcarl> Is it just the intermediate steps that is different?
715 2020-07-02T20:26:17  <sipa> i do have a problem it being used as motivation to introduce semantics changes, which wouldn't be needed for the (in my view) ideal end scenario
716 2020-07-02T20:26:39  <sipa> and i see no problem with just moving orphan_work_set as a per-peer structure, protected by cs_main, to CNodeState
717 2020-07-02T20:26:51  <sipa> and then later fixing the locks, without it ever becoming a global
718 2020-07-02T20:34:57  *** Evel-Knievel has quit IRC
719 2020-07-02T20:35:46  <sipa> i think per-peer data structures for net_processing things are a silly artefact that results from them historically having been stored in CNode, where there was no clear global lock to protect them
720 2020-07-02T20:39:24  <dongcarl> sipa: I'm confused, where would per-peer application-level data go if there were no per-peer data structures in net_processing?
721 2020-07-02T20:39:40  <sipa> dongcarl: per-peer data structures, absolutely!
722 2020-07-02T20:39:44  <sipa> no per-peer *locks*
723 2020-07-02T20:40:01  <sipa> because most things inherently need to access global coordination anyway
724 2020-07-02T20:40:12  *** Evel-Knievel has joined #bitcoin-core-dev
725 2020-07-02T20:41:28  <sipa> for example, g_already_asked_for is a global coordination structure, and there isn't much that can be done to the per-peer m_tx_process_time, m_tx_announced, m_tx_in_flight without also grabbing the lock that protects g_already_asked for... so there is no point in having per-peer locks for those
726 2020-07-02T20:41:58  <sipa> oh i see, i missed "locks" in my sentence above
727 2020-07-02T20:44:19  *** Guyver2 has quit IRC
728 2020-07-02T20:45:02  <dongcarl> Hmm I think I need to read more code. Will grep for those
729 2020-07-02T20:45:16  <jnewbery> dongcarl: they don't exist yet
730 2020-07-02T20:45:28  <sipa> ?
731 2020-07-02T20:45:30  <jnewbery> currently all per-peer data in net processing is protected by cs_main
732 2020-07-02T20:46:06  <jnewbery> there are more granular locks for data in CNode. I'm suggesting those move into PeerState along with the data they're protecting
733 2020-07-02T20:46:28  <sipa> dongcarl: don't familiarize yourself with them too much; they're all going away in #19184
734 2020-07-02T20:46:31  <gribble> https://github.com/bitcoin/bitcoin/issues/19184 | Overhaul transaction request logic by sipa · Pull Request #19184 · bitcoin/bitcoin · GitHub
735 2020-07-02T20:46:31  <jnewbery> sipa is saying that's an unnecessary complication because he'd like to get rid of them
736 2020-07-02T20:46:51  <dongcarl> he'd like to get rid of the granular locks?
737 2020-07-02T20:46:55  <sipa> yes
738 2020-07-02T20:46:56  <jnewbery> yes
739 2020-07-02T20:46:59  <sipa> they're stupid
740 2020-07-02T20:47:10  <dongcarl> Why not do john's first then get rid of the granular locks?
741 2020-07-02T20:47:18  <sipa> that's what i'm suggesting!
742 2020-07-02T20:47:25  <sipa> move them
743 2020-07-02T20:47:36  <sipa> and then get rid of them in a move to net_processing globals
744 2020-07-02T20:48:17  <sipa> but jnewbery seems to think that moving orphan_work_set to net_processing while keeping it under cs_main is sad
745 2020-07-02T20:48:48  <jnewbery> I don't see much benefit in removing the granular locks later, but we can save that for another day
746 2020-07-02T20:48:55  <sipa> :'(
747 2020-07-02T20:49:13  <jnewbery> jnewbery does think adding more data to cs_main is sad!
748 2020-07-02T20:49:56  *** promag has joined #bitcoin-core-dev
749 2020-07-02T20:50:00  <sipa> i don't see why - if it's where it belongs
750 2020-07-02T20:50:16  <sipa> if (nearly) all code accessing that data is already holding cs_main, it's the natural choice
751 2020-07-02T20:50:34  <jnewbery> I'm afraid I've muddied the waters. sipa is right that orphan_work_set can stay per-peer while moving stuff out of net. I also happen to think that moving it out of per-peer is a good change.
752 2020-07-02T20:51:04  <sipa> right, i'd be happy if we can have separate discussions about those two things
753 2020-07-02T20:51:29  <aj> combining code cleanup refactors and behaviour changes seems like a bad idea in general to me
754 2020-07-02T20:52:03  <jnewbery> aj: that's fair
755 2020-07-02T20:52:34  <aj> (but not in specific for any time i do it, of course)
756 2020-07-02T20:52:53  <jnewbery> if you don't like this, you're going to hate #19184. It changes behaviour and makes it a *lot* cleaner
757 2020-07-02T20:52:56  <gribble> https://github.com/bitcoin/bitcoin/issues/19184 | Overhaul transaction request logic by sipa · Pull Request #19184 · bitcoin/bitcoin · GitHub
758 2020-07-02T20:53:32  <aj> depends if the motivation is cleaning up code or changing the behaviour
759 2020-07-02T20:54:34  <aj> (ie, if you're trying to change behaviour, and happen to clean up the code, great. if you're trying to clean up the code, and happen to change the behaviour, horrible)
760 2020-07-02T20:54:37  <dongcarl> sipa: Is there any harm other than overhead if we keep the granular locks while moving things from CNode to PeerState?
761 2020-07-02T20:54:53  *** sipsorcery has quit IRC
762 2020-07-02T20:55:12  <aj> orphan_work_set currently doesn't have a lock of its own, i think?
763 2020-07-02T20:55:14  <sipa> dongcarl: i think it may discourage improvements that require breaking the granular locks; otherwise no
764 2020-07-02T20:55:40  <sipa> aj: all current accesses to it are protected by both cs_main and g_cs_orphans, i believe
765 2020-07-02T20:56:15  <aj> sipa: ProcessMessages checks whether it's empty() before (and after) grabbing those locks
766 2020-07-02T20:56:31  <sipa> good point
767 2020-07-02T20:57:10  <sipa> not a problem, as net_processing's processmessages (and functions called by it) are the only ones accessing it
768 2020-07-02T20:57:18  <sipa> but not a good state of affairs
769 2020-07-02T20:57:21  <jnewbery> it's implictly safe because we can only call ProcessMessages() oncee at a time
770 2020-07-02T20:58:45  <aj> i occassionally think it'd be nice to have dummy per-thread mutexes so we could say GUARDED_BY(ThreadMessageHandlerMutex) for stuff that's only ever called from a particular thread
771 2020-07-02T20:58:56  <sipa> https://github.com/sipa/bitcoin/commit/8bfa52ecefe974349b48603a7e498d20bbe71239 <- moves it to net_processing, protects it with g_cs_orphans
772 2020-07-02T21:00:01  *** corelax1 has quit IRC
773 2020-07-02T21:00:34  <sipa> clang static analyzer is happy
774 2020-07-02T21:01:28  <dongcarl> jnewbery: If we move application-level things from CNode to PeerState, are there cases where the code accessing the "things moved from CNode to PeerState" doesn't already hold cs_main?
775 2020-07-02T21:01:47  <jnewbery> that's one of the motivations
776 2020-07-02T21:01:55  <jnewbery> to reduce the scope of where we need to lock cs_main
777 2020-07-02T21:02:28  <sipa> dongcarl: technically, yes
778 2020-07-02T21:03:40  <sipa> jnewbery: yeah, i understand how you'd want to avoid the increase in cs_main like in the commit i linked above
779 2020-07-02T21:04:04  <sipa> (in this case it could also just remain unprotected, but uh...)
780 2020-07-02T21:05:17  <dongcarl> so wouldn't it be better to have granular locks if these cases of improvements exist? And perhaps in the future, we will have even more cases like these?
781 2020-07-02T21:05:37  <jnewbery> dongcarl: here's one example: the UpdatedBlockTip callback here: https://github.com/bitcoin/bitcoin/blob/7027c67cac852b27c6d71489e4135fabdd624226/src/net_processing.cpp#L1329-L1336 doesn't take cs_main, but access CNode data
782 2020-07-02T21:05:59  <sipa> i think having one giant net_processing lock is much better (and doesn't introduce cs_main locking)
783 2020-07-02T21:06:19  <jnewbery> (nStartingHeight and the fields that PushBlockHash() accesses are in CNode)
784 2020-07-02T21:06:50  <sipa> and also doesn't hurt performance in any way, because net processing is inherently single threaded
785 2020-07-02T21:07:28  <jnewbery> that callback runs on the scheduler thread. Currently it can be run in parallel to anything else happening in net processing. If we add a cs_net_processing, I don't think it can do that any more
786 2020-07-02T21:07:45  <sipa> jnewbery: interesting!
787 2020-07-02T21:10:16  <sipa> jnewbery: though i think you can achieve the same by having a global sub-lock for the relevant data (which inside processmessages would be accessed with cs_net_processing + sublock, and in the callback with just the sublock)
788 2020-07-02T21:10:24  <dongcarl> just for context: I'm interested in this cuz I've been looking at the async PNB stuff, which also introduces a PeerState class, and aims to reduce lock contention with that: https://github.com/bitcoin/bitcoin/pull/16323/commits/3a3b13e1988c319e35f42cd359b8fa36a3a2503d
789 2020-07-02T21:11:56  <jnewbery> I don't think cs_net_processing is necessary if you have sublocks for all the different subsections of data. But yes, there are different ways you can slice this.
790 2020-07-02T21:12:04  <sipa> agree
791 2020-07-02T21:13:17  <sipa> and sublocks for different types of data may permit some limited concurrency too (if they're actually separate...), like having each thread check "which locks does the next message from peer X need? if those are busy, skip to next peer; otherwise, grab and process"
792 2020-07-02T21:14:26  <sipa> but unfortunately that's hard to do in a clean way that accounts for locks needed by dependent modules (e.g. hard to make it skip things that need exclusive access to validation while validation is busy)
793 2020-07-02T21:15:06  <jnewbery> yes, that does seem difficult
794 2020-07-02T21:15:53  <sipa> of course, with cs_main reduced, it may be easier to turn validation into protected with readwrite locks... which would make it less of an issue too
795 2020-07-02T21:20:01  *** nik-j has quit IRC
796 2020-07-02T21:20:36  *** nik-j has joined #bitcoin-core-dev
797 2020-07-02T21:22:01  *** RhodiumToad1 has joined #bitcoin-core-dev
798 2020-07-02T21:22:13  *** nik-j has quit IRC
799 2020-07-02T21:22:27  *** nik-j has joined #bitcoin-core-dev
800 2020-07-02T21:24:01  *** bitcoin-git has joined #bitcoin-core-dev
801 2020-07-02T21:24:02  <bitcoin-git> [bitcoin] meshcollider pushed 5 commits to master: https://github.com/bitcoin/bitcoin/compare/7027c67cac85...a24806c25d7a
802 2020-07-02T21:24:02  <bitcoin-git> bitcoin/master 72f6bec Andrew Chow: rpc: show both UTXOs in decodepsbt
803 2020-07-02T21:24:03  <bitcoin-git> bitcoin/master 5279d8b Andrew Chow: psbt: Allow both non_witness_utxo and witness_utxo
804 2020-07-02T21:24:03  <bitcoin-git> bitcoin/master 4600479 Andrew Chow: psbt: always put a non_witness_utxo and don't remove it
805 2020-07-02T21:24:05  *** bitcoin-git has left #bitcoin-core-dev
806 2020-07-02T21:24:15  *** nik-j has joined #bitcoin-core-dev
807 2020-07-02T21:24:26  *** bitcoin-git has joined #bitcoin-core-dev
808 2020-07-02T21:24:26  <bitcoin-git> [bitcoin] meshcollider merged pull request #19215: psbt: Include and allow both non_witness_utxo and witness_utxo for segwit inputs (master...psbt-segwit-fixes) https://github.com/bitcoin/bitcoin/pull/19215
809 2020-07-02T21:24:27  *** bitcoin-git has left #bitcoin-core-dev
810 2020-07-02T21:27:30  *** marcoagner has quit IRC
811 2020-07-02T21:41:54  *** promag has quit IRC
812 2020-07-02T21:42:26  *** promag has joined #bitcoin-core-dev
813 2020-07-02T21:45:51  *** AaronvanW has joined #bitcoin-core-dev
814 2020-07-02T22:19:12  *** EagleTM has joined #bitcoin-core-dev
815 2020-07-02T22:22:13  *** EagleTM has quit IRC
816 2020-07-02T22:22:39  *** EagleTM has joined #bitcoin-core-dev
817 2020-07-02T22:25:38  *** proofofkeags has quit IRC
818 2020-07-02T22:25:53  *** mdunnio has quit IRC
819 2020-07-02T22:26:13  *** proofofkeags has joined #bitcoin-core-dev
820 2020-07-02T22:26:42  *** proofofkeags has quit IRC
821 2020-07-02T22:26:47  *** justanotheruser has quit IRC
822 2020-07-02T22:26:55  *** proofofkeags has joined #bitcoin-core-dev
823 2020-07-02T22:40:30  *** mdunnio has joined #bitcoin-core-dev
824 2020-07-02T22:40:32  *** mdunnio has quit IRC
825 2020-07-02T22:41:07  *** mdunnio has joined #bitcoin-core-dev
826 2020-07-02T22:41:12  *** EagleTM has quit IRC
827 2020-07-02T22:41:36  *** EagleTM has joined #bitcoin-core-dev
828 2020-07-02T22:44:08  *** justanotheruser has joined #bitcoin-core-dev
829 2020-07-02T22:49:56  *** mdunnio has quit IRC
830 2020-07-02T22:50:39  *** EagleTM has quit IRC
831 2020-07-02T22:51:02  *** EagleTM has joined #bitcoin-core-dev
832 2020-07-02T22:58:17  *** EagleTM has quit IRC
833 2020-07-02T22:58:42  *** EagleTM has joined #bitcoin-core-dev
834 2020-07-02T23:10:27  *** EagleTM has quit IRC
835 2020-07-02T23:10:51  *** EagleTM has joined #bitcoin-core-dev
836 2020-07-02T23:14:15  *** EagleTM has quit IRC
837 2020-07-02T23:14:38  *** EagleTM has joined #bitcoin-core-dev
838 2020-07-02T23:17:41  *** EagleTM has quit IRC
839 2020-07-02T23:18:08  *** EagleTM has joined #bitcoin-core-dev
840 2020-07-02T23:19:59  *** mdunnio has joined #bitcoin-core-dev
841 2020-07-02T23:23:27  *** abd has joined #bitcoin-core-dev
842 2020-07-02T23:24:14  *** mdunnio has quit IRC
843 2020-07-02T23:24:47  *** abd has quit IRC
844 2020-07-02T23:33:43  *** vasild has quit IRC
845 2020-07-02T23:35:44  *** vasild has joined #bitcoin-core-dev
846 2020-07-02T23:39:41  *** EagleTM has quit IRC
847 2020-07-02T23:40:08  *** EagleTM has joined #bitcoin-core-dev
848 2020-07-02T23:43:08  *** EagleTM has quit IRC
849 2020-07-02T23:43:32  *** EagleTM has joined #bitcoin-core-dev
850 2020-07-02T23:52:50  *** EagleTM has quit IRC