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
9685920102b6f8340aeec5493b07e55ae9628398 tests: fix a small race in the rescan_and_recover test (Antoine Poinsot)
Pull request description:
ACKs for top commit:
darosior:
self-ACK 9685920102b6f8340aeec5493b07e55ae9628398 -- trivial, tested locally by running many instances of the test in parallel
Tree-SHA512: b384eeac6eba7416a9316b271c4ace26d933f9bfb85a6f4cc59133937cafda0baa9086926fad8ba8c262a1edaf3479b1a6db190da339fd8ff78820aa46b90656
We used to store it there, if it's not within our own datadir copy it
from where it would have been stored by Liana v1.
Note we don't conditionally compile this on Windows so the codepath can
be tested with a functional test.
5c87937d4676354f19357e17b36655eb4853dbf9 Add more bitcoind-related setup logging (Antoine Poinsot)
Pull request description:
At startup it sometimes appear we may be hanging when setting up bitcoind. Add more logging to give more information about what's taking long to setup (for instance, loading the watchonly wallet).
Related to https://github.com/wizardsardine/liana/issues/475.
ACKs for top commit:
darosior:
ACK 5c87937d4676354f19357e17b36655eb4853dbf9
Tree-SHA512: 75c553cabf545d57a5fc2d251e5b4cd880a931a408c6f1b1052067f2ccc8e0728ad779f30de5a88c2566f9e67ac085b713393ee4fa193331e49c3a8e6112ecc8
It seems to be causing some races that i don't want to be investigating
at the moment. It's redundant anyways as we are already checking for the
return code of the process.
At startup it sometimes appear we may be hanging when setting up
bitcoind. Add more loading to give more information about what's taking
long to setup (for instance, loading the watchonly wallet).
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.
