< gmaxwell> sipa: do you have any thoughts about where in the block acceptance plumbing it it would be best to extract a "most recent time Peer X gave us an accepted block"? As I'm doing for transactions in #7082 ?
< gmaxwell> Anyone else seeing connections from 54.210.68.46 multiple times to your nodes, sitting running the mempool command as fast as its little feet will go?
< jgarzik> I've been inclined to disable mempool by default for months
< gmaxwell> I've banned this particular nussance several times, on several nodes in the past. And now it seems to be changing IPs and behaviors to avoid being banned.
< gmaxwell> If it's also spamming you, feel free to report to https://aws.amazon.com/forms/report-abuse
< gmaxwell> jgarzik: I think we should probably only let one request per peer per block, and only return the top couple blocks worth of mempool.
< tulip> gmaxwell: yes, multiple connections announced as /btcwire:0.2.0/.
< gmaxwell> tulip: yea, I have one node with four, one with two, one with one.
< gmaxwell> I haven't checked the others yet.
< jgarzik> gmaxwell, That's reasonable - though I really don't see much use of it. It occasionally gets used for diagnostics or by miners - though the fingerprint/analysis people will probably use it soon if not already.
< tulip> as much as I'd like to see it gone, people will probably turn to inventory hammering to learn memory pool contents :(
< gmaxwell> most of the stuff related to the anti-fungibility/anti-privacy attacks would be more completely addressed by fixing announcement so that sybil/etc. attacking the network doesn't teach you anything interesting.
< gmaxwell> tulip: good point. Yea, so long as there is a strong incentive to abusively data mine the network; people will do so.
< tulip> on second thought, it's probably just an incentive to stay connected to every peer permanently.
< gmaxwell> tulip: they'd use a lot less bandwidth that way.
< gmaxwell> I mean this current mempool sucker is using 10x the bandwidth of any of my other peers.
< jgarzik> We already account traffic up & down. You'd think this could be dealt with through generic resource accounting.
< tulip> modified clarks law: any sufficiently buggy software is indistinguishable from denial of service.
< gmaxwell> tulip: yea, my amazon report said that since it doesn't have a webpage or any other contact information I can't determine if its buggy or malicious.
< phantomcircuit> jgarzik, i assume that the mempool infinite loop is some kind of poorly thought out sybil attack
< jgarzik> phantomcircuit, plain ole asymmetric DoS
< phantomcircuit> jgarzik, responding to the mempool command is actually very cheap though
< gmaxwell> well they have to recieve the response but bandwidth is differentially valuable to them and me. :)
< phantomcircuit> both ends have equal bandwidth cost
< phantomcircuit> hmm true
< phantomcircuit> asymetric bandwidth value
< gmaxwell> phantomcircuit: I (and my partner) beg to differ!
< phantomcircuit> fyi btcwire is btcd
< gmaxwell> but I dunno that it's intentional, could just be some really ill advised anti-privacy attack, trying to bypass trickling by yanking the mempool super fast.
< jgarzik> might just be "real time network scope"
< gmaxwell> phantomcircuit: this isn't btcd; all the behavior is goofy, it's just someone using their p2p implementation.
< jgarzik> a new product
< phantomcircuit> ah
< tulip> nodes running on ADSL connections have a heavy impact from uploads, you could probably slow down most of those nodes downloading blocks by just requesting them to send you a few and ruining their ACK speed.
< phantomcircuit> jgarzik, ?
< tulip> requesting they send you a few blocks.
< jgarzik> phantomcircuit, speculating
< jgarzik> phantomcircuit, conceivably it's someone testing a new product that monitors mempools across the network in real time
< gmaxwell> tulip: one evidence that it could be an attack is that I _believe_ the same party (amazon, btcwire) used to be requesting a single big over and over again in a loop.
< phantomcircuit> jgarzik, if so they're doing a shit job of it
< phantomcircuit> the mempool command adds enough latency that it would actually damage their measurements
< tulip> jgarzik: if they wanted to do that, why are they using multiple connections, and why request with mempool when you can see inv messages in real time?
< gmaxwell> I don't know if the block looper stopped generally, becuase jonas' bandwidth limiter kills them. :)
< gmaxwell> (as the block they were requesting was months old)
< jgarzik> tulip, 'mempool' shows you when transactions leave the mempool too...
< phantomcircuit> jgarzik, that doesn't tell you much of anything though
< jgarzik> we're all guessing, who knows
< tulip> sure.
< phantomcircuit> im thinking gmaxwell is right and it's just an attack
< gmaxwell> maybe my abuse report will result in hearing something.
< jgarzik> I think it's a DoS attack
< gmaxwell> I could certantly see this as a misguided monitoring attempt. But regardless, the motivations aren't that important.
< jgarzik> but it's a useful exercise to try and figure out weird, unlikely attacks ;p
< gmaxwell> I think before this discussion I hadn't connected that mempool bypasses trickling... good thing to figure that out.
< davec> fwiw, 0.2.0 is months old too, so it's likely whoever is up to it has been at it for a while
< gmaxwell> yet another gaping privacy hole we added to the protocol. :(
< gmaxwell> davec: yea, I've seen it before; it's bubbled up to the level of complaining about it here because it has persisted and changed addresses.
< tulip> jgarzik: you're the BIP author by the looks of things, would you have any qualms about removing it completely? do you know of any software which makes use of it?
< gmaxwell> tulip: or made whitelisted peers only, perhaps?
< tulip> that sounds like a better option.
< gmaxwell> bluematt: does the relay node client use the mempool message?
< * gmaxwell> greps logs
< jgarzik> tulip, scroll up ;p
< tulip> jgarzik: whoops, missed the first assertion for removal, sorry.
< phantomcircuit> gmaxwell, it does but i cant remember how
< tulip> wouldn't the relay node be on a whitelisted port anyway?
< davec> doesn't the testing framework use it?
< davec> (mempool that is). It's been a while, but I seem to recall it being used by the block tester tool to compare mempool contents
< gmaxwell> it really shouldn't, we've complained about that before.
< gmaxwell> maybe it got fixed. :P
< gmaxwell> otherwise I dunno why all the recent mempool changes didn't make it fail all over itself.
< jgarzik> rpc-tests uses getrawmempool
< gmaxwell> my log data suggests that it's being used once per connection by some wallet or something.
< gmaxwell> hm. maybe.
< gmaxwell> http://0bin.net/paste/WPN3p8EyTg5QiXwe#oc16NOdpqju3WllP7+4FO4tbcphIwDQN5ZqHBv8bOux
< tulip> total count in the left column?
< gmaxwell> actually no, most of those are "/bitcoin-seeder:0.01/" "/Snoopy:0.2.1/"
< gmaxwell> ah so spv clients are doing a
< gmaxwell> 2015-11-24 02:17:45 received: filterload (1923 bytes) peer=4177
< gmaxwell> 2015-11-24 02:17:45 received: mempool (0 bytes) peer=4177
< gmaxwell> "/bitcoinj:0.13.2/MultiBitHD:0.1.4/"
< gmaxwell> but most of this stuff is ... things which are, uh, not in my interest to serve these responses to.
< tulip> bitcoin seeder does a mempool request? surely not
< gmaxwell> something claiming to be it does.
< tulip> yep.
< gmaxwell> anyways, if we don't kill it completely we should cap its size and find a way to fix the privacy leak. :(
< raskolnnikov> hello guys, I'm beginning work on a small research about SPV's transaction validation. Anyone has some 10 mins available to discuss a bit, I'm fairly lost inside the core src...
< gmaxwell> raskolnnikov: norm on IRC is to ask your question and pray for a response. :)
< davec> there is a definitely at least one bogus client out there claiming to be bitcoin-seeder
< tulip> there's lots claiming to be satoshi as well.
< gmaxwell> raskolnnikov: (and wait around long enough so people who aren't actively paying attention can answer)
< davec> I have noticed it not following the protocol
< gmaxwell> tulip: dunno who these things think they're fooling. "/Satoshi:0.10.2/" ... "why are you sending me filterloads?"
< gmaxwell> I like the ones that are copy and paste errors.
< raskolnnikov> ha, I'm fairly new to IRC!
< raskolnnikov> thanks!
< davec> Yeah that's are funny
< gmaxwell> "Satoshi:0.11.1/"
< davec> those*
< davec> I saw a few misspelled ones too
< gmaxwell> davec: I'm amagining a person with a nose-and-glasses setup.
< tulip> they're easy enough to detect, but even easier to evade again.
< raskolnnikov> SPV's wallets provide a txn hash to full nodes whenever they want to verify if the transaction exists on a previous block and is unspent. where in the SRC code does bitcoin-core process and replies to these kind of requests?
< raskolnnikov> I'd like to log these requests from a test network to a log file
< gmaxwell> raskolnnikov: ah, you can't find that because that isn't how SPV wallets work.
< gmaxwell> raskolnnikov: if someone were to give you a transaction, they could also give you the proof that it was in a block. So there is currently no facility in the protocol to just extract a proof for an arbritary transaction. (among other things this would require an index of all transactions, which bitcoin full nodes do not usually have)
< gmaxwell> raskolnnikov: instead you can download a block and filter the download to particular transactions or addresses, see MSG_FILTERED_BLOCK in the bitcoin core source code
< raskolnnikov> so there only is a request for the particular block (which I assume is extracted from some sort of block header, in the proof provided to the wallet) from the SPV to the full node
< raskolnnikov> and then the full node only provides the requested block back to the SPV?
< davec> a merkleblock (which is a filtered version) yes
< gmaxwell> raskolnnikov: it does not necessarily provide the whole block, the block can be filtered in way that proves the filtered subset is in there. See BIP37.
< gmaxwell> The gettxoutproof RPC in bitcoin core also provides an eqivilent message over the RPC.
< raskolnnikov> and is there a way to single out the requests that come from SPV wallets? cause I assume that other full nodes will be requesting blocks or filtered blocks as well...
< raskolnnikov> cause those are the one I'd like to put on a log
< tulip> why?
< raskolnnikov> I'd like to measure the impact of having modified full nodes
< raskolnnikov> which "lie" back to the SPV wallet
< tulip> do you realise that BIP37 only allows lying by omission in most circumstances?
< gmaxwell> raskolnnikov: full nodes can freely omit transactions, thats one of the limitations in the design of BIP37. There are other schemes which lack that limitation which are possible but have not been implemented yet.
< gmaxwell> raskolnnikov: as to your question, only SPV clients currently fetch filtered blocks.
< BlueMatt> gmaxwell: nope
< BlueMatt> gmaxwell: there is even a whole rpcclient class thinggy that could be used
< gmaxwell> ::sigh:: seems like my laptop has a spurrious warning on mainnet: "errors": "WARNING: check your network connection, 3 blocks received in the last 4 hours (24 expected)"
< gmaxwell> Anyone have a recent-ish count of how many distinct scriptpubkeys there are in the txout set?
< CodeShark> I used to keep a database of that but haven't been tracking that in a while
< GitHub82> [bitcoin] pstratem opened pull request #7087: [Net][WIP]Add -enforcenodebloom option (master...2015-11-23-bloom-disable) https://github.com/bitcoin/bitcoin/pull/7087
< GitHub191> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/0b0fc179ab87...f91e29fd4d0e
< GitHub191> bitcoin/master 3522f49 Wladimir J. van der Laan: http: add Boost 1.49 compatibility...
< GitHub191> bitcoin/master f91e29f Wladimir J. van der Laan: Merge pull request #7065...
< GitHub27> [bitcoin] laanwj closed pull request #7065: http: add Boost 1.49 compatibility (master...2015_11_httpserver_boost1_49) https://github.com/bitcoin/bitcoin/pull/7065
< pkthebud> Hello 😃
< GitHub187> [bitcoin] MarcoFalke opened pull request #7088: [trivial] pull secp256k1subtree (master...MarcoFalke-2015-syncSecp256k1) https://github.com/bitcoin/bitcoin/pull/7088
< GitHub13> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/f91e29fd4d0e...ed34e0577e8d
< GitHub13> bitcoin/master a0953cd MarcoFalke: [qa] python-bitcoinrpc is no longer a subtree...
< GitHub13> bitcoin/master ed34e05 Wladimir J. van der Laan: Merge pull request #7052...
< GitHub133> [bitcoin] laanwj closed pull request #7052: [qa] python-bitcoinrpc is no longer a subtree (master...MarcoFalke-2015-qaSubtree) https://github.com/bitcoin/bitcoin/pull/7052
< gmaxwell> Anyone know why zapwallettxes might use a lot more memory than a rescan?
< gmaxwell> my huge wallet test host (500k txn wallet) OOM killed on a zap, with about 24GB used--- where normally it uses about 15GB.
< GitHub69> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/ed34e0577e8d...b1fcdec68790
< GitHub69> bitcoin/master 2fcb849 fanquake: [doc][trivial] Remove source forge from Debian watch.
< GitHub69> bitcoin/master 70899d7 fanquake: [doc][trivial] Update Debian control description...
< GitHub69> bitcoin/master b1fcdec Wladimir J. van der Laan: Merge pull request #7042...
< GitHub56> [bitcoin] laanwj closed pull request #7042: [doc] Update Debian watch & control (master...update-debian) https://github.com/bitcoin/bitcoin/pull/7042
< GitHub105> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/b1fcdec68790...72dccfc29dfc
< GitHub105> bitcoin/master 2aa49ce Luke Dashjr: Bugfix: Use unique autostart filenames on Linux for testnet/regtest
< GitHub105> bitcoin/master 72dccfc Wladimir J. van der Laan: Merge pull request #7045...
< GitHub148> [bitcoin] laanwj closed pull request #7045: Bugfix: Use unique autostart filenames on Linux for testnet/regtest (master...linux_autostart_unique) https://github.com/bitcoin/bitcoin/pull/7045
< GitHub146> [bitcoin] laanwj closed pull request #7066: [Trivial,Doc] Add missing "blocktime" description to listtransactions help, fix formatting. (master...listtransactions_blocktime) https://github.com/bitcoin/bitcoin/pull/7066
< GitHub1> [bitcoin] laanwj closed pull request #6306: Prevent peer flooding inv request queue (redux) (master...no_duplicate_askfor) https://github.com/bitcoin/bitcoin/pull/6306
< GitHub98> [bitcoin] laanwj reopened pull request #7066: [Trivial,Doc] Add missing "blocktime" description to listtransactions help, fix formatting. (master...listtransactions_blocktime) https://github.com/bitcoin/bitcoin/pull/7066
< GitHub67> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/72dccfc29dfc...02a0f348c210
< GitHub67> bitcoin/master 5c2fd38 Pavel Janík: Add missing "blocktime" description to listtransactions help, fix formatting.
< GitHub67> bitcoin/master 02a0f34 Wladimir J. van der Laan: Merge pull request #7066...
< GitHub135> [bitcoin] laanwj closed pull request #7066: [Trivial,Doc] Add missing "blocktime" description to listtransactions help, fix formatting. (master...listtransactions_blocktime) https://github.com/bitcoin/bitcoin/pull/7066
< phantomcircuit> it's kind of comical how much more productive i am without video games
< phantomcircuit> gmaxwell, iirc zapwallettxs keeps two walletdb objects open, so will presumably use a minimum of double the memory
< sipa> phantomcircuit: you should sold *all* your gpus after the gpu mining era was over
< phantomcircuit> sipa, heh
< wumpus> heh I stopped playing video games at the same time I stopped striving to be a game developer. Just lost interest with modern games and the PC upgrade treadmill
< phantomcircuit> wumpus, it helps when you're using old 7970s from a mining farm
< phantomcircuit> wumpus, need anything else on 7087 ?
< wumpus> no, looks good to me now
< midnightmagic> wumpus: independent game devs can make it these days. A friend of mine is in the middle of doing it, he lives in norway right now.
< wumpus> midnightmagic: yes that still sounds kind of fun
< midnightmagic> definitely an appetite for cleverness that's within the grasp of small teams, it seems. and ios can be done by one or two people. retro gaming scene is hot.
< MarcoFalke> wumups, not sure we should silently "ignore" negative values from GetArg()?
< MarcoFalke> is done very rarely, though.
< MarcoFalke> *Sanitization is done very rarely, though.
< MarcoFalke> * wumpus,
< MarcoFalke> We need to collect all accepted config args and their valid range. Also warn about unknown config args.
< wumpus> MarcoFalke: definitely not ignore
< wumpus> yea, we're ignoring a lot of command-line issues, I have a very old issue about this https://github.com/bitcoin/bitcoin/issues/1044
< phantomcircuit> gmaxwell, did you see my PR for 7082 ?
< MarcoFalke> Changed it to uint_ and dropped the < 0 check like you suggested.
< MarcoFalke> Now, we get funny errors like Error: -maxmempool must be at least 1.84467e+16 MB, at least.
< wumpus> so a negative value is an explicit error now?
< wumpus> eh, that's definitely not an improvemnt
< wumpus> just leave it as is then
< MarcoFalke> I like it uint ;)
< wumpus> sure, uint makes sense, but that's only acceptable if you avoid the user from passing negative values and causing overflows
< wumpus> (which the old code did, so probably my suggestion was just wrong)
< MarcoFalke> Usually -1 means "just make it large enogh that I don't have to care about it"
< wumpus> it just looked weird to compare to both 0 and that limit value, a typical developer trap 'hey, this could be simpler...'
< wumpus> which is fine if you handle it explicitly, don't rely on integer overflow
< gmaxwell> phantomcircuit: thanks, I wouldn't have.
< gmaxwell> phantomcircuit: you dropped the node network compare, otherwise your PR is just a reordering, and a fine one, go fix that.
< phantomcircuit> gmaxwell, ha derp so i did
< phantomcircuit> gmaxwell, ok there you go
< MarcoFalke> Restored original behavior: "Error: -maxmempool must be at least -120 MB
< MarcoFalke> "
< wumpus> ok..
< wumpus> we probably want to check -limitdescendantsize against 0 as well, or can it be usefully negative?
< sdaftuar> wumpus: limitdescendantsize cannot be usefully negative
< wumpus> in other places we assign the result to a size_t
< wumpus> type-safe argument parsing would be great to have at some point in the future, bit of a mish-mash now, but checking that it is >=0 at least prevents this from being an issue
< MarcoFalke> Is this framework sipa mentions ready for copy & paste? https://github.com/bitcoin/bitcoin/pull/4194#issuecomment-44521490
< wumpus> I doubt it
< sipa> i started writing that a long time ago, but never finished it
< wumpus> although the approach is a lot saner than what we have now
< wumpus> I think it's complicated by various options that, in turn, determine each other's ranges. But even basic type and sane-range checking with a consistent system would be a way forward.
< wumpus> and at least having base argument parsing in one place so that it can flag unknown arguments
< GitHub197> [bitcoin] petertodd opened pull request #7090: Connect to Tor hidden services by default (master...2015-11-onion-by-default) https://github.com/bitcoin/bitcoin/pull/7090
< GitHub18> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/02a0f348c210...b19fe277dd62
< GitHub18> bitcoin/master 4846543 tulip: Move time data log print to 'net' category to reduce log noise
< GitHub18> bitcoin/master b19fe27 Wladimir J. van der Laan: Merge pull request #7075...
< GitHub64> [bitcoin] laanwj closed pull request #7075: [Trivial] Move time data log print to 'net' category to reduce noise (master...no-time-offset-logging) https://github.com/bitcoin/bitcoin/pull/7075
< GitHub149> [bitcoin] jtimon opened pull request #7091: Consensus build package (master...consensus-build) https://github.com/bitcoin/bitcoin/pull/7091
< MarcoFalke> travis is only running 3 jobs in parallel instead of 5, what it used to be...
< phantomcircuit> sipa, huh ActivateBestChain seems to be super slow when it's able to connect a large number of blocks all at once
< sipa> phantomcircuit: it is, it should cache things
< cfields_> jonasschnelli: ping. regarding osx perms, looks to me like perms are correct in gitian, but they're wrong in the created image
< cfields_> jtimon: will have a look
< jtimon> cfields_: awesome, for some reason I haven't found out yet travis doesn't like it, even if it built locally yesterday
< jonasschnelli> cfields_: so it could be a issue created by the dmg tool?
< cfields_> jtimon: you need $()
< cfields_> jonasschnelli: yea, looks that way to me. testing a fix now
< jonasschnelli> cfields_ : nice! Thanks for looking at it. It's an ugly OSX bug that we should solve in 0.12 and backport.
< cfields_> jonasschnelli: np. I'm still trying to figure out why it's not an issue for me
< cfields_> jonasschnelli: yea, that fixes
< cfields_> fixes the perms, anyway. I can't reproduce, but i assume setting the dir to 0755 is enough
< GitHub91> [bitcoin] theuni opened pull request #7092: build: Set osx permissions in the dmg to make Gatekeeper happy (master...osx-perm-fix) https://github.com/bitcoin/bitcoin/pull/7092
< jonasschnelli> cfields_: super. Thanks. Testing tomorrow (in about 10h).
< cfields_> jonasschnelli: great, thanks
< phantomcircuit> would anybody be opposed to adding a last ditch peer discovery mechanism (even after trying the hard coded nodes) of "scan the internetz"
< sipa> phantomcircuit: as in: try random IPs? :o
< jtimon> cfields_: in any case, I'm more interested in your opinion than making travis happy at this point
< cfields_> jtimon: from the looks, i believe that now involves building each object 3 times
< jtimon> how so?
< jtimon> one for libconsensus and one for the rest, no?
< jtimon> like before, no?
< cfields_> ah sorry, misread. yes.
< * jtimon> naively let himself think he was saving one build
< cfields_> jtimon: to save the build, you'd have to add the objects rather than the sources
< cfields_> don't think libtool will let you do that, though
< jtimon> never mind, I was expecting you to maybe complain about https://github.com/jtimon/bitcoin/commit/083e1344f7162d516ffcf2bb30622b28767e415e
< jtimon> or https://github.com/jtimon/bitcoin/commit/f37ff9b61375f8f667f17d529b6422e9c2c353c4 (it makes bitcoin-tx a little bit bigger)
< jtimon> not much bigger though
< cfields_> jtimon: yes, i'm not a fan of that. I see your reasoning, though.
< jtimon> but it signals the intend to put verifyBlock in the consensus package, which bitcoin-tx will still have to depend on
< jtimon> not a fan of which part?
< cfields_> breaking crypto out of its contained unit
< jtimon> I don't care much about the crypto part, it is currently well encapsulated and protected in the same way it would be in the consensus package, I guess we can leave that for the time when libconsensus gets its own repo (after exposing verifyblock)
< jtimon> any concerns about libconsensus depending on hmac_sha256 (without using it) by depending on the whole crypto package instead of all files but one?
< jtimon> that way we can say libconsensus contains the libsecp256k1, crypto and consensus packages
< jtimon> forget about that for now, I will update the branch without unifying crypto but in a way that I still like it and ask again
< phantomcircuit> sipa, yeah and why not?
< sipa> phantomcircuit: nobody implemented it :)
< GitHub147> [bitcoin] gmaxwell opened pull request #7093: Address mempool information leak and resource wasting attacks. (master...mempool_infoleak) https://github.com/bitcoin/bitcoin/pull/7093
< sipa> phantomcircuit: it's very rarely an issue
< gmaxwell> I think I fixed the mempool dos attack/information leak problem.
< gmaxwell> phantomcircuit: doing that pisses off network operators and gets your software firewalled off. Also we have such a low nodecount that it's not a very effective mechenism.
< phantomcircuit> gmaxwell, yes i was thinking of it as something that wouldn't run for at least 30 minutes
< phantomcircuit> and would be pretty slow even then
< gmaxwell> phantomcircuit: slow = never find peers. :P
< phantomcircuit> gmaxwell, "slowish"
< gmaxwell> phantomcircuit: go look at my candidate mempool attack fix
< phantomcircuit> gmaxwell, in about an hour i have to go move my car now...
< phantomcircuit> who does street sweeping at 3pm?
< * jgarzik> reads
< * sipa> sleeps
< cfields_> jtimon: how about something more like this: http://pastebin.com/raw.php?i=JBqjPYAr ?
< cfields_> (it's a quick hack, the other targets need to be updated, i only changed bitcoind)
< cfields_> that creates an internal helper lib that the other libs include. means everything's only built ones. and provides a clear distinction between what's consensus and what isn't
< jtimon> we still remove the files from util and common, I think I get your point, putting it all closer to the libconsensus stuff
< jtimon> should I put the .h directly in libbitcoinconsensusint_la_SOURCES?
< jtimon> btw, what does the int stand for?
< cfields_> jtimon: just an idea, i haven't really thought it through
< cfields_> internal. again, just a quick hack
< cfields_> gmaxwell: i'm having trouble parsing that too. You just want openssl optional, and only add the comparison tests if it's found/enabled ?
< jtimon> cfields_: I mean, the way I see it, this is mostly levearaing work that is already done, just make it harder or more obvious for contributors to break the existing encapsulation
< cfields_> jtimon: yea. that way, each lib-friendly module is built with its own flags. We can control include paths that way, making it not possible to accidentally add unsafe headers/objects
< jtimon> cfields_: one secondary goal is putting together whatever we plan to take out with libconsensus' subtree once it is complete
< jtimon> but that can wait
< gmaxwell> cfields_: Yes, it should be optional, and be switchable off. OpenSSL is actually a constant irritation for me when testing on random systems, because it throws a zillion and one valgrind errors (openssl is not valgrind clean unless compiled to be so)
< cfields_> gmaxwell: ok, that's already how it works. Looks like you just want the option to explicitly disable openssl rather than using it if it's found?
< gmaxwell> cfields_: yes.
< cfields_> roger
< jtimon> cfields_: got it, so no BITCOIN_CONSENSUS_H, put the .h and the .cpp together directly
< jtimon> cfields_: like in the crypto package
< cfields_> jtimon: i have no real preference there
< jtimon> cfields_: well, me neither, so...
< gmaxwell> cfields_: interested in running coverity again? we haven't run it in a while. I added directory masks for our new directories under src. (If you're no longer setup to run it, I'll run it... I just don't want to stomp on you if you are. Path changes can make the reports somewhat less useful if you bounce between systems that its run on)
< jtimon> I think I'll just copy the ways of crypto then
< cfields_> gmaxwell: go for it, i haven't messed with it in a while. I poke at it with clang tools now and then, i'd be curious to see how the results differ
< gmaxwell> cfields_: mostly I wanted to get a run against current libsecp256k1. :)
< cfields_> heh
< cfields_> gmaxwell: while you're at it, make sure to throw clang-tidy at it. It's turned up some useful stuff
< jtimon> but the main question...is it ok to add primitives/block, arith_uint256, version.h, serialize.h and company to the consensus package (and therefore to bitcoin-tx)?
< jtimon> s/"version.h, serialize.h"/"consensus/params.h, consensus/validation.h"/
< cfields_> jtimon: the headers are only listed to keep track of what files go into the tarball. they're not actually used in depenency resolution
< cfields_> so just do whatever makes it the most clear
< jtimon> cfields_: so nothing can fail in the build by .h files having circular package dependencies and stuff? what a pity
< cfields_> jtimon: i don't believe so. Need to control it with paths.
< jtimon> I think it still makes it clearer
< jtimon> currently https://github.com/libbitcoin/libbitcoin-consensus/tree/master/src/clone is more clear than libbitcoinconsensus_la_SOURCES
< jtimon> thanks for the feedback, I will force push an updated version
< cfields_> np
< jtimon> cfields_: could you push your raw patch somewhere on github for convenience?
< cfields_> jtimon: will do in a few min, that's mixed in with my current branch
< jtimon> oh, I see, no hurry
< jtimon> hehe, I wasn't sure if it was on top of master or one of my commits
< cfields_> nah, master
< jtimon> oh, never mind then
< cfields_> ok
< phantomcircuit> gmaxwell, we're keeping travis very busy