💬 ajtowns commented on pull request "net: Waste less time in socket handling":
(https://github.com/bitcoin/bitcoin/pull/34025#issuecomment-3621332052)
Not sure if the flame graph is usable, but:

GetBoolArg takes up 0.31% of total time, as part of PushMessage that takes up 1.75% of total time, in b-msghand.
GetTime takes up 0.82% of total time, as part of InactivityCheck that takes up 1.78% of total time, in b-net.
Converting from std::chrono::microseconds to NodeClock::time_point is a lot more intrusive (impacting at least net_processing and no
...
(https://github.com/bitcoin/bitcoin/pull/34025#issuecomment-3621332052)
Not sure if the flame graph is usable, but:

GetBoolArg takes up 0.31% of total time, as part of PushMessage that takes up 1.75% of total time, in b-msghand.
GetTime takes up 0.82% of total time, as part of InactivityCheck that takes up 1.78% of total time, in b-net.
Converting from std::chrono::microseconds to NodeClock::time_point is a lot more intrusive (impacting at least net_processing and no
...
💬 sedited commented on pull request "net: Waste less time in socket handling":
(https://github.com/bitcoin/bitcoin/pull/34025#issuecomment-3621358666)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/34025#issuecomment-3621358666)
Concept ACK
💬 ajtowns commented on pull request "test: p2p: check that peer's announced starting height is remembered":
(https://github.com/bitcoin/bitcoin/pull/33990#issuecomment-3621363780)
Seems fine to test it if we have code to export it over RPC.
I don't think I've ever seen it be useful for debugging anything, and just knowing the height without knowing if it's the same header you have for that height or what the chainwork is doesn't seem very useful. So dropping `peer->m_starting_height` entirely (and only reporting it in the `receive version message` debug log entry) might be better?
(https://github.com/bitcoin/bitcoin/pull/33990#issuecomment-3621363780)
Seems fine to test it if we have code to export it over RPC.
I don't think I've ever seen it be useful for debugging anything, and just knowing the height without knowing if it's the same header you have for that height or what the chainwork is doesn't seem very useful. So dropping `peer->m_starting_height` entirely (and only reporting it in the `receive version message` debug log entry) might be better?
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2595703803)
This is tested now by having zero worker threads.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2595703803)
This is tested now by having zero worker threads.
💬 yuvicc commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#issuecomment-3621481762)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/34022#issuecomment-3621481762)
Concept ACK
💬 Ataraxia009 commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#issuecomment-3621582615)
Personally I prefer this approach over https://github.com/bitcoin/bitcoin/pull/34018 @0xB10C @stickies-v
The only problem I see with this approach is the code duplication. Which I think you solved right? Using:
```
const std::string details = std::format(
"transport={} version={} blocks={} peer={}{}{}",
TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
pfrom.nVersion.load(),
int{peer->m_starting_height},
pfrom.GetId(),
(fLogIPs ? std::fo
...
(https://github.com/bitcoin/bitcoin/pull/34008#issuecomment-3621582615)
Personally I prefer this approach over https://github.com/bitcoin/bitcoin/pull/34018 @0xB10C @stickies-v
The only problem I see with this approach is the code duplication. Which I think you solved right? Using:
```
const std::string details = std::format(
"transport={} version={} blocks={} peer={}{}{}",
TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
pfrom.nVersion.load(),
int{peer->m_starting_height},
pfrom.GetId(),
(fLogIPs ? std::fo
...
💬 Ataraxia009 commented on pull request "log: exempt all category-specific logs from ratelimiting when running with debug":
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3621587297)
Concept NAck
Users that are having issues that are not power users would also use the `-debug` option, to help submit logs.
if `-debug` is used with this implementation, it would stop rate-limiting of all category logs.
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3621587297)
Concept NAck
Users that are having issues that are not power users would also use the `-debug` option, to help submit logs.
if `-debug` is used with this implementation, it would stop rate-limiting of all category logs.
💬 Ataraxia009 commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2596019236)
If you are going to do this, then add a comment here saying `ProcessNewBlockHeaders` assumes that the caller has checked for min pow or something along those lines, for clarity
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2596019236)
If you are going to do this, then add a comment here saying `ProcessNewBlockHeaders` assumes that the caller has checked for min pow or something along those lines, for clarity
💬 laanwj commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3621690386)
FWIW: My bisect is not done yet (2 steps to go, it will likely finish today), but in the current commit range https://github.com/gcc-mirror/gcc/commit/1d82fc2e6824bf83159389729c31a942f7b91b04 `optimize std::vector::push_back` is the most suspicious as it affects vector appending. It's a change to `libstdc++-v3`, not the compiler, so it doesn't directly point at a root cause.
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3621690386)
FWIW: My bisect is not done yet (2 steps to go, it will likely finish today), but in the current commit range https://github.com/gcc-mirror/gcc/commit/1d82fc2e6824bf83159389729c31a942f7b91b04 `optimize std::vector::push_back` is the most suspicious as it affects vector appending. It's a change to `libstdc++-v3`, not the compiler, so it doesn't directly point at a root cause.
📝 Chand-ra opened a pull request: "fuzz: Add tests for `CCoinControl` methods"
(https://github.com/bitcoin/bitcoin/pull/34026)
The `ccoincontrol` fuzzer misses tests for `GetSequence()` and `GetScripts()`. Add them.
(https://github.com/bitcoin/bitcoin/pull/34026)
The `ccoincontrol` fuzzer misses tests for `GetSequence()` and `GetScripts()`. Add them.
📝 ryslan25500-cloud opened a pull request: "Create PROPOSAL-V34-Tetrad-Research.md"
(https://github.com/bitcoin/bitcoin/pull/34027)
Add PROPOSAL-V34-Tetrad-Research.md
Detailed technical proposal for a federated research mining pool with deterministic 27-symbol tetrad embedding in Bitcoin blocks.
This is the V34 version of the Tetrad Research Proposal — a fully open, scientific, and strictly non-esoteric project aimed at:
• Creating the first long-term deterministic pattern dataset embedded directly into Bitcoin blocks • Studying potential emergent structures in perfectly uniform pseudorandom sequences • Demonstrat
...
(https://github.com/bitcoin/bitcoin/pull/34027)
Add PROPOSAL-V34-Tetrad-Research.md
Detailed technical proposal for a federated research mining pool with deterministic 27-symbol tetrad embedding in Bitcoin blocks.
This is the V34 version of the Tetrad Research Proposal — a fully open, scientific, and strictly non-esoteric project aimed at:
• Creating the first long-term deterministic pattern dataset embedded directly into Bitcoin blocks • Studying potential emergent structures in perfectly uniform pseudorandom sequences • Demonstrat
...
💬 Sjors commented on issue "rpc: getrawtransaction lookup by witness txid":
(https://github.com/bitcoin/bitcoin/issues/34013#issuecomment-3621878889)
> for a while
I would not expect this to land before v31 in spring 2026, so by that time @Fi3 might have finished implementing IPC support and could just use the more performant #34020.
(https://github.com/bitcoin/bitcoin/issues/34013#issuecomment-3621878889)
> for a while
I would not expect this to land before v31 in spring 2026, so by that time @Fi3 might have finished implementing IPC support and could just use the more performant #34020.
💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596211587)
https://github.com/bitcoin/bitcoin/compare/bbea9636a3b11df1c79664e97ded7907bbacfcf3..9efccb0d3e84d83e0ec29a4b18b72019ffc3d5c3
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596211587)
https://github.com/bitcoin/bitcoin/compare/bbea9636a3b11df1c79664e97ded7907bbacfcf3..9efccb0d3e84d83e0ec29a4b18b72019ffc3d5c3
💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596211598)
https://github.com/bitcoin/bitcoin/compare/bbea9636a3b11df1c79664e97ded7907bbacfcf3..9efccb0d3e84d83e0ec29a4b18b72019ffc3d5c3
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596211598)
https://github.com/bitcoin/bitcoin/compare/bbea9636a3b11df1c79664e97ded7907bbacfcf3..9efccb0d3e84d83e0ec29a4b18b72019ffc3d5c3
💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596211612)
https://github.com/bitcoin/bitcoin/compare/bbea9636a3b11df1c79664e97ded7907bbacfcf3..9efccb0d3e84d83e0ec29a4b18b72019ffc3d5c3
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596211612)
https://github.com/bitcoin/bitcoin/compare/bbea9636a3b11df1c79664e97ded7907bbacfcf3..9efccb0d3e84d83e0ec29a4b18b72019ffc3d5c3
💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596211643)
https://github.com/bitcoin/bitcoin/compare/bbea9636a3b11df1c79664e97ded7907bbacfcf3..9efccb0d3e84d83e0ec29a4b18b72019ffc3d5c3
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596211643)
https://github.com/bitcoin/bitcoin/compare/bbea9636a3b11df1c79664e97ded7907bbacfcf3..9efccb0d3e84d83e0ec29a4b18b72019ffc3d5c3
💬 stickies-v commented on pull request "log: exempt all category-specific logs from ratelimiting when running with debug":
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3621890490)
> if `-debug` is used with this implementation, it would stop rate-limiting of all category logs.
Debug logs are (by design) much higher frequency than info/warning/error level logs. They have never been ratelimited, because for debugging purposes it's important to see what's happening without limitation. When running with `-debug`, the overwhelming majority of logs produced are debug logs. This PR doesn't change anything about any of that.
Info logs are typically much higher signal than d
...
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3621890490)
> if `-debug` is used with this implementation, it would stop rate-limiting of all category logs.
Debug logs are (by design) much higher frequency than info/warning/error level logs. They have never been ratelimited, because for debugging purposes it's important to see what's happening without limitation. When running with `-debug`, the overwhelming majority of logs produced are debug logs. This PR doesn't change anything about any of that.
Info logs are typically much higher signal than d
...
💬 stickies-v commented on pull request "log: exempt all category-specific logs from ratelimiting when running with debug":
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3621894437)
Force-pushed to address merge conflict from #29641, no other changes.
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3621894437)
Force-pushed to address merge conflict from #29641, no other changes.
💬 sedited commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2596217297)
Not sure if that is really useful. We have a lot of functions that assume some check has been done beforehand to protect against dos. I don't see this function being misused again in the future by mistake, headers-presync is just too well established in the code base at this point. Maybe a more complete description of what the function does would be useful for people reading this the first time, that could then in turn describe why a dos check should be done before calling. I you want, I could a
...
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2596217297)
Not sure if that is really useful. We have a lot of functions that assume some check has been done beforehand to protect against dos. I don't see this function being misused again in the future by mistake, headers-presync is just too well established in the code base at this point. Maybe a more complete description of what the function does would be useful for people reading this the first time, that could then in turn describe why a dos check should be done before calling. I you want, I could a
...
💬 l0rinc commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2596215984)
I redid the change locally and I think this line is slightly different from the rest: could we do it in a separate commit before the refactor, so that after that the arg removal is a clean refactor?
We could also add `assert(min_pow_checked);` to the first commit, so that reviewers and CI can run all tests *with* the assert as an extra safety measure before the removal in the next one.
```C++
assert(min_pow_checked); // TODO removed in next commit
bool accepted{AcceptBlockHeader(header, state,
...
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2596215984)
I redid the change locally and I think this line is slightly different from the rest: could we do it in a separate commit before the refactor, so that after that the arg removal is a clean refactor?
We could also add `assert(min_pow_checked);` to the first commit, so that reviewers and CI can run all tests *with* the assert as an extra safety measure before the removal in the next one.
```C++
assert(min_pow_checked); // TODO removed in next commit
bool accepted{AcceptBlockHeader(header, state,
...