👍 l0rinc approved a pull request: "test: Prevent disk space warning during node_init_tests"
(https://github.com/bitcoin/bitcoin/pull/33391#pullrequestreview-3225850554)
tested ACK bdf01c6f61262cd6e211ead3c0dbc66ccb48b32f
It prints the warning without the change, no warning after the change
(https://github.com/bitcoin/bitcoin/pull/33391#pullrequestreview-3225850554)
tested ACK bdf01c6f61262cd6e211ead3c0dbc66ccb48b32f
It prints the warning without the change, no warning after the change
💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2349859713)
Please consider triggering a compiler warning instead.
```
₿ git grep "// no default case, so the compiler can warn about missing cases" |wc -l
70
```
https://github.com/bitcoin/bitcoin/blob/9b04e4f96200daf7516d1b8b6b0dfc4c077837e8/src/deploymentinfo.cpp#L36-L37
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2349859713)
Please consider triggering a compiler warning instead.
```
₿ git grep "// no default case, so the compiler can warn about missing cases" |wc -l
70
```
https://github.com/bitcoin/bitcoin/blob/9b04e4f96200daf7516d1b8b6b0dfc4c077837e8/src/deploymentinfo.cpp#L36-L37
🤔 mzumsande reviewed a pull request: "test: Prevent disk space warning during node_init_tests"
(https://github.com/bitcoin/bitcoin/pull/33391#pullrequestreview-3225928315)
utACK bdf01c6f61262cd6e211ead3c0dbc66ccb48b32f
(https://github.com/bitcoin/bitcoin/pull/33391#pullrequestreview-3225928315)
utACK bdf01c6f61262cd6e211ead3c0dbc66ccb48b32f
💬 hodlinator commented on pull request "Remove unnecessary casts when calling socket operations":
(https://github.com/bitcoin/bitcoin/pull/33378#discussion_r2349889597)
Agree that we can get rid of the `typedef` that is so rarely used.
Should probably use another kind of cast here and in *sock_tests.cpp*?
https://github.com/bitcoin/bitcoin/blob/da23e2e2e104fc9d8e210c3989f2ae4945c072fa/doc/developer-notes.md?plain=1#L60-L63
(https://github.com/bitcoin/bitcoin/pull/33378#discussion_r2349889597)
Agree that we can get rid of the `typedef` that is so rarely used.
Should probably use another kind of cast here and in *sock_tests.cpp*?
https://github.com/bitcoin/bitcoin/blob/da23e2e2e104fc9d8e210c3989f2ae4945c072fa/doc/developer-notes.md?plain=1#L60-L63
🤔 pinheadmz reviewed a pull request: "Introduce SockMan ("lite"): low-level socket handling for HTTP"
(https://github.com/bitcoin/bitcoin/pull/32747#pullrequestreview-3217223929)
Rebase to e007e1b57d addresses excellent thorough review from @hodlinator (THANKS!). Most significant changes involve casting arguments to socket functions (see #33378) and cleaning up protected/private membership of `SockMan`. I have locally rebased #32061 on these changes and everything still passes ;-) I will update that PR soon based on these changes.
(https://github.com/bitcoin/bitcoin/pull/32747#pullrequestreview-3217223929)
Rebase to e007e1b57d addresses excellent thorough review from @hodlinator (THANKS!). Most significant changes involve casting arguments to socket functions (see #33378) and cleaning up protected/private membership of `SockMan`. I have locally rebased #32061 on these changes and everything still passes ;-) I will update that PR soon based on these changes.
💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2345103522)
👍
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2345103522)
👍
💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2344946486)
Yes will make these data private/protected and add public helpers.
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2344946486)
Yes will make these data private/protected and add public helpers.
💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2345329538)
you're right thanks
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2345329538)
you're right thanks
💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2345201886)
i like it thanks
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2345201886)
i like it thanks
💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2344995517)
done
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2344995517)
done
💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2345192358)
👍
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2345192358)
👍
💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2349207386)
I guess the two sentences are a bit redundant, I'm going to keep the first and lose the second, though.
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2349207386)
I guess the two sentences are a bit redundant, I'm going to keep the first and lose the second, though.
💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2344997699)
You're right for new files we're removing the year(s) entirely.
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2344997699)
You're right for new files we're removing the year(s) entirely.
💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2345020780)
Sure taken
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2345020780)
Sure taken
💬 l0rinc commented on issue "v30.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3293705610)
I ran a full IBD with real nodes over Wi-Fi until block 914452 with a brand new Raspberry Pi 5 (16GB RAM, SSD) - it finished successfully in 16h12m using 6GB dbcache.
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3293705610)
I ran a full IBD with real nodes over Wi-Fi until block 914452 with a brand new Raspberry Pi 5 (16GB RAM, SSD) - it finished successfully in 16h12m using 6GB dbcache.
📝 sekomer opened a pull request: "refactor: Fix typo and correct template parameter inconsistency"
(https://github.com/bitcoin/bitcoin/pull/33394)
- Fixed typo in merkle.cpp where variable was named 'matchh' instead of 'match'
- Fixed Hash160 to use 'T' instead of 'T1' for single template parameter, making it consistent with other single-parameter template functions like Hash<T>
## Impact
These are refactoring changes with no functional impact:
- No behavior changes
- No consensus changes
- All existing tests pass without modification
- Improves code maintainability and reduces cognitive load for developers
(https://github.com/bitcoin/bitcoin/pull/33394)
- Fixed typo in merkle.cpp where variable was named 'matchh' instead of 'match'
- Fixed Hash160 to use 'T' instead of 'T1' for single template parameter, making it consistent with other single-parameter template functions like Hash<T>
## Impact
These are refactoring changes with no functional impact:
- No behavior changes
- No consensus changes
- All existing tests pass without modification
- Improves code maintainability and reduces cognitive load for developers
📝 mzumsande opened a pull request: "net: do not apply whitelist permissions to onion inbounds"
(https://github.com/bitcoin/bitcoin/pull/33395)
Tor inbound connections do not reveal the peer's actual network address. Do not apply whitelist permissions to them since address-based matching is ineffective.
(https://github.com/bitcoin/bitcoin/pull/33395)
Tor inbound connections do not reveal the peer's actual network address. Do not apply whitelist permissions to them since address-based matching is ineffective.
💬 l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3293797977)
> using std::uncaught_exception() to prevent throwing
https://en.cppreference.com/w/cpp/error/uncaught_exception.html indicates `std::uncaught_exception()` is deprecated in C++17 and removed in C++20 - but we could use `std::uncaught_exceptions()`.
Added a few comments, thanks for tackling this.
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3293797977)
> using std::uncaught_exception() to prevent throwing
https://en.cppreference.com/w/cpp/error/uncaught_exception.html indicates `std::uncaught_exception()` is deprecated in C++17 and removed in C++20 - but we could use `std::uncaught_exceptions()`.
Added a few comments, thanks for tackling this.
💬 l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2349822297)
This parameter change is a separate concern. Please split it into its own commit to keep the risky behavior change easy to review.
Though personally, I'd pass both by value and move into the members to handle rvalues efficiently.
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2349822297)
This parameter change is a separate concern. Please split it into its own commit to keep the risky behavior change easy to review.
Though personally, I'd pass both by value and move into the members to handle rvalues efficiently.
💬 l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2349820667)
Please add `#include <cstdlib>` for [std::abort()](https://en.cppreference.com/w/cpp/utility/program/abort)
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2349820667)
Please add `#include <cstdlib>` for [std::abort()](https://en.cppreference.com/w/cpp/utility/program/abort)