📝 fanquake opened a pull request: "ci: add libcpp hardening flags to macOS fuzz job"
(https://github.com/bitcoin/bitcoin/pull/33462)
Follows up to https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3323149107.
(https://github.com/bitcoin/bitcoin/pull/33462)
Follows up to https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3323149107.
💬 fanquake commented on pull request "ci: remove Clang build from msan fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3324313865)
> Unrelatedly(?), I wonder if it is possible to adjust the existing macos fuzz task to have stdlib hardening enabled.
Opened #33462 to have a look.
(https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3324313865)
> Unrelatedly(?), I wonder if it is possible to adjust the existing macos fuzz task to have stdlib hardening enabled.
Opened #33462 to have a look.
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3324325790)
@BitByBitByBitByBit
> I went through all the suggestions for Spanish and made the necessary edits. Thank you for this input! It is a very useful tool even though it requires some fine tuning. It would be helpful to take glossaries into account, as long as they are up-to-date for each language.
@ostruvek
> Hello, sorry for a delay on my side. I have reviewed the Czech suggestions and made the edits.
Thank you for your contributions! Both updated translations have been fetched from T
...
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3324325790)
@BitByBitByBitByBit
> I went through all the suggestions for Spanish and made the necessary edits. Thank you for this input! It is a very useful tool even though it requires some fine tuning. It would be helpful to take glossaries into account, as long as they are up-to-date for each language.
@ostruvek
> Hello, sorry for a delay on my side. I have reviewed the Czech suggestions and made the edits.
Thank you for your contributions! Both updated translations have been fetched from T
...
💬 fanquake commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3324342116)
> I'm not familiar enough with X to say why it's seemingly not possible to link all the xcb_utils statically, but this change seems sane to me.
Yea, maybe this is the direction we should go. Ideally we can land this change for 30, to remove the awkwardness here. I've updated the commit message and PR description.
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3324342116)
> I'm not familiar enough with X to say why it's seemingly not possible to link all the xcb_utils statically, but this change seems sane to me.
Yea, maybe this is the direction we should go. Ideally we can land this change for 30, to remove the awkwardness here. I've updated the commit message and PR description.
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3324395984)
reACK 5ac32aa441bf3ec1f1fa826f689405bc4a7b7354
just a rebase
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3324395984)
reACK 5ac32aa441bf3ec1f1fa826f689405bc4a7b7354
just a rebase
🤔 vasild reviewed a pull request: "net/rpc: Report inv information for debugging"
(https://github.com/bitcoin/bitcoin/pull/33448#pullrequestreview-3258413205)
ACK 2738b63e025d240618b3c72c28243c3e4d7d9c79
(https://github.com/bitcoin/bitcoin/pull/33448#pullrequestreview-3258413205)
ACK 2738b63e025d240618b3c72c28243c3e4d7d9c79
💬 vasild commented on pull request "net/rpc: Report inv information for debugging":
(https://github.com/bitcoin/bitcoin/pull/33448#discussion_r2372603900)
In the `else` branch, why set one of the new variables `stats.m_inv_to_send` to `0` but not the other new variable `stats.m_last_inv_seq`? Both have default value of `0`. I understand that if this code enters the `if` / `true` branch, it will never go to the `else` branch in subsequent calls. That is - `peer->GetTxRelay()` will not return `!= nullptr` once and later return `nullptr`. So, this means that there is no need to zero out stale values in the `else` branch, right? Then `stats.m_inv_to_s
...
(https://github.com/bitcoin/bitcoin/pull/33448#discussion_r2372603900)
In the `else` branch, why set one of the new variables `stats.m_inv_to_send` to `0` but not the other new variable `stats.m_last_inv_seq`? Both have default value of `0`. I understand that if this code enters the `if` / `true` branch, it will never go to the `else` branch in subsequent calls. That is - `peer->GetTxRelay()` will not return `!= nullptr` once and later return `nullptr`. So, this means that there is no need to zero out stale values in the `else` branch, right? Then `stats.m_inv_to_s
...
💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2372635323)
What about `AlreadyConnectedTo()` for the function and `peer` or `host_port` for the parameter?
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2372635323)
What about `AlreadyConnectedTo()` for the function and `peer` or `host_port` for the parameter?
💬 fanquake commented on pull request "ci: add libcpp hardening flags to macOS fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33462#issuecomment-3324447635)
Ah, this probably isn't doing anything, because we are using the oldest supported Apple Clang here. Will push something else.
(https://github.com/bitcoin/bitcoin/pull/33462#issuecomment-3324447635)
Ah, this probably isn't doing anything, because we are using the oldest supported Apple Clang here. Will push something else.
💬 jb55 commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#discussion_r2372668470)
ACK this ^
(https://github.com/bitcoin/bitcoin/pull/33453#discussion_r2372668470)
ACK this ^
👍 ryanofsky approved a pull request: "ci: Update Clang in "tidy" job"
(https://github.com/bitcoin/bitcoin/pull/33445#pullrequestreview-3258531271)
Code review ACK 1bb0f7f8b6d927046fefa2eb29d0132713415cca. Nice code cleanups, though did leave one comment about the NOLINT.
re: https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3315082319
> Could you please take a look at the following IPC-related warning:
Thanks, created https://github.com/capnproto/capnproto/pull/2417 to address it
(https://github.com/bitcoin/bitcoin/pull/33445#pullrequestreview-3258531271)
Code review ACK 1bb0f7f8b6d927046fefa2eb29d0132713415cca. Nice code cleanups, though did leave one comment about the NOLINT.
re: https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3315082319
> Could you please take a look at the following IPC-related warning:
Thanks, created https://github.com/capnproto/capnproto/pull/2417 to address it
💬 ryanofsky commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2372684495)
In commit "ci: Update Clang in "tidy" job" (1bb0f7f8b6d927046fefa2eb29d0132713415cca)
I'm confused about why these are necessary. What makes these two members different from the `fAnyoneCanPay` member and causes clang-tidy to warn about them but not `fAnyoneCanPay`? All three members seem to be set in the same way by the same `nHashTypeIn` constructor argument.
This seems more like a bug that should be reported upstream than a change that should be made to our code. And if the change is ne
...
(https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2372684495)
In commit "ci: Update Clang in "tidy" job" (1bb0f7f8b6d927046fefa2eb29d0132713415cca)
I'm confused about why these are necessary. What makes these two members different from the `fAnyoneCanPay` member and causes clang-tidy to warn about them but not `fAnyoneCanPay`? All three members seem to be set in the same way by the same `nHashTypeIn` constructor argument.
This seems more like a bug that should be reported upstream than a change that should be made to our code. And if the change is ne
...
💬 fanquake commented on pull request "CPack":
(https://github.com/bitcoin/bitcoin/pull/33455#discussion_r2372729201)
> CRCCheck force
Where are options like this being passed now?
(https://github.com/bitcoin/bitcoin/pull/33455#discussion_r2372729201)
> CRCCheck force
Where are options like this being passed now?
💬 maflcko commented on pull request "ci: add libcpp hardening flags to macOS fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33462#discussion_r2372747277)
hardening was added in xcode 16: https://developer.apple.com/documentation/xcode-release-notes/xcode-16-release-notes#C++-Standard-Library
```diff
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 52d21ef3ab..a0902e2700 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -145,8 +145,8 @@ jobs:
# Use the earliest Xcode supported by the version of macOS denoted in
# doc/release-notes-empty-template.md and providing at least t
...
(https://github.com/bitcoin/bitcoin/pull/33462#discussion_r2372747277)
hardening was added in xcode 16: https://developer.apple.com/documentation/xcode-release-notes/xcode-16-release-notes#C++-Standard-Library
```diff
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 52d21ef3ab..a0902e2700 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -145,8 +145,8 @@ jobs:
# Use the earliest Xcode supported by the version of macOS denoted in
# doc/release-notes-empty-template.md and providing at least t
...
✅ maflcko closed an issue: "ci: GHA fallback centos task runs out of space"
(https://github.com/bitcoin/bitcoin/issues/33293)
(https://github.com/bitcoin/bitcoin/issues/33293)
💬 RandyMcMillan commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-3324678979)
When displayed in the gui - a consistent finger print makes more sense to the user.

(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-3324678979)
When displayed in the gui - a consistent finger print makes more sense to the user.

💬 maflcko commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3324707093)
> Yes, I've seen GHA caching work, will try and find logs.
For reference, this still fails for me with `#4 ERROR: failed to parse error response 400: <h2>Our services aren't available right now</h2><p>We're working to restore all services as soon as possible. Please check back soon.</p>0eZvSaAAAAAClNmDP6CfYS7fcNJbDhpBqRE0yRURHRTA1MTYARWRnZQ==: invalid character '<' looking for beginning of value`
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3324707093)
> Yes, I've seen GHA caching work, will try and find logs.
For reference, this still fails for me with `#4 ERROR: failed to parse error response 400: <h2>Our services aren't available right now</h2><p>We're working to restore all services as soon as possible. Please check back soon.</p>0eZvSaAAAAAClNmDP6CfYS7fcNJbDhpBqRE0yRURHRTA1MTYARWRnZQ==: invalid character '<' looking for beginning of value`
💬 Sjors commented on pull request "ci: remove Clang build from msan fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33425#discussion_r2372889066)
Do I understand correctly that this also (positively) impacts the `ASan/TSan, depends` jobs?
(https://github.com/bitcoin/bitcoin/pull/33425#discussion_r2372889066)
Do I understand correctly that this also (positively) impacts the `ASan/TSan, depends` jobs?
💬 ismaelsadeeq commented on pull request "node: add `BlockTemplateCache`":
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3324812123)
Forced pushed from dca8200c71bb568ffe4821c68209e07a305db80d to 16d55585aa2d26e9177734927b8446faa72570e7 [5db80d..16d5558](https://github.com/bitcoin/bitcoin/compare/dca8200c71bb568ffe4821c68209e07a305db80d..16d55585aa2d26e9177734927b8446faa72570e7)
Main Changes are
1. Moved the `BlockTemplateCache` to `src/node/miner.cpp` to fix the circular dependency issue
2. The response now return the block template creation time
3. We search for match from right to left to return the fresh hit first
...
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3324812123)
Forced pushed from dca8200c71bb568ffe4821c68209e07a305db80d to 16d55585aa2d26e9177734927b8446faa72570e7 [5db80d..16d5558](https://github.com/bitcoin/bitcoin/compare/dca8200c71bb568ffe4821c68209e07a305db80d..16d55585aa2d26e9177734927b8446faa72570e7)
Main Changes are
1. Moved the `BlockTemplateCache` to `src/node/miner.cpp` to fix the circular dependency issue
2. The response now return the block template creation time
3. We search for match from right to left to return the fresh hit first
...
💬 l0rinc commented on pull request "test: Fix TestFlushBehavior when coin has non-zero dynamic memory usage":
(https://github.com/bitcoin/bitcoin/pull/33381#discussion_r2372927921)
I don't understand how this related to the PR. Is this really needed here or can it be split out to a different PR?
(https://github.com/bitcoin/bitcoin/pull/33381#discussion_r2372927921)
I don't understand how this related to the PR. Is this really needed here or can it be split out to a different PR?