💬 Christewart commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3135891423)
Alternatively I believe 29.1 is publishing RC's, maybe it could be documented in that release with a note about how this change wasn't documented in the 29.0 release? 🤷♂️
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3135891423)
Alternatively I believe 29.1 is publishing RC's, maybe it could be documented in that release with a note about how this change wasn't documented in the 29.0 release? 🤷♂️
💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2242347967)
HI @achow101 can you please approve this PR
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2242347967)
HI @achow101 can you please approve this PR
💬 willcl-ark commented on pull request "ci: Only pass documented env vars":
(https://github.com/bitcoin/bitcoin/pull/33002#discussion_r2242331540)
Could be worth moving the comment from `02_run_container.sh` to here as this is where this format is first contructed.
```
# Append $USER to /tmp/env to support multi-user systems and $CONTAINER_NAME
# to allow support starting multiple runs simultaneously by the same user.
```
(https://github.com/bitcoin/bitcoin/pull/33002#discussion_r2242331540)
Could be worth moving the comment from `02_run_container.sh` to here as this is where this format is first contructed.
```
# Append $USER to /tmp/env to support multi-user systems and $CONTAINER_NAME
# to allow support starting multiple runs simultaneously by the same user.
```
🤔 willcl-ark reviewed a pull request: "ci: Only pass documented env vars"
(https://github.com/bitcoin/bitcoin/pull/33002#pullrequestreview-3070987599)
This seems ok to me.
Should we update https://github.com/bitcoin/bitcoin/blob/00604296e178b13fb1a701a84deac1e20e15259d/ci/README.md?plain=1#L27 and https://github.com/bitcoin/bitcoin/blob/00604296e178b13fb1a701a84deac1e20e15259d/ci/README.md?plain=1#L46 as we now don't pass `$HOME` or `$PATH` in?
(https://github.com/bitcoin/bitcoin/pull/33002#pullrequestreview-3070987599)
This seems ok to me.
Should we update https://github.com/bitcoin/bitcoin/blob/00604296e178b13fb1a701a84deac1e20e15259d/ci/README.md?plain=1#L27 and https://github.com/bitcoin/bitcoin/blob/00604296e178b13fb1a701a84deac1e20e15259d/ci/README.md?plain=1#L46 as we now don't pass `$HOME` or `$PATH` in?
📝 waketraindev opened a pull request: "qt: clear command history when clearing the console"
(https://github.com/bitcoin-core/gui/pull/882)
Clears the command history in the qt console when the user clears the console output.
(https://github.com/bitcoin-core/gui/pull/882)
Clears the command history in the qt console when the user clears the console output.
💬 ajtowns commented on pull request "Move `FreespaceChecker` class into its own module":
(https://github.com/bitcoin-core/gui/pull/881#issuecomment-3135937149)
ACK 4542412ae063903654fc1cd95f77889bae76d61c
nit: qt/intro.h forward decl of `class FreespaceChecker` is redundant since the header is included now.
(https://github.com/bitcoin-core/gui/pull/881#issuecomment-3135937149)
ACK 4542412ae063903654fc1cd95f77889bae76d61c
nit: qt/intro.h forward decl of `class FreespaceChecker` is redundant since the header is included now.
✅ fanquake closed a pull request: "qt: clear command history when clearing the console"
(https://github.com/bitcoin/bitcoin/pull/33098)
(https://github.com/bitcoin/bitcoin/pull/33098)
💬 hebasto commented on pull request "Move `FreespaceChecker` class into its own module":
(https://github.com/bitcoin-core/gui/pull/881#issuecomment-3135971708)
> nit: qt/intro.h forward decl of `class FreespaceChecker` is redundant since the header is included now.
Thanks! Fixed.
(https://github.com/bitcoin-core/gui/pull/881#issuecomment-3135971708)
> nit: qt/intro.h forward decl of `class FreespaceChecker` is redundant since the header is included now.
Thanks! Fixed.
📝 fanquake opened a pull request: "ci: allow for any libc++ intrumentation & use it for TSAN"
(https://github.com/bitcoin/bitcoin/pull/33099)
Allow for instrumenting libc++ with a sanitizer other than MemoryWithOrigins.
Would also close #33087, as with the extra instrumentation, the issue from https://github.com/bitcoin/bitcoin/pull/32862#issuecomment-3114706601 is avoided (also see https://github.com/bitcoin/bitcoin/pull/33081), and we can drop `DEBUG_LOCKORDER`.
(https://github.com/bitcoin/bitcoin/pull/33099)
Allow for instrumenting libc++ with a sanitizer other than MemoryWithOrigins.
Would also close #33087, as with the extra instrumentation, the issue from https://github.com/bitcoin/bitcoin/pull/32862#issuecomment-3114706601 is avoided (also see https://github.com/bitcoin/bitcoin/pull/33081), and we can drop `DEBUG_LOCKORDER`.
👋 fanquake's pull request is ready for review: "ci: allow for any libc++ intrumentation & use it for TSAN"
(https://github.com/bitcoin/bitcoin/pull/33099)
(https://github.com/bitcoin/bitcoin/pull/33099)
💬 fanquake commented on pull request "rpc: use CScheduler for relocking wallet and remove RPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32862#issuecomment-3135990566)
> the problem seems to be that libc++ itself isn't instrumented.
Following up on this in #33099.
(https://github.com/bitcoin/bitcoin/pull/32862#issuecomment-3135990566)
> the problem seems to be that libc++ itself isn't instrumented.
Following up on this in #33099.
📝 fanquake opened a pull request: "ci: remove `ninja-build` from MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/33100)
It is part of CI_BASE_PACKAGES.
(https://github.com/bitcoin/bitcoin/pull/33100)
It is part of CI_BASE_PACKAGES.
👍 hebasto approved a pull request: "ci: remove `ninja-build` from MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/33100#pullrequestreview-3071150626)
ACK cab6736b701f203d6e823e1b5d619368d8d4c5e0, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/33100#pullrequestreview-3071150626)
ACK cab6736b701f203d6e823e1b5d619368d8d4c5e0, I have reviewed the code and it looks OK.
💬 ajtowns commented on pull request "Move `FreespaceChecker` class into its own module":
(https://github.com/bitcoin-core/gui/pull/881#issuecomment-3136166736)
ACK 3a03f075606b19e411b8bd19870242e0e0b58fcb
(https://github.com/bitcoin-core/gui/pull/881#issuecomment-3136166736)
ACK 3a03f075606b19e411b8bd19870242e0e0b58fcb
📝 hebasto opened a pull request: "cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED`"
(https://github.com/bitcoin/bitcoin/pull/33101)
The `SECP256K1_DISABLE_SHARED` CMake variable has been [removed](https://github.com/bitcoin-core/secp256k1/pull/1688) upstream.
This PR removes its usage ahead of the next `secp256k1` subtree update to prevent breakage and simplify integration.
(https://github.com/bitcoin/bitcoin/pull/33101)
The `SECP256K1_DISABLE_SHARED` CMake variable has been [removed](https://github.com/bitcoin-core/secp256k1/pull/1688) upstream.
This PR removes its usage ahead of the next `secp256k1` subtree update to prevent breakage and simplify integration.
📝 brunoerg opened a pull request: "fuzz: cover BanMan::IsDiscouraged"
(https://github.com/bitcoin/bitcoin/pull/33102)
This PR adds fuzz coverage for the `IsDiscouraged` function in the banman target. This is the only function missing from `BanMan`.
(https://github.com/bitcoin/bitcoin/pull/33102)
This PR adds fuzz coverage for the `IsDiscouraged` function in the banman target. This is the only function missing from `BanMan`.
💬 maflcko commented on pull request "fuzz: cover BanMan::IsDiscouraged":
(https://github.com/bitcoin/bitcoin/pull/33102#issuecomment-3136269998)
lgtm ACK c2ed576d2caf282a51aa9df5df55d844ab1da794
(https://github.com/bitcoin/bitcoin/pull/33102#issuecomment-3136269998)
lgtm ACK c2ed576d2caf282a51aa9df5df55d844ab1da794
💬 TheBlueMatt commented on pull request "[POC] wallet: Add Support for BIP-353 DNS-Based Bitcoin Address via External Resolver":
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3136293939)
> The multiprocess binaries are produced by this project, built using the same deterministic build system, using the same code as the monolithic binaries. There are already concrete ways that the binaries can be distributed with minimal changes to our own release process. Users who run the multiprocess binaries don't have to do any special configuration for them to work.
Right, I was ancitipating that, ultimately, an external-process DNS resolution thing would be built and dstributed the same
...
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3136293939)
> The multiprocess binaries are produced by this project, built using the same deterministic build system, using the same code as the monolithic binaries. There are already concrete ways that the binaries can be distributed with minimal changes to our own release process. Users who run the multiprocess binaries don't have to do any special configuration for them to work.
Right, I was ancitipating that, ultimately, an external-process DNS resolution thing would be built and dstributed the same
...
💬 maflcko commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#discussion_r2242662552)
```suggestion
export USE_INSTRUMENTED_LIBCPP="Thread"
```
Can flatten the option into a single one?
Also, adjust the check below to
```sh
if [ -n "${USE_INSTRUMENTED_LIBCPP}" ]; then
(https://github.com/bitcoin/bitcoin/pull/33099#discussion_r2242662552)
```suggestion
export USE_INSTRUMENTED_LIBCPP="Thread"
```
Can flatten the option into a single one?
Also, adjust the check below to
```sh
if [ -n "${USE_INSTRUMENTED_LIBCPP}" ]; then
💬 maflcko commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#issuecomment-3136330481)
lgtm, but the tsan depends build seems to be taking significantly longer now for some reason?
(https://github.com/bitcoin/bitcoin/pull/33099#issuecomment-3136330481)
lgtm, but the tsan depends build seems to be taking significantly longer now for some reason?