We introduce Taproot support in the test framework through a global
toggle. A few modifications are made to some tests to adapt them under
Taproot (notably the hardcoded fees / amounts).
This is based on my introduction of a quick and dirty support for
TapMiniscript in my python-bip380 library:
https://github.com/darosior/python-bip380/pull/23. In addition to this i
didn't want to implement a signer in the Python test suite so here we
introduce a simple Rust program based on our "hot signer" which will
sign a PSBT with an xpriv provided through its stdin and output the
signed PSBT on its stdout. Eventually it would be nicer to have a Python
signer instead of having to call a program.
The whole test suite should pass under both Taproot and P2WSH. Only a
single test is skipped for now under Taproot since it needs a finalizer
in the test suite.
I also caught a bug in the RBF tests which i fixed in place.
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.
3017b88e27849830530fc0ccf77df215889d17ba fuzz: fuzzing integrations, fuzz the descriptors module (Antoine Poinsot)
f7924fb9dcd4d91d404d1da736021f83619e2529 descriptors: lifting *can* fail (Antoine Poinsot)
Pull request description:
This introduces fuzzing into our project with two fuzz targets exercising our descriptor parsing logic. See the commit messages for details. This found a crash (first commit).
This was motivated by testing the work on Taproot (#985).
ACKs for top commit:
darosior:
ACK 3017b88e27849830530fc0ccf77df215889d17ba - it's not interfering with anything in the repo, been running these for half a day with no crash.
Tree-SHA512: 25c1b64a86585fc5f676c3526e2dae945b74c6b0cb4ce2d9db33dc48aa85aaa11a07b279838703d62c9ca00cf39cc34577ca19c0a8f9aaf5327266eb7be6dce0
This integrates fuzzing into our project by introducing two targets
which exercise the descriptor parsing and analysis logic.
The `descriptor_parse` is dead simple but not very effective. The
`descriptors` harness tries to be smarter by almost always generating a
valid Liana descriptor.
Of course, this is just a first integration and both could be made more
effective.
cc1de1d6d6710f1426a957806661e7f3461a7cb5 poller: unspend coins before spending new (jp1ac4)
1e7653e08a3778446ff677bb147df68b734a31fd tests: add function to wait while condition holds (jp1ac4)
Pull request description:
This change ensures that the spend txid of a coin is updated directly to another value in case a conflicting spend is detected.
The previous order caused the spend txid to first be cleared, which would misleadingly make the coin appear as confirmed
rather than spending.
I've added a new utils function for the functional tests that is a slight generalisation of `wait_for` with an additional condition that must always be met while waiting.
`wait_for` now calls this new function with the condition being one that is always true.
ACKs for top commit:
darosior:
ACK cc1de1d6d6710f1426a957806661e7f3461a7cb5
Tree-SHA512: e3f00804a63b0e94bc1b2cbee03cac63dd6e2555ca6d301589b356b2baf8e0cf27362e1dda44018d1d8282e300b187079fcf61f5d2754263b9e8b08cd34be06e
This change ensures that the spend txid of a coin is updated
directly to another value in case a conflicting spend is
detected.
The previous order caused the spend txid to first be cleared,
which would misleadingly make the coin appear as confirmed
rather than spending.
This adds a new utils function that is a slight generalisation
of `wait_for` with an additional condition that must always be
met while waiting.
`wait_for` now calls this new function with the condition being
one that is always true.
a997a7bcffac6eecbe472e383b7cb97bd2af738d gui: don't rely on cookie to check successful start (jp1ac4)
5e5c3330ee14159b14e43852d8db1d51a1b6bad8 gui: use rpcauth in installer for internal bitcoind (jp1ac4)
7584b6347bf6525d57d09596a60b4bdd2bccfb7f gui: start internal bitcoind with given config (jp1ac4)
b5980fbc4f6be006b083fbc8c7e4142e50598fb8 gui: add sections check to internal bitcoind config test (jp1ac4)
c3aad0f40c94ae86d1b14ff1b05178c0c043f732 gui: add optional rpcauth to internal bitcoind config (jp1ac4)
e172ecd2f1e910a5ba16ea4debb0091b70c2cb17 gui: move internal bitcoind config to bitcoind module (jp1ac4)
Pull request description:
This is to resolve#929.
The installer will generate an `rpcauth=` string for use in the managed bitcoind's bitcoin.conf file and will use the corresponding user and password in the Liana daemon config file.
~~The existence of the cookie file is still checked by `Bitcoind::start()` to determine if the process started successfully. An alternative approach could be considered in a follow-up PR (or as part of this one).~~
For https://github.com/wizardsardine/liana/issues/924, we might still need to use the cookie path to stop an already-running process, as we would probably no longer have the password used to generate the previous `rpcauth=` string.
Once https://github.com/wizardsardine/liana/pull/987 is merged, I'll update the Cargo files in this PR.
In the first commit, I move some code from the installer to the bitcoind module. In the end, this wasn't strictly required for the other changes I made, but I kept it as I thought it made sense anyway.
ACKs for top commit:
edouardparis:
ACK a997a7bcffac6eecbe472e383b7cb97bd2af738d
Tree-SHA512: ec475bb3c82db4e30c42612e93c63e9b0387311a26d5433421f275e24ac5b0923eba9bd1daff612e60a6cf0bdcc10de0812ff69972237e6785c0835b5a31450f
This will allow bitcoind to start also in case of using rpcauth.
The cookie file is canonicalized in the installer beforehand
instead.
There is now no need for the config parameter to be mutable.
daf60264d16a554f8877f28e11b5df6eab33d324 lib: expose random module (jp1ac4)
Pull request description:
This is motivated by #929. The GUI will need to generate a salt and password for RPC user/password authentication to the managed bitcoind, and can use this module to get random bytes.
ACKs for top commit:
darosior:
utACK daf60264d16a554f8877f28e11b5df6eab33d324
Tree-SHA512: a6ba822bffc6f3d2e5c96be986cb5f1024640b5e71b3bb8a611080e5bcc734b25281fbc52cc1eb15f89d7c72cac8495c163a630758f66ac7e72410ecb7be34cb
bfd6ca517b1811a888f83f14d3f6900c590dc3c1 commands: increment receive index on each use (jp1ac4)
cc5e396ace8b24e7e65bc356655463a690a25091 commands: increment change index on each use (jp1ac4)
c88224ca715ccfdc6530362b960dddd54ff64610 testutils: fix change_index (jp1ac4)
Pull request description:
This is a fix to ensure the change index in the database is incremented if a new spend is created with a change address derived from the current index, regardless of whether this new spend is broadcast or not.
EDIT: The same fix has been applied to the receive index.
ACKs for top commit:
darosior:
re-ACK bfd6ca517b1811a888f83f14d3f6900c590dc3c1
Tree-SHA512: 66f88fe95f4fb518cd93cde12787f74b6db61e6a2ad6ae27ee4f134ac039df6ce03243afde321412486c435641122ddc27baace2a15a167d3bfe20e17be22597
This is a fix to ensure the change index in the database is
incremented if a new spend is created with a change
address derived from the current index, regardless of whether
this new spend is broadcast or not.
fa39e2ced460918d6c6c94c0e2b7dae19b612fa9 database: convert `CoinStatus` to `listcoins` argument string (jp1ac4)
Pull request description:
This is a preliminary PR towards https://github.com/wizardsardine/liana/issues/677 and is the counterpart to the existing `CoinStatus::from_arg()` method.
This is needed so that the GUI can pass the required string arguments to the `listcoins` RPC command when using an external Liana daemon.
ACKs for top commit:
edouardparis:
ACK fa39e2ced460918d6c6c94c0e2b7dae19b612fa9
Tree-SHA512: e555739e641e4be66ce4942e8294606ab001572395ed73a6c40fa2af38f076357a7c3acba4e4d756fb6d334a11107827a95c53d3f3aecfe32ceca80fc85af83a
f2c418f79a6fe8f1d6138b12af9fdc7303b4725d get genesis timestamp from bitcoind (pythcoiner)
Pull request description:
fixes#904
ACKs for top commit:
jp1ac4:
ACK f2c418f79a.
Tree-SHA512: 7b1b7c13c21b657109ab0aab0d89deb47f5f6693f95687b86c219fc109ee304c38e3ebe865d071ebbbfa890a6a754c615f5dd70f2fe8c492bdea35d0394f36eb
c0d432057df6b2f0bc879f718489c761867fa88c spend: add warning about fee for ancestor (jp1ac4)
a38c1739b6d6bdc5944e2300f1cdf9121babf7d1 spend: return additional fee paid for ancestors (jp1ac4)
62bb4aded43a379bcf5607fd48c492fcfb84bfd5 commands: include unconfirmed change as candidates (jp1ac4)
b05b0f14e5e08ab32e78ea263531d8a587c5b184 commands: add ancestor info for user-selected unconfirmed coins (jp1ac4)
94ef66c03a17221cc9cddb8d1d463a46f0b489c9 spend: increase candidate weight to pay for ancestor (jp1ac4)
5f0022083df8354ceb03d79311317e378296629d bitcoin: add mempool_entry to interface (jp1ac4)
edbf00f17c3796dc326bfbe6ae8833c201bdc045 bitcoin: add ancestor size and fees to mempool entry (jp1ac4)
04503225bcd066dd7d4099997c56b836c29d5b31 func test: use utils function (jp1ac4)
Pull request description:
This is a first PR towards resolving #826 that adds unconfirmed change as coin selection candidates when creating a spend.
As per https://github.com/wizardsardine/liana/issues/826#issuecomment-1853058734, I haven't made any changes to the `rbfpsbt` command.
We will also need to apply the same change in the GUI when selecting candidates for coin selection there (see https://github.com/wizardsardine/liana/pull/863#discussion_r1422823362).
ACKs for top commit:
darosior:
ACK c0d4320
Tree-SHA512: 8c17f5f8c32913f1ffae3a93ca3e8ee52ac40ee86790e41d73def5ed0c057e110e101797f778715fcd5f6bded1cd170618209323b5114a4f69c02d0ce066a2f2
0c1a1b8d6d6ead20fdb1af659cace229f814d9fb gui: add redirection to selected transaction panel (edouardparis)
Pull request description:
based on #959
This PR removes the payment section about the transaction inputs outputs for a redirection to the payment transaction (The `See transaction details` button). It makes it easy to fee bump a payment by clicking on it to be redirected to the transaction to rbf
Maybe the UX/UI of the payment should be changed to remove more transaction information, but I fear it makes the panel a little bit empty.
ACKs for top commit:
jp1ac4:
ACK 0c1a1b8d6d.
Tree-SHA512: f767b1cbf86cafcd0b46e5dfc0625f15719b48314de93418b73e59bd32e6dc659ab2ea3a4e2d379680c4209bfbd633c5329fe9fa2616c042dd2851f62f860498
18e040e51ff835f5f6c8218392496865b6f8737a fix: override unpaginated pending events and txs (edouardparis)
2995df870f711966d7d275f67de70719dd1498ce fix psbts panel: keep list of psbts in background (edouardparis)
a6832ad0b7a881889e26c10c65f4d0ecd6e8c098 fix send panel: reload coins (edouardparis)
710883b6a2467e4c8fab9b2382e8227b602dfa42 Restart spend process according to current state (edouardparis)
a91fdd791aedc997e4b43e23db8af8c5382cab17 Encapsulate cache with its own message (edouardparis)
dc3d29e3f06b7cb2aa0d502edd6f7fcf0aadf458 Remove spend_txs from cache (edouardparis)
2d2cd12bda47feff6faa01cb5a1338d7f0f9f7a0 Restart new spending process on user demand (edouardparis)
ed363963b34235d49291a0da7719fd655ff7a8dc Change state load method for reload (edouardparis)
c15424abe5a3255b63d45903f14b1044af859e6a fix clippy (edouardparis)
fa4483a4b71ceb17463e89a27d1337c3cfc4d20d gui: add reload cycle to reset state (edouardparis)
c268c3a093594b76d424d935840c8c5ef6c83482 gui: load receive panel only once (edouardparis)
9b4b6fef1bee12e3e370a34c9d9c30eb437bf404 gui: refac app, keep all states (edouardparis)
Pull request description:
This refac keeps each panel state in parallel.
ACKs for top commit:
jp1ac4:
ACK 18e040e51f.
Tree-SHA512: 87937a5e7cf315325e06aeda0789024a68cfb2c137cd1319445e670fa2641775a6d58cbc787271f2b73d43ff5b83e934f0561a4968afd05f8f52cff85e81a27e
pending events and txs are passed unpaginated and
in full list through PendingTransactions and PendingPayments.
It is useless and armful to append to them to an existing
list of pending events as a new rbf replacement event should
override the previous one and only one of them must be displayed.
When going back to send panel, coins are fetched
again and passed to DefineSpendStep. A redraft
is triggered because some coins may have been
removed from the list and new coins were deposited.
