💬 epiccurious commented on pull request "test: speedup bip324_cipher.py unit test":
(https://github.com/bitcoin/bitcoin/pull/29390#issuecomment-1929537124)
> a cleaner alternative would probably be to introduce a special seek/skip_packets method and not touch the encrypt/decrypt routines

Why did you choose this approach rather than adding a seek/skip_packet method?

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479783373)
Yes 👍🏾 its better that way.

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479796113)
in "wallet: add maxfeerate wallet startup option" https://github.com/bitcoin/bitcoin/commit/4165dc41289fd337e279f3986dfc1a84345c900e

After adding `maxfeerate` option we now only check if the fee rate user set is less `minrelaytxfee`, less than wallet min fee, or whether it exceed `maxfeerate`.
So their is no need to check for `maxtxfee` here, because its not enforced here.

Will add this info in commit message, thanks

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479796527)
Yes 👍🏾

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1479814983)
Because after https://github.com/bitcoin/bitcoin/commit/eadc3a4500d6b7be263a2981602c20f6e5eb13a4 we now check for `maxfeerate` threshold first before checking `maxtxfee` threshold.
And the specified fee rate is above default `maxferate`, as such it will be hit first.

But I agree, I will add another test where specified bump fee rate is below the `maxfeerate` but we hit `maxtxfee` due to size or maybe just restart with `maxtxfee` set to a lower value ?

⚠️ glozow opened an issue: "wallet_reorgrestore.py failed"
(https://github.com/bitcoin/bitcoin/issues/29392)
### Motivation

https://cirrus-ci.com/task/5616118744743936
```
test 2024-02-06T11:27:14.369000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "/ci_contai
...

🤔 murchandamus reviewed a pull request: "wallet: Add CoinGrinder coin selection algorithm"
(https://github.com/bitcoin/bitcoin/pull/27877#pullrequestreview-1862978564)
Responded to feedback from @sipa and @sr-gi

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1478391942)
Rephrased to mention again all three possible state transitions with example

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1479832383)
Yes I think that would be equivalent.

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1478374197)
Added!

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1478362138)
Thanks, I’ve improved that sentence.

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1479813643)
@sr-gi: I did not count the root node, since we do not add a UTXO in the root node, so I did not count it among the inclusion branch nodes.

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1478371817)
Thanks, good catch

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1478375389)
Added

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1478399611)
I had also set it for BnB originally, but then I got the complaint that it wasn't being read anywhere yet. I’ll add it when I update BnB accordingly.

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1478371586)
Sure, added the mention of that.

💬 delta1 commented on pull request "test: speedup bip324_cipher.py unit test":
(https://github.com/bitcoin/bitcoin/pull/29390#issuecomment-1929792710)
> Why did you choose this approach rather than adding a seek/skip_packet method?

@epiccurious he explained in the very next line:

> that seemed overkill to me only for speeding up a unit test. Open for suggestions

💬 sr-gi commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1479906114)
Fair

💬 BrandonOdiwuor commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1929818726)
@maflcko this is because [`self.options.descriptors`](https://github.com/bitcoin/bitcoin/blob/4de84557d6d1f53255ab19f27c8b6ea0a712934a/test/functional/test_framework/test_node.py#L111-L112) == None in `TestNode(... , descriptors=self.options.descriptors, ...)` hence the `disable_wallet` flag is added when the TestNode is instantiated disabling the wallet RPCs just before the first [`start_node()`](https://github.com/bitcoin/bitcoin/blob/4de84557d6d1f53255ab19f27c8b6ea0a712934a/test/functional/te
...

💬 epiccurious commented on pull request "test: speedup bip324_cipher.py unit test":
(https://github.com/bitcoin/bitcoin/pull/29390#issuecomment-1929869880)
Tested ACK a8c3454ba137dfac20b3c89bc558374de0524114.

Before (commit 9eeee7caa3f95ee17a645e12d330261f8e3c2dbf):
```
Ran 2 tests in 15.541s
```

After:
```
Ran 2 tests in 0.204s
```