Bitcoin Core Github
42 subscribers
124K links
Download Telegram
💬 laanwj commented on pull request "init: Split file path handling out of -asmap option":
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3431948513)
Concept ACK. Splitting it to a separate filename option makes sense. imo it's better to avoid overloading the type (boolean or path) of options and introducing ambiguity. This is conceptually simpler for the user and simplifies the code.
💬 w0xlt commented on pull request "Add libbitcoinkernel example files":
(https://github.com/bitcoin/bitcoin/pull/33669#issuecomment-3431972960)
I wasn’t aware of the Charlatan’s website, but that’s literally the idea behind this PR — the user shouldn’t need to look for any external resources.
Also, many other Bitcoin libraries use the `examples` folder pattern.

That said, I’m not opposed to having these examples in another place.
💬 laanwj commented on issue "[flatpak] - I cannot choose or change datadir when running from flatpak":
(https://github.com/bitcoin/bitcoin/issues/24612#issuecomment-3431981297)
> Allow flatpak to use the new data folder:
>
> ```
> > flatpak override --user --filesystem=/my/new/folder/for-btc-data org.bitcoincore.bitcoin-qt
> ```

This is the correct solution. Flatpak applications are sandboxed, and there's nothing bitcoin-qt can do to reach outside that, unless given explicit permission.

i would guess that after giving it permission, as above, you can also just use the qt chooser dialog to pick the datadir, so that it gets picked automatically in the future:

```
> f
...
💬 laanwj commented on issue "[flatpak] - I cannot choose or change datadir when running from flatpak":
(https://github.com/bitcoin/bitcoin/issues/24612#issuecomment-3432030077)
> I don't know much about the flatpak, but I'm guessing the "flatpak run" command is invoking bitcoin-qt with a hardcoded -datadir= argument that preempts the -choosedatadir argument:

Looks like you're right. Flatpak creates a wrapper script that simply passes `-datadir` unconditionally.
https://github.com/flathub/org.bitcoincore.bitcoin-qt/blob/master/install_wrappers.sh

There would be multiple ways to solve this. One way to fix it without changes to this repository would be to adapt the scri
...
📝 maflcko opened a pull request: " ci: Retry image building once on failure "
(https://github.com/bitcoin/bitcoin/pull/33677)
This should fix https://github.com/bitcoin/bitcoin/issues/33640.

It also contains a few refactor cleanups, which are explained in the corresponding commits.
💬 l0rinc commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432095621)
I don't really see any speedup for `AssembleBlock`
<img width="679" height="476" alt="image" src="https://github.com/user-attachments/assets/fc804b80-241b-446f-bfc1-28b222d3190e" />

Change: gcc=-0.71%, clang=-0.82%

------

`CCoinsCaching` does seem to be improved a bit
<img width="664" height="477" alt="image" src="https://github.com/user-attachments/assets/fc248746-1716-41b1-ae0c-ac08b6c3dbd5" />

Change: gcc=+18.87%, clang=+11.79%


------


<details>
<summary>Benchmark</sum
...
💬 laanwj commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3432096233)
Concept ACK
👋 ismaelsadeeq's pull request is ready for review: "interfaces: enable cancelling running `waitNext` calls"
(https://github.com/bitcoin/bitcoin/pull/33676)
💬 cedwies commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432164685)
> > Not so sure about the `Assemble Block` bench though. Doesn't it measure block assembly **_after_** admission, therefore not timing the policy checks you changed?
>
> Thanks for the review & benchmarks. I'm positive about the fact that the `AssembleBlock` bench calls some of the impacted functions. I've not delved into details, and I think it's irrelevant considering performance is the same.

It _does_ call code on the policy path, but only during setup, not during the timed region.
In
...
💬 maflcko commented on issue "ci: short read: expected xxxxxxxxx bytes but got xxxxxxxxx: unexpected EOF":
(https://github.com/bitcoin/bitcoin/issues/33640#issuecomment-3432178761)
Similar variation: https://github.com/bitcoin/bitcoin/actions/runs/18713760603/job/53368943173#step:9:165:

```
Building ci_native_msan image tag to run in
+ docker buildx build --file=/home/admin/actions-runner/_work/bitcoin/bitcoin/ci/test_imagefile --build-arg=CI_IMAGE_NAME_TAG=mirror.gcr.io/ubuntu:24.04 --build-arg=FILE_ENV=./ci/test/00_setup_env_native_msan.sh --build-arg=BASE_ROOT_DIR=/home/admin/actions-runner/_work/_temp --platform=linux --label=bitcoin-ci-test --tag=ci_native_msan --cac
...
💬 Raimo33 commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432193066)
@l0rinc @cedwies the bench is not presented as evidence. it is presented as a transparent way of saying performance hasn't degraded. the PR body clearly states that it remained the same. I included it for the sake of transparency.
💬 waketraindev commented on pull request "addrman, net: filter during address selection via AddrPolicy to avoid underfill":
(https://github.com/bitcoin/bitcoin/pull/33663#issuecomment-3432260201)
* updated PR description and commit message to highlight P2p getaddr responses
* corrected formatting
💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3432270963)
Looks like the CI re-run passed after one month, so it seems reasonable stable
💬 cedwies commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432352303)
> But the code seems more complicated than before - is there a way to retain the speedup without complicating the code (i.e. minimizing the diff)?

It is more complicated. I would:

Keep:
- change from index-based to range-based loops.
- In `AreInputsStandard(...)`: Move `vSolutions` outside the for loop (this brings the performance gain in `CCoinsCaching`)
- In `IsWitnessStandard(...)`: Keep the variable name change from `stack` to `script_stack`

Discard (to minimize diff):
- In `Ar
...
💬 furszy commented on pull request "net: introduce block tracker to retry to download blocks after failure":
(https://github.com/bitcoin/bitcoin/pull/27837#issuecomment-3432381897)
closing for now, will re-open after rebasing it.
furszy closed a pull request: "net: introduce block tracker to retry to download blocks after failure"
(https://github.com/bitcoin/bitcoin/pull/27837)
furszy closed a pull request: "wallet: fix crash during migration due to invalid multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31378)
💬 furszy commented on pull request "wallet: fix crash during migration due to invalid multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-3432386506)
closing for now. Not sure if I will ever back to work on this PR.
💬 furszy commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#issuecomment-3432388684)
Probably still useful but closing for now due to lack of availability.
furszy closed a pull request: "descriptors: inference process, do not return unparsable multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31404)
📝 furszy opened a pull request: "http: limit RPC server threads to available cores"
(https://github.com/bitcoin/bitcoin/pull/33678)
The HTTP server currently spawns 16 threads by default, regardless of the system’s available ones.
This change limits the number of threads in the RPC server to the number of available CPUs,
preventing excessive context switching and improving overall responsiveness on systems with
fewer cores. If `-rpcthreads` exceeds that limit, it is adjusted to num_cores - 1 and a warning is
logged.