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.
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.
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.
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
13398982534d56a5723dfa86723c5917483c8653 commands: include missing amount in response (jp1ac4)
Pull request description:
This PR follows a discussion around https://github.com/wizardsardine/liana/pull/873#issuecomment-1886715468.
The GUI uses the `InsufficientFunds` error to get the missing amount when the user is creating a new spend, but it is not straightforward to extract this information in a general way from the RPC error (see https://github.com/wizardsardine/liana/issues/822#issuecomment-1836482355) and instead the spend module's `create_spend` is currently used (see https://github.com/wizardsardine/liana/pull/863).
With this PR, the missing amount will be included in the `createspend` response rather than as an error.
These changes are based on suggestions from @darosior and @edouardparis.
In a follow-up PR, the GUI should revert to using the `createspend` command to calculate the amount left to select.
ACKs for top commit:
darosior:
re-ACK 1339898
Tree-SHA512: bf702d6b355339e96e719c1d95824e7941ac4fbaece4ec4cccd00b56ea4683ce7fb0cefc43faa5731b57e7935ef99da3a2c73b84aaeb9fa5f67703c799be2196
The GUI uses the InsufficientFunds error to get the missing
amount when the user is creating a new spend.
It is not straightforward to extract this information in a
general way from the RPC error. Instead, this missing amount
will be included in the command response.
These changes are based on suggestions from darosior
and edouardparis.
This is a temporary partial fix for
https://github.com/bitcoindevkit/coin-select/issues/6 that should be
reverted once the upstream fix has been made.
When calculating the score, the excess should be added to changeless solutions
instead of those with change.
Given a solution has been found, this fix adds or removes the excess to its
incorrectly calculated score as required so that two changeless solutions can
be differentiated if one has higher excess (and therefore pays a higher fee).
Note that the `bound` function is also affected by this bug, which could mean
some branches are not considered when running BnB, but at least this fix will
mean the score for those solutions that are found is correct.
This adds a new `LowestFeeChangeCondition` metric for use in
coin selection. It's the same as `LowestFee` with the option
to only find solutions with change.
This option is then used for self-sends to ensure only a solution
with change will be returned.
The derivation index is required for
for client to derive and verify the address
on hardware wallets.
Co-Authored-By: Antoine Poinsot <darosior@protonmail.com>
When creating a new spend, if coin outpoints are not provided,
then coins will be selected automatically.
This automatic selection is such that the transaction fee is
minimized, taking into account the cost of creating any
change output now and the cost of spending it in the future.
If change is added, it must reduce the transaction waste and
be above the dust threshold. This same policy is applied also
in the case of manual coin selection, replacing the previous
logic for determining the change amount. This ensures that
creating a spend with auto-selection and another with manual
selection using the same auto-selected coins will give the
same change amount.
2660b77487d63218019413a4ca33b3a9629fbfc8 implement listadresses (pythcoiner)
Pull request description:
address #681
todo:
- [x] implement tests
- [x] update docs
edit: i'm really new to rust, don't hesitate to kick my ass when i write stupid code
ACKs for top commit:
darosior:
ACK 2660b77487d63218019413a4ca33b3a9629fbfc8 -- my requests are addressed in followup #808.
Tree-SHA512: a5fdfb4516dc0379bfec1be535e752795dec75d28cbc5b9fa4fe9898fa00b1cfaa9cee3b95f4dfd68365f4585426e1b4457a8366cc4f783600704994f879526f
This reverts commit 71056982636b408485ab24dab6628a555a6e7924, reversing
changes made to 03c37bd378f4f6bf11d90b224ed1db74b3596eaf.
This reverts PR #722. It turns out the Ledger Bitcoin app needs the
BIP32 derivation for all the keys in the Script, not only for the
spending path used. Therefore always create PSBT with all the BIP32
derivations. We'll add a way to prune them for talking to the Bitbox in
a future commit.
This reverts commit ec0c2426aa5fa6cee2efabd3ee6f175b41c35f64, reversing
changes made to 26d750d09c84734f56c2dc18cb332a232e24fb6d.
This reverts the fixes to the pruning of BIP32 derivation paths when
creating a PSBT, in preparation of reverting the merge of this feature
altogether. This is because always creating PSBT with only the BIP32
derivations for a single path broke the Ledger support.
7f3b0b021858cfb2fe914f3ba6b30a39e3ae05ff qa: test a PSBT has only the BIP32 derivations for a single spending path (Antoine Poinsot)
b71bd693d6ea4c1c3567194fc82be43fb70c05bb qa: don't use a static dummy origin for descriptor xpubs (Antoine Poinsot)
a81d39c81a89cdf5e70b9888cbeb3abaf290a365 commands: do not include BIP32 derivations for other spending paths (Antoine Poinsot)
Pull request description:
ACKs for top commit:
edouardparis:
ACK 7f3b0b021858cfb2fe914f3ba6b30a39e3ae05ff
Tree-SHA512: f0e132edf8d653c5575f843b1e85d995f155a2435a6e6257564dc945562df70ec2049c29d77f7580858d9e4a58290b0798f298f77c18255415c4cf26ccc07f33
