Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 hebasto commented on pull request "depends: Avoid using the `-ffile-prefix-map` compiler option":
(https://github.com/bitcoin/bitcoin/pull/31800#issuecomment-2637629490)
My Guix build:
```
aarch64
4ec7b4c22a31fa78161d7f826390e04a5ef50f04eba7c5e7eb5394166d613a16 guix-build-407062f2ac93/output/aarch64-linux-gnu/SHA256SUMS.part
7577f1d0847ab472e60b1d6e14e28ed442c36db190d1c214d41013b296581447 guix-build-407062f2ac93/output/aarch64-linux-gnu/bitcoin-407062f2ac93-aarch64-linux-gnu-debug.tar.gz
911518a3f02677a6a2ad0110c6b40b0e6f9789f30fa0f69229d39de54f565dfa guix-build-407062f2ac93/output/aarch64-linux-gnu/bitcoin-407062f2ac93-aarch64-linux-gnu.tar.gz
95858bf5
...
💬 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-2637644717)
Please give https://github.com/theuni/bitcoin/commits/pr31158/ a shot. It's not meant to be robust, but it should illustrate the fix (using dllimport for bitcoin-chainstate.exe).
💬 maflcko commented on pull request "miniscript: convert non-critical asserts to CHECK_NONFATAL":
(https://github.com/bitcoin/bitcoin/pull/31727#discussion_r1943443912)
Just for reference, `CHECK_NONFATAL` is safe (from an UB perspective) to be used as a drop-in replacement for `assert/Assert` (and vice-versa) with only a slight difference: An assertion failure will abort the whole program, and not continue to the next statement. A nonfatal check failure will also not continue to the next statement, but it will just throw an exception and not fatally abort the whole program.
👍 l0rinc approved a pull request: "feefrac: add support for evaluating at given size"
(https://github.com/bitcoin/bitcoin/pull/30535#pullrequestreview-2596619215)
Rebase didn't result in any code change, but CI compiles fuzzing for every commit now
💬 l0rinc commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#discussion_r1943451470)
Now that Fuzz is always compiled, this should likely be move to `feefrac: add helper functions for 96-bit division` (see https://github.com/bitcoin/bitcoin/actions/runs/13162889382/job/36735875466?pr=30535#step:6:2772)
💬 sipa commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#discussion_r1943467899)
Done.
💬 l0rinc commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#issuecomment-2637729330)
ACK 5600091e8384a69cfc7e95a7aea816e7a9076917

Build failure will likely be resolved by retriggering it later
💬 sipa commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#issuecomment-2637740773)
Added a few commit messages to explain the rationale for the changes more.
💬 ryanofsky commented on issue "test: 32-bit Clang `ipc_test` failure at `-O0`":
(https://github.com/bitcoin/bitcoin/issues/31772#issuecomment-2637754217)
> Interesting. It would be nice to reduce this

So I reduced the failure down to a standalone case, but it could be reduced further. Now it is unclear to me if there is a compiler bug here or not, because this is caused by a very specific interaction between the two compiler versions we are using:

- gcc version Ubuntu 13.3.0-6ubuntu2~24.04
- clang version 18.1.3 (1ubuntu1)

and maybe the specific flags we are using. The bug happens because the linker links together the

- `_ZN2kj4ctorIjIjEEEvRT
...
💬 l0rinc commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#issuecomment-2637792608)
ACK f6aa28cf8fad6a3240498b500524bb380855b18e

Added build failure to https://github.com/bitcoin/bitcoin/issues/31797#issuecomment-2637727127 - should be retriggered later
💬 ryanofsky commented on issue "test: 32-bit Clang `ipc_test` failure at `-O0`":
(https://github.com/bitcoin/bitcoin/issues/31772#issuecomment-2637794665)
Following seem to be minimal flags to reproduce this issue. None of the other flags except for `-fPIC` was making any difference, and without `-fPIC` the program just segfaults instead of printing the wrong value:

```bash
g++ -m32 -fPIC -c test_gcc.cpp -o test_gcc.o
clang++ -m32 -fPIC -c test_clang.cpp -o test_clang.o
clang++ -m32 -fPIC test_gcc.o test_clang.o
./a.out
```

So it seems like this version of gcc and clang just always (regardless of flags) assume incompatible calling conventions fo
...
💬 maflcko commented on pull request "wallet, rpc: deprecate settxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#issuecomment-2637796858)
I don't understand the changes here. Instead of repeating what the changes are in the pull description, it would be good to explain why they are done and why the downsides are acceptable.

I guess this deprecates an RPC without replacement, without release notes, without an explanation or long-term plan?

If yes, then I tend toward NACK.
💬 arejula27 commented on pull request "test: check `scanning` field from `getwalletinfo`":
(https://github.com/bitcoin/bitcoin/pull/31768#issuecomment-2637800811)
ACK `bb0879d`

The test looks good as it improves the verification of an RPC call field. However, the verified field produces different values depending on the scan situation:

> "scanning" : { (json object) current scanning details, or false if no scan is in progress>
>     "duration" : n, (numeric) elapsed seconds since scan start
>     "progress" : n (numeric) scanning pro
...
🤔 murchandamus reviewed a pull request: "feefrac: add support for evaluating at given size"
(https://github.com/bitcoin/bitcoin/pull/30535#pullrequestreview-2596789900)
Concept ACK, I also really like the idea of improving CFeeRate.

ACK up to 5147b5d602d1f55d7c9f497cf0cfc23263f5caf5, still mulling over the last three commits.
maflcko closed a pull request: "contrib: fix BUILDDIR in gen-bitcoin-conf script"
(https://github.com/bitcoin/bitcoin/pull/31332)
💬 maflcko commented on pull request "contrib: fix BUILDDIR in gen-bitcoin-conf script":
(https://github.com/bitcoin/bitcoin/pull/31332#issuecomment-2637843152)
Ok, closing for https://github.com/bitcoin/bitcoin/pull/31742
📝 theuni opened a pull request: "kernel: Avoid duplicating symbols in the kernel and downstream users"
(https://github.com/bitcoin/bitcoin/pull/31807)
The possibility of this happening came up in the discussion here: https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2635223309, and it turned that we already had a case of it in our code.

tl;dr: Certain symbols when defined in headers may end up existing in both the shared lib and the application using it, leading to subtle bugs. More info can be found [here](https://www.mail-archive.com/cfe-commits@lists.llvm.org/msg503669.html).

The solution is generally to avoid defining static
...
maflcko closed an issue: "bitcoind crash"
(https://github.com/bitcoin/bitcoin/issues/31712)
💬 maflcko commented on issue "bitcoind crash":
(https://github.com/bitcoin/bitcoin/issues/31712#issuecomment-2637856878)
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.

General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits, or any other place that you feel is well suited.