< bitcoin-git> [bitcoin] jimpo opened pull request #13033: Build txindex in parallel with validation (master...txindex-refactor-take2) https://github.com/bitcoin/bitcoin/pull/13033
< bitcoin-git> [bitcoin] jimpo closed pull request #11857: Build tx index in parallel with validation (master...txindex-refactor) https://github.com/bitcoin/bitcoin/pull/11857
< achow101> github responded to me about 11857 and they told me that they're looking into it. apparently some other PRs in other projects are seeing similar issues
< bitcoin-git> [bitcoin] promag opened pull request #13034: Introduce WalletManager (master...2018-04-walletmanager) https://github.com/bitcoin/bitcoin/pull/13034
< jimpo> ajtown: Nice tip on --color-moved. TIL.
< aj> jimpo: pfft, old news. i've known about it for days!
< sipa> what is --color-moved?!
< jimpo> aj: hahaha
< aj> sipa: "git diff HEAD^ --color-moved=zebra"
< sipa> doesn't work here
< aj> sipa: i found it while looking for easier ways to review #12885
< gribble> https://github.com/bitcoin/bitcoin/issues/12885 | Reduce implementation code inside CScript by sipa · Pull Request #12885 · bitcoin/bitcoin · GitHub
< aj> sipa: needs to be run on a moveonly-type commit, it uses colours so might depend on your terminal
< kallewoof> Oooh, neat
< sipa> i use git diff --patience HEAD~:src/file.cpp HEAD:src/newfile.cpp
< bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/0a8b7b4b33c9...8b262eb2d80b
< bitcoin-git> bitcoin/master b77b6e2 Jim Posen: MOVEONLY: Move logging code from util.{h,cpp} to new files.
< bitcoin-git> bitcoin/master 8b262eb Pieter Wuille: Merge #13021: MOVEONLY: Move logging code from util.{h,cpp} to new files....
< bitcoin-git> [bitcoin] sipa closed pull request #13021: MOVEONLY: Move logging code from util.{h,cpp} to new files. (master...logging-files) https://github.com/bitcoin/bitcoin/pull/13021
< aj> ah, needs git 2.15 which came out octoberish 2017
< sipa> seems i have 2.14
< bitcoin-git> [bitcoin] macraix opened pull request #13036: PicoStocks 100TH problem (master...master) https://github.com/bitcoin/bitcoin/pull/13036
< bitcoin-git> [bitcoin] fanquake closed pull request #13036: PicoStocks 100TH problem (master...master) https://github.com/bitcoin/bitcoin/pull/13036
<@wumpus> --color-moved=zebra? can we pick other animals too?
< * wumpus> looks at his git version... 2.7.4.. oh alright, maybe in a few years
< promag> wumpus: hi, do you think you can check #13017?
< gribble> https://github.com/bitcoin/bitcoin/issues/13017 | Add wallets management functions by promag · Pull Request #13017 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] charkala opened pull request #13037: Genesis (master...genesis) https://github.com/bitcoin/bitcoin/pull/13037
< bitcoin-git> [bitcoin] charkala closed pull request #13037: Genesis (master...genesis) https://github.com/bitcoin/bitcoin/pull/13037
< meshcollider> wumpus do you want to merge https://github.com/bitcoin-core/bitcoincore.org/pull/528 now its had plenty of review and has been sitting there for a while
<@wumpus> meshcollider: will have a look, thanks
< promag> wumpus: not sure if I understand your argument, currently CWalletRef is narrowly used. Depending on the case it can be acceptable to use either reference, raw pointer or shared pointer. When adding shared pointer I would like to review all places where it makes sense to use it. If we had CWalletRef everywhere then that would obscure that evaluation.
<@wumpus> promag: it's usually preferable to keep diffs minimal, without a type such as CWalletRef it means every usage has to be changed if the underlying type changes, the whole reason typedefs exist...
<@wumpus> most users of CWalletRef won't care how it works internally
<@wumpus> they just want to refer to a wallet
< promag> but my point is that not all CWallet* should be std::shared_ptr
<@wumpus> maybe not, i still think an abstract reference type makes sense
< promag> I think that makes sense if CWalletRef was used where it makes sense to switch to shared pointer, but that's not the case
<@wumpus> yes I agree they don't all need to be replaced
<@wumpus> what annoyed me is the back and forth, one refactor introduces a certain type then the next one removes it again
< promag> even if I keep CWalletRef, switching to std::shared_ptr will cause a (not that) large diff
<@wumpus> so let's just get on one page first
< promag> typedef aside, the code base must be refactored to shared pointers
<@wumpus> my point was not 'this is a bad idea' but 'this needs discussion with the people involved'
<@wumpus> if there is general agreement to undo the CWalletRef type again, that's ok with me
< promag> wumpus: fair enough, I can revert it too. I should have predicted this :D
<@wumpus> though it makes me wonder what reasoning introduced it and what changed since
< promag> let me blame it
<@wumpus> now that's a good idea
< promag> introduced in #8694 by luke-jr
< gribble> https://github.com/bitcoin/bitcoin/issues/8694 | Basic multiwallet support by luke-jr · Pull Request #8694 · bitcoin/bitcoin · GitHub
<@wumpus> let's try to get luke-jr on board there, then
< promag> wumpus: I guess from that PR that there was little discussion
< promag> right
< bitcoin-git> [bitcoin] laanwj opened pull request #13039: Add logging and error handling for file syncing (master...2018_04_fsync_noignore) https://github.com/bitcoin/bitcoin/pull/13039
< bitcoin-git> [bitcoin] AsKayDevs opened pull request #13040: Update README.md (master...master) https://github.com/bitcoin/bitcoin/pull/13040
< bitcoin-git> [bitcoin] practicalswift opened pull request #13041: build: Add linter checking for accidental introduction of locale dependence (master...lint-locale-dependence) https://github.com/bitcoin/bitcoin/pull/13041
< bitcoin-git> [bitcoin] fanquake closed pull request #13040: Update README.md (master...master) https://github.com/bitcoin/bitcoin/pull/13040
< bitcoin-git> [bitcoin] qshuai opened pull request #13042: Calculated nBits will be replaced by the following GetNextWorkRequire… (master...master) https://github.com/bitcoin/bitcoin/pull/13042
< bitcoin-git> [bitcoin] Sjors opened pull request #13043: [qt] OptionsDialog: add prune setting (master...2018/04/qt-prune) https://github.com/bitcoin/bitcoin/pull/13043
< cfields> wumpus: with our powers combined, we've finally arrived at &= :)
< cfields> no clue what I was thinking there.
< BlueMatt> sipa: whats your current feeling on caching witness hashes/the sighash components somewhere on mempool acceptance?
< sipa> BlueMatt: caching sure, but not in the same class that is used for everything
< BlueMatt> yea, ok, fair
< BlueMatt> I thought that was rejected on the first go around :/
< BlueMatt> wumpus: re: #12998: I'm open to doing whatever to fix it, that was just the simplest thing (fix it for the narrow case when those functions are #define'd by the compiler's headers). cfields suggested moving those functions to _int, but I'm not sure making a downstream project's build scripts easier is worth the refactor for that, but up to y'all
< gribble> https://github.com/bitcoin/bitcoin/issues/12998 | Default to defining endian-conversion DECLs in compat w/o config by TheBlueMatt · Pull Request #12998 · bitcoin/bitcoin · GitHub
< BlueMatt> I just wanna figure out if I should just apply that patch downstream or what
< jcohen> i have a half completed change somewhere that makes proper types out of TxId / WTxId instead of using uint256 somewhere - i can clean up if there's to be more pervasive use of witness ids - doesn't do anything other than readability / compiler safety
< luke-jr> wumpus: CWalletRef was intentionally introduced such that it can be replaced with a shared_ptr painlessly
< luke-jr> I probably still have a branch with that change
< bitcoin-git> [bitcoin] instagibbs opened pull request #13045: [p2p] getblock for 1-block reorgs in response to compact block message (master...cmpcttie) https://github.com/bitcoin/bitcoin/pull/13045
< cfields> BlueMatt: I think it's fine as-is. But if you want to take it further, I think it'd be pretty trivial to switch to _int.
< bitcoin-git> [bitcoin] skeees opened pull request #13046: [doc][trivial] no retargeting in regtest mode (master...doc-update) https://github.com/bitcoin/bitcoin/pull/13046
< bitcoin-git> [bitcoin] jnewbery opened pull request #13047: [trivial] Tidy blocktools.py (master...tidy_blocktools) https://github.com/bitcoin/bitcoin/pull/13047
< jnewbery> 13017 doesn't make replacing the raw pointers with shared_ptrs and easier or more difficult, so I don't think it really needs too much discussion.
< jimpo> Can I get a look on #12647? It's pretty trivial.
< gribble> https://github.com/bitcoin/bitcoin/issues/12647 | wallet: Fix possible memory leak in CreateWalletFromFile. by jimpo · Pull Request #12647 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] jnewbery opened pull request #13048: [tests] Fix feature_block flakiness (master...fix_feature_block_flakiness) https://github.com/bitcoin/bitcoin/pull/13048
< jtimon> btw aj cool your new test cached a bug
< promag> luke-jr: mind checking for the shared pointer branch, I would like to see it? There are lot of places where CWallet* must be changed to CWalletRef or std::shared_ptr<CWallet>, and IMO there are some places where CWalletRef should be CWallet* IMO. I've dropped CWalletRef so that the PR introducing shared_ptr would do it in the whole source.
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #13049: [0.16] qa: Backports (0.16...Mf1804-qa16Backports) https://github.com/bitcoin/bitcoin/pull/13049