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
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
We detect and store immature deposits (cause otherwise we won't go
through them again with listsinceblock), but mark them as such and
unconfirmed.
We only mark them as confirmed once they've matured. It's a bit clumsy
but it's not as if most of our users had coinbase deposits.
We need to keep track of such coins to:
- Track their maturation
- Avoid using them (for instance in coin selection)
Reorg handling for coinbase deposits that become immature is not
implemented (yet). That's reasonable because:
1. It would be very unlikely that we'd move back, so it's most likely
gonna be mature again immediately.
2. If there's a reorg of more than 100 blocks we've got bigger problems.
f4d184f6f7fb748bff17f8a846616c589a432342 descriptor: refuse non-normalized xpubs (Antoine Poinsot)
Pull request description:
We wouldn't be able to derive them. See https://github.com/wizardsardine/liana/issues/576#issuecomment-1642429179.
ACKs for top commit:
darosior:
self-ACK f4d184f6f7fb748bff17f8a846616c589a432342
Tree-SHA512: 2da34a8fccbeda1bab69d7bfd6e453173578884d76718ba54f6c505639f4117de9f8d03850b5bddb75bc3e23a57ddfc1ce4b885250c6ff468d591932e947f379
It's a much better UX to be able to share and verify one xpub and just
use different unhardened paths for the various public keys used in the
same script that originate from the same master key.
43a0cf232be58d40479ff51790765b94e92aaae4 test_misc.py: linter (Antoine Poinsot)
c421610b302d41b119a231114154b779a0ca5293 qa: test we retry requests to bitcoind when it's overloaded (Antoine Poinsot)
9007947e4e3600737f0de183ef198bbf1da988f8 bitcoind: use minreq as HTTP transport for JSONRPC (Antoine Poinsot)
e3ee50b7af30c313229b232017dfce9ec709c78f Update rust-jsonrpc dependency to latest version (Antoine Poinsot)
Pull request description:
This makes us take advantage of a more robust, but still lightweight, HTTP implementation.
This PR also cleanups our error handling code and adds a functional test checking we do retry request on transient bitcoind failures.
ACKs for top commit:
darosior:
ACK 43a0cf232be58d40479ff51790765b94e92aaae4 -- let's get it in early and try it as we go.
Tree-SHA512: 8b66663079a77df86ce0fc2f6f60c61b2d3fd50a6d5f8f06b35dcb5a8a942210260e68023e90647aaf8da70625b73d024eda2aba284203c0db99842ed4bb0ceb
7c3511e1e4516bef26f525b7280aeeda9299d206 add script for build .deb packages (pythcoiner)
Pull request description:
following #519
should i locate the script in some specific location instead of at the repo root?
is it a way in the CI workflow to automate the test for .deb installs on different versions?
ACKs for top commit:
darosior:
utACK 7c3511e1e4516bef26f525b7280aeeda9299d206 -- can be refined and integrated into the release process in a follow-up.
Tree-SHA512: 67a5a7bd81e975a2a68a3eb195bef79a75b171270d0a3cd9ba0f8e041d320aada20749591258fe7da7c5ed03fa355143885cf22cb7cbe7a43e855c526e884840
11a280bfe4e46031b55259d1ba4963ed4b6f2b8b doc(api): fix order of params for `createspend` (jp1ac4)
Pull request description:
This fixes the documented order of parameters for the `createspend` command.
I also added a full stop to the `destinations` description for consistency.
ACKs for top commit:
darosior:
Thanks! ACK 11a280bfe4e46031b55259d1ba4963ed4b6f2b8b.
Tree-SHA512: bb8778fcb25bb31f883825142e102741592593db22ccfad467b6704767055c67f135b39a5d8b2e3ba9f1e9bf53a422bd82423212c546882a5d302e512718beab
Instead of the hand-written HTTP implementation in the rust-jsonrpc
library, use the minreq crate. It's a small, maintained, low dependency
library actually focused on writing an HTTP client.
This also reworks and better document the request retry logic.
a19f2c1536cab6f6a3e85eedcba9be4b907dc33b daemon: drop the base64 dependency (Antoine Poinsot)
96e4cb53537a59f4b2e11011f77e7635ee4a1dd9 Update proc-macro2 to fix a nightly compilation bug (Antoine Poinsot)
b9753b48d03ec7d4398764ec6a91e04388bdb85e descriptors: update the satisfaction size estimation (Antoine Poinsot)
0ac4d80ddbdb61ef3ba9d7e6eb7b76114dfcf961 commands: fix two clippy lints (Antoine Poinsot)
e28010915f4d784ba06a1947ddadaeb5addf6082 lianad: update rust-miniscript (and rust-bitcoin) dependencies (Antoine Poinsot)
Pull request description:
It was a big chunk, especially in making sure we don't introduce any silent bug with all the upstream recent code movements.
Also, we no longer depend on my custom `rust-miniscript` branch! 🎉
ACKs for top commit:
darosior:
Self-ACK a19f2c1536cab6f6a3e85eedcba9be4b907dc33b.
Tree-SHA512: 7b785551b51bd247c233cac8a44148d25832be729772e2987d6e276643d9f781feb5016e81f2914845a2c93542f57ce861d06b31ba6c455f75cf93681fb98805
The latest rust-miniscript version deprecated the helper we were using,
in favour of one that gives the maximum size difference of a transaction
input before and after satisfaction. The new helper differs in that it
does not account for the empty ScriptSig byte (which uncovered we were
actually double-counting it), and assumes the non-satisfied transaction
input is already part of a Segwit transaction (which we rectified).
This uncovered a mistake in the computation of the witness script size
in the unit test. We also get rid of the needless wu_to_vb() standalone
function.
Rust-bitcoin, that we use through rust-miniscript, has seen plenty of
breaking changes in the latest version. I've tried to keep the necessary
changes here minimal, still it had to be a single commit to keep it
hygienic. But i'll try to summarize the main things here. Tobin also
wrote a guide about the release at
https://rust-bitcoin.org/blog/release-0.30.0/.
The most verbose change in this commit is probably due to the `Address`
type overhaul. It's overengineered if you ask me but hey here we are. I
tried to keep network validation in commands, and otherwise passing
around unchecked addresses (to avoid having to pass around a global
state between our various components).
Another non-obvious change was changes in hash types upstream and the
removal of `ToHex`, forcing us to get the hex representation of a txid
through its `Display` implementation. It is however displayed backward
in this case ("little-endian" if you will), and we need a regular hex
encoding for some queries to the database. We needed to make sure we
didn't implement any silent bug here.
The rest (Script type changes, PSBT serialization updates, ..) is
probably self-explanatory.
5eba9c3189d509e206198e5910dde5c8b8cfb946 Run tests for minimum supported bitcoin version (Wim van der Ham)
Pull request description:
Added the cirrus job for bitcoin-24.0.1. For now only for the `test_misc.py` but this can be extended to the other tests as well.
Fixes https://github.com/wizardsardine/liana/issues/548
ACKs for top commit:
darosior:
utACK 5eba9c3189d509e206198e5910dde5c8b8cfb946
Tree-SHA512: 0a55767b5dbb6668195e6203b5e3e1ceb2ec05773d75dfb1f7666ab0e386966fb724c5bfab5f16dd88a2cb4f07fc1b615aa2823b69192dc01af80a64d7621f71
b53236f9dd29e36a7c31dc8cbe138204e4674a71 Update libc wrapper and getrandom (Antoine Poinsot)
9b17d7029ff6e80eb177bf8d04efb85cdefa21ef Update rusqlite to 0.27 (Antoine Poinsot)
1271b646fb87b5d45bd001c616dda4ee1dd37bed Update backtrace dependency (Antoine Poinsot)
5d7a1b4fe5de2a1247f3f0445a87c575e71b129f Update serde dependencies (Antoine Poinsot)
c20c05cee7b25fe6bc9a638b48a14a4430185edc ci: upgrade lianad clippy to 1.70 (Antoine Poinsot)
b2f95ada585fb04c3d82d9419d33affebd9eaccc commands: take advantage of 1.48 compat riddance (Antoine Poinsot)
117221e9eed92fd197822b82a25de094b79cfafd Bump lianad MSRV to 1.54 (Antoine Poinsot)
Pull request description:
The latest Debian stable was released this month with support for Rust up to 1.63.
Fixes#69.
ACKs for top commit:
darosior:
self-ACK b53236f9dd29e36a7c31dc8cbe138204e4674a71
Tree-SHA512: f2b7fc1462a71cc76897d7cb0fe6644cf5927d5db60b42112fdc6c5c68334af22f45b6d99b8bdfdab1222812bd3832a9abf89ca429789d734cfdc8e9e8f753c8
The release is >1yo but they keep breaking the MSRV so we can't get the
fixes without upgrading to a bleeding edge compiler...
This upgrades the bundled SQLite to version 3.38, which had quite some
patch releases but no issue that should be relevant to us.
https://sqlite.org/releaselog/3_38_0.html
Full rusqlite changelog available here:
https://github.com/rusqlite/rusqlite/releases/tag/v0.27.0.
