< dcousens>
also, unrelated, but the total parse time (I realised that, before due to lazy evaluation, it was skipping ahead), when the result is piped to /dev/null, its parsing at ~210MiB/s, otherwise its blocked by output to about 110MiB/s
< dcousens>
(meaning, about 7 minutes for a full 50G parse w/ ~50G output)
< dcousens>
sipa: I also found a blog post detailing what I'm running into, tl;dr all the garbage is just a long run of 0's
< sipa>
dcousens: that's preallocation
< dcousens>
sipa: is it meant to be written though?
< sipa>
dcousens: we preallocate the block files when first creating them, and then overwrite pieces of it with blocks; at the end, when leaving the file, we truncate
< dcousens>
sipa: well, considering my bitcoind is closed, and the file still has a huge run of 0's at the end
< sipa>
but things can go wrong i guess (for example, if the first write after restart is to a new file, you never 'leave' the old file)
< dcousens>
I'm guessing the truncate isn't 100% solid
< sipa>
dcousens: the last file will always have zeroes at the end
< sipa>
as it isn't full yet
< dcousens>
ok, by leaving the file you meant moving onto the next
< sipa>
right, indeed
< gmaxwell>
we prefill with zeros to prevent fragmenting the crap out of non-fragmentation resistant file systems with our incremental writes.
< dcousens>
gmaxwell: gotcha
< dcousens>
though, it probably shouldn't be in the middle of a .dat though
< dcousens>
a 10MiB gap infact, then 252 more blocks after
< gmaxwell>
dcousens: are these old files? We previously had bugs that might cause behavior like that, but I think they're all fixed.
< GitHub175>
[bitcoin] ptschip opened pull request #7034: Fixed Windows secp256k1 compilation issue #7018 (master...secp256k1_compile) https://github.com/bitcoin/bitcoin/pull/7034
< dcousens>
gmaxwell: its a fresh IBD with a just-compiled 4 days ago bitcoin/bitcoin master
< dcousens>
at most the build would be within 2 weeks
< gmaxwell>
okay, that sounds like a bug then.
< dcousens>
Happy to donate time to debug it, but, not sure where to start :S
< gmaxwell>
wumpus: so valgrind is telling me the event_new TorController::disconnected_cb is creating a leak on clean shutdown with stop. But I've confirmed the destructor that calls event_del is correctly firing. Is there some extra libevent magic needed to cause this to get freed?
< dcousens>
gmaxwell: wumpus isn't here? :S
< sipa>
it seems he got truncated into wump
< gmaxwell>
I wondered why my client completed 'wump' :) (but apparently I didn't wonder that much)
< morcos>
gmaxwell: This is not a complaint but just a question, because I do think we should merge more of Jorge's refactoring changes, but we planning on trying to find a time to do a bunch of them so that we can consolidate the merge conflicts?
< gmaxwell>
Feel free to complain in any case; I was mindful of it creating some conflict-- but it didn't move any code and I thought refactors that created structural changes were the big concern there. I looked at existing open PRs (including yours) and concluded that the disruption from this would be small. (and if I felt that I could have merged yours first now and asked Jorge to rebase, I would have d
< gmaxwell>
one that).
< morcos>
yeah like i said, i'm not complaining, rebasing #7020 took a second, and #6898 is going to need some more work depending on #7020 and #7008 anyway. But I just wanted to be aware if we were about to have a slew of these, since we're also trying to get a bunch of stuff in before the freeze.
< gmaxwell>
morcos: if you go and fix your pull req, please provide feedback by screaming my name on IRC in proportion to what a pain in the ass iti is. :)
< gmaxwell>
Ah, no I wasn't planning on pulling most of the other ways. (there was another very minor change from jtimon that I just asked him to rebase wrt the one I just merged, that I think won't break anything)
< morcos>
speaking of, #6898 , the CreateNewBlock pull, it is quite a risky change, although with a big benefit. What else do you think we should do to get comfortable with it.
< morcos>
For instance, I called it out in the initial PR comment, but I didn't realize until reading your comments on IRC this weekend that not stopping to add txs at some minimum feerate is actually a pretty big negative change
< morcos>
I've since fixed that... But it worries me to change the code that miners everywhere will be relying on without extensive testing. What should that be?
< gmaxwell>
A couple things: we can probably add some tests that hand it simulated mempools and check some additional invarients, like that the selection is feerate monotone. We can deploy it early 'mining' (doesn't have to start actually mining, just call createnewblock and sanity check the output) on the main network. We can probably get some miners with non-trival hashpower to deploy it post merge durin
< gmaxwell>
g the 0.12 testing cycle; allowing us to catch an extreme stupidity before release.
< gmaxwell>
It's also bad news that we have so many speedups in 0.12. That means miners will likely upgrade very quickly.
< sipa>
morcos: would it be possible to add the new getblocktemplate as a new RPC (getblocktemplatebeta or so) ?
< sipa>
and leaving the old one as-is for now
< gmaxwell>
Otherwise we would have some buffering from "well, it'll take a while for everyone to deploy it."
< gmaxwell>
hm. if we do that I'd want to do something to identify the blocks created with it... but I don't know that we can. :(
< sipa>
it probably makes little difference
< gmaxwell>
well I just mean that if we see miners making pathalogically stupid blocks it would be nice to be able to tell if the new code is to blame. :)
< sipa>
gmaxwell: i mean that miners will likely use it anyway, even if it's called beta (although that does mean changing some software...)
< morcos>
sipa: i'm sure thats easy enough to do. Maybe that makes sense in that if there is a problem its easy to revert back without having to stop using 0.12
< dcousens>
a rescan shouldn't modify the .dat files, should it?
< gmaxwell>
sipa: that could also be a flag: oldcreatenewblock=0
< sipa>
dcousens: no
< gmaxwell>
Another reason to keep the old one around is if the miners are carrying patches to the old one, they can upgrade to 0.12 and keep them in place... OTOH, more for us to test.
< morcos>
I'd tested previously that it generated the exact same blocks, modulo the comments I put at the top of the pull. But I did forget one more. I'll go update that. I stop trying to fill the last little spot in the block early, but if I increase how much it'll search for that , they were the same
< morcos>
I think I like this idea though. If its a command line option then all we need is my duplicated CreateNewBlock code , and getblocktemplate checks the argument to see which CreateNewBlock it should call?
< phantomcircuit>
gmaxwell, the coinbase sig flags returned by getblocktemplate could be modified
< phantomcircuit>
im not sure if any miners actually use that though
< morcos>
Hmm.. kind of annoying for unit tests and rpc tests though....
< phantomcircuit>
when is feature freeze for 0.12 ?
< sipa>
morcos: or CreateNewBlock itself dispatches based on the command-line flag?
< gmaxwell>
phantomcircuit: lots of people ignore the coinbaseflags coming out of gbt. :(
< gmaxwell>
morcos: it could just be a commandline flag, doubt we want rpc callers picking it on the fly.
< morcos>
One other comment I'll make about 6898 is that its clearly a WIP in the larger sense too. If we can rip out priority for 0.13, it'll clean up the code a lot, and also we might rework it yet again to do some intelligent CPFP
< morcos>
so we should think a bit about how we might wan tto make future major changes to the code as well?
< morcos>
gmaxwell: yes thats what i meant (checks the command line argument) i agree you should not be able to toggle back and forth
< gmaxwell>
Well I do kind of like having the ability to swap in new algorithims with runtime selection.
< gmaxwell>
I don't know how realistic that is generally, since I think different algorithims will require different indexes for efficient implementation.
< morcos>
yes and keeping two algorithms properly tested is basically impossible
< gmaxwell>
well right now with the current one there is a montone property I think it will obey, in that it will always select the highest feerate transactions, so anything omitted must be equal to or lower than the bottom feerate. So testing on real and simulated loads will at least tell us if its behaving crazily (like failing to mine a particular transaction causing it to clog the mempool)
< gmaxwell>
But fancier algorithms will lose that simple property.
< morcos>
gmaxwell: you're assuming wiht a priority space of 0? Also that's not exactly correct, in that a high fee rate child could only have become elgible to be included when there was no longer enough space for it.
< gmaxwell>
Yes, that was assuming priority space of 0.
< morcos>
But I was suggesting we hack up the existing code to change the tie breaker and require that the blocks are identical for testing. I think that's easy enough to achieve. The problem is whether there is some degenerate situation where something bad happens
< gmaxwell>
ah, indeed, it still needs a topological sort.
< phantomcircuit>
morcos, so the answer i would like to give is "it doesn't matter they can run both and the block verification at the end will protect them from bugs"
< phantomcircuit>
but i know that lots of them dont have proper setups so that isn't really true
< phantomcircuit>
:(
< gmaxwell>
phantomcircuit: if you note above I mentioned the case of some high feerate txn not getting mined when it should.
< morcos>
So there are some very weird edge case differences I guess. For instance, one of the versions has the sigop check before the block priority space check and the other has it after. so if you have a transaction that fills the block priority space, you now switch to fee rate sorted, but if it fails the sigop test, then code A will have moved on to fee txs, but code B will still try for another priority tx
< morcos>
i can't remember which is which, and i haven't encountered that yet, nor do i expect to... but at a certain point, mimicking the old behavior is not what we want
< gmaxwell>
no reason to think much of the old behavior is any good.
< phantomcircuit>
gmaxwell, i assume that was from a block maxing out checksigs?
< dcousens>
gmaxwell: "One thought I had on this (before realizing there was already a pool"
< dcousens>
did you mean s/pool/pull/?
< BlueMatt>
gmaxwell: nope, actually, master still verifies with all the pgp sigs
< BlueMatt>
ofc jonasschnelli's key is completely unsigned, so......
< gmaxwell>
dcousens: yep
< sipa>
BlueMatt: i'll go see him when i'm in .ch (assuming he doesn't hate my face)
< BlueMatt>
sipa: sign wump's key while you're up north :p
< dcousens>
sipa_: so
< dcousens>
I'm still debugging this, and the garbage in my blk00289
< dcousens>
is random transactions, but its not at all an actual block
< dcousens>
that pastie is hard to decipher, but tldr I printed some bytes from the previous block to see if maybe the length was off, but, again, that block verifies so :/
< dcousens>
no idea, maybe theres a loose buffer overwrite somewhere?
< gmaxwell>
dcousens: while syncing, we write to the file and only flush the indexes periodically, if you shut off prematurely, it'll continue at the last index write, and overwrite whatever blocks were there. Might this be consistent with what you're seeing?
< dcousens>
gmaxwell: so you're thinking it had written a block, of say size N, it was then shutdown prematurely, and on re-open, it then wrote block of size M at that same index, where M < N?
< gmaxwell>
Yes.
< gmaxwell>
though unless it were the last block placed in the file (according to the block index), I don't know why it wouldn't have overwrote the lest.
< midnightmagic>
out-of-order block storage
< midnightmagic>
makes sense.
< midnightmagic>
sort of..
< dcousens>
gmaxwell: if I nuke this file
< dcousens>
and rescan
< dcousens>
Will it survive? :P
< gmaxwell>
it'll rescan up to that part and begin downloading the rest after it.
< gmaxwell>
(by rescan I mean reindex)
< dcousens>
uh yeah
< dcousens>
reindex*
< dcousens>
won't use any of the other .dat files?
< dcousens>
(as in, it'll abandon all blocks after it and re-download the remaining 15G?)
< gmaxwell>
it'll use all the earlier ones, and none of the later ones.
< dcousens>
ok
< gmaxwell>
It doesn't have a sophicated indirect membership management because ... that only makes sense if you're going to do crazy things like randomly delete a file in the middle. :)
< dcousens>
gmaxwell: :), well, at this point I can do this, or just ignore bytes until I reach a magic int then verify the block :/
< dcousens>
the 2.5 minute blockchain parse is too good to give up now haha
< gmaxwell>
dcousens: what are you doing this parse for?
< dcousens>
gmaxwell: custom index building for various dbs, its the fastest way to bootstrap, 5minutes to write it all out then just use the RPC to get anything else
< dcousens>
much nicer then waiting 40+hrs for an RPC level sync, or hacking bitcoind to do it for me
< gmaxwell>
You benchmark that rpc scan since we got libevent?
< dcousens>
Its just become a bit of a timesink now because of this weird run of 000's and this issue.
< dcousens>
gmaxwell: actually I haven't
< gmaxwell>
I don't know which changes made it faster, but its much faster than it used to be; still going to be slow compared to scanning the files directly probably.
< dcousens>
gmaxwell: but maybe not slow enough to warrant this, point
< gmaxwell>
(well also, if its slow but fast enough to use; I'd like to optimize it further!)
< dcousens>
gmaxwell: the point was also so I had a tooling to be able to do things like check ancestor/depths super quick etc
< dcousens>
for service level bootstrapping, the new RPC speed may be enough,but analytically I'd still like a fast parse.
< gmaxwell>
just take care, there have been some things that lost funds because they were doing block scanning and didn't know some blocks were orphans... :-/
< dcousens>
gmaxwell: indeed, noticed the orphans
< tulip>
there's also traps for people trying to do block parsing as well, some transactions contain blocks.
< dcousens>
haha, like an 80byte OP_RETURN?
< tulip>
the minimum block size is more than 80 bytes, but something like that.
< dcousens>
gmaxwell: I'll give a play at the RPC sync speed
< dcousens>
let you know :)
< gmaxwell>
anyone around that understands the ZMQ interface? I'm trying to figure out how it hooks in to get a signal of a new block. I see nothing in the codebase that references CZMQNotificationInterface::UpdatedBlockTip at all.
< dcousens>
gmaxwell: give me an hour and I might be sharing that question with you
< aj>
gmaxwell: src/validationinterface.cpp sets it up as a signal, and src/main.cpp triggers the signal?
< aj>
gmaxwell: or src/init.cpp has RegisterValidationInterface(pzmqNotificationInterface); to set it up in the first place?
< gmaxwell>
thanks!
< GitHub177>
[bitcoin] gmaxwell opened pull request #7037: Move the blocknotify callback ahead of peer announcement. (master...notify_early) https://github.com/bitcoin/bitcoin/pull/7037
< jonasschnelli>
sipa: BlueMatt: i'll go see him when i'm in .ch (assuming he doesn't hate my face) <--- Hah. The question is, if you don't hate my face after bothering your with tons of wallet/main.cpp questions. :)
< jonasschnelli>
sipa: I would be in Zurich next Tuesday 24., if you want to sign my key
< btcdrak>
I see wumpus has been untruncated thankfully :-P
< CodeShark>
The usual procedure for BIP proposals is to discuss on mailing list first. But given that at least a few important reviewers/critics either have unsubscribed to the ML entirely or else just ignore it, should we discuss it here or in bitcoin-dev?
< wumpus>
well definitely not here, this channel is just for bitcoin core related programming work. The proper answer to your question is stil 'the mailing list'. #bitcoin-dev is fine, but IRC discussion tends to be more ephermal than mailing lists
< wumpus>
(or #bitcoin-wizards if it is moonmath related :-) )
< wumpus>
btcdrak: tends to happen when I get disconnected from the IRC server, the client will automatically butcher my name when it is already used
< btcdrak>
wumpus: it have us all a giggle yesterday :)
< btcdrak>
I was going to suggest we all truncate in solidarity!
< btcdrak>
:)
< wumpus>
lol :)
< wumpus>
gmaxwell: re: valgrind tests, what version of libevent are you using?
< wumpus>
please use an up-to-date version of libevent, I know for fact a few (minor) memory leaks have been fixed over time
< wumpus>
(it's very possible that there is an actual leak caused by our code, but to avoid getting stuck in unnecessary rabbit holes I'd like to rule out issues that were already fixed years ago :-) )
< BlueMatt>
11.04 has been fully deprecated since 2013?!
< wumpus>
right - 11.04 is not a LTS release
< wumpus>
they should upgrade to 12.04, at the least
< rav3n_pl>
hello. im looking to simple way to check for double spends of unconfirmed transaction. ive found bitcoinXT that return some info on "gettransaction" rpc, but it can be used only for in-wallet transactions. is there a way, to check it for non-wallet transactions?
< rav3n_pl>
i can parse mempool transactions, but trying to avoid it
< phantomcircuit>
rav3n_pl, that's impossible
< rav3n_pl>
:/ bad
< phantomcircuit>
rav3n_pl, also you're way offtopic
< rav3n_pl>
:D
< rav3n_pl>
sorry
< rav3n_pl>
not found anythin closer in channel list
< wumpus>
#bitcoin please
< rav3n_pl>
okay, ty
< jtimon>
what I was supposed to do with "undefined reference to `secp256k1_ecdsa_sign_recoverable'", git clean -f and make clean didn't do it...
< phantomcircuit>
jtimon, git clean -fdx
< jtimon>
phantomcircuit: thanks
< fanquake>
Do you still need to pass —with-gui=qt5 to configure to get a qt5 build, or do we change it to default to qt5 when available?
< jtimon>
andytoshi: from what we talked previously, I think you like #6907
< wumpus>
fanquake: no longer necessary to provide that
< wumpus>
if you have qt4 and qt5 development headers installed it will now default to qt5
< fanquake>
wumpus nice; thought I'd seen a pull to change it. Good that we've made that change.
< jtimon>
fanquake: wumpus: maybe something for the release notes?
< sipa>
jtimon: the releases use qt5 for a while already, this only applies to people building from scratch, i think
< wumpus>
jtimon: there's already so many "notable changes" for 0.12 that I doubt this one will make the cut - it'll be in the long list of pulls/commits anyway
< wumpus>
sipa: right
< jtimon>
sipa: wumpus fair enough
< fanquake>
You use [skip-ci] in the body of a pull to skip travis right?
< wumpus>
yes, that should be possible
< wumpus>
(I'm not sure what the exact incantation is)
< sdaftuar>
sipa: wumpus: any thoughts on 7020? morcos and i both have pulls that change the CTxMemPoolEntry constructor which causes a lot of work updating unit tests.
< sdaftuar>
if 7020 is likely to go in i can rebase #6915 on it though
< sdaftuar>
(it needs to be rebased anyway)
< Luke-Jr>
gmaxwell: I can rebase it easily, but I wonder if I should try to get master to actually build first :/
< Luke-Jr>
looks like I had to manually re-configure. more build system issues, sigh
< MarcoFalke>
Trying to set up gitian in fedora. What should I do when ./bin/gbuild asks for a password for ubuntu@localhost?
< MarcoFalke>
wumpus ?
< sipa>
MarcoFalke: try 'ubuntu'
< wumpus>
with lxc? you could add a sudoers rule that allows 'lxc-start' without password, there's an example in doc/gitian-building.md
< MarcoFalke>
nice
< MarcoFalke>
Why would I need to type the password 'ubuntu' five times? I tried it up to three times and assumed it failed because there was no response.
< MarcoFalke>
Anyway, why isn't it using the keys I created?
< MarcoFalke>
I am using kvm
< wumpus>
with kvm it shouldn't be asking passwords at all, kvm sandboxes can be run as unprivileged users, IIRC you only need a few root commands to make the VM in make-base-vm
< wumpus>
but not on gbuild, gbuild asking passwords is absolutely wrong
< MarcoFalke>
make-base-vm asked for my password on the fedora machine
< wumpus>
yes, that's normal
< wumpus>
if gbuild asks for a password *inside* the image there's certainly something wrong with it
< wumpus>
what vmbuilder version did you use?
< MarcoFalke>
0.12.4
< wumpus>
installed from source or from a package?
< gmaxwell>
wumpus: I'll repro on something very new (though this particular system isn't an old one); mostly making sure that it wasn't known and harmless.
< GitHub126>
[bitcoin] instagibbs opened pull request #7044: RPC: Added additional config option for multiple RPC users. (master...multrpc) https://github.com/bitcoin/bitcoin/pull/7044
< GitHub121>
[bitcoin] luke-jr opened pull request #7045: Bugfix: Use unique autostart filenames on Linux for testnet/regtest (master...linux_autostart_unique) https://github.com/bitcoin/bitcoin/pull/7045
< Luke-Jr>
I wonder if #6978 (qt -fPIC stuff) should be backported?
< GitHub102>
[bitcoin] pstratem opened pull request #7046: [WIP] Net: Ignore "tx" messages in blocks only mode. (master...2015-11-17-blocksonly) https://github.com/bitcoin/bitcoin/pull/7046
< phantomcircuit>
gmaxwell, oops i just realized i messed up the whitelistalways stuff
< * phantomcircuit>
goes to fix
< phantomcircuit>
(it technically works but it drops the inv so the whitelisted peer has to actually push the tx blindly)
< phantomcircuit>
sipa, i end up calculating bool fAcceptTxs, is that something you would see being in CNodeState? (currently it's constant during runtime)
< phantomcircuit>
(unless you can remove the whitelist flag at runtime, can you?)
< sipa>
no, whitelist won't change at runtime
< phantomcircuit>
BlueMatt, can i disable the mempool by limiting the size to 0?
< phantomcircuit>
morcos, same question^
< BlueMatt>
i think you have to set it to some big number
< BlueMatt>
but i dont recall now
< phantomcircuit>
although i guess if im going to be relaying whitelisted peers transactions i might as well also be keeping them in the mempool
< phantomcircuit>
gmaxwell, any opinion there?
< sipa>
BlueMatt: he means disabling the mempool, not disabling limiting :)
< morcos>
phantomcircuit: also don't recall, sorry
< morcos>
yikes, it looks like you can't because of bad interaction with descendantlimit
< phantomcircuit>
so blocksonly=1 whitelistalwaysrelay=1 should the node be adding the transaction to it's mempool or just blindly passing it to every peer
< sipa>
tulip: oh, i think that's harmless
< morcos>
BlueMatt: do you think it should be completely prohibited from violating the desired ratio, or just warn
< BlueMatt>
phantomcircuit: I assume you'll break things if you do, but it should disable mempool, yes
< BlueMatt>
oh, yea, what morcos said
< phantomcircuit>
BlueMatt, im not sure what i'd break doing that though since it's already relaying whitelisted peers transactions even if they're rejected from the mempool entirely
< BlueMatt>
morcos: hmm? which ratio?
< morcos>
BlueMatt: if you look at 6557, i implemented that check differently, so the descendentsizelimit is automatically maxed at the right ratio of your mempool size unless you explicitly set it
< BlueMatt>
phantomcircuit: mostly "there be dragons" (of unspecified type)
< tulip>
sipa: nothing seems to be on fire otherwise. is it caused by something broken in my local state, or a recent change in master?
< morcos>
that might be a better parameter interaction
< BlueMatt>
morcos: sorry, I've been behind on bitcoin core...people were starting to notice the relay network had been broken for a long time so i figured i needed to fix it
< morcos>
BlueMatt: you return error if the mempool is less than 40 * the descendant size limit. i'm saying if the descendant size limit isn't set, you should just silently cap it at 1/40 of mempool. and if it is explicitly set, let any two values go, evne if lmiting will be adversely affected
< michagogo>
16:11:00 <fanquake> You use [skip-ci] in the body of a pull to skip travis right?
< morcos>
6557 was just our version of the limiting pull, its closed now, i'm just pointing out a different way to do the parameter interaction
< michagogo>
I think it's [skip ci] or [ci skip], both work
< michagogo>
I don't know if - works
< sipa>
tulip: i believe it's due to a recent change in master
< michagogo>
And I think it's the commit message, though I'm not sure about that
< phantomcircuit>
BlueMatt, hmm well then i'll have to muck about with the "tx" ProcessMessage logic a lot more than i wanted to
< phantomcircuit>
would anybody be opposed to moving it to it's own function?
< phantomcircuit>
(indeed most of the logic in ProcessMessage would do well in it's own function)
< morcos>
phantomcircuit: ha ha, sdaftuar and i would LOVE that. its the most annoying part of our historical data simulator right now
< phantomcircuit>
is anybody going to kill me if i submit a pr that moves each of the message handling routines into it's own function?
< phantomcircuit>
speak now or yell at me later
< BlueMatt>
morcos: hmm...I'm inclined to say leave it as is...I prefer the documented parameters to just refuse to run if it's gonna end up doing something dumb (people will shoot themselves in the foot if you let them
< BlueMatt>
phantomcircuit: please god do
< BlueMatt>
morcos: i wouldnt complain loudly if it was changed, but its not like the limit enforced now is crazy
< sipa>
phantomcircuit: i've argued against that before, because it will conflict with pretty much every PR there is
< sipa>
phantomcircuit: though that was under the assumption that some decent modularization of msg handling would happen instead, and so far, it hasn't
< phantomcircuit>
sipa, it probably will, and it probably always will
< sipa>
yes, but moving things now, and then doing clean modularization later means it breaks everything twice :)
< sipa>
for the same benefit
< phantomcircuit>
sipa, it'll at least be easier to modularize once it's "cleaned" up a bit
< sipa>
hardly, i think
< phantomcircuit>
sipa, when you say modularization you're referring to having a ping module etc
< phantomcircuit>
right?
< sipa>
yes
< phantomcircuit>
sipa, i would be worried about such a system being too complicated
< phantomcircuit>
the basic protocol is really very simple
< phantomcircuit>
the only complexity is in scheduling block download really
< phantomcircuit>
(which i assume is part of your interest in doing that?)
< devpo>
hi is anybdy here to help me a little
< devpo>
?
< devpo>
:'(
< btcdrak>
devpo: what's up?
< devpo>
i need a php script
< btcdrak>
better move to #bitcoin
< devpo>
ok
< sipa>
phantomcircuit: no, my interest is allowing the message handler to run in multiple threads
< sipa>
(and making things cleaner in the process)
< phantomcircuit>
sipa, assuming things are holding the correct locks i dont see why we cant do that now
< sipa>
phantomcircuit: the problem is that it isn't clear what those locks are
< sipa>
if all handler ran in their own module, with their own state, with their own encapsulated lock, it would be trivially correct
< sipa>
anyway, no big objection if you want to move code out of processblock into smaller functions, but not everything at once...
< phantomcircuit>
sipa, it would work today with very if any problems because of cs_vRecvMsg/cs_vSend
< phantomcircuit>
(assuming for a second that there's nothing in SendMessages looking at vRecvMsg
< phantomcircuit>
)
< sipa>
you can't do that now
< sipa>
all shared handler state is locked by cs_main
< phantomcircuit>
it doesn't get you much of anything though since virtually everything they're doing that's cpu intensive requires cs_main
< sipa>
yes
< sipa>
but many of those cs_main only protect handler state, and not consensus/block related structures
< sipa>
those need to be replaced by their own handler-spoecific locks
< phantomcircuit>
what would be an appropriate DoS level for a peer that's sending transactions despite fRelayTxs=false (keeping in mind fRelayTxs will be true for whitelisted peers)
< phantomcircuit>
(and it's sending a protocol version high enough to support fRelayTxs in version)
< gmaxwell>
phantomcircuit: I think we shouldn't DOS now.
< gmaxwell>
not without studying more what it'll do. I know BTCD in the past wanted the protocol to define sending unsolicited transactions as misbehavior.
< tulip>
phantomcircuit: a least one piece of SPV wallet software pushes all of their transactions directly without inv'ing them.
< gmaxwell>
perhaps in the blocks only context is a fine way to introduce that behavior, but I think we shouldn't rush into it.
< gmaxwell>
We can try asking davec if he has any insight into observed behavior.
< gmaxwell>
phantomcircuit: One thing you could do is log when a non-whitelisted peer sends a tx message in blocksonly mode.
< phantomcircuit>
gmaxwell, is it important that GetMainSignals().Inventory(inv.hash); runs regardless of whether we call pfrom->AskFor(inv); or not?
< phantomcircuit>
(if so it needs to be moved)
< gmaxwell>
phantomcircuit: my belief was no, if we're not relaying we don't care what others have relayed to us.
< gmaxwell>
(and in particular, we don't want to know: just another way for a peer to use our memory)
< phantomcircuit>
the version in master actually does call GetMainSignals().Inventory(inv.hash); even if we dont ask anybody for the data
< phantomcircuit>
yeah that's what i thought
< phantomcircuit>
im afraid to ask but, GetBoolArg has it's own locks... right?
< sipa>
phantomcircuit: no, but mapArgs etc are immutable :)
< sipa>
(apart from initialization which happens single-threadedly)
< phantomcircuit>
sipa, so... practically safe but technically a threading violation
< sipa>
phantomcircuit: no, no threading violation
< sipa>
reading data that can't change is safe
< gmaxwell>
phantomcircuit: the memory model is that shared reading is okay, but as soon as there is a potential write (even of the same data) then there must be locks that exclude all other readers and writers.
< gmaxwell>
as the complier is allowed to do things like "oh I can see in this block I will, for sure, overwrite location X.. that means I have exclusive access.. so until then I can spill random registers into it."
< phantomcircuit>
gmaxwell, ah right
< phantomcircuit>
gmaxwell, pr updated
< phantomcircuit>
only thing im not happy with is that it's not obvious why transactions dont end up in the orphan pool
< davec>
Regarding the DoS banning, in general I've never seen much point in allowing protocol level misbehavior. It's TCP, so if the peer isn't following the protocol, they should be disconnected with prejudice.
< phantomcircuit>
gmaxwell, can do
< phantomcircuit>
er is there an easy way to edit an arbitrary commit message?
< sipa>
git commit --amend
< davec>
git commit --amend for the top commit
< davec>
if it's further back, git rebase -i
< davec>
and use 'r'
< phantomcircuit>
oh rebase e
< phantomcircuit>
r*
< phantomcircuit>
that's nice
< tulip>
davec: in 0.8.x nodes were banned for their own internal misbehaviour which was distinctly non-optimal. it's good to be strict, but there's definitely some risk to it.
< sipa>
davec, tulip: i don't think the question is whether we should allow protocol misbehaviour (imho, we shouldn't), but what is considered misbehaviour :)