💬 fjahr commented on pull request "test: Avoid intermittent failures in feature_init":
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806842091)
> Jup, that is what I mean when I say we don't want to pull in a leveldb dependency into this test, unless there is a reason. leveldb has different code paths depending on the operating system. You are using macOS and I was using Linux. I presume you'll be able to reproduce if you also use Linux. Alternatively, you can try to run the test in a loop on macOS, but I can't do that, because I don't have macOS.
<details>
<summary>I couldn't reproduce the failure on Ubuntu 20.04 either.</summary>
...
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806842091)
> Jup, that is what I mean when I say we don't want to pull in a leveldb dependency into this test, unless there is a reason. leveldb has different code paths depending on the operating system. You are using macOS and I was using Linux. I presume you'll be able to reproduce if you also use Linux. Alternatively, you can try to run the test in a loop on macOS, but I can't do that, because I don't have macOS.
<details>
<summary>I couldn't reproduce the failure on Ubuntu 20.04 either.</summary>
...
🚀 fanquake merged a pull request: "test: fix node index bug when comparing peerinfo"
(https://github.com/bitcoin/bitcoin/pull/28849)
(https://github.com/bitcoin/bitcoin/pull/28849)
💬 maflcko commented on pull request "test: Avoid intermittent failures in feature_init":
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806853917)
Did you run the test in a loop on Linux?
So far two people reported local issues. There are also CI failures, for example https://cirrus-ci.com/task/5031659184062464?logs=ci#L4069
(This one also shows a memory leak due to the corruption, so that'd be another reason to not fiddle with levedb files, as it will cause an CI error, even if Bitcoin Core shut down with the correct error message)
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806853917)
Did you run the test in a loop on Linux?
So far two people reported local issues. There are also CI failures, for example https://cirrus-ci.com/task/5031659184062464?logs=ci#L4069
(This one also shows a memory leak due to the corruption, so that'd be another reason to not fiddle with levedb files, as it will cause an CI error, even if Bitcoin Core shut down with the correct error message)
📝 pablomartin4btc opened a pull request: "script, assumeutxo: Enhance validations in utxo_snapshot.sh"
(https://github.com/bitcoin/bitcoin/pull/28852)
This PR resolves #27841 and some more:
- Ensure that the snapshot height is higher than the pruned block height when the node is pruned (Suggested by @Sjors [here](https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1804941396)).
- Validate the correctness of the file path and check if the file already exists (@hazeycode's #27845).
- Make network activity disablement optional for the user (Suggested by @Sjors ([here](https://github.com/bitcoin/bitcoin/pull/16899#discussion_r342
...
(https://github.com/bitcoin/bitcoin/pull/28852)
This PR resolves #27841 and some more:
- Ensure that the snapshot height is higher than the pruned block height when the node is pruned (Suggested by @Sjors [here](https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1804941396)).
- Validate the correctness of the file path and check if the file already exists (@hazeycode's #27845).
- Make network activity disablement optional for the user (Suggested by @Sjors ([here](https://github.com/bitcoin/bitcoin/pull/16899#discussion_r342
...
💬 fanquake commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28851#issuecomment-1806856533)
Concept ACK. Looks better than the previous two changes, and it's clearer what's being done. Picked it into #28622.
(https://github.com/bitcoin/bitcoin/pull/28851#issuecomment-1806856533)
Concept ACK. Looks better than the previous two changes, and it's clearer what's being done. Picked it into #28622.
💬 maflcko commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1806857272)
re-ACK 4dd94ca18f6fc843137ffca3e6d3e97e4f19377b 👝
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 4dd94ca18f6fc843137f
...
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1806857272)
re-ACK 4dd94ca18f6fc843137ffca3e6d3e97e4f19377b 👝
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 4dd94ca18f6fc843137f
...
💬 hebasto commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806857458)
The PR description looks outdated after the recent push :)
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806857458)
The PR description looks outdated after the recent push :)
💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806857645)
Going to wait and see if it actually works first.
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806857645)
Going to wait and see if it actually works first.
💬 hebasto commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806858195)
Is new macOS SDK available at https://bitcoincore.org/depends-sources/sdks/?
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806858195)
Is new macOS SDK available at https://bitcoincore.org/depends-sources/sdks/?
💬 fjahr commented on pull request "test: Avoid intermittent failures in feature_init":
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806858917)
So I was actually able to reproduce this by just hardcoding the max values of the random range:
```
tf.seek(15000)
tf.write(b'1' * 2000)
```
<details>
<summary>failure details</summary>
```
$ ./test/functional/feature_init.py
2023-11-11T15:46:50.985000Z TestFramework (INFO): PRNG seed is: 7272134511591997577
2023-11-11T15:46:50.986000Z TestFramework (INFO): Initializing test directory /var/folders/9z/n7rz_6cj3bq__11k5kcrsvvm0000gn/T/bitcoin
...
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806858917)
So I was actually able to reproduce this by just hardcoding the max values of the random range:
```
tf.seek(15000)
tf.write(b'1' * 2000)
```
<details>
<summary>failure details</summary>
```
$ ./test/functional/feature_init.py
2023-11-11T15:46:50.985000Z TestFramework (INFO): PRNG seed is: 7272134511591997577
2023-11-11T15:46:50.986000Z TestFramework (INFO): Initializing test directory /var/folders/9z/n7rz_6cj3bq__11k5kcrsvvm0000gn/T/bitcoin
...
💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806858950)
Nothing has been uploaded yet. I don't think there would be any real difference, but 15.0.1 has been released, and 15.1 is currently in beta 2, so we may pick a slightly different SDK.
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806858950)
Nothing has been uploaded yet. I don't think there would be any real difference, but 15.0.1 has been released, and 15.1 is currently in beta 2, so we may pick a slightly different SDK.
💬 hebasto commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806859650)
> Nothing has been uploaded yet. I don't think there would be any real difference, but 15.0.1 has been released, and 15.1 is currently in beta 2, so we may pick a slightly different SDK.
I mean, that CI fails due to not being able to load the SDK...
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806859650)
> Nothing has been uploaded yet. I don't think there would be any real difference, but 15.0.1 has been released, and 15.1 is currently in beta 2, so we may pick a slightly different SDK.
I mean, that CI fails due to not being able to load the SDK...
📝 pablomartin4btc converted_to_draft a pull request: "script, assumeutxo: Enhance validations in utxo_snapshot.sh"
(https://github.com/bitcoin/bitcoin/pull/28852)
This PR resolves #27841 and some more:
- Ensure that the snapshot height is higher than the pruned block height when the node is pruned (Suggested by @Sjors [here](https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1804941396)).
- Validate the correctness of the file path and check if the file already exists (@hazeycode's #27845).
- Make network activity disablement optional for the user (Suggested by @Sjors [here](https://github.com/bitcoin/bitcoin/pull/16899#discussion_r3427
...
(https://github.com/bitcoin/bitcoin/pull/28852)
This PR resolves #27841 and some more:
- Ensure that the snapshot height is higher than the pruned block height when the node is pruned (Suggested by @Sjors [here](https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1804941396)).
- Validate the correctness of the file path and check if the file already exists (@hazeycode's #27845).
- Make network activity disablement optional for the user (Suggested by @Sjors [here](https://github.com/bitcoin/bitcoin/pull/16899#discussion_r3427
...
💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806860428)
Yea, anyone that wants to test should create the new SDK, check the new shasum, and then do a Guix build. If that works, the CI will.
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806860428)
Yea, anyone that wants to test should create the new SDK, check the new shasum, and then do a Guix build. If that works, the CI will.
💬 fjahr commented on pull request "test: Avoid intermittent failures in feature_init":
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806862003)
> I haven't looked into this deeper yet but my best guess would be that ldb doesn't care about this last, big file and doesn't check it?
No, that's not it, otherwise the hardcoded test wouldn't fail. If we don't understand what leveldb is doing then I guess we should probably remove perturbing its files altogether.
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806862003)
> I haven't looked into this deeper yet but my best guess would be that ldb doesn't care about this last, big file and doesn't check it?
No, that's not it, otherwise the hardcoded test wouldn't fail. If we don't understand what leveldb is doing then I guess we should probably remove perturbing its files altogether.
💬 maflcko commented on pull request "test: Avoid intermittent failures in feature_init":
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806862608)
> If we don't understand what leveldb is doing then I guess we should probably remove perturbing its files altogether.
sgtm, for a follow-up or alternative pull. The goal of this pull is not to change any test, but only to restore the previous code, which didn't intermittently fail.
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806862608)
> If we don't understand what leveldb is doing then I guess we should probably remove perturbing its files altogether.
sgtm, for a follow-up or alternative pull. The goal of this pull is not to change any test, but only to restore the previous code, which didn't intermittently fail.
💬 fanquake commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28851#issuecomment-1806863219)
Guix build (aarch64):
```bash
d832f85daf44833b8bdb8fbf3947e0c6e7a1364cf33991e8f5c32a68e3321c3b guix-build-05aca093819b/output/arm64-apple-darwin/SHA256SUMS.part
6c163bfdb697ff4821e87645ad62bd1588c862cad07ec4ae901835e2368aba69 guix-build-05aca093819b/output/arm64-apple-darwin/bitcoin-05aca093819b-arm64-apple-darwin-unsigned.tar.gz
bc73c31b3b45588264685ee98449e8cb6eff8885b5cb696d782dbb95f1608ce6 guix-build-05aca093819b/output/arm64-apple-darwin/bitcoin-05aca093819b-arm64-apple-darwin-unsign
...
(https://github.com/bitcoin/bitcoin/pull/28851#issuecomment-1806863219)
Guix build (aarch64):
```bash
d832f85daf44833b8bdb8fbf3947e0c6e7a1364cf33991e8f5c32a68e3321c3b guix-build-05aca093819b/output/arm64-apple-darwin/SHA256SUMS.part
6c163bfdb697ff4821e87645ad62bd1588c862cad07ec4ae901835e2368aba69 guix-build-05aca093819b/output/arm64-apple-darwin/bitcoin-05aca093819b-arm64-apple-darwin-unsigned.tar.gz
bc73c31b3b45588264685ee98449e8cb6eff8885b5cb696d782dbb95f1608ce6 guix-build-05aca093819b/output/arm64-apple-darwin/bitcoin-05aca093819b-arm64-apple-darwin-unsign
...
💬 fjahr commented on pull request "test: Avoid intermittent failures in feature_init":
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806863721)
ACK 44445ae8f1123c3affdcc0dbd7b3830eff5548ef
(https://github.com/bitcoin/bitcoin/pull/28831#issuecomment-1806863721)
ACK 44445ae8f1123c3affdcc0dbd7b3830eff5548ef
👋 pablomartin4btc's pull request is ready for review: "script, assumeutxo: Enhance validations in utxo_snapshot.sh"
(https://github.com/bitcoin/bitcoin/pull/28852)
(https://github.com/bitcoin/bitcoin/pull/28852)
💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806868093)
Looks like we'll have to add a `-Wl,-platform_version` into ldflags here.
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1806868093)
Looks like we'll have to add a `-Wl,-platform_version` into ldflags here.