💬 MarcoFalke commented on pull request "doc: clarify processing of mempool-msgs when NODE_BLOOM":
(https://github.com/bitcoin/bitcoin/pull/27559#discussion_r1189635504)
I think it makes sense to also document this in `src/protocol.h`, which generally refers to the p2p version and the BIP number. I read BIP 35, 37, and 111, but I couldn't find a mention that the message type is guarded by NODE_BLOOM. So maybe this can also be mentioned in `src/protocol.h`?
(https://github.com/bitcoin/bitcoin/pull/27559#discussion_r1189635504)
I think it makes sense to also document this in `src/protocol.h`, which generally refers to the p2p version and the BIP number. I read BIP 35, 37, and 111, but I couldn't find a mention that the message type is guarded by NODE_BLOOM. So maybe this can also be mentioned in `src/protocol.h`?
📝 fanquake opened a pull request: "[24.x] Backports for rc3"
(https://github.com/bitcoin/bitcoin/pull/27614)
Collecting backports for rc3. Currently:
* https://github.com/bitcoin/bitcoin/pull/27608
(https://github.com/bitcoin/bitcoin/pull/27614)
Collecting backports for rc3. Currently:
* https://github.com/bitcoin/bitcoin/pull/27608
💬 willcl-ark commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1541828229)
FWIW I am also seeing some sustained high CPU usage on a listening (non debug mode) 24.0.1 node from the `b-msghand` thread:

I am used to seeing this thread at ~1% on this machine:

I also have a number of these "hungry" peers:
```fish
bitcoin in ~ took 53s ➜ bitcoin-cli getpeer
...
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1541828229)
FWIW I am also seeing some sustained high CPU usage on a listening (non debug mode) 24.0.1 node from the `b-msghand` thread:

I am used to seeing this thread at ~1% on this machine:

I also have a number of these "hungry" peers:
```fish
bitcoin in ~ took 53s ➜ bitcoin-cli getpeer
...
💬 stickies-v commented on pull request "refactor: Replace global find_value function with UniValue::find_value method":
(https://github.com/bitcoin/bitcoin/pull/27605#discussion_r1189677819)
I think it would be best to add the commit message as code documentation to make it easier to track when this can be reverted?
I'm also not sure this commit fits the scope of the PR but I can see how it's related, and it's a separate commit anyway so probably fine.
(https://github.com/bitcoin/bitcoin/pull/27605#discussion_r1189677819)
I think it would be best to add the commit message as code documentation to make it easier to track when this can be reverted?
I'm also not sure this commit fits the scope of the PR but I can see how it's related, and it's a separate commit anyway so probably fine.
💬 ferzan98 commented on issue "Drop support for g++-8?":
(https://github.com/bitcoin/bitcoin/issues/27537#issuecomment-1541862923)
ack means short for acknowledgment
(https://github.com/bitcoin/bitcoin/issues/27537#issuecomment-1541862923)
ack means short for acknowledgment
💬 MarcoFalke commented on pull request "refactor: Replace global find_value function with UniValue::find_value method":
(https://github.com/bitcoin/bitcoin/pull/27605#discussion_r1189684949)
I am thinking that this may not be fixed by upstream gcc, and anyway, gcc-13.1.1 is released and will stick around, so this can't be reverted any time soon, anyway. So it might be best to just make it permanent now?
(https://github.com/bitcoin/bitcoin/pull/27605#discussion_r1189684949)
I am thinking that this may not be fixed by upstream gcc, and anyway, gcc-13.1.1 is released and will stick around, so this can't be reverted any time soon, anyway. So it might be best to just make it permanent now?
📝 hebasto opened a pull request: "msvc: Rename `libbitcoinconsensus` to `libbitcoin_consensus` and other adjustments"
(https://github.com/bitcoin/bitcoin/pull/27615)
The current Autotools-based build system operates with two build artifacts:
- [`LIBBITCOIN_CONSENSUS`](https://github.com/bitcoin/bitcoin/blob/3777c75d147954b24f770dc7781b27870eadf775/src/Makefile.am#L31) which is [defined as](https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md) "Stable, backwards-compatible consensus functionality used by _libbitcoin_node_ and _libbitcoin_wallet_"
- [`LIBBITCOINCONSENSUS`](https://github.com/bitcoin/bitcoin/blob/3777c75d147954b24f770dc7781b
...
(https://github.com/bitcoin/bitcoin/pull/27615)
The current Autotools-based build system operates with two build artifacts:
- [`LIBBITCOIN_CONSENSUS`](https://github.com/bitcoin/bitcoin/blob/3777c75d147954b24f770dc7781b27870eadf775/src/Makefile.am#L31) which is [defined as](https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md) "Stable, backwards-compatible consensus functionality used by _libbitcoin_node_ and _libbitcoin_wallet_"
- [`LIBBITCOINCONSENSUS`](https://github.com/bitcoin/bitcoin/blob/3777c75d147954b24f770dc7781b
...
💬 hebasto commented on pull request "Enable HW-accelerated implementations of SHA256 for MSVC builds":
(https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1541888040)
Drafted again as it seems worth going https://github.com/bitcoin/bitcoin/pull/27598 and https://github.com/bitcoin/bitcoin/pull/27615 first.
> UPD. Well, the reason is that
>
> https://github.com/bitcoin/bitcoin/blob/322ec63b01499c1ec52d3912ee382ebd59f2366b/src/bench/bench_bitcoin.cpp#L64
Fixed in https://github.com/bitcoin/bitcoin/pull/27615.
(https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1541888040)
Drafted again as it seems worth going https://github.com/bitcoin/bitcoin/pull/27598 and https://github.com/bitcoin/bitcoin/pull/27615 first.
> UPD. Well, the reason is that
>
> https://github.com/bitcoin/bitcoin/blob/322ec63b01499c1ec52d3912ee382ebd59f2366b/src/bench/bench_bitcoin.cpp#L64
Fixed in https://github.com/bitcoin/bitcoin/pull/27615.
👍 TheCharlatan approved a pull request: "ci: Remove CI_EXEC bloat in test/06_script_b.sh"
(https://github.com/bitcoin/bitcoin/pull/27573#pullrequestreview-1420334095)
ACK fa1dbd04cab8039440e721eddabb760a40ba8c61
(https://github.com/bitcoin/bitcoin/pull/27573#pullrequestreview-1420334095)
ACK fa1dbd04cab8039440e721eddabb760a40ba8c61
💬 fanquake commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1189718234)
Followed up in #27611.
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1189718234)
Followed up in #27611.
💬 fanquake commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1189718346)
Followed up in #27611.
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1189718346)
Followed up in #27611.
💬 fanquake commented on pull request "doc: clarify processing of mempool-msgs when NODE_BLOOM":
(https://github.com/bitcoin/bitcoin/pull/27559#discussion_r1189726780)
@0xB10C did you want to follow up here?
(https://github.com/bitcoin/bitcoin/pull/27559#discussion_r1189726780)
@0xB10C did you want to follow up here?
💬 hebasto commented on pull request "refactor: Replace global find_value function with UniValue::find_value method":
(https://github.com/bitcoin/bitcoin/pull/27605#discussion_r1189731837)
GCC 13 introduced a broken behavior, and we usually do not modify our correct code to make a broken compiler happy. Considering that this is the only line in test code, I lean to agree with this change.
(https://github.com/bitcoin/bitcoin/pull/27605#discussion_r1189731837)
GCC 13 introduced a broken behavior, and we usually do not modify our correct code to make a broken compiler happy. Considering that this is the only line in test code, I lean to agree with this change.
💬 fanquake commented on pull request "ci: Remove CI_EXEC bloat in test/06_script_b.sh":
(https://github.com/bitcoin/bitcoin/pull/27573#issuecomment-1541930822)
Merging this now. #27125 is going to be re-rebased to deal with the (minor) conflict. That PR is also waiting on at least one followup comment.
(https://github.com/bitcoin/bitcoin/pull/27573#issuecomment-1541930822)
Merging this now. #27125 is going to be re-rebased to deal with the (minor) conflict. That PR is also waiting on at least one followup comment.
🚀 fanquake merged a pull request: "ci: Remove CI_EXEC bloat in test/06_script_b.sh"
(https://github.com/bitcoin/bitcoin/pull/27573)
(https://github.com/bitcoin/bitcoin/pull/27573)
💬 josibake commented on pull request "add ryanofsky to trusted-keys":
(https://github.com/bitcoin/bitcoin/pull/27604#issuecomment-1541934810)
post-merge ACK https://github.com/bitcoin/bitcoin/commit/59ebee3fb4181baf20fab263cf1b587ece1bd5e2
Solid track record of review and work on the project. Aside from the obvious fit for an interfaces maintainer, ryanofsky has a track record of thorough review and being able to clearly articulate his reasoning
(https://github.com/bitcoin/bitcoin/pull/27604#issuecomment-1541934810)
post-merge ACK https://github.com/bitcoin/bitcoin/commit/59ebee3fb4181baf20fab263cf1b587ece1bd5e2
Solid track record of review and work on the project. Aside from the obvious fit for an interfaces maintainer, ryanofsky has a track record of thorough review and being able to clearly articulate his reasoning
💬 MarcoFalke commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1189743041)
Seems unrelated, so maybe do in a follow-up or not at all?
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1189743041)
Seems unrelated, so maybe do in a follow-up or not at all?
💬 hebasto commented on pull request "refactor: Replace global find_value function with UniValue::find_value method":
(https://github.com/bitcoin/bitcoin/pull/27605#discussion_r1189744019)
As the test is execute in a single thread, is it big deal to hold `::cs_main` during the entire test case?
Why not to use the pattern as https://github.com/bitcoin/bitcoin/blob/fa266c4bbf564308ddbc12653527226506902084/src/test/interfaces_tests.cpp#L130-L132
?
(https://github.com/bitcoin/bitcoin/pull/27605#discussion_r1189744019)
As the test is execute in a single thread, is it big deal to hold `::cs_main` during the entire test case?
Why not to use the pattern as https://github.com/bitcoin/bitcoin/blob/fa266c4bbf564308ddbc12653527226506902084/src/test/interfaces_tests.cpp#L130-L132
?
👋 MarcoFalke's pull request is ready for review: "ci: Run iwyu on all src files"
(https://github.com/bitcoin/bitcoin/pull/27571)
(https://github.com/bitcoin/bitcoin/pull/27571)
💬 MarcoFalke commented on pull request "refactor: Replace global find_value function with UniValue::find_value method":
(https://github.com/bitcoin/bitcoin/pull/27605#discussion_r1189746443)
Likely due to a LOCKS_EXCLUDED?
(https://github.com/bitcoin/bitcoin/pull/27605#discussion_r1189746443)
Likely due to a LOCKS_EXCLUDED?