We introduce support for tr() descriptors alongside wsh() descriptors in
creating (compiling from policy, parsing from string) and working with
(analyizing its policy, getting spend information) a descriptor.
When compiling a Taproot descriptor, if no key from the policy could be
used as single internal key we deterministically generate an unspendable
internal key as per
https://delvingbitcoin.org/t/unspendable-keys-in-descriptors/304/21.
Similarly when lifting the policy of a Taproot descriptor, if the
internal key matches the deterministic unspendable key for this
descriptor we discard it from the analysis.
To fill information about an output for signers, we re-use
rust-miniscript PSBT input updated instead of re-inventing the wheel. It
does necessitate a hack however to use a type they would accept.
We don't change the "max size of a spending input" for now, even though
it means we would significantly overpay fees for descriptors with a
spendable internal key.
f2c418f79a6fe8f1d6138b12af9fdc7303b4725d get genesis timestamp from bitcoind (pythcoiner)
Pull request description:
fixes#904
ACKs for top commit:
jp1ac4:
ACK f2c418f79a.
Tree-SHA512: 7b1b7c13c21b657109ab0aab0d89deb47f5f6693f95687b86c219fc109ee304c38e3ebe865d071ebbbfa890a6a754c615f5dd70f2fe8c492bdea35d0394f36eb
The most notable change is rust-bitcoin's change in the serialization of
transaction with no input. It now accounts for the segwit marker even
for those. The base tx weight in coin selection had to be adapted to
handle this.
See https://gnusha.org/bitcoin-rust/2024-01-04.log for details.
When not in IBD but catching up to the latest tip, our rounding up of
verificationprogress makes us start while bitcoind is still kind of far
from being caught up. Make sure it doesn't happen by not returning until
bitcoind validated all the blocks we've fetched the headers for.
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.
This makes it possible to have more than one recovery path in a Liana
descriptor. The descriptor and partial spend analysis are adapted to
report information about all recovery paths.
9394be645c698591da9c477dd77363010cb3298e [bugfix] descriptors: fix parsing of descriptor with 1-of-N multisig (Antoine Poinsot)
1a13b7a6f820e92ff436198bffc78b8ad785a758 descriptors: rename InheritanceDescriptor into SinglePathLianaDesc (Antoine Poinsot)
8d1c6de5dde85583a6fb3a03458774d113ccb7b9 descriptors: rename MultipathDescriptor into LianaDescriptor (Antoine Poinsot)
f6885e358bfe78a790226e6246dad4922cf82d02 descriptors: cleanup error types (Antoine Poinsot)
647d65fe045a71158041cfb4bf5b98e5200db2e8 descriptors: create Liana descriptors through the policy (Antoine Poinsot)
9b866300be53be8a4e49e7913be25ff8887eac63 descriptors: merge the semantic analysis in one place (Antoine Poinsot)
cd566b91af07a53f9651c034ef4da95a8a033c56 descriptors: rename LianaDescInfo into LianaPolicy (Antoine Poinsot)
757009536b489b333ddb6a2d6bf237196e930e4e descriptors: make sure there is at least one timelocked path when parsing (Antoine Poinsot)
eebfa4755944f14aaa23f8b1a293a1b6a0f0f30f descriptors: move descriptor policy analysis into its own submodule (Antoine Poinsot)
c0dd63dfb2b6831666fb260581dce36e6f7601fa descriptors: move the LianaDescKey to the keys submodules (Antoine Poinsot)
7772ae8d8a74dc0f52be0381a77e68e4d2e8478f descriptors: move derived keys into their own submodule (Antoine Poinsot)
9e78ac7e8dd8bfb7170f64aa314aea30921aca4b descriptors: make the module a folder. (Antoine Poinsot)
Pull request description:
We've been piling a bunch of new features since this module was first architectured, and it has become messy. This led to duplicate code, a confusing interface (`InheritanceDescriptor`, `LianaDescInfo`, ..) and more importantly bugs.
This is a complete re-organization of the module in view of introducing multi-paths descriptors soon. This PR contains two bugfixes but aside from that it should not change (correct) behaviour. It does however completely break the interface.
The new interface makes a lot more sense:
- A `LianaPolicy` representing a Liana spending policy, from which you can get the parameters for the various spending path, and you can create from those parameters.
- A `LianaDescriptor` which can be created from a `LianaPolicy`, and from which you can infer a `LianaPolicy` to retrieve the parameters of each spending path.
This bijection (although it will soon become a surjection as we'll introduce the Miniscript policy compiler to create a `LianaDescriptor` from a `LianaPolicy`) makes the life of a client of the API easier, but it also harmonizes the code: we've centralized the Miniscript Semantic Policy checks of a descriptor in a single place to make sure that we can parse only what, and all, descriptors we can create.
ACKs for top commit:
edouardparis:
ACK 9394be645c698591da9c477dd77363010cb3298e
Tree-SHA512: 784eee825644db43417ec040f85b9e20ab72bcc545eed68a2b9b5a5945f86bea6e2d7b091e438b7ba8d4e0a6963459f2b29af59995a407a3c509b5be0fd06e9b
It was named at a time where there was an over emphasis on inheritance
as a Liana usecase. In addition, "SinglePath" reflects better it is only
one part of the main, multipath, Liana descriptor.
This makes it possible for a LianaPolicy to be created from a user
configuration. This in turn centralizes the descriptor creation inside
it as well and make `MultipathDescriptor` take a `LianaPolicy` directly.
This is useful to centralize all the Miniscript and Miniscript policy
handling under in a single place as we'll soon be managing much more
complex policies (and make use of the Minsicript policy compiler).
Unfortunately this is an invasive API change. But at least the API now
makes a lot more sense: you can create a spending policy from a
configuration and create a descriptor from it. And vice-versa you can
infer a spending policy from a descriptor and inspect the configuration
from it.
It would be possible for users to create a descriptor with xpubs without
an origin set. In fact, not many are used to origins and it's a very
likely mistake. Signers need this information in order to be able to
sign. So they could potentially create a wallet and potentially never be
able to sign for one or multiple keys.
Fix this by requiring an origin for all keys in the descriptor.
We only allow to create "legacy" multisigs for now (that is, what fits
in a CHECKMULTISIG, ie k-of-n with n<=20). See
https://github.com/wizardsardine/liana/issues/53 for discussions around
this.
We are a bit more laxist on parsing, where any miniscript that
corresponds to a multisig policy will be accepted.
Note this makes the helper inferring the CSV value from semantic policy
incorrect, but this is fix in a later commit as part of a complete
refactor of this logic.
Instead of only incrementing it, allow to be able to set it to any
value. This will be useful for instance to set the derivation to the
latest used onchain, if another wallet is much further down the
derivation tree than we are (or after a rescan).
We'll need to store in persistent storage if a rescan was requested by a
user, and if so from what date.
For the SQLite implementation we introduce a rescan_timestamp to the
wallet table.
It seems that internally bitcoind might temporarily not have a pprev
pointer for a block. This will result in the optional
"previousblockhash" field to be null and would previously make us crash.
Handle that gracefully.
In config, expect to be given a multipath descriptor that contains a
derivation path for both receive and change addresses, but only for
those.
Instead of 'xpub/*', start using 'xpub/0/*' and 'xpub/1/*'.
When creating the watchonly wallet on bitcoind import both the receive
and change descriptors.
When polling, check for coins on both descriptors.
Our bitcoind watchonly wallet could, maybe, have other descriptors that
were imported. Sounds pretty unlikely since we use a dedicated wallet
but hey.
More importantly, we'll need to know the parent descriptor of the coin
in order to recognize it as newly received or change.
This also updates the indirect rust-bitcoin dependency to 0.29.
Major changes are in the descriptors management:
- The rust-miniscript descriptors don't support raw hashes
within the pk_h() fragments, so we don't need all the boutique
management for this.
- The key translator API changed.
- We now take a u16 for the timelock instead of our previous
checks. This was inspired by the new rust-bitcoin Sequence
type.
- We now take a path instead of just a derivation index in
DerivedKey. We might use paths after all...
As for rust-bitcoin it's just a few nits:
- No more 'global' field on PSBTs (yay)
- Couple more trait derivation on types
- Some APIs were renamed.
Hence add a 'spend_block_height' field to the 'coin' column in the
SQLite implementation. This also contains a couple cleanups, as well as
a fix (we were still checking if the blockheight was > 1).