1 2017-11-08 03:42:03	0|wumpus|BlueMatt: I'm all for adding different formats but please keep csv output support
  2 2017-11-08 03:42:16	0|wumpus|it's easy to parse for my own tooling, I don't eally like web tools
  3 2017-11-08 03:42:31	0|BlueMatt|wumpus: csv is still there and still default
  4 2017-11-08 03:42:48	0|BlueMatt|(and not my pr)
  5 2017-11-08 03:44:10	0|wumpus|relating the bench framework, #11562 seems ready for merge at least
  6 2017-11-08 03:44:12	0|gribble|https://github.com/bitcoin/bitcoin/issues/11562 | bench: use std::chrono rather than gettimeofday by theuni · Pull Request #11562 · bitcoin/bitcoin · GitHub
  7 2017-11-08 04:33:04	0|achow101|For some reason, all of my builds locally continuously fail p2p-fullblocktest.py (it's been happening for a while now, I've just been ignoring it). any ideas why?
  8 2017-11-08 04:33:19	0|achow101|it just times out
  9 2017-11-08 04:33:51	0|achow101|and it's unrelated to anything that I am working on, happens on master and checked out PRs that are passing Travis
 10 2017-11-08 04:35:20	0|wumpus|well it used to be that such issues were related to the cache, but I think that's properly cleared now
 11 2017-11-08 04:35:36	0|wumpus|(e.g. sticky issues that seem to happen only in one environment)
 12 2017-11-08 04:36:50	0|gmaxwell|how do timeouts even work if not running in travis?
 13 2017-11-08 04:38:31	0|wumpus|there are some timeouts during the test as well, say if it's waiting for two nodes to be synced and it takes too long, or when an RPC command takes too long
 14 2017-11-08 04:39:01	0|gmaxwell|makes sense.
 15 2017-11-08 04:39:12	0|gmaxwell|achow101: sounds concerning.
 16 2017-11-08 04:39:29	0|wumpus|these should be really high but just make sure it finishes some day even if there's some bug preventing forward progress
 17 2017-11-08 04:39:54	0|wumpus|in some cases they're set too tight though and they can trigger if a system is just at high load
 18 2017-11-08 04:40:05	0|wumpus|(then again that tends to happen on travis not locally)
 19 2017-11-08 04:40:39	0|achow101|It's a consistent test failure, happens on test 98 of p2p-fullblocktest.py and it is a timeout
 20 2017-11-08 04:40:54	0|achow101|I think it's literally the same line every single time, but I haven't actually bothered to investigate
 21 2017-11-08 04:41:27	0|wumpus|they're among the most annoying failures to debug, at least with wrong output there's something clear to investigate
 22 2017-11-08 04:41:49	0|gmaxwell|Which test is "98"?
 23 2017-11-08 04:42:39	0|gmaxwell|I don't see a number 98 in the file, so... (sorry, I'm still kind of embarassingly ignorant about a lot of the test framework)
 24 2017-11-08 04:42:50	0|achow101|gmaxwell: I dunno. It just says "Test 98: PASS" and then "(ERROR): Assertion failed"
 25 2017-11-08 04:42:58	0|achow101|ehh, I guess it's failing on test 99 then..
 26 2017-11-08 04:44:52	0|wumpus|any specifics on the assertion that fails?
 27 2017-11-08 04:46:22	0|achow101|https://0bin.net/paste/tsfO8D4nhJB+dN6i#PoyRukFXKiHKLFR-t8dxloT74Ys0SdaK6bhhBm9lVUR <-- traceback
 28 2017-11-08 04:50:32	0|achow101|gmaxwell: Test 99 is the thing that the 99th yield of get_tests() in p2p-fullblocktest.py "returns"
 29 2017-11-08 04:50:59	0|gmaxwell|If I understand this correctly, it's waiting for a ping response.
 30 2017-11-08 04:51:24	0|wumpus|oh the assertion is that a timeout is exceeded, yes that seems to be it gmaxwell
 31 2017-11-08 04:53:04	0|gmaxwell|it's really hard for me to tell from this traceback what the test conditions were where it croaked out.
 32 2017-11-08 04:54:42	0|achow101|gmaxwell: it should be whatever accepted() at this line is: https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p-fullblocktest.py#L1282
 33 2017-11-08 04:54:48	0|wumpus|there's a command line argument jump into the python debugger on a failure, so the conditions can be investigated
 34 2017-11-08 04:54:57	0|achow101|which is something that this loop https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/comptool.py#L299 does something with
 35 2017-11-08 05:10:18	0|fanquake|achow101 I have occasionally seen the same. Only that test.
 36 2017-11-08 07:15:40	0|wumpus|is this one of the extended tests?
 37 2017-11-08 07:29:34	0|wumpus|no it's not - strange, so I'm running it all the time and not triggering that FWIW
 38 2017-11-08 07:47:18	0|bitcoin-git|13bitcoin/06master 145ce7cb9 15Thomas Snider: [net] De-duplicate connection eviction logic
 39 2017-11-08 07:47:18	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/5776582b7f3e...5ef3b6967b5c
 40 2017-11-08 07:47:19	0|bitcoin-git|13bitcoin/06master 145ef3b69 15Wladimir J. van der Laan: Merge #11524: [net] De-duplicate connection eviction logic...
 41 2017-11-08 09:34:11	0|bitcoin-git|[13bitcoin] 15practicalswift opened pull request #11634: wallet: Add missing cs_wallet/cs_KeyStore locks to wallet (06master...06missing-wallet-locks) 02https://github.com/bitcoin/bitcoin/pull/11634
 42 2017-11-08 13:14:42	0|instagibbs|achow101, same issue for months locally
 43 2017-11-08 13:15:12	0|instagibbs|"glad" im not the only one
 44 2017-11-08 13:21:36	0|morcos|can you guys open an issue or something for these test failures.  they should not be failing.  they can be annoying to debug, but i feel like we are slowly improving them so lets not become immune to failures
 45 2017-11-08 13:35:16	0|jnewbery|#11468 should make comp test framework tests more debuggable (and we should remove them entirely at some point)
 46 2017-11-08 13:35:18	0|gribble|https://github.com/bitcoin/bitcoin/issues/11468 | [tests] Make comp test framework more debuggable by jnewbery · Pull Request #11468 · bitcoin/bitcoin · GitHub
 47 2017-11-08 13:37:29	0|luke-jr|I noticed timeouts give a useless assertion error with regard to times.. I wonder what it would take to have the message describe the actual condition
 48 2017-11-08 13:38:49	0|luke-jr|(otoh, the traceback typically includes that, so not a huge deal)
 49 2017-11-08 13:44:02	0|jnewbery|luke-jr: yeah, it's buried in there, but assert message could def be improved
 50 2017-11-08 13:45:18	0|luke-jr|just not sure how to do that kind of thing with Python. I guess catch it and analyze the traceback object to get info to re-throw with?
 51 2017-11-08 14:03:01	0|jnewbery|correct. That's possible with traceback: https://docs.python.org/3/library/traceback.html
 52 2017-11-08 15:17:35	0|MarcoFalke|re: test failure. Agree that an issue should be opened in case the fix is not trivial.
 53 2017-11-08 15:18:19	0|MarcoFalke|Also, the command line switch to jump into pdb only works on shutdown/teardown of nodes
 54 2017-11-08 15:18:37	0|MarcoFalke|Currently not possible where the assertion hits, iirc
 55 2017-11-08 16:24:50	0|BlueMatt|luke-jr: care to close #10593 or at least provide some justification? there were objections about it in principal and you didn't materially address them or explain what the pr was even trying to do.....
 56 2017-11-08 16:24:52	0|gribble|https://github.com/bitcoin/bitcoin/issues/10593 | Relax punishment for peers relaying invalid blocks and headers by luke-jr · Pull Request #10593 · bitcoin/bitcoin · GitHub
 57 2017-11-08 17:05:26	0|luke-jr|BlueMatt: the ONLY objection I see there is Suhas assuming it reverts 8305, which isn't "in principle" and isn't even true..
 58 2017-11-08 17:07:02	0|luke-jr|elaborated a bit on the OP description
 59 2017-11-08 18:07:05	0|karelb|Is 0.15.1 "canceled" now? :)
 60 2017-11-08 18:07:22	0|bitcoin-git|13bitcoin/06master 147536b08 15practicalswift: trivial: Fix typo – alreardy → already
 61 2017-11-08 18:07:22	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/77546a3182e5...0a2f46b0158b
 62 2017-11-08 18:07:23	0|bitcoin-git|13bitcoin/06master 140a2f46b 15MarcoFalke: Merge #11635: trivial: Fix typo – alreardy → already...
 63 2017-11-08 18:09:48	0|meshcollider|karelb: cancelled? Why?
 64 2017-11-08 18:09:52	0|luke-jr|karelb: meh, might as well get bugfixes out
 65 2017-11-08 18:09:55	0|luke-jr|meshcollider: 2X aborted
 66 2017-11-08 18:10:09	0|meshcollider|What really :O yay!
 67 2017-11-08 18:10:17	0|Chris_Stewart_5|... so is segwit support wallet support back on the menu for 0.15.1?? :P
 68 2017-11-08 18:10:32	0|luke-jr|meshcollider: https://lists.linuxfoundation.org/pipermail/bitcoin-segwit2x/2017-November/000685.html
 69 2017-11-08 18:10:56	0|meshcollider|Only just woke up, excellent start to the day \o/
 70 2017-11-08 18:11:08	0|gmaxwell|lol
 71 2017-11-08 18:11:16	0|gmaxwell|there are other fixes in 0.15.1
 72 2017-11-08 18:11:37	0|karelb|yeah I saw the backports list :)
 73 2017-11-08 18:12:05	0|gmaxwell|also, there is no guarentee that B2X won't fork anyways, it might not be completely centeralized.
 74 2017-11-08 18:12:47	0|Chris_Stewart_5|it will be interesting to see how fast the node count drops
 75 2017-11-08 18:12:50	0|luke-jr|gmaxwell: lol
 76 2017-11-08 18:14:19	0|gmaxwell|last three blocks still "NYA" signaling.
 77 2017-11-08 18:14:44	0|sipa|asia is asleep
 78 2017-11-08 18:14:52	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 4 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/0a2f46b0158b...f7388e93d3dd
 79 2017-11-08 18:14:53	0|bitcoin-git|13bitcoin/06master 145e5725c 15John Newbery: [tests] Add p2p connection to TestNode...
 80 2017-11-08 18:14:53	0|bitcoin-git|13bitcoin/06master 14b86c1cd 15John Newbery: [tests] fix TestNode.__getattr__() method
 81 2017-11-08 18:14:54	0|bitcoin-git|13bitcoin/06master 1432ae82f 15John Newbery: [tests] use TestNode p2p connection in tests
 82 2017-11-08 18:15:00	0|sipa|also, no reason to cancel the 0.15.1 release which is already in RC
 83 2017-11-08 18:15:12	0|sipa|though decision is up to wumpus i'd say
 84 2017-11-08 18:15:17	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #11182: [tests] Add P2P interface to TestNode (06master...06test_node_p2p) 02https://github.com/bitcoin/bitcoin/pull/11182
 85 2017-11-08 18:15:19	0|MarcoFalke|Is there be going to be a 15.2?
 86 2017-11-08 18:15:55	0|gmaxwell|I'm still in favor of 0.15.1 though I do think the urgency is reduced a little.
 87 2017-11-08 18:16:05	0|Chris_Stewart_5|MarcoFalke: It was my understanding that is the release for segwit wallet support because of s2x stuff
 88 2017-11-08 18:16:49	0|jnewbery|I reckon the improvements in v0.15.1 are helpful to fortify against any hostile hard fork. Si vis pacem, para bellum.
 89 2017-11-08 18:16:59	0|luke-jr|I thought we moved it to an early 0.16 *shrug*
 90 2017-11-08 18:17:17	0|MarcoFalke|Yeah. 15.1 has other bugfixes in as well. Though, not all that I liked to put in
 91 2017-11-08 18:17:24	0|MarcoFalke|We should get it out ntl
 92 2017-11-08 18:17:40	0|sipa|luke-jr: undecided in last meeting, but i think that's a reasonable thing to do - push 0.16 forward a bit rather than a 0.15.2 in between
 93 2017-11-08 18:18:41	0|gmaxwell|I hope a lot rather than a bit. :)
 94 2017-11-08 18:18:48	0|gmaxwell|that would be pretty awesome.
 95 2017-11-08 18:20:27	0|MarcoFalke|Can move it at most 1 or 2 months ahead. Moving it to december would be way too rushed, imo
 96 2017-11-08 18:22:15	0|aj|is it bringing 0.16 features forward or renaming 0.15.2 as 0.16 and 0.16 as 0.17?
 97 2017-11-08 18:23:02	0|luke-jr|meh, when it's ready it's ready..?
 98 2017-11-08 18:23:05	0|luke-jr|aj: IMO the latter
 99 2017-11-08 18:26:30	0|aj|MarcoFalke: 0.16 timeline has two months for translations, that seems like it puts an absolute minimum? ie, make the decision on friday, open translations this month some time, then no release prior to mid january?
100 2017-11-08 18:26:54	0|MarcoFalke|aj: Segwit wallet review/testing is still needed
101 2017-11-08 18:28:30	0|aj|MarcoFalke: yeah. and needed prior to merging and hence prior to translations?
102 2017-11-08 18:29:27	0|MarcoFalke|ideally
103 2017-11-08 18:41:37	0|bitcoin-git|[13bitcoin] 15MarcoFalke opened pull request #11637: Remove dead service bits code (06master...06Mf1711-p2pDead) 02https://github.com/bitcoin/bitcoin/pull/11637
104 2017-11-08 19:02:40	0|gmaxwell|MarcoFalke: there are thousands of b2x nodes that will go off in space still, and also still make bcash nodes that are off in space.
105 2017-11-08 19:02:55	0|gmaxwell|MarcoFalke: so I don't think removing the service flag thing makes sense.
106 2017-11-08 19:02:59	0|gmaxwell|unless I'm missing something.
107 2017-11-08 19:03:20	0|achow101|gmaxwell: for 0.16, not 0.15.x I think
108 2017-11-08 19:03:29	0|achow101|hopefully they'll be gone by then
109 2017-11-08 19:03:51	0|gmaxwell|they're not gone yet, and if they're not by then we need to add it back. Plus people run master.
110 2017-11-08 19:03:56	0|gmaxwell|(some people)
111 2017-11-08 19:04:02	0|MarcoFalke|Jup, this is meant for 0.16. We can merge it in a couple of weeks. That should be fine, no?
112 2017-11-08 19:04:33	0|gmaxwell|I don't expect it'll make it for 0.16.  There are still hundreds of bcash nodes that use the old magic.
113 2017-11-08 19:04:42	0|gmaxwell|no harm in having a pull req, I suppose.
114 2017-11-08 19:05:32	0|achow101|maybe we should hold off on that until shortly before 0.16 and re-evaluate then
115 2017-11-08 19:07:06	0|cfields|oh, commented on the PR before i saw the discussion here. Agree it's too early for that.
116 2017-11-08 19:36:35	0|achow101|can we remove the "good first issue" tag from the release schedule issue? It apparently confuses the people that I talk to about how they can start contributing to core
117 2017-11-08 19:38:46	0|sdaftuar|luke-jr: regarding #10593 -- as i read the code, a peer serving a header that doesn't connect will be assigned 20 dos points.  is that correct?
118 2017-11-08 19:38:48	0|gribble|https://github.com/bitcoin/bitcoin/issues/10593 | Relax punishment for peers relaying invalid blocks and headers by luke-jr · Pull Request #10593 · bitcoin/bitcoin · GitHub
119 2017-11-08 19:40:39	0|sdaftuar|luke-jr: actually, scratch that (i just looked at it again)
120 2017-11-08 19:40:54	0|sdaftuar|luke-jr: an outbound peer that serves a header that doesn't connect will be disconnected?
121 2017-11-08 19:41:05	0|luke-jr|sdaftuar: right
122 2017-11-08 19:41:39	0|luke-jr|we want our outbound peers to always be on the same chain ideally
123 2017-11-08 19:41:46	0|sdaftuar|so in particular someone mining a block 2 hours ahead can split the network
124 2017-11-08 19:42:35	0|luke-jr|disconnecting outbound peers only should never split the network, am I wrong?
125 2017-11-08 19:44:12	0|sdaftuar|i think, at the least, that being so aggressive is risky
126 2017-11-08 19:45:55	0|luke-jr|it might make sense to be softer on the time rule I guessw
127 2017-11-08 19:46:42	0|luke-jr|not sure that's related to this PR though
128 2017-11-08 19:47:03	0|sdaftuar|the point of that unconnecting headers PR was to avoid (eventual) disconnection in the situation where peers weren't on an incompatible chain
129 2017-11-08 19:47:10	0|sdaftuar|that's why i called your PR a reversion of that logic
130 2017-11-08 19:47:41	0|luke-jr|it only disconnects when the peer *is* on an incompatible chain, though?
131 2017-11-08 19:47:52	0|sdaftuar|i don't think so?  it disconnects if they serve a header that doesn't connect
132 2017-11-08 19:47:57	0|sdaftuar|which could be for a number of reasons
133 2017-11-08 19:48:04	0|sdaftuar|including the intermediate block being just over 2 hrs in the future
134 2017-11-08 19:48:17	0|sdaftuar|or because they sent a header when they shoudl have sent an inv or something
135 2017-11-08 19:48:33	0|sdaftuar|which is a p2p error, not a consensus error
136 2017-11-08 19:49:29	0|luke-jr|not sure disconnecting for p2p errors is a bad thing; are there any cases where it's neither a p2p issue nor a consensus error (which the 2h limit kindof is)?
137 2017-11-08 19:49:36	0|sdaftuar|but the original way i saw this was on testnet, where a peer using headers announcements for blocks (eg via the sendheaders p2p protocol) would announce a block, via header, that didn't connect, because the prior block was on the wrong side of the 2hr rule
138 2017-11-08 19:49:37	0|luke-jr|and don't we disconnect after N such headers anyway already?
139 2017-11-08 19:50:19	0|sdaftuar|we do, but we set N to be big-ish to account for this occasionally happening iirc
140 2017-11-08 19:51:00	0|sdaftuar|the thing that scares me is if an attacker can get us to disconnect an outbound peer
141 2017-11-08 19:51:12	0|sdaftuar|i mean, disconnect an outbound peer for spurious reasons
142 2017-11-08 19:51:21	0|sdaftuar|because that makes us more vulnerable to sybil attack
143 2017-11-08 19:51:29	0|luke-jr|hmm
144 2017-11-08 19:54:09	0|luke-jr|sdaftuar: can you think of any cases besides the time one this might be an issue for?
145 2017-11-08 19:57:59	0|sdaftuar|well i think matt hated the idea that peers couldn't just switch to announcing everything via header, rather than having to send an inv if they weren't sure the header would connect
146 2017-11-08 19:58:11	0|sdaftuar|the idea that we might disconnect a peer for an unconnecting header is an undocumented p2p behavior
147 2017-11-08 19:58:57	0|sdaftuar|so i think it's sort of a question of how the p2p layer ought to work, and what we should tolerate
148 2017-11-08 20:17:23	0|luke-jr|sdaftuar: so long as you know your peers have header X, sending X+1, X+2, etc should be safe so long as they're on the same chain?
149 2017-11-08 20:17:31	0|luke-jr|(and if they're not, you don't want them as a primary peer)
150 2017-11-08 20:18:35	0|sdaftuar|i'm not sure i understand your question, but i think the answer is yes. can you remind me what case you're concerned about addressing?
151 2017-11-08 20:19:22	0|sdaftuar|i think the recently merged PRs for 0.15.1 address all the cases of bad outbound peer that i was able to come up with, with the goal of ensuring that not all of the outbound peers are on bogus chains
152 2017-11-08 20:19:47	0|sdaftuar|(i'm actually working on a document now that explains all the cases and our logic, which i can share when i'm done)
153 2017-11-08 20:33:36	0|luke-jr|sdaftuar: "This is necessary to avoid banning peers that merely run old formerly-full nodes, after a softfork. We disconnect primary peers because we want compatible full nodes for that role, but allow non-full nodes to remain connected to inbound slots so they can sync the correct chain from us."
154 2017-11-08 20:34:34	0|luke-jr|I suppose there's no reason we couldn't leave the counter in too.
155 2017-11-08 20:59:03	0|sdaftuar|luke-jr: ok yes, i agree with that objective (sorry i haven't paged in your PR in a while). i will try to take a look with fresh eyes, i've lost track a little of where we are (i seem to recall some other PRs that try to address this as well)
156 2017-11-08 21:00:24	0|sdaftuar|to rephrase -- i think we have enough protection in place for outbound peers now (to ensure that we're not solely connected to bogus outbound peers).  but we can do more to protect inbound peers from disconnection, eg in the event that they are unaware of a softfork
157 2017-11-08 21:19:40	0|BlueMatt|luke-jr: you might consider rebasing on #11639
158 2017-11-08 21:19:41	0|gribble|https://github.com/bitcoin/bitcoin/issues/11639 | Rewrite the interface between validation and net_processing wrt DoS by TheBlueMatt · Pull Request #11639 · bitcoin/bitcoin · GitHub
159 2017-11-08 21:20:04	0|BlueMatt|then you get nice things like invalid reasons (including SOFT_FORK and BAD_TIME) so you can avoid splitting the network
160 2017-11-08 21:20:16	0|BlueMatt|plus the resulting pr will be much clearer
161 2017-11-08 21:34:59	0|p3scobar|Roger, you in here?
162 2017-11-08 22:24:51	0|luke-jr|BlueMatt: from the description, I wonder if 11639 makes 10593 unnecessary
163 2017-11-08 22:25:05	0|BlueMatt|it might, i still dont fully understand 10593
164 2017-11-08 22:25:13	0|BlueMatt|but 11639 (mostly) doesnt change behavior
165 2017-11-08 22:25:26	0|luke-jr|BlueMatt: the goal of 10593 is essentially to not ban pre-softfork nodes
166 2017-11-08 22:25:55	0|BlueMatt|I doubt its fully fixed by 11639, but I dont know exactly which case you're handling differently there
167 2017-11-08 22:30:52	0|promag|ryanofsky: ping
168 2017-11-08 22:31:51	0|ryanofsky|i'm here
169 2017-11-08 22:32:12	0|promag|does the qt test timeout?
170 2017-11-08 22:32:49	0|promag|if one callback stays pending
171 2017-11-08 22:34:04	0|ryanofsky|it will hang if an expected signal never arrives
172 2017-11-08 22:34:37	0|ryanofsky|existing wallettests also does this, probably there should be a global test timeout
173 2017-11-08 22:35:05	0|promag|ok, something to improve later if relevant
174 2017-11-08 22:38:35	0|boreddanman|Hey all, my name is Dan, and I'm a software engineer & crypto analyst. Figured i'd stop by the dev chat to see what's cookin'
175 2017-11-08 22:39:03	0|BlueMatt|not much, usually, usually pretty quiet unless people are talking about specific things - keeps the backlog short so folks can stay caught up easily
176 2017-11-08 22:40:04	0|boreddanman|gotchya. any other channels i should check out?
177 2017-11-08 22:41:44	0|BlueMatt|#bitcoin is more active, #bitcoin-wizards is occasionally interesting
178 2017-11-08 23:19:33	0|fanquake|morcos see #11632
179 2017-11-08 23:19:34	0|gribble|https://github.com/bitcoin/bitcoin/issues/11632 | p2p-fullblocktest.py fails occasionally · Issue #11632 · bitcoin/bitcoin · GitHub