< 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
< 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
< 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
< 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
< 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
< 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
< 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)
< 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
< 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"
< 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?
< 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
< 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
< 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 :)
< 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 ;)
< 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