💬 marcofleon commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1843774945)
As is, the test doesn't hit that part in `pow.cpp`. But I'll look to improve this test and its coverage in a future PR. Still thinking about the best way to do it. For now, I'm going leave it as is and just be sure that the crash is fixed.

👍 ryanofsky approved a pull request: "refactor: Avoid std::string format strings"
(https://github.com/bitcoin/bitcoin/pull/31287#pullrequestreview-2438629037)
Code review ACK fa1177e3d7ca9ef003ded4d0c915fa6dc22bd37d

I think I'll merge this PR shortly so we can make progress on related PRs #31061 and #31072, but you can let me know if you would prefer this _not_ to be merged now in case you want to address the [comment](https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1842156687) that using `strprintf("%s %s", ...)` instead of `+` makes code more confusing

💬 ryanofsky commented on pull request "refactor: Avoid std::string format strings":
(https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1843809721)
In commit "refactor: Avoid std::string format strings" (fa1177e3d7ca9ef003ded4d0c915fa6dc22bd37d)

Note: Precedes this PR, but there appears to be mixed translation bugs here and below. GetDisplayName() can return the english string "[default wallet]" while "Rescanning" is translated. This might become a non-issue after the legacy wallet removal where (I'm assuming) unnamed wallets will finally get names? I think having more type safety to make it harder to accidentally combine translated and
...

💬 ryanofsky commented on pull request "refactor: Avoid std::string format strings":
(https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1843821603)
re: https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1842156687

I think this confusion only backs up the original comment that:

```c++
ShowProgress(GetDisplayName() + " " + _("Rescanning…").translated, 0);
```

is clearer than:

```c++
ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…").translated), 0);
```

💬 TheCharlatan commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2479011646)
Guix build:
```
606d0909c4591fc7dac3759e230e7bd3de00555c1c535d437ca8bc19df85fc70 guix-build-ee1128ead846/output/aarch64-linux-gnu/SHA256SUMS.part
4aca1c476b6824d485044c6636ce2ecf45f542a89bc501493f4868cf16c13d1f guix-build-ee1128ead846/output/aarch64-linux-gnu/bitcoin-ee1128ead846-aarch64-linux-gnu-debug.tar.gz
46d17a50226b60af12124b9c2b70cdab1c257a2034463999c0340d8a8b573cdd guix-build-ee1128ead846/output/aarch64-linux-gnu/bitcoin-ee1128ead846-aarch64-linux-gnu.tar.gz
d82c381c8d4e888ea552
...

💬 maflcko commented on pull request "refactor: Avoid std::string format strings":
(https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1843917615)
Maybe a follow-up can fix this, so that this remains a refactor?

💬 maflcko commented on pull request "refactor: Avoid std::string format strings":
(https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1843918244)
I'll leave as-is for now. I think both are fine.

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1843921416)
touched up

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1843921461)
done

🚀 ryanofsky merged a pull request: "refactor: Avoid std::string format strings"
(https://github.com/bitcoin/bitcoin/pull/31287)

💬 luke-jr commented on pull request "test: autogenerate bash completion":
(https://github.com/bitcoin/bitcoin/pull/30860#discussion_r1844079617)
```suggestion
if self.options.completion is None or len(self.options.completion) == 0:
```

📝 hodlinator converted_to_draft a pull request: "util: Improve documentation and negation of args"
(https://github.com/bitcoin/bitcoin/pull/31212)
- Document `-color` as only applying to `-getinfo`, to be less confusing for bitcoin-cli users.
- No longer print version information when getting passed `-noversion`.
- Disallow `-nodatadir` as we cannot run without one. It was previously interpreted as a mix of unset and as a relative path of "0".
- Support `-norpccookiefile`
- Support `-nopid`
- Properly support `-noconf` (instead of working by accident). Also detect when directories are specified instead of files.

Prompted by investi
...

🤔 instagibbs reviewed a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2438909837)
feature seems to make sense, mostly reviewed tests to get myself familiar with the interface

💬 instagibbs commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1844113097)
note: this is also a key test of multiple blockhashes

💬 instagibbs commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1844010301)
should also test invalid descriptors

💬 instagibbs commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1844122547)
would be great if there was a case showing flipping descriptor ordering doesn't change result ordering

💬 instagibbs commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1844118607)
testing that this is results in the call getting rejected even if it's f.e. the second blockhash in a list is also a good idea

💬 instagibbs commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1844117146)
should also test that repeated blockhashes is acceptable(AFAICT it just repeats the event)

💬 instagibbs commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1844115700)
block order matters, run this case twice, once with blocks in reverse history order?

💬 instagibbs commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1844043247)
I think the lift for downstream wallets to encode spk to address is trivial, so I'd rather no expose "send from" type addresses, but I don't feel super strongly about it.

If not, making the address field an optional return as @tdb3 says is my ask. Makes it a little more clear conceptually what's being exposed.

regardless of the result here, there should be test coverage for an output with no address type and the delta from the other test case `test_multiple_addresses` kind of demonstrates
...