< gmaxwell> I think davec's comment was about the difference between disconnect instantly vs the score? I don't think the score has much use, but most of the reason we bother disconnecting is to avoid resource wasting attacks, and a little bit of vioation is fine on those terms. ... and handwave handwave, maybe the network is a bit more robust if its a little tolerant there. I'm skeptical.
< davec> indeed. We're extremely strict in btcd. For example, if you claim to be a protocol version after the nonce was added to ping/pong, but you don't include them, you aren't following the protocol, so you get disconnected, etc
< gmaxwell> Thats good feedback.
< davec> unless things have changed recently that I missed, core doesn't care
< gmaxwell> davec: do you disconnect on unsolicited TX messages (no getdata first?)
< davec> we used to, but BitcoinJ didn't work, so we allow them now
< phantomcircuit> gmaxwell, hmm that's kind of nasty the only way to relay a transaction is to place it into the mempool
< davec> Another thing we added which I don't believe core does is maximums for every protocol message type
< gmaxwell> phantomcircuit: Thats fine, we'll only allow whitelisted peers to do that.
< davec> so like a transaction can't possibly be < x or > y so it's disallowed
< phantomcircuit> gmaxwell, could soft change mempoolexpirey to 1 so at least they're thrown away rapidly
< phantomcircuit> or not
< gmaxwell> phantomcircuit: nah, I don't see a reason to.
< gmaxwell> they'll only come in from whitelisted peers. Don't whitelist (and allow relay for them) stuff you don't want adding things to your mempool.
< phantomcircuit> ok comments added and that commit removed
< gmaxwell> phantomcircuit: I understood this completely and was comfortable with it before merging your prior code, FWIW!
< phantomcircuit> gmaxwell, heh
< gmaxwell> davec: any idea if bitcoinj still sends unsolicited transactions if its connected to something that has set the relay flag to false?
< phantomcircuit> yes i tried to "fix" something which after thinking about it more is basically not fixable unless you're willing to push the tx messages instead of inv
< phantomcircuit> which isn't very nice
< gmaxwell> phantomcircuit: your first commit is changing too much of the logic, e.g. bypassing nSendsize check.
< phantomcircuit> gmaxwell, the nSendSize check still runs if we do something that could change it
< gmaxwell> phantomcircuit: right now. Creates non-local action; I'd prefer you just make the minimial change and then elseif the other reasons, and emit the log entry.
< phantomcircuit> ok
< davec> gmaxwell: I don't watch it's dev to know if it has changed, but back when we were getting them to work together the answer is yes. It always sent unsolicited txns regardless.
< davec> its*
< gmaxwell> davec: thanks. Yea, thats all I needed to know (expected the answer too)
< davec> I believe the reasoning was that if it authored the transaction they are 100% sure you don't have it, so no point in the round trip
< davec> except it did it for more than that case
< sipa> that's only true when relaying to just a single peer
< gmaxwell> also only true if you'd never relay a third party txn and don't care about privacy at all.
< davec> I agree, but that was the reasoning provided when we asked about it
< davec> at any rate, we either had to loosen up and allow them or not work with BitcoinJ, so we chose to interop
< tulip> I didn't think btcd implemented BIP37 anyway?
< gmaxwell> davec: sure, I'll see if I can convince the new maintership to change.
< davec> it does
< davec> We allow it to be disabled via --nobloom, but it fully supports it
< tulip> ok, crossed wires.
< dcousens> gmaxwell: to change [it]
< gmaxwell> Even if it doesn't ... we may start kicking when fRelay is off-- because it's in their interest if they're relaying to us and we're just going to drop it.
< dcousens> :P
< davec> oh, if it's of any interest to you, we do d/c on unsolicited blocks and have not seen anything that doesn't follow the inv/getdata model there
< sipa> ah, that is good to know!
< tulip> I don't think the relaynode software sends inv for blocks either, I'll have to check though.
< tulip> that looks right.
< phantomcircuit> gmaxwell, i can do it as if(inv.type != MSG_BLOCK) {if (fBlocksOnly) Log() else if (!fAlreadyHave && !fImporting && !fReindex) AskFor();}
< phantomcircuit> i think that's the cleanest change
< phantomcircuit> without it being slightly insane
< davec> tulip: hmmm I don't recall seeing any of them hit that, but I can comb back through the logs. We might have to change that as well.
< davec> Although it seems pretty wasteful to send a full block only to have it ignored by the other end because they already have it or have already reusted it from another peer and its in-flight, etc
< gmaxwell> phantomcircuit: I think you can do it clear, take existing code, add the whitelist. Then add an else that checks everything except the whitelist and the blocks only)
< gmaxwell> tulip: relaynetwork client purposefully does not, but it's local.
< tulip> davec: the relaynode software is only meant to communicate with local nodes anyway.
< GitHub144> [bitcoin] luke-jr opened pull request #7047: [WIP] Backports for 0.11.3 (updated to e8df8a5) (0.11...backports-for-0.11.3) https://github.com/bitcoin/bitcoin/pull/7047
< davec> tulpi: Oh, right. I thought you were talking about the relay network (which I know does things quite a bit differently)
< sipa> davec: he is, i think
< tulip> davec: we're talking about the same thing, just different name
< gmaxwell> he's talking about the relay network client local gateway.
< gmaxwell> As far as I know the normal relay network bitcoin nodes do nothing special on the bitcoin p2p protocol.
< davec> Yeah that makes sense. I would say that's probably a case for whitelisted nodes accepting them but not allowing random unknown p2p nodes to do it
< phantomcircuit> gmaxwell, like this?
< phantomcircuit> that definitely does what we want but... well it's the kind of thing im going to see in six months and go "what idiot did this! oh right me"
< gmaxwell> phantomcircuit: you can reorg the branches to eliminate the duplication but be functionally equal then.
< gmaxwell> also why are you keeping up with the fBlocks only flag. If its only getting used once, no need to make a new flag.
< davec> or even from local conns
< phantomcircuit> gmaxwell, without it as a flag the expression is insanely difficult to understand
< phantomcircuit> gmaxwell, to be pedantic fRelayTxes literally only prevents transactions from being relayed, so if we were to add a new inv type in the future that isn't MSG_BLOCK this logic would be wrong
< phantomcircuit> it would still do the right thing except for the logging
< davec> iirc there is already a MSG_FILTERED_BLOCK too
< phantomcircuit> davec, yeah but that's not relevant for "inv"
< sipa> davec: only in getdata, not in iv
< GitHub150> [bitcoin] fanquake opened pull request #7048: [doc][trivial] Update miniupnpc version in build-unix (master...miniupnpc-build-unix) https://github.com/bitcoin/bitcoin/pull/7048
< phantomcircuit> gmaxwell, oh and using the flag in "inv" prevents calling GetBoolArg in a potentially tight loop
< gmaxwell> phantomcircuit: K leave the flag but ditch the continue.
< davec> sipa/phantomcircuit: right. thanks -- been a while since I looked so forgot the particulars there
< phantomcircuit> gmaxwell, done
< dcousens> gmaxwell: maybe I should clarify, my NACK on that was weak
< dcousens> I just didn't see a need for it except in highly custom setups which IMHO don't fit the bill for most node operators, I may be wrong though
< gmaxwell> dcousens: ya thats fine, if I sounded too eager there in responding I'm sorry; wasn't communicating well.
< dcousens> On reviewing the code, its plain enough (less than 40-50 lines), the rest is tests
< gmaxwell> Does the thought of removing the old mechenism make it more appealing to you?
< dcousens> I just saw +240 and jumped the gun
< dcousens> and yes
< dcousens> If the alterior method is removed, then this is an easy win
< gmaxwell> whew
< gmaxwell> Okay, I confess some paranoia here-- I don't want this to be like the rpc hangs issue. Where we pigheadily avoid improvements for some silly footgun which is making people stop using bitcoin core and switch to hosted apis.
< gmaxwell> no reason to let me ramrod the feature though.
< gmaxwell> If it goes in it should be the best possible.
< dcousens> IMHO, 1 day, walletd, bitcoind, indexd. 1 day. Probably 10 years haha
< instagibbs> it's a beautiful dream :)
< dcousens> We need some Shia to make it happen
< gmaxwell> dcousens: agreed, though we can't halt incremental progress on the way to that vision; ... we'll never get there if we lose all the users who might help support that work. :)
< dcousens> haha, indeed
< instagibbs> Oh, I was thinking "Shia Muslim" and was quite confused...
< dcousens> instagibbs: haha, I meant Laboef
< dcousens> Labouef*
< gmaxwell> #include <culturally_insentive_joke_of_some_form.h>
< morcos> sipa: that "Leaving block file" showing 0's thing. There is no reason not to fix that right?
< morcos> nm, just did it.. :)
< GitHub85> [bitcoin] morcos opened pull request #7050: Fix debug log message for block files (master...nitfix) https://github.com/bitcoin/bitcoin/pull/7050
< phantomcircuit> gmaxwell, ok i've stopped updating 7046 every ten minutes
< phantomcircuit> gmaxwell, i still really do not like having Get*Arg everywhere
< phantomcircuit> but alas my brain is currently jello so im not doing that tonight
< GitHub23> [bitcoin] laanwj opened pull request #7051: ui: Add "Copy raw transaction data" to transaction list context menu (master...2015_11_transaction_hex2) https://github.com/bitcoin/bitcoin/pull/7051
< GitHub147> [bitcoin] gmaxwell pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/e8df8a5077df...f3ea48ad8b85
< GitHub147> bitcoin/master e855b01 Alex Morcos: Fix debug log message for block files
< GitHub147> bitcoin/master f3ea48a Gregory Maxwell: Merge pull request #7050...
< GitHub197> [bitcoin] gmaxwell closed pull request #7050: Fix debug log message for block files (master...nitfix) https://github.com/bitcoin/bitcoin/pull/7050
< GitHub3> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/f3ea48ad8b85...7f8e90da335e
< GitHub3> bitcoin/master 013a364 MarcoFalke: [contrib] Delete test-patches
< GitHub3> bitcoin/master 7f8e90d Wladimir J. van der Laan: Merge pull request #7030...
< GitHub125> [bitcoin] laanwj closed pull request #7030: [contrib] Delete test-patches (master...MarcoFalke-2015-contribC) https://github.com/bitcoin/bitcoin/pull/7030
< GitHub184> [bitcoin] MarcoFalke opened pull request #7052: [qa] python-bitcoinrpc is no longer a subtree (master...MarcoFalke-2015-qaSubtree) https://github.com/bitcoin/bitcoin/pull/7052
< GitHub111> [bitcoin] jtimon opened pull request #7053: Globals: Remove a bunch of Params() from main.cpp before 0.12 (master...globals-chainparams-main) https://github.com/bitcoin/bitcoin/pull/7053
< GitHub25> [bitcoin] jtimon closed pull request #5970: DEPENDENT: Globals: Avoid calling Params() (master...chainparams_cleanup) https://github.com/bitcoin/bitcoin/pull/5970
< GitHub151> [bitcoin] jonasschnelli pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/7f8e90da335e...03403d8c0f3b
< GitHub151> bitcoin/master 513686d MarcoFalke: [qt] Use maxTxFee instead of 10000000
< GitHub151> bitcoin/master 03403d8 Jonas Schnelli: Merge pull request #6951...
< GitHub190> [bitcoin] jonasschnelli closed pull request #6951: [qt] Use maxTxFee instead of 10000000 (master...MarcoFalke-2015-qtMaxFee) https://github.com/bitcoin/bitcoin/pull/6951
< dcousens> out of interest, why username:password vs just token for the RPC?
< dcousens> they're both sent in the clear right?
< sipa> dcousens: if it was just a token, you wouldn't know which one to try
< sipa> dcousens: which is needed if you want strengthening
< jonasschnelli> dcousens: The hashing improves the attack vectors a little bit (tiny, i agree). Isn't the hashing is a little step towards a http-auth-digest likes authentication?
< gmaxwell> dcousens: username is useful because you can log it. also it fits into the http auth scheme.
< gmaxwell> of course, you could set all your usernames to x and just use passwords.
< gmaxwell> oh yea, sipas point too.
< jgarzik> jonasschnelli, http digest auth has been brought up quite a few times...
< jonasschnelli> jgarzik: Yes. I once looked at a possible implementation and decided that it's not worth doing it.
< gmaxwell> It's also not compatible with a surprising amount of stuff.
< jgarzik> the question really comes down to SSL, IMO. We are sending plaintext passwords. Is that a problem on localhost? no. Is that a problem on WAN? Yes. Should you be sending unencrypted RPC over WAN? No, use stunnel. So now you're back to not needing digest.
< gmaxwell> right.
< jonasschnelli> agreed
< wumpus> right: if you want secure RPC, tunnel it over something secure. Digest auth would be pretty pointless. Yes, you protect the auth, but your walletpassphrase will still go over the line unencrypted.
< wumpus> a challenge/response authentication system with the key storage/signer would be nice, as it'd also solve the 'walletpassphrase leaks into process memory' problem, but then again, you'd still be leaking actual commands over the line, which can (at the least) hurt privacy
< jonasschnelli> Right. #7044 (multiple RPC users) is interesting and it would allow to make permissions levels
< wumpus> so no, I think digest auth (or seemingly more secure) RPC auth is optimziing the wrong thing
< wumpus> but we want to get rid of the account stuff; what would be the use of multiple RPC accounts?
< wumpus> if you have multiple, completely separate wallets it could make sense I guess
< dcousens> gmaxwell: sure, but he could have been showing you a curl script with the rpcpassword in it
< jonasschnelli> companies that run services based on bitcoind could have multiple users, one that only accessed public data (like blocks / transaction), another user could have permissions to change things, like ban, etc.
< dcousens> the use case just isn't relevant imho, since, I doubt he minded lol
< gmaxwell> wumpus: not for seperate wallets, but access list management purposes; e.g. you with to withdraw one host, and add a new one. Otherwise you must disrupt everything tfor a flag-day change, and take an outage.
< wumpus> I don't want a complex, complete authentication and permissions system in bitcoin core
< wumpus> we're not buliding an OS here
< wumpus> it has to stop somewhere with importing all kind of ancillary concens
< dcousens> gmaxwell: I'm just not sure if its worth adding, since it'll give people impression that RPC is safer across a non-localized env? Weren't we discouraging that?
< jonasschnelli> Right. This could make things really complicate.
< wumpus> do you want access control? build an RPC proxy and have that check user permissions
< gmaxwell> Sure, but this is basic access management that any other network facing application provides (and basically any web api provides)
< wumpus> sigh...
< jonasschnelli> Yes. Thats a better approach.
< dcousens> (crap, just saw all the text above, notifications daemon died, reading now)
< dcousens> gmaxwell: isn't the point that this doesn't face the network?
< dcousens> All the docs have warnings to avoid that
< gmaxwell> It most certantly does-- not public networks, but even localhost is a network. Stunnel to it is network access.
< gmaxwell> If it's not facing things it cannot be used at all.
< dcousens> Sure, in which case, your on the machine, and, unless its a multi-user env, the hashing is pointless?
< gmaxwell> The secondary use, not enabled yet, is so that logging can also record which account did it, which is useful for debugging and forensics.
< dcousens> by multi-user env, I mean, the syystem itself, not bitcoind
< gmaxwell> The fact that the credentials are hased is a basic security practice that prevents leaking them when handling the credentials themselves; it's also a checklist requirement on many security standards (and not an unreasonable one)
< sipa> a single sha256 would suffice for that, though
< gmaxwell> It would. and if the credential is properly machine generated, then nothing more is actually useful.
< dcousens> gmaxwell: for logging, you could even just use hash(token), not untraceable
< dcousens> but even then, fine, user:pass still if you want make that easier
< gmaxwell> dcousens: doesn't really fit how people use things, the http auth already sends a user/password in any case.
< dcousens> gmaxwell: it just seems to me like the hashed password is implying a different security model to what has been advised up until now
< dcousens> and, aside from the auditing aspect, the username:password scheme is also misleading since its entirely in the clear
< dcousens> so, I guess, are we changing the security model? or keeping it the same
< gmaxwell> dcousens: almost 40 years of system security says otherwise.
< dcousens> gmaxwell: I have *no* issue with hashing, if we're protecting against local breach
< dcousens> but, AFAIK, we haven't protected that to date
< gmaxwell> Seriously, people get _fired_ for keeping unhashed password databases. When a breach results from one it is an embarssment that results in litigation.
< GitHub96> [bitcoin] ptschip opened pull request #7055: MAX_PROTOCOL_MESSAGE_LENGTH description (master...maxmessage) https://github.com/bitcoin/bitcoin/pull/7055
< dcousens> gmaxwell: does that make sense? I'm asking if we're wanting to be more permissive in telling users "local breach isn't catastrophic"
< dcousens> To date, that isn't the case
< Greyboy> I'm late to this convo, but like security. may i ask what passwords you are discussing?
< * instagibbs> reading backlog
< Greyboy> Thank you.
< dcousens> instagibbs: good PR :), just noticed it changes the security model, so, just wanted that to be clear, because it'll likely mean we'll need to update docs which currently mention "dont ever expose the RPC", and, "dont use SSL", etc
< instagibbs> yeah I'm thinking through all this too
< dcousens> and everything being in the clear made that pretty obvious, whereas, now this seems half-half, in what was previously a blatantly unprotected env
< Greyboy> instagibbs, i know i'm new and my opinion is relatively worthless, but i think it's a good idea. my question is what percent of people who run bitcoin-core actually need this solution?
< wumpus> dcousens: hah - the 0.12 release notes specify how to use stunnel as well as transparant proxying, would be more useful to add that
< sipa> Greyboy: or what percentage doesn't run it because it misses features like this? :)
< instagibbs> dcousens, I understand and agree with the concerns, but if we want some basic audit logging, I don't think we should avoid basic improvements
< dcousens> or more recently
< instagibbs> keep the warnings, in general
< dcousens> though, thats a bit circular
< instagibbs> btw where are the warnings today
< dcousens> instagibbs: my main question doesn't stand in the way of doing logging
< dcousens> instagibbs: I'm just talking about the hashing, I'm ACK on everything else, even the hashing, just, clarifying that that is intended
< Greyboy> On a different note, I was wondering what bitcoin-core does that requires it to connect to dyndns?
< instagibbs> dcousens, would you feel better if it was a single hash :P
< gmaxwell> Well that would also drop any worry about where the pbkdf2 comes from.
< instagibbs> ^
< instagibbs> warn people the same things we warned before (strong random keys only)
< gmaxwell> I don't consider the iterated hash actually important, it only potentially saves misusers from themselves slightly.
< dcousens> instagibbs: sure, I mean, the key-stretching seemed a bit unnecessary if we encourage what is already encouraged, strong random keys etc
< sipa> Greyboy: what?
< Greyboy> sipa, i was wondering why bitcoin-core tries to connect to dyndns and alerts my IDS
< instagibbs> I was going for "why not", but if it makes it more palatable, and strictly superior to today, and drops openssl dep....
< sipa> Greyboy: what version?
< dcousens> anyway, its late here, hope I didn't stir up to much of a hornet nest haha. Just, recognizing this was a changed expectation. maybe its a better one, hard to tell
< Greyboy> I don't have it on this machine or i would check, but i've seen this behaviour in many versions including recent ones.
< instagibbs> dcousens, no this is really appreciated. My first Core PR getting tons of attention from smart folks is nice
< dcousens> instagibbs: jgarzik summed it up nicely
< dcousens> "the question really comes down to SSL, IMO. We are sending plaintext passwords. Is that a problem on localhost? no. Is that a problem on WAN? Yes. Should you be sending unencrypted RPC over WAN? No, use stunnel. So now you're back to not needing digest.
< dcousens> "
< instagibbs> I get the concern. Single hash at least gets the "peek over shoulder" security model.
< sipa> and sha256 we already have
< instagibbs> *the village rejoices*
< gmaxwell> dcousens: the digest thing is specifically referring to http digest auth, orthorgonal issue.
< dcousens> yeah just realised, was re-writing that
< dcousens> the other issue was, well, read up about 10 lines, night all
< instagibbs> dcousens, cheers
< sipa> if we want PBKDF2 in there, it's trivial to implement, but if we're going with just hashed passwords... even easier\
< wumpus> right - I think we don't want something with a lot of key stretching, as it'd increase the latency for RPC calls
< wumpus> but hashing the passwords in the config file is a great idea, not sure why we've not been doing that yet
< sipa> we can even do salting by using HMAC-SHA256, but without stretching
< sipa> though i don't think that adds much
< sipa> except "certification"
< gmaxwell> if the key is randomly generated as it should be, the salting does nothing.
< gmaxwell> but yea, it might fail some checkbox.
< wumpus> salting is pretty standard for password checks, yes
< gmaxwell> I suggest the format be such that we could add different types in the future and if we find out its a problem we can just make a new password type.
< gmaxwell> or keep the salt, I don't care. :)
< * wumpus> wonders what Tor uses for torcontrolpassword
< instagibbs> I was just going to drop salt, but either way is fine
< instagibbs> we're assuming people aren't foot-gunning bad passwords already
< gmaxwell> only reason to keep it is certificational, which is an issue here. But it might be answerable with "the salt is _in_ the password" :)
< instagibbs> Excuse my noobishness, what is "certificational" about a salt
< sipa> instagibbs: rainbow tables!
< wumpus> tor apparently does salting, hashing 'bla' repeatedly gives 16:DD2D9F35664C69D7604862845D52D4238AD5E702E8C9DF771108459C00 16:EC737D2A880C18996066882BAE3BA73E32543DA0F74D308572288E38F0 16:35D59809F0AA181160F34D05D6504F89958E2A1D393A29959937C577A0
< gmaxwell> thats a +1 for salt. :P
< instagibbs> damnit
< * instagibbs> re-adds
< instagibbs> :P
< gmaxwell> instagibbs: the certificational part is that there are security checklists that ask if you're using salted hashed passwords.
< instagibbs> ohhhh like actualy certificates. Got it.
< wumpus> ah, thanks sipa, so 16 is the algorithm identifier, then follows the salt and password concatenated
< sipa> wumpus: nope, the 16 means it's encoded in hex :)
< sipa> seriously
< sipa> it's also the only thing supported
< wumpus> huh. ok
< sipa> but ehm... interoperability with tor would be nice imho
< wumpus> I'm confused with $3$ or /etc/shadow etc
< sipa> yeah
< gmaxwell> oh interesting actually thinking of using the same scheme as tor? is it complex to implement?
< sipa> 47 lines of python
< wumpus> we can just steal there code I guess :p although they may lean on OpenSSL as well
< Greyboy> *their
< sipa> it's just sha1
< sipa> iterated
< GitHub43> [bitcoin] ptschip closed pull request #7055: MAX_PROTOCOL_MESSAGE_LENGTH description (master...maxmessage) https://github.com/bitcoin/bitcoin/pull/7055
< wumpus> there's one difference though: tor only authenticates on opening the connection, bitcoin http authenticates for every request. So they can use more key stretching without creating command latency issues.
< gmaxwell> well we already have a sha1 dependency.
< instagibbs> hmm
< instagibbs> link to related tor source?
< wumpus> but the python script sipa linked is more easily readable
< sipa> oh, it has a 1-byte iteration count which uses exponential notation
< sipa> 4 bits of mantissa, and 4 bits of exponent
< wumpus> yes, the 'indicator'
< sipa> up to max 65M iterations
< sipa> and min 1024
< gmaxwell> meh. thats kinda sad.
< gmaxwell> Again I remind: so long as the format has a type field we could add other types in the future if needed.
< sipa> the tor thing actually doesn't (though it starts with an undefined "16:" prefix, which could be redefined into being a type, if necessary, i guess)
< wumpus> normally iteration is done by feeding the previous hash back into the hasher, not in this case
< wumpus> "hash the salty password as many times as the length of the password divides into the count value"
< wumpus> so it defines (roughly) how many bytes will be passed into the hash function
< wumpus> yes, the 16: in case of tor is kind of a red herring, I think anyone would have expected it to be an algorithm id
< gmaxwell> wumpus: what are your thoughts on trying to make the final push to get openssl out of bitcoind? I think all code needed for it has already been written, then abandoned at varrious points in the past. The two big blockers were rpcssl and ecc, and both are resolved now.
< wumpus> we should definitely do it, but for 0.13 imo
< gmaxwell> OKAY, yes, that seems more reasonable I guess.
< sipa> agree
< wumpus> the deadline for 0.12 is getting closer and I'm sure there is high-prio stuff that still needs to get in
< gmaxwell> RNG replacement isn't a small project or one we'd want to do poorly. I think if not for that we could do the rest.
< wumpus> but as soon as 0.12 is branched off let's kill openssl
< sipa> i'll try to find the time after branching to turn the fortuna/entropy gathering code into an external library
< wumpus> I also have that on my list sipa, but feel free to do it :)
< gmaxwell> I am so tired of spurrious valgrind errors on various systems because after all these years openssl can't be bothered to not avoid undefined behavior in integrity critical RNG code. :-/
< wumpus> yes - RNG is kind of critical in our case
< wumpus> foremost importance is that if no good entropy source can be found on a system, it's better to fail than continue with a lousy one, many wallets made that mistake
< gmaxwell> yes, if you have no good entropy and know it. Irritatingly this failure mode is not as common as it should be. :(
< sipa> "Oh, you're using Internet Explorer 3? I'll just go use { return 4; // decided by a fair dice roll } as entry!
< wumpus> exactly :D
< sipa> s/ry/ropy/
< gmaxwell> openssl and libressl both fail that test though, even if they can't get any strong OS rng input, they keep on trucking.
< wumpus> gmaxwell: well I mean if the OS APIs somehow fail
< gmaxwell> wumpus: yea. the design Sipa had done before was also targeting improving the strength when they silently fail (happened somewhat recently in several OSes, :( ).. but certantly if the strong input fails it should full stop.
< wumpus> sure, it may happen that the OS happily returns values which happen to be perfectly predictable because it's only based on the timestamp of VM startup and timings on synthethic devices
< gmaxwell> for the GUI you can always pop up a initial start friendly game of 2048 to record user timings from; but alas, the enviroments most likely to be entropy starved are not the ones with a UI.
< wumpus> lol!
< wumpus> blockchain 2048
< wumpus> "combine the blocks to double the block reward"
< sipa> freicoin can use that 2048 version whose pawn disappear after a certain time...
< wumpus> hehehe
< gmaxwell> I've wondered what simple games extract the most entropy from users in the least time.
< sipa> gmaxwell: things with many keypresses, i'm sure :)
< gmaxwell> maybe a flashcard typing game.
< gmaxwell> oh that shows captcha flashcards so you make mistakes. :P
< wumpus> it's a nice change from the usual 'bash on the keyboard and move the mouse around like you have parkinson disease'
< sipa> LOL
< gmaxwell> wumpus: for a procedure at work I want to just instruct people to roll dice. actual dice rolls produce less entropy than the timing from typing them in, but its fun to do.
< wumpus> or combine the ideas, add a game that involves throwing dice
< wumpus> shuffling a pack of cards may also work, but it's so much work it enter it all :)
< wumpus> so sneaky, would have been so easy to miss "This should not be called with the 2 historical coinbase duplicate pairs because the new coins are marked fresh, and in the event the duplicate coinbase was spent before a flush, the now pruned coins would not properly overwrite the first coinbase of the pair. Simultaneous modifications are not allowed."
< wumpus> (re: #6932)
< GitHub151> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/03403d8c0f3b...73fa5e604356
< GitHub151> bitcoin/master 14470f9 Alex Morcos: ModifyNewCoins saves database lookups...
< GitHub151> bitcoin/master 03c8282 Alex Morcos: Make CCoinsViewTest behave like CCoinsViewDB
< GitHub151> bitcoin/master 1cf3dd8 Alex Morcos: Add unit test for UpdateCoins
< GitHub31> [bitcoin] laanwj closed pull request #6932: ModifyNewCoins saves database lookups (master...newCoinsThinAir) https://github.com/bitcoin/bitcoin/pull/6932
< gmaxwell> \O/
< sipa> someone should put a logprint in CCoinsViewDb on read and write, and reindex with max size dbcache
< sipa> there should only be 1 db operation per block left (the coinsbase overwrite)
< gmaxwell> save the logging until we've gotten that one fixed too? :)
< sipa> that needs #5967
< morcos> here you go:
< GitHub106> [bitcoin] morcos opened pull request #7056: Save last db read (master...saveLastDBRead) https://github.com/bitcoin/bitcoin/pull/7056
< gmaxwell> bam. theres to all the people shed painting over database engines, so long as you never stop your node the database no longer matters. :P
< sipa> gmaxwell: and have infinite dbcache
< gmaxwell> doesn't everyone have 128GB ram?
< wumpus> gmaxwell: is the UTXO set that big now? :(
< sipa> wumpus: he runs 20 bitcoind in parallel, i'm sure
< gmaxwell> wumpus: it's about 6GB in ram right now. Less with sipa's sold script vector replacement thing.
< wumpus> hehhe
< gmaxwell> right now I have a host importing one of the satoshi dice addresses into the wallet...
< gmaxwell> this might not have been a good idea; and .. I picked one with impossibly low odds too.
< gmaxwell> (does anyone have an address to sugget importing that has a lot of txn but not ALL the transactions?)
< btcdrak> gmaxwell: you can find a bunch here https://www.walletexplorer.com
< davec> gmaxwell: if you still need one I typically use 14WCrzmqVCD7Thf79vutWTB6y1hUc8pP19 for such stress tests since it has ~62k
< gmaxwell> davec: thanks.
< gmaxwell> hm. this might be a bit suboptimal; reindex with dbcach=8192. leave sitting for a day. start a wallet rescan and decide that I'd rather not, kill -9 the process. start back up with a deleted wallet.. chainstate is at height=174109
< wumpus> need more db flushes :)
< sipa> if we had flushes that didn't wipe everything, we could do them more frequently
< wumpus> I also once lost almost a whole sync in a kill -9, that's the cost of using -dbcache=8000 I guess. On the other hand it's great to be able to do a sync with almost no i/o.
< wumpus> with the standard dbcache setting it's not an issue
< gmaxwell> heh, yea, it wasn't a complaint so much as a "Oh, yea. thats ... not technically surprising."
< wumpus> does it still do a flush after IsInitialBlockDownload is complete?
< gmaxwell> I think it does, in my case I'd been reindexing and it doesn't on reindex complete AFAIK.
< wumpus> hmm maybe it should
< gmaxwell> I think so.
< wumpus> (I can't think of a reason why it'd be a bad idea to do a one-time flush after it completes indexing)
< gmaxwell> the only downside I can think of is that right now doing that will blow out the cache.
< instagibbs> For debugging core where does one put the flag to do different optimization levels?
< phantomcircuit> instagibbs, CFLAGS="" ./configure
< phantomcircuit> grep for CFLAGS in config.log to see what they are by default
< BlueMatt> morcos: so I'm kinda confused about the proposed "score index" to mempool
< BlueMatt> is there any case where we really want a feerate index and do not intend to use it as "priority to get into next block"
< phantomcircuit> Luke-Jr, posted performance numbers in 6851
< BlueMatt> it seems to me we shouldnt need to indexes for this
< morcos> BlueMatt: Well it probably wont' last as its current incarnation anyway
< morcos> Why 2 indices?
< morcos> we definitely need a mining index and an eviction index?
< morcos> are those the 2 you are referring to?
< phantomcircuit> yeah see this is what i was worried about
< phantomcircuit> "just add another index"
< BlueMatt> oh, this is the inverse index of the one we have? yea, nvm, ignore me
< morcos> phantomcircuit: we right now have 3 indices. time and descendant package fee rate for eviction. and hash for lookup.
< morcos> we have to have a 4th index for mining
< morcos> because descendant package fee rate is useless
< phantomcircuit> probably dont actually need an index for time if the time based eviction has 1 hour granularity
< morcos> phantomcircuit: agreed, we could probably skip that index, and just scan the whole mempool periodically, it should be super fast
< morcos> but since it doesn't change it probably also doesnt hurt
< phantomcircuit> memory usage?
< morcos> BlueMatt: but sdaftuar was trying to add feeDelta prioritisation to your eviction code and realized that the way I did score made it hard. because he doesn't want the modified fee rate. he wants the modified fee
< morcos> So I think he will change it around, but the index will still be sorting by the same thing for mining, the internal data structure might just be a bit different
< BlueMatt> morcos: yea, fair enough...I do wonder, however, if we're gonna do a modified-feerate thing for tx selection, we probably dont want any indexes based on feerate
< BlueMatt> just base everything on modified feerate
< morcos> phantomcircuit: sure, i suppose, its really rather minor memory usage though
< morcos> BlueMatt: yes, thats what will happen
< BlueMatt> even if its modified-feereate-with-package-tracking
< BlueMatt> yea, ok
< morcos> But we need to store the actual fee without modification as well (even though there is no index on it)
< morcos> because it is now required for block construction
< morcos> so it doesn't have to be recalculated
< phantomcircuit> morcos, i assumed as much, but do we have actual numbers for it?
< morcos> phantomcircuit: memory usage? i think sipa was approximating 3 pointers per entry per index, but i'm not sure where he got that?
< morcos> sigh... just realized i forgot to fix that for my 4th index!
< morcos> sipa: is that right, was it 3
< morcos> 3 pointers per index, or did i just make that up?
< phantomcircuit> morcos, wait why is the descendent fee index useless for mining?
< phantomcircuit> oh because you need to also track ancestor fees
< phantomcircuit> ha
< dcousens> gmaxwell: I wonder if the auth aspect will play into the RPC v REST speed, RPC can batch commands/per request which makes me think it'll lead for a while yet
< dcousens> though, without benchmarks, its hard to tell whether univalue is a factor too
< phantomcircuit> hmm
< phantomcircuit> there's no way for us to have more than 1 handle open for the berkeley db in the wallet code
< gmaxwell> univalue is pretty quick, at least my impression from AFLing it.
< phantomcircuit> so Flush is really only used for durability, right?
< phantomcircuit> ha
< phantomcircuit> i was going to ask why nMinutes is only set in CDB::Flush when we're in readonly mode, but satoshi wrote that code
< phantomcircuit> why would we be flushing the database at all if we were in read only mode?
< phantomcircuit> damn it satoshi come back so i can ask mundane questions about code
< phantomcircuit> huh