0d5041ca4a972a946244643db738e6d35b0d114e bitcoin: looper: avoid large sleeps when bitcoind is syncing (Antoine Poinsot)
Pull request description:
Sleeping for 30 whole seconds impedes the shutdown check. Sleep only .5s but still only do poll bitcoind one every 30s when it's syncing.
It was reported to make the GUI hang when closing it while syncing.
ACKs for top commit:
edouardparis:
utACK 0d5041ca4a972a946244643db738e6d35b0d114e
Tree-SHA512: 48c9121a02abaf8311d7b646ea64cbde4e14fda737c6f78521739bc185e6081642d05b3ca8088d3ce6782ffa00d6e5879e6a6e09da7f05c85f8476ac83b6860b
2a6b775f607dc0b56be874807bd36427922ac681 lib: non-blocking daemon shutdown (Antoine Poinsot)
Pull request description:
This makes it possible to trigger the shutdown of the daemon through the DaemonHandle, without having to block while waiting for the poller thread to join.
Incidently, this allows to avoid having to move `self` which in turns allows to fix a GUI bug (see
https://github.com/wizardsardine/liana/issues/622).
ACKs for top commit:
edouardparis:
ACK 2a6b775f607dc0b56be874807bd36427922ac681
Tree-SHA512: a443ba27f28b5f1acf602f5e4339400dda5a7b2c70c9d7c36df3dfccc32baf7a6ae2cf07fd17666c023e68b943c7961d2c61be8678bc9be1fad1bdeb7c7f1554
This makes it possible to trigger the shutdown of the daemon through the
DaemonHandle, without having to block while waiting for the poller
thread to join.
Incidently, this allows to avoid having to move `self` which in turns
allows to fix a GUI bug (see
https://github.com/wizardsardine/liana/issues/622).
Only available as an optional feature since `is_finished` needs rustc
1.61.
2888db425df61a5e64ce12f9425bbef7cb3452b6 contrib: remove former script to generate Debian packages (Antoine Poinsot)
3e23bf5e35ac594340703ca9c68d6448f6d85f5f contrib: Debian packaging for releases (Antoine Poinsot)
8a7c2832697f342692ee2e420212c8d387248dff contrib: use a zip archive for the MacOS sample app folder (Antoine Poinsot)
Pull request description:
This cleans up the MacOS distribution and adds Debian packaging to the release script. This gives users of Debian-based distributions (Ubuntu, etc..) a simple way of installing our software.
Fixes https://github.com/wizardsardine/liana/issues/519.
ACKs for top commit:
darosior:
ACK 2888db425df61a5e64ce12f9425bbef7cb3452b6
Tree-SHA512: 04de4d455138e214dd73cf3d7d9623aae341271bf32201ef50eae4b3f07c572a4d2d7b4f3af9705d24a788ceffccedbfdca156fafbe9d7c475a8f655edf8e49a
2b3ed5a8dd4cd36a15d23f1792b37a18023f08c2 README: add instructions for installing Liana from AUR (Antoine Poinsot)
c7189421debc4588fa9a26749a92e850ae69e569 contrib: add a PKGBUILD to publish Liana to AUR (Antoine Poinsot)
Pull request description:
ACKs for top commit:
darosior:
ACK 2b3ed5a8dd4cd36a15d23f1792b37a18023f08c2
Tree-SHA512: 65910e7b3ceb81fe19895106f54d08acb88ef73cdbc7f247ac6af27fafbc48d9f5ab8dffd2ea9f54a57b80c8d99c07e2b2c32203934faa144c1a7a9f53073da0
bba5cd63e81a491c28eddc14e4087ba179794a3c gui: installer: convey registering on a signing device isn't always required (Antoine Poinsot)
Pull request description:
As discussed in #545, in some cases we nudge the user to register the descriptor on their signing device although they might not have one. Those cases only ever arise when importing a descriptor (either when recovering from backup or participating in the creation of a descriptor on another laptop), since when the descriptor is created beforehand we can simply detect whether a signing device was used and thereby needs to be registered (implemented since https://github.com/wizardsardine/liana/pull/470).
Therefore, detect when the registration step arises as part of an import process and if so adjust the language to convey registration on a signing device may not be necessary.
Result:
Fixes#545.
ACKs for top commit:
edouardparis:
ACK bba5cd63e81a491c28eddc14e4087ba179794a3c
Tree-SHA512: 5b25b0c980f3e57563fc3c301802a7c4da50d7c80802d3e6a7517f98ee1da60e9d5ebe890ce3b8c34ca882220d920e31c59b79877c9e8f48b72b092d3809acda
30ce2dea081adb504fed261b09103d4fcbba5fb4 installer: use appended path instead of account number for xpubs (edouard)
Pull request description:
ACKs for top commit:
darosior:
ACK 30ce2dea081adb504fed261b09103d4fcbba5fb4 -- tested it locally and reviewed the descriptor analysis related changes. Edouard also tested it against a setup with a Specter device.
Tree-SHA512: d407c4d96e7990161595a94cc4341268676325a76a241e9a070cb84218a3f49b3cbf6c2918cf9d0f108e2019aefbed90fd86f4412001f0d540eb3edb51a1c58c
3372a8d461f1b06cff08d913b45328e3dcd75da7 bitcoind: add `stop` method (jp1ac4)
60ee667f6c0260053cc2c4dfb7c30c390e5a8390 bitcoind: make `BitcoinD` public (jp1ac4)
Pull request description:
This is needed as part of https://github.com/wizardsardine/liana/pull/592 for both checking connectivity to the internal `bitcoind` and stopping it when closing Liana.
ACKs for top commit:
darosior:
utACK 3372a8d461f1b06cff08d913b45328e3dcd75da7
Tree-SHA512: 804cf6a94688db7aa8d78b10a47d08f58ff93c322c9f91bee7b98625d7d5e0633282fa4d0a1b801e56a334a4448f0348930fe5ef3265b5719bd090b6128fc67a
d4da2bf6b451de0cd44630c077b83426ed581940 daemon bin: try to detect '--help' or '-h' (Antoine Poinsot)
01ce0f179d8ef059f04e56e8f7c125264e9949b1 daemon bin: more helpful error message on config file parsing error (Antoine Poinsot)
222c8bba810cb20eeb60270e89857ab09324884d daemon bin: a more helpful error message on unknown arguments. (Antoine Poinsot)
Pull request description:
Specifically, link to a sample configuration file. How are users supposed to know how to write the config file otherwise?
Fixes#559.
ACKs for top commit:
darosior:
self-ACK d4da2bf6b451de0cd44630c077b83426ed581940 -- trivial.
Tree-SHA512: 7ca05aa0e351c390b67051ad36c2e767b019cf6339d8207e1fdbd1ead47a3d79ab6cd9584c412d98ae876aa7e8ea906885c52261565917a7cd1126798806b5d2
0785bb7f9f63c788af7b7e755b118d642d9b0402 poller: don't filter out coins with already a spend txid when checking for spends (Antoine Poinsot)
Pull request description:
It turns out we didn't actually notice RBF'd spends.
Fixes#382. And also fixes#73.
ACKs for top commit:
edouardparis:
utACK 0785bb7f9f63c788af7b7e755b118d642d9b0402
Tree-SHA512: 2c37708fb1ecf4e6122fdb1d0f92b5a89978632ac4f31f24c7628a88160b76e1f6edf956544e88544dbefc46b7cf06e2cfb77db5477fd6f05ce698ff18b2d919
23a63f35e1344a0e6c247e564311cac3e965693c bitcoind: override watchonly wallet as to be loaded on startup (Antoine Poinsot)
Pull request description:
we used to not set 'load_on_startup' when creating a watchonly wallet. This could create some complications when people are using agressive pruning configurations.
Fix this by overwriting this parameter when loading watchonly wallets. We can always revert this in the future if for some reason a user wants to have it set to false and no be overwritten everytime Liana loads it.
Fixes#594.
ACKs for top commit:
darosior:
self-ACK 23a63f35e1344a0e6c247e564311cac3e965693c -- tested locally
Tree-SHA512: 8310918286a8a0102bfc76e9d5b2ba4a8b50cac3e4ce2fd26bdfae7c153eeba3bf516d70ed6ef6256c5f4bcc3955559c011a7035cbc293cc594b829ea9038a5c
605c3f657ebfdb92e03937f02e966fa8f9c18557 descriptors: allow the multipath step to be different than '<0;1>' (Antoine Poinsot)
Pull request description:
Ledger and wallet policies disallow having more than 2 depth after the
placeholder, therefore we can't do `@1/0/<0;1>/*`, `@1/1/<0;1>/*`, ..
Instead we have to do `@1/<0;1>/*`, `@1/<2;3>/*`, ..
Why not? Salvatore also says the cost of deriving another depth is
non-trivial on a signing device.
Don't pick a fight with Salvatore, instead just let the GUI (or whatever
creates the desc) use different multipath steps for keys derived from
the same xpubs.
Based on #584. We need to make sure we don't make the assumptions of the multipath step always being `<0;1>` anywhere else the codebase.
ACKs for top commit:
edouardparis:
ACK 605c3f657ebfdb92e03937f02e966fa8f9c18557
Tree-SHA512: acccc31730057a59cf0caccfb258c7b3ea2c27e4181636405e23b974952d6832b45a07f3ba9d627e09c3e9b58ab2d2ab95ba4a6d3c4f3d970f56cad981fa4aaf
10acc482e15c0f5832da576baddd10ce44dc85df descriptors: fix the signatures count analysis (Antoine Poinsot)
7a33040b83796b0517a4955ac69f8766577b8442 descriptors: make signed_pubkeys a mapping from fingerprint (Antoine Poinsot)
Pull request description:
Needs more testing but that should do the trick: we need to take into account the derivation path appended to the xpub in addition to the one from the origin!
ACKs for top commit:
darosior:
ACK 10acc482e15c0f5832da576baddd10ce44dc85df -- Edouard ACK'd the child PR at https://github.com/wizardsardine/liana/pull/599#pullrequestreview-1571990532
Tree-SHA512: 912f482c5e10ece4127ffc86620f8ec581d0a209f8e1d407fd9885a7b6d1dfb13e7d5898c75da782282b9d40f69a80ad835e5a7648ceddcfc809445cdf246efb
we used to not set 'load_on_startup' when creating a watchonly wallet.
This could create some complications when people are using agressive
pruning configurations.
Fix this by overwriting this parameter when loading watchonly wallets.
We can always revert this in the future if for some reason a user wants
to have it set to false and no be overwritten everytime Liana loads it.
Ledger and wallet policies disallow having more than 2 depth after the
placeholder, therefore we can't do `@1/0/<0;1>/*`, `@1/1/<0;1>/*`, ..
Instead we have to do `@1/<0;1>/*`, `@1/<2;3>/*`, ..
Why not? Salvatore also says the cost of deriving another depth is
non-trivial on a signing device.
Don't pick a fight with Salvatore, instead just let the GUI (or whatever
creates the desc) use different multipath steps for keys derived from
the same xpubs.
It's supposed to represent the number of signature per "master" key, i
guess. At the moment it would always be 1 because the origin changes
when we queried more keys from the signing device (because we increased
the account).
0f6fc48e88344a8d0f7f5be68503482ba92105cf qa: only run the RPC retry functional test with bitcoind 25.0 (Antoine Poinsot)
be2030af283f214e90230271f0df11990c25b512 qa: unflake the test_retry_on_workqueue_exceeded functional test (Antoine Poinsot)
b96db71a4de557c762fe583de8635b94f3bff210 qa: bitcoind: add a helper for sending a P2P message to bitcoind (Antoine Poinsot)
df396cccc8d009c53a47e0b9bcca2603f42bbeba qa: set load_on_startup for bitcoind internal wallet (Antoine Poinsot)
Pull request description:
This was racy as it was relying on the `waitfornewblock` timeout, so it
would occasionally fail in CI.
Make this test more robust by having `waitfornewblock` wait indefinitely
and submitting a block through the P2P interface to unstuck it.
Fixes#573.
ACKs for top commit:
darosior:
self-ACK 0f6fc48e88344a8d0f7f5be68503482ba92105cf -- extensively tested it locally.
Tree-SHA512: bd12fb525a46ccf543fb8774370f5b5064723f578de51723b15a420cbc576cb5ef579e7d6ffe38a287ee7c7742887e61a01b76be4fb0858313d4712f1b00f1c0
9a08f14aa45b01f553db677cf5c7f1712e9fa0dc contrib: use full path to check the Xcode archive (Antoine Poinsot)
Pull request description:
ACKs for top commit:
darosior:
ACK 9a08f14aa45b01f553db677cf5c7f1712e9fa0dc -- i used this to release 1.1
Tree-SHA512: 8b3ee16bda137ed59f67d0e2f54a16bc9c439a467f4dbc408759b472f21e571f676e4fc9a2d9e93e759a06fdcea19eb24481332fbfa574f8bc44099d5c5429da
64df2a15a2ab8c6c0ddd97a90fb3e4b8cc66ce13 db: keep NOT NULL constraint on is_immature in migration (Antoine Poinsot)
Pull request description:
As suggested by Edouard in https://github.com/wizardsardine/liana/pull/578/files#r1285476881 there is no reason not to keep the `NOT NULL` constraint. I also added the `CHECK .. IN (0, 1)`.
This adds an extraneous DEFAULT compared to the schema in freshly created databases, but anything else (altering the column in an SQLite-friendly way after setting all NULL values to 0) would be way too involved.
ACKs for top commit:
edouardparis:
utACK 64df2a15a2ab8c6c0ddd97a90fb3e4b8cc66ce13
Tree-SHA512: 28ab4426b18d139bd1d53d017fb667db20ec0e997b61d6ed6afeae0c006697b737bed1fa8e158813ad45fed98137af806c5a6b39d463d6cc0ff03318f64e5943
This was racy as it was relying on the `waitfornewblock` timeout, so it
would occasionally fail in CI.
Make this test more robust by having `waitfornewblock` wait indefinitely
and submitting a block through the P2P interface to unstuck it.
This adds an extraneous DEFAULT compared to the schema in freshly
created databases, but anything else (altering the column in an
SQLite-friendly way after setting all NULL values to 0) would be way too
involved.
097d5e71c19c0c5fc53d1e91087d91edfdd218f3 qa: adapt the migration functional test to also support 1.0 (Antoine Poinsot)
289f6581cccefad42afa870b382fc22eb0c5e46c qa: functional test createspend refuses immature outpoints (Antoine Poinsot)
95dd3e52935cf7a2edb1abdee428efe26ccff36c qa: fix and improve the coinbase deposit functional test (Antoine Poinsot)
6b82894614df3351d788451211d864ecff3f3544 bitcoin: track maturity of coinbase deposits (Antoine Poinsot)
6ab6161078af1932dd43bab813963c78887d7672 commands: expose whether a coin is immature in listcoins (Antoine Poinsot)
fd717123be45b73758f1e965659de8f44031fc90 commands: don't create spends with immature coins (Antoine Poinsot)
26add29b197eb6fdf8f5f86ece4c52576700fc6f database: record whether a coin comes from an immature coinbase (Antoine Poinsot)
Pull request description:
#567 uncovered that we were actually not tracking coinbase deposits correctly. In fact we would most likely miss them all in any real situation. This is because we would filter out immature coinbase deposits from the result of `listsinceblock` and not consider them newly received coins. But they would be confirmed, and as the chain moves forward we'd not scan this range anymore even once they've become mature.
This PR fixes it by the simplest possible manner: record immature coinbase deposits as unconfirmed and only mark them as confirmed once they've become mature. This is a bit clumsy, but should be fine for the number of users that would receive payouts from coinbase transactions (ie most likely 0). Also, we don't accurately update coinbase coins on reorg. This is unnecessary as it'd be very unlikely that a mature coinbase would become immature and if there is a 100 blocks reorg that would invalidate it altogether we'd have bigger problems.
Fixes#567.
~~Still as draft as i want to go over this one more time before asking for review, as this if pretty intricate and touches core parts of our codebase.~~
ACKs for top commit:
darosior:
self-ACK 097d5e71c19c0c5fc53d1e91087d91edfdd218f3. Didn't have the chance to re-review it but getting it in is best at this time. Edouard will still have a look post-merge.
Tree-SHA512: 4a5f0fb7561af1d4c51dcba26bc20ef5e7a8b2c730547f762782f75c1f28c26e2a577573aa514db8927c1eeb601685071e9eb85c11537621de58c75824435b05
8ae597fec11c2fdd7873c9bd011a75a5fbbd9301 bitcoind: create watchonly wallet with load_on_startup = true (Antoine Poinsot)
Pull request description:
Upon startup bitcoind will update loaded wallets before pruning historical chain data.
This is a trivial fix for people using an aggressively pruned `bitcoind` to not be forced to `-reindex` if they haven't started their Liana wallet for longer than their prune target allows for.
See https://github.com/wizardsardine/liana/issues/570#issuecomment-1658108967 for a discussion about this.
ACKs for top commit:
darosior:
self-ACK 8ae597fec11c2fdd7873c9bd011a75a5fbbd9301
Tree-SHA512: 022ab1224d182372963a6ceb9baaf13aecc568b9d0d546d490ac1fb15450a8c70611b4b25a3a8a2f6cf613e28e8b54c8f5e4c55a4da723927ffc4c6e179f2a63
28049d4f7cd2001207cc2b204c31b33d9640077d descriptor: make it possible to use the same xpub at different paths (Antoine Poinsot)
Pull request description:
This is unnecessary to bump a hardened step to get another xpub for the same participant:
- It's more hurdle when sharing it
- It's more hurdle when verifying it on the device's screen
- It's for the same person within the same script anyways
Fix this by allowing to use the same xpub multiple times in a descriptor as long as the derivation path is different. Then the GUI will be able to use a new unhardened derivation step instead of bumping the BIP48 "accounts". A unit test is introduced that showcases this.
See also https://github.com/wizardsardine/liana/issues/542.
ACKs for top commit:
edouardparis:
ACK 28049d4f7cd2001207cc2b204c31b33d9640077d
Tree-SHA512: e864a8193998667d36b34d96e6e32bfff1c507b3c31faa324b7f264604c41b5d2d4aefeec3b71a1d3edf5b0242966861887baf97d7a410e43e7449f22c3453f4
