6b5fc2d1faf94d8290a30bd7007113815c8936ea qa: fix flaky test of unconfirmed spend RBF (Antoine Poinsot)
Pull request description:
Since #617 the spend info for a coin may be wiped from the DB. Be robust to a temporarily `None` spend info in the functional test.
See for instance https://github.com/wizardsardine/liana/runs/18785511349.
ACKs for top commit:
darosior:
ACK 6b5fc2d1faf94d8290a30bd7007113815c8936ea -- trivial and i've run this test a few dozen times concurrently locally to make sure it's unflaked.
Tree-SHA512: 7470a1fd4c16ee7af6ee4c9d9e57271f5f18ac728062cc937a366681534714ec404f2b8e0a58e0d5f51a7461e28dcae6d93f49e5fc0f45297eae9b979e821f86
317ab964f707f049e51bbb4be7ae32975ca3da45 bitcoin: add a comment about the new spend detection logic (Antoine Poinsot)
6daf7ac2603a4a4f588e6106d3c4398228113b97 poller: don't check spending status of expired coins (Antoine Poinsot)
0e5634ce5986d70ab703f639882e35bba6ee571f bitcoin: poller: document where RBF is handled for spends (Antoine Poinsot)
cf7c4fbac96c421c311fd86255cdea26394954e3 bitcoin: drop spend txid for coins whose spending tx gets RBF'd (Antoine Poinsot)
544167dee4c631b324848d9f558ba48504582018 bitcoin: mark coins whose spending tx got double spent as unspent again (Antoine Poinsot)
f78e831c39311cd5d758ea81d6e855a85d13fa5c db: make it possible to mark coins back as unspent (Antoine Poinsot)
c30bc8cd18ac77f64179a8794a26770c68eb4916 bitcoin: optimize spend conflict confirmation lookup (Antoine Poinsot)
bc25addf342d5f570eb20f97b0413bb7109f0be9 bitcoin: don't assign incorrect spend_txid on conflict tx confirmation (Antoine Poinsot)
20ab30924fbdf62bb3886b2da75fe8c57555d2b6 qa: add a test describing current poller behaviour wrt replacements (Antoine Poinsot)
Pull request description:
We start by illustrating the current logic of the poller with regard to replacements in a functional test. This exposes a bug: we could incorrectly assign a transaction which conflicts with a spend transaction for one of our coin as spending this coin whereas it in fact didn't.
After fixing this bug, we proceed to make it possible to wipe the spending status back to unspent. First when a conflict is mined, then also when it's only accepted into our mempool. This matches how we treat replacements for deposit transactions.
ACKs for top commit:
jp1ac4:
ACK 317ab964f7.
Tree-SHA512: bfad864cf03947e5d42894de12ae281a8cff10964d299df5b6b74310efbe6bbefb347349b2d421f1fb3e9470fa929fd7c2c221c9f161dd602757f75b66355ea0
9e2407eb8a75bc59c16659bf4aa80f8adef0e3d2 gui: auto-select coins for spend (jp1ac4)
Pull request description:
This adds automated coin selection from #560 to the GUI.
The new "Auto-select" button uses automated coin selection to select coins for a spend, which the user can then modify as required.
The button can only be clicked once the recipients and feerate have been validated, and it does not appear when creating a self-send.
I haven't added any validation on the amount before making the button clickable, but it may be hard to anticipate all coin selection errors once fees are taken into consideration.
ACKs for top commit:
darosior:
light ACK 9e2407eb8a75bc59c16659bf4aa80f8adef0e3d2
Tree-SHA512: 6ce3389d849470b3beb6ac8df75d2c3b7b6c04ee881dd0e9116c4d87f54376a8ed6666cbfd0ff0152a3eb839c8f7f17a175fe078ef030f03451430b84ab40cb6
33384c89b5cdaec9d4a477644609ff0d1508cf9f Add derivation_index to GetAddressResult (edouardparis)
Pull request description:
My bad, i pushed my fixes in a rush in #815 and wrecked the PR.
ACKs for top commit:
darosior:
ACK 33384c89b5cdaec9d4a477644609ff0d1508cf9f
Tree-SHA512: 5f18506a84ff10fbe13757c2522b11a7ec005243f8698425f1d761d4b36378c256150d3d8337fefc1938232990004bbd629878b42b032b1dd0349d593a39386d
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>
2e2afc1d7bf85dec7319b78e5a2634558952bc25 doc: document deriv index in getnewaddress result (Antoine Poinsot)
Pull request description:
The derivation index is required for
for client to derive and verify the address
on hardware wallets.
ACKs for top commit:
darosior:
ACK 2e2afc1d7bf85dec7319b78e5a2634558952bc25
Tree-SHA512: dd6280662244ff307a8eceb76a48d7b0ed651cc6e512708029afd6dda4bde0b0d0f8a63d30b9a7710d819f03e2d4bad250e3e24abab23578b5b684665a429c52
cfa0f91dd36bc81d5820baf975930e94063ef691 commands: auto-select coins if none provided (jp1ac4)
Pull request description:
These are some initial changes towards #51.
I've added a `selectcoinsforspend` command that applies BnB coin selection using a waste metric.
This coin selection is then used in `createspend` if no coins are specified.
@darosior The changes are still in their early stages so I'm creating this draft PR to facilitate discussion about the approach in general and specific details.
ACKs for top commit:
darosior:
ACK cfa0f91dd36bc81d5820baf975930e94063ef691
Tree-SHA512: 2b94a8f4d335366e477fff54fa51d478ef459e2e729bac00a5d4ac21d04667409cb685642f27fd1936456a05a8d76d23483e45a24f5d342f9a26de904bb6639c
79c5f92d9c690c36308a9d3fa4b35a4c4f6e01d4 db: check for change using address strings only (jp1ac4)
Pull request description:
Comparing the address itself includes the network, but an unchecked signet address will have its network set as testnet by `assume_checked` and so change addresses will not be matched.
ACKs for top commit:
darosior:
utACK 79c5f92d9c690c36308a9d3fa4b35a4c4f6e01d4
Tree-SHA512: 50c87a6feac3f659584e92ad8781092bf513d63a590453f87d8617ec4b309ec3128050e6c10114d0150e08a373d4d1824a99b2530ab82eb5b14dd3790ec15237
Comparing the address itself includes the network, but an
unchecked signet address will have its network set as testnet
by `assume_checked` and so change addresses will not be
matched.
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
88d00ae1a3c99b32a5bd0aa457b16bb1a8ad064f gui: display headers sync progress too in loader (Antoine Poinsot)
Pull request description:
Maybe we should just get rid of the filtering since there is no stability guarantees wrt logging. On the other hand we'd probably need to increase the update frequency if we do that. In the meantime at least display also the blockheaders so the sync doesn't appear to be stuck for 5 minutes at startup.
Fixes#780.
ACKs for top commit:
jp1ac4:
ACK 88d00ae1a3.
Tree-SHA512: 7ec75c7eea18bba23691fe20e2e4a4680c724e50b77d834da5384c1f1fcbc1e1100d6bb8aacd4ec01e7dbd5051a489376c97168d214054465d7ae3a7ae1fc120
Maybe we should just get rid of the filtering since there is no stability guarantees wrt logging. On the other hand we'd probably need to increase the update frequency if we do that. In the meantime at least display also the blockheaders so the sync doesn't appear to be stuck for 5 minutes at startup.
73f168bd0986b86fc5a3059f6004515deceb3b2d gui: view: fix the display of PSBT missing signatures (Antoine Poinsot)
Pull request description:
The analysis was wrongly displayed. For instance it could display a fingerprint as having both signed and not signed yet.
Clean up the code and fix the analysis.
ACKs for top commit:
darosior:
ACK 73f168bd0986b86fc5a3059f6004515deceb3b2d -- reapplying Edouard's ACK from #783
Tree-SHA512: 1e59702b58e843248c62b62a5ac277f1b0d58841b7d0a9a6ca045fbbbf956999d5982e975f8812fcd59bb4b5c2366bb3c01a69a9a373fecc6d2f719ec046ab62
The analysis was wrongly displayed. For instance it could display a
fingerprint as having both signed and not signed yet.
Clean up the code and fix the analysis.
bdd16c73bcae64e17931bd62d70f4d85b9a6af9a installer: start new bitcoind download if not finished (jp1ac4)
Pull request description:
In case the user clicks on "Previous" before the bitcoind download has finished, this PR will clear the incomplete download (or a download that completed with errors) from `InternalBitcoindStep`.
If the user then returns to this step, either a new download will start or the finished download will be available still. This fixes a bug following #746 for users that already have bitcoind v25.0 installed. If they click "Previous" while downloading 25.1, then upon returning to this step, the existing 25.0 bitcoind will be started.
ACKs for top commit:
darosior:
ACK bdd16c73bcae64e17931bd62d70f4d85b9a6af9a -- code makes sense to me and Kevin tested it fixed (one of) the bug he reported.
Tree-SHA512: d1f39b1ee2b1f6fa15533f6c7880dc0e3edc9e7052be8ff315d77232eab9e74f8178252979c14cd8ccac8f164176cbe03a18faf523c969d841d11d6721b18fe1
9d9c4e57c2de0ae6e61bdf3d402dbfc172ed5ae3 doc: mention the bitbox too is supported now! (Antoine Poinsot)
Pull request description:
ACKs for top commit:
darosior:
self-ACK 9d9c4e57c2de0ae6e61bdf3d402dbfc172ed5ae3 -- trivial
Tree-SHA512: 09b3c38ec64014c0e961b5396b3ea70ec8d62cf40f5c61936a6e00344c26f5db44da55a0d29564e96ba953261119836b405a172708993e338c11b022a05b3fd0
In case user clicks on Previous before bitcoind download has
finished, this will clear the incomplete download from
`InternalBitcoindStep` so that a new download will start if the
user returns to this step.
d0065898f70fbe14be7d269a686110e3bbbe7af8 loader: don't read bitcoind's stdout to avoid a deadlock (edouardparis)
Pull request description:
"Forward port" of #770.
It was possible to get into a state where we were waiting for lianad to start, which was waiting on a response from bitcoind which in turn was waiting on the GUI to make room in its piped stdout buffer. Deadlock.
Instead of getting its logs from stdout, read its debug.log file.
ACKs for top commit:
edouardparis:
ACK d0065898f70fbe14be7d269a686110e3bbbe7af8
Tree-SHA512: 0c66190ba87b56b883d15463ad24e152e43b33bf235ba8fb2cab6978c6eace1fdf3b6280377548a92248b18b0fa05bd4ad698cb65beac92bb5f11bc8a4662052
a14f3afa2aab52f686c2bc0a445ca359282922ae installer: do not reload last step on next message (edouardparis)
Pull request description:
If a user clicked very fast a lot on the Next button to trigger multiple Next messages, then the last
step of the install reloads multiple time.
This commit introduce a check that ignore next message if the step is the last one.
ACKs for top commit:
darosior:
utACK a14f3afa2aab52f686c2bc0a445ca359282922ae -- code makes sense and the patch was tested by Kevin
Tree-SHA512: a0afb3eb0793e7d294bdf1d355816ea175f3652f0ce3b018150714ce22425a4ec1f8795173ac241cc43ad2d6dadb3668f0e0d7b720804cc83ad54cd99d9a27af
It was possible to get into a state where we were waiting for lianad to
start, which was waiting on a response from bitcoind which in turn was
waiting on the GUI to make room in its piped stdout buffer. Deadlock.
Instead of getting its logs from stdout, read its debug.log file.
Co-Authored-By: Antoine Poinsot <darosior@protonmail.com>
If a user clicked very fast a lot on the Next button
to trigger multiple Next messages, then the last
step of the install reloads multiple time.
This commit introduce a check that ignore next message
if the step is the last one.
d56a9a8a40671ad0ab0f4d8ccb8c6fa861d40429 fix single payment output label (edouardparis)
Pull request description:
When a transaction has only one payment,
then its txid label is attached to the payment label. While modifying the label of an output it did not change the whole transaction label, which is a bug.
backport of the fix for 3.x in #768
ACKs for top commit:
edouardparis:
Self-ACK d56a9a8a40671ad0ab0f4d8ccb8c6fa861d40429
Tree-SHA512: 0c2bca6f2a5527c37653c3644907926f6dacd01f826a2538a78c898fda407e49dceb40614ef8389ae7663228da1c1121decf4be21dc8e0f9a44fd788b3c76972
789f952433d7ccc0f09924359bd87b83b0bdcf55 fix gui: update labels if user signed an unsaved tx (edouardparis)
Pull request description:
It is a fix from #760
ACKs for top commit:
edouardparis:
Self-ACK 789f952433d7ccc0f09924359bd87b83b0bdcf55
Tree-SHA512: 3424577ba6d3215a3b7503a0861997ace0dbac0a82b0e43af5bc5fb5262450ab5074eaa54ce6a01021237b85dfb163d4eb436916d17f52b9dfd1a07cf7be269c
When a transaction has only one payment,
then its txid label is attached to the payment label.
While modifying the label of an output it did not change the
whole transaction label, which is a bug.
9a4b665fa1a471a7a6c7e0dea09065f9f01ec35d fix wallet name in settings during install (edouardparis)
Pull request description:
DEFAULT_WALLET_NAME aka simple Liana was still used during creation of wallet settings during install
this should have been noticed if I did test the wallet creation process in detail and if I removed the public exposition of the const variable. SHAME
ACKs for top commit:
edouardparis:
Self-ACK 9a4b665fa1a471a7a6c7e0dea09065f9f01ec35d
Tree-SHA512: 3afd98918a7ddcd10e702d0aed66a0645bea734fde61febe021f07c67f5b42a28018e1556e052f03e413db9c75686f121091a7ab871c7c3533d62b7202761625
