< bitcoin-git> [bitcoin] theuni closed pull request #9509: build: fix qt distdir builds (master...out-of-tree-build) https://github.com/bitcoin/bitcoin/pull/9509
< bitcoin-git> [bitcoin] theuni opened pull request #9513: build: fix qt distdir builds (retry) (master...out-of-tree-build) https://github.com/bitcoin/bitcoin/pull/9513
< bitcoin-git> [bitcoin] theuni opened pull request #9514: release: Windows signing script (master...win-signing-script) https://github.com/bitcoin/bitcoin/pull/9514
< paveljanik> jonasschnelli, it is a certificate only
< paveljanik> no privkey
< jonasschnelli> paveljanik: okay. Not familiar with windows code signing...
< paveljanik> -me is not familiar with Windows at all ů=0
< paveljanik> ;-)
< bitcoin-git> [bitcoin] kallewoof opened pull request #9516: Bug-fix: listsinceblock: use max depth value for blocks in reorg'd chains (master...listsinceblock-reorg-fix) https://github.com/bitcoin/bitcoin/pull/9516
< jtimon> rewarding 8994 (custom chainparams), how important it is that it loads the chainparams from a different conf file instead of regular args and the existing config file? the arguments will be ignored for main, testnet and regtest anyway
< jtimon> NicolasDorier: regarding your vision for libconsensus, do you mind if I make some more questions here?
< NicolasDorier> sure
< NicolasDorier> I think trying the following
< NicolasDorier> brand new project
< NicolasDorier> copy/pasta of bitcoin core
< NicolasDorier> then stripping everything not related to consensus
< NicolasDorier> removing all dependencies to third party libs
< jtimon> well, copy paste from core or doing it in core ignoring the rest is not that different is it?
< NicolasDorier> I think it is harder
< NicolasDorier> like there is so much reference to the utxo storage in the consensus code.
< NicolasDorier> I would prefer being able to strip that
< NicolasDorier> rather than trying to make interfaces
< jtimon> anyway, one frequent discussion is abstract storage yes no, you told me you say yes (like BlueMatt and me), but it seems your way of doing it it's a bit different. ie instead of function pointers, your API assumes that all the needed data will be provided directly in the parameters
< NicolasDorier> I changed my mind. I think the consensus lib does not have to know anything about storage, I think we can ask the client to pass the UTXO necessary for validating the block
< jtimon> but then the client needs to know which ones will be relevant before calling
< NicolasDorier> yes, and but the client know that
< jtimon> and will need to fetch them even if the tx/block was already invalid because of something trivial
< NicolasDorier> since he has the block
< NicolasDorier> correct. But the client can verify PoW before fetching
< jtimon> sure, just comparing with the function pointer version of it, in both cases it will need to have the data
< jtimon> another discussion was how the client gets that data
< NicolasDorier> this would be the client's business
< NicolasDorier> this is simple enough stuff to do
< NicolasDorier> iffunction pointer
< NicolasDorier> it is complicated to use, and
< jtimon> matt advocated for, using a function pointer like interface, he would expose something like processBlock rather than only verifyBlock, that way if the block is valid it will also call the necessary function pointer api for the abstracted storage to store the new block, update the utxo, etc
< NicolasDorier> if the implementation cross boundary between languages, there is huge perf hit
< NicolasDorier> I don't think this is the right approach. I think it is just easier to ask the client to fetch everything preventively
< jtimon> right, that's the advantage of your approach, less performance hit by avoiding the function pointers (although more in some cases by prefetching everything)
< jtimon> right, just comparing the different approaches
< NicolasDorier> this also make it harder for us to break users of consensus
< NicolasDorier> because we would make no assumption on how they deal with their storage
< jtimon> in my model, there was no processBlock, just verifyBlock that tells you whether a block is valid or not and nothing else: the caller needs to update the storage and manage reorg and the like on his own
< NicolasDorier> one method is not enough, you need also to verify PoW for example. The client wants to verify PoW before pulling out the UTXOs from storage
< jtimon> right, that's an advantage for both the abstracted storage API or the abstracted storage by prefetching
< jtimon> the people that don't like to abstract the storage (at least greg and pieter), are happy with libconsensus depending on levelDB
< NicolasDorier> I don't think libconsensus should have any dependencies
< NicolasDorier> it should be plain dumb lib like libsecpk)#*R#)*$
< jtimon> so you would expose a function to verify pow before fetching the data? why just pow? there's many things that can be validated without storage
< NicolasDorier> yes other things as well
< NicolasDorier> like CheckContextual *
< jtimon> well, I think at the very least it should have libsecp256k1 as a dependency, but I agree it shouldn
< NicolasDorier> yes
< jtimon> 't depend on levelDB
< jtimon> just pointing out that other people disagree
< NicolasDorier> yes
< jtimon> so maybe you would expose checkblock and verifyBlock separately
< NicolasDorier> but what I plan to do is making it the way I see, using it with my C# full node, and if people are happy with it proposing to Core
< jtimon> makes sense, I thought about it
< jtimon> do you plan to expose lower level functions like say verifyHeader or verifyTx ?
< NicolasDorier> mh depends if my full node need it. VerifyHeader yes, VerifyTx might be good for mempool, but the problem is that it should not check policy rules
< NicolasDorier> it would
< jtimon> yeah, I think I will finish my vision on top of 0.14 and then see what happens
< NicolasDorier> it would not check policy rules
< jtimon> I was kind of doing that for 0.12 but I decided I would wait for bip9 and segwit instead
< NicolasDorier> I am tempted to still use my C# script validator for mempool stuff
< NicolasDorier> not including verifyTx
< jtimon> right, verifyTx would not check policy rules, perhaps we can have a verifyAndAcceptTx function too, but that's more bitcoin core specific I think
< jtimon> C# script validator? you don't use the verifyScript in current libconsensus ?
< NicolasDorier> NBitcoin has its own script validator yes
< jtimon> libbitcoin has his own validator as well, but optionally they allow you to use libconsensus instead
< NicolasDorier> right now the full node I did does not depends on libconsensus, but I want people to be able to activate it if they want
< NicolasDorier> the user can turn it for the script validation as well if he wants, but I want to verify the full block with libconsensus
< jtimon> is there any reason why you don't use libconsensus::verifyScript in nbitcoin ? do you plan to do it later?
< NicolasDorier> you can use it
< NicolasDorier> I do not by default because there is deployment headache
< NicolasDorier> pure C# code just run everywhere
< jtimon> oh, so then NBitcoin optionally depends on libconsensus, got it
< NicolasDorier> yes
< NicolasDorier> I will also add a mode where you setup a trusted node, and just do not verify the blocks.
< jtimon> what about caches? what do you plan for caches? bip9 cache, sigcache?
< jtimon> I mean, how do you plan to handle that with your libconsensus ?
< NicolasDorier> not yes thought about it. BIP9 I think the easiest at beginning is that the user does it. I plan in v1 that the user will pass the consensus flags to activate
< NicolasDorier> so libconsensus will not depends on CBlockIndex
< NicolasDorier> hopefully, I would like to make that into libconsensus eventually though
< jtimon> he has to count the 95% in the last diff adjustment period manually?
< NicolasDorier> for the other cache, I have not decided yet I dont know
< NicolasDorier> yes
< NicolasDorier> not perfect for sure
< jtimon> I guess no approach is perfect, all have drawbacks and advantages
< jtimon> I don't think I have any more questions, thank you!
< NicolasDorier> I think it is possible to make it part of libconsensus, but not yet thought about it, I would like a v1 without CBlockIndex
< jtimon> and btw, congrats on completing a full node alt implementation, not an easy task
< NicolasDorier> thanks, still stuff to do though. But happy with the results so far :D
< NicolasDorier> it is compatible with config file of Core, and has also rpc server
< NicolasDorier> my goal is to reuse the test suite of bitcoin core
< NicolasDorier> as is
< NicolasDorier> to test my implementation
< jtimon> in my approach I planned to expose a getConsensusFlags, but it would depend on a function_pointer-based API for CBlockIndex (the same I was using for verifyHeader in #8493 )
< gribble> https://github.com/bitcoin/bitcoin/issues/8493 | Untested: libconsensus: Expose VerifyHeader by jtimon · Pull Request #8493 · bitcoin/bitcoin · GitHub
< NicolasDorier> yes I remember
< jtimon> reusing bitcoin core's test suite is very interesting
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/5754e0341b7c...bbf193fef049
< bitcoin-git> bitcoin/master 67ca130 Cory Fields: build: fix for out-of-tree/distdir qt builds
< bitcoin-git> bitcoin/master bbf193f Wladimir J. van der Laan: Merge #9513: build: fix qt distdir builds (retry)...
< bitcoin-git> [bitcoin] laanwj closed pull request #9513: build: fix qt distdir builds (retry) (master...out-of-tree-build) https://github.com/bitcoin/bitcoin/pull/9513
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/bbf193fef049...593a00ce1935
< bitcoin-git> bitcoin/master 74994c6 Pieter Wuille: Improve style w.r.t. if
< bitcoin-git> bitcoin/master 593a00c Wladimir J. van der Laan: Merge #9506: RFC: Improve style for if indentation...
< bitcoin-git> [bitcoin] laanwj closed pull request #9506: RFC: Improve style for if indentation (master...newstyle) https://github.com/bitcoin/bitcoin/pull/9506
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/593a00ce1935...ca615e6c05ef
< bitcoin-git> bitcoin/master 8217bd1 fanquake: [depends] libevent 2.1.7rc
< bitcoin-git> bitcoin/master ca615e6 Wladimir J. van der Laan: Merge #9471: [depends] libevent 2.1.7rc...
< bitcoin-git> [bitcoin] laanwj closed pull request #9471: [depends] libevent 2.1.7rc (master...depends-0-14-0-libevent) https://github.com/bitcoin/bitcoin/pull/9471
< wumpus> are there any remaining issues with #9375?
< gribble> https://github.com/bitcoin/bitcoin/issues/9375 | Relay compact block messages prior to full block connection by TheBlueMatt · Pull Request #9375 · bitcoin/bitcoin · GitHub
< instagibbs> same q for #9400
< gribble> https://github.com/bitcoin/bitcoin/issues/9400 | Set peers as HB peers upon full block validation by instagibbs · Pull Request #9400 · bitcoin/bitcoin · GitHub
< sipa> wumpus: i was planning to review 9375 today
< wumpus> sipa: okay, great
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/ca615e6c05ef...e2e624d9ce54
< bitcoin-git> bitcoin/master 1fc4ec7 mrbandrews: Add pruneblockchain RPC to enable manual block file pruning.
< bitcoin-git> bitcoin/master afffeea Russell Yanofsky: fixup! Add pruneblockchain RPC to enable manual block file pruning....
< bitcoin-git> bitcoin/master e2e624d Wladimir J. van der Laan: Merge #7871: Manual block file pruning....
< bitcoin-git> [bitcoin] kallewoof opened pull request #9517: [refactor] Switched httpserver.cpp to use RAII wrapped libevents. (master...raii-httpserver) https://github.com/bitcoin/bitcoin/pull/9517
< sipa> i wonder why unique_ptr does not have an emplace method like the stl containers have
< sipa> would be a neat and cheap way to avoid explicit new calls
< jtimon> oh, I didn't know about contrib/devtools/check-doc.py
< cfields> sipa: see std::make_unique in c++14. I assume that's what you mean?
< gmaxwell> Lets upgreade to C++14! :P
< sipa> cfields: but x.emplace(a,b) still looks nicer than x = std::make_unique<x_t>(a,b);
< * luke-jr> always found emplace to look ugly, but doesn't care enough to argue it
< cfields> sipa: ah, i see what you mean. seems a bit clumsy to me though, as emplace implies that you're constructing a new value in place, rather than replacing a (the) current one
< profall> Anyone have a link to the latest blockchain torrent?
< sipa> profall: the blockchain torrent has been discontinued (at least by the bitcoin core maintainers) since 0.10
< profall> ah ok
< sipa> as the bitcoin block download code is usually faster
< sipa> (it validates while downloading)
< profall> yea
< profall> I am still experiencing issues with my node randomly dropping out of sync every few hours
< profall> I thought maybe a fresh resync might fix something, not sure.
< sipa> define 'dropping out of sync'
< profall> It'll be on the correct block if compared to blockchain.info or any other explorer. Then I will come back a few hours later and it'll be stuck on some block from 2 hours ago and not in sync anymore.
< profall> This is an E3 processor, 100mbps connection in a datacenter, 16GB ram server. Not resource starved.
< sipa> anything in debug.log?
< profall> ping timeout 1200s
< profall> socket recv error Connection reset by peer (104)
< sipa> that's normal
< sipa> what version?
< sipa> of bitcoin core
< gmaxwell> well it's not normal if it happens to all peers?
< profall> 0.13.2 on Linux
< profall> I downloaded it straight from the website rather then use the repo, just in case as well.
< gmaxwell> profall: can you send one of us a copy of your debug log including one of these incidents? the debug log will identify your IP and potentially some of your transactions if you're using it as a wallet.
< sipa> could you share a few hours worth of debug.log somewhere?
< profall> Sure, ill PM you just give me a few moments to prepare it.
< profall> PM'd both of you with debug.log
< profall> Oops, realized it's empty...
< luke-jr> morcos: c0507f800475edf003adb744061d864741d3ee12796834d4d7f9a72b0bbd4fe6 is the only one on your list that shows up in my debug.log
< luke-jr> 2017-01-10 21:23:32 not keeping orphan with rejected parents c0507f800475edf003adb744061d864741d3ee1279
< luke-jr> And its parent was rejected because… 2017-01-10 21:23:32 d97b0571f984420ffebb8f4e69e3d1ed1467797105ad602747ff1ddff322ff40 from peer=14 was not accepted: bad-txns-inputs-spent (code 18)
< luke-jr> looks like a double-spend? … so the competing parent probably had enough fee for the mempool, but not enough to be mined, and d97b0571f9 didn't have enough more to replace it
< * luke-jr> wonders if this might be exploitable
< profall> Resent
< gmaxwell> BlueMatt: can you confirm that the new addnode behavior in master is sufficient to get rid of your fibre proxy thing?
< BlueMatt> gmaxwell: based on what you wrote in your pull, yes
< gmaxwell> okay, so if it does what it says on the tin you think its good to go. fine with me.
< BlueMatt> I'd need to go re-test to confirm, but I did test pre-merge
< BlueMatt> yea
< gmaxwell> (I just wanted to make sure if you needed any other features that I wrote them ASAP.)
< BlueMatt> heh, yea, all good thanks
< BlueMatt> in other news, I'm avoiding review because sick, and super pissed off about this :(
< sipa> pissed about being sick?
< BlueMatt> both - i have so much review to do :(
< sipa> should -maxmempool=0 imply -blocksonly ?
< gmaxwell> Probably, though blocksonly is still a hidden option.
< sipa> ah, really
< gmaxwell> It's hidden because it would probably be better if we had a service bit related to it before having lots of nodes running it.
< gmaxwell> probably thats too conservative, it's unlikely lots of nodes would run it.
< cfields> BlueMatt: heh, i'm very behind on review as well. slowly making my way through yours.
< sipa> likewise
< instagibbs> Someone have the list of high-priority review on hand
< sipa> i think we really want #9375 #9441 in
< gribble> https://github.com/bitcoin/bitcoin/issues/9375 | Relay compact block messages prior to full block connection by TheBlueMatt · Pull Request #9375 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/9441 | Net: Massive speedup. Net locks overhaul by theuni · Pull Request #9441 · bitcoin/bitcoin · GitHub
< cfields> BlueMatt: I've been stuck on the ActivateBestChain() kludge for quite a while now. I don't like it, but with no better argument than "it's ugly". Would it be reasonable instead to maintain a list of announced-but-not-stored hashes, combined with maybe just a quick hack like WaitForValidation(hash) ?
< cfields> (i assume that question has been posed already)
< BlueMatt> cfields: if you announce a block which turns out to be invalid, WaitForValidation wont help :/
< instagibbs> Ok, I really cant comment on the net locks overhaul but I'll re-review the former
< cfields> BlueMatt: well presumably it'd drop out of the list, valid or no
< cfields> BlueMatt: i suppose at an even higher level it could just be WaitForBestChainActivated()
< BlueMatt> cfields: i mean the ActivateBestChain kludge is only there for the case where you caught the cs_main lock right in the middle of ProcessNewBlock where it unlocks cs_main for a sec
< BlueMatt> ActivateBestChain literally is WaitForActivateBestChainActivated().....
< BlueMatt> its just the version for when you're already holding cs_main
< cfields> BlueMatt: understood. Just seems scary to allow that to be manipulated so easily. Like I said though, I can't come up with any realistic issue with it.
< BlueMatt> "manipulated"?
< BlueMatt> I mean I hope that its pretty much free if you're already on the best chain
< bitcoin-git> [bitcoin] ryanofsky opened pull request #9518: Return height of last block pruned by pruneblockchain RPC (master...pr/prune-height) https://github.com/bitcoin/bitcoin/pull/9518
< sipa> ryanofsky: feel like having a look at https://github.com/sipa/bitcoin/commits/pertxoutcache ?
< gmaxwell> I will be travling during the meeting tomorrow.
< cfields> BlueMatt: as an example of an unintended side-effect: miner mines a block, hands it to ProcessNewBlock, ProcessMessages (incoming GETBLOCKS) swoops in _just_ after AcceptBlock and runs ActivateBestChain, before the mining thread gets a chance to. As a result, miner has to read the block from disk
< cfields> BlueMatt: trivial and unlikely, but still a side-effect :\
< cfields> rather, the processor reads from disk.
< morcos> luke-jr: that was a replay, that tx was mined on 01-03
< bitcoin-git> [bitcoin] morcos opened pull request #9519: Exclude RBF replacement txs from fee estimation (master...excludeRBF) https://github.com/bitcoin/bitcoin/pull/9519
< bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/e2e624d9ce54...05950427d310
< bitcoin-git> bitcoin/master fe7e593 Suhas Daftuar: Fix use-after-free in CTxMemPool::removeConflicts()
< bitcoin-git> bitcoin/master 0595042 Pieter Wuille: Merge #9507: Fix use-after-free in CTxMemPool::removeConflicts()...
< bitcoin-git> [bitcoin] sipa closed pull request #9507: Fix use-after-free in CTxMemPool::removeConflicts() (master...fix-mempool-useafterfree) https://github.com/bitcoin/bitcoin/pull/9507
< BlueMatt> cfields: I suppose I'm not too concerned by that kind of super-crazy race....in the future we probably do want to cache stuff like that more
< cfields> BlueMatt: i'm not at all concerned by it. My only concern is that it's entirely non-obvious what the side-effects are. And that they're bound to change in the future.
< cfields> BlueMatt: The first path I was chasing down was whether or not, in the same scenario, the miner's validationinterface would only receive a null pointer (in the case of the dummy ActivateBestChain) rather than the real block. That's obviously not the case. My fear, though, is that if that _were_ the case, we might not have noticed.
< BlueMatt> yes, I did check for that when writing the code :p
< BlueMatt> cfields: I think the (real) solution is to make callbacks out of ValidationInterface go in a background thread, and make ProcessNewBlock hold the cs_main lock the whole time
< BlueMatt> but that is definitely not a 0.14 refactor
< gmaxwell> great, but would I know to enforce that invariant if I were changing the code later?
< cfields> ^^ what he said :)
< BlueMatt> wait, which callback are you referring to?
< BlueMatt> it was already the case that ActivateBestChain could be called "loose" and connect a block, so I dont think I changed the requirements on ABS?
< cfields> ABS ?
< BlueMatt> activatebestchain
< BlueMatt> but the version that comes out when I've got a head-cold :p
< cfields> ah, ActivateBeStchain :p
< BlueMatt> yea, that
< sipa> Next PR: "Correct ActivateBestChain to ActivateBestShain"
< cfields> no, that didn't change, but there's now a publicly triggerable way to (try to) force an ABS between ProcessBlock and the ABS below it
< BlueMatt> mmm, fair enough
< cfields> sipa: heh. Blockshain all the things!
< sipa> since 9 days ago, the memory usage of mapBlockIndex exceeded 100M
< sipa> it's using 224 bytes per entry
< sipa> some of which could be avoided
< BlueMatt> yes, lets fix that
< gmaxwell> not that much could be avoided though.
< sipa> it's using 2 allocations per entry
< sipa> (which are included in the 224 number)
< BlueMatt> wait, it is?
< sipa> it should be 0
< BlueMatt> gmaxwell: like 20 bytes per entry last I checked
< sipa> but at least 1 should be trivial to avoid
< sipa> using a map<uint256, CBlockIndex> instead of map<uint256, CBlockIndex*>
< gmaxwell> sipa: how can it be zero? the data structure grows and things keep around pointers to it? no.
< sipa> gmaxwell: direct hashtable
< sipa> and using map::iterators instead of CBlockIndex* all over the place
< gmaxwell> okay, perhaps we don't actually keep any of the pointers.
< sipa> but if we're going to do that, i'd want a proper encapsulation too, that has its own lock, and has accessor methods for the fields that can be accessed without lock
< sipa> ... post 0.14
< gmaxwell> sipa: well if the work is done to change how its accessed it should be encapsulated enough so that it could be diskbacked without changing any of the code.
< gmaxwell> even with all the overheads gone, it's a significant chunk of memory.
< sipa> right
< sipa> ideally we'd also split up the disk-storage part and the validity-related part
< sipa> into two structures
< bitcoin-git> [bitcoin] sipa closed pull request #8170: Remove lookup-tx-by-utxo functionality (master...betternotxindex) https://github.com/bitcoin/bitcoin/pull/8170
< bitcoin-git> [bitcoin] sipa opened pull request #9520: Deprecate non-txindex getrawtransaction and better warning (master...warntxindex) https://github.com/bitcoin/bitcoin/pull/9520
< sipa> some review on #9261 plz?
< gribble> https://github.com/bitcoin/bitcoin/issues/9261 | Add unstored orphans with rejected parents to recentRejects by morcos · Pull Request #9261 · bitcoin/bitcoin · GitHub