π hebasto's pull request is ready for review: "depends: Avoid using the `-ffile-prefix-map` compiler option"
(https://github.com/bitcoin/bitcoin/pull/31800)
(https://github.com/bitcoin/bitcoin/pull/31800)
π¬ hebasto commented on pull request "depends: Override default build type for `libevent`":
(https://github.com/bitcoin/bitcoin/pull/31661#discussion_r1943172637)
> Something that should probably be followed up with, given that most coverage builds are using depends, is the fact that `-ffile-prefix-map` keeps being hardcoded into depends, while CMakelists.txt says:
>
> ```shell
> # Avoiding the `-ffile-prefix-map` compiler option because it implies
> # `-fcoverage-prefix-map` on Clang or `-fprofile-prefix-map` on GCC,
> # which can cause issues with coverage builds,
> ```
>
> Might be less of an issue given these are deps but it's at least incon
...
(https://github.com/bitcoin/bitcoin/pull/31661#discussion_r1943172637)
> Something that should probably be followed up with, given that most coverage builds are using depends, is the fact that `-ffile-prefix-map` keeps being hardcoded into depends, while CMakelists.txt says:
>
> ```shell
> # Avoiding the `-ffile-prefix-map` compiler option because it implies
> # `-fcoverage-prefix-map` on Clang or `-fprofile-prefix-map` on GCC,
> # which can cause issues with coverage builds,
> ```
>
> Might be less of an issue given these are deps but it's at least incon
...
π¬ Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637244937)
Unless I'm missing something, I don't think @luke-jr's critique is specific to libmultiprocess, but rather his consistent position on including libraries.
I think we should subtree libmultiprocess the same way we do with the other subtrees (to the extend possible). We can revisit that entire process later imo.
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637244937)
Unless I'm missing something, I don't think @luke-jr's critique is specific to libmultiprocess, but rather his consistent position on including libraries.
I think we should subtree libmultiprocess the same way we do with the other subtrees (to the extend possible). We can revisit that entire process later imo.
π fanquake merged a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832)
(https://github.com/bitcoin/bitcoin/pull/25832)
π¬ maflcko commented on issue "lint: Qt exclusions are missing files":
(https://github.com/bitcoin/bitcoin/issues/31801#issuecomment-2637274149)
I guess longer term this may be fixed by https://github.com/bitcoin/bitcoin/pull/31308 , but I haven't confirmed this.
(https://github.com/bitcoin/bitcoin/issues/31801#issuecomment-2637274149)
I guess longer term this may be fixed by https://github.com/bitcoin/bitcoin/pull/31308 , but I haven't confirmed this.
π¬ Sjors commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1943202181)
The solvables wallet itself is watch-only, so I think `iswatchonly` should be `true`?
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1943202181)
The solvables wallet itself is watch-only, so I think `iswatchonly` should be `true`?
π¬ maflcko commented on issue "build: compiler flags in linker flags output":
(https://github.com/bitcoin/bitcoin/issues/31487#issuecomment-2637285799)
Maybe this can be closed, per https://github.com/bitcoin/bitcoin/pull/31726#issuecomment-2611841905 ? In any case, a debug output shouldn't be a blocker for the 29.x milestone.
(https://github.com/bitcoin/bitcoin/issues/31487#issuecomment-2637285799)
Maybe this can be closed, per https://github.com/bitcoin/bitcoin/pull/31726#issuecomment-2611841905 ? In any case, a debug output shouldn't be a blocker for the 29.x milestone.
π¬ maflcko commented on pull request "ci: Replace `CMAKE_CXX_FLAGS` with `APPEND_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/31726#discussion_r1943212167)
thx, could submit as a fresh pull?
(https://github.com/bitcoin/bitcoin/pull/31726#discussion_r1943212167)
thx, could submit as a fresh pull?
β
fanquake closed an issue: "build: compiler flags in linker flags output"
(https://github.com/bitcoin/bitcoin/issues/31487)
(https://github.com/bitcoin/bitcoin/issues/31487)
π¬ fanquake commented on issue "build: compiler flags in linker flags output":
(https://github.com/bitcoin/bitcoin/issues/31487#issuecomment-2637302626)
Yea I think this is just going to be the new normal.
(https://github.com/bitcoin/bitcoin/issues/31487#issuecomment-2637302626)
Yea I think this is just going to be the new normal.
π¬ Sjors commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1943215423)
I think there was an earlier bugfix PR / issue around this, but I can't find it.
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1943215423)
I think there was an earlier bugfix PR / issue around this, but I can't find it.
π¬ ajtowns commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2637310051)
Given that we do headers sync first these days, would it make sense to switch to reporting:
* verificationprogress = just blocks/headers
* headerrecency = max(0, now() - timestamp of best header)
Normally, verificationprogress will be 1.0; dropping to ~0.9999988 briefly when a new header comes in, and headerrecency will tend to be in the ~10 minute range. OTOH, if you're in IBD, then verificationprogress will report how far through validating the blocks you have headers for; and if you'
...
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2637310051)
Given that we do headers sync first these days, would it make sense to switch to reporting:
* verificationprogress = just blocks/headers
* headerrecency = max(0, now() - timestamp of best header)
Normally, verificationprogress will be 1.0; dropping to ~0.9999988 briefly when a new header comes in, and headerrecency will tend to be in the ~10 minute range. OTOH, if you're in IBD, then verificationprogress will report how far through validating the blocks you have headers for; and if you'
...
π¬ ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637351868)
> Unless I'm missing something, I don't think @luke-jr's critique is specific to libmultiprocess, but rather his consistent position on including libraries.
I see this but I think I actually agree with (or at least sympathize with) Luke's position on libraries, and am trying to ensure more clean integration with this library than other libraries that we depend on like leveldb, univalue, libevent. This PR is not forking the ilbrary or make making local changes to it. It is not replacing its bu
...
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637351868)
> Unless I'm missing something, I don't think @luke-jr's critique is specific to libmultiprocess, but rather his consistent position on including libraries.
I see this but I think I actually agree with (or at least sympathize with) Luke's position on libraries, and am trying to ensure more clean integration with this library than other libraries that we depend on like leveldb, univalue, libevent. This PR is not forking the ilbrary or make making local changes to it. It is not replacing its bu
...
π Sjors opened a pull request: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802)
Have depends make libmultiprocess by default, which has the effect of including it in the release binaries. Except for Windows and OpenBSD which are not supported yet, the latter due to a fairly trivial upstream issue.
The initial main use case is to enable experimental support for the Mining IPC interface. A working example of a Stratum v2 Template Provider client using this interface can be found here: https://github.com/Sjors/bitcoin/pull/48.
Additionally the `bitcoin-node` and `bitcoi
...
(https://github.com/bitcoin/bitcoin/pull/31802)
Have depends make libmultiprocess by default, which has the effect of including it in the release binaries. Except for Windows and OpenBSD which are not supported yet, the latter due to a fairly trivial upstream issue.
The initial main use case is to enable experimental support for the Mining IPC interface. A working example of a Stratum v2 Template Provider client using this interface can be found here: https://github.com/Sjors/bitcoin/pull/48.
Additionally the `bitcoin-node` and `bitcoi
...
π¬ luke-jr commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637357864)
Downloading something separately is not a real issue.
But once it's a subtree, you can't avoid downloading it, and it might even be messy to remove it (to ensure it isn't used).
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637357864)
Downloading something separately is not a real issue.
But once it's a subtree, you can't avoid downloading it, and it might even be messy to remove it (to ensure it isn't used).
π¬ Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2637377337)
All jobs happy!
I opened #31802 as draft which takes over the original intention for this PR. It flips the depends default and includes 743ae9b4e2746f48d707d49fc2c8ba908995034d which I dropped here.
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2637377337)
All jobs happy!
I opened #31802 as draft which takes over the original intention for this PR. It flips the depends default and includes 743ae9b4e2746f48d707d49fc2c8ba908995034d which I dropped here.
π¬ ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637412067)
> But once it's a subtree, you can't avoid downloading it, and it might even be messy to remove it (to ensure it isn't used).
By design, it is not messy to remove. The subtree is referenced in exactly one [`add_subdirectory`](https://github.com/bitcoin/bitcoin/blob/99b7b857d3b8b9ce9bd7501e2480583369740c55/src/ipc/CMakeLists.txt#L8) call, and nowhere else in the cmake build. If the `-DWITH_LIBMULTIPROCESS=ON` is set the `add_subdirectory` call is not made and a `find_package` call is made inst
...
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637412067)
> But once it's a subtree, you can't avoid downloading it, and it might even be messy to remove it (to ensure it isn't used).
By design, it is not messy to remove. The subtree is referenced in exactly one [`add_subdirectory`](https://github.com/bitcoin/bitcoin/blob/99b7b857d3b8b9ce9bd7501e2480583369740c55/src/ipc/CMakeLists.txt#L8) call, and nowhere else in the cmake build. If the `-DWITH_LIBMULTIPROCESS=ON` is set the `add_subdirectory` call is not made and a `find_package` call is made inst
...
π¬ Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2637415972)
Merge conflict is probably from #25832.
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2637415972)
Merge conflict is probably from #25832.
π¬ theuni commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2637460278)
> @theuni
>
> > Wait, I take that back. It seems like we could still get 2 separate `cs_main`s here: one in the lib and the other in the binary. ACK retracted while I investigate.
>
> To clarify this discussion, which binary you are referring toβ`bitcoind` or `bitcoin-chainstate`? I assume the latter, as the former uses the static kernel library.
What I'm building isn't really relevant... the combo of the inline var with hidden visibility when built as a shared lib is a potential proble
...
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2637460278)
> @theuni
>
> > Wait, I take that back. It seems like we could still get 2 separate `cs_main`s here: one in the lib and the other in the binary. ACK retracted while I investigate.
>
> To clarify this discussion, which binary you are referring toβ`bitcoind` or `bitcoin-chainstate`? I assume the latter, as the former uses the static kernel library.
What I'm building isn't really relevant... the combo of the inline var with hidden visibility when built as a shared lib is a potential proble
...
π¬ 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2637463025)
Using BIP155 network IDs is a good idea! Planning to address https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1943088145 in a followup.
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2637463025)
Using BIP155 network IDs is a good idea! Planning to address https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1943088145 in a followup.