19:01:00 #startmeeting 19:01:00 Meeting started Thu Jun 27 19:01:00 2019 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:01:00 Useful Commands: #action #agreed #help #info #idea #link #topic. 19:01:04 hi 19:01:09 hi 19:01:11 hi 19:01:17 hi 19:01:19 hello 19:01:44 hi 19:01:54 two topics on the list for today: 0.18.1: Backports #16035, depends build cache 19:01:57 https://github.com/bitcoin/bitcoin/issues/16035 | 0.18.1: Backports by MarcoFalke · Pull Request #16035 · bitcoin/bitcoin · GitHub 19:02:15 any last minute topic proposals? 19:03:02 #topic High priority for review 19:03:05 https://github.com/bitcoin/bitcoin/projects/8 19:03:24 5 blockers, 1 bugfix, 7(!) things requiring concept ACK 19:03:30 anything to add/remove/merge ? 19:04:03 I'd like to nominate #16257 for 0.18.1 19:04:05 https://github.com/bitcoin/bitcoin/issues/16257 | [wallet] abort when attempting to fund a transaction above -maxtxfee by Sjors · Pull Request #16257 · bitcoin/bitcoin · GitHub 19:04:33 swap #15450 for #16227 please 19:04:35 https://github.com/bitcoin/bitcoin/issues/15450 | [GUI] Create wallet menu option by achow101 · Pull Request #15450 · bitcoin/bitcoin · GitHub 19:04:37 https://github.com/bitcoin/bitcoin/issues/16227 | Refactor CWallets inheritance chain by achow101 · Pull Request #16227 · bitcoin/bitcoin · GitHub 19:04:53 provoostenator:if you want to nominate a backport might be better to do it in MarcoFalke 's topic? 19:05:06 Yes, sorry 19:05:48 oh, it's not merged yet to master 19:06:11 ok that should be under 'bugfix' the nI suppose 19:07:43 Based on stats from Blockchair on 0.1 BTC fees, I think quite a few people are firing that footgun (unless there's another wallet that produces exact 0.1 BTC fees). 19:07:59 achow101: done 19:08:25 provoostenator: that's worrying 19:08:31 https://blockchair.com/bitcoin/transactions?q=fee(10000000)# 19:08:51 It's beacuse if you set feeRate to "1" that doesn't mean 1 satoshi per byte. 19:09:42 right, sounds like a bug 19:10:04 provoostenator: holy crap that's insane 19:10:55 another case of quietly ignoring an error 19:11:03 that's always a red flag 19:11:04 It rounds down the fee instead of aborting. Which has been the case for years, but the "satoshi per byte" convention is newer, so maybe that's what causes the increase. 19:11:05 :o 19:11:18 And the new PSBT methods also have this setting. 19:11:32 provoostenator: that's 25 BTC in fees this month overall 19:12:01 this is the miner trolling X) 19:12:23 Some of these are batches, but quite a few are small txs. 19:12:42 For big batches something else happens: the user sets a higher fee, but then we round it down. 19:12:54 Which can cause large batch transactions to get stuck. 19:12:54 ^^ 19:13:17 In both cases, I think throwing an error is just better. User can always override the maxfee, or manually set a fee. 19:13:23 agree 19:13:27 how is that we are only running into this now? hasn't this behavior been in for ages? 19:13:57 agree 19:14:12 https://blockchair.com/bitcoin/transactions?q=fee(20000000)# 19:14:27 achow101: no one ever reported it AFAIK 19:14:35 those are old 19:14:59 this is the first time I hear this is the case, it sounds awful 19:15:10 it looks like in december 2017 there were ~100 cases of this per day as well 19:15:22 +1 for merging provoostenator's PR soon and doing 0.18.1 19:15:43 I'll be quick to address feedback on the PR. 19:16:15 thanks 19:16:15 thanks provoostenator for bringing this to attention 19:16:16 sipa: that was the ath period? 19:16:33 promag: i just looked at dec 20th 2017 19:16:34 December 2017 was fee madness yes. 19:16:51 So people start manually setting the fee. 19:17:07 so this is certainly not a few phenomenon, and also not the first that it seems actually impactful 19:17:13 s/few/new/ 19:17:24 And also when mempool "weather reports" became popular, and more wallets started supporting fee settings. Most using the satoshi per byte unit. 19:18:13 ack with fixing it 19:18:19 really wonder why this is never reported, not strange some people complain about high fees at least then :( 19:19:35 ok 19:19:40 #topic 0.18.1: Backports #16035 (MarcoFalke) 19:19:42 https://github.com/bitcoin/bitcoin/issues/16035 | 0.18.1: Backports by MarcoFalke · Pull Request #16035 · bitcoin/bitcoin · GitHub 19:20:33 maxfee should be batch sane 19:21:35 I don't know what Marco wants to discuss about this topic,doesn't seem like he's here 19:22:21 Marco! 19:22:24 sry 19:22:31 here, hi 19:22:39 so close. 19:22:40 I wrapped up on the backports 19:22:41 cfields: wow 19:22:53 heh 19:23:15 MarcoFalke: ^^ looks like we have a last-minute one by provoostenator and then really want to do 0.18.1 19:23:27 MarcoFalke: thanks 19:23:30 promag: all of dec 2017 had 6336 instances; way worse than now 19:23:56 Would be nice if one or two went through my cherry-picks (to check if they are solved correctly) and if the commits itself make sense 19:24:18 yes 19:24:37 #action check MarcoFalke's backports in #16035 19:24:39 provoostenator's fix still needs review. I'd rather have it backported after the existing backports are merged (and reviewed) 19:24:40 https://github.com/bitcoin/bitcoin/issues/16035 | 0.18.1: Backports by MarcoFalke · Pull Request #16035 · bitcoin/bitcoin · GitHub 19:24:53 MarcoFalke: absolutely 19:25:01 I think fanquake and promag already had a look 19:25:04 it should always be in master first 19:25:37 I don't want to nag them again to re-ACK, so my backport branch is final 19:25:55 ok 19:25:55 How do we know the .1 fees aren't miners? 19:26:26 yes, most of the backports are clean cherry picks, and the others are trivial. Also non critical changes imo. 19:26:32 ddustin: we don't, but 0.1 BTC is a suspicious number 19:26:47 ddustin: we don't, though it seems unlikely for miners to pay themselves so much fees when they can include their own transactions for free 19:27:06 are we tagging 0.18.1 after that PR? 19:27:21 promag: I think so 19:27:29 If nothing else pops up, hehe 19:27:39 right 19:28:24 :( 19:28:34 I think #13339 should be in 0.18 19:28:36 https://github.com/bitcoin/bitcoin/issues/13339 | wallet: Replace %w by wallet name in -walletnotify script by promag · Pull Request #13339 · bitcoin/bitcoin · GitHub 19:28:39 #topic depends build cache (cfields) 19:28:50 promag:that's a feature 19:29:20 I don't see why it'd be backported 19:29:49 multiwallet is kind of useless for integrators without that 19:29:53 actually; the cases in dec 2017 were not absurd; they were all paying reasonable feerates for that time 19:29:57 we're talking about 0.18.1 here the 0.19 feature freeze is somewhat further waway 19:30:22 wumpus: i understand, it's a bit sad it missed 0.18 19:30:40 (2019-09-15 to be exact) 19:30:49 promag: yes, blame windows and its absurd escaping rules 19:31:05 absurd and inconsistent 19:31:20 So for the travis/depends bottleneck issue, I thought of some low-hanging fruit that I think would have quite an impact. By simply sharing the intermediate depends binary packages globally among builds, we avoid situations where dozens of PRs are all rebuilding all of depends. 19:31:20 Instead, the first to finish would send it to the cache server, and each client would check for that package before building it itself. Because all packages-names are deterministically generated and unique, there should be no filename collisions, so maintenance should be effectively zero on the storage side. At most, a cron job to delete the oldest files now and then. 19:31:35 wumpus: I've asked if we could just ignore windows, let's move to the PR later 19:31:41 As a side-effect, it would also kick in and avoid complete depends builds when Travis fails to download its cache. 19:31:42 I'll try to hack it together this week. It may be enough that we don't need to make the bigger changes we discussed a few weeks ago. 19:32:35 cfields: how would this cache server work, e.g. how to prevent PRs from uploading arbitrary binary dependencies, or do you intend to build them outside of travis? 19:32:40 cfields: The cache would be read-write by anyone? 19:33:14 this has always been the problem with uploading any kind of data from travis 19:33:28 you can configure travis to have local secrets as environment variables 19:33:48 so you have an api key or something in one of the travis environment vars that lets you upload to the server 19:33:50 achow101: I could write a PR to dump those secrets? 19:34:04 You can disable secrets for prs 19:34:13 wumpus: Indeed. It's also going to be a problem with some of the other, more complicated splits that we discussed. I figure this is a much smaller surface to experiment with. 19:34:20 for branches that would be OK, that's proabably enough 19:34:53 no one is going to merge a PR that dumps secrets and if so we have much bigger issues :) 19:35:05 I feel like the same problem would be solved by having a shorter cache expiry on travis for pull requests 19:35:17 wumpus: I'm thinking it may be possible to leverage github tags somehow for "allowed to cache" or so. That way the cache is always primed before another PR branches from it. 19:35:35 I'm sad that we need this 19:35:37 https://docs.travis-ci.com/user/environment-variables#defining-encrypted-variables-in-travisyml 19:35:44 so the alternative CI ideas were a dead end? 19:36:00 travis lets you encrypt variables, but it's not available for PRs for the reason that promag said 19:36:22 Travis already re-generates and caches depends on master, the pull requests are just too slow to pick it up, since they still have their own cache 19:36:28 semaphore2 would have a nice cache tool 19:36:37 wumpus: nono, this was just something that occured to me today. I thought it was a good idea, but if you don't like it, no big deal. 19:36:42 that lets you manually control the key/storage-blobs 19:36:59 jonasschnelli: semaphore2 sounds great, but it doesn't allow viewing the test logs?!? 19:37:07 cfields: Have you seen my comment? 19:37:17 they promised to get this done in the next days... 19:37:27 cfields: it just feels so hacky to implement our own caching on an external server because travis is too stupid to handle that correctly, it seems a base thing ! 19:37:30 but,.. maybe its something we should follow but not do now 19:38:16 cfields: I'm not against your idea, but it seems to go from bad to worse, what's the next thing we have to implement for them :) 19:38:41 indeed 19:38:45 MarcoFalke: yes, that's what I was attempting to address. All PRs would immediately have access to those files instead of waiting on the cache. 19:39:00 since we are customer of travis, can we not request a feature? 19:39:21 that will probably take too long, if they pick it up 19:39:28 very likely 19:39:28 wumpus: I'm not sure that's fair. They have storage/upload capabilities, but we're just using the cache. 19:39:32 cfields: With immediately you mean "after the depends built finished"? 19:41:00 cfields: oh we'd be using their upload/storage capabilities? 19:41:11 So we'd have to wait either until our own depends server finishes the depends built or until travis finishes it. I don't see the difference 19:41:12 cfields: that makes sense, I wasn't aware of that 19:41:13 MarcoFalke: I mean: PR1 is created which touches depends, then PR2 is created, then PR1 is merged, PR2 rebuilds depends next time it's bumped whether it touched them or not. 19:41:23 (I believe I typed that out right) 19:41:34 cfields: I thought this would have to be some external server run by ourselves 19:41:39 cfields: also doesn't work for prs 19:41:56 wumpus: well, that was my open question, but I guess you've answered it. 19:42:09 promag: wait, really? 19:42:20 https://docs.travis-ci.com/user/uploading-artifacts/ 19:42:36 is this what you mean? 19:43:03 promag: ugh. Ok. That's probably why we don't do this already, huh? :) 19:43:41 promag: I don't believe that any blocks you from making the upload part of your script itself 19:44:28 Ok, I'll go back to the drawing board. Thanks, all. 19:44:47 cfields: I wonder what would happen if we disabled the cache for pull requests completely 19:44:47 thanks cfields for working on this 19:44:59 So they would always get the freshest cache from master 19:45:52 that seems preferable, as long as the PR doesn't change depends (in which case you can expect slowness anyway) 19:45:56 achow101: true, but then you have to manage all of that 19:46:20 MarcoFalke: Hmm, let's take a look after the meeting? 19:46:36 I guess it would make it impossible to run travis on pulls that change depends (as pointed out by wumpus) 19:46:37 I wasn't aware you could configure that. 19:46:49 (or forgot) 19:47:21 right 19:47:29 MarcoFalke: each push would just nuke its own cache and rebuild, I think? 19:49:18 So maybe we could wipe all pull request caches after 1-3 days? 19:49:43 Not a perfect solution, but might approximate well enough 19:50:24 if it's better than it's better 19:50:31 MarcoFalke: are there options for that now as well? 19:50:58 I could write a script for it (and ping travis on my issue from last year) 19:51:22 script via api? 19:51:29 Either way, +1 on the ping :) 19:51:38 it's definitely possible to wipe caches through the API 19:51:46 per PR or for all of them 19:53:19 Ok. Going to have to think on it some. But +1 for whatever makes it better. 19:53:31 ok, that concludes the meeting I think 19:54:44 thanks everyone 19:54:47 #endmeeting