💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446434963)
changed wording of commit

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446435012)
replaced with diagram check; should be fine now since it must be strictly superior.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446435046)
fixed

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446435113)
rephrased into the two cases

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446435138)
added

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446435204)
reworded

💬 achow101 commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#issuecomment-1883547121)
New compiler warning, using gcc 13.2.1

```
test/fuzz/connman.cpp: In function ‘void connman_fuzz_target(FuzzBufferType)’:
test/fuzz/connman.cpp:43:17: error: missing initializer for member ‘CConnman::Options::vSeedNodes’ [-Werror=missing-field-initializers]
43 | connman.Init({ .nMaxOutboundLimit = max_outbound_limit });
| ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test/fuzz/connman.cpp:43:17: error: missing initializer for member ‘CConnman::Options::vWhite
...

💬 brunoerg commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#issuecomment-1883552043)
> New compiler warning, using gcc 13.2.1

I'm checking it atm.

📝 Kaikookpk12 opened a pull request: "Kaikookpk12 patch 1"
(https://github.com/bitcoin/bitcoin/pull/29210)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ fanquake closed a pull request: "Kaikookpk12 patch 1"
(https://github.com/bitcoin/bitcoin/pull/29210)

📝 fanquake locked a pull request: "Kaikookpk12 patch 1"
(https://github.com/bitcoin/bitcoin/pull/29210)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 achow101 commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1446448724)
I think UniValue type interpretation would be fine.

📝 brunoerg opened a pull request: "fuzz: fix `connman` initialization"
(https://github.com/bitcoin/bitcoin/pull/29211)
Fixes https://github.com/bitcoin/bitcoin/pull/29172#issuecomment-1883547121

💬 brunoerg commented on pull request "fuzz: fix `connman` initialization":
(https://github.com/bitcoin/bitcoin/pull/29211#issuecomment-1883603293)
cc: @achow101

💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1883611431)
https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1855815358 exists, still, as well.

💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1883614535)
[clusterfuzz-testcase-mocked_descriptor_parse-5224675118481408.bin.not.txt](https://github.com/bitcoin/bitcoin/files/13877884/clusterfuzz-testcase-mocked_descriptor_parse-5224675118481408.bin.not.txt)

💬 jamesob commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1446491162)
> Given that LogInfo is unconditional, what would be your use case for adding a log category?

Simply that "info = unconditional" might not always be the case (and is an unusual convention to have hardcoded), and a user could rightly want to see only WARNING+ logs as well as, say, `net` or validation-related INFOs.

💬 ryanofsky commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1445260009)
In commit "desc spkm: Add functions to retrieve specific private keys" (ebd02d73241c0922f486cdf14719281c079b45c7)

Maybe add a comment saying it returns nullopt if the key doesn't exist, or can't be decrypted because the wallet is locked, or because there's decryption error. Otherwise it's not clear what the function is assuming or when it returns nullopt.

💬 ryanofsky commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1445262239)
In commit "wallet, rpc: Add gethdkeys RPC" (cbe990b1a95e6331106c3ce6d1f2abd860f88ea8)

I think this example is not right anymore since the parameter needs to be named. There seems to be a HelpExampleCliNamed function for this.

👍 ryanofsky approved a pull request: "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors"
(https://github.com/bitcoin/bitcoin/pull/29130#pullrequestreview-1809920961)
Code review ACK c10150b6150083440af4f0aa1110c8aa99ba2dc8. Left a lot of suggestions, but none are important. Overall this looks great and is very cleanly implemented.