Here, the min RBF feerate is 1 more than that of the transaction
to be replaced.
The feerate of the transaction to be replaced may vary slightly
depending on the signature size and is calculated during the test.
As such, the min feerate should be set according to this
calculated value.
8be0d9856b29eb30593ae7614fe74430ffd206d4 commands: add minimum feerate to TooLowFeerate error message (Aaron Carlucci)
Pull request description:
This PR adds the minimum feerate value into the `TooLowFeerate` error message for additional context. Addresses the second bullet point in #853.
ACKs for top commit:
jp1ac4:
ACK 8be0d9856b29eb30593ae7614fe74430ffd206d4. Thanks!
Tree-SHA512: 3a0e9c69ddb40433faf1e58c16dfee0212641b34478eddeda99388ebf82e2fb466633ab8f79c9bb0bb784f4a46807aa0a5abd65b9d92506144b84addb01dfdd8
f2791744d8ad3068d12a39bb07f3745012785a5f spend: set locktime for anti-fee sniping (jp1ac4)
Pull request description:
This is to resolve#44.
In the first commit, I combine the destinations and change parameters in `spend::create_spend` in order to avoid "too many arguments" error when adding a new parameter. I think these two parameters combine naturally so that `destinations` includes both recipients and change address.
The second commit sets locktime following the same approach as Bitcoin Core:
- locktime is set to current tip height, but randomly (about 10% of cases) value is set up to 100 blocks earlier
- if tip is more than 8 hours old, locktime is set to 0
For randomness, I'm currently using the current time's milliseconds in order not to add another dependency, which I think is good enough for this use case.
For consistency, I decided to use "locktime" everywhere instead of "lock time" or "lock-time".
ACKs for top commit:
darosior:
ACK f2791744d8ad3068d12a39bb07f3745012785a5f
Tree-SHA512: 69a57cf664e24b32a835c35eaf9016961b2d0f396891a826582044e6302b2ca04dcf5bf2617b5e18dcbfa25cc254a6e8025262718984095677c829ce051a66cc
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.
f2c418f79a6fe8f1d6138b12af9fdc7303b4725d get genesis timestamp from bitcoind (pythcoiner)
Pull request description:
fixes#904
ACKs for top commit:
jp1ac4:
ACK f2c418f79a.
Tree-SHA512: 7b1b7c13c21b657109ab0aab0d89deb47f5f6693f95687b86c219fc109ee304c38e3ebe865d071ebbbfa890a6a754c615f5dd70f2fe8c492bdea35d0394f36eb
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.
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
Multiple change indexes may be present in a spend
draft transaction and can be detected instead
in the response psbt with the bip32_derivation
outputs fields.
