Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ sipa commented on issue "cmake: GenerateHeaderFrom very slow":
(https://github.com/bitcoin/bitcoin/issues/30881#issuecomment-2346542352)
Reverting #30842 fixes the issue for me.
πŸ’¬ fanquake commented on pull request "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`":
(https://github.com/bitcoin/bitcoin/pull/30842#issuecomment-2346542625)
Opened #30881 to track this, given a second dev has reported performance issues.
πŸ‘ fanquake approved a pull request: "kernel: Create usable static kernel library"
(https://github.com/bitcoin/bitcoin/pull/30814#pullrequestreview-2300616256)
ACK 0dd16d7118f10ac291a45644769121cbdfd2352f - this looks like a good place to start.
πŸ’¬ fanquake commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#issuecomment-2346575257)
https://github.com/bitcoin/bitcoin/pull/30866/checks?check_run_id=29955732214:
```bash
In file included from /ci_container_base/src/script/descriptor.cpp:10:
/ci_container_base/src/script/miniscript.h: In instantiation of β€˜miniscript::Node<Key> miniscript::Node<Key>::Clone() const [with Key = unsigned int]’:
/ci_container_base/src/script/descriptor.cpp:1363:124: required from here
/ci_container_base/src/script/miniscript.h:535:33: error: moving a local object in a return statement prevent
...
πŸ’¬ ryanofsky commented on pull request "interfaces: #30697 follow ups":
(https://github.com/bitcoin/bitcoin/pull/30828#discussion_r1757085271)
> I think the only thing that "bothers" me a bit about the nullable class is the different meanings it could have, which require additional clarifications at the API level.

Yeah this is real concern, and I think a common thing that happens with null values, true/false values, empty strings, and special integer values. If you are sloppy with these values you will run into problems. If you are thoughtful you can come up with very clean interfaces, but it is hard to be careful. You can avoid pr
...
πŸ“ dergoegge opened a pull request: "wip: Split fuzz binary (take 2)"
(https://github.com/bitcoin/bitcoin/pull/30882)
Closes https://github.com/bitcoin/bitcoin/issues/28971

In addition to the benefits listed in #28971, this should also enable us to use https://github.com/ossf/fuzz-introspector provided by oss-fuzz. Our current runtime harness selection blocks introspector's static analysis from working properly (e.g. it can't statically determine which functions are reachable by a given harness).

This PR uses the approach suggested here: https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-184620474
...
βœ… fanquake closed an issue: "Testnet fixed seeds don't work"
(https://github.com/bitcoin/bitcoin/issues/29574)
πŸ’¬ fanquake commented on issue "Testnet fixed seeds don't work":
(https://github.com/bitcoin/bitcoin/issues/29574#issuecomment-2346588334)
This was fixed.
πŸ’¬ dergoegge commented on pull request "wip: Split fuzz binary (take 2)":
(https://github.com/bitcoin/bitcoin/pull/30882#discussion_r1757096120)
Currently an individual `test_fuzz_*` lib and `fuzz_*` binary is produced for each harness. It's kind of ugly to duplicate this loop but I'm not sure to avoid it. Another loop would likely need to be added for the wallet harnesses as well.

@hebasto @fanquake @maflcko Any ideas?
πŸ’¬ sr-gi commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#discussion_r1757098852)
I moved it up in [07f4ceb](https://github.com/bitcoin/bitcoin/pull/30233/commits/07f4cebe5780f1039541d989e64b70eccc5b4eb5) and renamed the internal `peer` to `peer_ref` to avoid a conflict
πŸ’¬ sr-gi commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#discussion_r1757099028)
Addressed in [07f4ceb](https://github.com/bitcoin/bitcoin/pull/30233/commits/07f4cebe5780f1039541d989e64b70eccc5b4eb5)
πŸ’¬ 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
πŸ’¬ 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
πŸ’¬ 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.
πŸ‘ 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
πŸ‘ 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
πŸ’¬ 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.
πŸ€” 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
...
πŸ“ 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.
πŸš€ fanquake merged a pull request: "kernel: Create usable static kernel library"
(https://github.com/bitcoin/bitcoin/pull/30814)