< bitcoin-git>
[bitcoin] luke-jr opened pull request #18572: Wallet: Accept "changedata" db key as an alias to "destdata" (master...changedata_forwardcompat) https://github.com/bitcoin/bitcoin/pull/18572
< wumpus>
as some people think it invokes strange undefined C++ behavior maybe that's better, though I'm not sure it's strictly worse than calling optional instruction sets in the initializers
< wumpus>
but I don't want to block 0.20 for it for days
< wumpus>
fanquake: we might want a linter at some point that checks for init.* sections in the object files for optional instruction sets
< fanquake>
wumpus: interesting
< wumpus>
i mean, .text.startup sections
< wumpus>
a quick hack using "find -name \*.o -print0 | xargs -0 grep -l '.text.startup'" doesn't seem to find any others that might be problematic
< fanquake>
Was playing around with this for #17929. We are going to swap to just passing the optimization flags to the linker, and I was examining the differences in the binaries.
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #18561: test: Properly raise FailedToStartError when rpc shutdown before warmup finished (master...2004-qaFailedToStartConnectionReset) https://github.com/bitcoin/bitcoin/pull/18561
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #18575: bench: Remove requirement that all benches use same testing setup (master...2004-benchNoGlobalReg) https://github.com/bitcoin/bitcoin/pull/18575
< bitcoin-git>
[bitcoin] gzhao408 opened pull request #18576: wip [test] use unittest for test_framework unit testing (master...framework-unittests) https://github.com/bitcoin/bitcoin/pull/18576
< bitcoin-git>
bitcoin/master 402ad5a Pieter Wuille: Only run sanity check once at the end
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #18529: Add fuzzer version of randomized prevector test (master...202004_prevector_fuzz) https://github.com/bitcoin/bitcoin/pull/18529
< achow101>
rc1 soon?
< MarcoFalke>
I wish rebroad could ACK it, since they reported the issue
< hebasto>
why 18553 could be a blocker?
< wumpus>
it's the only PR left that is tagged for 0.20
< wumpus>
hebasto: because if your system doesn't support the instruction it'll just crash before main()
< MarcoFalke>
Does anyone have that CPU to test?
< elichai2>
MarcoFalke: I'm surprised this is the first time we're hearing about this issue. I would've thought everyone without SSE* support will get this error
< sipa>
i'm not sure the problem will appear in every build (it may be compiler dependent)
< wumpus>
no, bitcoind master runs fine even on my oldest pc, but it might depend on compiler too
< sipa>
would i understand how our existing code is broken for systems that do not have sse4
< sipa>
*but i understand
< wumpus>
movaps is SSE2, right?
< promag>
hi
< wumpus>
if the init code contained *SSE4* oh sure we'd have noticed
< cfields>
*sse2
< cfields>
wumpus: right
< sipa>
i suspect he is executing an sse4 instruction
< wumpus>
almost(?) all amd64 processors support SSE2
< sipa>
because his system has sse2
< wumpus>
oh
< wumpus>
okay in that case we don't know if the PR fixes theproblem at all
< sipa>
it does
< wumpus>
I suggest we just move on with the branch-off and rc1
< cfields>
ah, I guess if we're targetting sse4 it's free to ignore the sse2 intrinsic. Annoying that those are only hints.
< sipa>
wumpus: the sha256-shani module is compiled with sse4 on, so any code the compiler produces in that module is allowed to have sse4 instructions
< sipa>
the fact that it has a global initializer is a bug regardless
< wumpus>
sipa: yes, I agree
< wumpus>
I'm the person who ACKed that PR, I think it's a good change
< elichai2>
anyways the current code is somewhat broken
< elichai2>
even if that's not his specific problem
< sipa>
we don't know the exact conditions to reproduce it (which is hard, as it's compiler dependent), but i believe my PR is a bugfix independently of that
< elichai2>
sipa: exactly
< wumpus>
yes
< wumpus>
it makes sense no matter what
< wumpus>
even reduces code size a bit
< sipa>
but if we want to make sure rebroad's issue is fixed in 0.20... we have no choice but to wait for him
< sipa>
i think we can do that in rc1
< jarthur>
If anyone needs time on a machine w/ hardware SHA-NI for profiling/memory sanitization, send me your SSH pubkey and I'll give you a VM
< sipa>
i have one too
< wumpus>
I'm not going to hold up rc1 on them testing it
< sipa>
a machine without sse4 would be more useful :p
< luke-jr>
(proposed topic: change destdata)
< sipa>
there are very few x86_64 systems without sse4 i think
< luke-jr>
although maybe better for wallet meeting tomorrow
< wumpus>
true, even my old dev machine has "sse4a"
< luke-jr>
(but does relate to 0.20)
< elichai2>
jarthur: I would like to run some UB sanitizer on the patch, just because I'm a bit uncomfortable with C++'s alignment rules
< wumpus>
in that case, let's do rc1 without it
< sipa>
ack
< cfields>
elichai2: if you're that uncomfortable, there's an intrinsic that doesn't require alignment.
< luke-jr>
should probably get at least #18572 into 0.20
< wumpus>
jeremyrubin: we're not discussing refactoring prevector again :)
< achow101>
so multisig signrawtransaction workflows don't work with descriptor wallets
< achow101>
I was thinking that because we have psbt now, we should deprecate the signrawtx RPCs
< MarcoFalke>
completely or only for multisig?
< jonasschnelli>
can you quickly elaborate why it won't work with desc. wallets?
< wumpus>
I'm not generally in favor of depracating signrawtx RPCs, many people use the raw transactions workflow
< achow101>
it should be a longer deprecation cycle because it's so widely used
< wumpus>
if we do it should be *very* well documented first
< sipa>
i'm not so convinced here
< jonasschnelli>
agree with wumpus
< sipa>
i believe it's good to "nudge" people towards PSBT, but deprecation is probably too hard a hammer for that
< luke-jr>
[19:23:38] <jonasschnelli> can you quickly elaborate why it won't work with desc. wallets?
< wumpus>
like make a blog post how to *old thing* in *new way*
< achow101>
jonasschnelli: because of the separation of watchonly things, currently we can't create a wallet that has both the multisig script and keys to sign for it
< wumpus>
yes, I agree
< achow101>
so doing a multisig becomes a half assed psbt workflow
< sipa>
achow101: i don't understand why we'd want descriptor wallets to not support private keys for multisig
< achow101>
sipa: the issue is with exporting the private keys to the multisig
< achow101>
IIRC there was contention about exporting keys that used unhardened derivation
< jonasschnelli>
are those technical or conceptual limitations?
< sipa>
achow101: so use hardened derivation?
< achow101>
jonasschnelli: conceptual and current implementation limitations
< sipa>
i understand there may be UI issues on how to make this easy
< sipa>
but say you have a private key, even generated manually or whatever... you should be able to import a descriptor for a multisig based on it, and have that private key in the same wallet
< achow101>
sipa: sure
< sipa>
and that would work fine with signraw*, right?
< achow101>
(this is also partly in the context of making all of the RPC tests work)
< achow101>
sipa: yes
< sipa>
ok
< achow101>
but conceptually, it feels like psbt should be a directly replacement to raw txs, so we should move to remove those eventually
< jonasschnelli>
I could understand why the signraw commands could refuse to work for BIP44-ish descriptor wallets (due to the hardening violation),... though for manual privkey-ckd it should work
< sipa>
jonasschnelli: once you have all the right things in a descriptor wallet, it doesn't matter - hardened or not
< sipa>
(is my understanding)
< sipa>
achow101: i think my preference would be to mark signraw* as in "maintenance mode" or so, where they don't receive new features (e.g. they wouldn't support taproot signing when that gets in)
< instagibbs>
+1
< sipa>
but i feel like deprecation is kind of ruthless
< achow101>
I was thinking of an extra long deprecation cycle
< wumpus>
yes
< instagibbs>
imo I think the tooling is too widely used and PSBT still has an adoption curve to hit
< wumpus>
agree
< achow101>
like 2 releases with a note saying it's deprecated, but don't disable yet. then 2 releases with it hidden behind -deprecatedprc. then remove
< instagibbs>
the tooling meaning *raw*
< wumpus>
maybe in a few years bring this up again :)
< * achow101>
adds to 2022 calendar
< jonasschnelli>
I can't completely follow why we should remove/deprecate it since in many use cases those commands work fine.
< jonasschnelli>
(the accounting system had conceptual flaw in contrast)
< jonasschnelli>
*flaws
< luke-jr>
jonasschnelli: what flaws?
< wumpus>
yes
< sipa>
jonasschnelli: i think the reasoning is "it's hard to make it work nicely with descriptors, and there is a better system already... would be easy to just get rid of it"
< luke-jr>
the accounting system worked fine AFAIK, just nobody cared to maintain it
< wumpus>
accounting system discussion is off topic
< luke-jr>
true
< luke-jr>
couldn't signrawtx be reimplemented as a wrapper around PSBT?
< jonasschnelli>
is the plan to only support descriptor wallets in the future?
< jonasschnelli>
with some sort of migration
< sipa>
luke-jr: yes (and probably should), but that wouldn't solve the problem
< achow101>
my plan is to make descriptor wallets the default wallet type
< achow101>
eventually
< jonasschnelli>
this would be fine. As long as "legacy" wallets are still supported, signwith* commands shound't go away?
< sipa>
the issue (as i understand it) is constructing a descriptor wallet that has all the same pieces of information as a current legacy wallet is unclear (where do the keys come from, how to import without reintroducing mixed wallets or watching the wrong kind of things...)
< achow101>
sipa: yes
< sipa>
i believe it would be nice to spend some time on actually solving that... because there is no technical reason why a descriptor wallet couldn't have that information
< jonasschnelli>
exactly
< sipa>
achow101: FWIW, i think your envisioned workflow (having two wallets, one with the multisig, and one with the private keys) is also pretty suboptimal
< jonasschnelli>
we shouldn't enforce modes of use due to solvable technical imitations
< MarcoFalke>
If the call doesn't work with descriptor wallets, it should be disabled for those wallets, not for legacy wallets as well.
< MarcoFalke>
agree with jonasschnelli
< sipa>
it isn't a technical limitation
< sipa>
it's an unsolved UI question
< sipa>
(where UI includes RPC and workflows)
< achow101>
i suppose it is
< sipa>
i don't think disabling signraw* for descriptor wallets would even solve the root of the issue - users would still need a workaround to do what they could before (having two wallets, and run PSBT RPCs on both)
< jonasschnelli>
achow101: maybe you could write a short gist/paper about the issue for help us to understand it better?
< sipa>
can i try in 3 lines?
< jonasschnelli>
plz
< achow101>
jonasschnelli: sure. I should write release notes for descriptor wallets anyways and there should be section on known limitations
< wumpus>
+1
< sipa>
currently you can construct a legacy wallet which has (1) a private key for one key and (2) watchonly records for multisigs involving that public key - this is crazy (because it means payments to the individual single key will be treated as incoming money, unable to separate it from the multisig funds), but it works great: you have the script information (the the multisig watchonly) and the private
< sipa>
key for one of the keys in one wallet,...
< sipa>
so it can do everything
< sipa>
in descriptor wallets, you'd need to explicitly import a descriptor for the multisig, and then add a private key for one - you can't have started with a wallet that had that private key already as a single-key wallet (because then you reintroduce the mixing of singlekey/multisig funds), and you can't export that private key from another wallet (because we don't want export of non-hardened keys)....
< sipa>
so where does it come from?
< sipa>
i think the only question is a UX one around construction of such wallets
< sipa>
</fin>
< jonasschnelli>
I see. So one would need a watch-only-ms desc wallet and a single-key desc wallet for signing the PSBT (or whatever)
< achow101>
yes
< achow101>
and with both psbt and raw tx, the workflow is the same
< sipa>
right, that would work - and the PSBT would carry the script information from the watch-only-ms wallet to the signing-key wallet
< instagibbs>
user stories may help cover cases, I tend to only think about MY use case
< jonasschnelli>
Which the signraw commands could construct in the background (assume providing all the infos)?
< luke-jr>
aren't the multisig funds classified as watchonly?
< achow101>
you go to the watch-only-ms wallet with a psbt or rawtx, it adds the scripts. then you go to the single-key wallet and sign it. it's the same workflow, but psbt is better suited for carrying this data
< sipa>
but it's ridiculous that you currently can't do multisig stuff without also having payments to individual keys as balance in your wallet
< jonasschnelli>
sipa: though that is rarely used, right?
< sipa>
jonasschnelli: "used" ?
< sipa>
it's an attack
< sipa>
you can send funds to a individual key in a multisig, and the user may think it's paid to the multisig
< luke-jr>
so we need a way to have can-sign non-ismine descriptors
< jonasschnelli>
Kida. Yes. I see. Agree that it is a flaw/ridiculous
< sipa>
luke-jr: descriptor wallets already do that
< achow101>
luke-jr: we already do that. it's a question of the scripts
< sipa>
we just need a good way to import a multisig descriptor + individual key into a descriptor wallet
< instagibbs>
IsMine implementation in descriptor is a relative beauty :P
< sipa>
if that works, signraw* and PSBT* will function just as before
< sipa>
if that doesn't work, it's going to be shitty to use for both
< jonasschnelli>
Now I see why it's a UX issue. We have set our own limitations which IMO could be worked around by creating the right structures on the fly for the signraw* commands when using desc.-wallets
< instagibbs>
achow101, if you do this you're also going to have to get rid of the "private keys disabled" hack we've been using
< instagibbs>
to detect if we want to do PSBT stuff or try to sign the transaction
< achow101>
instagibbs: I don't think so. but I'll experiment
< achow101>
jonasschnelli: which "right structures"
< instagibbs>
achow101, well, if there exists a private key, it will try to sign and fail unexpectedly, I think, but you can test yes :)
< jeremyrubin>
BTW: if you have thoughts on BIP-119 Next Steps please submit them in this survey; want to collect feedback from everyone https://forms.gle/rT3v4JjHbdn3RMnL6
< instagibbs>
The wallet should just probably know explicitly that it should try to auto-sign or return a PSBT, but that's yet another UX q.
< wumpus>
3 minutes to go
< jonasschnelli>
achow101: maybe I get it wrong. But if someone invokes a signraw, depending on the input, you could create either a watch-only-ms wallet or a single-privkey wallet in the background and use the PSBT workflow
< achow101>
but where does the multisig script come from?
< sipa>
jonasschnelli: the question is not about signraw* or PSBT*; the question is constructing a wallet that has the right information
< achow101>
it's not hard to wrap signrawtx around psbts so it uses psbt internally. but it just doesn't have all of the data there
< luke-jr>
descriptor wallets can or can't have multiple descriptors?
< jonasschnelli>
provide it manually or provide it via a second wallet
< sipa>
luke-jr: can; change and payments in general will come from distinct descriptors
< instagibbs>
luke-jr, you can import any number of descriptors, there are 6 "Active" ones, aka keypool, by default
< achow101>
luke-jr: can, but having a descriptor wallet contain both multisig and single key descriptors goes back to the mixed watchonly wallet thing
< instagibbs>
legacy/p2sh-segwit/bech32 x internal/external
< luke-jr>
achow101: not if you flag the single-keys descriptor as non-ismine?
< wumpus>
sorry, time to wrap up the meeting
< achow101>
jonasschnelli: yes, and that's the shitty ux sipa was talking about
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Apr 9 20:01:06 2020 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< sipa>
achow101: i think exporting descriptors (without private keys) should always be fine
< achow101>
sipa: yes, but it's only really useful if unhardened derivation is being used
< sipa>
agree
< bitcoin-git>
[bitcoin] promag opened pull request #18578: gui: Fix itemWalletAddress leak when not tree mode (master...2020-fix-coincontroldialog-leak) https://github.com/bitcoin/bitcoin/pull/18578
< achow101>
so one question is how to support the two use cases in the same wallet
< sipa>
i *think* that exporting descriptors including private keys is also fine, as it doesn't risk exporting something that leaks the entire wallet while the user thinks it's just a single key (as in: it'll always leak the entire thing)
< sipa>
but it shouldn't be possible to export derived private keys (except maybe if they're hardened... unsure)
< achow101>
yes
< achow101>
but exporting derived keys is necessary for the usual multisig workflow
< achow101>
otherwise all you can do are ranged multisigs when maybe you just want a one time thing
< sipa>
i feel that in that case you should also have started from just a single key, rather than an xpub
< sipa>
i see the complication, but i'm not sure how important it is
< achow101>
but how would you have started from just a single key?
< sipa>
that's a great question :p
< sipa>
it would be great if a ranged multisig setup already just worked
< sipa>
say there was an RPC generatexpub which would generate an xprv, and import the private key, but not watch anything, and then return the xpub
< achow101>
maybe if used a wallet global SigningProvider? so keys aren't associated with the scripts directly
< sipa>
yeah, i think that makes sense
< achow101>
ugh, rewriting it all again :(
< achow101>
i'm not sure that's necessarily useful though
< sipa>
you'd still need to be able to generate derived private keys
< sipa>
alternatively, i think it'd be fine if there was just a way to import descriptors with some public and some private keys... without worrying how someone would obtain those for now
< achow101>
sipa: yes, but worrying about how someone would obtain those is important
< sipa>
oh absolutely
< sipa>
but as a first step... that'd be great already
< sipa>
fanquake: what is link-time garbage collection?
< fanquake>
sipa: basically just the use of --gc-sections.
< fanquake>
Curious, was there anything in particular that prompted you to test building like that?
< sipa>
fanquake: seeing the size of bitcoin-asmap in #18573