💬 maflcko commented on issue "Running out of memory on a 2GB box - Initializing chainstate Chainstate [ibd] @ height -1 (null)":
(https://github.com/bitcoin/bitcoin/issues/31573#issuecomment-2885831745)
26.0 is EOL. Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?
Did you compile a debug build? Are you using the release bins?
(https://github.com/bitcoin/bitcoin/issues/31573#issuecomment-2885831745)
26.0 is EOL. Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?
Did you compile a debug build? Are you using the release bins?
👍 hebasto approved a pull request: "build: Revert "Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC"
(https://github.com/bitcoin/bitcoin/pull/32525#pullrequestreview-2845701454)
ACK fa0a4473a88017e525829ecd3db47f536c35fb62.
Should a note about the broken MSVC 17.12 be added to the build docs?
(https://github.com/bitcoin/bitcoin/pull/32525#pullrequestreview-2845701454)
ACK fa0a4473a88017e525829ecd3db47f536c35fb62.
Should a note about the broken MSVC 17.12 be added to the build docs?
💬 hebasto commented on pull request "cmake: Restrict MSVC-specific workaround to affected versions":
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2885835138)
Closing in favour of https://github.com/bitcoin/bitcoin/pull/32525.
(https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2885835138)
Closing in favour of https://github.com/bitcoin/bitcoin/pull/32525.
✅ hebasto closed a pull request: "cmake: Restrict MSVC-specific workaround to affected versions"
(https://github.com/bitcoin/bitcoin/pull/32499)
(https://github.com/bitcoin/bitcoin/pull/32499)
💬 maflcko commented on pull request "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`":
(https://github.com/bitcoin/bitcoin/pull/32432#issuecomment-2885841787)
> `std::string_view` is known (and usable) at compile time as a view over some static data (it can be constexpr), so it can be concatenated to RPC docs. [1]
Also, I don't think this is true. I don't think it is possible in C++20 to concatenate two string_view into a single string_view at compile time.
(https://github.com/bitcoin/bitcoin/pull/32432#issuecomment-2885841787)
> `std::string_view` is known (and usable) at compile time as a view over some static data (it can be constexpr), so it can be concatenated to RPC docs. [1]
Also, I don't think this is true. I don't think it is possible in C++20 to concatenate two string_view into a single string_view at compile time.
💬 davidgumberg commented on pull request "build: Revert "Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC":
(https://github.com/bitcoin/bitcoin/pull/32525#issuecomment-2885848315)
ACK https://github.com/bitcoin/bitcoin/pull/32525/commits/fa0a4473a88017e525829ecd3db47f536c35fb62.
I would be partial to disabling this for MSVC 17.12 in the vein of #32499, I mean if it is known to be broken, and it's documented that it's broken, one might as well just disable it?
But, edge case of an edge case, so this seems fine to me.
(https://github.com/bitcoin/bitcoin/pull/32525#issuecomment-2885848315)
ACK https://github.com/bitcoin/bitcoin/pull/32525/commits/fa0a4473a88017e525829ecd3db47f536c35fb62.
I would be partial to disabling this for MSVC 17.12 in the vein of #32499, I mean if it is known to be broken, and it's documented that it's broken, one might as well just disable it?
But, edge case of an edge case, so this seems fine to me.
💬 davidgumberg commented on pull request "build: Revert "Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC":
(https://github.com/bitcoin/bitcoin/pull/32525#issuecomment-2885854792)
reACK https://github.com/bitcoin/bitcoin/commit/fa0a4473a88017e525829ecd3db47f536c35fb62
(https://github.com/bitcoin/bitcoin/pull/32525#issuecomment-2885854792)
reACK https://github.com/bitcoin/bitcoin/commit/fa0a4473a88017e525829ecd3db47f536c35fb62
📝 maflcko opened a pull request: "fuzz: Delete wallet_notifications"
(https://github.com/bitcoin/bitcoin/pull/32526)
The fuzz target has many issues:
* I has never found a meaningful issue.
* It is still slow, despite https://github.com/bitcoin/bitcoin/pull/28933 and https://github.com/bitcoin/bitcoin/pull/31238.
* It is unmaintained, see https://github.com/bitcoin/bitcoin/pull/28882#issuecomment-1814654792 (missing meaningful coverage) or https://github.com/bitcoin/bitcoin/pull/31238#issuecomment-2460821784 (unstable) or https://github.com/bitcoin/bitcoin/pull/31467#issuecomment-2672649759 (fix slowness)
...
(https://github.com/bitcoin/bitcoin/pull/32526)
The fuzz target has many issues:
* I has never found a meaningful issue.
* It is still slow, despite https://github.com/bitcoin/bitcoin/pull/28933 and https://github.com/bitcoin/bitcoin/pull/31238.
* It is unmaintained, see https://github.com/bitcoin/bitcoin/pull/28882#issuecomment-1814654792 (missing meaningful coverage) or https://github.com/bitcoin/bitcoin/pull/31238#issuecomment-2460821784 (unstable) or https://github.com/bitcoin/bitcoin/pull/31467#issuecomment-2672649759 (fix slowness)
...
💬 hodlinator commented on pull request "qa: feature_framework_startup_failures.py fixes & improvements (#30660 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/32509#discussion_r2092465291)
> whats the rationale behind multiplying by factor of 60?
Default timeout-factor is 1, so we get 60 seconds.
Here's a similar pre-existing example:
https://github.com/bitcoin/bitcoin/blob/c47f634718d4248fd2a30e51a57944f89da72a64/test/functional/test_framework/test_framework.py#L765-L773
> on master I dont think running the functional test with timeout-factor=99998 will trigger any failure.
99998 does fail feature_framework_startup_failures.py on master, but skipping back before th
...
(https://github.com/bitcoin/bitcoin/pull/32509#discussion_r2092465291)
> whats the rationale behind multiplying by factor of 60?
Default timeout-factor is 1, so we get 60 seconds.
Here's a similar pre-existing example:
https://github.com/bitcoin/bitcoin/blob/c47f634718d4248fd2a30e51a57944f89da72a64/test/functional/test_framework/test_framework.py#L765-L773
> on master I dont think running the functional test with timeout-factor=99998 will trigger any failure.
99998 does fail feature_framework_startup_failures.py on master, but skipping back before th
...
✅ maflcko closed an issue: "wallet: Data race in GetOrCreateLegacyScriptPubKeyMan vs IsMine"
(https://github.com/bitcoin/bitcoin/issues/27354)
(https://github.com/bitcoin/bitcoin/issues/27354)
💬 maflcko commented on issue "wallet: Data race in GetOrCreateLegacyScriptPubKeyMan vs IsMine":
(https://github.com/bitcoin/bitcoin/issues/27354#issuecomment-2885915250)
closing for now. unless there are steps to reproduce how a real user can be affected by this, it may be best to postpone this for now.
(https://github.com/bitcoin/bitcoin/issues/27354#issuecomment-2885915250)
closing for now. unless there are steps to reproduce how a real user can be affected by this, it may be best to postpone this for now.
👍 hodlinator approved a pull request: "build: Revert "Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC"
(https://github.com/bitcoin/bitcoin/pull/32525#pullrequestreview-2845791215)
ACK fa2c6623626719b338880f7bb097b902019d5956
Confirmed at least later patch-versions of 17.13 work here: https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2883487720
It's possible that the regression in 17.12 never made it to any of the patch-versions of 17.13.
(https://github.com/bitcoin/bitcoin/pull/32525#pullrequestreview-2845791215)
ACK fa2c6623626719b338880f7bb097b902019d5956
Confirmed at least later patch-versions of 17.13 work here: https://github.com/bitcoin/bitcoin/pull/32499#issuecomment-2883487720
It's possible that the regression in 17.12 never made it to any of the patch-versions of 17.13.
🚀 fanquake merged a pull request: "build: Revert "Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVC"
(https://github.com/bitcoin/bitcoin/pull/32525)
(https://github.com/bitcoin/bitcoin/pull/32525)
💬 ajtowns commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2885938653)
ACK 35bcd8eed38130445aef5ebe217ab42248fa6f18 -- only minor tweaks since previous review
[I wrote](https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2832143503):
> I don't think this PR will have any impact on fee estimation (the only way that would occur is if a large majority of the mempool were transactions with multiple OP_RETURN outputs or an OP_RETURN output larger than 83 bytes, and that's not the case).
[gmaxwell wrote](https://github.com/bitcoin/bitcoin/pull/32406#
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2885938653)
ACK 35bcd8eed38130445aef5ebe217ab42248fa6f18 -- only minor tweaks since previous review
[I wrote](https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2832143503):
> I don't think this PR will have any impact on fee estimation (the only way that would occur is if a large majority of the mempool were transactions with multiple OP_RETURN outputs or an OP_RETURN output larger than 83 bytes, and that's not the case).
[gmaxwell wrote](https://github.com/bitcoin/bitcoin/pull/32406#
...
🚀 fanquake merged a pull request: "qt, wallet: Convert uint256 to Txid"
(https://github.com/bitcoin/bitcoin/pull/32238)
(https://github.com/bitcoin/bitcoin/pull/32238)
💬 fanquake commented on issue "test: failure in `mining_basic.py` AssertionError: not(4.656542373906924E-10 == 4.656542373906925E-10)":
(https://github.com/bitcoin/bitcoin/issues/32515#issuecomment-2885964963)
Yea. Have only seen it under Valgrind.
(https://github.com/bitcoin/bitcoin/issues/32515#issuecomment-2885964963)
Yea. Have only seen it under Valgrind.
💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2092562248)
Will do
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2092562248)
Will do
🚀 fanquake merged a pull request: "cmake: Add application manifests when cross-compiling for Windows"
(https://github.com/bitcoin/bitcoin/pull/32396)
(https://github.com/bitcoin/bitcoin/pull/32396)
💬 maflcko commented on pull request "ci: Enable feature_init and wallet_reorgsrestore in valgrind task":
(https://github.com/bitcoin/bitcoin/pull/32519#issuecomment-2886010555)
If someone wants to quickly test this outside the CI system a minimal reproducer would be:
```
───────┬────────────────────────────────────────────────────────────────────────
│ File: /tmp/a.py
───────┼────────────────────────────────────────────────────────────────────────
1 │ import subprocess
2 │ import time
3 │
4 │ process = subprocess.Popen(["bash", "/tmp/a.sh"])
5 │ print("Wait for the first print to happen ...")
6 │ time.sleep(0.1)
7
...
(https://github.com/bitcoin/bitcoin/pull/32519#issuecomment-2886010555)
If someone wants to quickly test this outside the CI system a minimal reproducer would be:
```
───────┬────────────────────────────────────────────────────────────────────────
│ File: /tmp/a.py
───────┼────────────────────────────────────────────────────────────────────────
1 │ import subprocess
2 │ import time
3 │
4 │ process = subprocess.Popen(["bash", "/tmp/a.sh"])
5 │ print("Wait for the first print to happen ...")
6 │ time.sleep(0.1)
7
...
💬 fanquake commented on pull request "depends: bump to latest config.guess and config.sub":
(https://github.com/bitcoin/bitcoin/pull/32505#discussion_r2092584341)
I guess this will allow someone to try and build depends with an LLVM libc target. i.e:
```bash
# master
make -C depends/ HOST=aarch64-unknown-linux-llvm
make: Entering directory '/root/ci_scratch/depends'
Invalid configuration 'aarch64-unknown-linux-llvm': OS 'llvm' not recognized
```
```bash
# this branch
make -C depends/ HOST=aarch64-unknown-linux-llvm
make: Entering directory '/root/ci_scratch/depends'
Extracting native_qt..
```
(https://github.com/bitcoin/bitcoin/pull/32505#discussion_r2092584341)
I guess this will allow someone to try and build depends with an LLVM libc target. i.e:
```bash
# master
make -C depends/ HOST=aarch64-unknown-linux-llvm
make: Entering directory '/root/ci_scratch/depends'
Invalid configuration 'aarch64-unknown-linux-llvm': OS 'llvm' not recognized
```
```bash
# this branch
make -C depends/ HOST=aarch64-unknown-linux-llvm
make: Entering directory '/root/ci_scratch/depends'
Extracting native_qt..
```