1 2017-01-06 00:57:38	0|luke-jr|sipa: I see your point. paveljanik: are you already working on a PR, or shall I?
  2 2017-01-06 00:57:44	0|gmaxwell|16:53 < Michail1> gmaxwell - Mental note:  bitcoin-0.13.2-win64-setup.exe is detected by Norton (File Insight) as not safe and  automatically removes it.  (Not that you can do anything about it, just letting you know.)  I am confirming the prior  versions are not auto deleted.
  3 2017-01-06 01:15:47	0|bitcoin-git|[13bitcoin] 15JeremyRubin opened pull request #9480: [trivial] De-duplicate SignatureCacheHasher (06master...06refactor-signaturecachehasher-visibility) 02https://github.com/bitcoin/bitcoin/pull/9480
  4 2017-01-06 01:25:43	0|bitcoin-git|[13bitcoin] 15kallewoof closed pull request #9478: Trivial refactor: BOOST_FOREACH(a, b) -> for (a : b) (06master...06replace-boostforeach) 02https://github.com/bitcoin/bitcoin/pull/9478
  5 2017-01-06 02:20:17	0|kallewoof|sipa: i have 4 nodes (n1-n4) partitioned into two nets (n12 n34). n2 and n3 both spend the same UTXO, then 6 blocks are generated on each side after which the nets are merged and an additional block is generated on n4 causing the n3 transaction to take precedence, knocking the n1 transaction out. this is sometimes listed in listsinceblock and sometimes not.
  6 2017-01-06 02:21:10	0|kallewoof|sipa: I tried putting a 1 sec delay after the generate to see if there was some wallet fiddling that did not finish in time but this did not change the outcome.
  7 2017-01-06 02:31:05	0|gmaxwell|listed in listsinceblock on which side?
  8 2017-01-06 02:31:19	0|gmaxwell|and which block are you listing since?
  9 2017-01-06 02:34:30	0|kallewoof|I am calling listsinceblock from n1
 10 2017-01-06 02:36:33	0|kallewoof|What's fascinating is that, for the MISSING case (i.e. when it doesn't list the now-orphaned transaction originating from n2), n1 has this in the debug.log file, and for the present case it does not have this:
 11 2017-01-06 02:36:35	0|kallewoof|2017-01-05 11:04:46 Transaction efd9e5d4daf8d47a2cc07db0e153513b2d02da2e031d3f2398f804b2a3d7ba03 (in block 2fd09b11259689722ec38873aeedc7e27753a587f66a542bb2ae64546b17943f) conflicts with wallet transaction 5c717b80ee4e4909e9f4a15bfacd728c3be8c1e75d6eed83a3e9324f6b9ea38c (both spend f72d9d38468c40777640e5b7731dab76cd961ee60cc93198b2ec706c21fb1801:0)
 12 2017-01-06 02:40:55	0|kallewoof|I guess since the transaction was overridden by another one, 'orphaned' is probably not the right term. 'Invalidated'? 'Betrayed'? Anyway, the tx that lost the UTXO race.
 13 2017-01-06 02:42:05	0|kallewoof|A bit verbose but you can compare the two cases here: http://pastebin.com/EqupZuFM (missing) and here: http://pastebin.com/Phy6mN3G (present)
 14 2017-01-06 04:02:42	0|phantomcircuit|kallewoof, conflicted
 15 2017-01-06 05:35:15	0|paveljanik|luke-jr, I'm not (I was sleeping ;-)
 16 2017-01-06 06:32:41	0|gmaxwell|morcos:  par=4 synctime of 24453.715550 (all signatures checked), and par=max (24 actual core host) 12182.296543...  so yea, I suppose it's scaling better than it used to.
 17 2017-01-06 06:33:11	0|gmaxwell|(these figures with dbcache=2000)
 18 2017-01-06 08:29:36	0|jonasschnelli|I'm not very familiar with bloom filters,... for the concept of a block filter committment, I get the point of creating a CBloomFilter for a block containing al inputs and outputs of the containing txs.
 19 2017-01-06 08:29:44	0|jonasschnelli|What I don't see how this would be possible is...
 20 2017-01-06 08:30:18	0|jonasschnelli|how you then could compare a CBloomFilter created with ones wallet pubkeys against the block committment CBloomFilter.
 21 2017-01-06 08:30:36	0|jonasschnelli|I though you have to compare each key from your local wallet against the block-wide committment filter?
 22 2017-01-06 09:48:55	0|bitcoin-git|[13bitcoin] 15jonasschnelli closed pull request #7826: [Qt] show conflicts of unconfirmed transactions in the UI (06master...062016/04/ui_mem_cflct) 02https://github.com/bitcoin/bitcoin/pull/7826
 23 2017-01-06 09:49:15	0|bitcoin-git|[13bitcoin] 15jonasschnelli closed pull request #7817: [Qt] attribute replaceable (RBF) transactions (06master...062016/04/qt_rbf) 02https://github.com/bitcoin/bitcoin/pull/7817
 24 2017-01-06 10:03:12	0|bitcoin-git|[13bitcoin] 15jonasschnelli opened pull request #9481: [Qt] Show more significant warning if we fall back to the default fee (06master...062017/01/fee_warning) 02https://github.com/bitcoin/bitcoin/pull/9481
 25 2017-01-06 10:13:53	0|btcdrak|jonasschnelli: I'm willing to test those this weekend...
 26 2017-01-06 10:14:29	0|jonasschnelli|btcdrak: They need overhaul from my side, don't bother
 27 2017-01-06 10:14:34	0|btcdrak|ok
 28 2017-01-06 10:14:52	0|jonasschnelli|I'll re-base overhaul them as soon as they rise on my pile-of-work
 29 2017-01-06 13:49:20	0|jonasschnelli|Request for review: #9294
 30 2017-01-06 13:49:22	0|gribble|https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub
 31 2017-01-06 13:55:59	0|morcos|gmaxwell: re: CreateTransaction logging, I think thats a great idea.  If needed we could even make fee estimation take a bool to output more debugging information for the times its called via CreateTransaction.  but even without that, just having the basic info would be nice.
 32 2017-01-06 14:23:25	0|jonasschnelli|Idea: would it be stupid to use the first 16 addrs of the dns-seeder DNS response as a "hidden" secp256k1 compact sig for the next 16 addrs of a complete response of 32 addrs?
 33 2017-01-06 14:24:00	0|jonasschnelli|Then ship apps with an ec pubkey per seeder (that supports signed dns responses)
 34 2017-01-06 14:24:24	0|jonasschnelli|I think this approach would be a simple hack and much less work then switching to DNSSEC
 35 2017-01-06 14:26:35	0|sipa|jonasschnelli: i believw intermediate resolvers can reorder/filter responses
 36 2017-01-06 14:29:15	0|gmaxwell|s/can/constantly/
 37 2017-01-06 14:30:04	0|gmaxwell|a lot of intermediate resolvers trim the results to just a few,  and many (most?) reorder them (e.g. under the assumption that the final device will use the first, then yielding a cached response they reorder to avoid overloading the source).
 38 2017-01-06 16:12:11	0|jonasschnelli|We do we try to connect to the dnsseeder on 8333? One-shot getaddr?
 39 2017-01-06 16:14:05	0|sipa|?
 40 2017-01-06 16:14:48	0|sipa|you mean why does the oneshot concept exist?
 41 2017-01-06 16:15:15	0|sipa|if you're connecting over Tor you shouldn't do DNS lookups, as they'd leak your traffic
 42 2017-01-06 16:15:38	0|jonasschnelli|sipa: That was the problem! Dam Qt settings...
 43 2017-01-06 16:16:05	0|jonasschnelli|Now I also know why my SPV block downloads where slower then expected. :)
 44 2017-01-06 16:16:21	0|sipa|so instead we "connect" to the seeder, which in practice means we're connecting to a Tor exit router, and make the router resoove the seeder, and connect tovit
 45 2017-01-06 16:16:39	0|sipa|we don't actually learn what IP we're connecting to in that case
 46 2017-01-06 16:38:11	0|bitcoin-git|[13bitcoin] 15sipa pushed 4 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/f646275b90b1...a55716abe566
 47 2017-01-06 16:38:12	0|bitcoin-git|13bitcoin/06master 1450bd12c 15Gregory Maxwell: Break addnode out from the outbound connection limits....
 48 2017-01-06 16:38:12	0|bitcoin-git|13bitcoin/06master 1490f13e1 15Gregory Maxwell: Add release notes for addnode changes.
 49 2017-01-06 16:38:13	0|bitcoin-git|13bitcoin/06master 14032ba3f 15Gregory Maxwell: RPC help documentation for addnode peerinfo....
 50 2017-01-06 16:38:26	0|bitcoin-git|[13bitcoin] 15sipa closed pull request #9319: Break addnode out from the outbound connection limits. (06master...06addnode_own_count) 02https://github.com/bitcoin/bitcoin/pull/9319
 51 2017-01-06 16:44:58	0|bitcoin-git|[13bitcoin] 15jonasschnelli opened pull request #9483: Complete hybrid full block SPV mode (06master...062017/01/spv) 02https://github.com/bitcoin/bitcoin/pull/9483
 52 2017-01-06 16:45:12	0|midnightmagic|\o/  YAYYY
 53 2017-01-06 17:08:48	0|morcos|cfields: just finished reading through 9441, didn't understand your last comment on the PR, you are going to change it back to what?
 54 2017-01-06 17:09:28	0|sipa|morcos: the current PR makes ProcessMessages only process one message at a timre
 55 2017-01-06 17:09:40	0|morcos|i don't think you need to change it (now) i think its fairly clear any edge case change from current behavior is a net benefit
 56 2017-01-06 17:09:45	0|morcos|sipa: but thats what it did before
 57 2017-01-06 17:10:03	0|sipa|yes
 58 2017-01-06 17:10:41	0|morcos|so you think he should change it to process more than one?
 59 2017-01-06 17:10:53	0|sipa|i'd be more confortable with that
 60 2017-01-06 17:11:12	0|BlueMatt|you'd be more comfortable with a change in behavior?
 61 2017-01-06 17:11:17	0|morcos|that seems like the change to me, possibly a good one, but the PR seems more clearly correct without doing that b/c it doesn't require thinking about that
 62 2017-01-06 17:11:21	0|BlueMatt|I mean I agree we should probably do that in the furture, but why change it now?
 63 2017-01-06 17:11:26	0|morcos|BlueMatt: +!
 64 2017-01-06 17:11:27	0|morcos|1
 65 2017-01-06 17:11:35	0|sipa|i'm confused
 66 2017-01-06 17:11:43	0|sipa|the current code processes multiple messages at once
 67 2017-01-06 17:11:50	0|BlueMatt|the pr does not change the behavior except for some stupid weird edge cases
 68 2017-01-06 17:11:56	0|sipa|his current PR changes that to only process one at a time
 69 2017-01-06 17:12:00	0|BlueMatt|specifically, currently, if you have a message with a bad hash, it will process more than once
 70 2017-01-06 17:12:09	0|morcos|morcos> sipa: but thats what it did before  (meaning master does not process more than one)
 71 2017-01-06 17:12:11	0|BlueMatt|but the current code does NOT process more than one message if it calls ProcessMessage
 72 2017-01-06 17:12:23	0|sipa|morcos: heh?
 73 2017-01-06 17:12:28	0|BlueMatt|(also the pr just disconnects on a bad hash, which I think is a change, but a good one imo)
 74 2017-01-06 17:12:34	0|sipa|master does process more than one, unless it's a block
 75 2017-01-06 17:12:37	0|BlueMatt|no
 76 2017-01-06 17:12:37	0|sipa|or the send buffer is full
 77 2017-01-06 17:12:39	0|BlueMatt|it does not
 78 2017-01-06 17:13:29	0|BlueMatt|I had the same misunderstanding initially
 79 2017-01-06 17:14:11	0|morcos|line 2566 in net_processing.cpp on master i think
 80 2017-01-06 17:14:55	0|sipa|ok, i was not aware of that
 81 2017-01-06 17:15:01	0|sipa|but that seems to be a bug
 82 2017-01-06 17:15:12	0|morcos|:) i think cfields tried to explain it multiple times
 83 2017-01-06 17:15:38	0|morcos|i think we all agree it may be better to process multiple messages, but it seems to me to make more sense to do that as a follow on PR
 84 2017-01-06 17:16:09	0|BlueMatt|(and the overhead of not doing so is (likely) not too terrible)
 85 2017-01-06 17:16:32	0|BlueMatt|(except for the bug fixed by #9315)
 86 2017-01-06 17:16:33	0|gribble|https://github.com/bitcoin/bitcoin/issues/9315 | Request announcement by cmpctblock AFTER requesting cmpctblock/blocktxn by rebroad · Pull Request #9315 · bitcoin/bitcoin · GitHub
 87 2017-01-06 17:18:48	0|morcos|we need to think carefully if there could be negative situations in the other direction..  you're about to announce blocks to all your peers or respond to their getblocktxns messages and some other peer manages to tie you up with a slew of expensive to process received msgs
 88 2017-01-06 17:18:59	0|BlueMatt|heh, fun github bug - if you "start a review" and then "add single comment", it will publish all pending comments
 89 2017-01-06 17:19:21	0|sipa|on a different page?
 90 2017-01-06 17:20:13	0|sipa|ok, so it seems i had completely forgotten about #3180 (> 3 years old)
 91 2017-01-06 17:20:14	0|gribble|https://github.com/bitcoin/bitcoin/issues/3180 | Reduce latency in network processing by pstratem · Pull Request #3180 · bitcoin/bitcoin · GitHub
 92 2017-01-06 17:20:37	0|BlueMatt|you dont /usually/ forget prs from 3 years ago???!
 93 2017-01-06 17:20:47	0|BlueMatt|man, I cant remember prs from 6 months ago
 94 2017-01-06 17:20:53	0|cfields|sipa: heh, i commented on it in about ~5 places :)
 95 2017-01-06 17:20:54	0|morcos|i think clearly we should do SOMETHING smarter, i mean if a block has been announced to you and is sitting in yoru receive queue, seems silly to announce it back just b/c you haven't read it...
 96 2017-01-06 17:21:02	0|cfields|sipa: i thought you just wanted to minimize the diff here
 97 2017-01-06 17:21:46	0|sipa|cfields: you said "in nearly all cases, only one message is processed" - i thought that referred to cases where the buffer was full or we're processing blocks - the pre-3180 behaviour
 98 2017-01-06 17:21:55	0|sipa|and i didn't understand why you'd be changing it
 99 2017-01-06 17:22:26	0|morcos|so do we all agree that cfields should leave 9441 alone, and any further change should be in a separate PR?
100 2017-01-06 17:22:47	0|sipa|yes.
101 2017-01-06 17:22:53	0|cfields|sipa: ah, sorry. the only cases for processing multiple are for bad hash, or bad header
102 2017-01-06 17:23:18	0|sipa|:)
103 2017-01-06 17:23:18	0|sipa|i was trying to ask "why are you changing behaviour" - it would have been clearer if you just had said "it doesn't"
104 2017-01-06 17:23:33	0|morcos|s/only cases/only preexisting cases were/
105 2017-01-06 17:23:36	0|jonasschnelli|luke-jr: I first was using the term "non-validation mode". But than – after reading satoshis whitepaper again – considered to use the name "Simple Payment Verification".
106 2017-01-06 17:23:39	0|BlueMatt|fucking engineers - always precise.....
107 2017-01-06 17:23:45	0|sipa|sorry, i was probably misreading what you said
108 2017-01-06 17:24:03	0|morcos|or something.. nm
109 2017-01-06 17:24:18	0|cfields|sipa: it's mentioned in a bunch of places and at one point you said "I certainly noticed it only processed one message at a time", so i thought we were on the same page
110 2017-01-06 17:24:27	0|cfields|no worries though, sounds like we're all good now
111 2017-01-06 17:24:45	0|morcos|cfields: i'll review again after you fix outstanding comments
112 2017-01-06 17:24:55	0|sipa|cfields: i noticed that your PR changed it to only processing one message at a time
113 2017-01-06 17:25:07	0|sipa|i didn't realize that that was not a change
114 2017-01-06 17:25:18	0|cfields|sipa: ah, heh. i misread you too then :)
115 2017-01-06 17:25:46	0|morcos|i'm not sure i 100% understand the wait_until condition,  is the idea that you don't want spurious wakeups to cause another loop?
116 2017-01-06 17:25:58	0|cfields|ok. I rebased this morning to address all nits and keep the loop in. Will back that out and push.
117 2017-01-06 17:26:02	0|sipa|anyway - misunderstandings in both directions. i should have read the code, instead of making (apparently 3-year old) assumptions about the code
118 2017-01-06 17:26:32	0|cfields|sipa: sure, no worries. It's not obvious behavior _at all_.
119 2017-01-06 17:27:05	0|cfields|morcos: the condition lets us wake the processor from the message handler thread when a new message comes in
120 2017-01-06 17:27:12	0|cfields|and yea, avoids spurious wakes too
121 2017-01-06 17:38:09	0|bitcoin-git|[13bitcoin] 15sipa pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/a55716abe566...46b249e578e8
122 2017-01-06 17:38:10	0|bitcoin-git|13bitcoin/06master 14325e400 15Jonas Schnelli: [Qt] Do proper shutdown
123 2017-01-06 17:38:10	0|bitcoin-git|13bitcoin/06master 149479f8d 15Jonas Schnelli: Allow shutdown during LoadMempool, dump only when necessary
124 2017-01-06 17:38:11	0|bitcoin-git|13bitcoin/06master 1446b249e 15Pieter Wuille: Merge #9408: Allow shutdown during LoadMempool, dump only when necessary...
125 2017-01-06 17:38:19	0|bitcoin-git|[13bitcoin] 15sipa closed pull request #9408: Allow shutdown during LoadMempool, dump only when necessary (06master...062016/12/memp_dump) 02https://github.com/bitcoin/bitcoin/pull/9408
126 2017-01-06 17:47:43	0|morcos|cfields: ah yes, ok good..  cool.  i like the new design...
127 2017-01-06 17:49:00	0|cfields|morcos: great, thanks.
128 2017-01-06 17:53:39	0|luke-jr|BlueMatt: how's this look now?
129 2017-01-06 17:57:26	0|cfields|morcos: updated. Very little changed from before, mostly just fixed some things in the interim commits to be more correct for easier review
130 2017-01-06 18:40:29	0|brg444|can someone confirm how many iterations of segwit testnet there were?
131 2017-01-06 18:41:22	0|sipa|4, i believe
132 2017-01-06 18:42:56	0|brg444|thanks
133 2017-01-06 19:19:00	0|morcos|cfields: one more question on that wait_until..  lets say a block message comes in, takes you a while to process..  if any peers sent you a new message in the interim, you won't wait b/c of fMsgProcWake correct?
134 2017-01-06 19:19:58	0|morcos|But if no peers sent you a message, you will announce quickly to peers N+1 -> MAX_CONNECTIONS, but then sleep up to 100ms before announcing to peers 1 -> N-1 ?
135 2017-01-06 19:20:53	0|gmaxwell|sipa: Processmessage _currently_ processes one at a time, there is a break stuck in the bottom of the loop.
136 2017-01-06 19:21:51	0|morcos|I thought I had seen somewhere talk about making the wait_until time be 100ms from start of loop and not end, or perhaps we should add a WakeMessageHandler for new tips in particular?
137 2017-01-06 19:23:04	0|gmaxwell|morcos: I opened a PR that did those things, and also explicitly asked about the fact that it changed the message handling back to process multiple messages. After no one replied for a bit I just changed it back to one message at a time.
138 2017-01-06 19:23:14	0|cfields|morcos: yes, that would probably make sense. I think gmaxwell's change included that
139 2017-01-06 19:23:14	0|gmaxwell|Then after cfields said he preferred his net refactors I closed it.
140 2017-01-06 19:23:48	0|gmaxwell|it didn't really make that much of a difference when the other problems were fixed.
141 2017-01-06 19:24:34	0|morcos|gmaxwell: I like the changes in 9441 and i think its good to be making progress as part of a larger roadmap..  i think it captures most of the improtant improvements..
142 2017-01-06 19:25:19	0|morcos|perhaps we can add quick relay of new tips,  and processing multiple messages requires a bit more thought in my view..  as we can see by the fact that it was ever taken out in the first place
143 2017-01-06 19:25:39	0|morcos|that said, i admit i did not look at your PRs
144 2017-01-06 19:25:54	0|morcos|hard to keep up!
145 2017-01-06 19:26:19	0|cfields|It's certainly reasonable to tweak it now that the dependencies are untangled, I just tried to keep the scope narrow at first
146 2017-01-06 19:26:30	0|gmaxwell|I know, that why I closed them rather than have more things to look at!
147 2017-01-06 19:26:58	0|gmaxwell|not gonna stop me from going 'I already pointed this out, if you only listened!' :P
148 2017-01-06 19:27:23	0|morcos|insufficient emoticons
149 2017-01-06 19:27:37	0|sipa|gmaxwell: i finally understand your comment on the PR
150 2017-01-06 19:27:55	0|sipa|gmaxwell: i thought you were trying to say that that PR changed it to only process one at a time (which i noticed)... and i was very confused by what you said afterwards about fixing it
151 2017-01-06 19:30:03	0|gmaxwell|sipa: it changed it to process multiple at a time initially, and I immediately added a line comment on that change asking for review of that (which github seems to have lost when I force pushed a change that reverted back to the old behavior)
152 2017-01-06 19:30:04	0|cfields|morcos: hmm, wait. Are you talking about the new quick-relay from 9375 in particular?
153 2017-01-06 19:30:16	0|morcos|cfields: no i'm talking about exactly not that
154 2017-01-06 19:30:21	0|sipa|seems i was just assuming i knew what master did, and i misinterpreted everything that people tried to explain it to me
155 2017-01-06 19:30:41	0|cfields|morcos: ok, good
156 2017-01-06 19:31:38	0|gmaxwell|sipa: I think it should probably handle multiple at a time subject to some time limit. ::shrugs::
157 2017-01-06 19:31:44	0|morcos|cfields: its very easy to see the behavior with 9375 now though b/c of the cached blocks making the annoucements so fast.  you watch annoucements in quick succession up to some high numbered peer and then a pause before announcements start to low numbered peers
158 2017-01-06 19:32:41	0|sipa|gmaxwell: absolutely agree
159 2017-01-06 19:33:08	0|sipa|but if we've done it this way since 0.10, i really don't care whether that happens in 0.14 or not
160 2017-01-06 19:33:44	0|morcos|sipa: gmaxwell: i think ideally we might do something even smarter, such as queue received transactions to be ATMP'ed after we'd gotten through block relay..  however its a bit tricky in that you dont' want to reorder those after any potential other cmpctblock messages for reconstruction purposes
161 2017-01-06 19:33:53	0|cfields|morcos: ah, i think that'd be a different culprit then
162 2017-01-06 19:34:10	0|gmaxwell|sipa:  in other things people probably didn't notice,  I've been complaining that our socket recieve buffer (5 MB) is _way_ too small given how long executions of the message handler take; and it is adversely impacting performance; even with cfields' PR.
163 2017-01-06 19:34:26	0|morcos|cfields: wait, why a different culprit?  didn't the old code have the same problem in a different way?
164 2017-01-06 19:35:27	0|cfields|morcos: to be clear, you're saying that you observe that behavior with the cached blocks?
165 2017-01-06 19:36:05	0|morcos|cfields: :) yes, but not the pre-announced cached blocks.  we use the cached blocks for regular announcment too.
166 2017-01-06 19:36:11	0|gmaxwell|sipa: e.g. right now we can, during a single handler run, connect 999 blocks which will take 2000 seconds even on a moderately fast computer... all the rx buffers just fill. (and on a really slow computer, we'll ping timeout our peers while connecting a wad of blocks)
167 2017-01-06 19:36:55	0|cfields|gmaxwell: https://github.com/bitcoin/bitcoin/pull/9441#issuecomment-270397280 . I think we should consider changing this for 0.14 as well.
168 2017-01-06 19:39:23	0|cfields|morcos: aha, i see. I'm with you now.
169 2017-01-06 19:39:49	0|morcos|they just make the rest of the loop so fast that the pause is more visible
170 2017-01-06 19:43:55	0|gmaxwell|morcos: my strategy was basically to make the loop run immediately again after anything interestin happened.  I think it's harmless to always execute the loop an extra time.
171 2017-01-06 19:47:08	0|morcos|gmaxwell: how would you define interesting?  any message processed?
172 2017-01-06 19:48:29	0|gmaxwell|morcos: yes, though my PR didn't bother catching any message processed, it did catch any message sent or any block recieved, I believe.
173 2017-01-06 19:49:29	0|gmaxwell|I think it would be fine to make it do any message sent or recieved.
174 2017-01-06 19:51:26	0|gmaxwell|I also made it run through the loop an extra time in any case it skipped something due to lock contention.
175 2017-01-06 19:51:37	0|sipa|gmaxwell: i'm aware... but i think it's just fundamentally broken that we're pegging the message handler thread for 2000s
176 2017-01-06 19:51:58	0|sipa|gmaxwell: it's not too hard to not always connect everything we have
177 2017-01-06 19:52:29	0|gmaxwell|sipa: That would be good, the change to accomplish that wasn't obvious to me or I would have PRed it already; though that is necessary it's not sufficient.
178 2017-01-06 19:53:20	0|gmaxwell|sipa: since even a _1_ second delay coupled with a 5MB buffer is enough to limit our IBD sync speed to far below what we can reindex-chainstate at on reasonable hardware.
179 2017-01-06 19:56:20	0|gmaxwell|morcos: the wake on lock contention seemed important to me, otherwise we could get a message from a peer, bounce off cs_main before getting to handle it, then sleep for 100ms before trying again.
180 2017-01-06 19:57:41	0|gmaxwell|(e.g. if we're competing with rpc for cs_main)
181 2017-01-06 19:59:28	0|morcos|gmaxwell: hmm.. that makes sense.  where are you referring to where we skip handling the message if we can't get cs_main?
182 2017-01-06 20:02:07	0|gmaxwell|morcos: ah, I think one of the just merged networking changes just removed where we did that.
183 2017-01-06 20:02:08	0|morcos|oh in SendMessages
184 2017-01-06 20:02:22	0|gmaxwell|oh no, there it is.
185 2017-01-06 20:05:05	0|morcos|gmaxwell: on another note, i guess our ability to make use of >4 cores isn't as good as I thought...  I had a whole series of PR's that removed successive bottlenecks, but i'd misremembered how much progress we could make wiht only the cuckoocache
186 2017-01-06 20:05:45	0|morcos|And then I got sidetracked by the near conesnsus bug with CCoinsViewCache..  but when I get a chance I'll go back and see if there are other parts of that that are still worth doing now
187 2017-01-06 20:05:49	0|gmaxwell|yea...
188 2017-01-06 20:07:26	0|BlueMatt|ugh, ffs, fibre cuts in india means fibre rtt between eu and asia is up 90ms on one path :(
189 2017-01-06 20:07:59	0|BlueMatt|luke-jr: did you fix https://github.com/bitcoin/bitcoin/pull/8775#discussion_r94985339 ?
190 2017-01-06 20:08:01	0|sipa|that try_lock cs_main can go away after #9419 (+ follow ups), i think, as it will be replaced by the nodestate locks
191 2017-01-06 20:08:04	0|gribble|https://github.com/bitcoin/bitcoin/issues/9419 | Stop Using cs_main for CNodeState/State() by TheBlueMatt · Pull Request #9419 · bitcoin/bitcoin · GitHub
192 2017-01-06 20:10:09	0|luke-jr|BlueMatt: no, where is it assumed to return non-NULL?
193 2017-01-06 20:10:28	0|gmaxwell|there is some advantage in skipping processing peers that need a lock, when other peers can be processed without it.
194 2017-01-06 20:10:49	0|BlueMatt|luke-jr: ok, please add documentation to note that it is assumed it can return null
195 2017-01-06 20:12:18	0|gmaxwell|though on the other hand, I think that that construction could leave us sending pings even if cs_main is deadlocked which would be undesirable. (not an actual issue because our message handler will get stuck elsewhere in the case, but still)
196 2017-01-06 20:12:31	0|luke-jr|I would think that's implied in returning a pointer type, but ok
197 2017-01-06 20:15:23	0|BlueMatt|luke-jr: could you not rebase when people are in the middle of review?
198 2017-01-06 20:18:23	0|luke-jr|haven't rebased that PR in a week, but ok
199 2017-01-06 20:18:49	0|BlueMatt|did you just rebase again?
200 2017-01-06 20:18:56	0|luke-jr|no, just added the comment
201 2017-01-06 20:19:06	0|BlueMatt|no? the previous head is now gone?
202 2017-01-06 20:19:37	0|luke-jr|indeed, I amended it
203 2017-01-06 20:19:47	0|BlueMatt|please do not do that
204 2017-01-06 20:25:41	0|BlueMatt|does anyone ask for regular squash/rebase mid-review?
205 2017-01-06 20:28:38	0|luke-jr|I don't usually distinguish (or have a way to) when others are actively reviewing. (note there was already amended commits before I became aware you were)
206 2017-01-06 20:29:11	0|gmaxwell|at what point is someone not reviewing?
207 2017-01-06 20:30:19	0|luke-jr|gmaxwell: exactly, hence why some desiring squashing vs others not liking squashing [at particular times] is confusing to resolve
208 2017-01-06 20:30:23	0|BlueMatt|when it is time to merge and/or there are limited comments showing up?
209 2017-01-06 20:30:52	0|BlueMatt|no one desires squashing unless its been a while since things have been commented on, though to be fair, you should, at a minimum, comment when you squash and note changes
210 2017-01-06 20:31:08	0|BlueMatt|eg respond to the previous comments and note where you did/didnt make changes, instead of letting them sit
211 2017-01-06 20:32:04	0|sipa|i usually prefer squashing trivial fixes immediately, unless it's a very complicated PR
212 2017-01-06 20:32:24	0|sipa|most of the time is understanding what the PR is trying to achieve and how... reading the code is easy
213 2017-01-06 20:33:08	0|gmaxwell|matt's style can be helpful on more complicated ones, but as someone who has often come to a PR to review later, I've found the style to really suck in that case.  I waste my time finding bugs that are already fixed in later updates.
214 2017-01-06 20:33:09	0|BlueMatt|not for a major refactor pr where most of the pr is just code movement
215 2017-01-06 20:33:20	0|sipa|gmaxwell: likewise
216 2017-01-06 20:33:42	0|sipa|(but i don't mind following either style... just my personal preference is usually fixing things immediately)
217 2017-01-06 20:33:43	0|BlueMatt|gmaxwell: I try to title such commits f "Commit title this should be squashed into" fix XYZ
218 2017-01-06 20:33:50	0|BlueMatt|which should at least make it easy to glance at such changes
219 2017-01-06 20:34:08	0|sipa|i guess things have improved with the "review" thing
220 2017-01-06 20:34:21	0|gmaxwell|also it leaves me having to re-review the squash since sometimes a pair of reasonable looking changes my reveal their brokenness once merged. (also we have no tools to tell if a squash was faithful in any case, so someone malicious could slip something in if people didn't review the squash just as carefully)
221 2017-01-06 20:34:39	0|sipa|so you can comment on an issue, and then later delete if you see it's already fixed ahead of time
222 2017-01-06 20:35:14	0|BlueMatt|yes, my bigger annoyance is having to re-review after squash was shit
223 2017-01-06 20:35:31	0|BlueMatt|its easy if its clear and the commits are just being cleanly squashed (can compare treeish)
224 2017-01-06 20:35:41	0|luke-jr|I have an alias that compares diffs that I've gotten used to using to see what changed in an amend
225 2017-01-06 21:10:47	0|owowo|is there a time when core drops a tx if unconfirmed for a long time? Or will it just keep on rebroadcasting it until the return of the Messiah?
226 2017-01-06 21:11:12	0|BlueMatt|the wallet will keep rebroadcasting, but you can "abandon" a transaction both in the gui and the rpc
227 2017-01-06 21:11:34	0|BlueMatt|the mempool will eventually drop them, but there are "helpful" nodes which like to rebroadcast lots of shit to their peers all the time
228 2017-01-06 21:12:19	0|owowo|ok, thx
229 2017-01-06 23:18:56	0|bitcoin-git|[13bitcoin] 15gmaxwell opened pull request #9484: Introduce assumevalid setting to skip validation presumed valid scripts. (06master...06script_elide_verified) 02https://github.com/bitcoin/bitcoin/pull/9484
230 2017-01-06 23:29:09	0|bitcoin-git|[13bitcoin] 15mcelrath opened pull request #9485: ZMQ example using python3 and asyncio (06master...06python3zmq) 02https://github.com/bitcoin/bitcoin/pull/9485