Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 pablomartin4btc commented on pull request "Add new "address type" column to the "receiving tab" address book page":
(https://github.com/bitcoin-core/gui/pull/753#issuecomment-2626204992)
The [issue](https://github.com/bitcoin-core/gui/pull/753#issuecomment-2598800192) has been fixed (+ manually tested - transparent/ no impact on the use case).
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1936795545)
(and these two defaults are very unlikely to change).
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1936806097)
Good point. `BlockAssembler::CreateNewBlock()` uses `nFees` internally. It's updated in `AddToBlock`. I could expose it via `CBlockTemplate`.

However this code will hopefully go away soon(tm) with Cluster Mempool, and I suspect it's extremely fast. So it's perhaps not worth refactoring. In any case it could be done in a followup without changing the interface.
💬 Sjors commented on pull request "test: Added coverage to the waitfornewblock rpc":
(https://github.com/bitcoin/bitcoin/pull/31746#issuecomment-2626512279)
> it could be an argument for preferring multi line if-statements over single line ones

Maybe, but it wouldn't catch `?` branches either. You could propose a linter change that insists on multi line if-statements. Though I expect ... strong opinions :-)
👋 hebasto's pull request is ready for review: "ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176)
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2626571182)
Rebased on top of the merged https://github.com/bitcoin/bitcoin/pull/31428.
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2626845471)
re https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2408594619

> Taking a step back, I wonder what was the last real issue found by Wine, which wasn't found by the native Windows task? If there are none, or not too many, I think this could also be made a nightly CI task, only run on master after a merge (without caching), or in a completely separate repo. The same is done for other stuff, like the valgrind tasks, or the s390x tasks.

Done in https://github.com/hebasto/bitcoin
...
💬 ryanofsky commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2627030718)
At this point (edc1fd44fa0d78168847b199d44ae107e33861e4) it seems like there are 2 failures left. One is the 403 download failure [[1]](https://cirrus-ci.com/task/4569652367458304), [[2]](https://cirrus-ci.com/task/4851127344168960), [[3]](https://cirrus-ci.com/task/4710389855813632)

The other is the [clang-tidy failure](https://cirrus-ci.com/task/6715899064877056) which I'm in the middle of fixing (slowly, since there a lot of warnings so I wanted to figure out how to get clang-tidy and iwyu
...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2627079274)
Updated 01a43b24436e0aed7b8f79d3857630a4bf6a0545 -> 10e71b4c47e1b199622280d100155ed5d6ef6d66 ([kernelApi_18](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_18) -> [kernelApi_19](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_19), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_18..kernelApi_19))

* Hooked up kernel tests to cmake so the CI can run them through `ctest`
* Added `test_kernel`, `libbitcoinkernel.so`, and `kernel/bitcoin-chainstate` to some more
...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1937145120)
I added it to the cross compiled windows job now, but it is going to take extra work (#31158) to add it to the native job too.
💬 hebasto commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2627116832)
The "clang-tidy" CI job shouldn't process sources in the `src/ipc/libmultiprocess/example/` [directory](https://github.com/hebasto/bitcoin/actions/runs/13072023868/job/36476232260):
```
[07:11:14.250] [ 13/719][0.1s] clang-tidy-19 -p=/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/ipc/libmultiprocess/example/printer.capnp.proxy-client.c++
[07:11:14.250] error: no input files [clang-
...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2627119867)
Rebased 10e71b4c47e1b199622280d100155ed5d6ef6d66 -> f4cdffba0d8c60ff6e6eb9732bd1b1f02fcada56 ([kernelApi_19](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_19) -> [kernelApi_20](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_20), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_19..kernelApi_20))
💬 ryanofsky commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2627152623)
> The "clang-tidy" CI job shouldn't process sources in the `src/ipc/libmultiprocess/example/` [directory](https://github.com/hebasto/bitcoin/actions/runs/13072023868/job/36476232260)

Maybe not, but I didn't see an easy way to exclude them, so I thought I would just fix the clang-tidy errors. I fixed the missing inputs problem by just adding necessary targets back to all:

```diff
--- a/src/ipc/CMakeLists.txt
+++ b/src/ipc/CMakeLists.txt
@@ -9,6 +9,10 @@ if (NOT WITH_LIBMULTIPROCESS)

...
⚠️ GregTonoski opened an issue: "incrementalrelayfee configuration option should be present in bitcoin.conf"
(https://github.com/bitcoin/bitcoin/issues/31769)
### Please describe the feature you'd like to see added.

... just like minrelaytxfee and others. Ideally, bitcoin.conf should encompass all configuration options.

### Is your feature related to a problem, if so please describe it.

As a user, I would like to specify value of the incrementalrelayfee configuration option in bitcoin.conf file so that I don't have to type it in a command line each time I start bitcoind.

In Bitcoin 28.1, the option is hidden, i.e. there isn't any description of th
...
💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2627276197)
ARM, TSAN and no wallet passed this time. I didn't wait for MSAN and VS 2022 before pushing your next fix...
⚠️ fanquake opened an issue: "fuzz: oss-fuzz coverage build is failing"
(https://github.com/bitcoin/bitcoin/issues/31770)
It first failed on the 25th (https://oss-fuzz-build-logs.storage.googleapis.com/log-7da28be4-dcf5-4e23-a6ca-153f67ad2f60.txt):
```bash
Step #7: [0m [0;31mwarning: The file '/usr/evmap.c' isn't covered.
Step #7: [0m [0;31merror: /workspace/out/libfuzzer-coverage-x86_64/usr/arc4random.c: No such file or directory
Step #7: [0m [0;31mwarning: The file '/usr/arc4random.c' isn't covered.
Step #7: [0m [0;31merror: /workspace/out/libfuzzer-coverage-x86_64/usr/buffer.c: No such file or directory
Step
...
🤔 vasild reviewed a pull request: "Add waitNext() to BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/31283#pullrequestreview-2585936766)
Approach ACK 6bf77e5a37
💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937133582)
I think the loop should be `while (now < deadline)` (instead of `<=`) and this should not exist. But then I do not understand what this means: "because then there's no wait to test its internals, e.g. the 20 minute testnet exception". Can you elaborate?
💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1936835457)
nit: strictly speaking it does not return `nullptr` but an empty `std::unique_ptr` (which owns `nullptr`).
💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937123826)
When `options.timeout` is `MillisecondsDouble::max()` it looks odd to add to it. This seems to work because of the underlying `double` type which ends up being `inf` and adding to `inf` results in `inf`. But if that is changed to an integer type this will break (overflow). Maybe at least comment on `BlockWaitOptions::timeout` that it must be of `double` type because of this, or to have `BlockWaitOptions::timeout` be of type `std::optional` with an empty optional meaning "no timeout". Then `deadl
...