< achow101>
zmq notifications don't seem to be working in windows
< btcdrak>
sipa: looks like a typo
< gmaxwell>
kanzure: it's intentional incompetence in bitcoinj... it used to be worse, google up the bitcoin-dev logs for this lovely conversation
< gmaxwell>
09:04 < gmaxwell> BlueMatt: someone was saying in here that bitcoinj used a ping interval of 200ms the other day? is that so?
< gmaxwell>
09:05 < gmaxwell> (200ms * 124 peers = 620 pps = about 400kbit/sec before adding in whatever the ping itself takes)
< 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
< gmaxwell>
...
< 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.
< 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.
< 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
< 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
< 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.
< gmaxwell>
09:31 < gmaxwell> I do not think this is reasonable in the general case.
< gmaxwell>
09:32 < gmaxwell> esp unlike blocks and such, pings are not reduced by recieving one first from another peer.
< 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.
< gmaxwell>
09:36 < gmaxwell> (or at least within a small factor of it)
< gmaxwell>
yadda yadda.
< btcdrak>
maybe andreas would be willing to dial that down a bit.
< luke-jr>
wouldn't have a choice if we release code that bans misbehaving peers <.<
< luke-jr>
but yeah, probably better to try the polite approach first
< 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.
< 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.
< gmaxwell>
so I think banning can't be done unless we also specify that a peer shall not have multiple pings in flight.
< cfields>
sipa: still around, by chance?
< gmaxwell>
I hope not.
< cfields>
heh, I never know what continent he's on
< 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
< gmaxwell>
\O/
< sipa>
cfields: \o/
< * gmaxwell>
looks at the clock
< * luke-jr>
replaces gmaxwell's clock with a tonal one
< cfields>
sipa: almost ready, maybe 1 more hour
< gmaxwell>
luke-jr: your tonal time is useless to me since there is no such thing as tonal atomic time.
< cfields>
(and ignore my stupid first comment, I didn't realize at first that the change is being made specifically for us :p)
< luke-jr>
cfields: I don't suppose you know a way to get GCC to link to a library from a specific directory btw?
< luke-jr>
without knowing the library filenames which vary by platform :/
< sipa>
specify the .a directly as a source file?
< luke-jr>
sipa: .so, except sometimes it's .dll, and sometimes it has a lib prefix and sometimes it has a -NNN suffix etc
< luke-jr>
oh, and don't forget Cygwin where it's a "cyg" prefix :x
< cfields>
luke-jr: ac_search_libs :(
< luke-jr>
:x
< luke-jr>
hmm
< luke-jr>
cfields: AC_SEARCH_LIBS doesn't seem to have a way to get the filename?
< 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
< luke-jr>
cfields: it won't tell me the .so/.dll, so not really
< * luke-jr>
ponders
< cfields>
luke-jr: you actually need the filename? Or you just need it to link against one of the possibilities?
< luke-jr>
cfields: well, I don't know how to get GCC to do it without the absolute filename
< 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
< luke-jr>
I could reverse the order of the lib stuff, but then it'd have the problem in the opposite situation
< cfields>
luke-jr: pkg-config ?
< luke-jr>
cfields: this is with pkg-config
< luke-jr>
-L/usr/local/lib -lbase58 from libbase58.pc, and -L/path/to/other/lib -lblkmaker from libblkmaker.pc
< cfields>
luke-jr: doesn't libblkmaker depend on libbase58?
< luke-jr>
cfields: yep
< luke-jr>
both /usr/local/lib/libblkmaker and /path/to/other/lib/libblkmaker use /usr/local/lib/libbase58
< 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
< cfields>
luke-jr: can you point me to what's linking them both in?
< luke-jr>
cfields: BFGMiner also directly depends on both
< cfields>
luke-jr: ah. Not sure what to tell you, then :\
< * luke-jr>
would have thought libtool and pkg-config solved this by now :\
< 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
< cfields>
er sorry, link against libbase58
< luke-jr>
3hmm
< cfields>
but i'm not sure if ld will whine about unresolved symbols, assuming you're handling visibility
< cfields>
sipa: is it possible to reduce the secp256k1 test runs for 32bit? They take several minutes for travis
< sipa>
cfields: i think so
< cfields>
sipa: ah, it takes a count arg :)
< gmaxwell>
cfields: yes, though the count might not really get it low enough, due to imbalances.
< cfields>
imbalances?
< sipa>
gmaxwell: but there is also a really huge count of tests run
< sipa>
cfields: is this for the secp repo, or the tests ran inside the bitcoind repo?
< cfields>
sipa: in the bitcoin repo. They're pretty redundant, I should think
< sipa>
at least there no huge number of different test combinations is tried
< gmaxwell>
cfields: we should not disable tests, since differences in build configuration are meaningful, but their count could be cut down.
< 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
< gmaxwell>
cfields: it's not even the same code, so----
< cfields>
gmaxwell: well i sure hope the code that's coming in via merge points has been tested at least once :)
< cfields>
but sure, point taken
< sipa>
gmaxwell: how do you mean it is not the same code?
< gmaxwell>
sipa: I mean upstream moves ahead.
< sipa>
cfields: how does this change make travis unusually slow?
< sipa>
cfields: before it is merged
< sipa>
?
< sipa>
some cache interaction between old and new PRs?
< cfields>
sipa: our special cache flag has been removed, so I'm afraid of that interaction, yes
< cfields>
sipa: in theory it's supposed to work, but i get the impression this path hasn't been tested on their end
< sipa>
ah, i see
< cfields>
sipa: I'll bump a PR real quick to test
< GitHub178>
[bitcoin] randy-waterhouse opened pull request #7944: Re-instate TARGET_OS=linux in configure.ac. Removed by 351abf9e035. (master...master) https://github.com/bitcoin/bitcoin/pull/7944
< randy-waterhouse>
without this gives a weird runttime qt error (core dump) not linking to xcb plugin correctly (depends build)
< jonasschnelli>
the signal UpdatedBlockTip() is called without holding cs_main, SyncWithWallets() is called while holding cs_main
< jonasschnelli>
I guess the signal listeners do also hold cs_main during the signal callback function?
< sipa>
i think no callbacks should happen while holding a lock
< jonasschnelli>
Yes. Agree. But ConnectTip is under cs_main, right?
< jonasschnelli>
sipa: and connectTip calls SyncWithWallets()
< sipa>
yes, should; not saying they are :)
< jonasschnelli>
I guess a dirty RAII breaking LEAVE_CRITICAL_SECTION() is not the way to go...
< luke-jr>
guess we better use Q_EMIT
< * luke-jr>
hides
< sipa>
how about using dbus?
< luke-jr>
that actually would make sense to add, even if not for internal use :x
< wumpus>
what about kdbus, obviously we should aim to bring it all into the kernel
< 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
< 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)
< wumpus>
sipa: the problem is that something in the GUI may try to get a cs_main lock in response to the signal
< sipa>
so i guess the reindex progress should be reported by extendid UodateTi
< wumpus>
e.g. inthe GUI thrad
< wumpus>
if your code is hogging that signal, it will only proceed after releasing the lock
< wumpus>
hogging that lock, I mean
< sipa>
why does ShowProgress need a lock?
< wumpus>
I don't know
< wumpus>
it may not actually need it, maybe it's a mistake
< wumpus>
ideally the GUI thread would never aquire cs_main at all, unfortunately we're not there yet
< sipa>
it's only used for the startup check now, i think
< sipa>
maybe it's just not or badly implemented for the main window
< 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
< sipa>
right
< wumpus>
I don't know for sure but it is usually the explanation for 'GUI stuck' issues
< wumpus>
some of the additional statistics fetches are actually under try_locks to avoid this
< wumpus>
but sometimes there's something sneaky that aquires a cs_main lock deep in the code
< jonasschnelli>
I'm now removing the cs_main lock from the SyncWithWallets signal
< wumpus>
in any case I could take a look at it later
< wumpus>
just push the code somewhere and tell me how to reproduce it
< jonasschnelli>
And I agree with wumpus: we should not allows cs_main locks from the GUI logic itself.
< jonasschnelli>
Though, the GUI can call something from the core classes that acquires cs_main
< 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
< wumpus>
getting data from the core should also happen in an auxiliary thread, and be passed using signals
< jonasschnelli>
Right. We already have something that could work: RPC. :)
< wumpus>
it's not rocket science but not trivial to get entirely right
< wumpus>
yes, you need exactly the same changes if the core were to be remotely
< wumpus>
that's a bonus.
< jonasschnelli>
I think the only change that is worth is to fully process separete the GUI.
< jonasschnelli>
First It could be only the node-GUI (non wallet)
< wumpus>
well it would be a step there
< wumpus>
if all communication to the core hapepns through signals, detaching it is almost trivial
< jonasschnelli>
Right. You need similar/same changes.
< wumpus>
in any case it's been on my todo list for years
< wumpus>
I could focus on it again, but playing around with databases is so much more fun :p
< wumpus>
and for years I believed the GUI would eventually be removed anyway, so didn't feel like spending effort on it
< wumpus>
but you changed things around jonasschnelli :)
< 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.
< wumpus>
hahahahah you're on the right track sipa
< jonasschnelli>
hehe...
< sipa>
wumpus: interesting... the Qt GUI was what brought you into bitcoin development in the first place, no?
< wumpus>
sipa: yes, it was
< sipa>
pedrobranco: hi there
< pedrobranco>
hi sipa :)
< 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
< jonasschnelli>
I think the Qt GUI is great! Sure, we could use some UX designers. But one step after the other..
< wumpus>
... but that didn't really happen, and myself I was countinously distracted by other changes that had to be done
< 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
< pedrobranco>
sipa: whats up?
< sipa>
pedrobranco: sorry for all the noise on your importmulti call... i really think we should have such a call
< jonasschnelli>
sipa: during ActivateBestChainStep, connectTip can only be get the pblock pointer (CBlock* pblock from ActivateBestChainStep()) or NULL? Right?
< jonasschnelli>
So we can only sync the transactions from the block passed into ActivateBestChainStep()?
< sipa>
heh, no
< sipa>
activatebestchainstep can activate or deactivate multiple blocks
< sipa>
that pblock is just an optimization
< jonasschnelli>
Right!
< jonasschnelli>
There is a ReadBlockFromDisk...
< jonasschnelli>
okay... will fix it.
< sipa>
so it doesn't need to load that specific block again from disk if we already habe it
< jonasschnelli>
Right. I think I form a std::list with all relevant transactions... should not exhaust the memory I guess.
< sipa>
a vector will be more efficient
< jonasschnelli>
sipa: because it doesn't need to check for duplicates?
< sdaftuar>
paveljanik: IBD takes a while if your initial headers sync is from a bad peer (in this case, a peer that is on a chain you think is invalid)
< sdaftuar>
paveljanik: it should clear up on its own after one of your other peers announces a new block.
< sdaftuar>
(at least, that's my guess)
< paveljanik>
ok, I'll wait a bit.
< paveljanik>
Thanks
< sdaftuar>
paveljanik: hm. after slightly more investigation, i don't have a good explanation of what you're seeing after all -- looks like the bad peer should have been disconnected, and then you should have requested headers from one of your other peers...
< paveljanik>
I did disconnect peers one by one
< paveljanik>
and now the same for other peer
< sdaftuar>
interesting
< sdaftuar>
looks to me like if you have 2 peers on the same bad chain
< sdaftuar>
and the first one feeds you an invalid header, you'll disconnect from it
< sdaftuar>
but then if the second one feeds the same bad header to you, you'll get exactly the error message you pasted, but no disconnect
< sdaftuar>
that seems unfortunate
< sdaftuar>
what height are you at?
< paveljanik>
787628
< paveljanik>
I'll print the block hash of the invalid block to the log to see more...
< paveljanik>
I'm at 787628
< sdaftuar>
checking my logs for invalid block headers...
< paveljanik>
and the logged message is: ERROR: AcceptBlockHeader: block 00000000000005354772cb50ea2decd1e9176724c41eb3427197943aea33194e is marked invalid
< paveljanik>
this is 787391
< paveljanik>
8)
< sdaftuar>
yeah i see a big invalidchainfound message about that
< sdaftuar>
looks like maybe the chain forked after CSV deployed on testnet
< sdaftuar>
ERROR: ConnectBlock: contains a non-BIP68-final transaction
< sdaftuar>
sounds like you need some 0.12.1 peers...
< paveljanik>
8) OK, I'll disconnect all nodes but 0.12.99
< paveljanik>
looks like everyone moved from testnet to segnets ;-)
< sdaftuar>
heh maybe!
< GitHub9>
[bitcoin] paveljanik opened pull request #7952: Log invalid block hash to make debugging easier. (master...20160426_Log_invalid_block) https://github.com/bitcoin/bitcoin/pull/7952
< Chris_Stewart_5>
Shouldn't this be a INVALID_STACK_OPERATION instead of a BAD_OPCODE?