< 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
< 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.
< 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
< 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.
< 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
< 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)
< 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/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...
< 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
< 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