💬 davidgumberg commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2334953579)
I'm trying to get a Windows box setup to reproduce this myself, but if you get a chance, could you do a run of both again with `-debug=bench` set, and post the final set of [bench] logs that get written to debug.log?
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2334953579)
I'm trying to get a Windows box setup to reproduce this myself, but if you get a chance, could you do a run of both again with `-debug=bench` set, and post the final set of [bench] logs that get written to debug.log?
💬 tdb3 commented on pull request "fix: handle invalid `-rpcbind` port earlier":
(https://github.com/bitcoin/bitcoin/pull/30679#issuecomment-2334961904)
> Hey, I'm reviewing this PR, and I saw that the test `run_invalid_bind_test` you implemented in `rpc_bind.py` is not invoking without explicitly providing the `--ipv4` or `--ipv6` flags, so is there any particular reason for that?
Thank you for the review!
In `test/functional/test_runner.py`, `rpc_bind.py` is run with three variations (`--ipv4`, `--ipv6`, and `--nonloopback`), so there is coverage for parsing rpcbind with both IPv4 and IPv6 addresses/port binds (loopback).
(https://github.com/bitcoin/bitcoin/pull/30679#issuecomment-2334961904)
> Hey, I'm reviewing this PR, and I saw that the test `run_invalid_bind_test` you implemented in `rpc_bind.py` is not invoking without explicitly providing the `--ipv4` or `--ipv6` flags, so is there any particular reason for that?
Thank you for the review!
In `test/functional/test_runner.py`, `rpc_bind.py` is run with three variations (`--ipv4`, `--ipv6`, and `--nonloopback`), so there is coverage for parsing rpcbind with both IPv4 and IPv6 addresses/port binds (loopback).
💬 theStack commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2335019463)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2335019463)
Concept ACK
📝 Monzurrz opened a pull request: "Create Satoshi diagnostics"
(https://github.com/bitcoin/bitcoin/pull/30839)
Looking for additional support. I can provide the original diagnostics helping build a better tomorrow today please provide additional support and guidance for and easier user friendly environent
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please pro
...
(https://github.com/bitcoin/bitcoin/pull/30839)
Looking for additional support. I can provide the original diagnostics helping build a better tomorrow today please provide additional support and guidance for and easier user friendly environent
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please pro
...
✅ Monzurrz closed a pull request: "Create Satoshi diagnostics"
(https://github.com/bitcoin/bitcoin/pull/30839)
(https://github.com/bitcoin/bitcoin/pull/30839)
💬 Monzurrz commented on pull request "Create Satoshi diagnostics":
(https://github.com/bitcoin/bitcoin/pull/30839#issuecomment-2335037817)
Diagnostic information for administrators:
Generating server: [CH4PR16MB6697.namprd16.prod.outlook.com](http://ch4pr16mb6697.namprd16.prod.outlook.com/)
[johann_hulstrom@hotmail.com](mailto:johann_hulstrom@hotmail.com)
Remote server returned '554 5.2.2 mailbox full; STOREDRV.Deliver.Exception:QuotaExceededException.MapiExceptionShutoffQuotaExceeded; Failed to process message due to a permanent exception with message [BeginDiagnosticData]The process failed to get the correct properties. 1.84
...
(https://github.com/bitcoin/bitcoin/pull/30839#issuecomment-2335037817)
Diagnostic information for administrators:
Generating server: [CH4PR16MB6697.namprd16.prod.outlook.com](http://ch4pr16mb6697.namprd16.prod.outlook.com/)
[johann_hulstrom@hotmail.com](mailto:johann_hulstrom@hotmail.com)
Remote server returned '554 5.2.2 mailbox full; STOREDRV.Deliver.Exception:QuotaExceededException.MapiExceptionShutoffQuotaExceeded; Failed to process message due to a permanent exception with message [BeginDiagnosticData]The process failed to get the correct properties. 1.84
...
💬 hebasto commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2335147801)
> > I don't think this describes the current behaviour precisely.
>
> Yes this is definitely not describing current behavior, it's trying to describe how current behavior "could be changed" to avoid writing the cache.
I'm still confused. Why should we "avoid writing the cache" for variables such as `WITH_ZMQ`? Using cache variables is a standard CMake method that allows the user to set their values.
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2335147801)
> > I don't think this describes the current behaviour precisely.
>
> Yes this is definitely not describing current behavior, it's trying to describe how current behavior "could be changed" to avoid writing the cache.
I'm still confused. Why should we "avoid writing the cache" for variables such as `WITH_ZMQ`? Using cache variables is a standard CMake method that allows the user to set their values.
💬 hebasto commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2335157778)
> For example cmake appends `-g` to `CMAKE_CXX_FLAGS_DEBUG_INIT` after the toolchain sets it
That's true. From CMake `CMAKE_<LANG>_FLAGS_<CONFIG>_INIT` variable's [docs](https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_FLAGS_CONFIG_INIT.html):
> CMake may prepend or append content to the value based on the environment and target platform.
> Suggestion above is to use CMAKE_USER_MAKE_RULES_OVERRIDE only to set _INIT variables more reliably than they can be set from the toolchain fil
...
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2335157778)
> For example cmake appends `-g` to `CMAKE_CXX_FLAGS_DEBUG_INIT` after the toolchain sets it
That's true. From CMake `CMAKE_<LANG>_FLAGS_<CONFIG>_INIT` variable's [docs](https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_FLAGS_CONFIG_INIT.html):
> CMake may prepend or append content to the value based on the environment and target platform.
> Suggestion above is to use CMAKE_USER_MAKE_RULES_OVERRIDE only to set _INIT variables more reliably than they can be set from the toolchain fil
...
💬 hebasto commented on pull request "cmake: add `USE_SOURCE_PERMISSIONS` to all `configure_file()` usage":
(https://github.com/bitcoin/bitcoin/pull/30823#issuecomment-2335160571)
If https://github.com/bitcoin/bitcoin/pull/30838 happens to fix https://github.com/bitcoin/bitcoin/issues/30815, it seems reasonable to switch to `NO_SOURCE_PERMISSIONS` in all cases for consistency.
(https://github.com/bitcoin/bitcoin/pull/30823#issuecomment-2335160571)
If https://github.com/bitcoin/bitcoin/pull/30838 happens to fix https://github.com/bitcoin/bitcoin/issues/30815, it seems reasonable to switch to `NO_SOURCE_PERMISSIONS` in all cases for consistency.
👍 l0rinc approved a pull request: "Remove unsafe uint256S() and test-only uint160S()"
(https://github.com/bitcoin/bitcoin/pull/30773#pullrequestreview-2287480459)
reACK 43cd83b0c7ba436b8ffc83d8a65e935714dffe70
Thanks for your perseverance!
(https://github.com/bitcoin/bitcoin/pull/30773#pullrequestreview-2287480459)
reACK 43cd83b0c7ba436b8ffc83d8a65e935714dffe70
Thanks for your perseverance!
💬 l0rinc commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1748063177)
nit1: you might as well delete the empty `(void)au256.GetHex();` a few lines below
nit2: `value()` is redundant here (i.e. `assert(uint256::FromHex(au256.GetHex()) == u256);` suffices) since `optional` has an `==`:
```C++
operator==(const optional<_Tp>& __x, const _Up& __v)
{
return static_cast<bool>(__x) ? *__x == __v : false;
}
```
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1748063177)
nit1: you might as well delete the empty `(void)au256.GetHex();` a few lines below
nit2: `value()` is redundant here (i.e. `assert(uint256::FromHex(au256.GetHex()) == u256);` suffices) since `optional` has an `==`:
```C++
operator==(const optional<_Tp>& __x, const _Up& __v)
{
return static_cast<bool>(__x) ? *__x == __v : false;
}
```
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30839)
Looking for additional support. I can provide the original diagnostics helping build a better tomorrow today please provide additional support and guidance for and easier user friendly environent
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please pro
...
(https://github.com/bitcoin/bitcoin/pull/30839)
Looking for additional support. I can provide the original diagnostics helping build a better tomorrow today please provide additional support and guidance for and easier user friendly environent
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please pro
...
📝 itornaza opened a pull request: "docs: Updated debug build instructions for cmake"
(https://github.com/bitcoin/bitcoin/pull/30840)
In the [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md) the section on [compiling for debug](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-debugging) displays outdated instructions that were applicable before the move to cmake build system.
This PR just gives instructions on how to build for debugging in the context of cmake.
(https://github.com/bitcoin/bitcoin/pull/30840)
In the [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md) the section on [compiling for debug](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-debugging) displays outdated instructions that were applicable before the move to cmake build system.
This PR just gives instructions on how to build for debugging in the context of cmake.
💬 l0rinc commented on pull request "refactor: Generalize `CScript`'s `std::vector` push to accept `std::array`, too":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1748069155)
Seems we're [getting an error on Windows](
In https://github.com/bitcoin/bitcoin/pull/30765/checks?check_run_id=29792259962) around `genesisOutputScript`, where `prevector#fill` is trying to write 65 bytes of data into a 32-byte buffer.
I can't reproduce it locally, any help would be appreciated.
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1748069155)
Seems we're [getting an error on Windows](
In https://github.com/bitcoin/bitcoin/pull/30765/checks?check_run_id=29792259962) around `genesisOutputScript`, where `prevector#fill` is trying to write 65 bytes of data into a 32-byte buffer.
I can't reproduce it locally, any help would be appreciated.
💬 vostrnad commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2335184072)
I've bisected this down to #28052. From the benchmark results it appears to be solely responsible for the slowdown between 27.0 and 28.0rc1 (so #30326 should be fine @andrewtoth).
@davidgumberg Please let me know if you still need those bench logs.
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2335184072)
I've bisected this down to #28052. From the benchmark results it appears to be solely responsible for the slowdown between 27.0 and 28.0rc1 (so #30326 should be fine @andrewtoth).
@davidgumberg Please let me know if you still need those bench logs.
💬 sipa commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2335186082)
@vostrnad Does the performance on Windows recover if you run with the `-blocksxor=0` option?
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2335186082)
@vostrnad Does the performance on Windows recover if you run with the `-blocksxor=0` option?
📝 hebasto opened a pull request: "ci: Post CMake-migration fixes and amendments"
(https://github.com/bitcoin/bitcoin/pull/30841)
This PR addresses the change in logging that [happened](https://cmake.org/cmake/help/latest/release/3.26.html#configure-log) in CMake 3.26.
Additionally, the `make` invocation replaced with `cmake --build`.
(https://github.com/bitcoin/bitcoin/pull/30841)
This PR addresses the change in logging that [happened](https://cmake.org/cmake/help/latest/release/3.26.html#configure-log) in CMake 3.26.
Additionally, the `make` invocation replaced with `cmake --build`.
💬 vostrnad commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2335190583)
@sipa No, it's exactly the same with `-blocksxor=0`.
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2335190583)
@sipa No, it's exactly the same with `-blocksxor=0`.
📝 hebasto opened a pull request: "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"
(https://github.com/bitcoin/bitcoin/pull/30842)
This PR aims to reduce the build time.
(https://github.com/bitcoin/bitcoin/pull/30842)
This PR aims to reduce the build time.
💬 hebasto commented on issue "guix: Build fails for `x86_64-apple-darwin`":
(https://github.com/bitcoin/bitcoin/issues/30810#issuecomment-2335384269)
```
$ uname -m
riscv64
$ guix shell --container --pure hello
$ hello
Hello, world!
$ exit
exit
```
The issue happens when switch to/form Darwin host. For both invocations
```
$ env SDK_PATH=/home/hebasto HOSTS="x86_64-w64-mingw32 x86_64-apple-darwin" ./contrib/guix/guix-build
```
and
```
$ env SDK_PATH=/home/hebasto HOSTS="x86_64-apple-darwin x86_64-w64-mingw32" ./contrib/guix/guix-build
```
build for the former host succeeds, then the error happens:
```
guix shell: error: c
...
(https://github.com/bitcoin/bitcoin/issues/30810#issuecomment-2335384269)
```
$ uname -m
riscv64
$ guix shell --container --pure hello
$ hello
Hello, world!
$ exit
exit
```
The issue happens when switch to/form Darwin host. For both invocations
```
$ env SDK_PATH=/home/hebasto HOSTS="x86_64-w64-mingw32 x86_64-apple-darwin" ./contrib/guix/guix-build
```
and
```
$ env SDK_PATH=/home/hebasto HOSTS="x86_64-apple-darwin x86_64-w64-mingw32" ./contrib/guix/guix-build
```
build for the former host succeeds, then the error happens:
```
guix shell: error: c
...