📝 hodlinator opened a pull request: "build: Clean lingering Windows registry & shortcuts (#32132 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/33422)
Prior to fb2b05b1259d3e69e6e675adfa30b429424c7625 / #32132 we installed using these paths. The lingering registry entries for the uninstaller would show up as "Bitcoin Core (64-bit)" in the list of installed programs and fail to work.
Disclaimer: not an NSIS expert - confirmed that added commands work both when items exist and when they don't.
(https://github.com/bitcoin/bitcoin/pull/33422)
Prior to fb2b05b1259d3e69e6e675adfa30b429424c7625 / #32132 we installed using these paths. The lingering registry entries for the uninstaller would show up as "Bitcoin Core (64-bit)" in the list of installed programs and fail to work.
Disclaimer: not an NSIS expert - confirmed that added commands work both when items exist and when they don't.
💬 hodlinator commented on pull request "build: Remove bitness suffix from Windows installer":
(https://github.com/bitcoin/bitcoin/pull/32132#issuecomment-3305995368)
My testing shows #33422 does the job. I'd say we should have it in the v30 release or we should do the less risky thing and revert this PR (#32132) from v30.
(https://github.com/bitcoin/bitcoin/pull/32132#issuecomment-3305995368)
My testing shows #33422 does the job. I'd say we should have it in the v30 release or we should do the less risky thing and revert this PR (#32132) from v30.
💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2357988037)
The Tidy CI job runs IWYU and doesn't complain: https://github.com/bitcoin/bitcoin/actions/runs/17605378583/job/50015040929?pr=33135#step:9:6218
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2357988037)
The Tidy CI job runs IWYU and doesn't complain: https://github.com/bitcoin/bitcoin/actions/runs/17605378583/job/50015040929?pr=33135#step:9:6218
💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2357991533)
I went for a slight variant.
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2357991533)
I went for a slight variant.
💬 Sjors commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2358007567)
That might be a case of #32821. I would just wait until that's merged.
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2358007567)
That might be a case of #32821. I would just wait until that's merged.
📝 hodlinator opened a pull request: "qa: Improvements to debug_assert_log + busy_wait_for_debug_log"
(https://github.com/bitcoin/bitcoin/pull/33423)
* Only compute the log string to be printed on failure *when we actually fail* instead of every 0.05s.
* As we find each needle (expected message) in the haystack (log output), stop searching for it. If we fail and time out, we will only complain about the needles (expected messages) we didn't find.
Found while developing a new test case in https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2351892330
(https://github.com/bitcoin/bitcoin/pull/33423)
* Only compute the log string to be printed on failure *when we actually fail* instead of every 0.05s.
* As we find each needle (expected message) in the haystack (log output), stop searching for it. If we fail and time out, we will only complain about the needles (expected messages) we didn't find.
Found while developing a new test case in https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2351892330
💬 willcl-ark commented on pull request "ci: Skip Centos job on GHA runners":
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3306304341)
> Looks fine. I don't have strong feelings, and wouldn't NACK this, but my preference leans more towards having things still run on forks on GHA runners as it does for the main org albeit slower. Also has the benefit of having a bit less vendor lock in.
I think if I was to e.g. append `|| true` to the commands in https://github.com/willcl-ark/bitcoin/blob/f5885d05160fe0ac167976f2b36b6d5bec373e08/.github/actions/clear-files/action.yml to make it "fail-proof" (i.e. don't error on failures) then
...
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3306304341)
> Looks fine. I don't have strong feelings, and wouldn't NACK this, but my preference leans more towards having things still run on forks on GHA runners as it does for the main org albeit slower. Also has the benefit of having a bit less vendor lock in.
I think if I was to e.g. append `|| true` to the commands in https://github.com/willcl-ark/bitcoin/blob/f5885d05160fe0ac167976f2b36b6d5bec373e08/.github/actions/clear-files/action.yml to make it "fail-proof" (i.e. don't error on failures) then
...
💬 fanquake commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3306477254)
> Here is a branch to do this: https://github.com/glozow/bitcoin/tree/2025-09-28.3-backport
Thanks. Looks like there are some issues with this branch. i.e:
```bash
test/mempool_tests.cpp:434: Entering test case "MempoolSizeLimitTest"
<snip>
test/mempool_tests.cpp:562: error: in "mempool_tests/MempoolSizeLimitTest": check pool.GetMinFee(1).GetFeePerK() == maxFeeRateRemoved.GetFeePerK() + 1000 has failed [19941 != 20841]
2025-09-18T09:27:52.459923Z (mocktime: 1970-01-01T12:00:42Z) [test] [
...
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3306477254)
> Here is a branch to do this: https://github.com/glozow/bitcoin/tree/2025-09-28.3-backport
Thanks. Looks like there are some issues with this branch. i.e:
```bash
test/mempool_tests.cpp:434: Entering test case "MempoolSizeLimitTest"
<snip>
test/mempool_tests.cpp:562: error: in "mempool_tests/MempoolSizeLimitTest": check pool.GetMinFee(1).GetFeePerK() == maxFeeRateRemoved.GetFeePerK() + 1000 has failed [19941 != 20841]
2025-09-18T09:27:52.459923Z (mocktime: 1970-01-01T12:00:42Z) [test] [
...
👋 fanquake's pull request is ready for review: "ci: re-add Valgrind job to the CI"
(https://github.com/bitcoin/bitcoin/pull/33411)
(https://github.com/bitcoin/bitcoin/pull/33411)
💬 fanquake commented on pull request "ci: re-add Valgrind job to the CI":
(https://github.com/bitcoin/bitcoin/pull/33411#issuecomment-3306492286)
Adjusted to remove the conflict with #31425 and add context to the final commit.
(https://github.com/bitcoin/bitcoin/pull/33411#issuecomment-3306492286)
Adjusted to remove the conflict with #31425 and add context to the final commit.
💬 fanquake commented on pull request "test: Avoid interface_ipc.py Duplicate ID errors":
(https://github.com/bitcoin/bitcoin/pull/33420#issuecomment-3306564783)
I also haven't reproduced the issue, using the steps provided. i.e installing libmulitprocess system-wide in a new VM, and then building Core & running `interface_ipc.py`. Can someone list the steps to reach the failure, using a clean VM.
(https://github.com/bitcoin/bitcoin/pull/33420#issuecomment-3306564783)
I also haven't reproduced the issue, using the steps provided. i.e installing libmulitprocess system-wide in a new VM, and then building Core & running `interface_ipc.py`. Can someone list the steps to reach the failure, using a clean VM.
📝 fanquake opened a pull request: "[30.0] Final changes + rc2"
(https://github.com/bitcoin/bitcoin/pull/33424)
Backports:
*
Finalise `v30.0rc2`
(https://github.com/bitcoin/bitcoin/pull/33424)
Backports:
*
Finalise `v30.0rc2`
💬 zaidmstrr commented on pull request "test: Avoid interface_ipc.py Duplicate ID errors":
(https://github.com/bitcoin/bitcoin/pull/33420#issuecomment-3306614491)
Here are the steps to reproduce the error in Ubuntu based systems:
1) Install `libmultiprocess` library system using `sudo make install`
2) Then install `capnp` system-wide. You can refer [this](https://capnproto.org/install.html) for the installation. After succesfully downloading the `capnp` library you can check using the `which capnp` command, which outputs the default location of the installed binary. In most of the cases the location was `/usr/local/bin/capnp`.
3) Now run the `interface
...
(https://github.com/bitcoin/bitcoin/pull/33420#issuecomment-3306614491)
Here are the steps to reproduce the error in Ubuntu based systems:
1) Install `libmultiprocess` library system using `sudo make install`
2) Then install `capnp` system-wide. You can refer [this](https://capnproto.org/install.html) for the installation. After succesfully downloading the `capnp` library you can check using the `which capnp` command, which outputs the default location of the installed binary. In most of the cases the location was `/usr/local/bin/capnp`.
3) Now run the `interface
...
💬 fanquake commented on pull request "test: Avoid interface_ipc.py Duplicate ID errors":
(https://github.com/bitcoin/bitcoin/pull/33420#issuecomment-3306666201)
Thanks, repro'd:
```bash
Remaining jobs: [interface_ipc.py]
1/1 - interface_ipc.py failed, Duration: 1 s
stdout:
2025-09-18T10:18:46.257357Z TestFramework (INFO): PRNG seed is: 5267162394658679552
2025-09-18T10:18:46.257836Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20250918_101846/interface_ipc_0
stderr:
terminate called after throwing an instance of 'kj::ExceptionImpl'
what(): mp/proxy.capnp:0: failed: Duplicate ID @0xcc316e3f71a040fb.
TES
...
(https://github.com/bitcoin/bitcoin/pull/33420#issuecomment-3306666201)
Thanks, repro'd:
```bash
Remaining jobs: [interface_ipc.py]
1/1 - interface_ipc.py failed, Duration: 1 s
stdout:
2025-09-18T10:18:46.257357Z TestFramework (INFO): PRNG seed is: 5267162394658679552
2025-09-18T10:18:46.257836Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20250918_101846/interface_ipc_0
stderr:
terminate called after throwing an instance of 'kj::ExceptionImpl'
what(): mp/proxy.capnp:0: failed: Duplicate ID @0xcc316e3f71a040fb.
TES
...
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2358417517)
Thanks, fixed.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2358417517)
Thanks, fixed.
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2358418317)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2358418317)
Fixed.
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2358419428)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2358419428)
Fixed.
🚀 fanquake merged a pull request: "test: Avoid interface_ipc.py Duplicate ID errors"
(https://github.com/bitcoin/bitcoin/pull/33420)
(https://github.com/bitcoin/bitcoin/pull/33420)
💬 fanquake commented on pull request "test: Avoid interface_ipc.py Duplicate ID errors":
(https://github.com/bitcoin/bitcoin/pull/33420#issuecomment-3306737091)
Backported to 30.x in #33424.
(https://github.com/bitcoin/bitcoin/pull/33420#issuecomment-3306737091)
Backported to 30.x in #33424.
💬 HowHsu commented on pull request "index: Fix missing case in the comment in NextSyncBlock()":
(https://github.com/bitcoin/bitcoin/pull/32875#issuecomment-3306769093)
> ```c++
> CBlockIndex* NextInclRewind(const CBlockIndex* pindex) const
> {
> if (!Contains(Assert(pindex))) {
> pindex = Assert(FindFork(pindex));
> }
> return (*this)[pindex->nHeight + 1];
> }
> ```
Hi Luke,
If I'm getting you wrong, `NextInclRewind()` will be a method member of CChain, but from my perspective, this function seems
tailored to a very specific scenario. To keep the class more general and lightweight, it might be better
...
(https://github.com/bitcoin/bitcoin/pull/32875#issuecomment-3306769093)
> ```c++
> CBlockIndex* NextInclRewind(const CBlockIndex* pindex) const
> {
> if (!Contains(Assert(pindex))) {
> pindex = Assert(FindFork(pindex));
> }
> return (*this)[pindex->nHeight + 1];
> }
> ```
Hi Luke,
If I'm getting you wrong, `NextInclRewind()` will be a method member of CChain, but from my perspective, this function seems
tailored to a very specific scenario. To keep the class more general and lightweight, it might be better
...