19:00:15 #startmeeting 19:00:15 Meeting started Thu Jan 5 19:00:15 2017 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:00:15 Useful Commands: #action #agreed #help #info #idea #link #topic. 19:00:17 may I propose a topic first? need to sleep 19:00:22 hi 19:00:26 jl2012: go 19:00:39 #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:44 Hi 19:00:49 proposed topic: repair the fork warning system: https://github.com/bitcoin/bitcoin/pull/9443 19:01:00 #topic repair the fork warning system 19:01:09 concept ack on fixing it if it's broken! 19:01:17 concept ack 100% 19:01:22 haha yes concept ack. Haven't looked at the code yet. 19:01:28 i haven't had time to look at the details... 0.14 is getting close 19:01:28 but this is more than just fixing it 19:01:37 but we've fucked that up several times already, so needs careful review, I think 19:01:44 here 19:01:56 jl2012: elaborate? 19:02:04 it also stores invalid headers 19:02:07 jl2012: are you trying to say you think it NEEDS to be in 0.14 19:02:43 huh what 19:02:43 oh, meeting, wasn't planning on attending today, but really nothing to do until one hour from now... 19:02:45 oh 19:02:47 specifically, it stores valid headers under an invalid block 19:03:00 hi. 19:03:01 also, headers with invalid nVersion are stored 19:03:10 jl2012: but those do have valid PoW? 19:03:14 yes 19:03:22 and valid nTime 19:03:23 iirc that is our only invariant for the storage of headers 19:03:30 jl2012: do we need to store them or can we just store them in memory? 19:03:38 but, I'm fine either way 19:04:03 looking at invalid headers with valid pow for user-warnings sounds good to me 19:04:09 stored headers are forever, both on disk and in memory, unfortunately 19:04:10 there may be some DoS avenues that open up due to it, if they get accepted for chains that fork off early on 19:04:14 jl2012: does this do anything to address that nodes sending us such chains will be DoS banned 19:04:15 ? 19:04:29 won't be banned 19:04:34 (but i shouldn't comment on that before looking at code) 19:04:38 I feel pretty blah about the utlity of that warning system, and warnings in general. I think we've burned people out with false warnings, and they weren't used usefully by most to begin with. :( 19:04:58 the alternative to fixing it would be to just disable the system, at least for 0.14 19:05:04 gmaxwell: having reliable warning messages is the first step towards fixing that trust :) 19:05:04 jl2012: so this removes the DoS banning for invalid blocks? 19:05:21 luke-jr: just for valid header 19:05:31 shipping it broken, especially with risk of false positives, indeed isn't going to do anything good 19:05:41 if the block content is invalid (e.g. invalid script), still banned 19:06:00 There is currently no false positive risk from it AFAIK. 19:06:03 jl2012: which will lead to you not getting future headers 19:06:07 jl2012: to be clear, if the block is too large it will be banned as well? 19:06:09 having to store more data forever just to be able to warn seems a bit inefficient to me though 19:06:27 mhmm, the block is still invalid here, no? https://github.com/bitcoin/bitcoin/pull/9443/files#diff-24efdb00bfbe56b140fb006b562cc70bR3038 19:06:29 the block headers are never pruned, and all loaded into memory at start 19:06:30 petertodd: i dont think it changes anything wrt consensus code? the way I read it 19:06:33 petertodd: we can't even deseralize it if its too large so how would we even know if the contet were invalid. 19:06:35 petertodd: should be, I don't change that part 19:06:37 petertodd: /any/ consensus logic 19:06:42 i wonder if we need a detection of internal consensus errors (database corruption, CPU overheating, ...) 19:06:54 gmaxwell: we can deserialize if it's only a little bit too large though IIRC 19:06:58 because 99.999% of all fork warning that are seen in practice as just broken nodes 19:07:03 An invarient we have right now is that we depnd on banning to make sure all our connection slots are not consumed by peers that are on an incompatible blockchain. 19:07:23 sipa: I think we do. 19:07:53 sipa: which ultimately should be used to trigger recovery from chainstate backup automatically. (I think) 19:07:55 but i don't know how without having a thread in the background that computes Pi or so :p 19:08:15 jl2012: i'd like to understand the context of the discussion, is that about the change in general or whether it shoudl be in 0.14. Is the current status of master somehow worse than 13.1/2? 19:08:25 detecting database corruption on disk is pretty easy as everything is CRC-ed, CPU overheating or memory corruption on the other hand... 19:09:01 morcos: I think that has broken for a few versions 19:09:22 Broken just means that it won't trigger in all cases it might trigger, right? 19:09:23 good to know, yes, if it's been broken for a few versions there's no hurry to fix it for 0.14 19:09:25 so you may consider it as a new feature 19:09:33 gmaxwell: yes 19:09:35 well it can be considered for 0.14.1 or so 19:09:43 if it's a bugfix (which i think it is) 19:10:00 I wonder if it can be done without storing more headers though 19:10:02 but it doesn't sounds like this PR will actually fix it, and fixing it may be more complicated than it seems due to the point gmaxwell raised 19:10:17 My understanding is that there are cases where invalid blocks make us ignore later headers (normally a good thing) which will prevent them from triggering the warning. 19:10:35 I really don't like storing otherwise unnecessary data forever 19:10:42 wumpus: store only headers required to prove to yourself upon restart that you should display a warning, and prune the (separate) storage for that? 19:10:49 we could prune old header chains 19:10:54 (both from disk and memory) 19:10:59 or that 19:11:03 from memory....sounds hard 19:11:08 we could, sure, but right now we don't, so should be careful not to store more than necessary 19:11:09 on-disk/restart-only sounds doable 19:11:14 wumpus: provided theres a reasonable minimum PoW to storing it, it'll never be all *that* much extra data given it's just headers 19:11:14 re: invariant for storage of headers: remind you that matt moved the nTime check from CheckBlockHeader() to ContextualCheckBlockHeader() 19:11:15 yeah, on restart it trivial 19:11:25 yes on startup would be good enough 19:11:33 I think it would be wise to use our limited time to concentrate on things people think are really important for 0.14, it doesn't sound like anyone is making that argument about this change? 19:11:47 agree 19:11:49 ok, please move on 19:11:54 I'd like improvements here, but I don't think it's 0.14 critical. 19:12:00 other proposed topics? 19:12:10 0.14 prs-to-review... 19:12:14 morcos: this change is mostly useful to plan for a future hardfork, but I don't think it's critical it's in 0.14 19:12:20 BlueMatt: https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+milestone%3A0.14.0 19:12:35 wumpus: many of those are almost certainly not gonna make it 19:12:38 luke-jr: heh, ok, i'll put tha ton my hard-fork planning list, i have it handy right here 19:12:55 BlueMatt: that's a chicken-egg problem though as it depends on who reviews it 19:13:00 true 19:13:13 so the topic here for the discussion should be what to prioritize for review 19:13:16 I agree, though 19:13:29 well if we're short for topics parallel processmessages...if folks think they will not have time to review it, then I'll skip it, but I have one based on cory's current net pr at https://github.com/theuni/bitcoin/compare/connman-locks...TheBlueMatt:2017-01-parallel-processmessages?expand=1 19:13:31 agree with sipa 19:13:47 and think it would be a huge improvement for some use-cases 19:13:49 I just meant that the lis is already very long, so let's at least try not to add much more 19:13:53 so open question: what would people like to see in 0.14, if review effort wasn't a bottleneck 19:14:01 named arguments 19:14:03 or, what is the priority 19:14:03 I really feel uncomfortable with last minute concurrency changes. 19:14:06 proposed topic: custom blockchains for 0.14 (ie how realistic it is to hope to get https://github.com/bitcoin/bitcoin/pull/8994 merged for 0.14 ? ) 19:14:13 there was some desire for #8775 #8694 #7533 in 0.14, but they're not tagged as such 19:14:15 https://github.com/bitcoin/bitcoin/issues/8775 | RPC refactoring: Access wallet using new GetWalletForJSONRPCRequest by luke-jr · Pull Request #8775 · bitcoin/bitcoin · GitHub 19:14:17 https://github.com/bitcoin/bitcoin/issues/8694 | Basic multiwallet support by luke-jr · Pull Request #8694 · bitcoin/bitcoin · GitHub 19:14:19 https://github.com/bitcoin/bitcoin/issues/7533 | RPC: sendrawtransaction: Allow the user to ignore/override specific rejections by luke-jr · Pull Request #7533 · bitcoin/bitcoin · GitHub 19:14:21 custom chainparams really 19:14:23 gmaxwell: it tries very hard to limit concurrency changes - its whitelisted on messages, plus other things 19:14:39 gmaxwell: same 19:14:54 i'd like to see the improvements to block relay #9375, #9441, #9447 and possibly parallel ProcessMessages 19:14:56 gmaxwell: no open prs of mine make any significant concurrency changes 19:14:57 https://github.com/bitcoin/bitcoin/issues/9375 | Relay compact block messages prior to full block connection by TheBlueMatt · Pull Request #9375 · bitcoin/bitcoin · GitHub 19:14:59 https://github.com/bitcoin/bitcoin/issues/9441 | Net: Massive speedup. Net locks overhaul by theuni · Pull Request #9441 · bitcoin/bitcoin · GitHub 19:15:00 https://github.com/bitcoin/bitcoin/issues/9447 | Allow 2 simultaneous block downloads by morcos · Pull Request #9447 · bitcoin/bitcoin · GitHub 19:15:08 gmaxwell: or at least they try hard not to 19:15:14 s/not// 19:15:18 wumpus: +1 for named arguments. Will re-review. 19:15:31 BlueMatt: we really have inadequate testing there, as cfield's version assert has shown us. 19:15:35 +1 on named arguments as well... will amke future PR's easier/better 19:15:49 gmaxwell: helgrind works well, it turns out :), but, agreed 19:16:03 BlueMatt: only if you actually execute the codepaths. 19:16:15 sure 19:16:29 Where are we with the multiwallet support? 19:16:35 BlueMatt: maybe you could briefly describe what you're hoping to immediately improve? 19:16:46 gmaxwell: there's one refactor outsanding I believe 19:16:59 gmaxwell: 3 PRs left, 9375 and 9441 19:17:15 9441? sure? 19:17:21 err 19:17:23 8775 19:17:26 8775 8694* 19:17:35 I had been hoping for that and the utxo scriptpubkey index (#8660) in 0.14, but it looks like the latter has been abandoned by its requester. 19:17:37 https://github.com/bitcoin/bitcoin/issues/8660 | txoutsbyaddress index (take 2) by djpnewton · Pull Request #8660 · bitcoin/bitcoin · GitHub 19:18:01 gmaxwell: yes, that's unfortunate 19:18:39 i'd like to see #9441 in to get the performance benefit 19:18:39 cfields: specifically, because many miners rely on bitcoind submitblock -> p2p network latency to relay their blocks out, this ends up being pretty important for miners in several ways, so speeding up the relay (hopefully without introducing a ton of new concurrency outside of cmpctblock/getblocktxn/blocktxn message processing) would be a massive win for many miners 19:18:41 https://github.com/bitcoin/bitcoin/issues/9441 | Net: Massive speedup. Net locks overhaul by theuni · Pull Request #9441 · bitcoin/bitcoin · GitHub 19:18:44 for efficiency, maybe https://github.com/bitcoin/bitcoin/pull/8498 helps, but nobody has found the time to benchmark that in years... 19:19:02 yes, so multiwallet and at least the currently-open net prs are review focus 19:19:03 wumpus: I think it's suffered less attention than it would have recieved because it's poorly labled and people keep thinking its a blockchain index. 19:19:19 ok well if i had to pick one, i think 9375 makes a huge difference 19:19:42 gmaxwell: looks like droark might pick it up 19:20:04 BlueMatt: well I had a PR that removed the biggest source of submitblock latency-- it's a couple lines changes, I assume its functionality has been duplicated in one of cfields overlapping PRs that I closed that one in favor of. 19:20:22 if nothing else the caching of the block/cmpctblock that you are about to send to all your peers reduces the time per per from 25ms to <1ms 19:20:58 gmaxwell: yes, its in the "Net: Massive speedup" one, but the validation time cost (which the parallel getblocktxn/processmessages stuff + the fast relay stuff fixes) 19:21:23 gmaxwell: for current-tip you really want both, but the net stuff you're referring to isnt the only thing here 19:21:25 I am going to be irritated if 9441 misses 0.14. I had an alternative PR that resulted in the same speedup which I think was much less invasive, but I closed it because cfields prefered the other change because it serviced his overall refactor planning. 19:22:01 I think that one is pretty far along, its gotten a lot of eyes even if not so much acks 19:22:42 yes, so multiwallet and at least the currently-open net prs are review focus <-- any other things to add to that list 19:22:44 ? 19:22:54 gmaxwell: let's focus on making that one get in then (I do think there's some regression risk, as it's a pretty invasive change to do last minute) 19:22:59 i'll focus on the net locks overhault, named args, early compact block relay 19:23:01 gmaxwell: so 9441 doesn't count as concurrency changes you are worried about merging now 19:23:12 wumpus: that was my feeling too but yea. 19:23:18 +1 sipa's list 19:23:27 morcos: right, I think it's invasive but it shouldn't be meaningfully creating new concurrency. 19:23:41 sgtm 19:23:44 #action focus on the net locks overhault, named args, early compact block relay 19:23:46 sip, ok, named args it is, also multi-wallet? 19:24:10 a 19:24:11 The caching improvements as part of early compact block are nice. One option is if we feel uncomfortable about early compact blocks is that we disable that part and just take the caching. 19:24:11 sipa 19:24:16 luke-jr: might make sense to split out QT stuff for time 19:24:29 instagibbs_: planning to do so, once 8775 is merged 19:24:33 multi-wallet may be still too many PRs away for 0.14 , dunno how realistic it is to get that in, though we certainly can make progress with the refactors 19:24:35 which pr is named args? 19:24:48 jtimon: #8811 19:24:51 https://github.com/bitcoin/bitcoin/issues/8811 | rpc: Add support for JSON-RPC named arguments by laanwj · Pull Request #8811 · bitcoin/bitcoin · GitHub 19:24:51 Obviously I think 9138 (improve fee estimation) needs to be merged, but i think its ACK'ed enough (excpet it had some rebases) 19:24:54 gmaxwell: the fast-relay stuff there has been /super/ scaled back...at this point its only a) if its the first thing you're annoucing at that height, and b) if its built directly on your tip 19:25:00 gmaxwell: so hopefully its easier to be comfortable with it 19:25:02 On the remaining multiwallet, I felt one of the outstanding refactor PRs introduced a fair amount of not very C++ish code, but who am I to judge? and I didn't know what to recommend instead, so I asked sipa to look at it but I doubt he's had a chance yet. 19:25:08 instagibbs_: although Qt stuff ties in with RPC stuff for the debug window 19:25:11 morcos: if something is ready for merge you should let me know :) 19:25:33 well at this point, my judgement isn't to be trusted.. :) 19:25:53 BlueMatt: remaining concerns are things like "are we going to accidentally DOS attack our peers by announcing something and then reorging" and things like that. 19:26:08 Multiwallet: I think need to rebase this one: #8764 19:26:10 https://github.com/bitcoin/bitcoin/issues/8764 | [Wallet] get rid of pwalletMain, add simple CWallets infrastructure by jonasschnelli · Pull Request #8764 · bitcoin/bitcoin · GitHub 19:26:13 gmaxwell: yes, hence scaling it back...been discussing it a bunch with folks in the past few days 19:26:28 jonasschnelli: that kinda conflicts with the current multiwallet stuff 19:26:29 Maybe https://github.com/bitcoin/bitcoin/projects/2 needs update 19:27:09 maybe https://github.com/bitcoin/bitcoin/projects/6 needs to be...closed? 19:27:32 might be more efficient to do 8764 after the basic parts are in 19:28:51 For 0.14 I really want to see the second pass through createtransaction change from morcos in... fixes some concerning fee overpayment corner case. 19:29:01 I don't know why #9312 isn't merged yet. 19:29:03 https://github.com/bitcoin/bitcoin/issues/9312 | Increase mempool expiry time to 2 weeks by morcos · Pull Request #9312 · bitcoin/bitcoin · GitHub 19:29:22 #9404 is super easy now, although needs rebase after #9465 is merged 19:29:24 https://github.com/bitcoin/bitcoin/issues/9404 | Smarter coordination of change and fee in CreateTransaction. by morcos · Pull Request #9404 · bitcoin/bitcoin · GitHub 19:29:25 https://github.com/bitcoin/bitcoin/issues/9465 | [Wallet] Do not perform ECDSA signing in the fee calculation inner loop. by gmaxwell · Pull Request #9465 · bitcoin/bitcoin · GitHub 19:29:45 i read a few things about accidental extreme fee 19:30:32 maybe related to estimate fee for the very next block now disabled? (no idea, just thinking out loud) 19:30:39 is that related to 9138 ? 19:30:41 gmaxwell: same holds for you, if something is ready for merging you should let me know 19:30:49 or 9404? 19:31:01 #9404 though I really want it in, I haven't actually reviewed the code becuase I know it'll be simpler after 9465. (the story there is a user reported an issue that might be that fee bug, I went go fix it, realized it would be easier to fix after factoring out the signing.. did that.. then realized your preexisting patch was already the fix I wanted. :P ) 19:31:02 https://github.com/bitcoin/bitcoin/issues/9404 | Smarter coordination of change and fee in CreateTransaction. by morcos · Pull Request #9404 · bitcoin/bitcoin · GitHub 19:31:12 9312 clearly is 19:31:21 sipa: both, well disabling fee estimates for 1 already went in, not part of 9138 19:31:24 jtimon: https://www.reddit.com/r/Bitcoin/comments/5ltw5n/bitcoin_core_v0131_sends_enormously_high_fee/ 19:31:35 perhaps ^ should be our next topic 19:31:35 but both could cause high fees 19:31:37 I think I stopped paying attention there when a certain person started trolling then lost track of it. 19:31:39 can someone explain what causes this? 19:31:46 (i don't visit reddit) 19:31:49 yes 19:31:55 ^ that 19:32:00 luke-jr: see 9404, i think 19:32:04 luke-jr: I believe its fixed by 9404. Of course, I can't know for sure, not enough info. 19:32:39 oh, is it change unnecessarily converted to fee? 19:32:40 gmaxwell: so it's wallet-related code, not estimation 19:32:41 the 9404 case is caused when you select coins to pay for a tx, calculate fee and realize you dont' have enough, so you have to go and reselect coins. you end up selecting many fewer for whatever reason, and now you have enough fee of course, and you end up paying the fee you calculated for the prior iteration 19:32:46 gmaxwell: well, OP's post there said he's using estimatefee :/ 19:32:57 gmaxwell: 9404 is already rebased on 9465 as of this morning, so easy peasy to review now 19:32:59 But the user seemed to be reporting that it payed several times the fee estimator figures (at least thats my read on it), which supports 9404 over 9138 though 9138 needs to go in too. 19:33:07 morcos: oh missed that, will review. 19:33:22 #9294 should also go into 0.14 (needs overhaul, my turn) and some form of a HD rescan would be great. 19:33:24 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:33:41 [13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/ce43630d1e97...a72f76ca3d5d 19:33:41 13bitcoin/06master 145f0e27f 15Alex Morcos: Increase mempool expiry time to 2 weeks 19:33:42 13bitcoin/06master 14a72f76c 15Wladimir J. van der Laan: Merge #9312: Increase mempool expiry time to 2 weeks... 19:33:42 gmaxwell: he sent you the debug log, did that indicate anything useful? 19:33:56 [13bitcoin] 15laanwj closed pull request #9312: Increase mempool expiry time to 2 weeks (06master...06longerexpiry) 02https://github.com/bitcoin/bitcoin/pull/9312 19:34:07 jonasschnelli: do we still have nothing that removes key from the keypool when they show up in transactions out of order, so that a rescan while unlocked would successfully find everything? 19:34:20 luke-jr: no. unfortunately. 19:34:43 well it didn't suggest that the known ways that estimatefee could be high weren't being hit. 19:34:45 gmaxwell: we don't. But I'm working on it. Got distracted with SPV/BFD and 2016 visualisation. 19:35:10 jonasschnelli: okay, I'll do it if you don't have time; I just figured you are much more recently familar with that code than I. Sorry to nag. 19:35:11 sipa: its also possible that fee estimation could temporarily be very high.. was MUCH more likely for estimatefee 1 though, which has already been disabled 19:35:22 morcos: ok 19:35:28 gmaxwell: I'm happy if you do it. 19:35:30 let's get those fixes in 19:36:10 gmaxwell: maybe use some prework form https://github.com/bitcoin/bitcoin/pull/9370 19:36:14 TBH I knew about the issue fied by 9404 long ago, but I thought it had since been fixed... :( 19:37:14 jonasschnelli: oh I thought I'd acked the fundraw reuse fix.. will review. 19:37:36 The correct one is: https://github.com/bitcoin/bitcoin/pull/9377 19:38:06 The one above is an older try that could be useful for hd restore. 19:38:41 Fun topic: 2016 Git Visualisation: I'd created a draft video, need feedback to overhaul it and place it on the bitcoincore.org website. 19:38:47 https://vimeo.com/198242328 19:38:51 Password coredev 19:38:57 (will be there for a couple of mins) 19:39:14 (sorry to spam the meeting) 19:39:27 okay I concept acked, well I'll complete the review. 19:40:04 jonasschnelli: why password protect it and post the password in public? :P 19:40:16 Security by obscurity. 19:40:18 luke-jr: MILITARY LEVEL BLOCKCHAIN SECURITY 19:40:24 heh 19:40:41 hehe 19:41:06 anyway, I think that constitutes an "effective access control" under the DMCA... 19:41:22 Who called this meeting? 19:41:28 jonasschnelli: short comment: overuse of capitalization (why are Day and Commit capitalized) and dashes (Code-contributors should be written with a space in between) 19:41:47 sipa: Thanks, will adapt 19:41:58 any other topics? 19:42:50 apparently not! 19:42:58 everyone's watching the video 19:43:08 chainparams? 19:43:20 looks like we'll close the first meeting of 2017 early 19:43:29 8994 19:43:30 what is there to discuss about chainparams? 19:43:46 for 0.14, i think. 19:43:53 final list of things to focus for review? multi-wallet, currently-open net prs, fee ones, what else? 19:44:07 oh, and multiargs 19:44:14 rpcarg thing 19:44:17 and hd chain-split/rstore 19:44:23 wumpus, on my part #8994 19:44:25 https://github.com/bitcoin/bitcoin/issues/8994 | Testchains: Introduce custom chain whose constructor... by jtimon · Pull Request #8994 · bitcoin/bitcoin · GitHub 19:44:39 I think that is really a lot less important than everything else listed. 19:44:58 (as people using that are also probably using git master) 19:44:58 are we really trying to get all these wallet changes in? 19:45:03 yes it doesn't seem to make a lot of sense to focus last-minute review attention on that 19:45:04 sure, feedback is still welcomed though 19:45:08 should we focus on some over others? 19:45:11 morcos: yea, I think thats more than will make it, but that was the list 19:45:11 (so merging it after 14 means just weeks of delay for someone who wants to use it) 19:45:20 jtimon: fair enough. 19:45:24 morcos: the fee fixes obviously have priority 19:45:44 I doubt it will get to 0.14 that's why I asked "how realistic..." 19:45:45 anything that can cause users to spend unnecessary coins should be first priority 19:45:52 well i guess i meant between hd-chain/split and multiwallet 19:45:59 i think maybe multi-wallet is less likely so maybe less priority, but maybe others disagree since that would be a super nice user-facing feature 19:46:07 I think multiwallet would be great... lots of demand for something like that 19:46:14 I would prioritize fee fixes, net/relay things, hd/rescan improvements, multiwallet, the thousand other open PRs. 19:46:17 but yes bigger changes 19:46:26 gmaxwell: in order? 19:46:34 There is a lot of demand for multiwallet and I feel like if we don't get it done it'll continue to slip. 19:46:35 don't forget named args? 19:46:38 morcos: yes that was an order. 19:46:39 >thousand other open PRs 19:46:50 gmaxwell: Would multiwallet in 0.14 include the GUI? 19:47:07 I think 0.14 multiwallet is too late - better to merge it as first thing for 0.15, then improve it in master 19:47:08 Have we already discussed how to select the wallet over RPC? 19:47:09 multiwallet is in Knots already, so may be less important in that sense (since users who want it can get it); stuff like HD split can't really go in Knots without being more final 19:47:13 it will need a lot of last-minute fixes Im sure 19:47:26 jonasschnelli: a number of times :p 19:47:30 a big feature like that is not done once it's merged 19:47:53 wumpus: it's not actually that big at this point 19:48:14 At least on my earlier review it seemed like most of it was the refactors. 19:48:17 Which helps. 19:48:20 99% of it is renaming pwalletMain to pwallet in rpc files 19:48:36 But we need to be careful. Running with many wallets needs some testing. 19:48:42 sorry i'm not trying to be a downer, but both 9375 (fast compact block relay) and 9441 (net speedup) are big heavy review changes, so we shouldn't spread ourselves too thin if we realyl want those in 19:48:43 in any case there are plenty of PRs to focus on, as said before they can't make it all in 19:48:45 Even if it's code-wise trivial 19:49:16 overall, I think multiwallet can be delayed in Core if other things need time 19:49:21 if we can't make any choices to postpone things, either 0.13 will slip incredibly (I'd hate that) or we'll have to randomly drop things last minute 19:49:31 agree with morcos 19:49:39 0.13 slipping now would indeed be terrible! 19:49:44 heh 19:49:48 ;) 19:49:49 hah 19:50:22 lol 19:50:47 wumpus: if we slip it we slip it, but if people are active on review and testing I think we don't need to. I really wish the net changes were less invasive but thats water under the bridge now. 19:51:07 I do believe the release will be delayed from fixes for just the things we already have in now. 19:51:16 am I likely to be of any help reviewing the net stuff if I'm not up to speed on the net refactoring so far? 19:51:21 gmaxwell: I don't want to let it slip on features in any case, on bugfixes is more acceptable 19:51:27 note: we have at least 4 regressions in master which are 0.14-blocking which do not yet have prs open to fix them 19:51:30 so it's clear what to focus on then 19:51:30 so....thats a thing 19:51:43 wumpus: right, well ... you could just merge everything outstanding and then all slips are bugfix related! :P 19:51:46 BlueMatt: there a list? 19:51:47 oh, sorry, 1 has an open pr 19:52:02 BlueMatt: are the issues tagged appropriately? 19:52:09 yes, tagged 0.14, i believe 19:52:13 ok 19:52:28 AFAIK we are not actually waiting on the competion of any feature changes. (except perhaps some rescan improvements).. E.g. almost everything I think we might have in 0.14 has a PR already open. 19:52:32 to reviewers, don't let the amount of commits in 9441 scare you. Almost all of them are very tiny, explicitly to make review easier 19:52:46 at least #9479, #9027, #9148 and #9212 19:52:48 https://github.com/bitcoin/bitcoin/issues/9479 | An error has occurred and has been logged. Please contact this bot's administrator for more information. 19:52:48 maybe others 19:52:48 https://github.com/bitcoin/bitcoin/issues/9027 | Unbounded reorg memory usage · Issue #9027 · bitcoin/bitcoin · GitHub 19:52:49 confirming what cfields said 19:52:49 https://github.com/bitcoin/bitcoin/issues/9148 | Wallet RPCs can return stale info due to ProcessNewBlock Race · Issue #9148 · bitcoin/bitcoin · GitHub 19:52:50 https://github.com/bitcoin/bitcoin/issues/9212 | Assertion failed: (nSendVersion != 0), function GetSendVersion, file ./net.h, line 775. · Issue #9212 · bitcoin/bitcoin · GitHub 19:52:51 oh shit, yeah sorry, one is mine.. , lets quickly decide which direction we want to take on that. Do we revert #9240 or do we not care about the notifications? I think #9371 is too late for 0.14 19:52:52 completion* 19:52:53 https://github.com/bitcoin/bitcoin/issues/9240 | Remove txConflicted by morcos · Pull Request #9240 · bitcoin/bitcoin · GitHub 19:52:54 https://github.com/bitcoin/bitcoin/issues/9371 | Notify on removal by morcos · Pull Request #9371 · bitcoin/bitcoin · GitHub 19:53:27 morcos: if 9371 is too late, we need to revert 9240... but maybe that is not something we need to know now? 19:53:32 a revert can be do at the last minute 19:53:39 *done 19:53:42 If we're reverting #9240, it'll conflict, definitely with 9138 and probably already, so let me know and I'll make a revert PR 19:53:43 yea, we can revert on the 0.14 branch at that point? 19:53:44 https://github.com/bitcoin/bitcoin/issues/9240 | Remove txConflicted by morcos · Pull Request #9240 · bitcoin/bitcoin · GitHub 19:53:51 * jtimon reiterates his dissapointment on not having removed checkpoints for everything except progress estimation, that doesn't have a PR... 19:54:35 jtimon: 9472 removes checkpoints for the purpose of progress estimation 19:54:39 sipa: well i like 9371, but it overlaps a lot with #8549 and we haven't resolved direction 19:54:40 :) 19:54:40 I mean gmaxwell you had that practically done already 19:54:41 https://github.com/bitcoin/bitcoin/issues/8549 | zmq: mempool notifications by jmcorgan · Pull Request #8549 · bitcoin/bitcoin · GitHub 19:54:52 sipa: oh, nice, will have a look 19:55:29 jtimon: not going to have one either, not any time soon (1) it requires a consensus change; and I don't have the appetite to carry forward in the adversarial climate of the mailing list (which I am not on for a long time now), and (2) Sipa disagrees with my one strategy for removing the signature checking dependency, and petertodd disagrees with the other. 19:55:37 Madars: what I don't like about 9371 is that is that it keeps the list of removed transactions on mempool, instead of an external object listening for signals 19:55:44 sorry s/Madars/Morcos 19:56:02 morcos: this means it can only support one client listening for removals at most 19:56:08 (sipa disagrees that we can just check all signatures; petertodd disagrees with the proposal that we use burried by 30 days of work+other conditions) 19:56:18 gmaxwell: mhmm, I didn't remember a consensus change, must be missing something, but sure, if it needs it, definitely no time to do it for 0.14 19:56:39 morcos: for the rest I'm ok with it, and it doesn't conflict with 8549 too much 19:56:58 gmaxwell: what about the idea of once crossing a certain amount of work, requiring a higher minimum difficulty? 19:57:09 jtimon: the part of it that didn't either need a consensus change (uppping the minimum difficulty) and didn't change the signature checking, has already been merged. 19:57:17 wumpus: i was trying to keep notifications from happening while we were locked in reorg.. couldn't we later make it so the mempoolremovaltracker could interface with multiple clients or something 19:57:22 ah, right, consensus change 19:57:31 sipa: it's a consensus change, and I implemented it, and asked for review which jtimon gave some, but... consensus change. 19:57:52 morcos: I just don't like making a notification mechanism stateful, but that may be a personal thing 19:58:15 thanks for the info, wasn't up to date on the subject 19:58:16 morcos: but ok maybe this is the only way to handle reorgs sanely 19:58:19 but I really have no interest in writing a bit and getting treated like shit by zander and voskull or whatever other trolls inhabit the list today. 19:58:20 but isn't that what we've just done on purpose with ConnectTrace? 19:58:38 i guess not quite the same thing... but accomplishes same goal 19:58:45 maybe in this 2 minutes...should I rebase the super-trivial #9279 ? 19:58:46 https://github.com/bitcoin/bitcoin/issues/9279 | Consensus: Move CFeeRate out of libconsensus by jtimon · Pull Request #9279 · bitcoin/bitcoin · GitHub 19:59:04 * luke-jr ponders if it would make sense to increase min difficulty to 50% of current difficulty. 19:59:09 morcos: well at least that's an external object passed in 19:59:32 gmaxwell: maybe you should explain your idea to luke-jr 19:59:54 i know... but so many layers.. anyway, ok we'll see what happens.. i'll make the revert later if i need to 20:00:04 sipa: https://github.com/gmaxwell/bitcoin/commit/2db190b183c5204da23191ca642c7f6cad412ae3 20:00:06 time of meeting has ended 20:00:11 #endmeeting