Bitcoin Core Github
45 subscribers
118K links
Download Telegram
💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2388637501)
Is this TODO still relevant? I'm planning to look more in-depth at c14a4bcbb1f84fb4776f43cbb918e7771164029b, but at least on a first rough glance I haven't discovered a scenario where it would be possible to run any of the three methods accessing this map (`FlatSigningProvider::{Set,Get}MuSig2SecNonce` and `FlatSigningProvider::DeleteMuSig2Session`) concurrently for the same `DescriptorScriptPubKeyMan`.
💬 ryanofsky commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3348090942)
Thanks! Output from LLDB shows what the problem is and suggests this should be fixed by the patch in #33387, which I will go ahead and make into a PR.

In the stack trace (which I annotated below):

- Thread 1 is the bitcoin node main thread, and it is just waiting for the IPC event loop thread to exit.
- Thread 3 is IPC event loop thread, and it is idle waiting for new I/O events.
- Thread 38 is an IPC worker thread calling `WaitTipChanged`, which is stuck calling `kernel_notifications.m_tip_bl
...
💬 plebhash commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3348121335)
> I am still unsure why I couldn't reproduce this problem with the python tests I tried to write https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3330765943, and also why the problem did not seem to happen when you called stop RPC instead of using ctrl-c.

FWIW, here's the code I used to trigger this. Like I mentioned previously, it's already outdated since I'm no longer making usage of `waitTipChanged` (and many other parts have also evolved).

https://github.com/plebhash/sv2-bitcoi
...
💬 stickies-v commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2388680046)
Oh, right, I was confused about the apple clang version indeed.

Since Xcode 15 satisfies our minimum macOS and clang versions, I'm not sure why we're bumping it here? The above docstring states:

> # Use the earliest Xcode supported by the version of macOS denoted in
> # doc/release-notes-empty-template.md and providing at least the
> # minimum clang version denoted in doc/dependencies.md.

It seems like the documentation is at conflict with the Xcode versi
...
💬 ryanofsky commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3348136502)
Thanks! I will try that out
👍 theStack approved a pull request: "test: set par=2 in default config for functional test framework"
(https://github.com/bitcoin/bitcoin/pull/33485#pullrequestreview-3280960110)
ACK dda5228e02ca6a839bf87ae7dbd133547563816a
📝 MichalMogilski opened a pull request: "Create base"
(https://github.com/bitcoin/bitcoin/pull/33501)
fix

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that
...
📝 23Konradbase opened a pull request: "Create base contract"
(https://github.com/bitcoin/bitcoin/pull/33502)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 23Konradbase commented on pull request "Create base contract":
(https://github.com/bitcoin/bitcoin/pull/33502#issuecomment-3348224788)
good job
💬 MichalMogilski commented on pull request "Create base":
(https://github.com/bitcoin/bitcoin/pull/33501#issuecomment-3348224837)
nice one
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2388743389)
I'm a bit conflicted here: this way we're all measuring something slightly different - which is especially problematic since the work here isn't even CPU bound. What if we did a min of npcu and 4?
💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2388733196)
I think we should add exact types here to make sure calculations like `end_index - local_batch_size` can't underflow
💬 kevkevinpal commented on pull request "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing":
(https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3348307035)
> > For me it is just weird to see the script failing here since creating a file or dir shouldn't fail
>
> It actually should fail. The goal of the check is to verify no mainnet datadir access is happening from any of the tests. Maybe the comment could be clarified to mention this?
>
> (Disabling this check would be better by just removing it)

gotcha that makes more sense to me now, I can open up PR modifying the comment to reflect that.
📝 kevkevinpal opened a pull request: "doc: add clarifying comment that creating the .bitcoin file is to avoid any mainnet datadir access"
(https://github.com/bitcoin/bitcoin/pull/33503)
referenced from https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3347684298

This adds a clarifying comment to tell the reader that the creation of the `${HOME}/.bitcoin` file is to ensure that there is no mainnet datadir access happening from any of the tests
💬 kevkevinpal commented on pull request "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing":
(https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3348325661)
I opened this PR https://github.com/bitcoin/bitcoin/pull/33503
💬 kevkevinpal commented on pull request "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`":
(https://github.com/bitcoin/bitcoin/pull/31823#issuecomment-3348355115)
reACK [43c2613](https://github.com/bitcoin/bitcoin/pull/31823/commits/43c2613305697255d294d9dd02f1cd19246913c9)
💬 mzumsande commented on pull request "wallet: reduce unconditional logging during load":
(https://github.com/bitcoin/bitcoin/pull/33299#issuecomment-3348361581)
Changed to approach by @furszy which removes the logging from the sqlite ctor similar as suggested by stickies-v, and should also pass the linter.

I'm only trying to remove the repeated logging here, no idea if the `VerifyWallets()` / `g_sqlite_count` interactions that the discussion here revealed are worth fixing - something for another PR or Issue perhaps.
👋 mzumsande's pull request is ready for review: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299)
💬 ryanofsky commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3348387503)
That worked! Am able to reproduce with `gdb -ex run --args build/bin/bitcoin -m node -ipcbind=unix -regtest` and `cargo run --example logger "$HOME/.bitcoin/regtest/node.sock"` and `killall bitcoin-node`.

Then I see a hang with same stack trace that you posted.

If I run `bitcoin-cli -regtest stop` instead of `killall` there is no problem and shutdown works.

Patch in #33387 does not seem sufficient to fix the problem and does not seem to stop the hang or change the stack trace. Apparently it d
...
💬 fanquake commented on pull request "ci: add libcpp hardening flags to macOS fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33462#discussion_r2388883509)
Rebased on #33489.