< luke-jr> fjahr: do we need #17843 in 0.19?
< gribble> https://github.com/bitcoin/bitcoin/issues/17843 | wallet: Reset reused transactions cache by fjahr . Pull Request #17843 . bitcoin/bitcoin . GitHub
< bitcoin-git> [bitcoin] fanquake opened pull request #18081: test: set a name for CI Docker containers (master...name_ci_docker_containers) https://github.com/bitcoin/bitcoin/pull/18081
< bitcoin-git> [bitcoin] fanquake opened pull request #18082: logging: enable thread_local usage on macOS (master...macos_thread_local) https://github.com/bitcoin/bitcoin/pull/18082
< bitcoin-git> [bitcoin] luke-jr opened pull request #18083: [0.19] wallet: Reset reused transactions cache (0.19...bugfix_reused_tx_cache-0.19.1) https://github.com/bitcoin/bitcoin/pull/18083
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/8a56f79d4912...4d211c8da112
< bitcoin-git> bitcoin/master acd644b fanquake: build: remove --large-address-aware linker flag
< bitcoin-git> bitcoin/master 4d211c8 fanquake: Merge #18003: build: remove --large-address-aware linker flag
< bitcoin-git> [bitcoin] fanquake merged pull request #18003: build: remove --large-address-aware linker flag (master...configure_large_address_aware) https://github.com/bitcoin/bitcoin/pull/18003
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/4d211c8da112...23fab1a3dfe6
< bitcoin-git> bitcoin/master acf8abc Joao Barbosa: gui: Fix unintialized WalletView::progressDialog
< bitcoin-git> bitcoin/master 23fab1a fanquake: Merge #18062: gui: Fix unintialized WalletView::progressDialog
< bitcoin-git> [bitcoin] fanquake merged pull request #18062: gui: Fix unintialized WalletView::progressDialog (master...2020-02-fix-unitialized-progressDialog) https://github.com/bitcoin/bitcoin/pull/18062
< goatpig> hello
< goatpig> what's the current dust limit?
< bitcoin-git> [bitcoin] promag opened pull request #18084: 0.19: gui: Fix unintialized WalletView::progressDialog (0.19...2020-02-backport-18062) https://github.com/bitcoin/bitcoin/pull/18084
< jonasschnelli> goatpig: see DUST_RELAY_TX_FEE
< jonasschnelli> and -dustrelayfee= config value
< jonasschnelli> it's default is currently 3000sats in master AFAIK
< jonasschnelli> 3000sats/kB to be more precise
< wumpus> promag: I'm not aware of any plans to bump the minimum qt version again, if you want to propose that feel free to open a PR, make sure you check for various linux distributions, I don't think 5.10+ is realistic as ubuntu bionic (18.04) still has 5.9 and no doubt others
< bitcoin-git> [bitcoin] laanwj closed pull request #17807: net: Remove unnecessary portability typedef (master...akh_socket_arg_type) https://github.com/bitcoin/bitcoin/pull/17807
< bitcoin-git> [bitcoin] laanwj reopened pull request #16995: Fix gcc 9 warnings (master...2019_09_resolve_gcc_warnings) https://github.com/bitcoin/bitcoin/pull/16995
< bitcoin-git> [bitcoin] laanwj closed pull request #18053: Silence "redundant move in return statement" warning on GCC9 (master...gcc9-silence-redundant-move) https://github.com/bitcoin/bitcoin/pull/18053
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to 0.19: https://github.com/bitcoin/bitcoin/compare/755b0734bb50...7d53995ff2b4
< bitcoin-git> bitcoin/0.19 b4e5363 Joao Barbosa: gui: Fix unintialized WalletView::progressDialog
< bitcoin-git> bitcoin/0.19 7d53995 fanquake: Merge #18084: 0.19: gui: Fix unintialized WalletView::progressDialog
< bitcoin-git> [bitcoin] fanquake merged pull request #18084: 0.19: gui: Fix unintialized WalletView::progressDialog (0.19...2020-02-backport-18062) https://github.com/bitcoin/bitcoin/pull/18084
< MarcoFalke> #proposedmeetingtopic (short topic) Remove i686 Linux download link from bitcoincore website
< promag> wumpus: sure, too soon then, lets see next year
< jonatack> Would someone kindly restart Travis for #17812
< gribble> https://github.com/bitcoin/bitcoin/issues/17812 | config, net, test: asmap functional tests and feature refinements by jonatack . Pull Request #17812 . bitcoin/bitcoin . GitHub
< luke-jr> jonatack: done
< jonatack> luke-jr: thank you
< wumpus> #startmeeting
< lightningbot> Meeting started Thu Feb 6 19:00:00 2020 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< MarcoFalke> ahoy
< wumpus> I don't expect many people to be here today with the London conference, but we can try...
< * BlueMatt>
< sipsorcery> hi
< sipa> hi
< jonasschnelli> hi
< emilengler> hi
< amiti> hi
< hebasto> hi
< * BlueMatt>
< wumpus> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator aj Chris_Stewart_5 dongcarl gwillen jamesob ken281221 ryanofsky gleb moneyball kvaciral ariard digi_james amiti fjahr
< wumpus> jeremyrubin lightlike emilengler jonatack hebasto jb55
< fjahr> hi
< wumpus> one proposed topic on https://gist.github.com/moneyball/071d608fdae217c2a6d7c35955881d8a: Remove i686 Linux download link from bitcoincore website (MarcoFalke)
< aj> hi
< wumpus> any last minute topic proposals?
< jeremyrubin> hi
< jeremyrubin> proposed topic: mempool project update
< jonatack> hi
< wumpus> jeremyrubin: ack
< moneyball> hi
< kanzure> hi
< jkczyz> hi
< kanzure> proposed topic: more topic selection (or actually, how about topics you don't want to hear about for march)
< promag> hi
< emilengler> proposed topic: the library for #17950, even if to use a library?
< gribble> https://github.com/bitcoin/bitcoin/issues/17950 | gui: Check the strength of an encryption password by emilengler . Pull Request #17950 . bitcoin/bitcoin . GitHub
< wumpus> #topic High priority for review
< wumpus> https://github.com/bitcoin/bitcoin/projects/8 -> 6 blockers, 1 bugfix, 6 chasing concept ACK
< wumpus> anything to add / remove or almost ready for merge?
< meshcollider> hi
< wumpus> hi
< MarcoFalke> The cs_main cs_wallet thing needs rebase and has something proposed by ryanofsky as a preparation pull request. Should these be exchanged?
< * luke-jr> glances at some chirping crickets
< wumpus> MarcoFalke: I suppose that makes sense, if the other one is a blocker for this one
< wumpus> either that or add it too
< MarcoFalke> #17954 is the prep I think
< gribble> https://github.com/bitcoin/bitcoin/issues/17954 | wallet: Remove calls to Chain::Lock methods by ryanofsky . Pull Request #17954 . bitcoin/bitcoin . GitHub
< jonasschnelli> half of the PR in high-prio do fail CI
< MarcoFalke> jonasschnelli: travis s390x?
< wumpus> ok, added
< luke-jr> maybe we should disable the s390x temporarily?
< ryanofsky> MarcoFalke, yes my preference would be to do 17954 first
< jonasschnelli> MarcoFalke: I don't know but makes people ignore CI (which is a QA issue in the long run
< MarcoFalke> Yes, maybe we should disable it on travis for now
< MarcoFalke> I do run it locally
< hebasto> How valuable is s390x tests?
< * jonasschnelli> think the CI should be less fragile
< wumpus> hebasto: big-endian testing
< MarcoFalke> jonasschnelli: travis is the only one that offers s390x native
< wumpus> that's basically the only reason s390x testinig is valuable
< luke-jr> too bad Travis doesn't have ppc64be
< jonasschnelli> I think its very valuable
< wumpus> no one runs bitcoind on that platfor mas far as I'm aware
< wumpus> so any other big-endian platform would do as well
< sipa> yeah, even if we expect literally noone ever to use bitcoin core in production on s390x, variety in test platforms can often expose bugs present but undetectable on other platforms
< MarcoFalke> I do run it, but it is through qemu
< MarcoFalke> sipa: It did find a bug in the tests :)
< sipa> MarcoFalke: the best kind of bug
< MarcoFalke> +1
< wumpus> especially for serialization changes it's very useful to test big endian correctness
< jonasschnelli> Maybe its not ready for a per branch update/PR base but as a manual check?
< jonatack> jonasschnelli: btw thank you for https://bitcoinbuilds.org. it is the first place i look for CI results.
< jonasschnelli> jonatack: It's running again smoothly.. but no native s390x supper...
< jonasschnelli> could try to get a qemu be env up. But I guess it will be too slow
< sipa> may be useful to run s390x qemu on master on a regular basis, but not on every PR?
< wumpus> power can can be big-endian as well, though, it's fairly rare (and travis doesn't offer that)
< jonasschnelli> +1
< MarcoFalke> If someone is knowledged in docker, #18016 is the problem we need fixed
< gribble> https://github.com/bitcoin/bitcoin/issues/18016 | travis: s390x ci build fails on travis because disk is too small . Issue #18016 . bitcoin/bitcoin . GitHub
< wumpus> agree sipa
< luke-jr> wumpus: not so rare; but can't be a simple chroot :/
< MarcoFalke> So I think #action is to either fix #18016 or remove it and run locally?
< gribble> https://github.com/bitcoin/bitcoin/issues/18016 | travis: s390x ci build fails on travis because disk is too small . Issue #18016 . bitcoin/bitcoin . GitHub
< wumpus> I don't think we can fix "disk too small" so that leaves removing it for now
< MarcoFalke> It can be fixed with some docker settings and restarting docker, but I don't know anything about this "docker" thing
< wumpus> me neither, I've always managed to avoid it
< MarcoFalke> There is a disk large enough in /var/snap/lxd/... on travis
< MarcoFalke> Anyway, next topic?
< wumpus> #topic Remove i686 Linux download link from bitcoincore website (MarcoFalke)
< wumpus> yes, let's do it
< jonasschnelli> ack
< MarcoFalke> So based on a twitter poll, a mailing list post, we only found one confrimed user of i686 #17504
< gribble> https://github.com/bitcoin/bitcoin/issues/17504 | Should we still ship 32-bit x86 Linux binaries? . Issue #17504 . bitcoin/bitcoin . GitHub
< MarcoFalke> So as a first step it could make sense to remove the i686 download link from the website
< emilengler> Shouldn't we wait until we don't produce any i686 anymore
< MarcoFalke> emilengler: Removing the link first gives everyone another chance to notice it
< luke-jr> ^
< emilengler> I think it's a better approach to add a note on the website and remove the link once the new version got released
< luke-jr> I'd like to be sure this doesn't turn into the Win32 situation
< MarcoFalke> emilengler: The i686 bin will still be uploaded for now
< luke-jr> the plan there afaik was simply to stop making binaries, but now we're removing the ability to even compile it
< MarcoFalke> luke-jr: That is not going to happen
< wumpus> we need to support 32 bit for self-compiles, period
< luke-jr> k
< MarcoFalke> luke-jr: We have a i686 centos build in ci, that is not going to be removed
< wumpus> ARM 32 bit is not dead, and neither is RISC-V 32 bit, and some others
< jonasschnelli> Indeed. There are a lot of Odroid in the field (Cortex A15).
< wumpus> I think I've been very clear everywhere that this is about the shipped binaries
< wumpus> for x86 32 bit
< luke-jr> wumpus: right, but that was true for Win32 too
< wumpus> win32 is really dead anyhow
< emilengler> The topic was to remove it from the website and nothing else. I feel this discussion drives a bit away to a more general topic about x86 in general. Could we come back to the original point?
< hebasto> even not all libs are available for x86
< luke-jr> hebasto: ?!
< harding> Removing it from the website to see if anyone complains while it's still easy to add it back makes sense to me.
< hebasto> see Centos 32-bit repo
< luke-jr> hebasto: that's too vague to mean anything to me
< wumpus> yes, so let's remove it from the website, but still build x86_32 binaries for 0.19.1
< MarcoFalke> +1
< luke-jr> sgtm
< wumpus> then for 0.20 stop building them
< emilengler> ack
< wumpus> #topic Mempool project update (jeremyrubin)
< jeremyrubin> hola
< jeremyrubin> So the first PR of the epoch mempool series has been merged
< wumpus> congrats!
< jeremyrubin> Thanks!
< jeremyrubin> I've opened up the next step, which gets rid of this big cache we built during reorgs
< jeremyrubin> Amiti's testing framework changes are making progress & seem good to go IMO
< wumpus> good to know
< jeremyrubin> It seems like there's been not too much attention on nanobench stuff, would be good to "just do it IMO" but I don't have many downstream toolchains
< wumpus> which PR is that?
< jeremyrubin> sorry scrambling for links...
< wumpus> (Amiti's changes, I mean)
< jeremyrubin> #18037
< gribble> https://github.com/bitcoin/bitcoin/issues/18037 | Util: Allow scheduler to be mocked by amitiuttarwar . Pull Request #18037 . bitcoin/bitcoin . GitHub
< jeremyrubin> and #18011
< gribble> https://github.com/bitcoin/bitcoin/issues/18011 | Replace current benchmarking framework with nanobench by martinus . Pull Request #18011 . bitcoin/bitcoin . GitHub
< jeremyrubin> Amiti has also opened up a new PR that carves out a good chunk of functionality for rebroadcasting
< jeremyrubin> is amiti here? I can ping her
< amiti> ya Im here
< jeremyrubin> #18037
< gribble> https://github.com/bitcoin/bitcoin/issues/18037 | Util: Allow scheduler to be mocked by amitiuttarwar . Pull Request #18037 . bitcoin/bitcoin . GitHub
< wumpus> ok so add #18037 to high priority for review?
< jeremyrubin> amiti: should people be taking a look at 18037 now?
< amiti> #18038 is the rebroadcast subset
< gribble> https://github.com/bitcoin/bitcoin/issues/18037 | Util: Allow scheduler to be mocked by amitiuttarwar . Pull Request #18037 . bitcoin/bitcoin . GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/18038 | P2P: Mempool tracks locally submitted transactions to improve privacy by amitiuttarwar . Pull Request #18038 . bitcoin/bitcoin . GitHub
< jeremyrubin> Ah right sorry
< amiti> if 18038 builds on 18037, so if 18037 gets merged in current state then 18038 is ready for review
< jeremyrubin> yes so I think 18037 is hi prio and can be merged with another ack or two (it's just testing stuff)
< jeremyrubin> And then we can pull some ears (not yet hi-prio) for 18038
< jonatack> jeremyrubin: perhaps add #18044 to your mempool dashboard? https://github.com/bitcoin/bitcoin/projects/14
< gribble> https://github.com/bitcoin/bitcoin/issues/18044 | Use wtxid for transaction relay by sdaftuar . Pull Request #18044 . bitcoin/bitcoin . GitHub
< wumpus> ok
< jeremyrubin> Same goes for #18063 -- once I get a reviewer or two I'd like to put it high priority so that progress can keep moving
< gribble> https://github.com/bitcoin/bitcoin/issues/18063 | Improve UpdateForDescendants by using Epochs and Removing CacheMap by JeremyRubin . Pull Request #18063 . bitcoin/bitcoin . GitHub
< jeremyrubin> is 18044 needed for package relay?
< jonatack> no, part of it changes the mempool, up to you
< jeremyrubin> is sdaftuar here and can talk more about it?
< jeremyrubin> I'll add it but from what I can tell this one requires a BIP to move forward?
< jonatack> yes, there is a wip bip draft for now
< sipa> i believe sdaftuar is working on one
< jeremyrubin> Ok great. I'll add it to the package relay track since that's mostly sdaftuar right now anyways, but I think logically it seems more on rebroadcasting
< jeremyrubin> amiti do you have any thoughts on that? Can you review 18044?
< amiti> yeah I've started taking a look, its more about initial broadcast than rebroadcast
< jeremyrubin> (and then I think we're good on mempool project unless anyone has any questions -- please see https://github.com/bitcoin/bitcoin/projects/14 to get references for what to review & look at)
< luke-jr> ...
< jeremyrubin> ?
< wumpus> #topic the library for #17950 (emilengler)
< gribble> https://github.com/bitcoin/bitcoin/issues/17950 | gui: Check the strength of an encryption password by emilengler . Pull Request #17950 . bitcoin/bitcoin . GitHub
< wumpus> I *really* do not like introducing a dependency for this
< emilengler> thanks
< emilengler> I agree with wumpus
< jonasschnelli> Yes. Please no dependency for a gimmick feature
< wumpus> it's somewhat nice to display a measure of password strength (if people can ever agree on one), but it's not worth large changes to our build process for
< sipa> i feel that anything that self-written is going to be too ad-hoc to be useful
< wumpus> exactly
< jonasschnelli> It is already handholding...
< sipa> so either it's depending on a well-maintained library, or we don't do it at all
< jeremyrubin> I think i'd rather just *suggest* a strong password
< luke-jr> there's conceptual problems in the first place
< wumpus> maybe it's a bad idea in the first place, thinking of it, we don't want to encourage a specific kind of password scheme, this only reduces security
< luke-jr> this shouldn't be a "strong" password, it should be a memorable one
< jeremyrubin> e.g., here are 12 random words
< wumpus> that, too
< luke-jr> encrypted wallets won't stop malware, just little brother
< luke-jr> the risk of losing access outweighs the benefits of a string passphrase here
< jonasschnelli> Can we just have a short text to help people do the right thing? Or a link (less likely)?
< wumpus> it's not a brainwallet, not the entire internet can attack it, the security needed depends on how secure the wallet file is kept
< gwillen> it is very hard to make a password-strength indicator that is not at least sometimes very misleading
< wumpus> making it too strong might cause people to forgt it
< MarcoFalke> I think more people have lost coins due to forgetting too strong passwords than first getting their wallet stolen, but not their password, and then got their password cracked offline
< wumpus> which is much worse
< jonasschnelli> Indeed
< sipa> gwillen: zxcvbn seems pretty sophisticated already
< sipa> *actually
< wumpus> MarcoFalke: agree
< sipa> it's very hard because users probably don't have a good intuition for what the requirements are
< wumpus> what would be nice is a feature that makes people write down their HD seed
< wumpus> aid recovery, not make it worse
< jeremyrubin> (I'm actually recovering a wallet for someone who forgot their password, so I agree)
< MarcoFalke> yeah
< wumpus> a lot of people lose their coins either by losing their wallet or paspphrase
< sipa> if the wallet.dat file leaking is an attack vector you want to protect against, the password needs to be *far* stronger than common recommendations for website login passwords
< jonasschnelli> wumpus: you mean adding BIP39 support?
< jeremyrubin> * attempting that is, let's hope the fragments are good enough
< gwillen> sipa: as such things go, it's pretty sophisticated, but it does not know your dog's name, or your mother's maiden name, or your birthday, or your favorite book you're quoting from, or any of a number of stupid things users do that lower effective entropy
< gwillen> while not lowering apparent entropy relative to the tool's model
< luke-jr> sipa: but if the wallet.dat file leaks, you probably have a keylogger on your PC anyway, so..
< wumpus> jonasschnelli: yes I suppose
< sipa> gwillen: of course it can only give an upper bound
< gwillen> it's never presented that way, though
< sipa> anyway
< sipa> i'm in favor of just not pursuing that feature
< jeremyrubin> luke-jr: disagree with those priors
< sipa> it's too hard to do right
< luke-jr> jeremyrubin: ?
< jeremyrubin> [11:39] <luke-jr> sipa: but if the wallet.dat file leaks, you probably have a keylogger on your PC anyway, so..
< MarcoFalke> Yeah, we should recommend users use a shorter password, if anything
< sipa> luke-jr: wallet.dat files get backed up
< wumpus> luke-jr: they might copy it to a cloud service or something
< sipa> MarcoFalke: i disagree with that as well
< luke-jr> sipa: hopefully encrypted!
< jeremyrubin> I think it's relatively likely you leak your file but don't get a keylogger
< sipa> luke-jr: right, but in that case, the passphrase needs to be strong
< luke-jr> sipa: no, I mean encrpying the file itself
< jeremyrubin> e.g., keeping backups on thumb drives
< sipa> luke-jr: it's hard to assume that people will use a strong password for an encrypted backup, but then not one inside the file?
< luke-jr> perhaps we should put a suggestion to that effect somewhere
< wumpus> in any case, there's no disagreement about whether the wallet encryption is useful or not, that's not the topic
< sipa> i disagree that in general we should advise weak passwords
< wumpus> no, we shouldn't advise that
< MarcoFalke> ok, we shouldn't advise on weak passwords, but we might want to explain the tradeoffs
< sipa> MarcoFalke: yes
< wumpus> that would make sense, yes
< wumpus> add an explanation
< MarcoFalke> I.e. what the password protects against and what it does not protect against
< sipa> "Losing this password will make your funds irrecoverably lost"
< jeremyrubin> I think also saying "writing down the password in a notebook is probably better than not having one"
< jeremyrubin> Or something to that effect
< jonasschnelli> I'm all for informing (text based) rather then applying rules that only works for certain use cases
< luke-jr> jeremyrubin: it really depends on your risk exposure
< jeremyrubin> I think people don't know that the password is not a seed
< jeremyrubin> if you just write down the password but they don't have the wallet.dat it's fine
< wumpus> jeremyrubin: yep, some people are confused by that
< jeremyrubin> luke-jr: if someone remote compromises your computer but you have a sticky note with a long password on your screen you're "fine"
< wumpus> because most wallets work with seeds nowadays
< jeremyrubin> (until you type it in, but let's assume read only compromise)
< jonasschnelli> The wallet encryption should be better explained. I would not wonder if some users encrypt their watch only wallets in the hope to not leak metadata to computer wide text pattern searches, etc.
< wumpus> agree with jonasschnelli on adding explanation text
< luke-jr> you can't encrypt watch-only I thought?
< emilengler> I think it may be a good way to add a way to encrypt the wallet in the intro dialog
< jonasschnelli> Can't you?
< hebasto> explanation is good
< wumpus> only private keys are encrypted, so encrypting watch-only would be a no-op
< luke-jr> jonasschnelli: what would it even do?
< sipa> jonasschnelli: of course not
< jonasschnelli> luke-jr: I guess you can because its mostly a mixed situation
< sipa> what would there be to encrypt?
< jonasschnelli> sipa: thats exactly the problem
< jonasschnelli> people expect things are encrypted
< wumpus> the metadata is never encrypted
< jonasschnelli> while we only encrypt the keys
< jonasschnelli> Yes. But we don't tell that to users
< sipa> jonasschnelli: a no-key wallet can't be encrypted, i think?
< luke-jr> it's obvious?
< jonasschnelli> So,.. IRS grabs wallet.dat file and reads transaction comments
< wumpus> this is why software needs documentation I guess
< sipa> luke-jr: don't assume things are obvious
< jeremyrubin> luke-jr: how is it obvious?
< jeremyrubin> That they can open the wallet and see stuff and only pw to sign?
< jonasschnelli> It's not obvious to most users
< luke-jr> you open Bitcoin Core and see your metadata, without entry of passphrase
< wumpus> jonasschnelli: well it doesn't ask th password at startup, only when you send
< jonasschnelli> encryption means for most users data can't be read by someone no knowing the secret
< jeremyrubin> Actually that sounds like a 2 birds one stone thing
< jonasschnelli> wumpus: sure. But novice users don't understand that either
< jeremyrubin> If people have to put their password in more often maybe they're less likely to forget it ;)
< luke-jr> jeremyrubin: hmm!
< jonasschnelli> I think adding more explanations on how the encryption work would be great in general
< jonasschnelli> works
< MarcoFalke> Opened an issue #18085
< gribble> https://github.com/bitcoin/bitcoin/issues/18085 | doc: Explain what the wallet password does . Issue #18085 . bitcoin/bitcoin . GitHub
< jonasschnelli> Nice
< wumpus> thanks
< jeremyrubin> I think there's also a lot of room for improvements in what users have available, e.g. shamir's secret shares
< jonasschnelli> We don't encrypt the wallet, we encrypt the keys
< jonatack> I sense new options/config args in our future here
< jeremyrubin> Even though we know multisig is better, user's are really struggling to do anything better than a plaintext wallet
< luke-jr> not sure it makes sense to put any effort into pre-Taproot multisig at this point?
< sipa> jeremyrubin: that's not really an option in a model where a wallet is a file and not a seed
< wumpus> we should be careful only to add features that are actually used and useful though, don't want to end up with some GPG-like tool that does a zillion things but with a lot of pitfalls
< jeremyrubin> Maybe organizing some discussion at coredev.tech would be good about conducting some user research to improve things.
< kanzure> i'll add that to the list then.
< kanzure> empirical user testing would be interesting
< sipa> luke-jr: multisig support at this point means improving PSBT integration, i think
< jeremyrubin> Can ask some of the ideo people to come by since they've been doing key managment UXs with a lot of projects
< sipa> luke-jr: which we should definitely work on
< luke-jr> good point
< kanzure> i'd prefer to forego ideo
< jeremyrubin> sipa: you can still do point-in-time non seed backups
< jeremyrubin> kanzure: Any specific reason?
< jonasschnelli> let's not sidetrack this topic. :)
< sipa> yes please
< wumpus> maybe more apprpriate for the wallet meeting, too
< sipa> yeah
< wumpus> not that we have any more topics for today
< sdaftuar> hi - i'm back, if anyone has questions about wtxid-relay i can discus
< jeremyrubin> yay! please do
< MarcoFalke> sdaftuar: There was a question whether it was needed for package relay
< sdaftuar> i think it's a nice-to-have, but non-essential
< sdaftuar> nice-to-have only because any tx-relay protocol change we make in the future (like erlay, or dandelion, etc) should be done on wtxid-based relay
< jeremyrubin> I guess more concretely where it fits into the https://github.com/bitcoin/bitcoin/projects/14 workflow and where you think it belongs timeline wise
< sdaftuar> well, i'm probably personally gated on it, as i don't want to work on more p2p relay things based on txid-relay at this point
< jeremyrubin> Like if new rebroadcasting stuff like what amiti is working on should be done on wtxids then do we try to slot this before it
< sdaftuar> barring some reason that wtxid-relay is a problem
< jeremyrubin> ah ok; so it slots before further package relay work for you
< luke-jr> I never really understood why we didn't do wtxid-relay from the start
< sdaftuar> luke-jr: we shoudl have! the second best time is now
< jeremyrubin> we didn't have wtxids before segwit
< luke-jr> (or if I did, I forgot)
< sdaftuar> it was just more work, and we were busy
< sdaftuar> but i think it's pretty straightforward, and we should do it, ideally before we make a standardness change to segwit transactions
< jonatack> ack
< sipa> i think initially it wasn't that clear that it was needed in the first place
< sipa> and when segwit was further along, it got pushed back to "later"
< sipa> seems later is now
< wumpus> 1 minute left
< sdaftuar> yeah i'm not sure how much anyone thought about it until petertodd pointed out the issues in #8279
< gribble> https://github.com/bitcoin/bitcoin/issues/8279 | Mempool DoS risk in segwit due to malleated transactions . Issue #8279 . bitcoin/bitcoin . GitHub
< jeremyrubin> My only concern looking at the code is that a new index in maptx kinda sucks
< sdaftuar> a bit more memory, but i don't see a way around it, and i think the tradeoff is well worth the benefit
< sipa> DING DONG
< wumpus> #endmeeting
< luke-jr> could we add both entries to the same index?
< lightningbot> Meeting ended Thu Feb 6 20:00:06 2020 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< hebasto> luke-jr: missed 32-bit packages in CentOS 7 repos - https://github.com/bitcoin/bitcoin/pull/17900#issuecomment-572697411
< luke-jr> maybe no benefit
< sdaftuar> luke-jr: we need to look up by both txid and wtxid
< sdaftuar> so two keys
< gwillen> so while people are here, and speaking of PSBT which got mentioned earlier -- I would love to see more reviews of 18027
< sdaftuar> we use the boost multiindex already, which i think is pretty efficient?
< gwillen> er, #18027
< gribble> https://github.com/bitcoin/bitcoin/issues/18027 | "PSBT Operations" dialog by gwillen . Pull Request #18027 . bitcoin/bitcoin . GitHub
< jonatack> (fwiw... did we finish the blockers/hi-prio part?)
< jeremyrubin> sdaftuar: you can actually reuse the saltedtxid hasher across both indexes I think
< sipa> sdaftuar: i need to revive my use-allocator-to-count-multiindex-memory-use stuff... i'm not sure how accurate we currently are
< luke-jr> hebasto: I would assume anyone building for 32-bit is using 32-bit to do it
< luke-jr> hebasto: and not everyone uses CentOS
< sdaftuar> sipa: ah yes i have no idea how to do that, if you can advise on how to update the memory calculation better than i did in the PR, please let me know
< jonatack> ^ +1
< sdaftuar> jeremyrubin: i don't think i follow
< sipa> sdaftuar: i have some WIP code that i can probably use to verify whether or current heuristic is accurate... actually replacing it is probably harder
< aj> is #14895 really still chasing concept ack?
< gribble> https://github.com/bitcoin/bitcoin/issues/14895 | Package relay design questions . Issue #14895 . bitcoin/bitcoin . GitHub
< jeremyrubin> sdaftuar: I need to think about it a little bit. Fundamentally you want to be able to index by either TXID or WTXID
< jeremyrubin> But because it's a hash table there's a lot of extra overhead (idk what load the boost table works well till)
< jeremyrubin> Just trying to think if there's a way to be able to index by either
< jeremyrubin> Do you envision that we ever remove the txid index?
< sdaftuar> we can't do that very easily
< jeremyrubin> Or you think it's there forever for compat
< sdaftuar> because transactions reference inputs by txid
< jeremyrubin> right
< sipa> jeremyrubin: UTXOs are indexed by txid
< luke-jr> hmm
< sdaftuar> so unless someone gave you a hint for what wtxid to look for, you're screwed
< luke-jr> sdaftuar: well, only for mempool-spending txs?
< jeremyrubin> OK I'm OK with it
< jeremyrubin> BUT
< sdaftuar> i think in the case of package relay though, i might imagine that we'll get those hints, but not in a generic enough way that we could ever git rid of the index
< sdaftuar> luke-jr: right
< jeremyrubin> You have to review my next two PRs first
< jeremyrubin> Because I get rid of mapTxLinks
< luke-jr> could hypothetically use wtxids in the tx structure there, and continue to use just the txids for signatures?
< jeremyrubin> which can pay for this new index ;)
< sdaftuar> luke-jr: that would be a big relay change though
< luke-jr> maybe worth it long-term
< sdaftuar> and it just seems like a lot of edge cases would break
< sdaftuar> yeah, i can't rule it out
< sipa> i don't understand what the point is
< luke-jr> gotta run
< sipa> not having wtxids in transactions is exactly what segwit made possible
< sdaftuar> yes :)
< sdaftuar> i think saving a little memory is not worth the effort here
< kanzure> i think the point was something about rebroadcast logic or first-seen issues?
< kanzure> right, bad witnesses or something?
< jeremyrubin> Yeah, I think given that we're going to kill mapTxLinks it's going to be fine (I just don't want people to have a reason not to upgrade to wtxid index)
< jeremyrubin> kanzure: anyone can malleate witnesses
< sdaftuar> jeremyrubin: i don't think this has anything to do with mapTxLinks though? i didn't need to touch it for the wtxid-relay PR
< jeremyrubin> sdaftuar: I'm saying that one of the PRs I pinged you on kills mapTxLinks
< sdaftuar> (unless i am missing something!)
< sdaftuar> right, i imagine that should be fine
< jeremyrubin> so the memory/hashing overhead going away there is probably the same as a new index
< sdaftuar> one way to think about this is that only the net_processing layer needs to be able to look things up in the mempool by wtxid
< sdaftuar> as that's the only place in our code where we need to deteremine whether we already have a wtxid someone is offering
< jeremyrubin> So I'm OK with not introducing a regression
< sdaftuar> anything internal to the mempool is unaffected by this change
< jeremyrubin> Because we have a way to pay for it
< sdaftuar> oh, you're saying that the extra memory is a wash with those other changes? even better :)
< jeremyrubin> Yes
< kanzure> sipa: i assume this https://github.com/bitcoin/bitcoin/pull/18044
< sipa> kanzure: yes that's what we're talking about
< sipa> jeremyrubin: seems completely unrelated; we need wtxid based relay i think, and even if the only way to do it is by adding memory, we should
< jeremyrubin> sdaftuar previously said it was nice ot have but not required
< sipa> and if we can get rid of mapTxLinks, we should, unrelated of wtxid based relay
< sdaftuar> jeremyrubin: that was for package relay
< jeremyrubin> ah
< sdaftuar> i think package relay could be done with or without wtxid relay
< sdaftuar> but wtxid relay is required to solve some bandwidth-waste issues
< jeremyrubin> I thought you meant in general
< jeremyrubin> sipa: I just don't want there to be a reason for economic nodes to not upgrade that's all
< jeremyrubin> the changes are obviously independent
< sdaftuar> the issue we have with txid relay is that when a peer announces a segwit transaction that doesn't pass your policy checks, then you don't know whether another peer announcing the same txid has the same transactionr or not
< sdaftuar> because maybe just the witness was malleated
< jeremyrubin> But that we're not increasing overheads overall means we can not worry at all
< sdaftuar> so you have to download it (otherwise, an attacker could malleate transactions to interfere with relay)
< sdaftuar> and this is wasteful, particularly after a policy change to segwit-transaction-acceptance (eg taproot, or any other policy change)
< sdaftuar> when you would expect old nodes to reject a certain category of new transaction
< sdaftuar> there's also a related CPU DoS issue with how we determine whether a transaction is witness-stripped, which will be alleviated in the future when we no longer need to worry about adding txid's for segwit transactions to our reject filter
< sdaftuar> so i think we definitely need wtxid relay, even if we support txid-based-relay indefinitely to support old software
< jeremyrubin> I guess it's not clear to me why this has to be a new mempool index rather than a fixed size separate cache only in net_proc
< jeremyrubin> will look more closely
< sdaftuar> jeremyrubin: i think code simplicity?
< sipa> jeremyrubin: for my current mempool, the extra index would be a 0.55% memory usage increase
< sdaftuar> maintaining a separate data structure just to shave a few bytes doesn't seem worth the effort to me. the mempool is probably already too big
< jeremyrubin> sipa: I'm concerned with hashing too, we're slowing down all inserts
< aj> yeah, extra indexes on multi_index are pretty cheap
< sdaftuar> inserts aren't in the critical path of block acceptance, i think they're small compared to transaction validation speeds
< sipa> yeah
< jeremyrubin> Cool -- these are all good things to document & measure in advocating this change
< sdaftuar> if you want to worry about CPU usage in transaction acceptance, we should reopen by parallel-script-check-thread PR for mempool acceptance
< sdaftuar> (i do worry about it, but i think the mempool data structures are far from our biggest concern)
< sipa> just assume all transactions are valid ethereum style
< sdaftuar> :)
< jeremyrubin> sdaftuar: after I finish the 100th epoch mempool patch I'll do that
< sdaftuar> sipa: we could just have every node randomly sample transactions to run script checking on, and turn back on sending reject messages, and whenever you learn of a reject from a peer you validate it yourself
< sdaftuar> <runs away>
< sipa> haha
< jeremyrubin> sdaftuar: actually you do a ZKP over your mempool proving it has no invalid txns
< jeremyrubin> you do a interactive setup with each peer so it's trustless
< sipa> lol
< aj> jeremyrubin: and that uses less cpu, right?
< sipa> aj: you do it in the cloud
< aj> sipa: i don't trust the cloud, that makes it trustless, right?
< jeremyrubin> aj: you don't care who generates the proof for your setup string
< jeremyrubin> so you outsource proving batches of transactions correct to the miners, who get paid for getting them accepted, or the users whose txns it is
< jeremyrubin> maybe this is better for #wizards ;)
< sipa> yes
< aj> sdaftuar: did you see https://github.com/bitcoin/bitcoin/pull/17303#issuecomment-581363980 ? there's a patch there that's a different approach for #15505 ; worth trying? (seems silly to add wtxid for mapRelay if we could just get rid of mapRelay first instead)
< gribble> https://github.com/bitcoin/bitcoin/issues/15505 | p2p: Request NOTFOUND transactions immediately from other outbound peers, when possible by sdaftuar . Pull Request #15505 . bitcoin/bitcoin . GitHub
< sdaftuar> aj: i would definitely prefer to get rid of mapRelay, but i think the additional memory i propose using in #18044 is very minor, it's just an extra key
< gribble> https://github.com/bitcoin/bitcoin/issues/18044 | Use wtxid for transaction relay by sdaftuar . Pull Request #18044 . bitcoin/bitcoin . GitHub
< sdaftuar> and it should be easy to remove either before or after the wtxid-relay PR
< sdaftuar> (if we can get rid of mapRelay before, i can easily pull that commit out of my branch)
< aj> sdaftuar: oh, yeah, not a meaningful criticism
< jeremyrubin> sdaftuar: btw can you re-run your reorg benchmark on #18063 for equal comparison to prior work?
< gribble> https://github.com/bitcoin/bitcoin/issues/18063 | Improve UpdateForDescendants by using Epochs and Removing CacheMap by JeremyRubin . Pull Request #18063 . bitcoin/bitcoin . GitHub
< sdaftuar> jeremyrubin: uh i will try to dig it up again :) thanks for reminding though, i'll take a look
< jeremyrubin> if you find it and can run it commit-by-commit that would help (there are only 2)
< sdaftuar> gotcha, will give it a try