π¬ dergoegge commented on pull request "wip: Split fuzz binary (take 2)":
(https://github.com/bitcoin/bitcoin/pull/30882#issuecomment-2346596824)
Trying to produce an introspector report using this branch: https://github.com/dergoegge/oss-fuzz/tree/2024-09-bitcoin-introspector
(https://github.com/bitcoin/bitcoin/pull/30882#issuecomment-2346596824)
Trying to produce an introspector report using this branch: https://github.com/dergoegge/oss-fuzz/tree/2024-09-bitcoin-introspector
π¬ sr-gi commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#issuecomment-2346597866)
Amended to cover @maflcko nits and rebased to include cmake
(https://github.com/bitcoin/bitcoin/pull/30233#issuecomment-2346597866)
Amended to cover @maflcko nits and rebased to include cmake
π¬ hebasto commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757100406)
`CCACHE_BASEDIR=${CMAKE_BINARY_DIR}` is effective when building in different build directories from the same source directory.
`CCACHE_BASEDIR=${CMAKE_SOURCE_DIR}` works for building from different source directories.
We need to choose one of these two options.
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757100406)
`CCACHE_BASEDIR=${CMAKE_BINARY_DIR}` is effective when building in different build directories from the same source directory.
`CCACHE_BASEDIR=${CMAKE_SOURCE_DIR}` works for building from different source directories.
We need to choose one of these two options.
π ryanofsky approved a pull request: "util: Use consteval checked format string in FatalErrorf, LogConnectFailure"
(https://github.com/bitcoin/bitcoin/pull/30546#pullrequestreview-2300689204)
Code review ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678
Just test code cleanup since last review
(https://github.com/bitcoin/bitcoin/pull/30546#pullrequestreview-2300689204)
Code review ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678
Just test code cleanup since last review
π ryanofsky approved a pull request: "kernel: Create usable static kernel library"
(https://github.com/bitcoin/bitcoin/pull/30814#pullrequestreview-2300694088)
Code review ACK 0dd16d7118f10ac291a45644769121cbdfd2352f
Just cmake style improvements since last reveiw
(https://github.com/bitcoin/bitcoin/pull/30814#pullrequestreview-2300694088)
Code review ACK 0dd16d7118f10ac291a45644769121cbdfd2352f
Just cmake style improvements since last reveiw
π¬ davidgumberg commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2346605483)
@achow101 Reworking the patch that fixes disabling XOR, will open a PR later today if @sipa's [patch](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-09-12#1726151549-1726151530;) to remove the `ftell` that happens on the XOR path doesn't fix this issue.
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2346605483)
@achow101 Reworking the patch that fixes disabling XOR, will open a PR later today if @sipa's [patch](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-09-12#1726151549-1726151530;) to remove the `ftell` that happens on the XOR path doesn't fix this issue.
π€ furszy reviewed a pull request: "test: Wait for local services to update in feature_assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/30880#pullrequestreview-2300701020)
Code review ACK [19f4a7c](https://github.com/bitcoin/bitcoin/pull/30880/commits/19f4a7c95a99162122068d4badffeea240967a65).
There is another potential race condition at line 621, though itβs different one. This one wouldn't cause a failure if it occurs, as the node already has the prune node service flag set. The check there verifies that services remain unchanged after the background sync is completed for prune nodes. The worst-case scenario would be if the services are actually changed and w
...
(https://github.com/bitcoin/bitcoin/pull/30880#pullrequestreview-2300701020)
Code review ACK [19f4a7c](https://github.com/bitcoin/bitcoin/pull/30880/commits/19f4a7c95a99162122068d4badffeea240967a65).
There is another potential race condition at line 621, though itβs different one. This one wouldn't cause a failure if it occurs, as the node already has the prune node service flag set. The check there verifies that services remain unchanged after the background sync is completed for prune nodes. The worst-case scenario would be if the services are actually changed and w
...
π hebasto opened a pull request: "Revert "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`""
(https://github.com/bitcoin/bitcoin/pull/30883)
This reverts commit b07fe666f27e2b2515d2cb5a0339512045e64761.
Fixes https://github.com/bitcoin/bitcoin/issues/30881.
(https://github.com/bitcoin/bitcoin/pull/30883)
This reverts commit b07fe666f27e2b2515d2cb5a0339512045e64761.
Fixes https://github.com/bitcoin/bitcoin/issues/30881.
π fanquake merged a pull request: "kernel: Create usable static kernel library"
(https://github.com/bitcoin/bitcoin/pull/30814)
(https://github.com/bitcoin/bitcoin/pull/30814)
π¬ hebasto commented on issue "cmake: GenerateHeaderFrom very slow":
(https://github.com/bitcoin/bitcoin/issues/30881#issuecomment-2346638003)
> Reverting #30842 fixes the issue for me.
Done in https://github.com/bitcoin/bitcoin/pull/30883.
(https://github.com/bitcoin/bitcoin/issues/30881#issuecomment-2346638003)
> Reverting #30842 fixes the issue for me.
Done in https://github.com/bitcoin/bitcoin/pull/30883.
π¬ maflcko commented on pull request "kernel: Create usable static kernel library":
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1757134512)
q: Is there a reason why PACKAGE_NAME is used above and below, but not here?
(https://github.com/bitcoin/bitcoin/pull/30814#discussion_r1757134512)
q: Is there a reason why PACKAGE_NAME is used above and below, but not here?
π€ HumayunMhmadi requested changes to a pull request: "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"
(https://github.com/bitcoin/bitcoin/pull/30842#pullrequestreview-2300746284)
IMG_20240904_132132_468.jpg](https://github.com/user-attachments/assets/bf11748e-d475-4f4d-870c-6691730baba7)
(https://github.com/bitcoin/bitcoin/pull/30842#pullrequestreview-2300746284)
IMG_20240904_132132_468.jpg](https://github.com/user-attachments/assets/bf11748e-d475-4f4d-870c-6691730baba7)
π¬ ryanofsky commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757143565)
> `CCACHE_BASEDIR=${CMAKE_BINARY_DIR}` is effective when building in different build directories from the same source directory.
>
> `CCACHE_BASEDIR=${CMAKE_SOURCE_DIR}` works for building from different source directories.
>
> We need to choose one of these two options.
Thanks, that clarifies a lot. I only do the second not the first, because I use git worktrees, and I assumed this PR was trying to make git worktrees and multiple checkouts work better. Maybe people who work on build sy
...
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757143565)
> `CCACHE_BASEDIR=${CMAKE_BINARY_DIR}` is effective when building in different build directories from the same source directory.
>
> `CCACHE_BASEDIR=${CMAKE_SOURCE_DIR}` works for building from different source directories.
>
> We need to choose one of these two options.
Thanks, that clarifies a lot. I only do the second not the first, because I use git worktrees, and I assumed this PR was trying to make git worktrees and multiple checkouts work better. Maybe people who work on build sy
...
π¬ maflcko commented on pull request "Revert "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"":
(https://github.com/bitcoin/bitcoin/pull/30883#issuecomment-2346656947)
review ACK fdeb717e78fd55fbfda199c87002358bdc5895af
(https://github.com/bitcoin/bitcoin/pull/30883#issuecomment-2346656947)
review ACK fdeb717e78fd55fbfda199c87002358bdc5895af
π¬ fanquake commented on pull request "build: Revert "Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"":
(https://github.com/bitcoin/bitcoin/pull/30883#issuecomment-2346666009)
Going to merge this, given multiple developers have reported issues.
(https://github.com/bitcoin/bitcoin/pull/30883#issuecomment-2346666009)
Going to merge this, given multiple developers have reported issues.
β
fanquake closed an issue: "cmake: GenerateHeaderFrom very slow"
(https://github.com/bitcoin/bitcoin/issues/30881)
(https://github.com/bitcoin/bitcoin/issues/30881)
π fanquake merged a pull request: "build: Revert "Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`""
(https://github.com/bitcoin/bitcoin/pull/30883)
(https://github.com/bitcoin/bitcoin/pull/30883)
π¬ jonatack commented on pull request "memusage: let PoolResource keep track of all allocated/deallocated memory":
(https://github.com/bitcoin/bitcoin/pull/28939#discussion_r1757158064)
@martinus PR needs rebase/update to cmake. This makefile has been removed.
(https://github.com/bitcoin/bitcoin/pull/28939#discussion_r1757158064)
@martinus PR needs rebase/update to cmake. This makefile has been removed.
π sipa opened a pull request: "streams: cache file position within AutoFile"
(https://github.com/bitcoin/bitcoin/pull/30884)
This is intended to address #30833.
(https://github.com/bitcoin/bitcoin/pull/30884)
This is intended to address #30833.
π instagibbs approved a pull request: "test: switch MiniWallet padding unit from weight to vsize"
(https://github.com/bitcoin/bitcoin/pull/30718#pullrequestreview-2300766803)
ACK 7d1c8d0aa3ebe30d7633caf9bc4765927e4d6e08
seems straight forward to me, and all usages appear to be concerned with wallet/mempool things, not consensus related activities which may require weight-level precision.
For weight-level consensus precision I suspect wallet-like functionality is less interesting.
(https://github.com/bitcoin/bitcoin/pull/30718#pullrequestreview-2300766803)
ACK 7d1c8d0aa3ebe30d7633caf9bc4765927e4d6e08
seems straight forward to me, and all usages appear to be concerned with wallet/mempool things, not consensus related activities which may require weight-level precision.
For weight-level consensus precision I suspect wallet-like functionality is less interesting.