💬 ryanofsky commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2334900625)
> 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. The first part of https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2329412411 describes ideal behavior and contrasts it with current behavior, and the second part describes a specific way I think ideal behavior could be implemented.

I think current behavior is ok, but i
...

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2334910034)
The push to f6dee50f85b2668e96e9dffd65e06d8f3d3a4da2 adds verbosity levels and more detailed information about the orphans.

💬 ryanofsky commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2334911382)
> > it seems like a very clean one is to set the [`CMAKE_USER_MAKE_RULES_OVERRIDE`](https://cmake.org/cmake/help/v3.3/variable/CMAKE_USER_MAKE_RULES_OVERRIDE.html) hook
>
> FWIW, something similar was [considered](https://github.com/hebasto/bitcoin/pull/240) during work on the CMake staging branch.

Interesting! Although that PR was using CMAKE_USER_MAKE_RULES_OVERRIDE it was doing something pretty different with it. It was overriding cmake's low level compile command lines, which is defini
...

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1747798635)
I just think if you see:

```capnp
using Mining = import "../ipc/capnp/mining.capnp";
```

it is very obvious what file is being imported while if you see

```capnp
using Mining = import "/ipc/capnp/mining.capnp"
```

it's less clear what file is being imported because you don't know what the root path is. This is also inconsistent with standard uses of the root path which have package prefixes "capnp" for "mp":

```capnp
using Cxx = import "/capnp/c++.capnp";
using Proxy = impor
...

💬 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?

💬 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).

💬 theStack commented on pull request "rpc: add getorphantxs":
(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
...

✅ Monzurrz closed a pull request: "Create Satoshi diagnostics"
(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
...

💬 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.

💬 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
...

💬 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.

👍 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!

💬 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;
}
```

📝 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
...

📝 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.

💬 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.

💬 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.

💬 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?