💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2635727935)
Rebased b4d397903d652bae1497140775591f755d857e65 -> 99b7b857d3b8b9ce9bd7501e2480583369740c55 ([`pr/subtree.7`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.7) -> [`pr/subtree.8`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.8), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.7-rebase..pr/subtree.8)) with some more fixes for downstream PR #30975 needed after switching depends build from `find_package` to `add_subdirectory`. Also includes a fix for guix mult
...
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2635727935)
Rebased b4d397903d652bae1497140775591f755d857e65 -> 99b7b857d3b8b9ce9bd7501e2480583369740c55 ([`pr/subtree.7`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.7) -> [`pr/subtree.8`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.8), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.7-rebase..pr/subtree.8)) with some more fixes for downstream PR #30975 needed after switching depends build from `find_package` to `add_subdirectory`. Also includes a fix for guix mult
...
💬 stratospher commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r1942239943)
98d68e0: shouldn't we have a `blocks.push_back(index)` in this block of code?
right now, only genesis block gets inserted into `std::vector<CBlockIndex*> blocks` and we don't enter into the interesting test cases.
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r1942239943)
98d68e0: shouldn't we have a `blocks.push_back(index)` in this block of code?
right now, only genesis block gets inserted into `std::vector<CBlockIndex*> blocks` and we don't enter into the interesting test cases.
👍 BrandonOdiwuor approved a pull request: "test: added additional coverage to waitforblock and waitforblockheight rpc's"
(https://github.com/bitcoin/bitcoin/pull/31784#pullrequestreview-2594718854)
Code Review ACK 7e0db87d4fff996c086f6e86b62338c98ef30c55
(https://github.com/bitcoin/bitcoin/pull/31784#pullrequestreview-2594718854)
Code Review ACK 7e0db87d4fff996c086f6e86b62338c98ef30c55
💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2635917190)
@luke-jr, some pros of this PR in @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2635697258) above. What do you think are the cons?
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2635917190)
@luke-jr, some pros of this PR in @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2635697258) above. What do you think are the cons?
💬 maflcko commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942384199)
I know, but this is unrelated to this pull request. `std::span` serialization is allowed today on current master (faaf4800aa752dde63b8987b1eb0de4e54acf717), so if someone wanted to remove this, it could be done unrelated to this pull request.
I don't really see the risk of documenting that span serialization is intended, but I am happy to reword or remove the comment.
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942384199)
I know, but this is unrelated to this pull request. `std::span` serialization is allowed today on current master (faaf4800aa752dde63b8987b1eb0de4e54acf717), so if someone wanted to remove this, it could be done unrelated to this pull request.
I don't really see the risk of documenting that span serialization is intended, but I am happy to reword or remove the comment.
💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942419081)
> The output of a depends build is locally deterministic based on the build id, which is comprised of the environment, toolchain, and source hashes. After this change, as far as I can tell, that's no longer true because it also depends on the contents of your hard drive, which may have been modified after checkout.
Even without this PR the output of depends build could change based on the contents of the hard drive, no? E.g. you can checkout `github/bitcoin/bitcoin`, then modify something in
...
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942419081)
> The output of a depends build is locally deterministic based on the build id, which is comprised of the environment, toolchain, and source hashes. After this change, as far as I can tell, that's no longer true because it also depends on the contents of your hard drive, which may have been modified after checkout.
Even without this PR the output of depends build could change based on the contents of the hard drive, no? E.g. you can checkout `github/bitcoin/bitcoin`, then modify something in
...
✅ maflcko closed an issue: "Anticipation quantum attack ; post quantum security"
(https://github.com/bitcoin/bitcoin/issues/31799)
(https://github.com/bitcoin/bitcoin/issues/31799)
💬 maflcko commented on issue "Anticipation quantum attack ; post quantum security":
(https://github.com/bitcoin/bitcoin/issues/31799#issuecomment-2636157846)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
Network-wide consensus and/or P2P changes first need to be discussed with the greater ecosystem, for example https://groups.google.com/g/bitcoindev, or https://delvingbitcoin.org/. Also, they need a BIP to be implemented in Bitcoin Core and other software that connects to the bitcoin P2P network.
If you follow those pointers, you will find recent and active discussions around this topic.
(https://github.com/bitcoin/bitcoin/issues/31799#issuecomment-2636157846)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
Network-wide consensus and/or P2P changes first need to be discussed with the greater ecosystem, for example https://groups.google.com/g/bitcoindev, or https://delvingbitcoin.org/. Also, they need a BIP to be implemented in Bitcoin Core and other software that connects to the bitcoin P2P network.
If you follow those pointers, you will find recent and active discussions around this topic.
💬 polespinasa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1942518250)
I think you're right, this seems like a rare edge case.
If I'm understanding correctly, the issue only arises if `pindex` is an old block (not the actual tip) and we also have no good headers available. In that scenario, the function would return 1.0 due to a lack of visibility beyond pindex.
I think the only way this happens is if a node just reconnected after being offline or is somehow isolated from the network. In that case, it might briefly estimate 1.0 until it receives newer headers.
...
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1942518250)
I think you're right, this seems like a rare edge case.
If I'm understanding correctly, the issue only arises if `pindex` is an old block (not the actual tip) and we also have no good headers available. In that scenario, the function would return 1.0 due to a lack of visibility beyond pindex.
I think the only way this happens is if a node just reconnected after being offline or is somehow isolated from the network. In that case, it might briefly estimate 1.0 until it receives newer headers.
...
💬 maflcko commented on issue "test: 32-bit Clang `ipc_test` failure at `-O0`":
(https://github.com/bitcoin/bitcoin/issues/31772#issuecomment-2636229967)
> This bug seems like it is is probably a compiler bug, but not one is necessarily going to happen reliably because if the unitialized memory location started of as 0 instead 1497966205, crash would not happen. So I'm not sure switching bitcoin compiler from clang to gcc really fixes the problem, or just makes it appear not to happen. And I"m not sure what I was doing before that caused the problem to disappear as well. It seems like there might be just be a problem with this version of gcc and
...
(https://github.com/bitcoin/bitcoin/issues/31772#issuecomment-2636229967)
> This bug seems like it is is probably a compiler bug, but not one is necessarily going to happen reliably because if the unitialized memory location started of as 0 instead 1497966205, crash would not happen. So I'm not sure switching bitcoin compiler from clang to gcc really fixes the problem, or just makes it appear not to happen. And I"m not sure what I was doing before that caused the problem to disappear as well. It seems like there might be just be a problem with this version of gcc and
...
💬 l0rinc commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942550868)
```suggestion
// don't work. This makes it possible to use the std::span constructor in a SFINAE
```
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942550868)
```suggestion
// don't work. This makes it possible to use the std::span constructor in a SFINAE
```
💬 maflcko commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942562336)
Thx, happy to push, but I'll wait until you are done with your review until you reached an ack-state before pushing again. Otherwise, I'll just spend cycles on finding the right commit to fixup the unrelated typo and then pushing it for each typo/nit separately. It is also easier for reviewers to place a single ack and a single re-ack, instead of re-acks for each typo individually.
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942562336)
Thx, happy to push, but I'll wait until you are done with your review until you reached an ack-state before pushing again. Otherwise, I'll just spend cycles on finding the right commit to fixup the unrelated typo and then pushing it for each typo/nit separately. It is also easier for reviewers to place a single ack and a single re-ack, instead of re-acks for each typo individually.
💬 laanwj commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1942568100)
> if pindex is an old block (not the actual tip) and we also have no good headers available. In that scenario, the function would return 1.0 due to a lack of visibility beyond pindex.
Maybe that's how it is used at the moment, but the documentation of the function implies that the estimate can be used on any block, not that pindex necessarily is the latest block.
> I think the only way this happens is if a node just reconnected after being offline or is somehow isolated from the network.
...
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1942568100)
> if pindex is an old block (not the actual tip) and we also have no good headers available. In that scenario, the function would return 1.0 due to a lack of visibility beyond pindex.
Maybe that's how it is used at the moment, but the documentation of the function implies that the estimate can be used on any block, not that pindex necessarily is the latest block.
> I think the only way this happens is if a node just reconnected after being offline or is somehow isolated from the network.
...
💬 l0rinc commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942588417)
I posted this again since I saw that the build was failing, thought you'll need to push soon anyway.
----
Will the [copyright changes](https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1941965691) remain part of this PR? I would prefer reviewing that separately, but if you insist, I can do it here as well.
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942588417)
I posted this again since I saw that the build was failing, thought you'll need to push soon anyway.
----
Will the [copyright changes](https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1941965691) remain part of this PR? I would prefer reviewing that separately, but if you insist, I can do it here as well.
💬 egruper commented on issue " Cannot figure out how to use std::atomic error for MacOS Sequoia 15.2":
(https://github.com/bitcoin/bitcoin/issues/31579#issuecomment-2636307637)
> I cannot reproduce the issue on my MacOS Sequoia 15.2 (24C101):
>
> ```
> % clang --version
> Apple clang version 16.0.0 (clang-1600.0.26.6)
> Target: arm64-apple-darwin24.2.0
> Thread model: posix
> InstalledDir: /Library/Developer/CommandLineTools/usr/bin
> % cmake -B build -DWITH_BDB=ON
> <snip>
> -- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC
> -- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC - Success
> <snip>
> ```
>
> Can anyone else provide th
...
(https://github.com/bitcoin/bitcoin/issues/31579#issuecomment-2636307637)
> I cannot reproduce the issue on my MacOS Sequoia 15.2 (24C101):
>
> ```
> % clang --version
> Apple clang version 16.0.0 (clang-1600.0.26.6)
> Target: arm64-apple-darwin24.2.0
> Thread model: posix
> InstalledDir: /Library/Developer/CommandLineTools/usr/bin
> % cmake -B build -DWITH_BDB=ON
> <snip>
> -- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC
> -- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC - Success
> <snip>
> ```
>
> Can anyone else provide th
...
👍 laanwj approved a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2595254264)
re-ACK e3622a969293feea75cfadc8f7c6083edcd6d8de
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2595254264)
re-ACK e3622a969293feea75cfadc8f7c6083edcd6d8de
💬 hebasto commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2636340086)
> master currently does not have this issue.
However, the `cs_main` object is duplicated:
```
$ git rev-parse HEAD
94ca99ac51dddbee79d6409ebcc43b1119b0aca9
$ objdump -t build/src/bitcoind | grep cs_main$
0000000000b9d220 g O .data 0000000000000028 cs_main
$ objdump -t build/src/kernel/libbitcoinkernel.so | grep cs_main$
00000000003dc0a0 g O .data 0000000000000028 cs_main
```
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2636340086)
> master currently does not have this issue.
However, the `cs_main` object is duplicated:
```
$ git rev-parse HEAD
94ca99ac51dddbee79d6409ebcc43b1119b0aca9
$ objdump -t build/src/bitcoind | grep cs_main$
0000000000b9d220 g O .data 0000000000000028 cs_main
$ objdump -t build/src/kernel/libbitcoinkernel.so | grep cs_main$
00000000003dc0a0 g O .data 0000000000000028 cs_main
```
✅ fanquake closed an issue: "macOS 13.7 depends build can't find qt (symlink)"
(https://github.com/bitcoin/bitcoin/issues/31050)
(https://github.com/bitcoin/bitcoin/issues/31050)
🚀 fanquake merged a pull request: "depends: Avoid hardcoding `host_prefix` in toolchain file"
(https://github.com/bitcoin/bitcoin/pull/31358)
(https://github.com/bitcoin/bitcoin/pull/31358)
🚀 fanquake merged a pull request: "test: added additional coverage to waitforblock and waitforblockheight rpc's"
(https://github.com/bitcoin/bitcoin/pull/31784)
(https://github.com/bitcoin/bitcoin/pull/31784)