🤔 jonatack reviewed a pull request: "doc: unify `datacarriersize` warning with release notes"
(https://github.com/bitcoin/bitcoin/pull/33224#pullrequestreview-3137349429)
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
  (https://github.com/bitcoin/bitcoin/pull/33224#pullrequestreview-3137349429)
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
💬 why9201110859430801-arc commented on something "":
(https://github.com/bitcoin/bitcoin/commit/e44d72b6480acb356f0a4793a76a5e2bc4e4118d#commitcomment-164320743)
Welcome to the work for run our all goals and next change with Bitcoin token"z or Z'token Bitcoin u understand this name is not me control follow sky can see company what u do ....please
  (https://github.com/bitcoin/bitcoin/commit/e44d72b6480acb356f0a4793a76a5e2bc4e4118d#commitcomment-164320743)
Welcome to the work for run our all goals and next change with Bitcoin token"z or Z'token Bitcoin u understand this name is not me control follow sky can see company what u do ....please
💬 purpleKarrot commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2288681559)
I think the deep nesting can be avoided with the approach mentioned in https://habla.news/u/purplekarrot.net/cmake-import-export
Also, I don't think the `#else` branch is needed. To my knowledge, there is no platform that supports C++20 but neither dllexport nor visibility.
  (https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2288681559)
I think the deep nesting can be avoided with the approach mentioned in https://habla.news/u/purplekarrot.net/cmake-import-export
Also, I don't think the `#else` branch is needed. To my knowledge, there is no platform that supports C++20 but neither dllexport nor visibility.
🚀 fanquake merged a pull request: "Release: Prepare "Translation string freeze" step"
(https://github.com/bitcoin/bitcoin/pull/33193)
  (https://github.com/bitcoin/bitcoin/pull/33193)
💬 instagibbs commented on pull request "[29.x] 33106 backport and final changes for rc2":
(https://github.com/bitcoin/bitcoin/pull/33226#issuecomment-3207112148)
@1ma
> What is the criteria for backporting PRs to point releases? Sub 1 s/vB standardization is certainly not a bugfix.
Any moderate or severe performance degradation is worth considering for backport, if it can be done cleanly, and has been done many times. Block propagation slowdowns that are avoidable are withing scope. Updating minor versions is much easier than main for production environments.
having not reviewed the backport effort here, concept ack
  (https://github.com/bitcoin/bitcoin/pull/33226#issuecomment-3207112148)
@1ma
> What is the criteria for backporting PRs to point releases? Sub 1 s/vB standardization is certainly not a bugfix.
Any moderate or severe performance degradation is worth considering for backport, if it can be done cleanly, and has been done many times. Block propagation slowdowns that are avoidable are withing scope. Updating minor versions is much easier than main for production environments.
having not reviewed the backport effort here, concept ack
🤔 danielabrozzoni reviewed a pull request: "headerssync: Preempt unrealistic unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3137421954)
Code Review ACK 53341ea10dc2f7df371b416060863bbc094b8773
I reviewed the code and checked that tests were passing locally, but didn't do any extensive testing.
This PR makes the `HEADER_COMMITMENT_PERIOD` and `REDOWNLOAD_BUFFER_SIZE` configurable by the tests, instead of using an hardcoded value, to avoid ending up testing an unrealistic behavior once `REDOWNLOAD_BUFFER_SIZE` surpasses 15_000 in 0.30.
(I like how we can test what could happen using the first commit and investigating the
...
  (https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3137421954)
Code Review ACK 53341ea10dc2f7df371b416060863bbc094b8773
I reviewed the code and checked that tests were passing locally, but didn't do any extensive testing.
This PR makes the `HEADER_COMMITMENT_PERIOD` and `REDOWNLOAD_BUFFER_SIZE` configurable by the tests, instead of using an hardcoded value, to avoid ending up testing an unrealistic behavior once `REDOWNLOAD_BUFFER_SIZE` surpasses 15_000 in 0.30.
(I like how we can test what could happen using the first commit and investigating the
...
💬 luke-jr commented on pull request "Fix compatibility with `-debuglogfile` command-line option":
(https://github.com/bitcoin/bitcoin/pull/33215#discussion_r2288730023)
```suggestion
RPCResult::Type::BOOL, "", "Verification finished successfully. If false, check debug log for reason."},
```
I don't think it makes sense to include the real filename in RPC results.
  (https://github.com/bitcoin/bitcoin/pull/33215#discussion_r2288730023)
```suggestion
RPCResult::Type::BOOL, "", "Verification finished successfully. If false, check debug log for reason."},
```
I don't think it makes sense to include the real filename in RPC results.
💬 luke-jr commented on pull request "Fix compatibility with `-debuglogfile` command-line option":
(https://github.com/bitcoin/bitcoin/pull/33215#discussion_r2288731766)
```suggestion
throw JSONRPCError(RPC_MISC_ERROR, "Unable to import mempool file, see debug log for details.");
```
  (https://github.com/bitcoin/bitcoin/pull/33215#discussion_r2288731766)
```suggestion
throw JSONRPCError(RPC_MISC_ERROR, "Unable to import mempool file, see debug log for details.");
```
💬 luke-jr commented on pull request "Fix compatibility with `-debuglogfile` command-line option":
(https://github.com/bitcoin/bitcoin/pull/33215#discussion_r2288703479)
```suggestion
InitError(strprintf(_("A fatal internal error occurred, see %s for details: %s"), fs::PathToString(LogInstance().m_file_path.filename()), message));
```
  (https://github.com/bitcoin/bitcoin/pull/33215#discussion_r2288703479)
```suggestion
InitError(strprintf(_("A fatal internal error occurred, see %s for details: %s"), fs::PathToString(LogInstance().m_file_path.filename()), message));
```
💬 luke-jr commented on pull request "Fix compatibility with `-debuglogfile` command-line option":
(https://github.com/bitcoin/bitcoin/pull/33215#discussion_r2288702414)
```suggestion
tfm::format(std::cerr, "Error during initialization - check %s for details\n", fs::PathToString(LogInstance().m_file_path.filename()));
```
  (https://github.com/bitcoin/bitcoin/pull/33215#discussion_r2288702414)
```suggestion
tfm::format(std::cerr, "Error during initialization - check %s for details\n", fs::PathToString(LogInstance().m_file_path.filename()));
```
🚀 fanquake merged a pull request: "test: use local `CBlockIndex` in block read hash mismatch check"
(https://github.com/bitcoin/bitcoin/pull/33154)
  (https://github.com/bitcoin/bitcoin/pull/33154)
👋 fanquake's pull request is ready for review: "[29.x] 33106 backport and final changes for rc2"
(https://github.com/bitcoin/bitcoin/pull/33226)
  (https://github.com/bitcoin/bitcoin/pull/33226)
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2288751458)
Yes, will clean this up properly on the next push.
  (https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2288751458)
Yes, will clean this up properly on the next push.
👋 polespinasa's pull request is ready for review: "doc: truc packages allow sub min feerate transactions"
(https://github.com/bitcoin/bitcoin/pull/33220)
  (https://github.com/bitcoin/bitcoin/pull/33220)
💬 achow101 commented on pull request "kernel: improve BlockChecked ownership semantics":
(https://github.com/bitcoin/bitcoin/pull/33078#issuecomment-3207402503)
ACK 1d9f1cb4bd6b119e1d56cbdd7f6ce4d4521fffa3
  (https://github.com/bitcoin/bitcoin/pull/33078#issuecomment-3207402503)
ACK 1d9f1cb4bd6b119e1d56cbdd7f6ce4d4521fffa3
💬 janb84 commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3207403645)
> re: [#31802 (comment)](https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206751392)
>
> > They'll encounter the old `bitcoind` binary, a new `bitcoin` binary, and a new `bitcoin-node` binary in a weird directory that seems to do the same thing as `bitcoind` if they try to run it.
>
> This also seems like actionable feedback. Note there is also a `test_bitcoin` binary in the `libexec/` directory and a [readme](https://github.com/bitcoin/bitcoin/blob/master/doc/README.md) file in
...
  (https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3207403645)
> re: [#31802 (comment)](https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206751392)
>
> > They'll encounter the old `bitcoind` binary, a new `bitcoin` binary, and a new `bitcoin-node` binary in a weird directory that seems to do the same thing as `bitcoind` if they try to run it.
>
> This also seems like actionable feedback. Note there is also a `test_bitcoin` binary in the `libexec/` directory and a [readme](https://github.com/bitcoin/bitcoin/blob/master/doc/README.md) file in
...
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3207416840)
@janb84 you can see for yourself with both `(sudo) cmake --install build` and a guix build that the `bitcoin-node` and `bitcoin-gui` binaries go into `libexec`. They're only in `build/bin` (not in PATH) when you build from source without installing.
  (https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3207416840)
@janb84 you can see for yourself with both `(sudo) cmake --install build` and a guix build that the `bitcoin-node` and `bitcoin-gui` binaries go into `libexec`. They're only in `build/bin` (not in PATH) when you build from source without installing.
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3207439196)
> It's not the `libexec` directory that's the issue , it's the `bin` directory ( packaged ), or will the v30 packaged also contain a libexec directory?
Yep, the v30 package will contain a `libexec/` directory alongside the `bin/` directory. This was implemented in #31679. In binary releases following that PR, the `libexec/` directory just contains a `test_bitcoin` file. (You can see the complete file list in https://github.com/bitcoin/bitcoin/blob/master/doc/files.md#installed-files.)
This
...
  (https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3207439196)
> It's not the `libexec` directory that's the issue , it's the `bin` directory ( packaged ), or will the v30 packaged also contain a libexec directory?
Yep, the v30 package will contain a `libexec/` directory alongside the `bin/` directory. This was implemented in #31679. In binary releases following that PR, the `libexec/` directory just contains a `test_bitcoin` file. (You can see the complete file list in https://github.com/bitcoin/bitcoin/blob/master/doc/files.md#installed-files.)
This
...
🚀 achow101 merged a pull request: "kernel: improve BlockChecked ownership semantics"
(https://github.com/bitcoin/bitcoin/pull/33078)
  (https://github.com/bitcoin/bitcoin/pull/33078)
🤔 jlest01 reviewed a pull request: "wallet: Remove isminetypes"
(https://github.com/bitcoin/bitcoin/pull/32523#pullrequestreview-3137706983)
reACK https://github.com/bitcoin/bitcoin/pull/32523/commits/be776a1443fdf1a72e0d363c1566d71cb0cda8b5
  (https://github.com/bitcoin/bitcoin/pull/32523#pullrequestreview-3137706983)
reACK https://github.com/bitcoin/bitcoin/pull/32523/commits/be776a1443fdf1a72e0d363c1566d71cb0cda8b5