< achow101> sipa: hardware wallets can't do the generic way of signing
< achow101> Our generic way of signing was to get a SigningProvider and pass that to ProduceSignature. but you can't get a SigningProvider with private keys for a hardware wallet
< sipa> achow101: hmm, you imagine having a SPKM for hardware wallets?
< achow101> yes
< achow101> I imagine it would be a subclass of DescriptorScriptPubKeyMan
< sipa> hmm, i had never imagined it that way
< sipa> it may make sense
< achow101> what were you thinking it would be?
< sipa> a descriptorscriptpubkeyman that simply had no private keys, and maybe a meta data field saying "use hardware device X", so the UI could suggest the user try PSBT interface/etc
< sipa> i wasn't expecting dealing with hardware wallets so deep down in the stack
< sipa> i guess if you want to transparently support hardware wallets, without having the user jump through hoops exporting/importing PSBTs, etc, or specialized UIs for it, this is how you'd do it
< achow101> my end goal for hardware wallets is to have the seamless experience where it calls out to the device automatically on send
< achow101> so this seemed to be the easiest way to do that
< achow101> instead of sprinkling around a bunch of hardware wallet things everywhere we had signing code
< sipa> still it feels awkward that SPKMs (which really ought to just deal with keys and scripts) need to be aware of transactions and messages
< achow101> i guess
< sipa> it also means things like signet (block signing) and generic message signing need to add their own methods to every SPKM implementation?
< ysangkok> achow101: forgive me if it is a stupid question, but if SignTransaction needs to block and wait for a HW wallet, won't it need to be more complicated than just a function call (because of asynchrony)? i guess it would block for e.g. 1 minute while the user interacts with the HW wallet. is that how it will be?
< achow101> sipa: we can have a default implementation that calls a protected GetSigningProvider and then does the signing thing generically. then if the specific SPKM can't do that, they can override it and implement it differently
< achow101> ysangkok: it only blocks the thread for that part of the ui (or specific RPC). IIRC we already have async dialogs so it shouldn't be a problem
< achow101> but I haven't thought about that too much yet
< sipa> achow101: right
< sipa> achow101: SignTransaction can probably be written as a wrapper around PSBT operations, actually
< achow101> it'd be nice if the wallet moved entirely to psbt operations
< sipa> so maybe it suffices if SPKMs (or even some.other class) can just provide "here is another method you can try to uodate a PSBT"
< sipa> maybe related: why does the SigningProvider returned not give access to private keys?
< achow101> that SigningProvider is specifically just for the solving date (hence the function name GetSolvingProvider)
< midnight> btw, psbt has made offline signing *incredibly much easier*. Seriously, thank you.
< achow101> so the no private keys thing is to be explicit about it not actually being useful for signing
< sipa> i see
< achow101> there was a suggestion of moving the public parts of SigningProvider to a parent SolvingProvider
< achow101> to make that clearer
< sipa> that may make sense
< sipa> but there should be a way to get access to private keys too, no?
< sipa> or is that only possible through the SignTransacrion and SignMessage functions?
< achow101> i'm not sure that it useful to have the privkeys given SignTransaction and SignMessage
< sipa> well i hope that at some point we don't need tx specific logic in SPKMs
< sipa> but i see why that's the case now
< sipa> thanks
< bitcoin-git> [bitcoin] practicalswift opened pull request #18363: tests: Add fuzzing harness for HTTPRequest, libevent's evhttp and related functions (master...fuzzers-http_request) https://github.com/bitcoin/bitcoin/pull/18363
< bitcoin-git> [bitcoin] fanquake opened pull request #18364: random: remove getentropy() fallback for macOS < 10.12 (master...macos_remove_1012_fallback) https://github.com/bitcoin/bitcoin/pull/18364
< vasild> fanquake: I find it strange that we used to test whether the getentropy() function is present with "if (&getentropy != nullptr)".
< vasild> Would it even compile if getentropy() is missing? Maybe old macos versions define the symbol but set it to nullptr!?
< fanquake> vaslid: From my understanding it would compile, but crash at runtime on a version of macOS, because of the way new SDK symbols are handled. There's some more discussion in #15103
< gribble> https://github.com/bitcoin/bitcoin/issues/15103 | fix getentropy import check on osx by jameshilliard · Pull Request #15103 · bitcoin/bitcoin · GitHub
< fanquake> opps *vasild
< vasild> good that it is going to be deleted now :)
< bitcoin-git> [bitcoin] jnewbery opened pull request #18366: tests: simplify next_block() function in feature_block (master...2020-03-feature-block-next-block) https://github.com/bitcoin/bitcoin/pull/18366
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/7060d2d97a28...8fc5f2bdf01d
< bitcoin-git> bitcoin/master 612a931 John Newbery: tests: simplify next_block() function in feature_block
< bitcoin-git> bitcoin/master 8fc5f2b MarcoFalke: Merge #18366: tests: simplify next_block() function in feature_block
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #18366: tests: simplify next_block() function in feature_block (master...2020-03-feature-block-next-block) https://github.com/bitcoin/bitcoin/pull/18366
< bitcoin-git> [bitcoin] laanwj pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/8fc5f2bdf01d...1d64dfe4fa05
< bitcoin-git> bitcoin/master a889711 fanquake: rand: remove getentropy() fallback for macOS < 10.12
< bitcoin-git> bitcoin/master f9f210d fanquake: doc: fix GetTimeMicros() comment in random.cpp
< bitcoin-git> bitcoin/master 1d64dfe Wladimir J. van der Laan: Merge #18364: random: remove getentropy() fallback for macOS < 10.12
< bitcoin-git> [bitcoin] laanwj merged pull request #18364: random: remove getentropy() fallback for macOS < 10.12 (master...macos_remove_1012_fallback) https://github.com/bitcoin/bitcoin/pull/18364
< bitcoin-git> [bitcoin] MarcoFalke pushed 6 commits to master: https://github.com/bitcoin/bitcoin/compare/1d64dfe4fa05...d2d0a04a661f
< bitcoin-git> bitcoin/master cb4eec1 practicalswift: tests: Add fuzzing harness for count_seconds(...)
< bitcoin-git> bitcoin/master 0579a27 practicalswift: tests: Add fuzzing harness for CBlockHeader
< bitcoin-git> bitcoin/master 7726f3b practicalswift: tests: Add fuzzing harness for CFeeRate
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #18353: tests: Add fuzzing harnesses for classes CBlockHeader, CFeeRate and various functions (master...fuzzers-coverage) https://github.com/bitcoin/bitcoin/pull/18353
< bitcoin-git> [bitcoin] MarcoFalke pushed 8 commits to master: https://github.com/bitcoin/bitcoin/compare/d2d0a04a661f...ad04f0d8a533
< bitcoin-git> bitcoin/master f31fc0e John Newbery: [tests] fix flake8 warnings in script.py and bignum.py
< bitcoin-git> bitcoin/master 1dc68ae John Newbery: [tests] add function comments to bignum
< bitcoin-git> bitcoin/master 9a60bef John Newbery: [tests] don't encode the integer size in bignum
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #17319: Tests: remove bignum module (master...2019-10-bignum) https://github.com/bitcoin/bitcoin/pull/17319
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/ad04f0d8a533...39497d1f3268
< bitcoin-git> bitcoin/master ec30a79 Gregory Sanders: Fix UB with bench on genesis block
< bitcoin-git> bitcoin/master 39497d1 MarcoFalke: Merge #15283: log: Fix UB with bench on genesis block
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15283: log: Fix UB with bench on genesis block (master...ub_genesis_bench) https://github.com/bitcoin/bitcoin/pull/15283
< bitcoin-git> [bitcoin] sipa opened pull request #18367: Avoid strncopy for CMessageHeader::pchCommand (master...202003_strncopy_warning) https://github.com/bitcoin/bitcoin/pull/18367
< bitcoin-git> [bitcoin] MarcoFalke pushed 4 commits to master: https://github.com/bitcoin/bitcoin/compare/39497d1f3268...ce87d5613a55
< bitcoin-git> bitcoin/master fa70ccc MarcoFalke: scheduler: Use C++11 member initialization, add shutdown assert
< bitcoin-git> bitcoin/master fadafb8 MarcoFalke: scheduler: Make schedule* methods type safe
< bitcoin-git> bitcoin/master fa36f3a MarcoFalke: refactor: move DUMP_BANS_INTERVAL to banman.h
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #18289: refactor: Make scheduler methods type safe (master...2003-schedTypeChrono) https://github.com/bitcoin/bitcoin/pull/18289
< fanquake> aj can you follow up in #13990
< gribble> https://github.com/bitcoin/bitcoin/issues/13990 | Allow fee estimation to work with lower fees by ajtowns · Pull Request #13990 · bitcoin/bitcoin · GitHub
< aj> fanquake: yeah