💬 hebasto commented on pull request "doc: Update documentation to include Clang/llvm based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#issuecomment-2676143578)
cc @vasild

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2676164722)
Updated c72b2c2883d4c8791267133f326e3f9347d1520b -> 29513955891e40e78466f2c666dfa13e9c1b2914 ([kernelApi_26](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_26) -> [kernelApi_27](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_27), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_26..kernelApi_27))

* Cleaned up some dead code missed while removing the `kernel_ValidationInterface`.

📝 fanquake locked a pull request: "Update COPYING"
(https://github.com/bitcoin/bitcoin/pull/31935)
test

💬 maflcko commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2676190482)
I don't think it is (trivially) possible to have a CI check for the fallback code without running the CI on the erroneous chip. However, I am not sure if it is worth it to integrate (and maintain) the erroneous chip into the CI in this repo just to check code that is added and never modified (apart from maybe minor refactorings every couple of years). I wouldn't recommend to blindly trust CI anyway, so local testing of the feature should be done either way. (And if no one can be bothered to tes
...

💬 Prabhat1308 commented on pull request "doc: Update documentation to include Clang/llvm based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#discussion_r1966516634)
updated.

💬 Prabhat1308 commented on pull request "doc: Update documentation to include Clang/llvm based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#discussion_r1966516641)
added the newline

✅ maflcko closed an issue: "Bitcoin Core GUI getrawtransaction implementation"
(https://github.com/bitcoin-core/gui/issues/645)

💬 maflcko commented on issue "Bitcoin Core GUI getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/issues/645#issuecomment-2676217437)
The feature request didn't seem to attract much attention in the past. Thus, closing due to lack of interest, progress and direction.

Pull requests with improvements are always welcome. Moreover, it is possible to re-open this issue or create a new issue referencing it, if there is fresh interest.

🤔 rkrux reviewed a pull request: "descriptor: check whitespace in keys within fragments"
(https://github.com/bitcoin/bitcoin/pull/31603#pullrequestreview-2634807894)
Re: https://github.com/bitcoin/bitcoin/pull/31603#issuecomment-2675329745

My CLI command called the regtest node in which those mainnet valid private keys failed to parse, which is correct behaviour. I tried getting that descriptor info in the mainnet and was able to do so in master, which is erroneous behaviour, and I see the newly added error in this branch when I try to get the descriptor info or import it.

I noticed that I could import a space containing descriptor in master but when
...

💬 rkrux commented on pull request "descriptor: check whitespace in keys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#discussion_r1966518694)
In case you retouch

`contains a whitespace`

💬 rkrux commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#issuecomment-2676235043)
https://github.com/bitcoin/bitcoin/pull/31603#pullrequestreview-2634807894

Not exactly what this PR is fixing but I unintentionally ended up trying network inconsistent keys in my node and faced a not so detailed error, which took some debugging to figure out. So I'm definitely in favour of better error parsing like this PR intends to do.

🤔 hodlinator reviewed a pull request: "contrib: Add deterministic-unittest-coverage"
(https://github.com/bitcoin/bitcoin/pull/31901#pullrequestreview-2634421646)
Concept ACK fafce12a4871ed66a868b460b779e794a89218eb

💬 hodlinator commented on pull request "contrib: Add deterministic-unittest-coverage":
(https://github.com/bitcoin/bitcoin/pull/31901#discussion_r1966329661)
Current output on `!success()` due to use of `assert!` for other things than logical invariants:
```
₿ cargo run --release ~/b2/build miniscript
Compiling deterministic-unittest-coverage v0.1.0 (/home/hodlinator/b2/contrib/devtools/deterministic-unittest-coverage)
Finished `release` profile [optimized] target(s) in 0.36s
Running `target/release/deterministic-unittest-coverage /home/hodlinator/b2/build miniscript`
Test setup error: no test cases matching filter or all test cases
...

💬 hodlinator commented on pull request "contrib: Add deterministic-unittest-coverage":
(https://github.com/bitcoin/bitcoin/pull/31901#discussion_r1966311321)
One write to *stderr* should suffice? Also `{err}` when we can. (Think I've seen discouragement of `std::endl` in C++ land in favor of `\n`, this felt similar).
```suggestion
eprintln!(
"Error: {err}\n\
\n\
Usage: program ./build_dir ./qa-assets/fuzz_corpora fuzz_target_name\n\
\n\
Refer to the devtools/README.md for more details."
);
```

💬 hodlinator commented on pull request "contrib: Add deterministic-unittest-coverage":
(https://github.com/bitcoin/bitcoin/pull/31901#discussion_r1966288594)
Detected diff between fuzz and unit test.
```suggestion
.expect("Failed to execute git command")
```

💬 hodlinator commented on pull request "contrib: Add deterministic-unittest-coverage":
(https://github.com/bitcoin/bitcoin/pull/31901#discussion_r1966332995)
For the sake of easy copy-paste when viewing the .md file directly within editor:
````suggestion
To execute the tool, compilation has to be done with the build options:
```
-DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DCMAKE_CXX_FLAGS='-fprofile-instr-generate -fcoverage-mapping'
```
Both llvm-profdata and llvm-cov must be installed.
````
Same for the fuzz-version.

🤔 hodlinator reviewed a pull request: "http: Make server shutdown more robust"
(https://github.com/bitcoin/bitcoin/pull/31929#pullrequestreview-2634894126)
The [fuzz-failure](https://github.com/bitcoin/bitcoin/pull/31929#issuecomment-2674904296) was me underestimating a default-initialized argument. Fixed in 2nd push.

💬 hodlinator commented on pull request "http: Make server shutdown more robust":
(https://github.com/bitcoin/bitcoin/pull/31929#discussion_r1966534152)
To verify the improved accuracy of the documentation here one can apply:
```diff
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 52fa8463d2..a3efaa2579 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -557,6 +557,10 @@ void StopHTTPServer()
// Schedule a callback to call evhttp_free in the event base thread, as
// otherwise eventHTTP often keeps internal events alive, meaning
// aforementioned thread would never run out of events and exit
...

💬 mabu44 commented on pull request "test: chain reorg for coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/31885#issuecomment-2676290001)
@fjahr I don't think my new proposed test actually has any advantages over the existing functional one, so I'm converting the PR to draft. I'll evaluate whether to approach the coverage increase differently.

📝 mabu44 converted_to_draft a pull request: "test: chain reorg for coinstatsindex"
(https://github.com/bitcoin/bitcoin/pull/31885)
Improved coverage of src/index/coinstatsindex.cpp with a new unit test that simulate a reorg of the chain (one block deep).