Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸš€ fanquake merged a pull request: "contrib: [tracing] fix pointer argument handling in mempool_monitor.py"
(https://github.com/bitcoin/bitcoin/pull/33086)
πŸ’¬ fanquake commented on pull request "contrib: [tracing] fix pointer argument handling in mempool_monitor.py":
(https://github.com/bitcoin/bitcoin/pull/33086#issuecomment-3131654211)
Backported to `29.x` in #33074.
πŸ’¬ moonfury-ops commented on issue "Bitcoin Kernel Library Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27587#issuecomment-3131701221)
Hello @TheCharlatan

I’m particularly interested in the following areas:

API design and language bindings β€” I’d be happy to contribute or test bindings for additional environments such as JavaScript, Kotlin, or Dart.
Read-only clients and context-minimal validation β€” These directions align well with lightweight and alternative Bitcoin client use cases. Abstracting data storage and validation dependencies is a meaningful step toward broader adoption.
Standalone repository plans β€” The goal of ma
...
πŸ’¬ moonfury-ops commented on issue "kernel: feedback on using kernel in alternative implementations":
(https://github.com/bitcoin/bitcoin/issues/31878#issuecomment-3131724379)
Hello @Davidson-Souza

The need for a context-minimal, SANS-IO validation interface is especially compelling.
Being able to reuse Bitcoin Core's consensus logic without depending on its internal database (e.g., leveldb-backed UTXO set) would enable a wide range of innovative projects to maintain compatibility and safety without needing to reimplement fragile or underspecified parts of the protocol.
Your suggestion of a middle-ground approachβ€”a separate module or mode under the libbitcoinkernel
...
πŸ‘ TheCharlatan approved a pull request: "kernel: create monolithic kernel static library"
(https://github.com/bitcoin/bitcoin/pull/33077#pullrequestreview-3066726770)
ACK fdbade6f8ded63519b637c8fa6f39914a400ab5e

The alternative of requiring upstream consumers to use cmake to import the library topology into their projects seems a bit heavy handed to me.
πŸ’¬ fanquake commented on pull request "depends: hard-code necessary c(xx)flags rather than setting them per-host":
(https://github.com/bitcoin/bitcoin/pull/32584#issuecomment-3131746003)
@ryanofsky made some changes to try and address your feedback.
πŸ“ fanquake opened a pull request: "doc: move `cmake -B build -LH` up in Unix build docs"
(https://github.com/bitcoin/bitcoin/pull/33088)
#32269 rebased.

> I had trouble building bitcoin core the way I wanted since now more features require a flag while building. IMO it makes sense to make it a bit more prominent in the build docs how to get the needed flags.

> Related issue: https://github.com/bitcoin/bitcoin/issues/32258
βœ… fanquake closed a pull request: "doc: update unix build doc with build flags"
(https://github.com/bitcoin/bitcoin/pull/32269)
πŸ’¬ fanquake commented on pull request "doc: update unix build doc with build flags":
(https://github.com/bitcoin/bitcoin/pull/32269#issuecomment-3131921976)
Rebased this is in #33088.
πŸ€” danielabrozzoni reviewed a pull request: "cli: return local services in -netinfo"
(https://github.com/bitcoin/bitcoin/pull/31886#pullrequestreview-3066765486)
tACK 943ee6768d9f94da70aaf90d346d4cf2ea1fa39d

I left a micro nit, feel free to ignore if you don't need to retouch
πŸ’¬ danielabrozzoni commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2239320315)
nit: add local services to the comment
πŸ‘ ryanofsky approved a pull request: "depends: hard-code necessary c(xx)flags rather than setting them per-host"
(https://github.com/bitcoin/bitcoin/pull/32584#pullrequestreview-3067012972)
Code review ACK 9954d6c833381a44e1ea34d182ffe4d61b65d2ba. No changes since last review other than improving the commit message. Change overall makes sense because it deduplicates host definitions, stops dropping `-std` flags from package builds when custom CFLAGS/CXXFLAGS environment variables are set, and stops passing duplicate flags to cmake that have no effect.
πŸ‘‹ sdaftuar's pull request is ready for review: "[WIP] Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676)
πŸ’¬ sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3132158578)
> I fuzzed this branch ([3bfaedb](https://github.com/bitcoin/bitcoin/commit/3bfaedbd7d9d6e71e9df03fbbb51a33c6184cca6)) with [fuzzamoto](https://github.com/dergoegge/fuzzamoto) and it found an assertion crash in `CTxMemPool::check`.

@dergoegge @instagibbs Thank you both for detecting and pinpointing the bug! Should be fixed now in this branch.
πŸ’¬ purpleKarrot commented on pull request "kernel: create monolithic kernel static library":
(https://github.com/bitcoin/bitcoin/pull/33077#issuecomment-3132165160)
For https://habla.news/u/purplekarrot.net/cmake-import-export, I have set up several example projects for exporting cmake packages and I deliberately mismatched consumers by using gcc, clang, and msvc.

I am making the bold claim that there is no working alternative to shipping a shared library with cmake package information. Consuming a static library requires a consistent toolchain. `.pc` files are not reloatable.

I will happily remove my NACK once I am proven wrong with a counterexample.
...
πŸ‘ stickies-v approved a pull request: "doc: move `cmake -B build -LH` up in Unix build docs"
(https://github.com/bitcoin/bitcoin/pull/33088#pullrequestreview-3067205611)
ACK 6757052fc439bedd1fa88ee1d23b4f17cf2c4f7a

For all other platforms, this configure help lives close to the `cmake --build build` command, which makes most sense to me. I did note that `build-windows.md` and `build-windows-msvc.md` documentation does not provide the `cmake -B build -LH` help at all. If relevant, could be added in this PR? No blocker.

Example diff (untested):

<details>
<summary>git diff on 6757052fc4</summary>

````diff
diff --git a/doc/build-windows-msvc.md b/doc/b
...
πŸ’¬ enirox001 commented on issue "build: Build warnings from deprecated std::get_temporary_buffer in std::stable_sort calls":
(https://github.com/bitcoin/bitcoin/issues/33080#issuecomment-3132302364)
Thanks for looking into this. To clarify - I'm actually using Clang 20.1.8, not GCC. The GCC paths in the warnings are because Clang uses the system's GCC standard library headers on Linux.

Exact reproduction steps:
1. `cmake --preset=libfuzzer`
2. `cmake --build build_fuzz`

System: Ubuntu 22.04, Clang 20.1.8

The warnings appear specifically with this libfuzzer preset configuration. If this is a Clang-specific issue or configuration-specific, feel free to close if not worth addressing.
πŸ’¬ sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3132351636)
> Follow-up from a while back: let's just explicitly say which layer of txgraph we're querying: [instagibbs@bb48f6c](https://github.com/instagibbs/bitcoin/commit/bb48f6c4736c227bcee6c4dda8e95b0b0287cfef)

@instagibbs Done, should be fixed in my last push.
πŸ‘ brunoerg approved a pull request: "doc: add note for watch-only wallet migration"
(https://github.com/bitcoin/bitcoin/pull/32866#pullrequestreview-3067374697)
reACK 5888b4a2a5566c64141b78a0e7660a166ec99775

minimal changes since my last review.
πŸ’¬ maflcko commented on issue "build: Build warnings from deprecated std::get_temporary_buffer in std::stable_sort calls":
(https://github.com/bitcoin/bitcoin/issues/33080#issuecomment-3132392576)
This warning is harmless and can be ignored. It should go away the next time you upgrade your Ubuntu (and thus the GCC package and std lib). Alternatively, you can use libc++ instead.