< bitcoin-git> [bitcoin] Empact opened pull request #13234: Break circular dependency: chain -> pow -> chain (master...chain-pow-chain) https://github.com/bitcoin/bitcoin/pull/13234
< gmaxwell> re: 13191 is there no gain from making a 64-byte specalized sha256^2 that is 1-way SSE4?
< sipa> gmaxwell: there is a pure C++ one
< sipa> there is probably a way to combine the advantages of the 1-way SSE4 code and the fixed-64-byte input one
< sipa> but since that SSE4 code is transpiled intel asm code, it's not trivial
< gmaxwell> sipa: IIRC the 64-byte specialized non-asm code was a fair bit slower than the sse4 non-64-byte-specialized code when tested before.
< sipa> gmaxwell: correct, which is why the sse4 1-way non-64 code is used on sse systems when there are no hashes simultaneously
< sipa> but a combined sse4-1way-64byte should presumably be even faster
< bitcoin-git> [bitcoin] Empact opened pull request #13235: Break circular dependency: init -> * -> init by extracting shutdown.h (master...init-circ-init) https://github.com/bitcoin/bitcoin/pull/13235
< savantgarde> Thanks promag ๐Ÿ™‚
< wumpus> the board running my IRC client died yesterday, so I was gone for a while and have no logs. Currently using a temporary workaround hosting elsewhere. Did I miss anything?
< aj> wumpus: bluematt pinged you and fanquake mentioned #13093 at you i think
< gribble> https://github.com/bitcoin/bitcoin/issues/13093 | [0.15] backport: depends qt patches by fanquake ยท Pull Request #13093 ยท bitcoin/bitcoin ยท GitHub
< wumpus> aj: thanks!
< bitcoin-git> [bitcoin] laanwj pushed 3 new commits to 0.15: https://github.com/bitcoin/bitcoin/compare/cb7ef312ff34...1618c63095db
< bitcoin-git> bitcoin/0.15 9bb1a16 fanquake: [Depends] Fix Qt build with Xcode 9.2...
< bitcoin-git> bitcoin/0.15 93b9a61 fanquake: depends: Fix Qt build with XCode 9.3...
< bitcoin-git> bitcoin/0.15 1618c63 Wladimir J. van der Laan: Merge #13093: [0.15] backport: depends qt patches...
< fanquake> thanks aj
< fanquake> wumpus I just assumed you'd finally started to ignore me :p
< wumpus> fanquake: you were talking to a ghost :)
< fanquake> :o well if the real wumpus is back, I think #13234 can go in
< gribble> https://github.com/bitcoin/bitcoin/issues/13234 | Break circular dependency: chain -> pow -> chain by Empact ยท Pull Request #13234 ยท bitcoin/bitcoin ยท GitHub
< wumpus> fanquake: the real wumpus, or at least a newly-checked out clone. But I'll take a look.
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/81c533c6f481...3b84ebb5bc0d
< bitcoin-git> bitcoin/master 5b35b92 Ben Woosley: Break circular dependency: chain -> pow -> chain...
< bitcoin-git> bitcoin/master 3b84ebb Wladimir J. van der Laan: Merge #13234: Break circular dependency: chain -> pow -> chain...
< bitcoin-git> [bitcoin] laanwj closed pull request #13234: Break circular dependency: chain -> pow -> chain (master...chain-pow-chain) https://github.com/bitcoin/bitcoin/pull/13234
< bitcoin-git> [bitcoin] kallewoof closed pull request #12966: [WIP] Mempool optimized feerate (master...mempool-optimized-feerate) https://github.com/bitcoin/bitcoin/pull/12966
< kallewoof> wumpus: Is it possible to add #12634 to the high priority list? It would be very helpful in trimming down the output group (bundle address reuse UTXO) PR.
< gribble> https://github.com/bitcoin/bitcoin/issues/12634 | [refactor] Make TransactionWithinChainLimit more flexible by kallewoof ยท Pull Request #12634 ยท bitcoin/bitcoin ยท GitHub
< harding> jonasschnelli, wumpus: for the website, the GitHub is setup so that I can't merge my own PRs unless they have an "Approve" review from someone else with commit access. The following PR numbers there have ACKs from known contributors and so are just waiting for someone besides me with commit access to Approve them: 546, 547, 550, 551, 552, and 553.
< fanquake> kallewoof I've added it
< kallewoof> fanquake: Thanks
< murrayn> any chance
< murrayn> of getting some feedback on 12881?
< promag> murrayn: i think it's mergeable
< murrayn> yes i saw your utACK, thanks
< promag> murrayn: np, you can pay me by reviewing #13193 x)
< gribble> https://github.com/bitcoin/bitcoin/issues/13193 | Avoid 2nd mapTx lookup in CTxMemPool::UpdateTransactionsFromBlock by promag ยท Pull Request #13193 ยท bitcoin/bitcoin ยท GitHub
< wumpus> kallewoof: sure - I think it's usually most helpful to discuss the high priority list during the meeting, but we can do it in-between if you're in a hurry. Oh I see someone already added it anyway :)
< wumpus> harding: yes I'm extremely behind on website review, will take a look!
< kallewoof> wumpus: Yeah, I just tend to not be awake 4 am. I am trying to pull it off though. Maybe this week...
< wumpus> kallewoof: yes that's a crazy time, if you can't make it, no problem
< kallewoof> wumpus: It should be doable, but I tend to wake up late. 4 am is usually a few hours after I fall asleep :)
< wumpus> murrayn: I haven't looked at it since my comment about locale-specific functions, will re-review
< wumpus> murrayn: what was your motivation for optimizing bech32::Decode? is this on any critical path?
< bitcoin-git> [bitcoin] ken2812221 opened pull request #13236: break circular dependency: random -> util -> random (master...random_util) https://github.com/bitcoin/bitcoin/pull/13236
< wumpus> harding: looks like the problem is also that people are using ACKs, not the approval system
< wumpus> should probably become routine in that repository that people who utACK/ACK also 'approve' the PR
< wumpus> unfortunately github doesn't count in-text ACKs
< murrayn> promag, i have reviewed 13193
< murrayn> wumpus, the original motivation was just something that caught my eye as an optimization.
< wumpus> murrayn: right, I see your point, but at some point I wonder about the cognitive load of reviewing that a change is correct versus the reason to make it
< wumpus> murrayn: so your conclusion in https://github.com/bitcoin/bitcoin/pull/12881#issuecomment-383039089 is pretty much "it doesn't matter if compiler optimizations are enabled"
< murrayn> wumpus, yes i see your point re: the cognitive load. in this case the change and its effect is very localized.
< wumpus> there's certainly a place for micro-optimizations, but I'm not sure this is it
< wumpus> I agree it's a very localized change, sure
< harding> wumpus: true about ACKs versus Approvals, although I think most of the ACKs on those PRs are from people without commit access, so GitHub doesn't wouldn't count their Approves towards unlocking mergability. That's a bit unfortunate to my mind, as I think it somewhat discourages people who don't alreday have commit access from reviewing.
< wumpus> murrayn: anyhow I'm going to ACK it - but please next time if you optimize, try to find something that is on the critical path (e.g. validation) and can clearly demonstrate improvement
< wumpus> murrayn: and this makes the code somewhat easier to read, that's good
< wumpus> harding: oh, I didn't know only approvals of repo members counted. Maybe we should remove this github-side requirement and leave it up to the committer to count the ACKs?
< murrayn> wumpus, ACK. as sipa pointed out, it's not an important function to optimize, so it does come down to readability.
< wumpus> murrayn: apart from validation there's some other things that would benefit from optimization: for example some GUI things with regard to handling lots of transactions/addresses, network code, RPC/web server latency
< wumpus> handling of large wallets
< murrayn> wumpus, i'm working on it, getting my feet wet :-)
< harding> wumpus: I'm in favor of that. I initially thought it was a security requirement, and it makes sense to me that you'd want to prevent any person from unilaterally pushing to the master branch for a live-deployed website. But then I realzed how easy it would be to circumvent by creating a secondary account to open PRs from, so I'd prefer to indeed just do the usual 'count PRs from known contributors and ensure there are enough
< harding> given the impact of the PR'.
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/3b84ebb5bc0d...1d4662f5dcf6
< bitcoin-git> bitcoin/master 60f61f9 murrayn: Tighten up bech32::Decode(); add tests.
< bitcoin-git> bitcoin/master 1d4662f Wladimir J. van der Laan: Merge #12881: Minor optimizations to bech32::Decode(); add tests....
< bitcoin-git> [bitcoin] laanwj closed pull request #12881: Minor optimizations to bech32::Decode(); add tests. (master...bech32_decode) https://github.com/bitcoin/bitcoin/pull/12881
< wumpus> harding: yes from a security point of view it makes some sense, espeically if only repo members are counted
< wumpus> harding: so are you sure only people with *write access* count, or everyone who is part of the repository team, also read-only members? if the latter we could just add everyone from the bitcoincore organization in it
< harding> wumpus: good question. The GitHub UI says, "At least 1 approving review is required by reviewers with write access." Looking deeper at the documentation, it seems to confirm that, and I think that's what I observed for one of MarcoFalke's Approves (that the UI still said I needed a review by someone with write access).
< harding> wumpus: in any case, thanks for all your reviews this morning!
< wumpus> harding: np, feel free to poke me when you need review for the website again
< mess110> hi, can I please get some reviews on https://github.com/bitcoin/bitcoin/pull/11491 ?
< wumpus> mess110: yes
< provoostenator> Two PRs with baby steps towards making RBF easier to use: #12096, #12818
< gribble> https://github.com/bitcoin/bitcoin/issues/12096 | [rpc] [wallet] Allow specifying the output index when using bumpfee by kallewoof ยท Pull Request #12096 ยท bitcoin/bitcoin ยท GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/12818 | [qt] TransactionView: highlight replacement tx after fee bump by Sjors ยท Pull Request #12818 ยท bitcoin/bitcoin ยท GitHub
< bitcoin-git> [bitcoin] Sjors closed pull request #12565: [qt] bumpfee: offer user to reduce output for transactions without change (master...2018/02/qt-bumpfee-reduce-output) https://github.com/bitcoin/bitcoin/pull/12565
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/1d4662f5dcf6...13da2899ae42
< bitcoin-git> bitcoin/master 244f4ba practicalswift: scheduler: Add Clang thread safety annotations for variables guarded by m_cs_callbacks_pending
< bitcoin-git> bitcoin/master 13da289 MarcoFalke: Merge #13125: scheduler: Add Clang thread safety annotations for variables guarded by m_cs_callbacks_pending...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #13125: scheduler: Add Clang thread safety annotations for variables guarded by m_cs_callbacks_pending (master...m_cs_callbacks_pending) https://github.com/bitcoin/bitcoin/pull/13125
< promag> I guess could be merged #10757
< gribble> https://github.com/bitcoin/bitcoin/issues/10757 | RPC: Introduce getblockstats to plot things by jtimon ยท Pull Request #10757 ยท bitcoin/bitcoin ยท GitHub
< promag> jnewbery: ping
< jnewbery> promag: I'm here
< promag> jnewbery: I see you are already checking out #10740
< gribble> https://github.com/bitcoin/bitcoin/issues/10740 | [wallet] `loadwallet` RPC - load wallet at runtime by jnewbery ยท Pull Request #10740 ยท bitcoin/bitcoin ยท GitHub
< provoostenator> I have two nodes on master and a v0.16.0 which all seem to refuse to relay 1 sat / byte transactions, despite "minrelaytxfee": 0.00010000 ...
< provoostenator> The nodes on master don't throw any errors when you use the GUI or sendrawtransaction. The 0.16.0 node however compalins that the minimum relay fee isn't met.
< sipa> provoostenator: 0.0001 is 10sat/byte, no?
< sipa> so not relaying 1sat/byte seems expected
< provoostenator> I set maxmempool to something really large, but on the 0.16 node I forgot to remove minrelaytxfee, which indeed set it to 10.
< provoostenator> The other two nodes have 0.000010000, but once I fixed the 0.16.0 node transactions get relayed as expected, so nvm...
< provoostenator> Different issue: I'm comparing #12404 with master on a node with 1 GB ram. I didn't confiugre dbcache, yet it OOM'd at cache=568 MiB (on master as well as my PR)
< gribble> https://github.com/bitcoin/bitcoin/issues/12404 | Prune more aggressively during IBD by Sjors ยท Pull Request #12404 ยท bitcoin/bitcoin ยท GitHub
< provoostenator> Is there room wiggle room above the default 450 MB or could this be a bug?
< sipa> there's a couple hundred MB in other things, yes
< provoostenator> Wouldn't it be better for dbache to be inclusive of these "other things"? So it would flush when the total hits 450, rather than some unpredicatable(?) point?
< provoostenator> It flushed at 716 MB (April 2013), so I'll try dbcache=200 on the 1 GB machine which leave 100 MB margin of error. This is useful too: https://gist.github.com/laanwj/efe29c7661ce9b6620a7
< provoostenator> (that last number was on a machine with more RAM, I'll keep it running to see where the other peaks are)
< sipa> provoostenator: it's very hard to account for all memory
< sipa> we could include a few easily-accountable things like block headers
< sipa> and i'm generally in favor of ways of making total memory more predictable
< provoostenator> But the log entries say "cache=223.1MiB" so at least that number is known and could be kept below dbcache, right?
< sipa> yes it should be
< provoostenator> (except during the flush itself)
< sipa> below dbcache+mempoolsize
< provoostenator> Ah, so setting maxmempool would also make it more predicatable?
< sipa> mempoolsize + dbcache should always be below -maxmempool + -dbcache
< provoostenator> Isn't the mempool empty during IBD?
< sipa> yes, that's the point
< sipa> dbcache can "borrow" the unused mempool memory
< sipa> because during IBD we have mempool memory to spare, and more dbcache helps a lot
< provoostenator> That makes sense. Would be useful to mention that in the -dbcache command documentation.
< provoostenator> Though the "reducing bitcoind memory usage" doc does mention this.
< sipa> sounds good
< sipa> I wasn't aware it wasn't mentioned
< BlueMatt> sipa (or whoever): you wanna re-ack-and-merge #13023? then we can tag 16.1rc1 =D
< gribble> https://github.com/bitcoin/bitcoin/issues/13023 | Fix some concurrency issues in ActivateBestChain() by skeees ยท Pull Request #13023 ยท bitcoin/bitcoin ยท GitHub
< BlueMatt> (the fd things I dont think make sense to wait for, they haven't made much progress that I see, unless cfields wants to object)
< skeees> =)
< bitcoin-git> [bitcoin] Empact opened pull request #13239: [moveonly] Fix CConnman template methods to be fully-defined in net.h (master...net-template-methods) https://github.com/bitcoin/bitcoin/pull/13239
< cfields> BlueMatt: whoops, never ended up getting back to that
< cfields> I'm not too worried though
< BlueMatt> cfields: yea, I'm not either
< bitcoin-git> [bitcoin] Empact opened pull request #13241: scripted-diff: Avoid temporary copies when looping over std::map (master...pair-const-key) https://github.com/bitcoin/bitcoin/pull/13241