Bitcoin Core Github
44 subscribers
121K links
Download Telegram
⚠️ fanquake opened an issue: "cmake: searching across directories for config files"
(https://github.com/bitcoin/bitcoin/issues/32938)
This can be recreated on at least macOS, due to #32707.

If there are two checkouts of the source, the build in one directory, can fail to configure, because of files in the other directory:
```bash
git clone https://github.com/bitcoin/bitcoin/ bitcoin_a
git clone https://github.com/bitcoin/bitcoin/ bitcoin_b

cd bitcoin_a
make -C depends HOST=x86_64-w64-mingw32
# fails to build libevent (#32707)
cd ../bitcoin_b
# fails to configure, because of the libevent config files (for a different target)
...
💬 maflcko commented on issue "ci: don't pass GIT_EXEC_PATH inside test container":
(https://github.com/bitcoin/bitcoin/issues/32935#issuecomment-3057091584)
I think the possible solutions here are:

* Keep everything as-is and just let people figure out that they need to follow the docs exactly.
* Rename all env vars used internally in the ci to have some prefix like `CI_...` and then filter for those and only pass them. (easy-ish change, but still a breaking change)
* Wrap the ci invocation into an arg-parser (larger re-write, also a breaking change)
* something else?
💬 ryanofsky commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-3057186819)
Updated 1a87ed000e09b10c173ba87d4bf2f79bf63abfa5 -> 2910315d1d8a3879f4e0bd50bf90870b642bfede ([`pr/mine.26`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.26) -> [`pr/mine.27`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.27), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.26..pr/mine.27)) fixing unused python import lint error https://cirrus-ci.com/task/5421333066022912
💬 Sjors commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-3057188134)
ACK 2910315d1d8a3879f4e0bd50bf90870b642bfede
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-3057258422)
[Latest push](https://github.com/bitcoin/bitcoin/compare/e036ba4ddaeb24a2361c356f7f270ee60bfff984..64fbca986cbaf3990d15d26bf6336fa8116f1d56) contains (as usual, the second push is just a changeless rebase):
* `Obfuscation` constructor only accepts fixed-size span now - and is explicit
* fuzz test changes were moved earlier to fix per-commit tests and we're using `ConsumeFixedLengthByteVector` now instead of either a uint64_t or random length vector
* `Obfuscation::SIZE_BYTES` was originally n
...
🚀 fanquake merged a pull request: "bench: Avoid tmp files in pwd"
(https://github.com/bitcoin/bitcoin/pull/32890)
👍 Sjors approved a pull request: "ipc: Handle unclean shutdowns better"
(https://github.com/bitcoin/bitcoin/pull/32345#pullrequestreview-3005455760)
utACK 0a9a1940782fad8dc3494c4aacee0a06c030eb5c
💬 Sjors commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2197581718)
In f8fd3959d51f6f80b428828c4fd965e06a8fa19e _ipc: Use EventLoopRef instead of addClient/removeClient_: if you have to retouch it would be good to document `m_loop` and `m_loop_ref`.
📝 l0rinc opened a pull request: "refactor: avoid double hashing in `SourceLocationHasher`"
(https://github.com/bitcoin/bitcoin/pull/32939)
Feeds the file name string directly into the hasher instead of pre-hashing it first. The resulting hash changes of course, but that doesn't affect us since it's only stored in-memory.

Fixes: https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2188644325

A few other related nits were also addressed here:
* Comment was redundant, `CSipHasher` is used in other hashers all over the codebase
* `static_cast<size_t>` skewed the whole line, changed to shorter functional-style cast
* struc
...
💬 stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2197642119)
> "descriptors" documentation could be improved, just quickly wrote something down. E.g. not sure if order matters here, or if there's anything else to look out for.

We should probably just leave the name as "scanobjects" (instead of my change to "descriptors"), to avoid introducing backwards incompatible API changes. The description should be change-able without issue.
💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2197642575)
Fixed it in https://github.com/bitcoin/bitcoin/pull/32939
🤔 stickies-v reviewed a pull request: "refactor: avoid double hashing in `SourceLocationHasher`"
(https://github.com/bitcoin/bitcoin/pull/32939#pullrequestreview-3005632806)
Since there are quite a few follow-ups in #32604, it might make sense to do them all in one (or at least larger, related PRs) instead of carving it out into dozens of small ones?
💬 stickies-v commented on pull request "refactor: avoid double hashing in `SourceLocationHasher`":
(https://github.com/bitcoin/bitcoin/pull/32939#discussion_r2197694485)
As we've already discussed [here](https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2163614334), I don't think it would be an improvement to remove this comment. Using `CSipHasher` over `std::hash` is non-trivial and helped me understand the code.
💬 hebasto commented on issue "cmake: searching across directories for config files":
(https://github.com/bitcoin/bitcoin/issues/32938#issuecomment-3057410237)
Could you please confirm whether the following patch helps:
```diff
--- a/depends/patches/libevent/cmake_fixups.patch
+++ b/depends/patches/libevent/cmake_fixups.patch
@@ -7,7 +7,7 @@ cmake: set minimum version to 3.5
#

-cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
-+cmake_minimum_required(VERSION 3.5 FATAL_ERROR)
++cmake_minimum_required(VERSION 3.15 FATAL_ERROR)

if (POLICY CMP0054)
cmake_policy(SET CMP0054 NEW)
```
?
🤔 marcofleon reviewed a pull request: "p2p: improve TxOrphanage denial of service bounds"
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-3005543794)
The `protected_count == 0` assertion failed in `txorphan_protected`.
<details>
<summary>crash</summary>

```bash
../../../../src/test/fuzz/txorphan.cpp:395 operator(): Assertion `protected_count == 0' failed.
==2872074== ERROR: libFuzzer: deadly signal
#0 0x558b2f142c41 in __sanitizer_print_stack_trace (/root/bitcoin/txorphanfuzzbuild/bin/fuzz+0x1c92c41) (BuildId: 2ed4a047818a3d1ea39c53799e090813bde04dba)
#1 0x558b2f097c08 in fuzzer::PrintStackTrace() (/root/bitcoin/txorphanfuzzb
...
💬 marcofleon commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2197640228)
What's the point of `std::floor` here? Also I noticed this doesn't match the calculation in `GetLatencyScore()`. Is it meant to be an overestimate?
💬 marcofleon commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2197660618)
We increment by the latency score above but then decrement here by announcement count. These aren't guaranteed to be the same afaict, so this might be what's causing the failure of the `protected_count == 0` assertion.
l0rinc closed a pull request: "refactor: avoid double hashing in `SourceLocationHasher`"
(https://github.com/bitcoin/bitcoin/pull/32939)
💬 maflcko commented on pull request "refactor: Avoid copies by using const references or by move-construction":
(https://github.com/bitcoin/bitcoin/pull/31650#issuecomment-3057477770)
> Please update the PR description now that `readability-avoid-const-params-in-decls` was reapplied.

thx, done.



> Re-ran tidy as it looks like 20 will pickup more issues: https://cirrus-ci.com/task/5041587878625280?logs=ci#L1916:

thx, rebased.

Should be easy to re-review.
💬 fanquake commented on pull request "Enable `-Werror=dev` in CI & Guix":
(https://github.com/bitcoin/bitcoin/pull/32937#issuecomment-3057491388)
Guix Build:
```bash
c7870f6c402ac407d0de4edc21d42f5040d19374deaba50b496e4f99a421c8e8 guix-build-35e0331493e9/output/aarch64-linux-gnu/SHA256SUMS.part
d64532e1bd83568f2386a6b67c49e0843f08c0d486828b1cd0ce955a179b07bb guix-build-35e0331493e9/output/aarch64-linux-gnu/bitcoin-35e0331493e9-aarch64-linux-gnu-debug.tar.gz
a158a955bf7b82ef1572db4ad5fd4d5b2885631cad810102b67d16eaadb4e3df guix-build-35e0331493e9/output/aarch64-linux-gnu/bitcoin-35e0331493e9-aarch64-linux-gnu.tar.gz
225421a4d478e602
...