19:01:00 <wumpus> #startmeeting
19:01:00 <lightningbot> 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 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:01:04 <jonasschnelli> hi
19:01:09 <cfields> hi
19:01:11 <achow101> hi
19:01:17 <meshcollider> hi
19:01:19 <promag> hello
19:01:44 <moneyball> hi
19:01:54 <wumpus> two topics on the list for today: 0.18.1: Backports #16035, depends build cache
19:01:57 <gribble> https://github.com/bitcoin/bitcoin/issues/16035 | 0.18.1: Backports by MarcoFalke · Pull Request #16035 · bitcoin/bitcoin · GitHub
19:02:15 <wumpus> any last minute topic proposals?
19:03:02 <wumpus> #topic High priority for review
19:03:05 <wumpus> https://github.com/bitcoin/bitcoin/projects/8
19:03:24 <wumpus> 5 blockers, 1 bugfix, 7(!) things requiring concept ACK
19:03:30 <wumpus> anything to add/remove/merge ?
19:04:03 <provoostenator> I'd like to nominate #16257 for 0.18.1
19:04:05 <gribble> 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 <achow101> swap #15450 for #16227 please
19:04:35 <gribble> https://github.com/bitcoin/bitcoin/issues/15450 | [GUI] Create wallet menu option by achow101 · Pull Request #15450 · bitcoin/bitcoin · GitHub
19:04:37 <gribble> https://github.com/bitcoin/bitcoin/issues/16227 | Refactor CWallets inheritance chain by achow101 · Pull Request #16227 · bitcoin/bitcoin · GitHub
19:04:53 <wumpus> provoostenator:if you want to nominate a backport might be better to do it in MarcoFalke 's topic?
19:05:06 <provoostenator> Yes, sorry
19:05:48 <wumpus> oh, it's not merged yet to master
19:06:11 <wumpus> ok that should be under 'bugfix' the nI suppose
19:07:43 <provoostenator> 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 <wumpus> achow101: done
19:08:25 <wumpus> provoostenator: that's worrying
19:08:31 <provoostenator> https://blockchair.com/bitcoin/transactions?q=fee(10000000)#
19:08:51 <provoostenator> It's beacuse if you set feeRate to "1" that doesn't mean 1 satoshi per byte.
19:09:42 <wumpus> right, sounds like a bug
19:10:04 <sipa> provoostenator: holy crap that's insane
19:10:55 <wumpus> another case of quietly ignoring an error
19:11:03 <wumpus> that's always a red flag
19:11:04 <provoostenator> 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 <promag> :o
19:11:18 <provoostenator> And the new PSBT methods also have this setting.
19:11:32 <sipa> provoostenator: that's 25 BTC in fees this month overall
19:12:01 <promag> this is the miner trolling X)
19:12:23 <provoostenator> Some of these are batches, but quite a few are small txs.
19:12:42 <provoostenator> For big batches something else happens: the user sets a higher fee, but then we round it down.
19:12:54 <provoostenator> Which can cause large batch transactions to get stuck.
19:12:54 <jonasschnelli> ^^
19:13:17 <provoostenator> 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 <sipa> agree
19:13:27 <achow101> how is that we are only running into this now? hasn't this behavior been in for ages?
19:13:57 <wumpus> agree
19:14:12 <promag> https://blockchair.com/bitcoin/transactions?q=fee(20000000)#
19:14:27 <wumpus> achow101: no one ever reported it AFAIK
19:14:35 <promag> those are old
19:14:59 <wumpus> this is the first time I hear this is the case, it sounds awful
19:15:10 <sipa> it looks like in december 2017 there were ~100 cases of this per day as well
19:15:22 <wumpus> +1 for merging provoostenator's PR soon and doing 0.18.1
19:15:43 <provoostenator> I'll be quick to address feedback on the PR.
19:16:15 <wumpus> thanks
19:16:15 <jonasschnelli> thanks provoostenator for bringing this to attention
19:16:16 <promag> sipa: that was the ath period?
19:16:33 <sipa> promag: i just looked at dec 20th 2017
19:16:34 <provoostenator> December 2017 was fee madness yes.
19:16:51 <provoostenator> So people start manually setting the fee.
19:17:07 <sipa> so this is certainly not a few phenomenon, and also not the first that it seems actually impactful
19:17:13 <sipa> s/few/new/
19:17:24 <provoostenator> 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 <achow101> ack with fixing it
19:18:19 <wumpus> really wonder why this is never reported, not strange some people complain about high fees at least then :(
19:19:35 <wumpus> ok
19:19:40 <wumpus> #topic 0.18.1: Backports #16035 (MarcoFalke)
19:19:42 <gribble> https://github.com/bitcoin/bitcoin/issues/16035 | 0.18.1: Backports by MarcoFalke · Pull Request #16035 · bitcoin/bitcoin · GitHub
19:20:33 <jonasschnelli> maxfee should be batch sane
19:21:35 <wumpus> I don't know what Marco wants to discuss about this topic,doesn't seem like he's here
19:22:21 <cfields> Marco!
19:22:24 <MarcoFalke> sry
19:22:31 <MarcoFalke> here, hi
19:22:39 <cfields> so close.
19:22:40 <MarcoFalke> I wrapped up on the backports
19:22:41 <promag> cfields: wow
19:22:53 <jonasschnelli> heh
19:23:15 <wumpus> MarcoFalke: ^^ looks like we have a last-minute one by provoostenator and then really want to do 0.18.1
19:23:27 <wumpus> MarcoFalke: thanks
19:23:30 <sipa> promag: all of dec 2017 had 6336 instances; way worse than now
19:23:56 <MarcoFalke> 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 <wumpus> yes
19:24:37 <wumpus> #action check MarcoFalke's backports in #16035
19:24:39 <MarcoFalke> provoostenator's fix still needs review. I'd rather have it backported after the existing backports are merged (and reviewed)
19:24:40 <gribble> https://github.com/bitcoin/bitcoin/issues/16035 | 0.18.1: Backports by MarcoFalke · Pull Request #16035 · bitcoin/bitcoin · GitHub
19:24:53 <wumpus> MarcoFalke: absolutely
19:25:01 <MarcoFalke> I think fanquake and promag already had a look
19:25:04 <wumpus> it should always be in master first
19:25:37 <MarcoFalke> I don't want to nag them again to re-ACK, so my backport branch is final
19:25:55 <wumpus> ok
19:25:55 <ddustin> How do we know the .1 fees aren't miners?
19:26:26 <promag> yes, most of the backports are clean cherry picks, and the others are trivial. Also non critical changes imo.
19:26:32 <sipa> ddustin: we don't, but 0.1 BTC is a suspicious number
19:26:47 <wumpus> 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 <promag> are we tagging 0.18.1 after that PR?
19:27:21 <wumpus> promag: I think so
19:27:29 <MarcoFalke> If nothing else pops up, hehe
19:27:39 <wumpus> right
19:28:24 <promag> :(
19:28:34 <promag> I think #13339 should be in 0.18
19:28:36 <gribble> 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 <wumpus> #topic depends build cache (cfields)
19:28:50 <wumpus> promag:that's a feature
19:29:20 <wumpus> I don't see why it'd be backported
19:29:49 <promag> multiwallet is kind of useless for integrators without that
19:29:53 <sipa> actually; the cases in dec 2017 were not absurd; they were all paying reasonable feerates for that time
19:29:57 <wumpus> we're talking about 0.18.1 here the 0.19 feature freeze is somewhat further waway
19:30:22 <promag> wumpus: i understand, it's a bit sad it missed 0.18
19:30:40 <wumpus> (2019-09-15 to be exact)
19:30:49 <wumpus> promag: yes, blame windows and its absurd escaping rules
19:31:05 <wumpus> absurd and inconsistent
19:31:20 <cfields> 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 <cfields> 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 <promag> wumpus: I've asked if we could just ignore windows, let's move to the PR later
19:31:41 <cfields> As a side-effect, it would also kick in and avoid complete depends builds when Travis fails to download its cache.
19:31:42 <cfields> 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 <wumpus> 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 <MarcoFalke> cfields: The cache would be read-write by anyone?
19:33:14 <wumpus> this has always been the problem with uploading any kind of data from travis
19:33:28 <achow101> you can configure travis to have local secrets as environment variables
19:33:48 <achow101> 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 <promag> achow101: I could write a PR to dump those secrets?
19:34:04 <MarcoFalke> You can disable secrets for prs
19:34:13 <cfields> 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 <wumpus> for branches that would be OK, that's proabably enough
19:34:53 <wumpus> no one is going to merge a PR that dumps secrets and if so we have much bigger issues :)
19:35:05 <MarcoFalke> I feel like the same problem would be solved by having a shorter cache expiry on travis for pull requests
19:35:17 <cfields> 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 <wumpus> I'm sad that we need this
19:35:37 <achow101> https://docs.travis-ci.com/user/environment-variables#defining-encrypted-variables-in-travisyml
19:35:44 <wumpus> so the alternative CI ideas were a dead end?
19:36:00 <achow101> travis lets you encrypt variables, but it's not available for PRs for the reason that promag said
19:36:22 <MarcoFalke> 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 <jonasschnelli> semaphore2 would have a nice cache tool
19:36:37 <cfields> 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 <jonasschnelli> that lets you manually control the key/storage-blobs
19:36:59 <wumpus> jonasschnelli: semaphore2 sounds great, but it doesn't allow viewing the test logs?!?
19:37:07 <MarcoFalke> cfields: Have you seen my comment?
19:37:17 <jonasschnelli> they promised to get this done in the next days...
19:37:27 <wumpus> 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 <jonasschnelli> but,.. maybe its something we should follow but not do now
19:38:16 <wumpus> 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 <jonasschnelli> indeed
19:38:45 <cfields> 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 <jonasschnelli> since we are customer of travis, can we not request a feature?
19:39:21 <wumpus> that will probably take too long, if they pick it up
19:39:28 <jonasschnelli> very likely
19:39:28 <cfields> wumpus: I'm not sure that's fair. They have storage/upload capabilities, but we're just using the cache.
19:39:32 <MarcoFalke> cfields: With immediately you mean "after the depends built finished"?
19:41:00 <wumpus> cfields: oh we'd be using their upload/storage capabilities?
19:41:11 <MarcoFalke> 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 <wumpus> cfields: that makes sense, I wasn't aware of that
19:41:13 <cfields> 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 <cfields> (I believe I typed that out right)
19:41:34 <wumpus> cfields: I thought this would have to be some external server run by ourselves
19:41:39 <promag> cfields: also doesn't work for prs
19:41:56 <cfields> wumpus: well, that was my open question, but I guess you've answered it.
19:42:09 <cfields> promag: wait, really?
19:42:20 <promag> https://docs.travis-ci.com/user/uploading-artifacts/
19:42:36 <promag> is this what you mean?
19:43:03 <cfields> promag: ugh. Ok. That's probably why we don't do this already, huh? :)
19:43:41 <achow101> promag: I don't believe that any blocks you from making the upload part of your script itself
19:44:28 <cfields> Ok, I'll go back to the drawing board. Thanks, all.
19:44:47 <MarcoFalke> cfields: I wonder what would happen if we disabled the cache for pull requests completely
19:44:47 <wumpus> thanks cfields for working on this
19:44:59 <MarcoFalke> So they would always get the freshest cache from master
19:45:52 <wumpus> that seems preferable, as long as the PR doesn't change depends (in which case you can expect slowness anyway)
19:45:56 <promag> achow101: true, but then you have to manage all of that
19:46:20 <cfields> MarcoFalke: Hmm, let's take a look after the meeting?
19:46:36 <MarcoFalke> I guess it would make it impossible to run travis on pulls that change depends (as pointed out by wumpus)
19:46:37 <cfields> I wasn't aware you could configure that.
19:46:49 <cfields> (or forgot)
19:47:21 <wumpus> right
19:47:29 <cfields> MarcoFalke: each push would just nuke its own cache and rebuild, I think?
19:49:18 <MarcoFalke> So maybe we could wipe all pull request caches after 1-3 days?
19:49:43 <MarcoFalke> Not a perfect solution, but might approximate well enough
19:50:24 <wumpus> if it's better than it's better
19:50:31 <cfields> MarcoFalke: are there options for that now as well?
19:50:58 <MarcoFalke> I could write a script for it (and ping travis on my issue from last year)
19:51:22 <cfields> script via api?
19:51:29 <cfields> Either way, +1 on the ping :)
19:51:38 <wumpus> it's definitely possible to wipe caches through the API
19:51:46 <wumpus> per PR or for all of them
19:53:19 <cfields> Ok. Going to have to think on it some. But +1 for whatever makes it better.
19:53:31 <wumpus> ok, that concludes the meeting I think
19:54:44 <wumpus> thanks everyone
19:54:47 <wumpus> #endmeeting