1 2016-04-26 01:19:37 0|GitHub160|[13bitcoin] 15kazcw opened pull request #7942: lock cs_main for State/Misbehaving/chainActive (06master...06locking) 02https://github.com/bitcoin/bitcoin/pull/7942 2 2016-04-26 01:57:59 0|achow101|zmq notifications don't seem to be working in windows 3 2016-04-26 03:57:08 0|btcdrak|sipa: looks like a typo 4 2016-04-26 04:01:32 0|gmaxwell|kanzure: it's intentional incompetence in bitcoinj... it used to be worse, google up the bitcoin-dev logs for this lovely conversation 5 2016-04-26 04:01:35 0|gmaxwell|09:04 < gmaxwell> BlueMatt: someone was saying in here that bitcoinj used a ping interval of 200ms the other day? is that so? 6 2016-04-26 04:01:38 0|gmaxwell|09:05 < gmaxwell> (200ms * 124 peers = 620 pps = about 400kbit/sec before adding in whatever the ping itself takes) 7 2016-04-26 04:01:52 0|gmaxwell|09:12 < TD> a ping every 200msec is not even going to stress a pocket calculator, but yeah, i'll see what's going on there 8 2016-04-26 04:02:28 0|gmaxwell|... 9 2016-04-26 04:02:29 0|gmaxwell|09:19 < TD> how about making the default 1 second. 44 bytes once a second is trivial compared to the cost of running a full node, even if you have a large number of peers. 10 2016-04-26 04:02:42 0|gmaxwell|09:21 < gmaxwell> TD: Any reason it needs to be that fast? The network radius is many seconds now. I was thinking of conferring DOS when pings come faster than once per second, so actually sending at once per second would be right on the wire. 11 2016-04-26 04:02:55 0|gmaxwell|09:24 < TD> because responsiveness matters, a ton. it's trying to figure out which of the peers it was able to find can shovel it the chain fastest, ie, is not overloaded 12 2016-04-26 04:03:01 0|gmaxwell|09:25 < TD> pings are super cheap. if we go up to even a pathetic level of traffic like 5-6 transactions per second, invs will be far more expensive bandwidth and cpu wise 13 2016-04-26 04:03:30 0|gmaxwell|09:30 < gmaxwell> TD: nothing on the network happens within a one second time frame. The time it takes to get a message across the network is multiple seconds. Any sane peer has multiple connections. 1 second is still on the order of (20+28+12+32)*8*125 = 92kbit/sec for a node with 125 peers. 14 2016-04-26 04:03:35 0|gmaxwell|09:31 < gmaxwell> I do not think this is reasonable in the general case. 15 2016-04-26 04:03:38 0|gmaxwell|09:32 < gmaxwell> esp unlike blocks and such, pings are not reduced by recieving one first from another peer. 16 2016-04-26 04:04:07 0|gmaxwell|09:36 < gmaxwell> TD: it's also as much as we'll spend transmitting the blockchain on average when all our peers are full nodes. 17 2016-04-26 04:04:10 0|gmaxwell|09:36 < gmaxwell> (or at least within a small factor of it) 18 2016-04-26 04:04:46 0|gmaxwell|yadda yadda. 19 2016-04-26 04:17:55 0|btcdrak|maybe andreas would be willing to dial that down a bit. 20 2016-04-26 04:19:35 0|luke-jr|wouldn't have a choice if we release code that bans misbehaving peers <.< 21 2016-04-26 04:19:59 0|luke-jr|but yeah, probably better to try the polite approach first 22 2016-04-26 04:33:13 0|gmaxwell|I wrote code a while back that banned when it got a new ping less than a second after the last pong response, and it immediately banned all bitcoinj peers, which is what started that discussion. 23 2016-04-26 04:34:00 0|gmaxwell|even with bitcoinj backed off, and the banning backed off, it still had false positives due to nodes continuing to send pings when they hadn't had a response, resulting in a backlog... and so whenever the network burped a bunch of pings would come in a burst. 24 2016-04-26 04:34:27 0|gmaxwell|so I think banning can't be done unless we also specify that a peer shall not have multiple pings in flight. 25 2016-04-26 04:42:54 0|cfields|sipa: still around, by chance? 26 2016-04-26 04:43:11 0|gmaxwell|I hope not. 27 2016-04-26 04:43:26 0|cfields|heh, I never know what continent he's on 28 2016-04-26 04:45:15 0|cfields|the switch is flipped travis-side. I'm working on getting everything to turn green so that we can merge the Trusty change. Until then, things will be wonky. I might have to disable qt on a few builds, seems to take longer to build on Trusty. We can re-enable after some experimentation 29 2016-04-26 04:46:13 0|gmaxwell|\O/ 30 2016-04-26 05:21:10 0|sipa|cfields: \o/ 31 2016-04-26 05:24:32 0|cfields|sipa: almost ready, maybe 1 more hour 32 2016-04-26 05:26:04 0|gmaxwell|luke-jr: your tonal time is useless to me since there is no such thing as tonal atomic time. 33 2016-04-26 05:26:25 0|luke-jr|:< 34 2016-04-26 05:31:13 0|cfields|luke-jr: btw, you might want to keep an eye on https://github.com/travis-ci/travis-build/pull/706, since you use a similar hack iirc 35 2016-04-26 05:31:44 0|cfields|(and ignore my stupid first comment, I didn't realize at first that the change is being made specifically for us :p) 36 2016-04-26 05:33:51 0|luke-jr|cfields: I don't suppose you know a way to get GCC to link to a library from a specific directory btw? 37 2016-04-26 05:34:05 0|luke-jr|without knowing the library filenames which vary by platform :/ 38 2016-04-26 05:35:08 0|sipa|specify the .a directly as a source file? 39 2016-04-26 05:36:23 0|luke-jr|sipa: .so, except sometimes it's .dll, and sometimes it has a lib prefix and sometimes it has a -NNN suffix etc 40 2016-04-26 05:37:10 0|luke-jr|oh, and don't forget Cygwin where it's a "cyg" prefix :x 41 2016-04-26 05:37:46 0|cfields|luke-jr: ac_search_libs :( 42 2016-04-26 05:38:00 0|luke-jr|:x 43 2016-04-26 05:38:10 0|luke-jr|hmm 44 2016-04-26 06:04:41 0|luke-jr|cfields: AC_SEARCH_LIBS doesn't seem to have a way to get the filename? 45 2016-04-26 06:06:52 0|cfields|luke-jr: no, not really. But using it with [foo foo-NNN etc] would get you part of the way there, since that will also take care of .so/.dll 46 2016-04-26 06:09:03 0|luke-jr|cfields: it won't tell me the .so/.dll, so not really 47 2016-04-26 06:09:24 0|cfields|luke-jr: you actually need the filename? Or you just need it to link against one of the possibilities? 48 2016-04-26 06:09:43 0|luke-jr|cfields: well, I don't know how to get GCC to do it without the absolute filename 49 2016-04-26 06:10:14 0|luke-jr|problem is it's using -L/usr/local/lib for -lbase58, and the latter -L for libblkmaker is ignored because and older libblkmaker is in /usr/local/lib also 50 2016-04-26 06:10:34 0|luke-jr|I could reverse the order of the lib stuff, but then it'd have the problem in the opposite situation 51 2016-04-26 06:11:35 0|cfields|luke-jr: pkg-config ? 52 2016-04-26 06:11:40 0|luke-jr|cfields: this is with pkg-config 53 2016-04-26 06:12:09 0|luke-jr|-L/usr/local/lib -lbase58 from libbase58.pc, and -L/path/to/other/lib -lblkmaker from libblkmaker.pc 54 2016-04-26 06:14:35 0|cfields|luke-jr: doesn't libblkmaker depend on libbase58? 55 2016-04-26 06:14:47 0|luke-jr|cfields: yep 56 2016-04-26 06:15:10 0|luke-jr|both /usr/local/lib/libblkmaker and /path/to/other/lib/libblkmaker use /usr/local/lib/libbase58 57 2016-04-26 06:16:33 0|cfields|luke-jr: i'm missing something then. It seems like libbase58 should be private in the .pc's, and not added to the linker path since it's an indirect dep 58 2016-04-26 06:16:50 0|cfields|luke-jr: can you point me to what's linking them both in? 59 2016-04-26 06:17:15 0|luke-jr|cfields: BFGMiner also directly depends on both 60 2016-04-26 06:19:40 0|cfields|luke-jr: ah. Not sure what to tell you, then :\ 61 2016-04-26 06:21:38 0|cfields|luke-jr: well i'm pretty sure you don't actually need to link against libblkmaker, since the syms will be resolved recursively at runtime 62 2016-04-26 06:21:48 0|cfields|er sorry, link against libbase58 63 2016-04-26 06:22:35 0|luke-jr|3hmm 64 2016-04-26 06:22:54 0|cfields|but i'm not sure if ld will whine about unresolved symbols, assuming you're handling visibility 65 2016-04-26 06:26:40 0|cfields|sipa: is it possible to reduce the secp256k1 test runs for 32bit? They take several minutes for travis 66 2016-04-26 06:30:11 0|sipa|cfields: i think so 67 2016-04-26 06:37:29 0|cfields|sipa: ah, it takes a count arg :) 68 2016-04-26 06:39:30 0|gmaxwell|cfields: yes, though the count might not really get it low enough, due to imbalances. 69 2016-04-26 06:42:48 0|cfields|imbalances? 70 2016-04-26 06:44:56 0|sipa|gmaxwell: but there is also a really huge count of tests run 71 2016-04-26 06:45:19 0|sipa|cfields: is this for the secp repo, or the tests ran inside the bitcoind repo? 72 2016-04-26 06:45:50 0|cfields|sipa: in the bitcoin repo. They're pretty redundant, I should think 73 2016-04-26 06:47:09 0|sipa|at least there no huge number of different test combinations is tried 74 2016-04-26 06:47:20 0|gmaxwell|cfields: we should not disable tests, since differences in build configuration are meaningful, but their count could be cut down. 75 2016-04-26 06:48:51 0|cfields|gmaxwell: we could make sure the bitcoin configs are covered in the downstream matrix, but yea, I suppose it's best to keep them running in case we get out of sync 76 2016-04-26 06:49:25 0|gmaxwell|cfields: it's not even the same code, so---- 77 2016-04-26 06:50:17 0|cfields|gmaxwell: well i sure hope the code that's coming in via merge points has been tested at least once :) 78 2016-04-26 06:50:36 0|cfields|but sure, point taken 79 2016-04-26 06:50:59 0|sipa|gmaxwell: how do you mean it is not the same code? 80 2016-04-26 06:51:39 0|gmaxwell|sipa: I mean upstream moves ahead. 81 2016-04-26 07:00:27 0|sipa|cfields: how does this change make travis unusually slow? 82 2016-04-26 07:00:37 0|sipa|cfields: before it is merged 83 2016-04-26 07:00:39 0|sipa|? 84 2016-04-26 07:00:52 0|sipa|some cache interaction between old and new PRs? 85 2016-04-26 07:02:36 0|cfields|sipa: our special cache flag has been removed, so I'm afraid of that interaction, yes 86 2016-04-26 07:02:53 0|cfields|sipa: in theory it's supposed to work, but i get the impression this path hasn't been tested on their end 87 2016-04-26 07:03:11 0|sipa|ah, i see 88 2016-04-26 07:03:16 0|cfields|sipa: I'll bump a PR real quick to test 89 2016-04-26 07:46:41 0|GitHub178|[13bitcoin] 15randy-waterhouse opened pull request #7944: Re-instate TARGET_OS=linux in configure.ac. Removed by 351abf9e035. (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/7944 90 2016-04-26 07:47:48 0|randy-waterhouse|wumpus: cfields : https://github.com/bitcoin/bitcoin/pull/7944 91 2016-04-26 07:48:51 0|randy-waterhouse|without this gives a weird runttime qt error (core dump) not linking to xcb plugin correctly (depends build) 92 2016-04-26 10:02:47 0|jonasschnelli|the signal UpdatedBlockTip() is called without holding cs_main, SyncWithWallets() is called while holding cs_main 93 2016-04-26 10:02:52 0|jonasschnelli|I guess the signal listeners do also hold cs_main during the signal callback function? 94 2016-04-26 10:03:55 0|sipa|i think no callbacks should happen while holding a lock 95 2016-04-26 10:04:20 0|jonasschnelli|Yes. Agree. But ConnectTip is under cs_main, right? 96 2016-04-26 10:04:47 0|jonasschnelli|sipa: and connectTip calls SyncWithWallets() 97 2016-04-26 10:04:52 0|sipa|yes, should; not saying they are :) 98 2016-04-26 10:05:56 0|jonasschnelli|I guess a dirty RAII breaking LEAVE_CRITICAL_SECTION() is not the way to go... 99 2016-04-26 10:34:57 0|luke-jr|guess we better use Q_EMIT 100 2016-04-26 10:35:51 0|sipa|how about using dbus? 101 2016-04-26 10:37:02 0|luke-jr|that actually would make sense to add, even if not for internal use :x 102 2016-04-26 10:54:56 0|wumpus|what about kdbus, obviously we should aim to bring it all into the kernel 103 2016-04-26 10:59:04 0|wumpus|btw, Q_EMIT works the same as boost signals if it's used within one thread, the event handler is called synchronously. It becomes more interesting if you use it between thread, or somehow with Qt::QueuedConnection, then it will be called later in that event loop 104 2016-04-26 10:59:13 0|sipa|wumpus: i tried uses the ShowProgress signal for better reindex information, but it seems that using that while the main window is up makes it irresponsive (and a non-colored small window shows up) 105 2016-04-26 10:59:46 0|wumpus|sipa: the problem is that something in the GUI may try to get a cs_main lock in response to the signal 106 2016-04-26 10:59:52 0|sipa|so i guess the reindex progress should be reported by extendid UodateTi 107 2016-04-26 10:59:52 0|wumpus|e.g. inthe GUI thrad 108 2016-04-26 11:00:08 0|wumpus|if your code is hogging that signal, it will only proceed after releasing the lock 109 2016-04-26 11:00:13 0|wumpus|hogging that lock, I mean 110 2016-04-26 11:00:25 0|sipa|why does ShowProgress need a lock? 111 2016-04-26 11:00:30 0|wumpus|I don't know 112 2016-04-26 11:00:38 0|wumpus|it may not actually need it, maybe it's a mistake 113 2016-04-26 11:00:59 0|wumpus|ideally the GUI thread would never aquire cs_main at all, unfortunately we're not there yet 114 2016-04-26 11:01:03 0|sipa|it's only used for the startup check now, i think 115 2016-04-26 11:01:20 0|sipa|maybe it's just not or badly implemented for the main window 116 2016-04-26 11:01:36 0|wumpus|but sometimes the GUI wants to fetch some additional statistics from the core, which are not passed in the signal parameters, which require the lock 117 2016-04-26 11:02:04 0|sipa|right 118 2016-04-26 11:02:05 0|wumpus|I don't know for sure but it is usually the explanation for 'GUI stuck' issues 119 2016-04-26 11:02:24 0|wumpus|some of the additional statistics fetches are actually under try_locks to avoid this 120 2016-04-26 11:02:38 0|wumpus|but sometimes there's something sneaky that aquires a cs_main lock deep in the code 121 2016-04-26 11:02:47 0|jonasschnelli|I'm now removing the cs_main lock from the SyncWithWallets signal 122 2016-04-26 11:02:58 0|wumpus|in any case I could take a look at it later 123 2016-04-26 11:03:12 0|jonasschnelli|And I agree with wumpus: we should not allows cs_main locks from the GUI logic itself. 124 2016-04-26 11:03:12 0|wumpus|just push the code somewhere and tell me how to reproduce it 125 2016-04-26 11:03:31 0|jonasschnelli|Though, the GUI can call something from the core classes that acquires cs_main 126 2016-04-26 11:03:40 0|wumpus|jonasschnelli: that should be the eventual goal; it's pretty hard to do at the moment though, e.g. sending coins and such should happen in an auxiliary thread 127 2016-04-26 11:03:56 0|wumpus|getting data from the core should also happen in an auxiliary thread, and be passed using signals 128 2016-04-26 11:04:12 0|jonasschnelli|Right. We already have something that could work: RPC. :) 129 2016-04-26 11:04:27 0|wumpus|it's not rocket science but not trivial to get entirely right 130 2016-04-26 11:04:37 0|wumpus|yes, you need exactly the same changes if the core were to be remotely 131 2016-04-26 11:04:40 0|wumpus|that's a bonus. 132 2016-04-26 11:04:55 0|jonasschnelli|I think the only change that is worth is to fully process separete the GUI. 133 2016-04-26 11:05:07 0|jonasschnelli|First It could be only the node-GUI (non wallet) 134 2016-04-26 11:05:08 0|wumpus|well it would be a step there 135 2016-04-26 11:05:27 0|wumpus|if all communication to the core hapepns through signals, detaching it is almost trivial 136 2016-04-26 11:05:28 0|jonasschnelli|Right. You need similar/same changes. 137 2016-04-26 11:05:51 0|wumpus|in any case it's been on my todo list for years 138 2016-04-26 11:06:37 0|wumpus|I could focus on it again, but playing around with databases is so much more fun :p 139 2016-04-26 11:07:41 0|wumpus|and for years I believed the GUI would eventually be removed anyway, so didn't feel like spending effort on it 140 2016-04-26 11:08:04 0|wumpus|but you changed things around jonasschnelli :) 141 2016-04-26 11:08:20 0|sipa|i'm sure bitcoin 0.13 will ship with the ability to maintain the utxo set in RAM, in LevelDB, in LMDB, in BDB and in CSV files. 142 2016-04-26 11:08:49 0|wumpus|hahahahah you're on the right track sipa 143 2016-04-26 11:08:59 0|jonasschnelli|hehe... 144 2016-04-26 11:09:09 0|sipa|wumpus: interesting... the Qt GUI was what brought you into bitcoin development in the first place, no? 145 2016-04-26 11:09:30 0|wumpus|sipa: yes, it was 146 2016-04-26 11:09:54 0|sipa|pedrobranco: hi there 147 2016-04-26 11:10:24 0|pedrobranco|hi sipa :) 148 2016-04-26 11:10:32 0|wumpus|I was really optimistic about it in the beginning, but it turned out to be more than I bargained for, I chose qt (with a standard approach) because it is a well-known toolkit in the hope it would attract more developers to it 149 2016-04-26 11:10:37 0|jonasschnelli|I think the Qt GUI is great! Sure, we could use some UX designers. But one step after the other.. 150 2016-04-26 11:11:02 0|wumpus|... but that didn't really happen, and myself I was countinously distracted by other changes that had to be done 151 2016-04-26 11:11:52 0|wumpus|so at some point with all the complaints about the GUI, and with most developers not interested in it anyway, I didn't give it much time before someone would decide to remove it completely 152 2016-04-26 11:12:35 0|pedrobranco|sipa: whats up? 153 2016-04-26 11:12:37 0|sipa|pedrobranco: sorry for all the noise on your importmulti call... i really think we should have such a call 154 2016-04-26 11:12:59 0|MarcoFalke|Interesting, we didn't yet switch to trusty or py3, yet https://github.com/bitcoin/bitcoin/pull/7944 fails due to https://github.com/bitcoin/bitcoin/issues/7893 ... 155 2016-04-26 11:13:57 0|pedrobranco|sipa: no need to sorry, i thank you for the feedback. yes, i think we should too.