Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3384371268)
Updated 4fce466d1a3e345836434f8fb375980f626981f1 -> 2a80cd9f1db45dfc1775ded6698b034691e7bf2d ([kernelApi_71](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_71) -> [kernelApi_72](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_72), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_71..kernelApi_72))

* Fixed windows CI errors by excluding bitcoin-chainstate and test_kernel from manifest checks.
* Added `inline` to logging function definitions in the wrapper.
💬 maflcko commented on pull request "ci: Properly include $FILE_ENV in DEPENDS_HASH":
(https://github.com/bitcoin/bitcoin/pull/33581#issuecomment-3384392670)
Nice catch. Tested via:

```
sh-5.2$ ( export FILE_ENV="./ci/test/00_setup_env_win64.sh" && git ls-tree HEAD depends "ci/test/$FILE_ENV" | sha256sum | cut -d' ' -f1 )
9e598ae5c5f89396b330088c224cd5e1b391f8d101906c0270a48fa3acbdce07

sh-5.2$ ( export FILE_ENV="./ci/test/00_setup_env_win64.sh" && git ls-tree HEAD depends "$FILE_ENV" | sha256sum | cut -d' ' -f1 )
6e4783c2f98f56e05e4b0d535b1709fce7c5f318a67932b0c8ed7a34b0a7ffbb
```

First the erroneous hash, and then the intended hash, bot
...
💬 Sjors commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2415867744)
We also recommend this for the verification process and I find it much more convenient than looking up the full URL. I haven't run out of refs :-)
👍 hodlinator approved a pull request: "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions"
(https://github.com/bitcoin/bitcoin/pull/33569#pullrequestreview-3317673822)
crACK ec2476c8115e249ddfa3c8a1e8c892d5f822167b
💬 hodlinator commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2415901630)
Thanks!

Nice mention regarding clang-tidy's misc-throw-by-value-catch-by-reference allowing for throwing & catching string literals.

### ++nit

Now that there is a proposed fix in https://github.com/llvm/llvm-project/pull/162546, we can see that although the parent issue to this one (#33550) is about libc++, that is not the case here. Going by the llvm-project PR, the issue is in the Clang compiler's code generation (when compiling for Windows which uses SEH).

Commit message could be made mor
...
💬 hodlinator commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2415923655)
meganit: Uses `()` in 3 `throw`s and `{}` in 3.
💬 rkrux commented on pull request "wallet: Store transactions in a separate sqlite table":
(https://github.com/bitcoin/bitcoin/pull/33034#issuecomment-3384679811)
Concept ACK a25557a

I have gone through the PR description and have skimmed over the commit messages as of now. At the outset, I find myself agreeing with the intent here. I am not aware of a strong enough reason to keep using SQLite as a KVS for all purposes, using its relational database properties for wallet transactions seem like a good start.
I did notice some serialisation specifics of transaction properties in a couple previous PRs that I didn't fully understand why were required; it
...
⚠️ fanquake opened an issue: "test: pycapnp doesn't support free threaded Python"
(https://github.com/bitcoin/bitcoin/issues/33582)
Python 3.14.0 has been released, and ["Free-threaded Python is officially supported"](https://docs.python.org/3/whatsnew/3.14.html#whatsnew314-free-threaded-now-supported). However using Python `3.14.0t` and running the functional tests results in a failure in `interface_ipc.py`:
```bash
./build/test/functional/test_runner.py interface_ipc.py
Temporary test directory at /var/folders/sq/z88fhjzj0b19ftsd2_bjrmjm0000gn/T/test_runner_₿_🏃_20251009_094704
Remaining jobs: [interface_ipc.py]
1/1 - inte
...
💬 fanquake commented on issue "test: pycapnp doesn't support free threaded Python":
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3384824516)
cc @Sjors @ryanofsky
🤔 stratospher reviewed a pull request: "net_processing: rename RelayTransaction to better describe what it does"
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3317916661)
ACK 44a7261.
💬 maflcko commented on issue "test: pycapnp doesn't support free threaded Python":
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3384980563)
I'd expect there is a bunch of our own test code which relies on the GIL. I guess there is no way to find such "unsafe" Python code other than to try to run with the GIL disabled and wait for an intermittent issue to pop up at some point in time?
💬 optout21 commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2416151924)
nit: This declaration of `outpoint` is enough to be placed in the `else` branch

``` diff
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index 3928120a3f..055c3c4b31 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -59,13 +59,12 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
if (random_coin.IsSpent()) {
return;
}
- COutPoin
...
📝 Aathish101 opened a pull request: "Update .style.yapf"
(https://github.com/bitcoin/bitcoin/pull/33583)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 Aathish101 commented on pull request "Update .style.yapf":
(https://github.com/bitcoin/bitcoin/pull/33583#issuecomment-3384994424)
_updated_
💬 optout21 commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#issuecomment-3385012657)
ACK c4293c7b21971e7ddd813ad3fbe574eb5605d60e

With the recent commit restructuring the steps are clear.
The code change is sound and looks well tested.
I've left one non-blocking nit, no other comment.
💬 maflcko commented on issue "test: pycapnp doesn't support free threaded Python":
(https://github.com/bitcoin/bitcoin/issues/33582#issuecomment-3385035667)
Generally, I'd say we should continue to treat the no-GIL as experimental, possibly accept fixes where they make sense. However, longer term, I wonder what the goal should be. According to https://peps.python.org/pep-0779/#rationale there is a performance and memory overhead of the no-GIL version. Also, the test code is largely single threaded (at least logic wise), with sync and wait loops put basically after every non-sync statement. So my guess is that, if we wanted to make meaningful use of
...
maflcko closed a pull request: "Update .style.yapf"
(https://github.com/bitcoin/bitcoin/pull/33583)
💬 maflcko commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2416218945)
nit: I guess a more minimal one-line temporary(?) workaround would be to just add `&` here? Ref https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3381846204

```suggestion
BOOST_CHECK_EXCEPTION(CheckNumFormatSpecifiers<WrongNumArgs>(wrong_fmt), const char*&, HasReason{error});
```

> I don't mind adding such a check, but it seemed to me contrib/devtools/bitcoin-tidy isn't actively developed.

In theory we could add a tidy check for this, but I wonder how much it is worth i
...
💬 maflcko commented on pull request "doc: bump the template macOS version since 14 is now the minimum supported version":
(https://github.com/bitcoin/bitcoin/pull/33573#discussion_r2416237758)
```suggestion
placeholder: e.g. "MacOS 26.0" or "Ubuntu 26.04 LTS"
```

nit: Those are just placeholders, so the value doesn't really matter. Though, if updating them, it may be best to update them in such a way that they won't need to be touched again for the longest time.

I understand that Ubuntu 26.04 doesn't exist yet, but there will be a daily ISO starting next month, which seems good enough.
maflcko closed a pull request: "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator{h,cpp}`"
(https://github.com/bitcoin/bitcoin/pull/33218)