< achow101> any opinions on sqlite3 amalgamation source code (a ginormous sqlite3.c file that is all of sqlite's source code, provided by them), or linking against system/depends?
< sipa> given their strong guarantees for forward and backward compatibility, i think we should just link against the system version
< sipa> (and pin one version in depends)
< achow101> the only issue that could be a problem is if the system version was compiled with some option like THREADSAFE=0 which probably wouldn't be good
< luke-jr> achow101: hard NACK on bundling it..
< luke-jr> why does our end need to use threads for sqlite?
< luke-jr> bdb isn't threadsafe, is ti?
< achow101> bdb is definitely threadsafe
< achow101> we don't need to use threads, but I'm not convinced that we don't
< achow101> and I'd rather that the library does the threadsafe stuff even if it isn't needed
< luke-jr> sqlite3_threadsafe()
< luke-jr> would be nicer to detect during configure tho
< sipa> at configure time you can't know what flags the runtime version the user will be dynamically linking against has enabled...
< luke-jr> right
< bitcoin-git> [bitcoin] achow101 opened pull request #19077: wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets (master...sqlite-wallet) https://github.com/bitcoin/bitcoin/pull/19077
< fanquake> + 240'000 lines
< fanquake> sqlite3.c is quite the amalgamation
< achow101> the sqlite3.c amalgamation file is very large
< sipa> achow101: as luke-jr mentioned, if we need thread-safeness, you can include an assert(sqlite_threadsafe())
< sipa> so i don't think that's a reason to bundle
< bitcoin-git> [bitcoin] meshcollider pushed 12 commits to master: https://github.com/bitcoin/bitcoin/compare/4af01b37d402...520e435b5e56
< bitcoin-git> bitcoin/master c877709 Andrew Chow: wallettool: Add a salvage command
< bitcoin-git> bitcoin/master cdd955e Andrew Chow: Add basic test for bitcoin-wallet salvage
< bitcoin-git> bitcoin/master d321046 Andrew Chow: wallet: remove -salvagewallet
< bitcoin-git> [bitcoin] meshcollider merged pull request #18918: wallet: Move salvagewallet into wallettool (master...wallettool-salvagewallet) https://github.com/bitcoin/bitcoin/pull/18918
< fanquake> The linter is having an absolute meltdown over sqlite3.c heh: https://travis-ci.org/github/bitcoin/bitcoin/jobs/691575513
< achow101> yeah, trying to figure out how to exlucde it from everything
< achow101> sipa: there are some other options that could be problematic. I haven't fully explored them yet
< luke-jr> achow101: don't, just remove it
< meshcollider> Please explore them quickly then :)
< sipa> achow101: i doubt that, as long as we use no crazy extensions
< achow101> SQLITE_DEFAULT_FILE_FORMAT could be useful in the future for compatibility stuff
< achow101> but I suppose if that matters we can deal with it later
< luke-jr> achow101: you can configure it at runtime..
< achow101> now I need to figure out how to add sqlite to depends and travis
< luke-jr> Travis should be easy :x
< provoostenator> harding: #17219 was merged on April 17th, which if after 0.20 branched off
< gribble> https://github.com/bitcoin/bitcoin/issues/17219 | wallet: allow transaction without change if keypool is empty by Sjors · Pull Request #17219 · bitcoin/bitcoin · GitHub
< provoostenator> I plan to do a write-up on hardware wallet pull requests and they relate. The meat and potatos is in #16546, which I keep up to date regularly.
< gribble> https://github.com/bitcoin/bitcoin/issues/16546 | External signer support - Wallet Box edition by Sjors · Pull Request #16546 · bitcoin/bitcoin · GitHub
< provoostenator> It depends on an improvment to sendmany sendaddress to work with PSBT. There's multiple ways to do that, including writing a whole new send RPC method like in #16378
< gribble> https://github.com/bitcoin/bitcoin/issues/16378 | The ultimate send RPC by Sjors · Pull Request #16378 · bitcoin/bitcoin · GitHub
< provoostenator> It also needs a way to communicate with the device, which for now I'm using Boost::Process for. Similar to the dicussion with multiprocess, that method can be swapped out later
< provoostenator> But I haven't seen any worked out alternatives in over a year, so #15382 appears the way to go.
< gribble> https://github.com/bitcoin/bitcoin/issues/15382 | util: add runCommandParseJSON by Sjors · Pull Request #15382 · bitcoin/bitcoin · GitHub
< provoostenator> Finally there's a GUI PR which is more rough, and depends on all the above. But it nicely illustrates what a relatively user friendly experience could look like. #16546
< gribble> https://github.com/bitcoin/bitcoin/issues/16546 | External signer support - Wallet Box edition by Sjors · Pull Request #16546 · bitcoin/bitcoin · GitHub
< provoostenator> I mean #16549
< gribble> https://github.com/bitcoin/bitcoin/issues/16549 | UI external signer support (e.g. hardware wallet) by Sjors · Pull Request #16549 · bitcoin/bitcoin · GitHub
< provoostenator> I keep that one rebased too.
< provoostenator> Everything is designed to work with HWI, but any other script or program with the same interface works too.
< provoostenator> Which I imagine could be more than just a locally connected hardware wallet. E.g. perhaps an online multisig cosigner service.
< jonatack> provoostenator: per our recent discussion offline, it would be great to move these forward by highlighting the highest or first priority one to focus review on, and maybe add to the blockers tomorrow... 15382 or 16546 IIUC?
< provoostenator> 15382 would make sense for that, since it has no dependencies
< provoostenator> But also kallewoof's #11413 (though he has a Signet PR in prioriy list)
< gribble> https://github.com/bitcoin/bitcoin/issues/11413 | [wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option by kallewoof · Pull Request #11413 · bitcoin/bitcoin · GitHub
< provoostenator> ^ that PR is a such a bit change to the send RPC calls that it blocks other work on them
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/520e435b5e56...48e114e0a86f
< bitcoin-git> bitcoin/master 1c91ffe pad: doc : add link to readme.md in the first section
< bitcoin-git> bitcoin/master 48e114e fanquake: Merge #19061: doc: Add link to Visual Studio build readme
< bitcoin-git> [bitcoin] fanquake merged pull request #19061: doc: Add link to Visual Studio build readme (master...master) https://github.com/bitcoin/bitcoin/pull/19061
< jonatack> provoostenator: ok, noting for my review list. (i probably won't be at the irc meeting since they are during my sleep hours ATM, but i'll check the blockers list anyway)
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/48e114e0a86f...cffbf1eb9a47
< bitcoin-git> bitcoin/master 4c82579 Elichai Turkel: Remove outdated comment about DER encoding
< bitcoin-git> bitcoin/master cffbf1e fanquake: Merge #19073: Remove outdated comment about DER encoding
< bitcoin-git> [bitcoin] fanquake merged pull request #19073: Remove outdated comment about DER encoding (master...2020-05-remove-der-comment) https://github.com/bitcoin/bitcoin/pull/19073
< harding> provoostenator: yeah, I don't know why 17219 seemed to be described in the 0.20 draft release notes. Let me see if I can figure out who added it.
< fanquake> It might have been achow101 on my misdirection. I thought that PR was part of 0.20.0
< harding> Ok, mystery solved. I just wanted to make sure I wasn't removing something that actually did apply to 0.20. I've got it queued for a PR to master for when I finish editing all the 0.20 stuff.
< meshcollider> instagibbs: I *will* get around to reviewing #17331 (and its parent) next week even though I hate coin selection, I promise. The next couple of days are just going to be super busy but after that, as you requested...
< gribble> https://github.com/bitcoin/bitcoin/issues/17331 | Use effective values throughout coin selection by achow101 · Pull Request #17331 · bitcoin/bitcoin · GitHub
< meshcollider> Same with #18275, sorry kalle, it's been on my list for a while but I promise it isn't forgotten
< gribble> https://github.com/bitcoin/bitcoin/issues/18275 | wallet: error if an explicit fee rate was given but the needed fee rate differed by kallewoof · Pull Request #18275 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/cffbf1eb9a47...9ccaee1d5e2e
< bitcoin-git> bitcoin/master c57f03c Calvin Kim: refactor: Replace const char* to std::string
< bitcoin-git> bitcoin/master 9ccaee1 MarcoFalke: Merge #19004: refactor: Replace const char* to std::string
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19004: refactor: Replace const char* to std::string (master...replace-char-with-string) https://github.com/bitcoin/bitcoin/pull/19004
< elichai2> Hi, why is the nVersion of the block header the top bits instead of the "regular" ones? (ie why `0x20000000UL` instead of `4`)
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #19078: test: Add salvage test for wallet tool (master...2005-testWalletToolSalvage) https://github.com/bitcoin/bitcoin/pull/19078
< harding> I added some previously-undocumented changes to the 0.20 release notes draft. Review is appreciated; I'm happy to make any edits: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/0.20.0-Release-Notes-Draft/_compare/cd86fd949c5dc3bf8c85f7f4935cac0c1ae0fb55...d87bc6f05e506ae1ff358ccff4e7f4cbbe2c1fb3
< fanquake> harding: probably just squashing the "build system" section added at the bottom with the "build system section" at the top of the notes. Unless they were kept split for a reason.
< jonatack> harding: lgtm. micro-nit: s/parameter than can be used/parameter that can be used/
< harding> fanquake, jonatack: thanks!
< roconnor> elichai2: See BIP-9
< bitcoin-git> [bitcoin] gillichu opened pull request #19082: Moved the CScriptNum asserts into the unit test in script.py (master...issue24) https://github.com/bitcoin/bitcoin/pull/19082
< bitcoin-git> [bitcoin] rajarshimaitra closed pull request #19063: rpc: testmempoolaccept returns transaction fee (master...fee-in-testmempoolaccept) https://github.com/bitcoin/bitcoin/pull/19063
< bitcoin-git> [bitcoin] gzhao408 opened pull request #19083: test: msg_mempool, fRelay, and other bloomfilter tests (master...test-bloomfilter) https://github.com/bitcoin/bitcoin/pull/19083
< bitcoin-git> [bitcoin] MarcoFalke pushed 8 commits to master: https://github.com/bitcoin/bitcoin/compare/9ccaee1d5e2e...55b4c65bd1d8
< bitcoin-git> bitcoin/master c3cf2f5 Anthony Towns: rpc/blockchain.cpp: Remove g_utxosetscan mutex that is only protecting a s...
< bitcoin-git> bitcoin/master de7c5f4 Anthony Towns: wallet/wallet.h: Remove mutexScanning which was only protecting a single a...
< bitcoin-git> bitcoin/master 8b5af3d Anthony Towns: net: fMsgProcWake use LOCK instead of lock_guard
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #16127: More thread safety annotation coverage (master...201905-locking) https://github.com/bitcoin/bitcoin/pull/16127