< fanquake> Now that 18.04 is available for WSL I think we might now have a somewhat sane build process for windows users.
< fanquake> Scratch that. Half sure that a depends build just crashed VirtualBox..
< bitcoin-git> [bitcoin] fanquake opened pull request #13251: GUI: Rephrase Bech32 checkbox texts, and enable it with legacy address default (master...rephrase-bech-32) https://github.com/bitcoin/bitcoin/pull/13251
< bitcoin-git> [bitcoin] fanquake closed pull request #12208: GUI: Rephrase Bech32 checkbox texts, and enable it with legacy address default (master...gui_legacy_bech32) https://github.com/bitcoin/bitcoin/pull/12208
< bitcoin-git> [bitcoin] Empact opened pull request #13252: Wallet: Refactor ReserveKeyFromKeyPool for safety (master...refactor_wallet_RKFKP) https://github.com/bitcoin/bitcoin/pull/13252
< bitcoin-git> [bitcoin] fanquake closed pull request #9537: Wallet: Refactor ReserveKeyFromKeyPool for safety (master...refactor_wallet_RKFKP) https://github.com/bitcoin/bitcoin/pull/9537
< bitcoin-git> [bitcoin] fanquake opened pull request #13253: [0.16] Further Backports (0.16...0-16-further-backports) https://github.com/bitcoin/bitcoin/pull/13253
< fanquake> Going to drop the "Needs Backport" label off the PRs in 13253 so we can get a good idea of what's left.
< bitcoin-git> [bitcoin] Empact opened pull request #13254: Revert "Merge #12870: make clean removes src/qt/moc_ files" (master...make-clean-qt-moc) https://github.com/bitcoin/bitcoin/pull/13254
< bitcoin-git> [bitcoin] fanquake opened pull request #13255: trivial: Fixed typos and cleaned up language (master...language-cleanup) https://github.com/bitcoin/bitcoin/pull/13255
< fanquake> ^ Can either be merged quickly or closed for now.
< bitcoin-git> [bitcoin] fanquake closed pull request #13010: Trivial: Language Cleanup (master...master) https://github.com/bitcoin/bitcoin/pull/13010
< bitcoin-git> [bitcoin] fanquake closed pull request #10708: Connecttrace fewer blocks (master...connecttrace-fewer-blocks) https://github.com/bitcoin/bitcoin/pull/10708
< bitcoin-git> [bitcoin] fanquake closed pull request #10470: Fix for listsinceblock not filtering conflicted transactions (master...listsinceblock-filter-conflicts) https://github.com/bitcoin/bitcoin/pull/10470
< fanquake> Added "Up for Grabs" labels to both of those, no updates to either for 6moths and nearly a year.
< kallewoof> fanquake: Yeah, if no reaction even with feedback I think closing is ok. I don't think we should close PR's just because they lie around though. I mean, I have PRs that I am maintaining, with no feedback, that can lie around for months and months. Doesn't mean they should be closed. :)
< kallewoof> I do think generally we should say 'Close this?' and let the author have a say before closing, since non-maintainers can't reopen their PRs even if they wanted to.
< kallewoof> I.e. let the authors have an opportunity to close the PR themselves in case they wanna come back to it when they have more time/energy/inspiration.
< fanquake> kallewoof sure. I have to run out but will comment when I'm back
< kallewoof> fanquake: No worries, just idly musing.
< bitcoin-git> [bitcoin] Empact opened pull request #13257: wallet: Fix ReserveKeyFromKeyPool to always test key classification, even if last in the pool. (master...fix-reserve-key-classification-check) https://github.com/bitcoin/bitcoin/pull/13257
< bitcoin-git> [bitcoin] Empact closed pull request #13257: wallet: Fix ReserveKeyFromKeyPool to always test key classification, even if last in the pool. (master...fix-reserve-key-classification-check) https://github.com/bitcoin/bitcoin/pull/13257
< Bonney> 0xC0D332838f14EF42Fcde1cf2518c427dDB676729
< wumpus> BlueMatt: yes, going to untag the fds one at least
< wumpus> ones*
< wumpus> oh, that's all that is left besides backports, right
< bitcoin-git> [bitcoin] kallewoof opened pull request #13258: uint256: Remove unnecessary crypto/common.h use (alternative) (master...uint256-no-crypto-alt) https://github.com/bitcoin/bitcoin/pull/13258
< fanquake> wumpus I've sorted out a few. There's a couple that should be backported in individual PRs though I think.
< bitcoin-git> [bitcoin] kallewoof closed pull request #13242: uint256: Remove unnecessary crypto/common.h use (master...uint256-no-crypto) https://github.com/bitcoin/bitcoin/pull/13242
< bitcoin-git> [bitcoin] kallewoof opened pull request #13259: validation: add a macro for determining if a block is pruned or not (master...block-pruned-macro) https://github.com/bitcoin/bitcoin/pull/13259
< wumpus> fanquake: thanks, I'll do some review on backports today
< bitcoin-git> [bitcoin] jonasschnelli opened pull request #13262: Wallet/RPC: Add listsincetx with a stateless (server-side) long polling option (master...2018/05/listsincetx) https://github.com/bitcoin/bitcoin/pull/13262
< bitcoin-git> [bitcoin] GreatSock opened pull request #13264: [qt] Satoshi unit (master...satoshis) https://github.com/bitcoin/bitcoin/pull/13264
< promag> fyi can't attend meeting today
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/4cfe17c3382b...ef0e5cd5174c
< bitcoin-git> bitcoin/master 7ab1c6f Luke Dashjr: GUI: Rephrase Bech32 checkbox text/tooltip...
< bitcoin-git> bitcoin/master 82dda6b Luke Dashjr: GUI: Allow generating Bech32 addresses with a legacy-address default
< bitcoin-git> bitcoin/master ef0e5cd MarcoFalke: Merge #13251: GUI: Rephrase Bech32 checkbox texts, and enable it with legacy address default...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #13251: GUI: Rephrase Bech32 checkbox texts, and enable it with legacy address default (master...rephrase-bech-32) https://github.com/bitcoin/bitcoin/pull/13251
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/ef0e5cd5174c...1b53e4f67c6d
< bitcoin-git> bitcoin/master 84f4194 Chun Kuan Lee: break circular dependency: random/sync -> util -> random/sync
< bitcoin-git> bitcoin/master 1b53e4f MarcoFalke: Merge #13236: break circular dependency: random/sync -> util -> random/sync...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #13236: break circular dependency: random/sync -> util -> random/sync (master...random_util) https://github.com/bitcoin/bitcoin/pull/13236
< promag> will trade reviews =) #13097
< gribble> https://github.com/bitcoin/bitcoin/issues/13097 | ui: Support wallets loaded dynamically by promag · Pull Request #13097 · bitcoin/bitcoin · GitHub
< jonasschnelli> promag: you can remove the description part where it says "builds on top of" in 13097
< promag> ok
< promag> ty
< jonasschnelli> will review
< promag> thanks!
< sipa> meeting?
< wumpus> #startmeeting
< lightningbot> Meeting started Thu May 17 19:00:35 2018 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< jonasschnelli> hi
< sipa> hi
< promag> hi
< jamesob> howdy
< wumpus> #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 achow101 meshcollider jnewbery maaku fanquake promag provoostenator
< kanzure> hi.
< wumpus> proposed topics: 0.16.1 I guess
< jimpo> hi
< cfields> hi
< wumpus> #topic High priority for review
< sipa> i'd like to add #13142
< gribble> https://github.com/bitcoin/bitcoin/issues/13142 | Separate IsMine from solvability by sipa · Pull Request #13142 · bitcoin/bitcoin · GitHub
< wumpus> we merged #10740 this week, #12254 #10757 #13011 #13097 #12979 #12634 are left
< jonasschnelli> I'd like to add #12196
< jnewbery> hello
< gribble> https://github.com/bitcoin/bitcoin/issues/10740 | [wallet] `loadwallet` RPC - load wallet at runtime by jnewbery · Pull Request #10740 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/12254 | BIP 158: Compact Block Filters for Light Clients by jimpo · Pull Request #12254 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/10757 | RPC: Introduce getblockstats to plot things by jtimon · Pull Request #10757 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/13011 | Cache witness hash in CTransaction by MarcoFalke · Pull Request #13011 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/13097 | ui: Support wallets loaded dynamically by promag · Pull Request #13097 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/12979 | Split validationinterface into parallel validation/mempool interfaces by TheBlueMatt · Pull Request #12979 · bitcoin/bitcoin · GitHub
< wumpus> there's quite a lot of things on the list yet, should we also remove something?
< gribble> https://github.com/bitcoin/bitcoin/issues/12634 | [refactor] Make TransactionWithinChainLimit more flexible by kallewoof · Pull Request #12634 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/12196 | Add scantxoutset RPC method by jonasschnelli · Pull Request #12196 · bitcoin/bitcoin · GitHub
< BlueMatt> #13011 looks merge-ableish
< gribble> https://github.com/bitcoin/bitcoin/issues/13011 | Cache witness hash in CTransaction by MarcoFalke · Pull Request #13011 · bitcoin/bitcoin · GitHub
< jtimon> hi
< BlueMatt> I dunno if #12254 should stay on there, there's now discussion of the bip on the ml so its gonna be some time yet, I think
< BlueMatt> jimpo: thoughts?
< gribble> https://github.com/bitcoin/bitcoin/issues/12254 | BIP 158: Compact Block Filters for Light Clients by jimpo · Pull Request #12254 · bitcoin/bitcoin · GitHub
< jonasschnelli> What is the maxlen for high-prio-list?
< BlueMatt> 1 per regular contributor :p
< wumpus> ok added #12196 #13142
< gribble> https://github.com/bitcoin/bitcoin/issues/12196 | Add scantxoutset RPC method by jonasschnelli · Pull Request #12196 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/13142 | Separate IsMine from solvability by sipa · Pull Request #13142 · bitcoin/bitcoin · GitHub
< jonasschnelli> Thanks wumpus
< jonasschnelli> Agree with BlueMatt about 12254
< wumpus> yes, what BlueMatt says, though PRs that are not actively updated should be removed
< wumpus> agree, removed 12254
< jtimon> I was expecting https://github.com/bitcoin/bitcoin/pull/10757 to be merged soonish and thus go out of the list
< BlueMatt> topic: 0.16.1
< promag> and I guess 13097 can be merged after jonasschnelli review
< wumpus> something that is still being discussed on the mailing list certainly doesn't belong in the blocker slist
< jonasschnelli> Yes. 13097 is in review here...
< jimpo> Can I get #13243 then so progress continues? :-)
< gribble> https://github.com/bitcoin/bitcoin/issues/13243 | Make reusable base class for auxiliary indices by jimpo · Pull Request #13243 · bitcoin/bitcoin · GitHub
< MarcoFalke> #12979 needs a rebase
< sipa> sgtm
< gribble> https://github.com/bitcoin/bitcoin/issues/12979 | Split validationinterface into parallel validation/mempool interfaces by TheBlueMatt · Pull Request #12979 · bitcoin/bitcoin · GitHub
< BlueMatt> MarcoFalke: yup, doing now
< BlueMatt> i was waiting on sdaftuar's review so I could take nits at the same time
< jonasschnelli> unicorn for 10757
< sipa> great.
< BlueMatt> topic: replacing github
< jonasschnelli> heh
< promag> wumpus: fyi 13063 on high priority after 13097 merge
< BlueMatt> topic: replacing github (not entirely unserious)
< jonasschnelli> don't make queues for highprio list. :)
< wumpus> promag: you already have one on the list!
< promag> wumpus: right, after 13097 merge
< wumpus> due to the length of the list I'm going to have to enforce one PR per person, sorry
< wumpus> #topic 0.16.1
< BlueMatt> huh? we always enforce one per person (well, one nomination per person, you can nominate someone elses')
< BlueMatt> I think we just need to finish backports and tag for 0.16.1rc1, no?
< wumpus> mostly #13253
< gribble> https://github.com/bitcoin/bitcoin/issues/13253 | [0.16] Further Backports by fanquake · Pull Request #13253 · bitcoin/bitcoin · GitHub
< wumpus> BlueMatt: we had multiple theuni PRs on there at some point :)
< BlueMatt> well those got removed, and cfields confirmed that was ok
< wumpus> there's also three issues marked 0.16.1: #13110 #12646 #12337
< gribble> https://github.com/bitcoin/bitcoin/issues/13110 | 0.16.0 bitcoin-qt: "Assertion `copyFrom failed" during launch · Issue #13110 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/12646 | Assertion failure during rescan · Issue #12646 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/12337 | 0.16 Shutdown assertion · Issue #12337 · bitcoin/bitcoin · GitHub
< wumpus> not sure whether they are critical or can just be bumped to 0.16.2 or so
< sipa> we have one issue marked 0.15.2 which i don't understand
< wumpus> I don't know either, but at least that's not a blocker for 0.16.1
< BlueMatt> jonasschnelli: was the last to comment on 12337 " I'll try to find a solution for this..."
< MarcoFalke> wumpus: The assertion is a regression if I am not mistaken
< MarcoFalke> 13110
< MarcoFalke> wasn't 12337 fixed?
< jonasschnelli> Will look into 12337...
< wumpus> I proposed a fix in 13110 and it apparently worked
< cfields> yes
< wumpus> jonasschnelli: sure
< wumpus> so that leaves 12646
< jonasschnelli> maybe jnewbery can look into 12646?
< wumpus> anyone want to look int othat? or can we just bump it forward if it's not so importent
< jonasschnelli> I think its okay to bump this forward (as long as we track it)
< wumpus> yes I don't mean closing it
< jnewbery> Yes, I can look at 12646 (next week)
< wumpus> moved to 0.16.2
< wumpus> other proposed topics?
< BlueMatt> trashing github
< wumpus> #topic Trashing github
< BlueMatt> so it hasnt been working for like 3 weeks now
< BlueMatt> and I'd kinda like to have something self-hosted with better review tools anyway, which I know a lot of people wanted
< jonasschnelli> Should we give it more time?... I'm pretty sure they are aware of it
< sipa> there was some suggestiin (was it on twitter) to use gitlab
< BlueMatt> so I figured we should do a "do people think its actually a good idea to switch to something self-hosted" semi-poll
< wumpus> gitlab seems ok
< BlueMatt> or we could switch to gitlab
< jonasschnelli> BlueMatt: what alternatives would you propose?
< jtimon> I like gitlab
< sdaftuar> seems to me like it's way harder to get it right hosting ourselves
< BlueMatt> though gitlab seems to have no better review tools than github
< wumpus> self-hosted I don't know, who is going to babysit this, monitor it and apply security patches etc?
< sipa> it would e really cool if all pr comment history was in git too
< BlueMatt> sdaftuar: oh? I mean I kinda disagree
< sdaftuar> dude we can't even keep the computers in our office running
< jamesob> self-hosted is very risky and potentially time-consumptive IMO
< BlueMatt> sipa: does gitlab do that?
< BlueMatt> sdaftuar: bitcoincore.org does just fine....
< sipa> BlueMatt: i have no clue
< wumpus> if no one is, it's going to become worse, not better, at least Github has a dedicated paid team
< sdaftuar> definitely agree with wumpus
< cfields> general nack, self-hosting issues aside, Github's network effect is too strong imo.
< sipa> blockstream uses gitlab internally, which seems to work fine (pribably due to people maintaining it)
< BlueMatt> i mean we could do self-hosted gitlab
< MarcoFalke> what advantage does that give, BlueMatt?
< jtimon> I assume the goal is less unicorns?
< jnewbery> cfields: +1
< wumpus> someone from github promised to look into the unicorn issue, maybe we should give them some more time
< MarcoFalke> gitlab also does hostign for free
< jimpo> Cursory internet search turned up reviewable.io, which is like a hosted layer on top of GitHub
< sipa> jnewbery, cfields: what do you suggest instead? waiting until github fixes it?
< jimpo> free for public repos
< BlueMatt> MarcoFalke: we can do our own security additions like putting the pr and comment history in git
< cfields> I can't be the only one who gets irrationally frustrated when the code I want to mess with is on BitBucket..
< BlueMatt> and have stuff that verifies it
< sipa> cfields: we'd mirror on github of course
< jnewbery> In the absence of something better, we should continue nagging them
< wumpus> cfields: yes, only players like freedesktop can really afford to host on separate infrastructure, for smaller projects the lack of network effect (and having to register separately) is bad
< cfields> sipa: all things considered, yes, I'd say waiting it out makes the most sense.
< BlueMatt> jnewbery: I think we *do* have ideas for better things
< cfields> I think I'm in the minority there, though :)
< jonasschnelli> Moving away from GitHub seems meh,... especially for a self-hostes solutions... looks after centralizing development
< jimpo> Who has reached out to GitHub and through what channel so far?
< BlueMatt> the self-hosting question is more a "will it be maintained" question
< BlueMatt> not "will it be better"
< jtimon> I'm ok with both people working on a gitlab instance and people nagging github devs
< sipa> cfields: i'moretty unconfortable with the fact that network effect is making us stick with a specific provider, even in the oresence of obvious issies
< sipa> of course, it's not like we could migrate quickly anyway
< wumpus> 'will it be maintained' is really important though to not end up blaming each other
< jamesob> how much effort will, e.g., DoS protection be for something self-hosted?
< wumpus> at least now we can blame github people :)
< sipa> haha
< jtimon> we could perhaps save money on travis workers by using gitlab-CI too?
< wumpus> jamesob: exactly...
< BlueMatt> jamesob: dos protection is 6$/month
< BlueMatt> and works perfectly
< cfields> sipa: it's worth considering that the 0.16.1 issues might've never been reported if not for Github's issues
< jnewbery> BlueMatt: Yes, we have ideas, but that's different from something that's actually running. I don't have any interest in maintaining a self-hosted solution, and I don't think it's worth anyone else's time doing it either
< sipa> cfields: that's a good point
< wumpus> we could still use github for *issues*
< wumpus> gitlab would be for patches and review
< jtimon> jnewbery: yeah, I'm personally not interested in maintaining it either
< wumpus> doesn't necessarily need to be the same place
< sipa> let's move back to sourceforge
< wumpus> yes tbh I don't think we should change the issue reporting URL
< BlueMatt> I'm not a fan of using issues and patches/review being in separate places
< wumpus> I've been spamming that to so many people over the years
< jamesob> BlueMatt: I don't think CloudFlare works with git protocol, so you need to reveal underlying IPs: https://stackoverflow.com/questions/31817004/git-push-not-working-after-using-cloudflare-reverse-proxy
< jonasschnelli> if the unicorns is the showstopper, then better mirror github PR with comments > 20 via API with comment through API function
< jtimon> or just everything on gitlab, but since we have the github mirror, we will see issues created there by people who missed that the project moved
< wumpus> I don't really want to move it anywhere else. THe unicorns are only an issue for code review.
< BlueMatt> jamesob: bitcoincore.org does not use cloudflare (and costs 6$/month), cloudflare sucks ass
< jnewbery> I also think that the network effect thing is important. What percentage of new contributors/issue reports would we lose if we weren't on github?
< BlueMatt> jamesob: (and that's for redundant providers)
< BlueMatt> jonasschnelli: I'd actually be much happier with github if we had a client-side api-based cli-only github interface
< BlueMatt> (that verified eg pgp signatures on comments)
< wumpus> there is a github cli interface
< jtimon> wumpus: well the main point of using github is for code review, no?
< kanzure> email seems to work for long reviews (diffs)
< jonasschnelli> BlueMatt: that seems easyer then installing a gitlab solution on a custom server
< jonasschnelli> *easier
< BlueMatt> jonasschnelli: its the difference between building a whole app and installing one
< BlueMatt> so...no, not really
< jimpo> jnewberry: Who has reached out to GitHub and through what channel so far?
< moneyball> I am happy to follow-up with GitHub to try and accelerate a fix. Can someone provide me background info on the existing communication we have with GitHub?
< jnewbery> I've contacted Github support. I don't know who else has
< jtimon> I think someone reached to them on twitter too
< wumpus> I have contacted them through tthe contact site, and was told by support that many others have
< moneyball> Is there an open issue # that I can reference?
< jnewbery> jtimon: that was me
< jimpo> I can try asking someone I know that used to work there supporting open source projects
< jnewbery> moneyball: I'll forward you the email thread
< BlueMatt> moneyball: a few folks have emailed support@github, which historically has always gotten a response, some other projects were posting responses they got where they were saying "we dont actually know what change we made that triggered these issues, hold on"
< BlueMatt> buts its been like 3 weeks ago
< moneyball> jnewbery ok great i'll use that as context and follow-up
< moneyball> jimpo we can coordinate our efforts if you'd like
< jimpo> yeah, we can chat about it outside the meeting
< BlueMatt> ok, so it sounds like consensus is "stick with the broken shit we have now" :/
< BlueMatt> or at least no consensus on moving to something else
< wumpus> feel fre to set up something better and convince us to use it
< sipa> yeah i believe it will require someone to set up a demo
< wumpus> if not, this is just empty talk, we *have* nothing better
< wumpus> any other topics?
< MarcoFalke> And a plan to transition all open patches to the new review system?
< sipa> right; the question is whether we should look into alternatives
< sipa> not so much whether we should or shouldn't move
< wumpus> right
< cfields> iirc some alternatives support oath login via github
< BlueMatt> yea, it seems like lack of consensus to move even if we find something good
< cfields> that would go a long way towards shutting me up
< BlueMatt> which was mostly why I brought it up
< wumpus> I'm open to being convinced to use something else, if it's really better
< jimpo> cfields: I agree a code review tool with GitHub integration (where main repo is still hosted there) is ideal
< jnewbery> BlueMatt: If there was something else better running now AND there was a way to migrate all state AND we wouldn't lose contributors by switching AND we have someone committed to maintaining it, then I'd be open to it. Without that, I think it's a non-starter.
< jonasschnelli> Something like https://github.com/piotrmurach/github_cli seems a better start to deal with the unicorns
< wumpus> jonasschnelli: yes, I plan to look into github cli commands as well, there's a few (also "hub" IIRC)
< MarcoFalke> Agree with jnewbery. I am open to switching, but not without solving the transition issues first.
< wumpus> we'd end up with parallel infrastructure for a while anyway
< jtimon> well BlueMatt I think it would be easier to get consensus to move to something else if somebody had it working with CI and all
< jnewbery> topic request: separate wallet from node (#10973)
< gribble> https://github.com/bitcoin/bitcoin/issues/10973 | Refactor: separate wallet from node by ryanofsky · Pull Request #10973 · bitcoin/bitcoin · GitHub
< jonasschnelli> Moving away from Github just because of load issues for three weeks seems insane...
< BlueMatt> jtimon: it sounds like definite "no", so I'm not gonna spend time looking into it
< jtimon> in the meantime, prs with many comments get the unicorn and we have to try again until it loads
< wumpus> #topic separate wallet from node (jnewbery)
< BlueMatt> jonasschnelli: there are way more reasons people dont like github
< sipa> jonasschnelli: not being able to move away from github just because of network effect seems scary...
< jamesob> the upside is that this is an additional incentive to keep PRs small :)
< jtimon> BlueMatt: fair enough
< cfields> jonasschnelli: I agree. But I think the frustration comes from the helplessness that it's brought to light.
< wumpus> this discussion is starting to repeat itself
< jonasschnelli> sipa: that is a point we should take into consideration,.. but stop focusing on load issues
< sipa> ok, next topic it seems
< BlueMatt> oh, I have 2 more topics....
< wumpus> I don't think there's realistically any chance of anything replacing github until someone sets up a feasible alternative and shows us that it is better
< sipa> jnewbery: your topic
< jonasschnelli> (#topic separate wallet from node (jnewbery))
< BlueMatt> sipa: well I'm gonna look into having a cli app that checks signatures off github api comments/etc
< jnewbery> #10973 is a big PR, but I think it's very worthwhile
< BlueMatt> cause I think that would alleviate a lot of it
< gribble> https://github.com/bitcoin/bitcoin/issues/10973 | Refactor: separate wallet from node by ryanofsky · Pull Request #10973 · bitcoin/bitcoin · GitHub
< jnewbery> jamesob and I have both reviewed now, but it requires continual rebase
< jnewbery> There are a lot of PRs competing for high priority for review, but I think it'd be great to make some progress on this one
< promag> place in high priority this week?
< jamesob> I'll start a round of manual testing if we can get another reviewer or two
< jimpo> can it be broken up at all?
< jnewbery> so, next steps would be: concecpt ACK/NACKs
< BlueMatt> sounds like a high-priority-for-review nomination?
< jnewbery> and if people think it's too much to review in one go, ryanofsky is happy to split
< jimpo> 1500+ lines is too big IMO
< wumpus> oh no, not more high priority for review
< jonasschnelli> jnewbery: this is orthogonal of using light client mode to decouple the wallet from the node? right,... it's more architectural?
< jnewbery> jimpo: it's mostly very mechanical changes, but yes it's a large +-loc
< wumpus> is it blocking anything? is it importnat for 0.17?
< jnewbery> there are only a couple of commits that require deep review
< sipa> jnewbery: thanks for bringing it up; big PRs are sometimes unnecessarily scary to dig into
< jnewbery> wumpus: it blocks (but doesn't necessarily have to lead to) process separation
< jonasschnelli> But I guess it's hard/impossible to break it into smaller PRs
< jamesob> jonasschnelli: it makes explicit the bitcoind interface that the wallet relies on, which is in itself useful but also if we want to do any kind of process separation
< jnewbery> jonasschnelli: you're correct
< wumpus> process separation is not something we'll have for 0.17 anyway
< jnewbery> so at this stage it's more of a concept review beg from me, and a poll on whether russ should spend time splitting it up
< jimpo> I'll give it a pass by tomorrow
< jnewbery> The reason I raised it is that because of the frequent rebases, reviews go stale very quickly, and it's now had two ACKs
< jnewbery> thanks jimpo
< ryanofsky> i actually don't think it's hard to split up, first 6 commits seem to group together nicely, with rest of changes more independent
< jnewbery> that's all from me. If 2 or 3 regular reviewers are happy to concept review, I think that's good progress
< jonasschnelli> If we assume the long term goal is process separation (where the wallet will turn into a pure transaction-filtering-thinkg), isn't it, that the coin-selection & signing elements in this interface will get throw away later?
< BlueMatt> topic: unverified-block-message
< BlueMatt> topic: queue drain lock assertions to avoid deadlocks
< wumpus> #topic unverified-block-message (BlueMatt)
< ryanofsky> jonasschnelli, there aren't any coin-selection or signing things in node/wallet interface in that pr
< jonasschnelli> maybe I should review the PR first...
< sipa> BlueMatt: your topic
< BlueMatt> so sdaftuar pointed out an old issue that we never really addressed where if you relay someone a script-invalid block they may announce it to their peers via compact blocks high-bandwidth-mode and then if you for some reason fall back to downloading it via getdata due to short id collision or so (we dont think there is a way to game it), then you'll end up disconnecting that peer for never responding to your request
< ryanofsky> jonasschnelli, yeah, you can just take a look at https://github.com/ryanofsky/bitcoin/blob/pr/wipc-sep/src/interfaces/chain.h to see the interface (first link in PR description)
< jtimon> re 10973 I agree I would preffer a few smaller ones, specially if there's commits that needs deeper review
< BlueMatt> we only came up with two real potential solutions: a "no, I'm not gonna send you that block" message (ie a not-batshit-insane reject message) or a "here is a block that may be invalid, but is valid pow+merkle root ala compact blocks requirement" message
< BlueMatt> or, I guess the second one is a getdata type
< sipa> BlueMatt: we have "notfound" also
< BlueMatt> sipa: isnt that a reject type?
< sipa> no
< wumpus> NetMsgType::NOTFOUND
< jnewbery> *5 chaincoders furiously grep for notfound*
< sipa> it's just a "i can't give you these INVs"
< sdaftuar> oh wow
< BlueMatt> sipa: ah, but we dont use it for blocks
< BlueMatt> only txn
< sipa> ah
< BlueMatt> still, easier would be a "here is a block that may be invalid" as that would remove the ABC in ProcessGetBlockData
< wumpus> the client ignores it just the same though
< cfields> BlueMatt: so, lots of NOTFOUNDs :)
< sipa> BlueMatt: we should have had that before compact blocks, i guess
< BlueMatt> sipa: should have had what?
< BlueMatt> sdaftuar: points out that it can be wholly independant, just a new getdata type
< sipa> a possibly invalid block relay
< sdaftuar> i think we could just add a new BLOKC response type
< BlueMatt> well or both or whatever
< sipa> 0xDEADB10C
< sdaftuar> where if someone requests a block but you don't know if its valid, or you know it's invalid, you return a different message containing the block to indicate that
< wumpus> hehe
< BlueMatt> yea, so old nodes would ignore it, and you'd just be sending a new message type
< sdaftuar> currently we would let the peer time us out instead
< sipa> sdaftuar: but only if you know they support such a message
< BlueMatt> or not
< sdaftuar> meh, sure, but not even needed imo
< BlueMatt> doesnt really matter
< BlueMatt> I mean they're about to disconnect you either way
< sipa> fair
< sipa> that makes compatibility really easy
< sipa> i guess... suggest something and write a bip?
< BlueMatt> oh, wait, no, you also want a new getdata type
< BlueMatt> cause otherwise you still need the ActivateBestChain in ProcessGetBlockData
< BlueMatt> which would require negotiation
< sdaftuar> yeah ok
< BlueMatt> so either that, or we start using NOTFOUNDs, I guess
< BlueMatt> I'm not sure what I prefer, so.....thoughts?
< sdaftuar> i think notfounds are worse because of the case where the block might not have been validated either way
< sdaftuar> it complicates download logic
< wumpus> if there is no specific reason to re-use NOTFOUND, a new message is much better imo
< BlueMatt> yea, its not really "notfound" its "I may find this soon"
< BlueMatt> or, really, "I dont currently find this"
< sipa> i don't think we should do protocol design in this meeting
< BlueMatt> well, there's a few options, so...good to ask
< wumpus> not the design but discussing it as a concern is valid
< wumpus> agree BlueMatt
< BlueMatt> obviously requires BIP and whatever else
< wumpus> yes
< wumpus> #topic queue drain lock assertions to avoid deadlocks (BlueMatt)
< BlueMatt> this one is less interesting, i realize now I should just open a pr and people will see it
< BlueMatt> its kinda knotty to describe
< BlueMatt> but, essentially, if you call ABC within a validationinterface callback you're hosed
< wumpus> ok, well only 4 minutes t ogo anyhow
< BlueMatt> which sucks, but I dont think we have a way to fix it
< BlueMatt> so current approach is to document it and DEBUG_LOCKORDER-assert
< BlueMatt> we can relax the requirement a bit with skeees' proposed validation-in-its-own-thread stuff
< BlueMatt> but there will still be some call restrictions
< BlueMatt> ok endtopic
< wumpus> #endmeeting
< lightningbot> Meeting ended Thu May 17 19:58:35 2018 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< cfields> my only request (same as always) is avoiding "if(on_thread1) foo() else bar()" type logic
< BlueMatt> cfields: nah, more a DUMMY_LOCK(on_thread1); AssertLockNotHeld(on_thread1) :p
< promag> jonasschnelli: will fix
< jonasschnelli> thx
< promag> np, see you later
< bitcoin-git> [bitcoin] laanwj opened pull request #13265: wallet: Exit SyncMetaData if there are no transactions to sync (master...2018_05_wallet_syncmetadata_empty_range) https://github.com/bitcoin/bitcoin/pull/13265
< gmaxwell> cfields: I dunno if you discussed this with pieter before, but an alternative to your UHS that I suggested to him previously was that if you have a per-node secret salt S, then for all "hash-like" scriptpubkey types, you could store as a key H(s || script-type || txid || vout || scriptpubkey-data)[0:16] and value is just the compact_amount. This would allow storing almost all utxo in about 20
< gmaxwell> bytes, have a reasonable security argument, and require no network communications overhead.
< achow101> damn, I forgot there was a meeting today
< achow101> wumpus: can I get #12136 for high prio?
< gmaxwell> The security argument is that with an attacker unknown salt S, the only property you need from the hash resistance to chance collisions. (also even if there were one, it would just break a single node).
< gribble> https://github.com/bitcoin/bitcoin/issues/12136 | Implement BIP 174 Partially Signed Bitcoin Transactions by achow101 · Pull Request #12136 · bitcoin/bitcoin · GitHub
< jamesob> #13219 seems to have high review-effort:reward ratio in case anyone's bored
< gribble> https://github.com/bitcoin/bitcoin/issues/13219 | bench: Add block assemble benchmark by MarcoFalke · Pull Request #13219 · bitcoin/bitcoin · GitHub
< gmaxwell> cfields: if it isn't obvious how you verify against it, when a transaction comes in, you take its input txin and vout indexes, and look at the scriptsig and construct the appropriate scriptpubkey from it. Then check if thats in the database.
< jamesob> *low, ugh
< cfields> gmaxwell: yea, we discussed that as well. I'm not opposed, but I wanted to introduced the idea as a near drop-in for the status-quo, security wise. That salt ends up acting as a private key somewhat, which is not major but not trivial either
< cfields> gmaxwell: please mention that on the thread though. I just wanted to introduce the paradigm, I'm sure there are lots of better ways to execute it :)