19:00:10 #startmeeting 19:00:10 Meeting started Thu Jan 19 19:00:10 2017 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:00:10 Useful Commands: #action #agreed #help #info #idea #link #topic. 19:00:14 ello 19:00:23 cfields: If you need the gitian log for the failing build: https://bitcoin.jonasschnelli.ch/nightlybuilds/2017-01-19/build-win.log 19:00:44 MarcoFalke: ah, didn't realize gitian was actually failing. Thanks. 19:00:47 #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 instagibbs 19:00:54 here 19:00:58 Hi 19:01:11 prezent 19:01:19 topics? 19:01:20 here 19:01:27 suggested topic #9583 and #9584 19:01:28 topic: last-minute merges before feature freeze 19:01:29 https://github.com/bitcoin/bitcoin/issues/9583 | Move wallet callbacks into cs_main (this effectively reverts #7946) by TheBlueMatt · Pull Request #9583 · bitcoin/bitcoin · GitHub 19:01:30 https://github.com/bitcoin/bitcoin/issues/9584 | Synchronization problems with wallet. · Issue #9584 · bitcoin/bitcoin · GitHub 19:01:35 Last stuff to shove in before freeze naturally... 19:01:38 hi. 19:01:57 I guess https://github.com/bitcoin/bitcoin/issues/9294 is ready... 19:02:02 * btcdrak is half here 19:02:13 #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier 19:02:19 any 0.14 milestoned PRs that we don't expect are reasonable to make it? 19:02:27 suggested topic, what's missing to branch 0.14 19:02:28 #9535 got thourough review from jtimon (and others) and is a big win 19:02:28 do we all agree 9294 is ready? 19:02:30 https://github.com/bitcoin/bitcoin/issues/9535 | Split CNode::cs_vSend: message processing and message sending by TheBlueMatt · Pull Request #9535 · bitcoin/bitcoin · GitHub 19:02:39 any multiwallet stuff isn't going to make it I assume 19:02:46 i like 9294, but i think it needs another review 19:02:53 multiwallet was already untagged 19:02:54 I'm ok with merge as long as one or two folks give it a postumous ack 19:03:05 i have not reviewed 9294, sorry 19:03:12 https://github.com/bitcoin/bitcoin/milestone/21 19:03:14 (but i plan to, whether it's merged or not) 19:03:36 we can always fix issues after the freeze 19:03:40 I could give it an updated review, but not sure if that's enough 19:03:43 there's a pre-MW PR that's probably ready, but not a prioirty 19:04:02 pre-mimblewimble? 19:04:06 heh 19:04:08 I think #9526 should be dropped from that. (perhaps we should do something later, but it shouldn't be tagged #14) 19:04:08 multiwallet ;) 19:04:09 https://github.com/bitcoin/bitcoin/issues/9526 | -blocksonly should disable sharing of mempool with dbcache · Issue #9526 · bitcoin/bitcoin · GitHub 19:04:10 https://github.com/bitcoin/bitcoin/issues/14 | bitcoin: URI and/or bitcoin-request MIME type for click-to-pay · Issue #14 · bitcoin/bitcoin · GitHub 19:04:36 issue #14 ? 19:04:38 https://github.com/bitcoin/bitcoin/issues/14 | bitcoin: URI and/or bitcoin-request MIME type for click-to-pay · Issue #14 · bitcoin/bitcoin · GitHub 19:04:41 I'd consider 9526 is a bugfix, but i guess i dont care strongly either way 19:04:47 #8775 specifically 19:04:50 https://github.com/bitcoin/bitcoin/issues/8775 | RPC refactoring: Access wallet using new GetWalletForJSONRPCRequest by luke-jr · Pull Request #8775 · bitcoin/bitcoin · GitHub 19:04:50 i think 9526 is a bugfix 19:04:59 but it seems it conflicted again, so I guess less than ready anyway :x 19:05:51 ok, so to conclude, #9461 and #9294 - 9461 i think is ready-ish (one more look-over, please, its easy?), and 9294 I think we should merge with a few commitments to postumous reviews 19:05:53 https://github.com/bitcoin/bitcoin/issues/9461 | [Qt] Improve progress display during headers-sync and peer-finding by jonasschnelli · Pull Request #9461 · bitcoin/bitcoin · GitHub 19:05:56 https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub 19:06:37 I generally dislike that we fork the branch knowing that some fix will be needed in both branches in advance 19:06:40 9377 we agreed previously was bugfix, and 9526, if we merge it for 14, i'd call a bugfix 19:07:04 jtimon: There won't be a branch today 19:07:09 jtimon: no, we branch in 2 weeks 19:07:19 We still have next week to fix bugs 19:07:22 the whole "we can merge it after fork, because it's a bugfix" concept 19:07:37 the fork is only in 2 weeks 19:07:37 who is talking about a fork? 19:07:45 bugfixes can go in in between 19:07:53 bugfixes can be merged, by definition, after the feature freeze 19:07:57 oh, I see, just mean 0.14 git fork, ie just branching 19:07:59 because it's a feature freeze nto a bug fix freeze 19:08:12 Yes. And technically 9294 is kind-of-a-fix for the missed HD chain split in 0.13. And there are no things to fix... only stuff to improve 19:08:24 jtimon: today (or whenever we decide) is the feature freeze. the actual 0.14 branch is only created in 2 weeks 19:08:29 9294 has string changes, so must be today or not at all 19:08:34 the branch is created at rc1 time 19:08:54 sipa: thanks I mixed feature freeze with branching 19:08:56 BlueMatt: Yes. I'm happy to merge it without the Consensus::Params::nPowTargetTimespan change 19:08:56 (so that releases happen from a branch, not from master) 19:09:06 If no objections... 19:09:12 jonasschnelli: open a new pr for that change, and then merge it, I'd say 19:09:25 jonasschnelli: Agree 19:09:25 (merge the one without the Consensus::Params thing, then open a pr to change it) 19:09:29 Okay. 19:09:34 so ideally all the bugfixes we know will be merged before branching, forget about my previous comment then 19:09:50 I apologize for not reviewing 9294, but i feel like i never got up to speed enough with the code in question. I do thik that although it's not critical and isn't already tagged 0.14, #9535 could be merged now and i know cfields wants it too 19:09:52 https://github.com/bitcoin/bitcoin/issues/9535 | Split CNode::cs_vSend: message processing and message sending by TheBlueMatt · Pull Request #9535 · bitcoin/bitcoin · GitHub 19:10:11 [13bitcoin] 15jonasschnelli pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/2ef52d3cf11b...b25068697fdb 19:10:11 13bitcoin/06master 1440ec7c7 15Jonas Schnelli: [Qt] Improve progress display during headers-sync and peer-finding 19:10:12 13bitcoin/06master 14b250686 15Jonas Schnelli: Merge #9461: [Qt] Improve progress display during headers-sync and peer-finding... 19:10:17 all the bugfixes we know and can realistically make the release (or are critical enough to delay it) should be merged before rc1, yes, thus before the branch 19:10:18 luke-jr: multiwallet pains me. because darn, such a simple set of changes remaining. we need to get out of this mode where all the intensity is in the week before feature freeze. :P (maybe new major version every month. :P ) 19:10:20 i think 9525 is pretty trivial 19:10:26 [13bitcoin] 15jonasschnelli closed pull request #9461: [Qt] Improve progress display during headers-sync and peer-finding (06master...062017/01/qt_sync) 02https://github.com/bitcoin/bitcoin/pull/9461 19:10:35 eh, 9535 19:11:22 all the intensity isn't in the week before feature freeze, we've merged tons of stuff in the last months 19:11:29 it's got enough ack's are there any objections to 9535 sipa? 19:11:33 it's okay 19:11:47 sipa: ok, so press the button? I'd call jtimon's review pretty thourough (even ignoring all the lock testing I plan on doing in the next 2 weeks) 19:11:53 and some things won't make a release, that's okay 19:12:45 BlueMatt: I wouldn't call it complete, but I noted the parts I did not do 19:12:46 priority for 0.14 is solving the nasty remaining issues, like the wallet sync problems 19:12:46 ok, so we need to figure out what to do about #9294, does anyone have any objections to merging so that we can freeze and getting postumous acks? 19:12:48 https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub 19:12:56 assuming someone is about to press merge on 9535, the only open question is do we hold off the feature freeze for 9294 (what he said) 19:13:05 wumpus: next topic...lets finalize list of things for freeze today first :p 19:13:41 BlueMatt: I agree with the two you mentioned 19:14:01 i'm afraid i'm unable to provide meaningful review on 9294. I had a few nits that weren't worth pointing out, but nothing else 19:14:02 #9461 and #9294 19:14:04 https://github.com/bitcoin/bitcoin/issues/9461 | [Qt] Improve progress display during headers-sync and peer-finding by jonasschnelli · Pull Request #9461 · bitcoin/bitcoin · GitHub 19:14:07 https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub 19:14:22 what about #9519 and #9377. are those bugfixes? 19:14:24 9461 is merged 19:14:25 https://github.com/bitcoin/bitcoin/issues/9519 | Exclude RBF replacement txs from fee estimation by morcos · Pull Request #9519 · bitcoin/bitcoin · GitHub 19:14:26 https://github.com/bitcoin/bitcoin/issues/9377 | fundrawtransaction: Keep change-output keys by default, make it optional by jonasschnelli · Pull Request #9377 · bitcoin/bitcoin · GitHub 19:14:35 sipa: yes, bugfixes with no translation string changes 19:14:47 ok 19:14:48 9519 is a bugfix and it's extremely simple 19:14:50 (if we decide to merge them, I'm confident in calling both bugfixes) 19:14:55 * jonasschnelli going to rebase 9377 19:15:18 cfields: same for me, I just did concept aCK for #9294 19:15:22 https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub 19:15:34 If it helps hd split get in, I promise a tACK after the fact 19:16:07 sipa: do you plan on needing to change any behavior or meaning of any options for #9526? 19:16:08 https://github.com/bitcoin/bitcoin/issues/9526 | -blocksonly should disable sharing of mempool with dbcache · Issue #9526 · bitcoin/bitcoin · GitHub 19:16:17 ok that leaves #9294 then, let's all review that 19:16:20 https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub 19:16:28 ok, so acks on the following for today: hd split (9294), net lock split (9535) 19:16:29 #action review #9294 asap so it can still make the cut 19:16:33 https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub 19:16:44 then we can move on to next topic 19:17:29 Should we touch/chat about the wallet sync issue? https://github.com/bitcoin/bitcoin/pull/9583 19:17:32 ok, next topic: wallet inconsistency (revert #7946 for 0.14 is pr 9583), see issue #9584 and #9148 19:17:34 https://github.com/bitcoin/bitcoin/issues/7946 | Reduce cs_main locks during ConnectTip/SyncWithWallets by jonasschnelli · Pull Request #7946 · bitcoin/bitcoin · GitHub 19:17:34 ? 19:17:35 https://github.com/bitcoin/bitcoin/issues/9584 | Synchronization problems with wallet. · Issue #9584 · bitcoin/bitcoin · GitHub 19:17:36 https://github.com/bitcoin/bitcoin/issues/9148 | Wallet RPCs can return stale info due to ProcessNewBlock Race · Issue #9148 · bitcoin/bitcoin · GitHub 19:17:44 #topic #9583 and #9584 (morcos) 19:17:46 https://github.com/bitcoin/bitcoin/issues/9583 | Move wallet callbacks into cs_main (this effectively reverts #7946) by TheBlueMatt · Pull Request #9583 · bitcoin/bitcoin · GitHub 19:17:47 https://github.com/bitcoin/bitcoin/issues/9584 | Synchronization problems with wallet. · Issue #9584 · bitcoin/bitcoin · GitHub 19:18:11 gmaxwell: please make sure you see this so you don't complain later that you didn't realize we were sticking everything back into cs_main again 19:18:12 I apologise for 7946,... I wasn't aware that this could cause sync issues 19:18:26 I tagged 9535 for 0.14 (I uess that's the intent?) 19:18:39 jonasschnelli: ehh, nbd, thats why it was early in a release cycle...sadly no one fixed it before now :( 19:18:47 wumpus: yes or just merge. i think its ready, but not sure why it hasn't been 19:18:48 turns out there is complicated machinery to fix it, eg #9570 19:18:50 https://github.com/bitcoin/bitcoin/issues/9570 | Block Wallet RPCs until wallet is synced to our current chain by TheBlueMatt · Pull Request #9570 · bitcoin/bitcoin · GitHub 19:18:50 but like x2 19:19:10 I'm working on a version of it, but I really dont like that much change this late in cycle 19:19:25 so hopefully we can get the changes in super early in 0.15, and get lots of eyes on it through that cylc 19:19:26 e 19:19:31 ^ this is my recommendation 19:19:39 morcos: well it's not tagged for 0.14, so it has been hidden for me as that's what I've been focusing on 19:19:40 which is merge 9583 19:20:06 wumpus: the issue to track this (9148) has been tagged for 14 all along 19:20:11 What's the target date for 0.15? 19:20:11 BlueMatt: which is your recommendation? 9538? 19:20:15 cfields: yes 19:20:25 [13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/b25068697fdb...82274c02ed2d 19:20:25 heh, laggy. 19:20:26 13bitcoin/06master 14d7c58ad 15Matt Corallo: Split CNode::cs_vSend: message processing and message sending... 19:20:26 13bitcoin/06master 14376b3c2 15Matt Corallo: Make the cs_sendProcessing a LOCK instead of a TRY_LOCK... 19:20:27 13bitcoin/06master 1482274c0 15Wladimir J. van der Laan: Merge #9535: Split CNode::cs_vSend: message processing and message sending... 19:20:42 [13bitcoin] 15laanwj closed pull request #9535: Split CNode::cs_vSend: message processing and message sending (06master...062017-01-cs-vsend-split) 02https://github.com/bitcoin/bitcoin/pull/9535 19:21:05 Can anyone think of any downside for merging 9583? Is there any chance we made further changes later that somehow were depending on the fact that we weren't holding cs_main through the wallet updates any more? 19:21:07 BlueMatt: sadly, I think I agree. I've been down the rabbit hole today trying to come up with something simple, and it gets more complicated (and I become less comfortable) quickly. 19:21:22 CodeShark: see https://github.com/bitcoin/bitcoin/issues/8719 19:21:33 I can't think of anything htat could make sense, but really thats the only downside I could imagine... Otherwise its just not making an improvement that would have been nice to make.. 19:22:00 wumpus: thx 19:22:01 morcos: Yes. Downside is slighly slower sync/rescan 19:22:02 (and I do not believe it is (yet) a major performance regression because this is pretty much all called from the single ProcessMessages thread) 19:22:10 I don't think any design depended on not holding it, varrious testing might have. 19:22:19 jonasschnelli: I don't see how it could result in a slower sync, it's all in one thread. 19:22:41 (and the networking thread doesn't itself grab cs_main) 19:23:09 morcos: isn't there still one site where it gets called without cs_main though? 19:23:12 Hmm.. I guess I'm wrong. #7946 didn't and it was acctually a stepping stone for stuff that's not PRed. 19:23:14 https://github.com/bitcoin/bitcoin/issues/7946 | Reduce cs_main locks during ConnectTip/SyncWithWallets by jonasschnelli · Pull Request #7946 · bitcoin/bitcoin · GitHub 19:23:16 my intention for 0.15 is to move these callbacks into a background thread asap 19:24:34 cfields: that will not be true after the revert, i think 19:24:40 cfields: Do you mean after the reversion in 9583? I don't think so? 19:25:10 ok, maybe i traced it wrong. Will do again. 19:25:14 ok, if no one has any conceptual objections to 9583, then I dont think there is much to discuss on it now, just note that thoruough review is needed 19:25:49 any other proposed topics? 19:25:56 sad, but i accept that 9583 is probably the only viable solution for 0.14 19:26:25 indeed 19:26:35 one step forward, one step back, but at least we learned something 19:26:39 2 steps forward for 0.15 :) 19:26:43 ☺ 19:27:07 We could wrap it in #ifdef WALLET_ENABLED... *duck* 19:27:26 its used in net_processing 19:27:56 I meant the cs_main lock for SyncTransaction, but just kidding. 19:29:52 any other proposed topics? 19:30:06 gmaxwell: are we still doing #9501 for 0.14? 19:30:08 https://github.com/bitcoin/bitcoin/issues/9501 | Final Alert for 0.14 · Issue #9501 · bitcoin/bitcoin · GitHub 19:30:28 AFAIK we are. When should we be sending it to the network? 19:30:39 i think this is a good a time as any 19:30:39 gmaxwell: +1 19:30:47 *as 19:30:52 We can't PR the message send until we're ready for the message to hit the network. 19:31:02 #topic Final Alert for 0.14 19:31:07 Okay I can do that today, I don't think we need any delays or announcements given the prior alert. 19:31:10 gmaxwell: we could PR it without the signature 19:31:14 gmaxwell: ACK 19:31:19 let's just do it 19:31:23 fine with me 19:31:28 ACK 19:31:32 wumpus: 19:32:26 K. well at least we don't have to discuss text for it. 19:33:01 I can pr an update to the bitcoin.org post 19:33:01 hehe 19:33:16 9108 needs an 0.14 tag, i believe 19:33:45 BlueMatt: not necessarily 19:33:49 i vote 9392 gets a non-0.14 tag 19:33:53 jonasschnelli: it fixes an 0.14-tagged issue 19:34:02 so either that or 9034 loses its tag 19:34:07 WatchOnly where always with birthday 0 19:34:13 https://cdn.meme.am/cache/instances/folder963/500x/74859963.jpg 19:34:15 indeed 19:34:29 There are a number of importmulti serious bugfixes I have queued which I was waiting until after the freeze to finish. 19:35:01 ack on untag #9034 for 0.14? 19:35:03 https://github.com/bitcoin/bitcoin/issues/9034 | importmulti does not respect the given timestamp · Issue #9034 · bitcoin/bitcoin · GitHub 19:35:28 BlueMatt: tagged 19:35:31 gmaxwell: I assume that is related to #9491? 19:35:33 https://github.com/bitcoin/bitcoin/issues/9491 | Importmulti api is confusing in a way that could lead to funds loss. · Issue #9491 · bitcoin/bitcoin · GitHub 19:35:54 wait i'm confused... gmaxwell you don't want those merged before 0.14 or you do? 19:36:55 I guess he don't.. 19:36:57 nm, BlueMatt confused me... importmulti fixes should be in 0.14, i think we all agree 19:37:14 yes 19:37:27 Ah.. okay. I read it wrong. 19:37:30 the impression I got is that gmaxwell just has more work to do on them, and was prioritising stuff before it 19:37:38 agree 19:37:54 I'm ok with untagging #9027 for 14 - it was pointed out that we can do a simple fix to address the issue mentioned there, but there are other issues so its nontrivial to *really* fix 19:37:55 https://github.com/bitcoin/bitcoin/issues/9027 | Unbounded reorg memory usage · Issue #9027 · bitcoin/bitcoin · GitHub 19:39:42 so in the category of fixes 19:40:23 wumpus and sipa when you get a chance, take a look at #9371... i think thats the direction you wanted me to go... and if we do 9583.. its pretty clearly no change in behavior from what txConflicted would have done.. 19:40:25 https://github.com/bitcoin/bitcoin/issues/9371 | Notify on removal by morcos · Pull Request #9371 · bitcoin/bitcoin · GitHub 19:41:42 [13bitcoin] 15paveljanik opened pull request #9587: Do not shadow local variable named `tx`. (06master...0620170119_Wshadow_net_processing) 02https://github.com/bitcoin/bitcoin/pull/9587 19:42:23 morcos: will do 19:45:30 ok, any other topics? 19:45:39 i propose lunch 19:45:39 I'm done (finally) :p 19:45:48 sipa: too late, already did that 19:46:01 let's end early then 19:46:06 #endmeeting