💬 stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2224838548)
> It's also consistent with most of the other unit tests under `src/wallet/test`
Fair enough, I wasn't aware.
> and reduces the chance of symbol collisions
I think all of this stuff is meant to have internal linkage, so if reducing symbol collisions is the goal (which seems sensible) an anonymous namespace might make more sense? Potentially with a `using namespace wallet`, even though at the moment we're only using one `wallet` symbol.
Anyway, not important, and following convention
...
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2224838548)
> It's also consistent with most of the other unit tests under `src/wallet/test`
Fair enough, I wasn't aware.
> and reduces the chance of symbol collisions
I think all of this stuff is meant to have internal linkage, so if reducing symbol collisions is the goal (which seems sensible) an anonymous namespace might make more sense? Potentially with a `using namespace wallet`, even though at the moment we're only using one `wallet` symbol.
Anyway, not important, and following convention
...
✅ frankomosh closed a pull request: "fuzz: add mempool_dag fuzzer for transaction dependency testing"
(https://github.com/bitcoin/bitcoin/pull/33038)
(https://github.com/bitcoin/bitcoin/pull/33038)
💬 frankomosh commented on pull request "fuzz: add mempool_dag fuzzer for transaction dependency testing":
(https://github.com/bitcoin/bitcoin/pull/33038#issuecomment-3106595570)
> Historically LLM generated pull requests in this repo were easy to dismiss, because they were obviously wrong on the surface level and usually the tests and CI were failing in similarly obvious ways. However, it now seems that non-trivial LLM generated pull request in this repo looked reasonable on a first glance (compiles and passes CI), but they still miss the point (adding tests without increasing test coverage, or adding features that are useless and counter-productive [#32949 (comment)](h
...
(https://github.com/bitcoin/bitcoin/pull/33038#issuecomment-3106595570)
> Historically LLM generated pull requests in this repo were easy to dismiss, because they were obviously wrong on the surface level and usually the tests and CI were failing in similarly obvious ways. However, it now seems that non-trivial LLM generated pull request in this repo looked reasonable on a first glance (compiles and passes CI), but they still miss the point (adding tests without increasing test coverage, or adding features that are useless and counter-productive [#32949 (comment)](h
...
✅ fanquake closed an issue: "Enable PCP by default?"
(https://github.com/bitcoin/bitcoin/issues/31663)
(https://github.com/bitcoin/bitcoin/issues/31663)
🚀 fanquake merged a pull request: "Enable `-natpmp` by default"
(https://github.com/bitcoin/bitcoin/pull/33004)
(https://github.com/bitcoin/bitcoin/pull/33004)
💬 Zeegaths commented on pull request "docs: adds correct updated documentation links":
(https://github.com/bitcoin/bitcoin/pull/32699#issuecomment-3106720428)
Fixed!
On Tue, 22 Jul 2025 at 23:44, Winnie Gitau ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/init.cpp
> <https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2223787842>:
>
> > @@ -438,6 +438,18 @@ static void registerSignalHandler(int signal, void(*handler)(int))
> }
> #endif
>
> +std::string GetDocumentationUrl(const std::string& doc_path)
> +{
> + if (CLIENT_VERSION_MINOR == 99) {
> + std::
...
(https://github.com/bitcoin/bitcoin/pull/32699#issuecomment-3106720428)
Fixed!
On Tue, 22 Jul 2025 at 23:44, Winnie Gitau ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/init.cpp
> <https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2223787842>:
>
> > @@ -438,6 +438,18 @@ static void registerSignalHandler(int signal, void(*handler)(int))
> }
> #endif
>
> +std::string GetDocumentationUrl(const std::string& doc_path)
> +{
> + if (CLIENT_VERSION_MINOR == 99) {
> + std::
...
🚀 fanquake merged a pull request: "doc: Add release notes for 32521 (MAX_TX_LEGACY_SIGOPS)"
(https://github.com/bitcoin/bitcoin/pull/33037)
(https://github.com/bitcoin/bitcoin/pull/33037)
💬 fanquake commented on pull request "[29.x] Backport #32521":
(https://github.com/bitcoin/bitcoin/pull/33013#issuecomment-3106781944)
Can you squash the last two commits into `doc: update release notes for 29.x`? Although, I think this change should have it's full release note displayed (#33037). i.e something like this: https://github.com/fanquake/bitcoin/commit/fc2e2ef1709d553a43e141db216a7f1959e1bb4c.
(https://github.com/bitcoin/bitcoin/pull/33013#issuecomment-3106781944)
Can you squash the last two commits into `doc: update release notes for 29.x`? Although, I think this change should have it's full release note displayed (#33037). i.e something like this: https://github.com/fanquake/bitcoin/commit/fc2e2ef1709d553a43e141db216a7f1959e1bb4c.
💬 fanquake commented on issue "intermittent timeout in wallet_signer.py : sendall timed out":
(https://github.com/bitcoin/bitcoin/issues/33015#issuecomment-3106810453)
https://cirrus-ci.com/task/6232770030075904?logs=ci#L1711
(https://github.com/bitcoin/bitcoin/issues/33015#issuecomment-3106810453)
https://cirrus-ci.com/task/6232770030075904?logs=ci#L1711
💬 maflcko commented on pull request "doc: update headers and TOC in `developer-notes.md`":
(https://github.com/bitcoin/bitcoin/pull/33040#issuecomment-3106851769)
lgtm ACK f314f01fb833a873c8c75c0d8a21023cf733de9a
(https://github.com/bitcoin/bitcoin/pull/33040#issuecomment-3106851769)
lgtm ACK f314f01fb833a873c8c75c0d8a21023cf733de9a
💬 fanquake commented on pull request "Fix BaseIndex::Commit false error":
(https://github.com/bitcoin/bitcoin/pull/26903#issuecomment-3106911198)
Removed "Up for Grabs", as this looks like it was fixed in #29671.
(https://github.com/bitcoin/bitcoin/pull/26903#issuecomment-3106911198)
Removed "Up for Grabs", as this looks like it was fixed in #29671.
💬 fanquake commented on pull request "build: deprecate UPnP support":
(https://github.com/bitcoin/bitcoin/pull/22644#issuecomment-3106927971)
No longer "Up for grabs". Replaced by #31130.
(https://github.com/bitcoin/bitcoin/pull/22644#issuecomment-3106927971)
No longer "Up for grabs". Replaced by #31130.
💬 fanquake commented on pull request "build: add data.h dependency to raw files":
(https://github.com/bitcoin/bitcoin/pull/18265#issuecomment-3106934537)
Removing "Up for Grabs" post-CMake.
(https://github.com/bitcoin/bitcoin/pull/18265#issuecomment-3106934537)
Removing "Up for Grabs" post-CMake.
💬 fanquake commented on pull request "gui: grey out used address in address book":
(https://github.com/bitcoin/bitcoin/pull/17355#issuecomment-3106938955)
Removing "Up for grabs" here. Followup could start in https://github.com/bitcoin-core/gui/issues/528, or the QML repo.
(https://github.com/bitcoin/bitcoin/pull/17355#issuecomment-3106938955)
Removing "Up for grabs" here. Followup could start in https://github.com/bitcoin-core/gui/issues/528, or the QML repo.
💬 fanquake commented on pull request "travis: Run functional tests in GUI once":
(https://github.com/bitcoin/bitcoin/pull/16522#issuecomment-3106946710)
@maflcko Is this still "Up for grabs"? Could turn into a new issue for visibility, if it's something you still think we should do.
(https://github.com/bitcoin/bitcoin/pull/16522#issuecomment-3106946710)
@maflcko Is this still "Up for grabs"? Could turn into a new issue for visibility, if it's something you still think we should do.
💬 fanquake commented on pull request "[devtools translations] catch invalid specifiers":
(https://github.com/bitcoin/bitcoin/pull/13472#issuecomment-3106950746)
Removing "Up for grabs". This tooling now exists in this repo: https://github.com/bitcoin-core/bitcoin-maintainer-tools.
(https://github.com/bitcoin/bitcoin/pull/13472#issuecomment-3106950746)
Removing "Up for grabs". This tooling now exists in this repo: https://github.com/bitcoin-core/bitcoin-maintainer-tools.
💬 fanquake commented on pull request "RPC: Strict JSON-RPC 2.0 compliance (gated behind flag)":
(https://github.com/bitcoin/bitcoin/pull/12435#issuecomment-3106956665)
Removing "Up for grabs" as JSON 2.0 was done in #27101. cc @pinheadmz
(https://github.com/bitcoin/bitcoin/pull/12435#issuecomment-3106956665)
Removing "Up for grabs" as JSON 2.0 was done in #27101. cc @pinheadmz
💬 fanquake commented on pull request "WIP: switch to libevent for node socket handling":
(https://github.com/bitcoin/bitcoin/pull/11227#issuecomment-3106974564)
Removing "Up for grabs", given we are now in the process of trying to remove libevent.
(https://github.com/bitcoin/bitcoin/pull/11227#issuecomment-3106974564)
Removing "Up for grabs", given we are now in the process of trying to remove libevent.
💬 fanquake commented on pull request "Package-aware fee estimation":
(https://github.com/bitcoin/bitcoin/pull/23074#issuecomment-3106983608)
Removing "Up for grabs" given #30079. cc @ismaelsadeeq.
(https://github.com/bitcoin/bitcoin/pull/23074#issuecomment-3106983608)
Removing "Up for grabs" given #30079. cc @ismaelsadeeq.
💬 fanquake commented on pull request "build: Only use `@` prefix for `echo` command in Makefiles":
(https://github.com/bitcoin/bitcoin/pull/19480#issuecomment-3107193070)
Removed "Up for grabs" given the switch to CMake.
(https://github.com/bitcoin/bitcoin/pull/19480#issuecomment-3107193070)
Removed "Up for grabs" given the switch to CMake.
💬 yuvicc commented on pull request "doc: update headers and TOC in `developer-notes.md`":
(https://github.com/bitcoin/bitcoin/pull/33040#issuecomment-3107360337)
ACK f314f01fb833a873c8c75c0d8a21023cf733de9a
Thanks!
(https://github.com/bitcoin/bitcoin/pull/33040#issuecomment-3107360337)
ACK f314f01fb833a873c8c75c0d8a21023cf733de9a
Thanks!