Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2006272875)
The [latest force-push](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_29..kernelApi_30) broke this logic by leaving `m_chainparams` and `m_notifications` uninitialized if `options` is non-nullptr, but the respective options members are nullptr.

Suggested fix:

<details>
<summary>git diff on 2dc27e2860</summary>

```diff
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
index 0cb2d69cec..1e6c582357 100644
--- a/src/kernel/bitcoinkernel.cpp
+++ b/src/
...
💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2000864918)
typo nit (+ in 2 other log functions)
```suggestion
* all existing `kernel_LoggingConnection instances.
```
💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2006290487)
What is the rationale behind requiring to first add, and then {enable,disable} the category? An alternative would be a single `kernel_set_log_level_category`, which immediately "enables" (I think it's a strange term anyway) the category at the given level. "Disabling" would be achieved by calling `kernel_set_log_level_category` again with a higher (i.e. less granular) level, again taking effect immediately.

I can't think of any use cases that require separating this in 3 functions? I think it
...
💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2006309594)
> I don't think we need to rely on the order inside the enumeration here

I don't think it's required, but it is slightly convenient when they are? E.g. when implementing `py-bitcoinkernel`'s logging, being able to rely on the order of `kernel_LogLevel` helps the implementation a bit (and it is also how these level enums are usually implemented in most logging libraries, I think). As one example, I think using the same values as the python `logging` library could be sensible: https://docs.pyth
...
💬 willcl-ark commented on issue "b-msghand invoked oom-killer: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2741665911)
I may take a stab at trying to reproduce this to see if it's still a problem with master today (unless you fancy doing so).

Just a few questions first:

> Configuration is just -txindex=1

- Their (basic) 8GB droplets appear to have 160 GiB (SSD), so I presume you also had pruning on?
- Your logs show at least `bench` and `validation` debug levels, did you use `-debug=1`, or only these (or only these on a subsequent run to debug a crash)?
- Am I likely to need to sync to ~ `height=814565` to se
...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2006471733)
Thanks! I also added a regression test. Sorry for not catching this earlier!
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2741688322)
Updated 2dc27e2860b97c2bffa5f18706917b21858e5594 -> 9fc6accf89ed001f70e107a8e9936f6dc3a35f41 ([kernelApi_30](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_30) -> [kernelApi_31](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_31), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_30..kernelApi_31))

* Addressed @stickies-v's [comment](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2000864918), fixed naming in docstring for `kernel_LoggingConnection`.
...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2006477122)
This should just mirror the internal code at the moment, but I agree that it is not really useful to split this up. Will see if I can consolidate this.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2006504759)
It seems there is an issue with older CMake versions. I was able to reproduce it on Ubuntu `x86_64` while compiling natively:
```
$ cmake --version
cmake version 3.22.6

CMake suite maintained and supported by Kitware (kitware.com/cmake).
$ gmake -C depends -j 16 qt
<snip>
Building qt...
ninja: error: 'qtbase/src/platformsupport/input/CMakeFiles/InputSupportPrivate.dir/cmake_pch.hxx.gch', needed by 'qtbase/src/platformsupport/input/CMakeFiles/InputSupportPrivate.dir/InputSupportPrivate_
...
⚠️ yellowred opened an issue: "Add a script for Signet or Regtest to unlock fee estimator"
(https://github.com/bitcoin/bitcoin/issues/32105)
### Please describe the feature you'd like to see added.

Currently, the fee estimator won't work in a private signet or regtest network, because there are usually not enough transactions being added. This forces clients to hardcode a default fee or have some custom logic for these networks.
The proposal is to add a script to /contrib/signet that would simulate a transactions volume just enough to allow `estimatesmartfee` to work.
It would be useful to have it in the bitcoin core repo, because:

...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2741785465)
Rebased 9fc6accf89ed001f70e107a8e9936f6dc3a35f41 -> 29f05b91cf8a479e403b0322afeb5ff1133da221 ([kernelApi_31](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_31) -> [kernelApi_32](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_32), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_31..kernelApi_32))

* Fixed silent merge conflict with #31519
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2006536581)
The problem occurs with CMake versions older than 3.25.0.
💬 willcl-ark commented on issue "Source code mapping for debugger has changed since cmake":
(https://github.com/bitcoin/bitcoin/issues/31204#issuecomment-2741816753)
This seems to work for me on Linux without any `.lldbinit` file:

```bash
will@ubuntu in …/src/core/bitcoin on  master [$] via △ v3.31.6 : 🐍 (core)
$ just clean build-depends
depends build running
copying packages: boost libevent qt expat libxcb xcb_proto libXau xproto freetype fontconfig libxkbcommon libxcb_util libxcb_util_render libxcb_util_keysyms libxcb_util_image libxcb_util_wm qrencode bdb sqlite systemtap zeromq
to: /home/will/src/core/bitcoin/depends/x86_64-pc-linux-gnu
To build Bitco
...
💬 willcl-ark commented on pull request "ci: run test-each-commit on merge to master":
(https://github.com/bitcoin/bitcoin/pull/32103#issuecomment-2741873830)
> > I think what you want is to checkout each old commit and then merge it (inside the CI task) with master. Not sure how to achieve it otherwise. Maybe you can share your steps to reproduce with `git`?
>
> To clarify, my suggestion would be to modify the `git rebase --exec 'run_test' base` into `git rebase --exec 'git log -1 && git merge --no-commit master && echo run_test && git merge --abort' base`. However, I haven't tested this.

Would it not in that case be simpler to checkout the PR
...
💬 jimhashhq commented on issue "Add a `indexesdir` option to hold the indexes directory":
(https://github.com/bitcoin/bitcoin/issues/32099#issuecomment-2741897166)
Do you have `txindex` enabled or disabled?

When `-txindex` is _disabled_, the (internal/primary SSD) index storage requirements were only 2% of total blockchain size I thought.. Running instead _with_ `-txindex` would indeed bump internal/primary storage up to around 10-12%, which may be what you're seeing..

Don't know if it would help your situation, but have you tried running w/o `-txindex`?
pinheadmz closed an issue: "Bitcoin Core Config Generator"
(https://github.com/bitcoin/bitcoin/issues/32106)
💬 pinheadmz commented on issue "Bitcoin Core Config Generator":
(https://github.com/bitcoin/bitcoin/issues/32106#issuecomment-2741933825)
Whatever your issue is I think you need to open it in this repo: https://github.com/jlopp/bitcoin-core-config-generator/
🚀 fanquake merged a pull request: "fuzz: Use immediate task runner to increase fuzz stability"
(https://github.com/bitcoin/bitcoin/pull/31841)